From 7b0405d7fada65c6d805a85f2ee02e7938a9988f Mon Sep 17 00:00:00 2001
From: Niklas Harrysson <niklas.harrysson@gmail.com>
Date: Fri, 30 Sep 2022 16:59:43 +0200
Subject: [PATCH] Remove operators 'add' and 'multiply'  for shader types
 (#1089)

This change list removes the `add` and `multiply` operator nodes for shader semantic types, as agreed on at the last TSC meeting.
---
 .../stdlib/genglsl/mx_add_surfaceshader.glsl  |  5 -
 .../mx_multiply_surfaceshader_color3.glsl     |  5 -
 .../mx_multiply_surfaceshader_float.glsl      |  5 -
 .../stdlib/genglsl/stdlib_genglsl_impl.mtlx   |  3 -
 .../stdlib/genmdl/stdlib_genmdl_impl.mtlx     |  3 -
 .../stdlib/genosl/stdlib_genosl_impl.mtlx     |  3 -
 libraries/stdlib/stdlib_defs.mtlx             | 45 ---------
 .../pbrlib/surfaceshader/shader_ops.mtlx      | 14 +--
 .../MaterialXGenMdl/mdl/materialx/stdlib.mdl  | 99 -------------------
 .../MaterialXGenGlsl/GenGlsl.cpp              |  2 +-
 .../MaterialXTest/MaterialXGenMdl/GenMdl.cpp  |  2 +-
 .../MaterialXTest/MaterialXGenOsl/GenOsl.cpp  |  2 +-
 .../MaterialXGenShader/GenShaderUtil.cpp      |  6 --
 13 files changed, 4 insertions(+), 190 deletions(-)
 delete mode 100644 libraries/stdlib/genglsl/mx_add_surfaceshader.glsl
 delete mode 100644 libraries/stdlib/genglsl/mx_multiply_surfaceshader_color3.glsl
 delete mode 100644 libraries/stdlib/genglsl/mx_multiply_surfaceshader_float.glsl

diff --git a/libraries/stdlib/genglsl/mx_add_surfaceshader.glsl b/libraries/stdlib/genglsl/mx_add_surfaceshader.glsl
deleted file mode 100644
index bb60c7126d..0000000000
--- a/libraries/stdlib/genglsl/mx_add_surfaceshader.glsl
+++ /dev/null
@@ -1,5 +0,0 @@
-void mx_add_surfaceshader(surfaceshader shader1, surfaceshader shader2, out surfaceshader returnshader)
-{
-    returnshader.color = shader1.color + shader2.color;
-    returnshader.transparency = shader1.transparency + shader2.transparency;
-}
diff --git a/libraries/stdlib/genglsl/mx_multiply_surfaceshader_color3.glsl b/libraries/stdlib/genglsl/mx_multiply_surfaceshader_color3.glsl
deleted file mode 100644
index dfdb041321..0000000000
--- a/libraries/stdlib/genglsl/mx_multiply_surfaceshader_color3.glsl
+++ /dev/null
@@ -1,5 +0,0 @@
-void mx_multiply_surfaceshader_color3(surfaceshader shader1, vec3 value, out surfaceshader returnshader)
-{
-    returnshader.color = shader1.color * value;
-    returnshader.transparency = shader1.transparency;
-}
diff --git a/libraries/stdlib/genglsl/mx_multiply_surfaceshader_float.glsl b/libraries/stdlib/genglsl/mx_multiply_surfaceshader_float.glsl
deleted file mode 100644
index 198b1a10d5..0000000000
--- a/libraries/stdlib/genglsl/mx_multiply_surfaceshader_float.glsl
+++ /dev/null
@@ -1,5 +0,0 @@
-void mx_multiply_surfaceshader_float(surfaceshader shader1, float value, out surfaceshader returnshader)
-{
-    returnshader.color = shader1.color * vec3(value);
-    returnshader.transparency = shader1.transparency * value;
-}
diff --git a/libraries/stdlib/genglsl/stdlib_genglsl_impl.mtlx b/libraries/stdlib/genglsl/stdlib_genglsl_impl.mtlx
index 47b95c1990..781f4527c4 100644
--- a/libraries/stdlib/genglsl/stdlib_genglsl_impl.mtlx
+++ b/libraries/stdlib/genglsl/stdlib_genglsl_impl.mtlx
@@ -222,7 +222,6 @@
   <implementation name="IM_add_matrix33FA_genglsl" nodedef="ND_add_matrix33FA" file="mx_add.inline" target="genglsl" />
   <implementation name="IM_add_matrix44_genglsl" nodedef="ND_add_matrix44" file="mx_add.inline" target="genglsl" />
   <implementation name="IM_add_matrix44FA_genglsl" nodedef="ND_add_matrix44FA" file="mx_add.inline" target="genglsl" />
-  <implementation name="IM_add_surfaceshader_genglsl" nodedef="ND_add_surfaceshader" function="mx_add_surfaceshader" file="mx_add_surfaceshader.glsl" target="genglsl" />
 
   <!-- <subtract> -->
   <implementation name="IM_subtract_float_genglsl" nodedef="ND_subtract_float" file="mx_subtract.inline" target="genglsl" />
@@ -255,8 +254,6 @@
   <implementation name="IM_multiply_vector4FA_genglsl" nodedef="ND_multiply_vector4FA" file="mx_multiply.inline" target="genglsl" />
   <implementation name="IM_multiply_matrix33_genglsl" nodedef="ND_multiply_matrix33" file="mx_multiply.inline" target="genglsl" />
   <implementation name="IM_multiply_matrix44_genglsl" nodedef="ND_multiply_matrix44" file="mx_multiply.inline" target="genglsl" />
-  <implementation name="IM_multiply_surfaceshaderF_genglsl" nodedef="ND_multiply_surfaceshaderF" function="mx_multiply_surfaceshader_float" file="mx_multiply_surfaceshader_float.glsl" target="genglsl" />
-  <implementation name="IM_multiply_surfaceshaderC_genglsl" nodedef="ND_multiply_surfaceshaderC" function="mx_multiply_surfaceshader_color3" file="mx_multiply_surfaceshader_color3.glsl" target="genglsl" />
 
   <!-- <divide> -->
   <implementation name="IM_divide_float_genglsl" nodedef="ND_divide_float" file="mx_divide.inline" target="genglsl" />
diff --git a/libraries/stdlib/genmdl/stdlib_genmdl_impl.mtlx b/libraries/stdlib/genmdl/stdlib_genmdl_impl.mtlx
index 0f89e7bdcc..bb792523b1 100644
--- a/libraries/stdlib/genmdl/stdlib_genmdl_impl.mtlx
+++ b/libraries/stdlib/genmdl/stdlib_genmdl_impl.mtlx
@@ -224,7 +224,6 @@
   <implementation name="IM_add_matrix33FA_genmdl" nodedef="ND_add_matrix33FA" sourcecode="{{in1}} + float3x3({{in2}})" target="genmdl" />
   <implementation name="IM_add_matrix44_genmdl" nodedef="ND_add_matrix44" sourcecode="{{in1}} + {{in2}}" target="genmdl" />
   <implementation name="IM_add_matrix44FA_genmdl" nodedef="ND_add_matrix44FA" sourcecode="{{in1}} + float4x4({{in2}})" target="genmdl" />
-  <implementation name="IM_add_surfaceshader_genmdl" nodedef="ND_add_surfaceshader" sourcecode="mx::stdlib::mx_add_surfaceshader({{in1}}, {{in2}})" target="genmdl" />
 
   <!-- <subtract> -->
   <implementation name="IM_subtract_float_genmdl" nodedef="ND_subtract_float" sourcecode="{{in1}} - {{in2}}" target="genmdl" />
@@ -257,8 +256,6 @@
   <implementation name="IM_multiply_vector4FA_genmdl" nodedef="ND_multiply_vector4FA" sourcecode="{{in1}} * {{in2}}" target="genmdl" />
   <implementation name="IM_multiply_matrix33_genmdl" nodedef="ND_multiply_matrix33" sourcecode="{{in1}} * {{in2}}" target="genmdl" />
   <implementation name="IM_multiply_matrix44_genmdl" nodedef="ND_multiply_matrix44" sourcecode="{{in1}} * {{in2}}" target="genmdl" />
-  <implementation name="IM_multiply_surfaceshaderF_genmdl" nodedef="ND_multiply_surfaceshaderF" sourcecode="mx::stdlib::mx_multiply_surfaceshaderF({{in1}}, {{in2}})" target="genmdl" />
-  <implementation name="IM_multiply_surfaceshaderC_genmdl" nodedef="ND_multiply_surfaceshaderC" sourcecode="mx::stdlib::mx_multiply_surfaceshaderC({{in1}}, {{in2}})" target="genmdl" />
 
   <!-- <divide> -->
   <implementation name="IM_divide_float_genmdl" nodedef="ND_divide_float" sourcecode="{{in1}} / {{in2}}" target="genmdl" />
diff --git a/libraries/stdlib/genosl/stdlib_genosl_impl.mtlx b/libraries/stdlib/genosl/stdlib_genosl_impl.mtlx
index b7a1b0363c..73bc5d709d 100644
--- a/libraries/stdlib/genosl/stdlib_genosl_impl.mtlx
+++ b/libraries/stdlib/genosl/stdlib_genosl_impl.mtlx
@@ -225,7 +225,6 @@
   <implementation name="IM_add_matrix33FA_genosl" nodedef="ND_add_matrix33FA" file="mx_add.inline" target="genosl" />
   <implementation name="IM_add_matrix44_genosl" nodedef="ND_add_matrix44" file="mx_add.inline" target="genosl" />
   <implementation name="IM_add_matrix44FA_genosl" nodedef="ND_add_matrix44FA" file="mx_add.inline" target="genosl" />
-  <implementation name="IM_add_surfaceshader_genosl" nodedef="ND_add_surfaceshader" file="mx_add.inline" target="genosl" />
 
   <!-- <subtract> -->
   <implementation name="IM_subtract_float_genosl" nodedef="ND_subtract_float" file="mx_subtract.inline" target="genosl" />
@@ -258,8 +257,6 @@
   <implementation name="IM_multiply_vector4FA_genosl" nodedef="ND_multiply_vector4FA" file="mx_multiply.inline" target="genosl" />
   <implementation name="IM_multiply_matrix33_genosl" nodedef="ND_multiply_matrix33" file="mx_multiply.inline" target="genosl" />
   <implementation name="IM_multiply_matrix44_genosl" nodedef="ND_multiply_matrix44" file="mx_multiply.inline" target="genosl" />
-  <implementation name="IM_multiply_surfaceshaderF_genosl" nodedef="ND_multiply_surfaceshaderF" file="mx_multiply.inline" target="genosl" />
-  <implementation name="IM_multiply_surfaceshaderC_genosl" nodedef="ND_multiply_surfaceshaderC" file="mx_multiply.inline" target="genosl" />
 
   <!-- <divide> -->
   <implementation name="IM_divide_float_genosl" nodedef="ND_divide_float" file="mx_divide.inline" target="genosl" />
diff --git a/libraries/stdlib/stdlib_defs.mtlx b/libraries/stdlib/stdlib_defs.mtlx
index d1f8c29468..1b8a3ca7b3 100644
--- a/libraries/stdlib/stdlib_defs.mtlx
+++ b/libraries/stdlib/stdlib_defs.mtlx
@@ -1185,21 +1185,6 @@
     <input name="in2" type="matrix44" value="0.0,0.0,0.0,0.0, 0.0,0.0,0.0,0.0, 0.0,0.0,0.0,0.0, 0.0,0.0,0.0,0.0" />
     <output name="out" type="matrix44" defaultinput="in1" />
   </nodedef>
-  <nodedef name="ND_add_surfaceshader" node="add" nodegroup="math">
-    <input name="in1" type="surfaceshader" value="" />
-    <input name="in2" type="surfaceshader" value="" />
-    <output name="out" type="surfaceshader" defaultinput="in1" />
-  </nodedef>
-  <nodedef name="ND_add_displacementshader" node="add" nodegroup="math">
-    <input name="in1" type="displacementshader" value="" />
-    <input name="in2" type="displacementshader" value="" />
-    <output name="out" type="displacementshader" defaultinput="in1" />
-  </nodedef>
-  <nodedef name="ND_add_volumeshader" node="add" nodegroup="math">
-    <input name="in1" type="volumeshader" value="" />
-    <input name="in2" type="volumeshader" value="" />
-    <output name="out" type="volumeshader" defaultinput="in1" />
-  </nodedef>
   <nodedef name="ND_add_color3FA" node="add" nodegroup="math">
     <input name="in1" type="color3" value="0.0, 0.0, 0.0" />
     <input name="in2" type="float" value="0.0" />
@@ -1386,36 +1371,6 @@
     <input name="in2" type="float" value="1.0" />
     <output name="out" type="vector4" defaultinput="in1" />
   </nodedef>
-  <nodedef name="ND_multiply_surfaceshaderF" node="multiply" nodegroup="math">
-    <input name="in1" type="surfaceshader" value="" />
-    <input name="in2" type="float" value="1.0" />
-    <output name="out" type="surfaceshader" defaultinput="in1" />
-  </nodedef>
-  <nodedef name="ND_multiply_displacementshaderF" node="multiply" nodegroup="math">
-    <input name="in1" type="displacementshader" value="" />
-    <input name="in2" type="float" value="1.0" />
-    <output name="out" type="displacementshader" defaultinput="in1" />
-  </nodedef>
-  <nodedef name="ND_multiply_volumeshaderF" node="multiply" nodegroup="math">
-    <input name="in1" type="volumeshader" value="" />
-    <input name="in2" type="float" value="1.0" />
-    <output name="out" type="volumeshader" defaultinput="in1" />
-  </nodedef>
-  <nodedef name="ND_multiply_surfaceshaderC" node="multiply" nodegroup="math">
-    <input name="in1" type="surfaceshader" value="" />
-    <input name="in2" type="color3" value="1.0, 1.0, 1.0" />
-    <output name="out" type="surfaceshader" defaultinput="in1" />
-  </nodedef>
-  <nodedef name="ND_multiply_volumeshaderC" node="multiply" nodegroup="math">
-    <input name="in1" type="volumeshader" value="" />
-    <input name="in2" type="color3" value="1.0, 1.0, 1.0" />
-    <output name="out" type="volumeshader" defaultinput="in1" />
-  </nodedef>
-  <nodedef name="ND_multiply_displacementshaderV" node="multiply" nodegroup="math">
-    <input name="in1" type="displacementshader" value="" />
-    <input name="in2" type="vector3" value="1.0, 1.0, 1.0" />
-    <output name="out" type="displacementshader" defaultinput="in1" />
-  </nodedef>
 
   <!--
     Node: <divide>
diff --git a/resources/Materials/TestSuite/pbrlib/surfaceshader/shader_ops.mtlx b/resources/Materials/TestSuite/pbrlib/surfaceshader/shader_ops.mtlx
index c4fa986647..ad82daa8a9 100644
--- a/resources/Materials/TestSuite/pbrlib/surfaceshader/shader_ops.mtlx
+++ b/resources/Materials/TestSuite/pbrlib/surfaceshader/shader_ops.mtlx
@@ -1,18 +1,6 @@
 <?xml version="1.0"?>
 <materialx version="1.38">
   <nodegraph name="nodegraph1">
-    <add name="add_surface_shader" type="surfaceshader">
-      <input name="in1" type="surfaceshader" value="" nodename="multiply_surface_shaderFloat" />
-      <input name="in2" type="surfaceshader" value="" nodename="multiply_surface_shaderColor" />
-    </add>
-    <multiply name="multiply_surface_shaderFloat" type="surfaceshader">
-      <input name="in1" type="surfaceshader" value="" nodename="standard_surface1" />
-      <input name="in2" type="float" value="0.4000" />
-    </multiply>
-    <multiply name="multiply_surface_shaderColor" type="surfaceshader">
-      <input name="in1" type="surfaceshader" value="" nodename="mix_surface_shader" />
-      <input name="in2" type="color3" value="1.0000, 0.0, 0.0" />
-    </multiply>
     <mix name="mix_surface_shader" type="surfaceshader">
       <input name="fg" type="surfaceshader" value="" nodename="standard_surface1" />
       <input name="bg" type="surfaceshader" value="" nodename="standard_surface2" />
@@ -24,6 +12,6 @@
     <standard_surface name="standard_surface2" type="surfaceshader">
       <input name="base_color" type="color3" value="0.1, 0.1, 0.85" />
     </standard_surface>
-    <output name="out" type="surfaceshader" nodename="add_surface_shader" />
+    <output name="out" type="surfaceshader" nodename="mix_surface_shader" />
   </nodegraph>
 </materialx>
diff --git a/source/MaterialXGenMdl/mdl/materialx/stdlib.mdl b/source/MaterialXGenMdl/mdl/materialx/stdlib.mdl
index c7572f854b..5e022ec7d7 100644
--- a/source/MaterialXGenMdl/mdl/materialx/stdlib.mdl
+++ b/source/MaterialXGenMdl/mdl/materialx/stdlib.mdl
@@ -1322,105 +1322,6 @@ export float mx_time_float(
 	return ::state::animation_time();
 }
 
-export material mx_add_surfaceshader(
-	material mxp_in1,
-	material mxp_in2 [[ anno::unused() ]]
-)
-	[[
-		anno::description("Node Group: math")
-	]]
-= mxp_in1; // TODO 
-
-export material mx_add_displacementshader(
-	material mxp_in1,
-	material mxp_in2 [[ anno::unused() ]]
-)
-	[[
-		anno::description("Node Group: math")
-	]]
-= mxp_in1; // TODO 
-
-export material mx_add_volumeshader(
-	material mxp_in1,
-	material mxp_in2 [[ anno::unused() ]]
-)
-	[[
-		anno::description("Node Group: math")
-	]]
-= mxp_in1; // TODO 
-
-export material mx_multiply_surfaceshaderF(
-	material mxp_in1,
-	float mxp_in2 = float(1.0)
-    [[
-        anno::unused()
-    ]]
-)
-	[[
-		anno::description("Node Group: math")
-	]]
-= mxp_in1; // TODO 
-
-export material mx_multiply_displacementshaderF(
-	material mxp_in1,
-	float mxp_in2 = float(1.0)
-    [[
-        anno::unused()
-    ]]
-)
-	[[
-		anno::description("Node Group: math")
-	]]
-= mxp_in1; // TODO 
-
-export material mx_multiply_volumeshaderF(
-	material mxp_in1,
-	float mxp_in2 = float(1.0)
-    [[
-        anno::unused()
-    ]]
-)
-	[[
-		anno::description("Node Group: math")
-	]]
-= mxp_in1; // TODO 
-
-export material mx_multiply_surfaceshaderC(
-	material mxp_in1,
-	color mxp_in2 = color(1.0, 1.0, 1.0)
-    [[
-        anno::unused()
-    ]]
-)
-	[[
-		anno::description("Node Group: math")
-	]]
-= mxp_in1; // TODO 
-
-export material mx_multiply_volumeshaderC(
-	material mxp_in1,
-	color mxp_in2 = color(1.0, 1.0, 1.0)
-    [[
-        anno::unused()
-    ]]
-)
-	[[
-		anno::description("Node Group: math")
-	]]
-= mxp_in1; // TODO 
-
-export material mx_multiply_displacementshaderV(
-	material mxp_in1,
-	float3 mxp_in2 = float3(1.0, 1.0, 1.0)
-    [[
-        anno::unused()
-    ]]
-)
-	[[
-		anno::description("Node Group: math")
-	]]
-= mxp_in1; // TODO 
-
 export color mx_modulo_color3(
 	color mxp_in1 = color(0.0, 0.0, 0.0),
 	color mxp_in2 = color(1.0, 1.0, 1.0)
diff --git a/source/MaterialXTest/MaterialXGenGlsl/GenGlsl.cpp b/source/MaterialXTest/MaterialXGenGlsl/GenGlsl.cpp
index bd40e1bd70..8033378533 100644
--- a/source/MaterialXTest/MaterialXGenGlsl/GenGlsl.cpp
+++ b/source/MaterialXTest/MaterialXGenGlsl/GenGlsl.cpp
@@ -85,7 +85,7 @@ TEST_CASE("GenShader: GLSL Implementation Check", "[genglsl]")
 
     mx::StringSet generatorSkipNodeTypes;
     mx::StringSet generatorSkipNodeDefs;
-    GenShaderUtil::checkImplementations(context, generatorSkipNodeTypes, generatorSkipNodeDefs, 54);
+    GenShaderUtil::checkImplementations(context, generatorSkipNodeTypes, generatorSkipNodeDefs, 48);
 }
 
 TEST_CASE("GenShader: GLSL Unique Names", "[genglsl]")
diff --git a/source/MaterialXTest/MaterialXGenMdl/GenMdl.cpp b/source/MaterialXTest/MaterialXGenMdl/GenMdl.cpp
index 6b9f814bb5..92d4bab534 100644
--- a/source/MaterialXTest/MaterialXGenMdl/GenMdl.cpp
+++ b/source/MaterialXTest/MaterialXGenMdl/GenMdl.cpp
@@ -92,7 +92,7 @@ TEST_CASE("GenShader: MDL Implementation Check", "[genmdl]")
     generatorSkipNodeTypes.insert("light");
     mx::StringSet generatorSkipNodeDefs;
 
-    GenShaderUtil::checkImplementations(context, generatorSkipNodeTypes, generatorSkipNodeDefs, 55);
+    GenShaderUtil::checkImplementations(context, generatorSkipNodeTypes, generatorSkipNodeDefs, 49);
 }
 
 void MdlShaderGeneratorTester::compileSource(const std::vector<mx::FilePath>& sourceCodePaths)
diff --git a/source/MaterialXTest/MaterialXGenOsl/GenOsl.cpp b/source/MaterialXTest/MaterialXGenOsl/GenOsl.cpp
index 37e8d57adf..70738bfb2c 100644
--- a/source/MaterialXTest/MaterialXGenOsl/GenOsl.cpp
+++ b/source/MaterialXTest/MaterialXGenOsl/GenOsl.cpp
@@ -89,7 +89,7 @@ TEST_CASE("GenShader: OSL Implementation Check", "[genosl]")
     generatorSkipNodeTypes.insert("light");
     mx::StringSet generatorSkipNodeDefs;
 
-    GenShaderUtil::checkImplementations(context, generatorSkipNodeTypes, generatorSkipNodeDefs, 55);
+    GenShaderUtil::checkImplementations(context, generatorSkipNodeTypes, generatorSkipNodeDefs, 49);
 }
 
 TEST_CASE("GenShader: OSL Unique Names", "[genosl]")
diff --git a/source/MaterialXTest/MaterialXGenShader/GenShaderUtil.cpp b/source/MaterialXTest/MaterialXGenShader/GenShaderUtil.cpp
index c1e6769750..7f2f9c9f08 100644
--- a/source/MaterialXTest/MaterialXGenShader/GenShaderUtil.cpp
+++ b/source/MaterialXTest/MaterialXGenShader/GenShaderUtil.cpp
@@ -109,13 +109,7 @@ void checkImplementations(mx::GenContext& context,
     // Explicit set of node defs to skip temporarily
     mx::StringSet skipNodeDefs =
     {
-        "ND_add_displacementshader",
-        "ND_add_volumeshader",
         "ND_add_vdf",
-        "ND_multiply_displacementshaderF",
-        "ND_multiply_displacementshaderV",
-        "ND_multiply_volumeshaderF",
-        "ND_multiply_volumeshaderC",
         "ND_multiply_vdfF",
         "ND_multiply_vdfC",
         "ND_mix_displacementshader",