metal-rs icon indicating copy to clipboard operation
metal-rs copied to clipboard

Make function parameter names and order consistent with Apple's documentation

Open chinedufn opened this issue 5 years ago • 3 comments

In Apple's documentation, the order of function parameters might look like this:

func replace(region: MTLRegion, 
 mipmapLevel level: Int, 
   withBytes pixelBytes: UnsafeRawPointer, 
 bytesPerRow: Int)

whereas in metal-rs it might look like this

    pub fn replace_region(
        &self,
        region: MTLRegion,
        mipmap_level: NSUInteger,
        stride: NSUInteger,
        bytes: *const std::ffi::c_void,
    ) {
        // ...
    }

I found that the order differences (last two parameters swapped) and name differences (stride vs. bytes_per_row) made it difficult to cross reference Apple's Metal docs when working with metal-rs.

My understanding is that these were all written by hand over time (I'm just basing this assumption on the first few commits from 2016 https://github.com/gfx-rs/metal-rs/commits/master?after=09433e64682ffe4498988118c062e608a4971eb6+220)

Given that - it's probably quite a bit of manual work and breaking changes to get consistent (or at least to the degree possible) with the official metal APIs.

How does metal-rs typically think about breaking changes? Is this a no-go? Or could this fit in at some point in the future?

I could use some insight here.

Cheers!

chinedufn avatar May 02 '20 00:05 chinedufn

We are totally fine with breaking changes like that, but we adhere to semver like any other crate. Therefore, breaking changes will go into the breaking revision.

I agree in this case that we should rename the arg to bytes_per_row. There are other cases, which can be more complex, i.e. setBuffer in Metal accepts the slot in a non-obvious position. We can discuss all of them here.

kvark avatar May 02 '20 01:05 kvark

Sounds good thanks for explaining.

Sweet. I'll bring them up here on a case by case basis whenever I'm working with metal-rs and run into one.

chinedufn avatar May 02 '20 02:05 chinedufn

I aligned the texture API in 461148ed39a14e319992a2c1478d9a978cc32ffd

kvark avatar Jul 20 '20 14:07 kvark