From 825ec5b4f628a20f70b2c4d472fb7a32c66e33d8 Mon Sep 17 00:00:00 2001
From: Jerry Gamache <jerry.gamache@autodesk.com>
Date: Fri, 12 Jan 2024 09:40:05 -0500
Subject: [PATCH] LOOKDEVX-2354 - Fix the transform nodes crash

A transform node with empty space values will cause the shadergen to
crash. This has been fixed in future MaterialX 1.38.9 by introducing
ShaderPort::getValueString() in https://github.com/AcademySoftwareFoundation/MaterialX/pull/1560

We have backported that fix in local copies of the transform node
C++ implementations in order to prevent crashing with the current
versions of MaterialX.
---
 .../render/MaterialXGenOgsXml/CMakeLists.txt  |  9 ++
 .../GlslFragmentGenerator.cpp                 | 26 +++++
 .../Nodes/MayaTransformNormalNodeGlsl.cpp     | 54 ++++++++++
 .../Nodes/MayaTransformNormalNodeGlsl.h       | 28 ++++++
 .../Nodes/MayaTransformPointNodeGlsl.cpp      | 25 +++++
 .../Nodes/MayaTransformPointNodeGlsl.h        | 26 +++++
 .../Nodes/MayaTransformVectorNodeGlsl.cpp     | 98 +++++++++++++++++++
 .../Nodes/MayaTransformVectorNodeGlsl.h       | 36 +++++++
 8 files changed, 302 insertions(+)
 create mode 100644 lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformNormalNodeGlsl.cpp
 create mode 100644 lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformNormalNodeGlsl.h
 create mode 100644 lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformPointNodeGlsl.cpp
 create mode 100644 lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformPointNodeGlsl.h
 create mode 100644 lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformVectorNodeGlsl.cpp
 create mode 100644 lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformVectorNodeGlsl.h

diff --git a/lib/mayaUsd/render/MaterialXGenOgsXml/CMakeLists.txt b/lib/mayaUsd/render/MaterialXGenOgsXml/CMakeLists.txt
index afc78958f6..918c950020 100644
--- a/lib/mayaUsd/render/MaterialXGenOgsXml/CMakeLists.txt
+++ b/lib/mayaUsd/render/MaterialXGenOgsXml/CMakeLists.txt
@@ -13,6 +13,15 @@ target_sources(${PROJECT_NAME}
         PugiXML/pugixml.cpp
 )
 
+if(MaterialX_VERSION VERSION_LESS "1.38.9")
+target_sources(${PROJECT_NAME} 
+    PRIVATE
+        Nodes/MayaTransformNormalNodeGlsl.cpp
+        Nodes/MayaTransformPointNodeGlsl.cpp
+        Nodes/MayaTransformVectorNodeGlsl.cpp
+)
+endif()
+
 set(HEADERS
     CombinedMaterialXVersion.h
     GlslFragmentGenerator.h
diff --git a/lib/mayaUsd/render/MaterialXGenOgsXml/GlslFragmentGenerator.cpp b/lib/mayaUsd/render/MaterialXGenOgsXml/GlslFragmentGenerator.cpp
index 6280198b87..a9602ba8c5 100644
--- a/lib/mayaUsd/render/MaterialXGenOgsXml/GlslFragmentGenerator.cpp
+++ b/lib/mayaUsd/render/MaterialXGenOgsXml/GlslFragmentGenerator.cpp
@@ -7,6 +7,11 @@
 
 #include "Nodes/SurfaceNodeMaya.h"
 #include "Nodes/TexcoordNodeMaya.h"
+#if MX_COMBINED_VERSION <= 13809
+#include "Nodes/MayaTransformNormalNodeGlsl.h"
+#include "Nodes/MayaTransformPointNodeGlsl.h"
+#include "Nodes/MayaTransformVectorNodeGlsl.h"
+#endif
 
 #include <mayaUsd/render/MaterialXGenOgsXml/CombinedMaterialXVersion.h>
 #include <mayaUsd/render/MaterialXGenOgsXml/GlslOcioNodeImpl.h>
@@ -180,6 +185,27 @@ GlslFragmentGenerator::GlslFragmentGenerator()
             "IM_texcoord_vector3_" + GlslShaderGenerator::TARGET, TexcoordNodeGlslMaya::create);
     }
 
+    // The MaterialX transform node will crash if one of the "space" inputs is empty. This will be
+    // fixed in 1.38.9. In the meantime we use patched nodes to replace those previously added in
+    // the base class.
+#if MX_COMBINED_VERSION <= 13809
+    // <!-- <ND_transformpoint> ->
+    registerImplementation(
+        "IM_transformpoint_vector3_" + GlslShaderGenerator::TARGET,
+        MayaTransformPointNodeGlsl::create);
+
+    // <!-- <ND_transformvector> ->
+    registerImplementation(
+        "IM_transformvector_vector3_" + GlslShaderGenerator::TARGET,
+        MayaTransformVectorNodeGlsl::create);
+
+    // <!-- <ND_transformnormal> ->
+    registerImplementation(
+        "IM_transformnormal_vector3_" + GlslShaderGenerator::TARGET,
+        MayaTransformNormalNodeGlsl::create);
+
+#endif
+
     for (auto&& implName : GlslOcioNodeImpl::getOCIOImplementations()) {
         registerImplementation(implName, GlslOcioNodeImpl::create);
     }
diff --git a/lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformNormalNodeGlsl.cpp b/lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformNormalNodeGlsl.cpp
new file mode 100644
index 0000000000..ac7249779f
--- /dev/null
+++ b/lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformNormalNodeGlsl.cpp
@@ -0,0 +1,54 @@
+//
+// Copyright Contributors to the MaterialX Project
+// SPDX-License-Identifier: Apache-2.0
+//
+
+#include "MayaTransformNormalNodeGlsl.h"
+
+#include <mayaUsd/render/MaterialXGenOgsXml/CombinedMaterialXVersion.h>
+
+MATERIALX_NAMESPACE_BEGIN
+
+ShaderNodeImplPtr MayaTransformNormalNodeGlsl::create()
+{
+    return std::make_shared<MayaTransformNormalNodeGlsl>();
+}
+
+void MayaTransformNormalNodeGlsl::emitFunctionCall(
+    const ShaderNode& node,
+    GenContext&       context,
+    ShaderStage&      stage) const
+{
+    MayaTransformVectorNodeGlsl::emitFunctionCall(node, context, stage);
+
+#if MX_COMBINED_VERSION >= 13807
+    DEFINE_SHADER_STAGE(stage, Stage::PIXEL)
+    {
+#else
+    BEGIN_SHADER_STAGE(stage, Stage::PIXEL)
+#endif
+        const ShaderGenerator& shadergen = context.getShaderGenerator();
+        const ShaderOutput*    output = node.getOutput();
+        shadergen.emitLineBegin(stage);
+        shadergen.emitOutput(output, false, false, context, stage);
+        shadergen.emitString(" = normalize(" + output->getVariable() + ")", stage);
+        shadergen.emitLineEnd(stage);
+#if MX_COMBINED_VERSION >= 13807
+    }
+#else
+    END_SHADER_STAGE(shader, Stage::PIXEL)
+#endif
+}
+
+const string&
+MayaTransformNormalNodeGlsl::getMatrix(const string& fromSpace, const string& toSpace) const
+{
+    if ((fromSpace == MODEL || fromSpace == OBJECT) && toSpace == WORLD) {
+        return HW::T_WORLD_INVERSE_TRANSPOSE_MATRIX;
+    } else if (fromSpace == WORLD && (toSpace == MODEL || toSpace == OBJECT)) {
+        return HW::T_WORLD_TRANSPOSE_MATRIX;
+    }
+    return EMPTY_STRING;
+}
+
+MATERIALX_NAMESPACE_END
diff --git a/lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformNormalNodeGlsl.h b/lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformNormalNodeGlsl.h
new file mode 100644
index 0000000000..4ad4cc90a4
--- /dev/null
+++ b/lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformNormalNodeGlsl.h
@@ -0,0 +1,28 @@
+//
+// Copyright Contributors to the MaterialX Project
+// SPDX-License-Identifier: Apache-2.0
+//
+
+#ifndef MATERIALX_MAYATRANSFORMNORMALNODEGLSL_H
+#define MATERIALX_MAYATRANSFORMNORMALNODEGLSL_H
+
+#include "MayaTransformVectorNodeGlsl.h"
+
+MATERIALX_NAMESPACE_BEGIN
+
+/// TransformNormal node implementation for GLSL
+class MayaTransformNormalNodeGlsl : public MayaTransformVectorNodeGlsl
+{
+public:
+    static ShaderNodeImplPtr create();
+
+protected:
+    void emitFunctionCall(const ShaderNode& node, GenContext& context, ShaderStage& stage)
+        const override;
+
+    const string& getMatrix(const string& fromSpace, const string& toSpace) const override;
+};
+
+MATERIALX_NAMESPACE_END
+
+#endif
diff --git a/lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformPointNodeGlsl.cpp b/lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformPointNodeGlsl.cpp
new file mode 100644
index 0000000000..e6337b746a
--- /dev/null
+++ b/lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformPointNodeGlsl.cpp
@@ -0,0 +1,25 @@
+//
+// Copyright Contributors to the MaterialX Project
+// SPDX-License-Identifier: Apache-2.0
+//
+
+#include "MayaTransformPointNodeGlsl.h"
+
+#include <MaterialXGenShader/Shader.h>
+
+MATERIALX_NAMESPACE_BEGIN
+
+ShaderNodeImplPtr MayaTransformPointNodeGlsl::create()
+{
+    return std::make_shared<MayaTransformPointNodeGlsl>();
+}
+
+string MayaTransformPointNodeGlsl::getHomogeneousCoordinate(
+    const ShaderInput* in,
+    GenContext&        context) const
+{
+    const ShaderGenerator& shadergen = context.getShaderGenerator();
+    return "vec4(" + shadergen.getUpstreamResult(in, context) + ", 1.0)";
+}
+
+MATERIALX_NAMESPACE_END
diff --git a/lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformPointNodeGlsl.h b/lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformPointNodeGlsl.h
new file mode 100644
index 0000000000..6aaec62cfb
--- /dev/null
+++ b/lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformPointNodeGlsl.h
@@ -0,0 +1,26 @@
+//
+// Copyright Contributors to the MaterialX Project
+// SPDX-License-Identifier: Apache-2.0
+//
+
+#ifndef MATERIALX_MAYATRANSFORMPOINTNODEGLSL_H
+#define MATERIALX_MAYATRANSFORMPOINTNODEGLSL_H
+
+#include "MayaTransformVectorNodeGlsl.h"
+
+MATERIALX_NAMESPACE_BEGIN
+
+/// TransformPoint node implementation for GLSL
+class MayaTransformPointNodeGlsl : public MayaTransformVectorNodeGlsl
+{
+public:
+    static ShaderNodeImplPtr create();
+
+protected:
+    virtual string
+    getHomogeneousCoordinate(const ShaderInput* in, GenContext& context) const override;
+};
+
+MATERIALX_NAMESPACE_END
+
+#endif
diff --git a/lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformVectorNodeGlsl.cpp b/lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformVectorNodeGlsl.cpp
new file mode 100644
index 0000000000..94752a9104
--- /dev/null
+++ b/lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformVectorNodeGlsl.cpp
@@ -0,0 +1,98 @@
+//
+// Copyright Contributors to the MaterialX Project
+// SPDX-License-Identifier: Apache-2.0
+//
+
+#include "MayaTransformVectorNodeGlsl.h"
+
+#include <MaterialXGenShader/Shader.h>
+
+MATERIALX_NAMESPACE_BEGIN
+
+ShaderNodeImplPtr MayaTransformVectorNodeGlsl::create()
+{
+    return std::make_shared<MayaTransformVectorNodeGlsl>();
+}
+
+void MayaTransformVectorNodeGlsl::createVariables(
+    const ShaderNode& node,
+    GenContext&,
+    Shader& shader) const
+{
+    string toSpace = getPortValueString(node.getInput(TO_SPACE));
+    string fromSpace = getPortValueString(node.getInput(FROM_SPACE));
+
+    const string& matrix = getMatrix(fromSpace, toSpace);
+    if (!matrix.empty()) {
+        ShaderStage& ps = shader.getStage(Stage::PIXEL);
+        addStageUniform(HW::PRIVATE_UNIFORMS, Type::MATRIX44, matrix, ps);
+    }
+}
+
+void MayaTransformVectorNodeGlsl::emitFunctionCall(
+    const ShaderNode& node,
+    GenContext&       context,
+    ShaderStage&      stage) const
+{
+#if MX_COMBINED_VERSION >= 13807
+    DEFINE_SHADER_STAGE(stage, Stage::PIXEL)
+    {
+#else
+    BEGIN_SHADER_STAGE(stage, Stage::PIXEL)
+#endif
+        const ShaderGenerator& shadergen = context.getShaderGenerator();
+
+        const ShaderInput* inInput = node.getInput("in");
+        if (inInput->getType() != Type::VECTOR3 && inInput->getType() != Type::VECTOR4) {
+            throw ExceptionShaderGenError(
+                "Transform node must have 'in' type of vector3 or vector4.");
+        }
+
+        string toSpace = getPortValueString(node.getInput(TO_SPACE));
+        string fromSpace = getPortValueString(node.getInput(FROM_SPACE));
+
+        shadergen.emitLineBegin(stage);
+        shadergen.emitOutput(node.getOutput(), true, false, context, stage);
+        shadergen.emitString(" = (", stage);
+        const string& matrix = getMatrix(fromSpace, toSpace);
+        if (!matrix.empty()) {
+            shadergen.emitString(matrix + " * ", stage);
+        }
+        shadergen.emitString(getHomogeneousCoordinate(inInput, context), stage);
+        shadergen.emitString(").xyz", stage);
+        shadergen.emitLineEnd(stage);
+#if MX_COMBINED_VERSION >= 13807
+    }
+#else
+    END_SHADER_STAGE(shader, Stage::PIXEL)
+#endif
+}
+
+const string&
+MayaTransformVectorNodeGlsl::getMatrix(const string& fromSpace, const string& toSpace) const
+{
+    if ((fromSpace == MODEL || fromSpace == OBJECT) && toSpace == WORLD) {
+        return HW::T_WORLD_MATRIX;
+    } else if (fromSpace == WORLD && (toSpace == MODEL || toSpace == OBJECT)) {
+        return HW::T_WORLD_INVERSE_MATRIX;
+    }
+    return EMPTY_STRING;
+}
+
+string MayaTransformVectorNodeGlsl::getHomogeneousCoordinate(
+    const ShaderInput* in,
+    GenContext&        context) const
+{
+    const ShaderGenerator& shadergen = context.getShaderGenerator();
+    return "vec4(" + shadergen.getUpstreamResult(in, context) + ", 0.0)";
+}
+
+string MayaTransformVectorNodeGlsl::getPortValueString(const ShaderInput* input) const
+{
+    if (!input || !input->getValue()) {
+        return EMPTY_STRING;
+    }
+    return input->getValue()->getValueString();
+}
+
+MATERIALX_NAMESPACE_END
diff --git a/lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformVectorNodeGlsl.h b/lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformVectorNodeGlsl.h
new file mode 100644
index 0000000000..57f7510445
--- /dev/null
+++ b/lib/mayaUsd/render/MaterialXGenOgsXml/Nodes/MayaTransformVectorNodeGlsl.h
@@ -0,0 +1,36 @@
+//
+// Copyright Contributors to the MaterialX Project
+// SPDX-License-Identifier: Apache-2.0
+//
+
+#ifndef MATERIALX_MAYATRANSFORMVECTORNODEGLSL_H
+#define MATERIALX_MAYATRANSFORMVECTORNODEGLSL_H
+
+#include <MaterialXGenGlsl/GlslShaderGenerator.h>
+
+MATERIALX_NAMESPACE_BEGIN
+
+/// TransformVector node implementation for GLSL
+class MayaTransformVectorNodeGlsl : public GlslImplementation
+{
+public:
+    static ShaderNodeImplPtr create();
+
+    void
+    createVariables(const ShaderNode& node, GenContext& context, Shader& shader) const override;
+
+    void emitFunctionCall(const ShaderNode& node, GenContext& context, ShaderStage& stage)
+        const override;
+
+protected:
+    virtual const string& getMatrix(const string& fromSpace, const string& toSpace) const;
+    virtual string getHomogeneousCoordinate(const ShaderInput* in, GenContext& context) const;
+
+    // Fixes crash getting a value string that is potentially nullptr. This was fixed in MaterialX
+    // pull request 1560, so we only need to use patched nodes until v1.38.9.
+    string getPortValueString(const ShaderInput* input) const;
+};
+
+MATERIALX_NAMESPACE_END
+
+#endif