From d5634c83335766952026f6120dcea7b902894c44 Mon Sep 17 00:00:00 2001 From: Jonathan Stone Date: Tue, 17 Oct 2023 10:18:51 -0700 Subject: [PATCH 1/8] 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/8] 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/8] 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/8] 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/8] 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. From 578118d645657f800b4fd96f24c140325f9cb6bb Mon Sep 17 00:00:00 2001 From: Pablo Delgado Date: Sat, 28 Oct 2023 02:29:31 +0200 Subject: [PATCH 6/8] Fix MDL implementation of mx_hsvtorgb (#1584) --- source/MaterialXGenMdl/mdl/materialx/hsv.mdl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/source/MaterialXGenMdl/mdl/materialx/hsv.mdl b/source/MaterialXGenMdl/mdl/materialx/hsv.mdl index 888d4ada86..9adb1505cb 100644 --- a/source/MaterialXGenMdl/mdl/materialx/hsv.mdl +++ b/source/MaterialXGenMdl/mdl/materialx/hsv.mdl @@ -25,16 +25,16 @@ import ::limits::*; export float3 mx_hsvtorgb(float3 hsv) { // from "Color Imaging, Fundamentals and Applications", Reinhard et al., p. 442 - // A hue of 1.0 is questionably valid, and needs to be interpreted as 0.0f - float h_prime = (hsv.x < 1.0f) ? hsv.x * 6.0f : 0.0f; // H * 360.0/60.0 - float h_floor = math::floor(h_prime); - float f = h_prime - h_floor; + float h = 6.0 * (hsv.x - math::floor(hsv.x)); + int hi = int(h); // truncate + float f = h - float(hi); + float zy = hsv.z*hsv.y; float a = hsv.z - zy; float b = hsv.z - zy*f; float c = a + zy*f; - switch(int(h_floor)) { + switch(hi) { default: // hue out of [0,1] range... // fall through... From 6a5a1adf7f5fdc408f141eb1dcc1e916ec783f5f Mon Sep 17 00:00:00 2001 From: Jonathan Stone Date: Fri, 27 Oct 2023 17:42:44 -0700 Subject: [PATCH 7/8] Add initial Python 3.12 test This changelist adds an initial Python 3.12 test to GitHub CI, leveraging the support for this Python version in the Windows VS2022 environment. --- .github/workflows/main.yml | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 4fdcefb7fb..c842d3b530 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -37,7 +37,6 @@ jobs: compiler: gcc compiler_version: "12" python: 3.11 - upload_shaders: ON - name: Linux_GCC_CoverageAnalysis os: ubuntu-22.04 @@ -104,18 +103,19 @@ jobs: python: 3.7 cmake_config: -G "Visual Studio 16 2019" -A "Win32" -DMATERIALX_BUILD_SHARED_LIBS=ON - - name: Windows_VS2022_x64_Python39 + - name: Windows_VS2022_x64_Python311 os: windows-2022 architecture: x64 - python: 3.9 + python: 3.11 cmake_config: -G "Visual Studio 17 2022" -A "x64" + test_shaders: ON - - name: Windows_VS2022_x64_Python311 + - name: Windows_VS2022_x64_Python312 os: windows-2022 architecture: x64 - python: 3.11 + python: 3.12 cmake_config: -G "Visual Studio 17 2022" -A "x64" - test_shaders: ON + upload_shaders: ON steps: - name: Sync Repository @@ -172,6 +172,10 @@ jobs: python-version: ${{ matrix.python }} architecture: ${{ matrix.architecture }} + - name: Install Python Dependencies + if: matrix.python != 'None' + run: pip install setuptools + - name: Install Emscripten if: matrix.build_javascript == 'ON' run: | From 50062e5da4d6f279bcc134a61651096ea6a2c26c Mon Sep 17 00:00:00 2001 From: krohmerNV <42233792+krohmerNV@users.noreply.github.com> Date: Wed, 1 Nov 2023 00:17:38 +0100 Subject: [PATCH 8/8] Add fileTextureVerticalFlip option for MDL generation (#1418) When investigating the differences in the MaterialXTest I implemented the fileTextureVerticalFlip in the MDL shader gen. In theory this option is not needed because the MaterialX and MDL define the image coordinate both in the lower left. But it might be useful to align with existing but non standard conform renderers. It could for instance be used in the MaterialXTest to align with the flipped GLSL and OSL results. --- .../stdlib/genmdl/stdlib_genmdl_impl.mtlx | 12 ++--- source/MaterialXGenMdl/MdlShaderGenerator.cpp | 9 ++++ source/MaterialXGenMdl/Nodes/ImageNodeMdl.cpp | 50 +++++++++++++++++ source/MaterialXGenMdl/Nodes/ImageNodeMdl.h | 34 ++++++++++++ .../MaterialXGenMdl/mdl/materialx/stdlib.mdl | 54 ++++++++++++++++--- .../MaterialXTest/MaterialXGenMdl/GenMdl.cpp | 18 ++++++- 6 files changed, 164 insertions(+), 13 deletions(-) create mode 100644 source/MaterialXGenMdl/Nodes/ImageNodeMdl.cpp create mode 100644 source/MaterialXGenMdl/Nodes/ImageNodeMdl.h diff --git a/libraries/stdlib/genmdl/stdlib_genmdl_impl.mtlx b/libraries/stdlib/genmdl/stdlib_genmdl_impl.mtlx index 0081dec924..022f0896f1 100644 --- a/libraries/stdlib/genmdl/stdlib_genmdl_impl.mtlx +++ b/libraries/stdlib/genmdl/stdlib_genmdl_impl.mtlx @@ -22,22 +22,22 @@ - + - + - + - + - + - + diff --git a/source/MaterialXGenMdl/MdlShaderGenerator.cpp b/source/MaterialXGenMdl/MdlShaderGenerator.cpp index f64b555c20..e10d586d69 100644 --- a/source/MaterialXGenMdl/MdlShaderGenerator.cpp +++ b/source/MaterialXGenMdl/MdlShaderGenerator.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -187,6 +188,14 @@ MdlShaderGenerator::MdlShaderGenerator() : // registerImplementation("IM_sheen_bsdf_" + MdlShaderGenerator::TARGET, LayerableNodeMdl::create); + + // + registerImplementation("IM_image_float_" + MdlShaderGenerator::TARGET, ImageNodeMdl::create); + registerImplementation("IM_image_color3_" + MdlShaderGenerator::TARGET, ImageNodeMdl::create); + registerImplementation("IM_image_color4_" + MdlShaderGenerator::TARGET, ImageNodeMdl::create); + registerImplementation("IM_image_vector2_" + MdlShaderGenerator::TARGET, ImageNodeMdl::create); + registerImplementation("IM_image_vector3_" + MdlShaderGenerator::TARGET, ImageNodeMdl::create); + registerImplementation("IM_image_vector4_" + MdlShaderGenerator::TARGET, ImageNodeMdl::create); } ShaderPtr MdlShaderGenerator::generate(const string& name, ElementPtr element, GenContext& context) const diff --git a/source/MaterialXGenMdl/Nodes/ImageNodeMdl.cpp b/source/MaterialXGenMdl/Nodes/ImageNodeMdl.cpp new file mode 100644 index 0000000000..502711fe01 --- /dev/null +++ b/source/MaterialXGenMdl/Nodes/ImageNodeMdl.cpp @@ -0,0 +1,50 @@ +// +// Copyright Contributors to the MaterialX Project +// SPDX-License-Identifier: Apache-2.0 +// + +#include +#include +#include +#include + +MATERIALX_NAMESPACE_BEGIN + +const string ImageNodeMdl::FLIP_V = "flip_v"; + +ShaderNodeImplPtr ImageNodeMdl::create() +{ + return std::make_shared(); +} + +void ImageNodeMdl::addInputs(ShaderNode& node, GenContext& context) const +{ + BASE::addInputs(node, context); + node.addInput(ImageNodeMdl::FLIP_V, Type::BOOLEAN)->setUniform(); +} + +bool ImageNodeMdl::isEditable(const ShaderInput& input) const +{ + if (input.getName() == ImageNodeMdl::FLIP_V) + { + return false; + } + return BASE::isEditable(input); +} + +void ImageNodeMdl::emitFunctionCall(const ShaderNode& _node, GenContext& context, ShaderStage& stage) const +{ + DEFINE_SHADER_STAGE(stage, Stage::PIXEL) + { + ShaderNode& node = const_cast(_node); + ShaderInput* flipUInput = node.getInput(ImageNodeMdl::FLIP_V); + ValuePtr value = TypedValue::createValue(context.getOptions().fileTextureVerticalFlip); + if (flipUInput) + { + flipUInput->setValue(value); + } + BASE::emitFunctionCall(_node, context, stage); + } +} + +MATERIALX_NAMESPACE_END diff --git a/source/MaterialXGenMdl/Nodes/ImageNodeMdl.h b/source/MaterialXGenMdl/Nodes/ImageNodeMdl.h new file mode 100644 index 0000000000..fe88b2ce09 --- /dev/null +++ b/source/MaterialXGenMdl/Nodes/ImageNodeMdl.h @@ -0,0 +1,34 @@ +// +// Copyright Contributors to the MaterialX Project +// SPDX-License-Identifier: Apache-2.0 +// + +#ifndef MATERIALX_IMAGENODEMDL_H +#define MATERIALX_IMAGENODEMDL_H + +#include + +#include "SourceCodeNodeMdl.h" + +MATERIALX_NAMESPACE_BEGIN + +/// Image node implementation for MDL +class MX_GENMDL_API ImageNodeMdl : public SourceCodeNodeMdl +{ + using BASE = SourceCodeNodeMdl; + + public: + static const string FLIP_V; ///< the empty string "" + + static ShaderNodeImplPtr create(); + + void addInputs(ShaderNode& node, GenContext& context) const override; + + bool isEditable(const ShaderInput& input) const override; + + void emitFunctionCall(const ShaderNode& node, GenContext& context, ShaderStage& stage) const override; +}; + +MATERIALX_NAMESPACE_END + +#endif diff --git a/source/MaterialXGenMdl/mdl/materialx/stdlib.mdl b/source/MaterialXGenMdl/mdl/materialx/stdlib.mdl index 8089feb7f0..4ba4f0875a 100644 --- a/source/MaterialXGenMdl/mdl/materialx/stdlib.mdl +++ b/source/MaterialXGenMdl/mdl/materialx/stdlib.mdl @@ -139,6 +139,11 @@ export float mx_image_float( anno::description("Enumeration {constant,clamp,periodic,mirror}."), anno::display_name("Frame End Action"), anno::unused() + ]], + uniform bool mxp_flip_v = false + [[ + anno::usage("for applying the 'fileTextureVerticalFlip' shader generator option."), + anno::hidden() ]] ) [[ @@ -153,7 +158,9 @@ export float mx_image_float( return mxp_default; float returnValue = ::tex::lookup_float(tex: mxp_file, - coord: mxp_texcoord, + coord: mxp_flip_v + ? float2(mxp_texcoord.x, 1.0f - mxp_texcoord.y) + : mxp_texcoord, wrap_u: map_addressmode(mxp_uaddressmode), wrap_v: map_addressmode(mxp_vaddressmode)); return returnValue; @@ -207,6 +214,11 @@ export color mx_image_color3( anno::description("Enumeration {constant,clamp,periodic,mirror}."), anno::display_name("Frame End Action"), anno::unused() + ]], + uniform bool mxp_flip_v = false + [[ + anno::usage("for applying the 'fileTextureVerticalFlip' shader generator option."), + anno::hidden() ]] ) [[ @@ -221,7 +233,9 @@ export color mx_image_color3( return mxp_default; color returnValue = ::tex::lookup_color(tex: mxp_file, - coord: mxp_texcoord, + coord: mxp_flip_v + ? float2(mxp_texcoord.x, 1.0f - mxp_texcoord.y) + : mxp_texcoord, wrap_u: map_addressmode(mxp_uaddressmode), wrap_v: map_addressmode(mxp_vaddressmode)); return returnValue; @@ -275,6 +289,11 @@ export color4 mx_image_color4( anno::description("Enumeration {constant,clamp,periodic,mirror}."), anno::display_name("Frame End Action"), anno::unused() + ]], + uniform bool mxp_flip_v = false + [[ + anno::usage("for applying the 'fileTextureVerticalFlip' shader generator option."), + anno::hidden() ]] ) [[ @@ -289,7 +308,9 @@ export color4 mx_image_color4( return mxp_default; color4 returnValue = mk_color4( ::tex::lookup_float4(tex: mxp_file, - coord: mxp_texcoord, + coord: mxp_flip_v + ? float2(mxp_texcoord.x, 1.0f - mxp_texcoord.y) + : mxp_texcoord, wrap_u: map_addressmode(mxp_uaddressmode), wrap_v: map_addressmode(mxp_vaddressmode))); return returnValue; @@ -343,6 +364,11 @@ export float2 mx_image_vector2( anno::description("Enumeration {constant,clamp,periodic,mirror}."), anno::display_name("Frame End Action"), anno::unused() + ]], + uniform bool mxp_flip_v = false + [[ + anno::usage("for applying the 'fileTextureVerticalFlip' shader generator option."), + anno::hidden() ]] ) [[ @@ -357,7 +383,9 @@ export float2 mx_image_vector2( return mxp_default; float2 returnValue = ::tex::lookup_float2(tex: mxp_file, - coord: mxp_texcoord, + coord: mxp_flip_v + ? float2(mxp_texcoord.x, 1.0f - mxp_texcoord.y) + : mxp_texcoord, wrap_u: map_addressmode(mxp_uaddressmode), wrap_v: map_addressmode(mxp_vaddressmode)); return returnValue; @@ -411,6 +439,11 @@ export float3 mx_image_vector3( anno::description("Enumeration {constant,clamp,periodic,mirror}."), anno::display_name("Frame End Action"), anno::unused() + ]], + uniform bool mxp_flip_v = false + [[ + anno::usage("for applying the 'fileTextureVerticalFlip' shader generator option."), + anno::hidden() ]] ) [[ @@ -425,7 +458,9 @@ export float3 mx_image_vector3( return mxp_default; float3 returnValue = ::tex::lookup_float3(tex: mxp_file, - coord: mxp_texcoord, + coord: mxp_flip_v + ? float2(mxp_texcoord.x, 1.0f - mxp_texcoord.y) + : mxp_texcoord, wrap_u: map_addressmode(mxp_uaddressmode), wrap_v: map_addressmode(mxp_vaddressmode)); return returnValue; @@ -479,6 +514,11 @@ export float4 mx_image_vector4( anno::description("Enumeration {constant,clamp,periodic,mirror}."), anno::display_name("Frame End Action"), anno::unused() + ]], + uniform bool mxp_flip_v = false + [[ + anno::usage("for applying the 'fileTextureVerticalFlip' shader generator option."), + anno::hidden() ]] ) [[ @@ -493,7 +533,9 @@ export float4 mx_image_vector4( return mxp_default; float4 returnValue = ::tex::lookup_float4(tex: mxp_file, - coord: mxp_texcoord, + coord: mxp_flip_v + ? float2(mxp_texcoord.x, 1.0f - mxp_texcoord.y) + : mxp_texcoord, wrap_u: map_addressmode(mxp_uaddressmode), wrap_v: map_addressmode(mxp_vaddressmode)); return returnValue; diff --git a/source/MaterialXTest/MaterialXGenMdl/GenMdl.cpp b/source/MaterialXTest/MaterialXGenMdl/GenMdl.cpp index 1a3e127cde..22ad0d456e 100644 --- a/source/MaterialXTest/MaterialXGenMdl/GenMdl.cpp +++ b/source/MaterialXTest/MaterialXGenMdl/GenMdl.cpp @@ -288,7 +288,10 @@ void MdlShaderGeneratorTester::compileSource(const std::vector& so std::string iblFile = (rootPath / "resources/lights/san_giuseppe_bridge.hdr").asString(); renderCommand += " --hdr \"" + iblFile + "\" --hdr_rotate 90"; // set scene - renderCommand += " --uv_scale 0.5 1.0 --uv_offset 0.0 0.0 --uv_repeat --uv_flip"; + renderCommand += " --uv_scale 0.5 1.0 --uv_offset 0.0 0.0 --uv_repeat"; + renderCommand += " --uv_flip"; // this will flip the v coordinate of the vertices, which flips all the + // UV operations. In contrast, the fileTextureVerticalFlip option will + // only flip the image access nodes. renderCommand += " --camera 0 0 3 0 0 0 --fov 45"; // set the material @@ -365,6 +368,19 @@ TEST_CASE("GenShader: MDL Shader Generation", "[genmdl]") mx::GenOptions genOptions; genOptions.targetColorSpaceOverride = "lin_rec709"; + + // Flipping the texture lookups for the test renderer only. + // This is because OSL testrender does not allow to change the UV layout of their sphere (yet) and the MaterialX test suite + // adopts the OSL behavior in order to produce comparable results. This means that raw texture coordinates, or procedurals + // that use the texture coordinates, do not match what might be expected when reading the MaterialX spec: + // "[...] the image is mapped onto the geometry based on geometry UV coordinates, with the lower-left corner of an image + // mapping to the (0,0) UV coordinate [...]" + // This means for MDL: here, and only here in the test suite, we flip the UV coordinates of mesh using the `--uv_flip` option + // of the renderer, and to correct the image orientation, we apply `fileTextureVerticalFlip`. + // In regular MDL integrations this is not needed because MDL and MaterialX define the texture space equally with the origin + // at the bottom left. + genOptions.fileTextureVerticalFlip = true; + mx::FilePath optionsFilePath = searchPath.find("resources/Materials/TestSuite/_options.mtlx"); tester.validate(genOptions, optionsFilePath); }