From d5634c83335766952026f6120dcea7b902894c44 Mon Sep 17 00:00:00 2001 From: Jonathan Stone Date: Tue, 17 Oct 2023 10:18:51 -0700 Subject: [PATCH 1/5] Refinements to Graph class - Move the ordered group vector in Graph::createNodeUIList to a constant at module scope. - Use the joinStrings helper function in Graph::addPinPopup to simplify its implementation. - Update comments to align with coding conventions. --- source/MaterialXGraphEditor/Graph.cpp | 105 ++++++++++++-------------- 1 file changed, 48 insertions(+), 57 deletions(-) diff --git a/source/MaterialXGraphEditor/Graph.cpp b/source/MaterialXGraphEditor/Graph.cpp index 439f720e2e..889789706b 100644 --- a/source/MaterialXGraphEditor/Graph.cpp +++ b/source/MaterialXGraphEditor/Graph.cpp @@ -23,6 +23,32 @@ const ImVec2 DEFAULT_NODE_SIZE = ImVec2(138, 116); const int DEFAULT_ALPHA = 255; const int FILTER_ALPHA = 50; +const std::array NODE_GROUP_ORDER = +{ + "texture2d", + "texture3d", + "procedural", + "procedural2d", + "procedural3d", + "geometric", + "translation", + "convolution2d", + "math", + "adjustment", + "compositing", + "conditional", + "channel", + "organization", + "global", + "application", + "material", + "shader", + "pbr", + "light", + "colortransform", + "none" +}; + // Based on ImRect_Expanded function in ImGui Node Editor blueprints-example.cpp ImRect expandImRect(const ImRect& rect, float x, float y) { @@ -1220,41 +1246,15 @@ void Graph::createNodeUIList(mx::DocumentPtr doc) { _nodesToAdd.clear(); - std::vector ordered_groups = { - "texture2d", - "texture3d", - "procedural", - "procedural2d", - "procedural3d", - "geometric", - "translation", - "convolution2d", - "math", - "adjustment", - "compositing", - "conditional", - "channel", - "organization", - "global", - "application", - "material", - "shader", - "pbr", - "light", - "colortransform", - "no_group" - }; - auto nodeDefs = doc->getNodeDefs(); std::unordered_map> groupToNodeDef; for (const auto& nodeDef : nodeDefs) { std::string group = nodeDef->getNodeGroup(); - if (group.empty()) { - group = "no_group"; + group = NODE_GROUP_ORDER.back(); } if (groupToNodeDef.find(group) == groupToNodeDef.end()) @@ -1264,7 +1264,7 @@ void Graph::createNodeUIList(mx::DocumentPtr doc) groupToNodeDef[group].push_back(nodeDef); } - for (const auto& group : ordered_groups) + for (const auto& group : NODE_GROUP_ORDER) { auto it = groupToNodeDef.find(group); if (it != groupToNodeDef.end()) @@ -2526,7 +2526,7 @@ void Graph::setDefaults(mx::InputPtr input) void Graph::addLink(ed::PinId startPinId, ed::PinId endPinId) { - // prefer to assume left to right - start is an output, end is an input; swap if inaccurate + // Prefer to assume left to right - start is an output, end is an input; swap if inaccurate if (UiPinPtr inputPin = getPin(endPinId); inputPin && inputPin->_kind != ed::PinKind::Input) { auto tmp = startPinId; @@ -2575,7 +2575,7 @@ void Graph::addLink(ed::PinId startPinId, ed::PinId endPinId) return; } - // make sure there is an implementation for node + // Make sure there is an implementation for node const mx::ShaderGenerator& shadergen = _renderer->getGenContext().getShaderGenerator(); // Prevent direct connecting from input to output @@ -2613,7 +2613,7 @@ void Graph::addLink(ed::PinId startPinId, ed::PinId endPinId) { if (linksItr->_endAttr == end_attr) { - // found existing link - remove it; adapted from deleteLink + // Found existing link - remove it; adapted from deleteLink // note: ed::BreakLinks doesn't work as the order ends up inaccurate deleteLinkInfo(linksItr->_startAttr, linksItr->_endAttr); _currLinks.erase(linksItr); @@ -2644,7 +2644,8 @@ void Graph::addLink(ed::PinId startPinId, ed::PinId endPinId) if (pin->_pinId == inputPinId) { addNodeInput(uiDownNode, pin->_input); - // update value to be empty + + // Update value to be empty if (uiDownNode->getNode() && uiDownNode->getNode()->getType() == mx::SURFACE_SHADER_TYPE_STRING) { if (uiUpNode->getOutput() != nullptr) @@ -2657,12 +2658,11 @@ void Graph::addLink(ed::PinId startPinId, ed::PinId endPinId) } else { - // node graph if (uiUpNode->getNodeGraph() != nullptr) { for (UiPinPtr outPin : uiUpNode->outputPins) { - // set pin connection to correct output + // Set pin connection to correct output if (outPin->_pinId == outputPinId) { mx::OutputPtr outputs = uiUpNode->getNodeGraph()->getOutput(outPin->_name); @@ -2689,10 +2689,6 @@ void Graph::addLink(ed::PinId startPinId, ed::PinId endPinId) mx::NodePtr upstreamNode = _graphNodes[upNode]->getNode(); mx::NodeDefPtr upstreamNodeDef = upstreamNode->getNodeDef(); bool isMultiOutput = upstreamNodeDef ? upstreamNodeDef->getOutputs().size() > 1 : false; - - // This is purely to avoid adding a reference to an update node only 1 output, - // as currently validation consides adding this an error. Otherwise - // it will add an "output" attribute all the time. if (!isMultiOutput) { pin->_input->setConnectedNode(uiUpNode->getNode()); @@ -2701,7 +2697,7 @@ void Graph::addLink(ed::PinId startPinId, ed::PinId endPinId) { for (UiPinPtr outPin : _graphNodes[upNode]->outputPins) { - // set pin connection to correct output + // Set pin connection to correct output if (outPin->_pinId == outputPinId) { mx::OutputPtr outputs = uiUpNode->getNode()->getOutput(outPin->_name); @@ -2718,7 +2714,7 @@ void Graph::addLink(ed::PinId startPinId, ed::PinId endPinId) { for (UiPinPtr outPin : uiUpNode->outputPins) { - // set pin connection to correct output + // Set pin connection to correct output if (outPin->_pinId == outputPinId) { mx::OutputPtr outputs = uiUpNode->getNodeGraph()->getOutput(outPin->_name); @@ -2729,14 +2725,14 @@ void Graph::addLink(ed::PinId startPinId, ed::PinId endPinId) } } - pin->setConnected(true); pin->_input->removeAttribute(mx::ValueElement::VALUE_ATTRIBUTE); connectingInput = pin->_input; break; } } - // create new edge and set edge information + + // Create new edge and set edge information createEdge(_graphNodes[upNode], _graphNodes[downNode], connectingInput); } else if (_graphNodes[downNode]->getOutput() != nullptr) @@ -2744,19 +2740,19 @@ void Graph::addLink(ed::PinId startPinId, ed::PinId endPinId) mx::InputPtr connectingInput = nullptr; _graphNodes[downNode]->getOutput()->setConnectedNode(_graphNodes[upNode]->getNode()); - // create new edge and set edge information + // Create new edge and set edge information createEdge(_graphNodes[upNode], _graphNodes[downNode], connectingInput); } else { - // create new edge and set edge info + // Create new edge and set edge info UiEdge newEdge = UiEdge(_graphNodes[upNode], _graphNodes[downNode], nullptr); if (!edgeExists(newEdge)) { _graphNodes[downNode]->edges.push_back(newEdge); _currEdge.push_back(newEdge); - // update input node num and output connections + // Update input node num and output connections _graphNodes[downNode]->setInputNodeNum(1); _graphNodes[upNode]->setOutputConnection(_graphNodes[downNode]); } @@ -3779,23 +3775,18 @@ void Graph::addPinPopup() { ed::Suspend(); UiPinPtr pin = getPin(ed::GetHoveredPin()); - std::string connected = ""; - std::string value = ""; + std::string connected; + std::string value; if (pin->_connected) { - connected = "\nConnected to"; - int size = static_cast(pin->getConnections().size()); - for (int i = 0; i < size; i++) + mx::StringVec connectedNames; + for (UiPinPtr connectedPin : pin->getConnections()) { - UiPinPtr connectedPin = pin->getConnections()[i]; - connected = connected + " " + connectedPin->_name; - if (i != size - 1) - { - connected = connected + ","; - } + connectedNames.push_back(connectedPin->_name); } + connected = "\nConnected to " + mx::joinStrings(connectedNames, ", "); } - else if (pin->_input != nullptr) + else if (pin->_input) { value = "\nValue: " + pin->_input->getValueString(); } From 1c78f460e582a83d0b79117428996b74b6b7b60c Mon Sep 17 00:00:00 2001 From: nicolassavva-autodesk <61437351+nicolassavva-autodesk@users.noreply.github.com> Date: Wed, 18 Oct 2023 16:16:03 -0700 Subject: [PATCH 2/5] Remove unused uniforms in HW generation for heighttonormal (#1576) This PR addresses the undesirable shaderGen for Metal and GLSL targets when using the HeightToNormalNode. The parent ConvolutionNode class emits a couple of filter kernel arrays, for box and gaussian weights, that may cause shader compiler issues when they remain unused. --- source/MaterialXGenGlsl/Nodes/HeightToNormalNodeGlsl.cpp | 5 +++++ source/MaterialXGenGlsl/Nodes/HeightToNormalNodeGlsl.h | 2 ++ source/MaterialXGenMsl/Nodes/HeightToNormalNodeMsl.cpp | 5 +++++ source/MaterialXGenMsl/Nodes/HeightToNormalNodeMsl.h | 2 ++ 4 files changed, 14 insertions(+) diff --git a/source/MaterialXGenGlsl/Nodes/HeightToNormalNodeGlsl.cpp b/source/MaterialXGenGlsl/Nodes/HeightToNormalNodeGlsl.cpp index 157cd5be04..e98a4ab75c 100644 --- a/source/MaterialXGenGlsl/Nodes/HeightToNormalNodeGlsl.cpp +++ b/source/MaterialXGenGlsl/Nodes/HeightToNormalNodeGlsl.cpp @@ -33,6 +33,11 @@ ShaderNodeImplPtr HeightToNormalNodeGlsl::create() return std::make_shared(); } +void HeightToNormalNodeGlsl::createVariables(const ShaderNode&, GenContext&, Shader&) const +{ + // Default filter kernels from ConvolutionNode are not used by this derived class. +} + void HeightToNormalNodeGlsl::computeSampleOffsetStrings(const string& sampleSizeName, const string& offsetTypeString, unsigned int, StringVec& offsetStrings) const { diff --git a/source/MaterialXGenGlsl/Nodes/HeightToNormalNodeGlsl.h b/source/MaterialXGenGlsl/Nodes/HeightToNormalNodeGlsl.h index 638623ea43..fc6e6b421f 100644 --- a/source/MaterialXGenGlsl/Nodes/HeightToNormalNodeGlsl.h +++ b/source/MaterialXGenGlsl/Nodes/HeightToNormalNodeGlsl.h @@ -18,6 +18,8 @@ class MX_GENGLSL_API HeightToNormalNodeGlsl : public ConvolutionNode public: static ShaderNodeImplPtr create(); + void createVariables(const ShaderNode&, GenContext&, Shader& shader) const override; + void emitFunctionDefinition(const ShaderNode& node, GenContext& context, ShaderStage& stage) const override; void emitFunctionCall(const ShaderNode& node, GenContext& context, ShaderStage& stage) const override; diff --git a/source/MaterialXGenMsl/Nodes/HeightToNormalNodeMsl.cpp b/source/MaterialXGenMsl/Nodes/HeightToNormalNodeMsl.cpp index 12b1a9675c..7282e5e9df 100644 --- a/source/MaterialXGenMsl/Nodes/HeightToNormalNodeMsl.cpp +++ b/source/MaterialXGenMsl/Nodes/HeightToNormalNodeMsl.cpp @@ -33,6 +33,11 @@ ShaderNodeImplPtr HeightToNormalNodeMsl::create() return std::make_shared(); } +void HeightToNormalNodeMsl::createVariables(const ShaderNode&, GenContext&, Shader&) const +{ + // Default filter kernels from ConvolutionNode are not used by this derived class. +} + void HeightToNormalNodeMsl::computeSampleOffsetStrings(const string& sampleSizeName, const string& offsetTypeString, unsigned int, StringVec& offsetStrings) const { diff --git a/source/MaterialXGenMsl/Nodes/HeightToNormalNodeMsl.h b/source/MaterialXGenMsl/Nodes/HeightToNormalNodeMsl.h index 752c411ed5..035ad300b4 100644 --- a/source/MaterialXGenMsl/Nodes/HeightToNormalNodeMsl.h +++ b/source/MaterialXGenMsl/Nodes/HeightToNormalNodeMsl.h @@ -18,6 +18,8 @@ class MX_GENMSL_API HeightToNormalNodeMsl : public ConvolutionNode public: static ShaderNodeImplPtr create(); + void createVariables(const ShaderNode&, GenContext&, Shader& shader) const override; + void emitFunctionDefinition(const ShaderNode& node, GenContext& context, ShaderStage& stage) const override; void emitFunctionCall(const ShaderNode& node, GenContext& context, ShaderStage& stage) const override; From 78d27622a0edc00f4936d7d7c613b29f5d181fe0 Mon Sep 17 00:00:00 2001 From: Rasmus Bonnedal Date: Thu, 19 Oct 2023 02:57:37 +0200 Subject: [PATCH 3/5] Update overlay node logic to match reference (#1539) This commit changes the overlay node to match the overlay operation in the Adobe PDF spec. It also changes the implementation to graph based instead of code snippets. --- .../Specification/MaterialX.Specification.md | 2 +- libraries/stdlib/genglsl/mx_overlay.glsl | 25 ---- .../stdlib/genglsl/mx_overlay_color3.glsl | 6 - .../stdlib/genglsl/mx_overlay_color4.glsl | 6 - .../stdlib/genglsl/stdlib_genglsl_impl.mtlx | 5 - .../stdlib/genmdl/stdlib_genmdl_impl.mtlx | 5 - .../stdlib/genmsl/stdlib_genmsl_impl.mtlx | 5 - libraries/stdlib/genosl/mx_overlay_color3.osl | 16 --- libraries/stdlib/genosl/mx_overlay_color4.osl | 22 ---- .../stdlib/genosl/stdlib_genosl_impl.mtlx | 5 - libraries/stdlib/stdlib_defs.mtlx | 4 +- libraries/stdlib/stdlib_ng.mtlx | 112 ++++++++++++++++++ .../stdlib/compositing/compositing.mtlx | 12 +- .../MaterialXGenMdl/mdl/materialx/stdlib.mdl | 60 ---------- 14 files changed, 121 insertions(+), 164 deletions(-) delete mode 100644 libraries/stdlib/genglsl/mx_overlay.glsl delete mode 100644 libraries/stdlib/genglsl/mx_overlay_color3.glsl delete mode 100644 libraries/stdlib/genglsl/mx_overlay_color4.glsl delete mode 100644 libraries/stdlib/genosl/mx_overlay_color3.osl delete mode 100644 libraries/stdlib/genosl/mx_overlay_color4.osl diff --git a/documents/Specification/MaterialX.Specification.md b/documents/Specification/MaterialX.Specification.md index 4b99956fb4..895b4756c5 100644 --- a/documents/Specification/MaterialX.Specification.md +++ b/documents/Specification/MaterialX.Specification.md @@ -1496,7 +1496,7 @@ Blend nodes take two 1-4 channel inputs and apply the same operator to all chann | **`burn`** | 1-(1-B)/F | float, colorN | | **`dodge`** | B/(1-F) | float, colorN | | **`screen`** | 1-(1-F)(1-B) | float, colorN | -| **`overlay`** | 2FB if F<0.5;
1-(1-F)(1-B) if F>=0.5 | float, colorN | +| **`overlay`** | 2FB if B<0.5;
1-2(1-F)(1-B) if B>=0.5 | float, colorN | #### Merge Nodes diff --git a/libraries/stdlib/genglsl/mx_overlay.glsl b/libraries/stdlib/genglsl/mx_overlay.glsl deleted file mode 100644 index e1bec8ac44..0000000000 --- a/libraries/stdlib/genglsl/mx_overlay.glsl +++ /dev/null @@ -1,25 +0,0 @@ -float mx_overlay(float fg, float bg) -{ - return (fg < 0.5) ? (2.0 * fg * bg) : (1.0 - (1.0 - fg) * (1.0 - bg)); -} - -vec2 mx_overlay(vec2 fg, vec2 bg) -{ - return vec2(mx_overlay(fg.r, bg.r), - mx_overlay(fg.g, bg.g)); -} - -vec3 mx_overlay(vec3 fg, vec3 bg) -{ - return vec3(mx_overlay(fg.r, bg.r), - mx_overlay(fg.g, bg.g), - mx_overlay(fg.b, bg.b)); -} - -vec4 mx_overlay(vec4 fg, vec4 bg) -{ - return vec4(mx_overlay(fg.r, bg.r), - mx_overlay(fg.g, bg.g), - mx_overlay(fg.b, bg.b), - mx_overlay(fg.a, bg.a)); -} diff --git a/libraries/stdlib/genglsl/mx_overlay_color3.glsl b/libraries/stdlib/genglsl/mx_overlay_color3.glsl deleted file mode 100644 index 3b6ae67804..0000000000 --- a/libraries/stdlib/genglsl/mx_overlay_color3.glsl +++ /dev/null @@ -1,6 +0,0 @@ -#include "mx_overlay.glsl" - -void mx_overlay_color3(vec3 fg, vec3 bg, float mix, out vec3 result) -{ - result = mix * mx_overlay(fg, bg) + (1.0-mix) * bg; -} diff --git a/libraries/stdlib/genglsl/mx_overlay_color4.glsl b/libraries/stdlib/genglsl/mx_overlay_color4.glsl deleted file mode 100644 index 411e0da372..0000000000 --- a/libraries/stdlib/genglsl/mx_overlay_color4.glsl +++ /dev/null @@ -1,6 +0,0 @@ -#include "mx_overlay.glsl" - -void mx_overlay_color4(vec4 fg, vec4 bg, float mix, out vec4 result) -{ - result = mix * mx_overlay(fg, bg) + (1.0-mix) * bg; -} diff --git a/libraries/stdlib/genglsl/stdlib_genglsl_impl.mtlx b/libraries/stdlib/genglsl/stdlib_genglsl_impl.mtlx index 90d3120bd7..07aae6ce66 100644 --- a/libraries/stdlib/genglsl/stdlib_genglsl_impl.mtlx +++ b/libraries/stdlib/genglsl/stdlib_genglsl_impl.mtlx @@ -560,11 +560,6 @@ - - - - - diff --git a/libraries/stdlib/genmdl/stdlib_genmdl_impl.mtlx b/libraries/stdlib/genmdl/stdlib_genmdl_impl.mtlx index ab0d776936..0081dec924 100644 --- a/libraries/stdlib/genmdl/stdlib_genmdl_impl.mtlx +++ b/libraries/stdlib/genmdl/stdlib_genmdl_impl.mtlx @@ -568,11 +568,6 @@ - - - - - diff --git a/libraries/stdlib/genmsl/stdlib_genmsl_impl.mtlx b/libraries/stdlib/genmsl/stdlib_genmsl_impl.mtlx index fcab8fd69d..dea1c49636 100644 --- a/libraries/stdlib/genmsl/stdlib_genmsl_impl.mtlx +++ b/libraries/stdlib/genmsl/stdlib_genmsl_impl.mtlx @@ -560,11 +560,6 @@ - - - - - diff --git a/libraries/stdlib/genosl/mx_overlay_color3.osl b/libraries/stdlib/genosl/mx_overlay_color3.osl deleted file mode 100644 index 387653fe05..0000000000 --- a/libraries/stdlib/genosl/mx_overlay_color3.osl +++ /dev/null @@ -1,16 +0,0 @@ -float overlay(float fg, float bg) -{ - return (fg < 0.5) ? (2 * fg * bg) : (1 - (1 - fg) * (1 - bg)); -} - -color overlay(color fg, color bg) -{ - return color(overlay(fg[0], bg[0]), - overlay(fg[1], bg[1]), - overlay(fg[2], bg[2])); -} - -void mx_overlay_color3(color fg, color bg, float mix, output color out) -{ - out = mix * overlay(fg, bg) + (1-mix) * bg; -} diff --git a/libraries/stdlib/genosl/mx_overlay_color4.osl b/libraries/stdlib/genosl/mx_overlay_color4.osl deleted file mode 100644 index 1ae6a72c15..0000000000 --- a/libraries/stdlib/genosl/mx_overlay_color4.osl +++ /dev/null @@ -1,22 +0,0 @@ -float overlay(float fg, float bg) -{ - return (fg < 0.5) ? (2 * fg * bg) : (1 - (1 - fg) * (1 - bg)); -} - -color overlay(color fg, color bg) -{ - return color(overlay(fg[0], bg[0]), - overlay(fg[1], bg[1]), - overlay(fg[2], bg[2])); -} - -color4 overlay(color4 fg, color4 bg) -{ - return color4(overlay(fg.rgb, bg.rgb), - overlay(fg.a, bg.a)); -} - -void mx_overlay_color4(color4 fg, color4 bg, float mix, output color4 out) -{ - out = mix * overlay(fg, bg) + (1-mix) * bg; -} diff --git a/libraries/stdlib/genosl/stdlib_genosl_impl.mtlx b/libraries/stdlib/genosl/stdlib_genosl_impl.mtlx index 1ba22ef6a3..42828ce641 100644 --- a/libraries/stdlib/genosl/stdlib_genosl_impl.mtlx +++ b/libraries/stdlib/genosl/stdlib_genosl_impl.mtlx @@ -561,11 +561,6 @@ - - - - - diff --git a/libraries/stdlib/stdlib_defs.mtlx b/libraries/stdlib/stdlib_defs.mtlx index bb5e8edd1f..3f187e2be1 100644 --- a/libraries/stdlib/stdlib_defs.mtlx +++ b/libraries/stdlib/stdlib_defs.mtlx @@ -3343,8 +3343,8 @@ diff --git a/libraries/stdlib/stdlib_ng.mtlx b/libraries/stdlib/stdlib_ng.mtlx index e031e72412..4983aaa477 100644 --- a/libraries/stdlib/stdlib_ng.mtlx +++ b/libraries/stdlib/stdlib_ng.mtlx @@ -3391,6 +3391,118 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/resources/Materials/TestSuite/stdlib/compositing/compositing.mtlx b/resources/Materials/TestSuite/stdlib/compositing/compositing.mtlx index e7a742c581..b125e70dca 100644 --- a/resources/Materials/TestSuite/stdlib/compositing/compositing.mtlx +++ b/resources/Materials/TestSuite/stdlib/compositing/compositing.mtlx @@ -183,24 +183,24 @@ - - + + - - + + - - + + diff --git a/source/MaterialXGenMdl/mdl/materialx/stdlib.mdl b/source/MaterialXGenMdl/mdl/materialx/stdlib.mdl index bce4a44ea5..8089feb7f0 100644 --- a/source/MaterialXGenMdl/mdl/materialx/stdlib.mdl +++ b/source/MaterialXGenMdl/mdl/materialx/stdlib.mdl @@ -2777,66 +2777,6 @@ export color4 mx_screen_color4( return color4(rgb,a); } -float mx_overlay(float mxp_fg, float mxp_bg) -{ - return (mxp_fg < 0.5) ? (2 * mxp_fg * mxp_bg) : (1 - (1 - mxp_fg) * (1 - mxp_bg)); -} -float2 mx_overlay(float2 mxp_fg, float2 mxp_bg) [[ anno::unused() ]] -{ - return float2( - mx_overlay(mxp_fg.x, mxp_bg.x), - mx_overlay(mxp_fg.y, mxp_bg.y) - ); -} -color mx_overlay(color mxp_fg, color mxp_bg) -{ - float3 fg(mxp_fg); - float3 bg(mxp_bg); - return color( - mx_overlay(fg.x, bg.x), - mx_overlay(fg.y, bg.y), - mx_overlay(fg.z, bg.z) - ); -} - -export float mx_overlay_float( - float mxp_fg = 0.0, - float mxp_bg = 0.0, - float mxp_mix = 1.0 -) - [[ - anno::description("Node Group: compositing") - ]] -{ - return mxp_mix * mx_overlay(mxp_fg, mxp_bg) + (1-mxp_mix) * mxp_bg; -} - -export color mx_overlay_color3( - color mxp_fg = color(0.0), - color mxp_bg = color(0.0), - float mxp_mix = 1.0 -) - [[ - anno::description("Node Group: compositing") - ]] -{ - return mxp_mix * mx_overlay(mxp_fg, mxp_bg) + (1-mxp_mix) * mxp_bg; -} - -export color4 mx_overlay_color4( - color4 mxp_fg = mk_color4(0.0, 0.0, 0.0, 0.0), - color4 mxp_bg = mk_color4(0.0, 0.0, 0.0, 0.0), - float mxp_mix = float(1.0) -) - [[ - anno::description("Node Group: compositing") - ]] -{ - color rgb = mxp_mix * mx_overlay(mxp_fg.rgb, mxp_bg.rgb) + (1-mxp_mix) * mxp_bg.rgb; - float a = mxp_mix * mx_overlay(mxp_fg.a , mxp_bg.a ) + (1-mxp_mix) * mxp_bg.a; - return color4(rgb,a); -} - export color4 mx_disjointover_color4( color4 mxp_fg = mk_color4(0.0, 0.0, 0.0, 0.0), color4 mxp_bg = mk_color4(0.0, 0.0, 0.0, 0.0), From a5b832dc8addc24cb2d661d6c1a04faacd684fe9 Mon Sep 17 00:00:00 2001 From: Stefan Habel <19556655+StefanHabel@users.noreply.github.com> Date: Tue, 24 Oct 2023 08:15:32 -0700 Subject: [PATCH 4/5] Fixed off-by-one index check in setChildIndex (#1582) Fixed an off-by-one index check in Element::setChildIndex(). --- source/MaterialXCore/Element.cpp | 2 +- source/MaterialXTest/MaterialXCore/Element.cpp | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/source/MaterialXCore/Element.cpp b/source/MaterialXCore/Element.cpp index 18d028efd3..5131dc8070 100644 --- a/source/MaterialXCore/Element.cpp +++ b/source/MaterialXCore/Element.cpp @@ -168,7 +168,7 @@ void Element::setChildIndex(const string& name, int index) return; } - if (index < 0 || index > (int) _childOrder.size()) + if (index < 0 || index >= (int) _childOrder.size()) { throw Exception("Invalid child index"); } diff --git a/source/MaterialXTest/MaterialXCore/Element.cpp b/source/MaterialXTest/MaterialXCore/Element.cpp index fc4939a74f..3865934ccb 100644 --- a/source/MaterialXTest/MaterialXCore/Element.cpp +++ b/source/MaterialXTest/MaterialXCore/Element.cpp @@ -54,6 +54,9 @@ TEST_CASE("Element", "[element]") REQUIRE(*doc2 != *doc); doc2->setChildIndex("elem1", doc2->getChildIndex("elem2")); REQUIRE(*doc2 == *doc); + REQUIRE_THROWS_AS(doc2->setChildIndex("elem1", -100), mx::Exception); + REQUIRE_THROWS_AS(doc2->setChildIndex("elem1", -1), mx::Exception); + REQUIRE_THROWS_AS(doc2->setChildIndex("elem1", 2), mx::Exception); REQUIRE_THROWS_AS(doc2->setChildIndex("elem1", 100), mx::Exception); REQUIRE(*doc2 == *doc); From 926ac2756c36f5541d44f6c3f8814bfebf228f3b Mon Sep 17 00:00:00 2001 From: Ashwin Bhat <1727158+ashwinbhat@users.noreply.github.com> Date: Tue, 24 Oct 2023 14:45:51 -0700 Subject: [PATCH 5/5] Update Catch2 to version 2.13.10 (#1566) - Update Catch2 version to v2.13.10 - Catch2 v2.13.0 supports BENCHMARK tests. - A new BENCHMARK test "GenShader: ShaderGen Performance" to benchmark document load, validation and shadergen times --- CMakeLists.txt | 5 ++ source/MaterialXTest/CMakeLists.txt | 4 + source/MaterialXTest/External/Catch/catch.hpp | 28 ++++--- .../MaterialXGenGlsl/GenGlsl.cpp | 11 +++ .../MaterialXGenShader/GenShaderUtil.cpp | 79 +++++++++++++++++++ .../MaterialXGenShader/GenShaderUtil.h | 3 + 6 files changed, 119 insertions(+), 11 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index de6aa7442f..0d6a64d6b1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -45,6 +45,7 @@ option(MATERIALX_BUILD_GEN_MSL "Build the MSL shader generator back-end." ON) option(MATERIALX_BUILD_RENDER "Build the MaterialX Render modules." ON) option(MATERIALX_BUILD_OIIO "Build OpenImageIO support for MaterialXRender." OFF) option(MATERIALX_BUILD_TESTS "Build unit tests." ON) +option(MATERIALX_BUILD_BENCHMARK_TESTS "Build benchmark tests." OFF) option(MATERIALX_BUILD_SHARED_LIBS "Build MaterialX libraries as shared rather than static." OFF) option(MATERIALX_PYTHON_LTO "Enable link-time optimizations for MaterialX Python." ON) @@ -131,6 +132,7 @@ mark_as_advanced(MATERIALX_BUILD_GEN_MSL) mark_as_advanced(MATERIALX_BUILD_RENDER) mark_as_advanced(MATERIALX_BUILD_OIIO) mark_as_advanced(MATERIALX_BUILD_TESTS) +mark_as_advanced(MATERIALX_BUILD_BENCHMARK_TESTS) mark_as_advanced(MATERIALX_BUILD_SHARED_LIBS) mark_as_advanced(MATERIALX_NAMESPACE_SUFFIX) mark_as_advanced(MATERIALX_LIBNAME_SUFFIX) @@ -177,6 +179,9 @@ endif() if(MATERIALX_TEST_RENDER) add_definitions(-DMATERIALX_TEST_RENDER) endif() +if (MATERIALX_BUILD_BENCHMARK_TESTS) + add_definitions(-DMATERIALX_BUILD_BENCHMARK_TESTS) +endif() if (MATERIALX_BUILD_GEN_MDL) add_definitions(-DMATERIALX_MDLC_EXECUTABLE=\"${MATERIALX_MDLC_EXECUTABLE}\") diff --git a/source/MaterialXTest/CMakeLists.txt b/source/MaterialXTest/CMakeLists.txt index e66db4bd2e..862a176b50 100644 --- a/source/MaterialXTest/CMakeLists.txt +++ b/source/MaterialXTest/CMakeLists.txt @@ -121,6 +121,10 @@ set_target_properties( VERSION "${MATERIALX_LIBRARY_VERSION}" SOVERSION "${MATERIALX_MAJOR_VERSION}") +if(MATERIALX_BUILD_BENCHMARK_TESTS) + target_compile_definitions(MaterialXTest PRIVATE -DCATCH_CONFIG_ENABLE_BENCHMARKING) +endif() + target_link_libraries( MaterialXTest ${CMAKE_DL_LIBS}) diff --git a/source/MaterialXTest/External/Catch/catch.hpp b/source/MaterialXTest/External/Catch/catch.hpp index d2a12427b2..9b309bddc6 100644 --- a/source/MaterialXTest/External/Catch/catch.hpp +++ b/source/MaterialXTest/External/Catch/catch.hpp @@ -1,6 +1,6 @@ /* - * Catch v2.13.9 - * Generated: 2022-04-12 22:37:23.260201 + * Catch v2.13.10 + * Generated: 2022-10-16 11:01:23.452308 * ---------------------------------------------------------- * This file has been merged from multiple headers. Please don't edit it directly * Copyright (c) 2022 Two Blue Cubes Ltd. All rights reserved. @@ -15,7 +15,7 @@ #define CATCH_VERSION_MAJOR 2 #define CATCH_VERSION_MINOR 13 -#define CATCH_VERSION_PATCH 9 +#define CATCH_VERSION_PATCH 10 #ifdef __clang__ # pragma clang system_header @@ -7395,8 +7395,6 @@ namespace Catch { template struct ObjectStorage { - using TStorage = typename std::aligned_storage::value>::type; - ObjectStorage() : data() {} ObjectStorage(const ObjectStorage& other) @@ -7439,7 +7437,7 @@ namespace Catch { return *static_cast(static_cast(&data)); } - TStorage data; + struct { alignas(T) unsigned char data[sizeof(T)]; } data; }; } @@ -7949,7 +7947,7 @@ namespace Catch { #if defined(__i386__) || defined(__x86_64__) #define CATCH_TRAP() __asm__("int $3\n" : : ) /* NOLINT */ #elif defined(__aarch64__) - #define CATCH_TRAP() __asm__(".inst 0xd4200000") + #define CATCH_TRAP() __asm__(".inst 0xd43e0000") #endif #elif defined(CATCH_PLATFORM_IPHONE) @@ -13558,7 +13556,7 @@ namespace Catch { // Handle list request if( Option listed = list( m_config ) ) - return static_cast( *listed ); + return (std::min) (MaxExitCode, static_cast(*listed)); TestGroup tests { m_config }; auto const totals = tests.execute(); @@ -15391,7 +15389,7 @@ namespace Catch { } Version const& libraryVersion() { - static Version version( 2, 13, 9, "", 0 ); + static Version version( 2, 13, 10, "", 0 ); return version; } @@ -17526,12 +17524,20 @@ namespace Catch { #ifndef __OBJC__ +#ifndef CATCH_INTERNAL_CDECL +#ifdef _MSC_VER +#define CATCH_INTERNAL_CDECL __cdecl +#else +#define CATCH_INTERNAL_CDECL +#endif +#endif + #if defined(CATCH_CONFIG_WCHAR) && defined(CATCH_PLATFORM_WINDOWS) && defined(_UNICODE) && !defined(DO_NOT_USE_WMAIN) // Standard C/C++ Win32 Unicode wmain entry point -extern "C" int wmain (int argc, wchar_t * argv[], wchar_t * []) { +extern "C" int CATCH_INTERNAL_CDECL wmain (int argc, wchar_t * argv[], wchar_t * []) { #else // Standard C/C++ main entry point -int main (int argc, char * argv[]) { +int CATCH_INTERNAL_CDECL main (int argc, char * argv[]) { #endif return Catch::Session().run( argc, argv ); diff --git a/source/MaterialXTest/MaterialXGenGlsl/GenGlsl.cpp b/source/MaterialXTest/MaterialXGenGlsl/GenGlsl.cpp index 339d3853cb..2cc4d7ccf4 100644 --- a/source/MaterialXTest/MaterialXGenGlsl/GenGlsl.cpp +++ b/source/MaterialXTest/MaterialXGenGlsl/GenGlsl.cpp @@ -119,6 +119,17 @@ TEST_CASE("GenShader: Bind Light Shaders", "[genglsl]") REQUIRE_NOTHROW(mx::HwShaderGenerator::bindLightShader(*spotLightShader, 66, context)); } +#ifdef MATERIALX_BUILD_BENCHMARK_TESTS +TEST_CASE("GenShader: Performance Test", "[genglsl]") +{ + mx::GenContext context(mx::GlslShaderGenerator::create()); + BENCHMARK("Load documents, validate and generate shader") + { + return GenShaderUtil::shaderGenPerformanceTest(context); + }; +} +#endif + enum class GlslType { Glsl400, diff --git a/source/MaterialXTest/MaterialXGenShader/GenShaderUtil.cpp b/source/MaterialXTest/MaterialXGenShader/GenShaderUtil.cpp index 261aacdefe..2ca8afa86c 100644 --- a/source/MaterialXTest/MaterialXGenShader/GenShaderUtil.cpp +++ b/source/MaterialXTest/MaterialXGenShader/GenShaderUtil.cpp @@ -314,6 +314,85 @@ void testUniqueNames(mx::GenContext& context, const std::string& stage) REQUIRE(sgNode1->getOutput()->getVariable() == "unique_names_out"); } +// Test ShaderGen performance +void shaderGenPerformanceTest(mx::GenContext& context) +{ + mx::DocumentPtr nodeLibrary = mx::createDocument(); + mx::FilePath currentPath = mx::FilePath::getCurrentPath(); + const mx::FileSearchPath libSearchPath(currentPath); + + // Load the standard libraries. + loadLibraries({ "libraries" }, libSearchPath, nodeLibrary); + context.registerSourceCodeSearchPath(libSearchPath); + + // Enable Color Management + mx::ColorManagementSystemPtr colorManagementSystem = + mx::DefaultColorManagementSystem::create(context.getShaderGenerator().getTarget()); + + REQUIRE(colorManagementSystem); + if (colorManagementSystem) + { + context.getShaderGenerator().setColorManagementSystem(colorManagementSystem); + colorManagementSystem->loadLibrary(nodeLibrary); + } + + // Enable Unit System + mx::UnitSystemPtr unitSystem = mx::UnitSystem::create(context.getShaderGenerator().getTarget()); + REQUIRE(unitSystem); + if (unitSystem) + { + context.getShaderGenerator().setUnitSystem(unitSystem); + unitSystem->loadLibrary(nodeLibrary); + // Setup Unit converters + unitSystem->setUnitConverterRegistry(mx::UnitConverterRegistry::create()); + mx::UnitTypeDefPtr distanceTypeDef = nodeLibrary->getUnitTypeDef("distance"); + unitSystem->getUnitConverterRegistry()->addUnitConverter(distanceTypeDef, mx::LinearUnitConverter::create(distanceTypeDef)); + mx::UnitTypeDefPtr angleTypeDef = nodeLibrary->getUnitTypeDef("angle"); + unitSystem->getUnitConverterRegistry()->addUnitConverter(angleTypeDef, mx::LinearUnitConverter::create(angleTypeDef)); + context.getOptions().targetDistanceUnit = "meter"; + } + + // Read mtlx documents + mx::FilePathVec testRootPaths; + testRootPaths.push_back("resources/Materials/Examples/StandardSurface"); + + std::vector loadedDocuments; + mx::StringVec documentsPaths; + mx::StringVec errorLog; + + for (const auto& testRoot : testRootPaths) + { + mx::loadDocuments(testRoot, libSearchPath, {}, {}, loadedDocuments, documentsPaths, + nullptr, &errorLog); + } + + REQUIRE(loadedDocuments.size() > 0); + REQUIRE(loadedDocuments.size() == documentsPaths.size()); + + // Shuffle the order of documents and perform document library import validatation and shadergen + std::random_device random_dev; + std::mt19937 generator(random_dev()); + std::shuffle(loadedDocuments.begin(), loadedDocuments.end(), generator); + for (const auto& doc : loadedDocuments) + { + doc->importLibrary(nodeLibrary); + std::vector elements = mx::findRenderableElements(doc); + + REQUIRE(elements.size() > 0); + + std::string message; + bool docValid = doc->validate(&message); + + REQUIRE(docValid == true); + + mx::StringVec sourceCode; + mx::ShaderPtr shader = nullptr; + shader = context.getShaderGenerator().generate(elements[0]->getName(), elements[0], context); + + REQUIRE(shader != nullptr); + REQUIRE(shader->getSourceCode(mx::Stage::PIXEL).length() > 0); + } +} void ShaderGeneratorTester::checkImplementationUsage(const mx::StringSet& usedImpls, const mx::GenContext& context, diff --git a/source/MaterialXTest/MaterialXGenShader/GenShaderUtil.h b/source/MaterialXTest/MaterialXGenShader/GenShaderUtil.h index ea34170d71..9c471466aa 100644 --- a/source/MaterialXTest/MaterialXGenShader/GenShaderUtil.h +++ b/source/MaterialXTest/MaterialXGenShader/GenShaderUtil.h @@ -52,6 +52,9 @@ void checkImplementations(mx::GenContext& context, // Utility test to check unique name generation on a shader generator void testUniqueNames(mx::GenContext& context, const std::string& stage); +// Utility to perfrom simple performance test to load, validate and generate shaders +void shaderGenPerformanceTest(mx::GenContext& context); + // // Render validation options. Reflects the _options.mtlx // file in the test suite area.