Skip to content

Commit 9fea546

Browse files
doug-walkerdoug-walker
authored
Fix Vulkan texture binding index issue (#2226)
* Fix texture binding index issue Signed-off-by: Doug Walker <[email protected]> * Add texture binding index getters Signed-off-by: Doug Walker <[email protected]> * Adjust addTexture return and fix Python binding Signed-off-by: Doug Walker <[email protected]> * Fix unit tests Signed-off-by: Doug Walker <[email protected]> --------- Signed-off-by: Doug Walker <[email protected]>
1 parent d7917d4 commit 9fea546

File tree

13 files changed

+66071
-125
lines changed

13 files changed

+66071
-125
lines changed

‎include/OpenColorIO/OpenColorIO.h‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3503,8 +3503,9 @@ class OCIOEXPORT GpuShaderCreator
35033503
* The 'values' parameter contains the LUT data which must be used as-is as the dimensions and
35043504
* origin are hard-coded in the fragment shader program. So, it means one GPU texture per entry.
35053505
*
3506-
* \return Index of the texture. For shading languages using explicit texture bindings, the return
3507-
* value is the same as the texture binding index in the generated shader program.
3506+
* \return Shader binding index of the texture. For shading languages using explicit texture bindings,
3507+
* the return value is the same as the texture binding index in the generated shader program.
3508+
* The setDescriptorSetIndex function may be used to offset the starting index value.
35083509
**/
35093510
virtual unsigned addTexture(const char * textureName,
35103511
const char * samplerName,
@@ -3522,8 +3523,9 @@ class OCIOEXPORT GpuShaderCreator
35223523
* and origin are hard-coded in the fragment shader program. So, it means one GPU 3D texture
35233524
* per entry.
35243525
*
3525-
* \return Index of the texture. For shading languages using explicit texture bindings, the return
3526-
* value is the same as the texture binding index in the generated shader program.
3526+
* \return Shader binding index of the texture. For shading languages using explicit texture bindings,
3527+
* the return value is the same as the texture binding index in the generated shader program.
3528+
* The setDescriptorSetIndex function may be used to offset the starting index value.
35273529
**/
35283530
virtual unsigned add3DTexture(const char * textureName,
35293531
const char * samplerName,
@@ -3775,7 +3777,12 @@ class OCIOEXPORT GpuShaderDesc : public GpuShaderCreator
37753777
**/
37763778
virtual std::size_t getUniformBufferSize() const noexcept = 0;
37773779

3778-
// 1D lut related methods
3780+
/**
3781+
* The getTexture methods are used to access Lut1D arrays to upload to the GPU as textures.
3782+
* Please note that the index used here is based on the total number of Lut1Ds used by
3783+
* the Processor and is different from the texture shader binding index, which may be
3784+
* obtained using the corresponding function.
3785+
*/
37793786
virtual unsigned getNumTextures() const noexcept = 0;
37803787
virtual void getTexture(unsigned index,
37813788
const char *& textureName,
@@ -3786,15 +3793,24 @@ class OCIOEXPORT GpuShaderDesc : public GpuShaderCreator
37863793
TextureDimensions & dimensions,
37873794
Interpolation & interpolation) const = 0;
37883795
virtual void getTextureValues(unsigned index, const float *& values) const = 0;
3796+
/// Get the index used to declare the texture in the shader for languages such as Vulkan.
3797+
virtual unsigned getTextureShaderBindingIndex(unsigned index) const = 0;
37893798

3790-
// 3D lut related methods
3799+
/**
3800+
* The get3DTexture methods are used to access Lut3D arrays to upload to the GPU as textures.
3801+
* Please note that the index used here is based on the total number of Lut3Ds used by
3802+
* the Processor and is different from the texture shader binding index, which may be
3803+
* obtained using the corresponding function.
3804+
*/
37913805
virtual unsigned getNum3DTextures() const noexcept = 0;
37923806
virtual void get3DTexture(unsigned index,
37933807
const char *& textureName,
37943808
const char *& samplerName,
37953809
unsigned & edgelen,
37963810
Interpolation & interpolation) const = 0;
37973811
virtual void get3DTextureValues(unsigned index, const float *& values) const = 0;
3812+
/// Get the index used to declare the texture in the shader for languages such as Vulkan.
3813+
virtual unsigned get3DTextureShaderBindingIndex(unsigned index) const = 0;
37983814

37993815
/// Get the complete OCIO shader program.
38003816
const char * getShaderText() const noexcept;

‎src/OpenColorIO/GpuShader.cpp‎

Lines changed: 57 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ class PrivateImpl
7070
GpuShaderDesc::TextureType channel,
7171
unsigned dimensions,
7272
Interpolation interpolation,
73+
unsigned textureShaderBindingIndex,
7374
const float * v)
7475
: m_textureName(textureName)
7576
, m_samplerName(samplerName)
@@ -79,6 +80,7 @@ class PrivateImpl
7980
, m_type(channel)
8081
, m_dimensions(dimensions)
8182
, m_interp(interpolation)
83+
, m_textureShaderBindingIndex(textureShaderBindingIndex)
8284
{
8385
if (!textureName || !*textureName)
8486
{
@@ -113,6 +115,7 @@ class PrivateImpl
113115
GpuShaderDesc::TextureType m_type;
114116
unsigned m_dimensions;
115117
Interpolation m_interp;
118+
unsigned m_textureShaderBindingIndex;
116119

117120
std::vector<float> m_values;
118121

@@ -214,11 +217,13 @@ class PrivateImpl
214217
<< width << " > " << get1dLutMaxWidth();
215218
throw Exception(ss.str().c_str());
216219
}
217-
unsigned textureIndex = static_cast<unsigned>(m_textures.size());
220+
unsigned textureShaderBindingIndex = static_cast<unsigned>(m_textures.size())
221+
+ static_cast<unsigned>(m_textures3D.size());
218222
unsigned numDimensions = static_cast<unsigned>(dimensions);
219-
Texture t(textureName, samplerName, width, height, 1, channel, numDimensions, interpolation, values);
223+
Texture t(textureName, samplerName, width, height, 1, channel, numDimensions, interpolation,
224+
textureShaderBindingIndex, values);
220225
m_textures.push_back(t);
221-
return textureIndex;
226+
return textureShaderBindingIndex;
222227
}
223228

224229
void getTexture(unsigned index,
@@ -268,6 +273,20 @@ class PrivateImpl
268273
values = &t.m_values[0];
269274
}
270275

276+
unsigned getTextureShaderBindingIndex(unsigned index) const
277+
{
278+
if(index >= m_textures.size())
279+
{
280+
std::ostringstream ss;
281+
ss << "1D LUT access error: index = " << index
282+
<< " where size = " << m_textures.size();
283+
throw Exception(ss.str().c_str());
284+
}
285+
286+
const Texture & t = m_textures[index];
287+
return t.m_textureShaderBindingIndex;
288+
}
289+
271290
unsigned add3DTexture(const char * textureName,
272291
const char * samplerName,
273292
unsigned edgelen,
@@ -282,12 +301,13 @@ class PrivateImpl
282301
throw Exception(ss.str().c_str());
283302
}
284303

285-
unsigned textureIndex = static_cast<unsigned>(m_textures3D.size());
304+
unsigned textureShaderBindingIndex = static_cast<unsigned>(m_textures.size())
305+
+ static_cast<unsigned>(m_textures3D.size());
286306
Texture t(textureName, samplerName, edgelen, edgelen, edgelen,
287307
GpuShaderDesc::TEXTURE_RGB_CHANNEL, 3,
288-
interpolation, values);
308+
interpolation, textureShaderBindingIndex, values);
289309
m_textures3D.push_back(t);
290-
return textureIndex;
310+
return textureShaderBindingIndex;
291311
}
292312

293313
void get3DTexture(unsigned index,
@@ -325,6 +345,20 @@ class PrivateImpl
325345
values = &t.m_values[0];
326346
}
327347

348+
unsigned get3DTextureShaderBindingIndex(unsigned index) const
349+
{
350+
if(index >= m_textures3D.size())
351+
{
352+
std::ostringstream ss;
353+
ss << "3D LUT access error: index = " << index
354+
<< " where size = " << m_textures3D.size();
355+
throw Exception(ss.str().c_str());
356+
}
357+
358+
const Texture & t = m_textures3D[index];
359+
return t.m_textureShaderBindingIndex;
360+
}
361+
328362
unsigned getNumUniforms() const
329363
{
330364
return (unsigned)m_uniforms.size();
@@ -544,7 +578,9 @@ unsigned GenericGpuShaderDesc::addTexture(const char * textureName,
544578
Interpolation interpolation,
545579
const float * values)
546580
{
547-
return getImplGeneric()->addTexture(textureName, samplerName, width, height, channel, dimensions, interpolation, values);
581+
return getImplGeneric()->addTexture(textureName, samplerName, width, height, channel,
582+
dimensions, interpolation, values)
583+
+ getTextureBindingStart();
548584
}
549585

550586
void GenericGpuShaderDesc::getTexture(unsigned index,
@@ -555,14 +591,20 @@ void GenericGpuShaderDesc::getTexture(unsigned index,
555591
TextureDimensions & dimensions,
556592
Interpolation & interpolation) const
557593
{
558-
getImplGeneric()->getTexture(index, textureName, samplerName, width, height, channel, dimensions, interpolation);
594+
getImplGeneric()->getTexture(index, textureName, samplerName, width, height, channel,
595+
dimensions, interpolation);
559596
}
560597

561598
void GenericGpuShaderDesc::getTextureValues(unsigned index, const float *& values) const
562599
{
563600
getImplGeneric()->getTextureValues(index, values);
564601
}
565602

603+
unsigned GenericGpuShaderDesc::getTextureShaderBindingIndex(unsigned index) const
604+
{
605+
return getImplGeneric()->getTextureShaderBindingIndex(index) + getTextureBindingStart();
606+
}
607+
566608
unsigned GenericGpuShaderDesc::getNum3DTextures() const noexcept
567609
{
568610
return unsigned(getImplGeneric()->m_textures3D.size());
@@ -574,7 +616,8 @@ unsigned GenericGpuShaderDesc::add3DTexture(const char * textureName,
574616
Interpolation interpolation,
575617
const float * values)
576618
{
577-
return getImplGeneric()->add3DTexture(textureName, samplerName, edgelen, interpolation, values);
619+
return getImplGeneric()->add3DTexture(textureName, samplerName, edgelen, interpolation, values)
620+
+ getTextureBindingStart();
578621
}
579622

580623
void GenericGpuShaderDesc::get3DTexture(unsigned index,
@@ -591,6 +634,11 @@ void GenericGpuShaderDesc::get3DTextureValues(unsigned index, const float *& val
591634
getImplGeneric()->get3DTextureValues(index, values);
592635
}
593636

637+
unsigned GenericGpuShaderDesc::get3DTextureShaderBindingIndex(unsigned index) const
638+
{
639+
return getImplGeneric()->get3DTextureShaderBindingIndex(index) + getTextureBindingStart();
640+
}
641+
594642
void GenericGpuShaderDesc::Deleter(GenericGpuShaderDesc* c)
595643
{
596644
delete c;

‎src/OpenColorIO/GpuShader.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ class GenericGpuShaderDesc : public GpuShaderDesc
6767
TextureDimensions & dimensions,
6868
Interpolation & interpolation) const override;
6969
void getTextureValues(unsigned index, const float *& values) const override;
70+
unsigned getTextureShaderBindingIndex(unsigned index) const override;
7071

7172
// Accessors to the 3D textures built from 3D LUT
7273
//
@@ -82,6 +83,7 @@ class GenericGpuShaderDesc : public GpuShaderDesc
8283
unsigned & edgelen,
8384
Interpolation & interpolation) const override;
8485
void get3DTextureValues(unsigned index, const float *& value) const override;
86+
unsigned get3DTextureShaderBindingIndex(unsigned index) const override;
8587

8688
private:
8789

‎src/OpenColorIO/GpuShaderDesc.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,10 @@ void GpuShaderCreator::setDescriptorSetIndex(unsigned index, unsigned textureBin
181181
{
182182
throw Exception("Texture binding start index must be greater than 0.");
183183
}
184+
AutoMutex lock(getImpl()->m_cacheIDMutex);
184185
getImpl()->m_descriptorSetIndex = index;
185186
getImpl()->m_textureBindingStart = textureBindingStart;
187+
getImpl()->m_cacheID.clear();
186188
}
187189

188190
unsigned GpuShaderCreator::getDescriptorSetIndex() const noexcept
@@ -270,6 +272,8 @@ const char * GpuShaderCreator::getCacheID() const noexcept
270272
os << getImpl()->m_resourcePrefix << " ";
271273
os << getImpl()->m_pixelName << " ";
272274
os << getImpl()->m_numResources << " ";
275+
os << getImpl()->m_descriptorSetIndex << " ";
276+
os << getImpl()->m_textureBindingStart << " ";
273277
os << getImpl()->m_shaderCodeID;
274278
getImpl()->m_cacheID = os.str();
275279
}

‎src/OpenColorIO/GpuShaderUtils.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -845,11 +845,11 @@ std::string GpuShaderText::getSamplerName(const std::string& textureName)
845845

846846
void GpuShaderText::declareTex1D(const std::string & textureName,
847847
unsigned descriptorSetIndex,
848-
unsigned textureIndex, unsigned textureBindingStart)
848+
unsigned textureIndex)
849849
{
850850
std::string textureDecl, samplerDecl;
851851
getTexDecl<1>(m_lang, textureName, getSamplerName(textureName), textureDecl, samplerDecl,
852-
descriptorSetIndex, textureIndex + textureBindingStart);
852+
descriptorSetIndex, textureIndex);
853853

854854
if (!textureDecl.empty())
855855
{
@@ -864,11 +864,11 @@ void GpuShaderText::declareTex1D(const std::string & textureName,
864864

865865
void GpuShaderText::declareTex2D(const std::string & textureName,
866866
unsigned descriptorSetIndex,
867-
unsigned textureIndex, unsigned textureBindingStart)
867+
unsigned textureIndex)
868868
{
869869
std::string textureDecl, samplerDecl;
870870
getTexDecl<2>(m_lang, textureName, getSamplerName(textureName), textureDecl, samplerDecl,
871-
descriptorSetIndex, textureIndex + textureBindingStart);
871+
descriptorSetIndex, textureIndex);
872872

873873
if (!textureDecl.empty())
874874
{
@@ -883,11 +883,11 @@ void GpuShaderText::declareTex2D(const std::string & textureName,
883883

884884
void GpuShaderText::declareTex3D(const std::string& textureName,
885885
unsigned descriptorSetIndex,
886-
unsigned textureIndex, unsigned textureBindingStart)
886+
unsigned textureIndex)
887887
{
888888
std::string textureDecl, samplerDecl;
889889
getTexDecl<3>(m_lang, textureName, getSamplerName(textureName), textureDecl, samplerDecl,
890-
descriptorSetIndex, textureIndex + textureBindingStart);
890+
descriptorSetIndex, textureIndex);
891891

892892
if (!textureDecl.empty())
893893
{

‎src/OpenColorIO/GpuShaderUtils.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,11 +171,11 @@ class GpuShaderText
171171
static std::string getSamplerName(const std::string& textureName);
172172

173173
// Declare the global texture and sampler information for a 1D texture.
174-
void declareTex1D(const std::string& textureName, unsigned descriptorSetIndex, unsigned textureIndex, unsigned textureBindingStart);
174+
void declareTex1D(const std::string& textureName, unsigned descriptorSetIndex, unsigned textureIndex);
175175
// Declare the global texture and sampler information for a 2D texture.
176-
void declareTex2D(const std::string& textureName, unsigned descriptorSetIndex, unsigned textureIndex, unsigned textureBindingStart);
176+
void declareTex2D(const std::string& textureName, unsigned descriptorSetIndex, unsigned textureIndex);
177177
// Declare the global texture and sampler information for a 3D texture.
178-
void declareTex3D(const std::string& textureName, unsigned descriptorSetIndex, unsigned textureIndex, unsigned textureBindingStart);
178+
void declareTex3D(const std::string& textureName, unsigned descriptorSetIndex, unsigned textureIndex);
179179

180180
// Get the texture lookup call for a 1D texture.
181181
std::string sampleTex1D(const std::string& textureName, const std::string& coords) const;

0 commit comments

Comments
 (0)