Add support for RGBA format in GpuShaderCreator for better Metal support?

[wRosie]

Hi everyone,

Only single channel and RGB texture formats are currently supported in GpuShaderCreator::TextureType. However, it appears that Apple does not support simple RGB format textures in Metal. Please refer to https://developer.apple.com/documentation/metal/mtlpixelformat.

Working with OCIO shaders in Metal becomes inconvenient. When I query the GpuShaderDesc for a Metal shader and its textures, I sometimes get RGB textures that are not accepted by Metal. I have to insert a placeholder alpha channel to convert them to RGBA format.

Is it an intentional decision that OCIO does not support RGBA texture? If not, is it a plan for the future?

Thanks a lot!

[doug-walker]

@Morteeza, since you are our Metal expert, could you please offer some help with this question?

[remia]

Note there was a similar issue created in the past https://github.com/AcademySoftwareFoundation/OpenColorIO/issues/1685

[Morteeza]

As you mentioned, we don't have RGB texture support in Metal. You may have noticed we similarly insert placeholder alpha channel in OCIO metal app.mm, too.

I believe main reason we didn't add RGBA texture type was that, it will touch OCIO interface. So, maybe in future we can add that? @hodoulp do you think we can make this change in a major release?

[wRosie]

I am happy to help fix it. This will save us all from copying textures in memory. I agree it will touch the OCIO interface. But.. When I query for a metal shader, it makes more sense to get textures that work with it, right? Please consider doing it in a major release.

[doug-walker]

@wRosie , thank you for your offer to help, that would be great! This is something we could include in the OCIO 2.4 release this fall. I'd suggest starting by just proposing the API change, to get feedback, before doing the full implementation.

[wRosie]

Thanks @doug-walker. Here are some thoughts on interface change.

• Add TEXTURE_RGBA_CHANNEL value to enum TextureType

• Add a TextureType field to add3DTexture(). We could make the field optional, with a default value of TEXTURE_RGB_CHANNEL (the current behavior). This would avoid breaking existing function calls. However, I prefer adding the field to keep the signature consistent with addTexture().

• Add a TextureType& field to get3DTexture().

The addTexture()/getTexture() functions already take TextureType as a parameter, so I will not change their function signature. However, I'd like to update their behavior -- I’ll change GPUShaderCreator to use RGBA formats when GPULanguage is Metal. That involves updating functions such as GetLut1DGPUShaderProgram() and GetLut3DGPUShaderProgram(). Also, getTexture() and get3DTexture() will return textures in RGBA type in Metal mode.

I am not aware of other OCIO-supported shading languages that require RGBA format other than metal. If there is any, please point it out.

Please let me know if this makes sense to you, or if I missed anything. Thanks a lot!

[doug-walker]

@Morteeza, yes that's right, we couldn't modify the API at that time because of the release timing and the desire to release Metal support sooner rather than later.

@remia, thanks for linking to the other issue.

@wRosie, thank you, that sounds like an excellent proposal. I agree that it would be better to keep the signatures consistent and not use default arguments.

I think you are definitely on the right track, please proceed when you're ready!

[timeinpixels]

Thanks for refreshing my original request @wRosie - looking forward to having the change in the major release!