From 8c515e8d3f7e795e156346e41c955ec5c3f0bec7 Mon Sep 17 00:00:00 2001
From: ld-kerley <154285602+ld-kerley@users.noreply.github.com>
Date: Mon, 3 Jun 2024 15:25:29 -0700
Subject: [PATCH 1/2] Fix OSL syntax for Convert nodes (#1863)
PR #1854, previously removed the C++ implementations for `convert` and replaced them with direct source code implementations. The testing done wasn't correctly running `oslc` so a few syntax errors slipped through.
These are corrected in this PR.
Note:
`vector2`, `vector4` and `color4` are implemented as structs in OSL, and so do not have constructors in the typical sense. The following is not legal OSL syntax.
```
vector4 a = vector4(0,0,0,0);
```
instead you must use a list initializer to initialize the struct.
```
vector4 a = {0,0,0,0};
```
Conversely, currently it is NOT legal to initialize a builtin type, for instance `color` or `vector` using a list initializer.
The following is not legal OSL syntax.
```
color a = {0,0,0};
```
instead you must use the type constructor.
```
color a = color(0,0,0);
```
Also note, subscript access also is also not legal OSL syntax because these types are implemented as structs. For example to access the green component of a `color4`, you must use `myColor.rgb[1]`.
---
.../stdlib/genosl/stdlib_genosl_impl.mtlx | 22 +++++++++----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/libraries/stdlib/genosl/stdlib_genosl_impl.mtlx b/libraries/stdlib/genosl/stdlib_genosl_impl.mtlx
index ab957b8b4b..42daa95a57 100644
--- a/libraries/stdlib/genosl/stdlib_genosl_impl.mtlx
+++ b/libraries/stdlib/genosl/stdlib_genosl_impl.mtlx
@@ -686,20 +686,20 @@
-
-
+
+
-
-
-
+
+
+
-
-
-
+
+
+
-
-
-
+
+
+
From e613fc7514f224d847174d2d640a0b71a0dcf8e3 Mon Sep 17 00:00:00 2001
From: ld-kerley <154285602+ld-kerley@users.noreply.github.com>
Date: Mon, 3 Jun 2024 16:20:41 -0700
Subject: [PATCH 2/2] Remove Combine C++ node (#1855)
Replace with direct source code implementation for `combine2`, `combine3` and `combine4`.
---
.../stdlib/genglsl/stdlib_genglsl_impl.mtlx | 22 +--
.../stdlib/genmdl/stdlib_genmdl_impl.mtlx | 16 +-
.../stdlib/genmsl/stdlib_genmsl_impl.mtlx | 22 +--
.../stdlib/genosl/stdlib_genosl_impl.mtlx | 16 +-
.../MaterialXGenGlsl/GlslShaderGenerator.cpp | 14 --
source/MaterialXGenMdl/MdlShaderGenerator.cpp | 11 --
.../MaterialXGenMdl/Nodes/CombineNodeMdl.cpp | 94 ------------
source/MaterialXGenMdl/Nodes/CombineNodeMdl.h | 26 ----
source/MaterialXGenMsl/MslShaderGenerator.cpp | 14 --
source/MaterialXGenOsl/OslShaderGenerator.cpp | 11 --
.../MaterialXGenShader/Nodes/CombineNode.cpp | 143 ------------------
source/MaterialXGenShader/Nodes/CombineNode.h | 24 ---
12 files changed, 42 insertions(+), 371 deletions(-)
delete mode 100644 source/MaterialXGenMdl/Nodes/CombineNodeMdl.cpp
delete mode 100644 source/MaterialXGenMdl/Nodes/CombineNodeMdl.h
delete mode 100644 source/MaterialXGenShader/Nodes/CombineNode.cpp
delete mode 100644 source/MaterialXGenShader/Nodes/CombineNode.h
diff --git a/libraries/stdlib/genglsl/stdlib_genglsl_impl.mtlx b/libraries/stdlib/genglsl/stdlib_genglsl_impl.mtlx
index 8d8b620a2b..fd43c9e118 100644
--- a/libraries/stdlib/genglsl/stdlib_genglsl_impl.mtlx
+++ b/libraries/stdlib/genglsl/stdlib_genglsl_impl.mtlx
@@ -704,15 +704,19 @@
-
-
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/libraries/stdlib/genmdl/stdlib_genmdl_impl.mtlx b/libraries/stdlib/genmdl/stdlib_genmdl_impl.mtlx
index e8fca2128c..3ec74a5deb 100644
--- a/libraries/stdlib/genmdl/stdlib_genmdl_impl.mtlx
+++ b/libraries/stdlib/genmdl/stdlib_genmdl_impl.mtlx
@@ -713,18 +713,18 @@
-
-
-
-
+
+
+
+
-
-
+
+
-
-
+
+
diff --git a/libraries/stdlib/genmsl/stdlib_genmsl_impl.mtlx b/libraries/stdlib/genmsl/stdlib_genmsl_impl.mtlx
index 3d1bdaf7fa..6e5f271164 100644
--- a/libraries/stdlib/genmsl/stdlib_genmsl_impl.mtlx
+++ b/libraries/stdlib/genmsl/stdlib_genmsl_impl.mtlx
@@ -704,15 +704,19 @@
-
-
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/libraries/stdlib/genosl/stdlib_genosl_impl.mtlx b/libraries/stdlib/genosl/stdlib_genosl_impl.mtlx
index 42daa95a57..fd339948d7 100644
--- a/libraries/stdlib/genosl/stdlib_genosl_impl.mtlx
+++ b/libraries/stdlib/genosl/stdlib_genosl_impl.mtlx
@@ -704,18 +704,18 @@
-
-
-
-
+
+
+
+
-
-
+
+
-
-
+
+
diff --git a/source/MaterialXGenGlsl/GlslShaderGenerator.cpp b/source/MaterialXGenGlsl/GlslShaderGenerator.cpp
index 0e76154615..e114b4fe2a 100644
--- a/source/MaterialXGenGlsl/GlslShaderGenerator.cpp
+++ b/source/MaterialXGenGlsl/GlslShaderGenerator.cpp
@@ -19,7 +19,6 @@
#include
#include
-#include
#include
#include
#include
@@ -80,19 +79,6 @@ GlslShaderGenerator::GlslShaderGenerator() :
};
registerImplementation(elementNames, SwitchNode::create);
- //
- elementNames = {
- "IM_combine2_vector2_" + GlslShaderGenerator::TARGET,
- "IM_combine2_color4CF_" + GlslShaderGenerator::TARGET,
- "IM_combine2_vector4VF_" + GlslShaderGenerator::TARGET,
- "IM_combine2_vector4VV_" + GlslShaderGenerator::TARGET,
- "IM_combine3_color3_" + GlslShaderGenerator::TARGET,
- "IM_combine3_vector3_" + GlslShaderGenerator::TARGET,
- "IM_combine4_color4_" + GlslShaderGenerator::TARGET,
- "IM_combine4_vector4_" + GlslShaderGenerator::TARGET,
- };
- registerImplementation(elementNames, CombineNode::create);
-
//
registerImplementation("IM_position_vector3_" + GlslShaderGenerator::TARGET, HwPositionNode::create);
//
diff --git a/source/MaterialXGenMdl/MdlShaderGenerator.cpp b/source/MaterialXGenMdl/MdlShaderGenerator.cpp
index a41b0d8f0c..fa984729a6 100644
--- a/source/MaterialXGenMdl/MdlShaderGenerator.cpp
+++ b/source/MaterialXGenMdl/MdlShaderGenerator.cpp
@@ -12,7 +12,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -92,16 +91,6 @@ MdlShaderGenerator::MdlShaderGenerator() :
//
registerImplementation("IM_surface_" + MdlShaderGenerator::TARGET, SurfaceNodeMdl::create);
- //
- registerImplementation("IM_combine2_vector2_" + MdlShaderGenerator::TARGET, CombineNodeMdl::create);
- registerImplementation("IM_combine2_color4CF_" + MdlShaderGenerator::TARGET, CombineNodeMdl::create);
- registerImplementation("IM_combine2_vector4VF_" + MdlShaderGenerator::TARGET, CombineNodeMdl::create);
- registerImplementation("IM_combine2_vector4VV_" + MdlShaderGenerator::TARGET, CombineNodeMdl::create);
- registerImplementation("IM_combine3_color3_" + MdlShaderGenerator::TARGET, CombineNodeMdl::create);
- registerImplementation("IM_combine3_vector3_" + MdlShaderGenerator::TARGET, CombineNodeMdl::create);
- registerImplementation("IM_combine4_color4_" + MdlShaderGenerator::TARGET, CombineNodeMdl::create);
- registerImplementation("IM_combine4_vector4_" + MdlShaderGenerator::TARGET, CombineNodeMdl::create);
-
//
registerImplementation("IM_blur_float_" + MdlShaderGenerator::TARGET, BlurNodeMdl::create);
registerImplementation("IM_blur_color3_" + MdlShaderGenerator::TARGET, BlurNodeMdl::create);
diff --git a/source/MaterialXGenMdl/Nodes/CombineNodeMdl.cpp b/source/MaterialXGenMdl/Nodes/CombineNodeMdl.cpp
deleted file mode 100644
index 9d6789c94c..0000000000
--- a/source/MaterialXGenMdl/Nodes/CombineNodeMdl.cpp
+++ /dev/null
@@ -1,94 +0,0 @@
-//
-// Copyright Contributors to the MaterialX Project
-// SPDX-License-Identifier: Apache-2.0
-//
-
-#include
-
-#include
-#include
-#include
-#include
-
-MATERIALX_NAMESPACE_BEGIN
-
-ShaderNodeImplPtr CombineNodeMdl::create()
-{
- return std::make_shared();
-}
-
-void CombineNodeMdl::emitFunctionCall(const ShaderNode& node, GenContext& context, ShaderStage& stage) const
-{
- DEFINE_SHADER_STAGE(stage, Stage::PIXEL)
- {
- // Custom handling for color3 type input, all other types
- // can are handled by our parent class below.
- // Custom handling is needed since in MDL color must be converted
- // to float3 before accessing its sub-component.
-
- const ShaderInput* in1 = node.getInput(0);
- const ShaderOutput* out = node.getOutput();
- if (!in1 || !out)
- {
- throw ExceptionShaderGenError("Node '" + node.getName() + "' is not a valid convert node");
- }
-
- if (in1->getType() == Type::COLOR3)
- {
- const ShaderInput* in2 = node.getInput(1);
- if (!in2 || in2->getType() != Type::FLOAT)
- {
- throw ExceptionShaderGenError("Node '" + node.getName() + "' is not a valid convert node");
- }
-
- const ShaderGenerator& shadergen = context.getShaderGenerator();
-
- // If in1 is unconnected we must declare a local variable
- // for it first, in order to access components from it below.
- string in1Variable = in1->getConnection() ? in1->getConnection()->getVariable() : in1->getVariable();
- if (!in1->getConnection())
- {
- string variableValue = in1->getValue() ? shadergen.getSyntax().getValue(in1->getType(), *in1->getValue()) : shadergen.getSyntax().getDefaultValue(in1->getType());
- shadergen.emitLine(shadergen.getSyntax().getTypeName(in1->getType()) + " " + in1Variable + " = " + variableValue, stage);
- }
-
- // Get the value components to use for constructing the new value.
- StringVec valueComponents;
-
- // Get components from in1
- const StringVec& in1Members = shadergen.getSyntax().getTypeSyntax(in1->getType()).getMembers();
- size_t memberSize = in1Members.size();
- if (memberSize)
- {
- valueComponents.resize(memberSize + 1);
- for (size_t i = 0; i < memberSize; i++)
- {
- valueComponents[i] = "float3(" + in1Variable + ")" + in1Members[i];
- }
- }
- else
- {
- memberSize = 1;
- valueComponents.resize(2);
- valueComponents[0] = in1Variable;
- }
- // Get component from in2
- valueComponents[memberSize] = shadergen.getUpstreamResult(in2, context);
-
- // Let the TypeSyntax construct the value from the components.
- const TypeSyntax& outTypeSyntax = shadergen.getSyntax().getTypeSyntax(out->getType());
- const string result = outTypeSyntax.getValue(valueComponents, false);
-
- shadergen.emitLineBegin(stage);
- shadergen.emitOutput(node.getOutput(), true, false, context, stage);
- shadergen.emitString(" = " + result, stage);
- shadergen.emitLineEnd(stage);
- }
- else
- {
- CombineNode::emitFunctionCall(node, context, stage);
- }
- }
-}
-
-MATERIALX_NAMESPACE_END
diff --git a/source/MaterialXGenMdl/Nodes/CombineNodeMdl.h b/source/MaterialXGenMdl/Nodes/CombineNodeMdl.h
deleted file mode 100644
index d8361820cf..0000000000
--- a/source/MaterialXGenMdl/Nodes/CombineNodeMdl.h
+++ /dev/null
@@ -1,26 +0,0 @@
-//
-// Copyright Contributors to the MaterialX Project
-// SPDX-License-Identifier: Apache-2.0
-//
-
-#ifndef MATERIALX_COMBINENODEMDL_H
-#define MATERIALX_COMBINENODEMDL_H
-
-#include
-
-#include
-
-MATERIALX_NAMESPACE_BEGIN
-
-/// Custom combine node implementation for MDL
-class MX_GENMDL_API CombineNodeMdl : public CombineNode
-{
- public:
- static ShaderNodeImplPtr create();
-
- void emitFunctionCall(const ShaderNode& node, GenContext& context, ShaderStage& stage) const override;
-};
-
-MATERIALX_NAMESPACE_END
-
-#endif
diff --git a/source/MaterialXGenMsl/MslShaderGenerator.cpp b/source/MaterialXGenMsl/MslShaderGenerator.cpp
index f88de98808..798c3b00b2 100644
--- a/source/MaterialXGenMsl/MslShaderGenerator.cpp
+++ b/source/MaterialXGenMsl/MslShaderGenerator.cpp
@@ -19,7 +19,6 @@
#include
#include
-#include
#include
#include
#include
@@ -84,19 +83,6 @@ MslShaderGenerator::MslShaderGenerator() :
};
registerImplementation(elementNames, SwitchNode::create);
- //
- elementNames = {
- "IM_combine2_vector2_" + MslShaderGenerator::TARGET,
- "IM_combine2_color4CF_" + MslShaderGenerator::TARGET,
- "IM_combine2_vector4VF_" + MslShaderGenerator::TARGET,
- "IM_combine2_vector4VV_" + MslShaderGenerator::TARGET,
- "IM_combine3_color3_" + MslShaderGenerator::TARGET,
- "IM_combine3_vector3_" + MslShaderGenerator::TARGET,
- "IM_combine4_color4_" + MslShaderGenerator::TARGET,
- "IM_combine4_vector4_" + MslShaderGenerator::TARGET,
- };
- registerImplementation(elementNames, CombineNode::create);
-
//
registerImplementation("IM_position_vector3_" + MslShaderGenerator::TARGET, HwPositionNode::create);
//
diff --git a/source/MaterialXGenOsl/OslShaderGenerator.cpp b/source/MaterialXGenOsl/OslShaderGenerator.cpp
index 922eb58fa0..6302230f74 100644
--- a/source/MaterialXGenOsl/OslShaderGenerator.cpp
+++ b/source/MaterialXGenOsl/OslShaderGenerator.cpp
@@ -9,7 +9,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -54,16 +53,6 @@ OslShaderGenerator::OslShaderGenerator() :
registerImplementation("IM_switch_matrix33I_" + OslShaderGenerator::TARGET, SwitchNode::create);
registerImplementation("IM_switch_matrix44I_" + OslShaderGenerator::TARGET, SwitchNode::create);
- //
- registerImplementation("IM_combine2_vector2_" + OslShaderGenerator::TARGET, CombineNode::create);
- registerImplementation("IM_combine2_color4CF_" + OslShaderGenerator::TARGET, CombineNode::create);
- registerImplementation("IM_combine2_vector4VF_" + OslShaderGenerator::TARGET, CombineNode::create);
- registerImplementation("IM_combine2_vector4VV_" + OslShaderGenerator::TARGET, CombineNode::create);
- registerImplementation("IM_combine3_color3_" + OslShaderGenerator::TARGET, CombineNode::create);
- registerImplementation("IM_combine3_vector3_" + OslShaderGenerator::TARGET, CombineNode::create);
- registerImplementation("IM_combine4_color4_" + OslShaderGenerator::TARGET, CombineNode::create);
- registerImplementation("IM_combine4_vector4_" + OslShaderGenerator::TARGET, CombineNode::create);
-
//
registerImplementation("IM_blur_float_" + OslShaderGenerator::TARGET, BlurNodeOsl::create);
registerImplementation("IM_blur_color3_" + OslShaderGenerator::TARGET, BlurNodeOsl::create);
diff --git a/source/MaterialXGenShader/Nodes/CombineNode.cpp b/source/MaterialXGenShader/Nodes/CombineNode.cpp
deleted file mode 100644
index 0237830c52..0000000000
--- a/source/MaterialXGenShader/Nodes/CombineNode.cpp
+++ /dev/null
@@ -1,143 +0,0 @@
-//
-// Copyright Contributors to the MaterialX Project
-// SPDX-License-Identifier: Apache-2.0
-//
-
-#include
-#include
-#include
-#include
-#include
-
-MATERIALX_NAMESPACE_BEGIN
-
-ShaderNodeImplPtr CombineNode::create()
-{
- return std::make_shared();
-}
-
-void CombineNode::emitFunctionCall(const ShaderNode& node, GenContext& context, ShaderStage& stage) const
-{
- DEFINE_SHADER_STAGE(stage, Stage::PIXEL)
- {
- const ShaderGenerator& shadergen = context.getShaderGenerator();
-
- const ShaderInput* in1 = node.getInput(0);
- const ShaderOutput* out = node.getOutput();
- if (!in1 || !out)
- {
- throw ExceptionShaderGenError("Node '" + node.getName() + "' is not a valid convert node");
- }
-
- // Check the node signature to determine which
- // type conversion to perform, and get the value
- // components to use for constructing the new value.
- //
- StringVec valueComponents;
- if (in1->getType() == Type::FLOAT)
- {
- // Get the components of the input values.
- const size_t numInputs = node.numInputs();
- valueComponents.resize(numInputs);
- for (size_t i = 0; i < numInputs; ++i)
- {
- const ShaderInput* input = node.getInput(i);
- valueComponents[i] = shadergen.getUpstreamResult(input, context);
- }
- }
- else if (in1->getType() == Type::COLOR3 || in1->getType() == Type::VECTOR3)
- {
- const ShaderInput* in2 = node.getInput(1);
- if (!in2 || in2->getType() != Type::FLOAT)
- {
- throw ExceptionShaderGenError("Node '" + node.getName() + "' is not a valid convert node");
- }
-
- // If in1 is unconnected we must declare a local variable
- // for it first, in order to access components from it below.
- string in1Variable = in1->getConnection() ? in1->getConnection()->getVariable() : in1->getVariable();
- if (!in1->getConnection())
- {
- string variableValue = in1->getValue() ? shadergen.getSyntax().getValue(in1->getType(), *in1->getValue()) : shadergen.getSyntax().getDefaultValue(in1->getType());
- shadergen.emitLine(shadergen.getSyntax().getTypeName(in1->getType()) + " " + in1Variable + " = " + variableValue, stage);
- }
-
- // Get components from in1
- const StringVec& in1Members = shadergen.getSyntax().getTypeSyntax(in1->getType()).getMembers();
- size_t memberSize = in1Members.size();
-
- // Get the components of the input values.
- if (memberSize)
- {
- valueComponents.resize(memberSize + 1);
- for (size_t i = 0; i < memberSize; i++)
- {
- valueComponents[i] = in1Variable + in1Members[i];
- }
- }
- else
- {
- memberSize = 1;
- valueComponents.resize(2);
- valueComponents[0] = in1Variable;
- }
- // Get component from in2
- valueComponents[memberSize] = shadergen.getUpstreamResult(in2, context);
- }
- else if (in1->getType() == Type::VECTOR2)
- {
- const ShaderInput* in2 = node.getInput(1);
- if (!in2 || (in2->getType() != Type::VECTOR2))
- {
- throw ExceptionShaderGenError("Node '" + node.getName() + "' is not a valid convert node");
- }
-
- // If in1 is unconnected we must declare a local variable
- // for it first, in order to access components from it below.
- string in1Variable = in1->getConnection() ? in1->getConnection()->getVariable() : in1->getVariable();
- if (!in1->getConnection())
- {
- string variableValue = in1->getValue() ? shadergen.getSyntax().getValue(in1->getType(), *in1->getValue()) : shadergen.getSyntax().getDefaultValue(in1->getType());
- shadergen.emitLine(shadergen.getSyntax().getTypeName(in1->getType()) + " " + in1Variable + " = " + variableValue, stage);
- }
-
- // If in2 is unconnected we must declare a local variable
- // for it first, in order to access components from it below.
- string in2Variable = in2->getConnection() ? in2->getConnection()->getVariable() : in2->getVariable();
- if (!in2->getConnection())
- {
- string variableValue = in2->getValue() ? shadergen.getSyntax().getValue(in2->getType(), *in2->getValue()) : shadergen.getSyntax().getDefaultValue(in2->getType());
- shadergen.emitLine(shadergen.getSyntax().getTypeName(in2->getType()) + " " + in2Variable + " = " + variableValue, stage);
- }
-
- // Get the components of the input values.
- valueComponents.resize(4);
-
- // Get components from in1.
- const StringVec& in1Members = shadergen.getSyntax().getTypeSyntax(in1->getType()).getMembers();
- valueComponents[0] = in1Variable + in1Members[0];
- valueComponents[1] = in1Variable + in1Members[1];
-
- // Get components from in2.
- const StringVec& in2Members = shadergen.getSyntax().getTypeSyntax(in2->getType()).getMembers();
- valueComponents[2] = in2Variable + in2Members[0];
- valueComponents[3] = in2Variable + in2Members[1];
- }
-
- if (valueComponents.empty())
- {
- throw ExceptionShaderGenError("Node '" + node.getName() + "' is not a valid convert node");
- }
-
- // Let the TypeSyntax construct the value from the components.
- const TypeSyntax& outTypeSyntax = shadergen.getSyntax().getTypeSyntax(out->getType());
- const string result = outTypeSyntax.getValue(valueComponents, false);
-
- shadergen.emitLineBegin(stage);
- shadergen.emitOutput(node.getOutput(), true, false, context, stage);
- shadergen.emitString(" = " + result, stage);
- shadergen.emitLineEnd(stage);
- }
-}
-
-MATERIALX_NAMESPACE_END
diff --git a/source/MaterialXGenShader/Nodes/CombineNode.h b/source/MaterialXGenShader/Nodes/CombineNode.h
deleted file mode 100644
index c77ba1396b..0000000000
--- a/source/MaterialXGenShader/Nodes/CombineNode.h
+++ /dev/null
@@ -1,24 +0,0 @@
-//
-// Copyright Contributors to the MaterialX Project
-// SPDX-License-Identifier: Apache-2.0
-//
-
-#ifndef MATERIALX_COMBINENODE_H
-#define MATERIALX_COMBINENODE_H
-
-#include
-
-MATERIALX_NAMESPACE_BEGIN
-
-/// Combine node implementation
-class MX_GENSHADER_API CombineNode : public ShaderNodeImpl
-{
- public:
- static ShaderNodeImplPtr create();
-
- void emitFunctionCall(const ShaderNode& node, GenContext& context, ShaderStage& stage) const override;
-};
-
-MATERIALX_NAMESPACE_END
-
-#endif