-
Notifications
You must be signed in to change notification settings - Fork 447
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for RGBA format in GpuShaderCreator for better Metal support? #1956
Comments
@Morteeza, since you are our Metal expert, could you please offer some help with this question? |
Note there was a similar issue created in the past #1685 |
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? |
I am happy to help fix it. This will save us all from copying textures in memory. |
@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. |
Thanks @doug-walker. Here are some thoughts on interface change.
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 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! |
@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! |
Thanks for refreshing my original request @wRosie - looking forward to having the change in the major release! |
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!
The text was updated successfully, but these errors were encountered: