Fix Metal shader errors with MaterialX 1.39

* mx_microfacet.glsl uses functions from mx_math.metal, so the latter must be included first. * Metal defining atan(y, x) for GLSL compatibility conflicts with mx_math.metal using ::atan(y_over_x). Make OpenGL and Vulkan define atan2 instead. * textureQueryLevels(u_envRadiance) does not take into account that this texture is wrapped in a MetalTexture class. Signed-off-by: Brecht Van Lommel <[email protected]>
PixarAnimationStudios · Feb 5, 2025 · 8e186bc · 8e186bc
1 parent 0fff6d9
commit 8e186bc
Show file tree

Hide file tree

Showing 7 changed files with 18 additions and 9 deletions.
diff --git a/pxr/imaging/glf/shaders/simpleLighting.glslfx b/pxr/imaging/glf/shaders/simpleLighting.glslfx
@@ -328,7 +328,7 @@ projectSphericalToLatLong(vec3 sample3D)
     // project spherical coord onto latitude-longitude map with
     // latitude: +y == pi/2 and longitude: +z == 0, +x == pi/2
     const float PI = 3.1415;
-    vec2 coord = vec2((atan(sample3D.z, sample3D.x) + 0.5 * PI) / (2.0 * PI),
+    vec2 coord = vec2((atan2(sample3D.z, sample3D.x) + 0.5 * PI) / (2.0 * PI),
                       acos(sample3D.y) / PI);
     return coord;
 }

diff --git a/pxr/imaging/hdSt/materialXShaderGen.cpp b/pxr/imaging/hdSt/materialXShaderGen.cpp
@@ -447,8 +447,17 @@ HdStMaterialXShaderGen<Base>::_EmitMxInitFunction(
         emitLine("u_envIrradiance = HdGetSampler_domeLightFallback()", mxStage);
         emitLine("u_envRadiance = HdGetSampler_domeLightFallback()", mxStage);
         emitLine("#endif", mxStage, false);
+        emitLine("u_envRadianceMips = textureQueryLevels(u_envRadiance)", mxStage);
+    }
+    else {
+        if (std::is_same_v<Base, MaterialX::MslShaderGenerator>) {
+            // Msl has this wrapped in a MetalTexture class
+            emitLine("u_envRadianceMips = textureQueryLevels(u_envRadiance.tex)", mxStage);
+        }
+        else {
+            emitLine("u_envRadianceMips = textureQueryLevels(u_envRadiance)", mxStage);
+        }
     }
-    emitLine("u_envRadianceMips = textureQueryLevels(u_envRadiance)", mxStage);
     Base::emitLineBreak(mxStage);
 
     // Initialize MaterialX Texture samplers with HdGetSampler equivalents
@@ -1309,12 +1318,12 @@ HdStMaterialXShaderGenMsl::_EmitMxFunctions(
     mx::GenContext& mxContext,
     mx::ShaderStage& mxStage) const
 {
-    mx::ShaderGenerator::emitLibraryInclude(
-        "pbrlib/" + mx::GlslShaderGenerator::TARGET
-        + "/lib/mx_microfacet.glsl", mxContext, mxStage);
     mx::ShaderGenerator::emitLibraryInclude(
         "stdlib/" + mx::MslShaderGenerator::TARGET
         + "/lib/mx_math.metal", mxContext, mxStage);
+    mx::ShaderGenerator::emitLibraryInclude(
+        "pbrlib/" + mx::GlslShaderGenerator::TARGET
+        + "/lib/mx_microfacet.glsl", mxContext, mxStage);
     _EmitConstantsUniformsAndTypeDefs(
         mxContext, mxStage,_syntax->getConstantQualifier());
 

diff --git a/pxr/imaging/hdSt/shaders/domeLight.glslfx b/pxr/imaging/hdSt/shaders/domeLight.glslfx
@@ -60,7 +60,7 @@ vec3 SanitizeChannels(vec3 value)
 
 // sample lat/long env map input texture
 vec3 SampleEnvMapLod(vec3 sampleVec, float sampleLod) {
-    vec2 coord = vec2((atan(sampleVec.z, sampleVec.x) + PI) / (2.0 * PI),
+    vec2 coord = vec2((atan2(sampleVec.z, sampleVec.x) + PI) / (2.0 * PI),
                       acos(sampleVec.y) / PI);
     vec3 value = HgiTextureLod_inTexture(coord, sampleLod).rgb;
 

diff --git a/pxr/imaging/hdSt/shaders/surfaceHelpers.glslfx b/pxr/imaging/hdSt/shaders/surfaceHelpers.glslfx
@@ -17,7 +17,7 @@ vec2 ProjectToLatLong(vec3 sample3D)
 {
     // project spherical coord onto latitude-longitude map with
     // latitude: +y == pi/2 and longitude: +z == 0, +x == pi/2
-    float x = (atan(sample3D.z, sample3D.x) + 0.5 * PI) / (2.0 * PI);
+    float x = (atan2(sample3D.z, sample3D.x) + 0.5 * PI) / (2.0 * PI);
     float y = acos(sample3D.y) / PI;
 
     return vec2(x,y);

diff --git a/pxr/imaging/hgiGL/shaderGenerator.cpp b/pxr/imaging/hgiGL/shaderGenerator.cpp
@@ -246,6 +246,7 @@ HgiGLShaderGenerator::_WriteMacros(std::ostream &ss)
     // if items are in device or thread domain.
     ss << "#define REF(space,type) inout type\n"
           "#define FORWARD_DECL(func_decl) func_decl;\n"
+          "#define atan2(y, x) atan(y, x)\n"
           "#define ATOMIC_LOAD(a) (a)\n"
           "#define ATOMIC_STORE(a, v) (a) = (v)\n"
           "#define ATOMIC_ADD(a, v) atomicAdd(a, v)\n"

diff --git a/pxr/imaging/hgiMetal/shaderGenerator.mm b/pxr/imaging/hgiMetal/shaderGenerator.mm
@@ -515,8 +515,6 @@ void _Init(
               "template <typename T>\n"
               "T mod(T y, T x) { return fmod(y, x); }\n\n"
               "template <typename T>\n"
-              "T atan(T y, T x) { return atan2(y, x); }\n\n"
-              "template <typename T>\n"
               "T bitfieldReverse(T x) { return reverse_bits(x); }\n\n"
               "template <typename T>\n"
               "T bitfieldExtract(T value, int offset, int bits) {\n"

diff --git a/pxr/imaging/hgiVulkan/shaderGenerator.cpp b/pxr/imaging/hgiVulkan/shaderGenerator.cpp
@@ -221,6 +221,7 @@ HgiVulkanShaderGenerator::_WriteMacros(std::ostream &ss)
 {
     ss << "#define REF(space,type) inout type\n"
           "#define FORWARD_DECL(func_decl) func_decl\n"
+          "#define atan2(y, x) atan(y, x)\n"
           "#define ATOMIC_LOAD(a) (a)\n"
           "#define ATOMIC_STORE(a, v) (a) = (v)\n"
           "#define ATOMIC_ADD(a, v) atomicAdd(a, v)\n"