From 99c21fc6db60217cd86bdccb13b357c09d2b047b Mon Sep 17 00:00:00 2001 From: Ashwin Bhat Date: Mon, 21 Aug 2023 16:08:24 -0700 Subject: [PATCH] Fix uniform name conflicts in Storm Storm injects a built-in scale and bias uniform for texture types. When using materialx graphs, this can cause name collision when materialx inputs. For example if a node has inputs called normal (texture) and normal_scale (float), this will conflict when storm adds a normal_scale (for texture). The workaround or fix is to use a unique name. We use a named suffix _hdSt_ to define Storm generated uniforms. --- pxr/imaging/hdSt/codeGen.cpp | 56 ++++++++++++++++++---------- pxr/imaging/hdSt/materialNetwork.cpp | 6 ++- pxr/imaging/hdSt/tokens.h | 3 +- 3 files changed, 42 insertions(+), 23 deletions(-) diff --git a/pxr/imaging/hdSt/codeGen.cpp b/pxr/imaging/hdSt/codeGen.cpp index eac138a7ce..949969373a 100644 --- a/pxr/imaging/hdSt/codeGen.cpp +++ b/pxr/imaging/hdSt/codeGen.cpp @@ -3433,12 +3433,12 @@ static void _EmitTextureAccessors( // Forward declare texture scale and bias if (hasTextureScaleAndBias) { accessors - << "#ifdef HD_HAS_" << name << "_" << HdStTokens->scale << "\n" - << "FORWARD_DECL(vec4 HdGet_" << name << "_" << HdStTokens->scale + << "#ifdef HD_HAS_" << name << "_" << HdStTokens->stormGenerated << "_" << HdStTokens->scale << "\n" + << "FORWARD_DECL(vec4 HdGet_" << name << "_" << HdStTokens->stormGenerated << "_" << HdStTokens->scale << "());\n" << "#endif\n" - << "#ifdef HD_HAS_" << name << "_" << HdStTokens->bias << "\n" - << "FORWARD_DECL(vec4 HdGet_" << name << "_" << HdStTokens->bias + << "#ifdef HD_HAS_" << name << "_" << HdStTokens->stormGenerated << "_" << HdStTokens->bias << "\n" + << "FORWARD_DECL(vec4 HdGet_" << name << "_" << HdStTokens->stormGenerated << "_" << HdStTokens->bias << "());\n" << "#endif\n"; } @@ -3595,11 +3595,11 @@ static void _EmitTextureAccessors( } } accessors - << "#ifdef HD_HAS_" << name << "_" << HdStTokens->scale << "\n" - << " * HdGet_" << name << "_" << HdStTokens->scale << "()\n" + << "#ifdef HD_HAS_" << name << "_" << HdStTokens->stormGenerated << "_" << HdStTokens->scale << "\n" + << " * HdGet_" << name << "_" << HdStTokens->stormGenerated << "_" << HdStTokens->scale << "()\n" << "#endif\n" - << "#ifdef HD_HAS_" << name << "_" << HdStTokens->bias << "\n" - << " + HdGet_" << name << "_" << HdStTokens->bias << "()\n" + << "#ifdef HD_HAS_" << name << "_" << HdStTokens->stormGenerated << "_" << HdStTokens->bias << "\n" + << " + HdGet_" << name << "_" << HdStTokens->stormGenerated << "_" << HdStTokens->bias << "()\n" << "#endif\n" << ")" << swizzle << ");\n"; } else { @@ -3655,12 +3655,12 @@ static void _EmitTextureAccessors( << name << HdSt_ResourceBindingSuffixTokens->fallback << fallbackSwizzle << ")\n" - << "#ifdef HD_HAS_" << name << "_" << HdStTokens->scale << "\n" - << " * HdGet_" << name << "_" << HdStTokens->scale + << "#ifdef HD_HAS_" << name << "_" << HdStTokens->stormGenerated << "_" << HdStTokens->scale << "\n" + << " * HdGet_" << name << "_" << HdStTokens->stormGenerated << "_" << HdStTokens->scale << "()" << swizzle << "\n" << "#endif\n" - << "#ifdef HD_HAS_" << name << "_" << HdStTokens->bias << "\n" - << " + HdGet_" << name << "_" << HdStTokens->bias + << "#ifdef HD_HAS_" << name << "_" << HdStTokens->stormGenerated << "_" << HdStTokens->bias << "\n" + << " + HdGet_" << name << "_" << HdStTokens->stormGenerated << "_" << HdStTokens->bias << "()" << swizzle << "\n" << "#endif\n" << ");\n" @@ -5937,13 +5937,17 @@ HdSt_CodeGen::_GenerateShaderParameters(bool bindlessTextureEnabled) accessors << "#ifdef HD_HAS_" << it->second.name << "_" + << HdStTokens->stormGenerated << "_" << HdStTokens->scale << "\n" << "vec4 HdGet_" << it->second.name << "_" + << HdStTokens->stormGenerated << "_" << HdStTokens->scale << "();\n" << "#endif\n" << "#ifdef HD_HAS_" << it->second.name << "_" + << HdStTokens->stormGenerated << "_" << HdStTokens->bias << "\n" << "vec4 HdGet_" << it->second.name << "_" + << HdStTokens->stormGenerated << "_" << HdStTokens->bias << "();\n" << "#endif\n"; @@ -5985,13 +5989,17 @@ HdSt_CodeGen::_GenerateShaderParameters(bool bindlessTextureEnabled) << HdSt_ResourceBindingSuffixTokens->fallback << fallbackSwizzle << ")\n" << "#ifdef HD_HAS_" << it->second.name << "_" + << HdStTokens->stormGenerated << "_" << HdStTokens->scale << "\n" << " * HdGet_" << it->second.name << "_" + << HdStTokens->stormGenerated << "_" << HdStTokens->scale << "()" << swizzle << "\n" << "#endif\n" << "#ifdef HD_HAS_" << it->second.name << "_" + << HdStTokens->stormGenerated << "_" << HdStTokens->bias << "\n" << " + HdGet_" << it->second.name << "_" + << HdStTokens->stormGenerated << "_" << HdStTokens->bias << "()" << swizzle << "\n" << "#endif\n" << " );\n }\n"; @@ -6000,13 +6008,17 @@ HdSt_CodeGen::_GenerateShaderParameters(bool bindlessTextureEnabled) accessors << " return (ret\n" << "#ifdef HD_HAS_" << it->second.name << "_" + << HdStTokens->stormGenerated << "_" << HdStTokens->scale << "\n" << " * HdGet_" << it->second.name << "_" + << HdStTokens->stormGenerated << "_" << HdStTokens->scale << "()\n" << "#endif\n" << "#ifdef HD_HAS_" << it->second.name << "_" + << HdStTokens->stormGenerated << "_" << HdStTokens->bias << "\n" << " + HdGet_" << it->second.name << "_" + << HdStTokens->stormGenerated << "_" << HdStTokens->bias << "()\n" << "#endif\n" << " )" << swizzle << ";\n}\n"; @@ -6057,13 +6069,17 @@ HdSt_CodeGen::_GenerateShaderParameters(bool bindlessTextureEnabled) accessors << "#ifdef HD_HAS_" << it->second.name << "_" + << HdStTokens->stormGenerated << "_" << HdStTokens->scale << "\n" << "FORWARD_DECL(vec4 HdGet_" << it->second.name << "_" + << HdStTokens->stormGenerated << "_" << HdStTokens->scale << "());\n" << "#endif\n" << "#ifdef HD_HAS_" << it->second.name << "_" + << HdStTokens->stormGenerated << "_" << HdStTokens->bias << "\n" << "FORWARD_DECL(vec4 HdGet_" << it->second.name << "_" + << HdStTokens->stormGenerated << "_" << HdStTokens->bias << "());\n" << "#endif\n"; @@ -6107,14 +6123,14 @@ HdSt_CodeGen::_GenerateShaderParameters(bool bindlessTextureEnabled) << HdSt_ResourceBindingSuffixTokens->fallback << fallbackSwizzle << ")\n" << "#ifdef HD_HAS_" << it->second.name << "_" - << HdStTokens->scale << "\n" + << HdStTokens->stormGenerated << "_" << HdStTokens->scale << "\n" << " * HdGet_" << it->second.name << "_" - << HdStTokens->scale << "()" << swizzle << "\n" + << HdStTokens->stormGenerated << "_" << HdStTokens->scale << "()" << swizzle << "\n" << "#endif\n" << "#ifdef HD_HAS_" << it->second.name << "_" - << HdStTokens->bias << "\n" + << HdStTokens->stormGenerated << "_" << HdStTokens->bias << "\n" << " + HdGet_" << it->second.name << "_" - << HdStTokens->bias << "()" << swizzle << "\n" + << HdStTokens->stormGenerated << "_" << HdStTokens->bias << "()" << swizzle << "\n" << "#endif\n" << " );\n }\n"; } @@ -6122,14 +6138,14 @@ HdSt_CodeGen::_GenerateShaderParameters(bool bindlessTextureEnabled) accessors << " return (ret\n" << "#ifdef HD_HAS_" << it->second.name << "_" - << HdStTokens->scale << "\n" + << HdStTokens->stormGenerated << "_" << HdStTokens->scale << "\n" << " * HdGet_" << it->second.name << "_" - << HdStTokens->scale << "()\n" + << HdStTokens->stormGenerated << "_" << HdStTokens->scale << "()\n" << "#endif\n" << "#ifdef HD_HAS_" << it->second.name << "_" - << HdStTokens->bias << "\n" + << HdStTokens->stormGenerated << "_" << HdStTokens->bias << "\n" << " + HdGet_" << it->second.name << "_" - << HdStTokens->bias << "()\n" + << HdStTokens->stormGenerated << "_" << HdStTokens->bias << "()\n" << "#endif\n" << " )" << swizzle << ";\n}\n"; diff --git a/pxr/imaging/hdSt/materialNetwork.cpp b/pxr/imaging/hdSt/materialNetwork.cpp index 03f5f78535..0014d4afa3 100644 --- a/pxr/imaging/hdSt/materialNetwork.cpp +++ b/pxr/imaging/hdSt/materialNetwork.cpp @@ -954,7 +954,8 @@ _MakeMaterialParamsForTexture( // Handle texture scale and bias HdSt_MaterialParam texScaleParam; texScaleParam.paramType = HdSt_MaterialParam::ParamTypeFallback; - texScaleParam.name = TfToken(paramName.GetString() + "_" + + texScaleParam.name = TfToken(paramName.GetString() + "_" + + HdStTokens->stormGenerated.GetString() + "_" + HdStTokens->scale.GetString()); texScaleParam.fallbackValue = VtValue(_ResolveParameter(node, sdrNode, @@ -964,7 +965,8 @@ _MakeMaterialParamsForTexture( HdSt_MaterialParam texBiasParam; texBiasParam.paramType = HdSt_MaterialParam::ParamTypeFallback; - texBiasParam.name = TfToken(paramName.GetString() + "_" + + texBiasParam.name = TfToken(paramName.GetString() + "_" + + HdStTokens->stormGenerated.GetString() + "_" + HdStTokens->bias.GetString()); texBiasParam.fallbackValue = VtValue(_ResolveParameter(node, sdrNode, diff --git a/pxr/imaging/hdSt/tokens.h b/pxr/imaging/hdSt/tokens.h index ac36c7daff..0d84a4bdac 100644 --- a/pxr/imaging/hdSt/tokens.h +++ b/pxr/imaging/hdSt/tokens.h @@ -75,7 +75,8 @@ PXR_NAMESPACE_OPEN_SCOPE (pointSizeScale) \ (screenSpaceWidths) \ (minScreenSpaceWidths) \ - (shadowCompareTextures) + (shadowCompareTextures) \ + ((stormGenerated, "hdSt")) #define HDST_TEXTURE_TOKENS \ (wrapS) \