Bugfix: RGB/Hue Curve Performance with MSL

[carolalynn]

While implementing RGB and Hue Curves, noticed a severe performance drop in playback on edited curves vs. other OCIO ops. This fix is specific to Metal on MacOS, so I have already asked @Morteeza to take a look on my fork - fix confirmed.

The MSL class wrapper gave the generated struct owning copies of every array uniform, with a constructor that copied the used range out of constant memory into per-thread private memory and then zero-filled the remainder up to the full declared capacity. The struct is instantiated once per shader invocation, so a compute shader dispatched one thread per pixel paid that copy per pixel: for a dynamic GradingRGBCurve that is 8 + 120 + 8 + 360 = 496 private-memory writes for every pixel, regardless of how many knots the curve actually has.

Point at the constant memory instead. The array declarations arrive from the ops shared with the other GPU languages, so the wrapper rewrites them to constant pointers rather than changing the ops; it already knows which parameters are arrays. Indexing syntax is unchanged, so every read site in the generated shader is untouched, and the generated function signature is unchanged so existing MSL consumers keep working.

Dropping the zero-fill does not change any result. Reads of knots/coefs are bounded by the offset taken from the offsets array, and the offsets arrays are always fully populated -- GetNumOffsetValues() is a compile-time constant equal to the declared capacity (8 for RGB, 16 for hue) -- so no read past the used range was ever possible.

Measured with a Metal compute kernel over an RGBA16F 4K UHD frame, one thread per pixel, output texels bit-identical before and after:

rgb curve only 33.45 ms -> at the read+write bandwidth floor
luma curve only 35.65 ms -> at the floor
hue curve only 35.46 ms -> at the floor
rgb + luma + hue 115.28 ms -> at the floor

GradingRGBCurve and GradingHueCurve are the only ops that declare array uniforms; everything else passes scalars or textures and is unaffected.

MetalSupport9 pins the generated text, so its expected shader is updated. Three assertions are added alongside it stating the no-copy invariant on its own, so a future refresh of the expected text cannot quietly restore the per-pixel copy.

Assisted by: Claude / Opus 5

[doug-walker]

Looks like a nice improvement!

[doug-walker]

Is it true that "the struct is typically instantiated once per pixel"? Isn't it more typical that a group of pixels is processed in a batch, to amortize cost like this?

I'm all in favor of the change to improve performance for the single-pixel case, but I suspect it's not the "typical" situation.

[doug-walker]

Maybe this is a nit pick but it seems like kind of an odd test. The first two asserts are redundant after the comparison of the shader program above. And for the third, if we don't want people to change the code back in the future, perhaps just a comment where the original code is should suffice? If someone did try to return to the original approach, it would have to be written exactly the same in order to fail this test (e.g., maybe someone uses j rather than i for their loop)?