1.38 Explicit nodegraph inputs and nodegraph input connections #1053

bernardkwok · 2020-12-08T21:14:33Z

Update #849

Initial support:

"interfacename" name validation for nodegraphs
Traversal from nodegraph inputs with interfacenames to associated nodegraph inputs.
Traversal from nodegraph inputs to upstream node or nodegraphs.
Traversal from nodegraph inputs to upstream output on nodegraph associated with a definition.
Get Runtime I/O working to write and read explicit input connections on nodegraphs.

Test 1: 3 nodegraphs connected

upstream_graph:
graph_graph:
surf_graph_graph:

Test: node with nodegraph implementation connected downstream nodegraphs

NG_upstream_graph (nodegraph implementation):
nd_graph_graph:
nd_surf_graph_graph:

node with non-nodegraph implementation connected to downstream nodegraph

graph_to_node:
surf_graph_node:

3 non-pattern graphs

upstream3:
upstream2:
upstream1:

Fix runtime mtlx export to output "nodegraph" or "nodename" as required based on connection type.

- multiple output connectins - downstream node connections.

Add new test to start testing cascaded nodegraphs.

bernardkwok · 2020-12-09T20:34:41Z

resources/Materials/TestSuite/stdlib/upgrade/1_38_parameter_to_input.mtlx

@@ -2,10 +2,10 @@
 <materialx version="1.37">
  <nodegraph name="parameter_as_input" fileprefix="resources/Images/">
    <constant name="constant" type="color3">
-      <input name="value" type="color3" interfacename="value1" value="1, 0, 0" />
+      <input name="value" type="color3" value="1, 0, 0" />


This invalid case of an interfacename w/o a proper input is now caught.

bernardkwok · 2020-12-09T20:37:17Z

source/MaterialXCore/Interface.cpp

        ConstGraphElementPtr graph = getAncestorOfType<GraphElement>();
-        NodePtr node = graph ? graph->getNode(getNodeName()) : nullptr;
+        NodePtr node = graph ? graph->getNode(nodeName) : nullptr;


For a given nodegraph input check at the parent nodegraph level for a connection.

I think there is a risk of ambiguity here, if a node outside the graph has the same name as a node inside the graph. It's a possible setup since they have two different naming scopes.

So if a nodegraph input connects to a node "foo1" outside the graph (nodename="foo1"), but there is also a node inside the graph named "foo1", the node inside the graph will be used, since both scopes are checked and inside nodes takes precedence. This would naturally be invalid since a node inside the graph is not connectable to a nodegraph input.

I think we have to handle graph inputs and node inputs separately in this traversal, since graph inputs can only connect to elements outside the graph scope, and node inputs can only connected to elements in the same scope as the node.

I was trying to make this as reusable as possible. I'll need to take a closer look but getAncestorOfType<>() on an input in a graph is what being handled here which requires traversing twice as the first returned value is the graph itself and not it's parent.

Basically, the rule for node vs nodegraph does not differ -- look for the referenced item on the parent.

I'm mainly thinking of name collisions with nodes inside vs outside the graph scope. As in this example:

<constant name="foo1" type="color3"> <parameter name="value" type="color3" value="1, 0, 0"/> </constant> <nodegraph name="graph1" > <input name="in1" type="color3" nodename="foo1" /> <input name="in2" type="color3" value="0, 1, 0" /> <constant name="foo1" type="float"> <parameter name="value" type="float" value="0.5"/> </constant> <mix name="mix1" type="color3"> <input name="bg" type="color3" interfacename="in1" /> <input name="fg" type="color3" interfacename="in1" /> <input name="mix" type="float" nodename="foo1" /> </mix> <output name="out" type="color3" nodename="mix1" /> </nodegraph>

If the internal scope is always searched first, in this case the internal node "foo1" would be returned as being connected to the input "in1", when in fact it is the external node "foo1" that is connected.

A test case like this could be added to make sure this is handled correctly.

@bernardkwok @niklasharrysson As discussed in a separate thread, I think it would be valuable to merge the latest work from the v1.38 branch of MaterialX before proceeding too far with these new improvements, as there's a good deal of code that's been removed from Interface.cpp.

I've done the merge so this logic still needs to be resolved :).
@niklasharrysson, looking at the same level will give the wrong result for "in1". So I guess the branching logic would need to be:
if (input is a child of a nodegraph)
{
look for referenced node/graph from parent of nodegraph.
}
else
{
look for referenced node/graph from parent of node.
}

This basically means to always look up 2 levels from the input (if they exist).

We don't have the concept of a input which is not a child of a node or nodegraph (i.e. at the Document level) so this does not need to be considered.

source/MaterialXCore/Interface.cpp

bernardkwok · 2020-12-09T20:38:49Z

source/MaterialXCore/Interface.cpp

 NodePtr Input::getConnectedNode() const
 {
+    // Traverse through interface names to nodegraph input
+    InputPtr graphInput = getInterface();
+    if (graphInput && (graphInput->hasNodeName() || graphInput->hasNodeGraphString()))


If this input has an interfacename, then traverse through to that input to find any upstream connection.

bernardkwok · 2020-12-09T20:40:09Z

source/MaterialXCore/Interface.cpp

        ConstGraphElementPtr graph = getAncestorOfType<GraphElement>();
-        NodePtr node = graph ? graph->getNode(getNodeName()) : nullptr;
+        NodePtr node = graph ? graph->getNode(nodeName) : nullptr;


For a given nodegraph input check at the parent nodegraph level for a connection.

bernardkwok · 2020-12-09T20:44:42Z

source/MaterialXCore/Node.cpp

@@ -634,6 +647,33 @@ bool NodeGraph::validate(string* message) const
            validateRequire(getOutputCount() == nodeDef->getActiveOutputs().size(), res, message, "NodeGraph implementation has a different number of outputs than its NodeDef");
        }
    }
+    // Check interfaces on nodegraphs which are not definitions
+    if (!hasNodeDefString())


Check interfacename input existence and if that input has connections that they exist as well.

bernardkwok · 2020-12-09T20:46:39Z

source/MaterialXRuntime/RtFileIo.cpp

+        }
+    }
+
+    void createNodeGraphConnections(const vector<NodeGraphPtr>& nodeElements, PvtPrim* parent, PvtStage* stage, const PvtRenamingMapper& mapper)


No real change in logic above. Just make it so that we can check for connections between nodegraph and upstream nodes / nodegraphs now. Note added 2 interfaces since can't really cast from a constant array of node vs nodegraph ptrs so just allowed 2 interfaces and the per item check will cast to an interfaceElement check.

bernardkwok · 2020-12-09T20:47:11Z

source/MaterialXRuntime/RtFileIo.cpp

-                        RtNode sourceNode = source.getParent();
-                        input->setNodeName(sourceNode.getName());
-                        if (sourceNode.numOutputs() > 1)
+                        RtPrim sourcePrim = source.getParent();


Fix save so that either "nodename" or "nodegraph" is written depending on the upstream connection.

bernardkwok · 2020-12-09T20:47:33Z

source/MaterialXTest/MaterialXCore/Traversal.cpp

+
+    mx::FilePath testFile = mx::FilePath::getCurrentPath() / mx::FilePath("resources/Materials/TestSuite/stdlib/nodegraph_inputs/nodegraph_nodegraph.mtlx");
+    mx::readFromXmlFile(doc, testFile, searchPath);
+    REQUIRE(doc->validate());


Validation does most of the checking.

bernardkwok · 2020-12-09T20:47:54Z

source/MaterialXTest/MaterialXCore/Traversal.cpp

+        {
+            if (interfaceInput && (!interfaceInput->getNodeName().empty() || !interfaceInput->getNodeGraphString().empty()))
+            {
+                REQUIRE(interfaceInput->getConnectedNode() != nullptr);                    


Additional upstream node check.

bernardkwok · 2020-12-09T20:56:54Z

source/MaterialXCore/Node.cpp

@@ -118,7 +118,20 @@ OutputPtr Node::getNodeDefOutput(ElementPtr connectingElement)
        OutputPtr output = OutputPtr();
        if (connectedInput)
        {
-            output = connectedInput->getConnectedOutput();
+            InputPtr interfaceInput = nullptr;
+            if (connectedInput->hasInterfaceName())


@jstone-lucasfilm, Niklas mentioned this was added by you at some point for multioutput support.
I found an issue with the config of a multioutput upstream node connected to a downstream multiinput nodegraph as it would not find a nodedef output just by using the input itself. This is because the input specified by the "interfacename" actually has the "output" specifier.

The test case is found in nodegraph_nodegraph.mtlx:

<nodedef name="ND_upstream_graph" node="upstream_graph_def" version="1.0" isdefaultversion="true" nodegroup="procedural2d"> <input name="nd_file" type="filename" uniform="true" value="resources/Images/grid.png" /> <input name="nd_file2" type="filename" uniform="true" value="resources/Images/cloth.png" /> <output name="nd_graph_out_image" type="color3" value="0, 0, 0" /> <output name="nd_graph_out_image2" type="color3" value="0, 0, 0" /> </nodedef> <nodegraph name="NG_upstream_graph" nodedef="ND_upstream_graph" > <input name="nd_file" type="filename" uniform="true" value="resources/Images/grid.png" /> <input name="nd_file2" type="filename" uniform="true" value="resources/Images/cloth.png" /> <image name="image" type="color3" xpos="-79" ypos="-448.596"> <input name="file" type="filename" uniform="true" interfacename="nd_file" value="resources/Images/grid.png" /> </image> <image name="image2" type="color3"> <input name="file" type="filename" uniform="true" interfacename="nd_file2" value="resources/Images/cloth.png" /> </image> <output name="nd_graph_out_image" type="color3" nodename="image" /> <output name="nd_graph_out_image2" type="color3" nodename="image2" /> </nodegraph> <upstream_graph_def name="upstream_graph_instance" type="multioutput"> <output name="nd_graph_out_image" type="color3" /> <output name="nd_graph_out_image2" type="color3" /> </upstream_graph_def> <nodegraph name="nd_graph_graph" > <input name="nd_input" type="color3" nodename="upstream_graph_instance" output="nd_graph_out_image" value="1, 0, 0" /> <input name="nd_input2" type="color3" nodename="upstream_graph_instance" output="nd_graph_out_image2" value="1, 0, 0" /> <multiply name="multiply" type="color3"> <input name="in1" type="color3" interfacename="nd_input" value="1, 0, 0" /> <input name="in2" type="color3" value="0.4, 0.4, 0.4" /> </multiply> <multiply name="multiply2" type="color3"> <input name="in1" type="color3" interfacename="nd_input2" value="1, 0, 0" /> <input name="in2" type="color3" value="0.4, 0.4, 0.4" /> </multiply> <output name="nd_graph_graph_out" type="color3" nodename="multiply" /> <output name="nd_graph_graph_out2" type="color3" nodename="multiply2" /> </nodegraph> <nodegraph name="ng_surf_graph_graph" > <input name="nd_input" type="color3" nodename="upstream_graph_instance" output="nd_graph_out_image" value="1, 1, 1" /> <input name="nd_input2" type="color3" nodename="upstream_graph_instance" output="nd_graph_out_image2" value="1, 1, 1" /> <standard_surface name="default_shader" type="surfaceshader"> <input name="base_color" type="color3" interfacename="nd_input" value="1, 1, 1" /> </standard_surface> <standard_surface name="default_shader2" type="surfaceshader"> <input name="base_color" type="color3" interfacename="nd_input2" value="1, 1, 1" /> </standard_surface> <output name="nd_surf_graph_graph_out" type="surfaceshader" nodename="default_shader" /> <output name="nd_surf_graph_graph_out2" type="surfaceshader" nodename="default_shader2" /> </nodegraph>

source/MaterialXTest/MaterialXCore/Traversal.cpp

This changelist removes the legacy Parameter class from the v1.38 branch of MaterialX.

- Add initial C++ unit tests for looks and material assignments. - Extend material and look unit tests to Python. - Rename Look::getReferencedMaterialNode to Look::getReferencedMaterial for simplicity.

- Clarify the lifetime and ownership of WindowWrapper objects using shared pointers. - Improve alignment of OpenGL context management between platforms. - Minor formatting and comment updates for consistency and clarity.

This changelist makes two strings local to the ConvolutionNode module, in order to work around a compiler issue on Linux.

- Simplify platform-specific logic in GLContext. - Add a null check to the Linux SimpleWindow destructor. - Remove a context-changing call from the GlslRenderer destructor. - Remove an unused header from the glew library. - Clarify an initializer in the GlslProgram constructor.

…odegraph_nodegraph

# Conflicts: # source/MaterialXCore/Interface.h # source/MaterialXRenderGlsl/TextureBaker.cpp

niklasharrysson · 2020-12-17T20:12:20Z

CHANGELOG.md

@@ -1,5 +1,6 @@
 # Change Log

+<<<<<<< HEAD


Merge leftover

niklasharrysson

LGTM! Only found a small merge issue above.

bernardkwok added 9 commits December 3, 2020 14:42

Initial check-in + test.

6ff89e0

Add testing and validation.

3983d41

Fix runtime mtlx export to output "nodegraph" or "nodename" as required based on connection type.

Merge branch 'adsk_contrib/dev' into adsk_contrib/nodegraph_nodegraph

4fe36e5

Fix invalid test document found via new interface validation.

e2260d5

Add test for :

a92ad8a

- multiple output connectins - downstream node connections.

multi-output node -> nodegraph update.

2e25652

Fix test.

92546db

Merge branch 'adsk_contrib/dev' into adsk_contrib/nodegraph_nodegraph

a68aac2

Add call to connect nodegraphs to each other same as node for Runtime.

134fb2d

Add new test to start testing cascaded nodegraphs.

bernardkwok added this to the 1.38 milestone Dec 8, 2020

bernardkwok added the Do Not Merge This PR should not be merged label Dec 8, 2020

bernardkwok added 2 commits December 8, 2020 17:01

Fix codacy warning.

c5fd321

Merge branch 'adsk_contrib/dev' into adsk_contrib/nodegraph_nodegraph

0189c3c

bernardkwok removed the Do Not Merge This PR should not be merged label Dec 9, 2020

bernardkwok requested a review from niklasharrysson December 9, 2020 20:33

bernardkwok commented Dec 9, 2020

View reviewed changes

bernardkwok requested a review from jstone-lucasfilm December 9, 2020 20:57

bernardkwok commented Dec 9, 2020

View reviewed changes

niklasharrysson reviewed Dec 10, 2020

View reviewed changes

source/MaterialXTest/MaterialXCore/Traversal.cpp Outdated Show resolved Hide resolved

bernardkwok and others added 9 commits December 10, 2020 08:13

Merge branch 'adsk_contrib/dev' into adsk_contrib/nodegraph_nodegraph

7ca7460

Merge branch 'adsk_contrib/dev' into adsk_contrib/nodegraph_nodegraph

3f95de6

Simplify image codec test

459c16c

Additional parameter updates for 1.38

2cc92db

Remove Parameter class

68e97c0

This changelist removes the legacy Parameter class from the v1.38 branch of MaterialX.

Remove ShaderRef classes

6b68b55

Material element class

dcc2826

Additional material unit tests

340d957

Initial look unit tests

9c5dc54

- Add initial C++ unit tests for looks and material assignments. - Extend material and look unit tests to Python. - Rename Look::getReferencedMaterialNode to Look::getReferencedMaterial for simplicity.

jstone-lucasfilm and others added 20 commits December 14, 2020 15:52

OpenGL improvements for software rendering

f5eccbc

Add material baking to GitHub Actions CI

da83fa9

Improved robustness in MaterialXRenderGlsl

1b381a4

- Clarify the lifetime and ownership of WindowWrapper objects using shared pointers. - Improve alignment of OpenGL context management between platforms. - Minor formatting and comment updates for consistency and clarity.

Make strings local to module

9258b8d

This changelist makes two strings local to the ConvolutionNode module, in order to work around a compiler issue on Linux.

82d3d17

2e8e67a

Improved color space handling in MaterialXRenderGlsl

6a887a4

Disable software render tests in v1.38

9026165

Patch.

8087988

Patch.

ad320a3

Patch texture baker.

8ad4e98

Fix test.

cf45883

Baker patch.

710c654

Fix crash in viewer.

b7a1ff2

Wasm dormant.

46600e8

Merge branch 'adsk_contrib/ILM_merge_Dec_14_2020' into adsk_contrib/n…

acde996

…odegraph_nodegraph

Merge branch 'adsk_contrib/ILM_merge_Dec_14_2020' into adsk_contrib/n…

da21e8f

…odegraph_nodegraph

Merge branch 'adsk_contrib/dev' into adsk_contrib/nodegraph_nodegraph

91d6b54

# Conflicts: # source/MaterialXCore/Interface.h # source/MaterialXRenderGlsl/TextureBaker.cpp

Review fixes.

d4e5dcd

Review updates.

a160b32

bernardkwok requested a review from niklasharrysson December 16, 2020 14:02

niklasharrysson reviewed Dec 17, 2020

View reviewed changes

CHANGELOG.md Outdated

@@ -1,5 +1,6 @@

# Change Log

<<<<<<< HEAD

Copy link

niklasharrysson Dec 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge leftover

niklasharrysson approved these changes Dec 17, 2020

View reviewed changes

Remove merge leftover.

12c481f

bernardkwok self-assigned this Dec 17, 2020

bernardkwok merged commit b40a45b into adsk_contrib/dev Dec 17, 2020

bernardkwok deleted the adsk_contrib/nodegraph_nodegraph branch December 17, 2020 21:14

ashwinbhat pushed a commit that referenced this pull request Aug 23, 2022

Add MDL implementation for mx_subsurface_bsdf (#1053)

322bf9b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

1.38 Explicit nodegraph inputs and nodegraph input connections #1053

1.38 Explicit nodegraph inputs and nodegraph input connections #1053

bernardkwok commented Dec 8, 2020 •

edited

Loading

bernardkwok Dec 9, 2020 •

edited

Loading

bernardkwok Dec 9, 2020

niklasharrysson Dec 10, 2020

bernardkwok Dec 11, 2020 •

edited

Loading

niklasharrysson Dec 11, 2020

jstone-lucasfilm Dec 12, 2020

bernardkwok Dec 15, 2020

bernardkwok Dec 9, 2020

bernardkwok Dec 9, 2020

bernardkwok Dec 9, 2020

bernardkwok Dec 9, 2020

bernardkwok Dec 9, 2020

bernardkwok Dec 9, 2020

bernardkwok Dec 9, 2020

bernardkwok Dec 9, 2020 •

edited

Loading

niklasharrysson Dec 17, 2020

niklasharrysson left a comment

1.38 Explicit nodegraph inputs and nodegraph input connections #1053

1.38 Explicit nodegraph inputs and nodegraph input connections #1053

Conversation

bernardkwok commented Dec 8, 2020 • edited Loading

Initial support:

Test 1: 3 nodegraphs connected

Test: node with nodegraph implementation connected downstream nodegraphs

node with non-nodegraph implementation connected to downstream nodegraph

3 non-pattern graphs

bernardkwok Dec 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bernardkwok Dec 11, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bernardkwok Dec 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niklasharrysson left a comment

Choose a reason for hiding this comment

bernardkwok commented Dec 8, 2020 •

edited

Loading

bernardkwok Dec 9, 2020 •

edited

Loading

bernardkwok Dec 11, 2020 •

edited

Loading

bernardkwok Dec 9, 2020 •

edited

Loading