Fix Vulkan texture binding index issue

[doug-walker]

Addresses defect #2225.

The binding index was only taking into account the number of textures of a given kind (1D or 3D) rather than the total number of textures.

[Copilot]

Pull request overview

This PR fixes a bug in Vulkan texture binding index calculation. Previously, binding indices were incorrectly calculated by counting only textures of the same dimension type (1D or 3D), when they should have been counting all textures regardless of type.

Key Changes:

• Updated texture index calculation to include both 1D and 3D textures in the count
• Added comprehensive test case validating correct binding index sequencing for mixed texture types

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File	Description
src/OpenColorIO/GpuShader.cpp	Fixed texture binding index calculation to count all texture types
tests/cpu/GpuShader_tests.cpp	Added test verifying correct Vulkan binding indices for multiple LUT types

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Shootfast]

LGTM!

[cozdas]

LGTM

[michaelHADSK]

Looks good but this way the index that is returned by addTexture() is a different one than the one that is passed to getTexture() and doesn't correspond to the array index of the texture anymore, this could be a problem. If you just get the number of textures and loop over them to pass them to Vulkan you might get wrong indices. For example, if you do something like this:

for(int i=0; i < shaderDesc->getNumTextures(); ++i)
{
vulkan->setTextureIndex(i);
}
for(int i=0; i < shaderDesc->getNum3DTextures(); ++i)
{
vulkan->setTexture3DIndex(shaderDesc->getNumTextures() + i);
}

If the 3D and 1D/2D texture indices are mixed in the generated shader code, the above code will produce wrong values. You would need another function that returns for each texture the index that was returned when calling the addTexture() function. Something like getTextureIndexInShader( textureIndexInTextureArray, out_textureIndexInShader);. Clients that have code like the one above would have to update their code then to make sure the right index is passed.
The original issue might also be solved by using separate processors for each LUT so you can give each of them a unique descriptor set index.

[michaelHADSK]

To clarify my other comment a bit more:
First maybe rename textureIndex to something that fits better like textureShaderBindingIndex.
Extend Texture struct to also store the binding index Texture t(textureName, ..., textureShaderBindingIndex);
Then add methods to the public API to retrieve it uint getTexture[3D]ShaderBindingIndex(textureIndex) { return m_textures[textureIndex].textureShaderBindingIndex;}
This way users have all the information to properly bind textures to the correct indices.

[doug-walker]

Thank you @michaelHADSK, that was very helpful. Please check if my latest commit does what you have in mind.

Note that the new getTextureShaderBindingIndex methods include the textureBindingStart. I'm considering having the index returned by the addTexture methods include that too, so that the code in the Ops doesn't have to worry about that. What do you think?

And I will add the new methods to the Python binding, once I have the C++ doing what you'd like.

[michaelHADSK]

Yes, thanks, that's what I had in mind, approved it. Including the textureBindingStart sounds good to me. I'm not sure when I will be able to test the changes in our software but as far as I can see everything should work this way.

[michaelHADSK]

LGTM

[michaelHADSK]

typo "ndex"