Skip to content

Commit

Permalink
Fix uniform name conflicts in Storm
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@ashwinbhat
ashwinbhat committed Nov 4, 2023
1 parent ceb3699 commit 99c21fc
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 23 deletions.
56 changes: 36 additions & 20 deletions pxr/imaging/hdSt/codeGen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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";

Expand Down Expand Up @@ -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";
Expand All @@ -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";
Expand Down Expand Up @@ -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";

Expand Down Expand Up @@ -6107,29 +6123,29 @@ 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";
}

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";

Expand Down
6 changes: 4 additions & 2 deletions pxr/imaging/hdSt/materialNetwork.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion pxr/imaging/hdSt/tokens.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ PXR_NAMESPACE_OPEN_SCOPE
(pointSizeScale) \
(screenSpaceWidths) \
(minScreenSpaceWidths) \
(shadowCompareTextures)
(shadowCompareTextures) \
((stormGenerated, "hdSt"))

#define HDST_TEXTURE_TOKENS \
(wrapS) \
Expand Down

0 comments on commit 99c21fc

Please sign in to comment.