AcademySoftwareFoundation / MaterialX

MaterialX is an open standard for the exchange of rich material and look-development content across applications and renderers.
http://www.materialx.org/
Apache License 2.0
1.82k stars 332 forks source link

Nodegraph inputs are not propagated to all connections #779

Open rasmusbonnedal opened 2 years ago

rasmusbonnedal commented 2 years ago

We are experimenting with standard_surface_brick_procedural.mtlx and changing its properties. It seems to me that the inputs in the nodegraph (for example hue_variation) are copied by value to each node that is connected to it, and then is only one of those nodes stored as a path for the input variable. Here is a simple repro to show what I mean:

<?xml version="1.0"?>
<materialx version="1.38" colorspace="lin_rec709">
  <nodegraph name="NG_Split">
    <input name="in_color" type="color3" value="0.5, 0.5, 0.5" />
    <splitlr name="node_split_0" type="color3">
      <input name="valuel" type="color3" interfacename="in_color" />
      <input name="valuer" type="color3" interfacename="in_color" />
    </splitlr>
    <output name="base_color_output" type="color3" nodename="node_split_0" />
  </nodegraph>
  <standard_surface name="N_StandardSurface" type="surfaceshader">
    <input name="base_color" type="color3" nodegraph="NG_Split" output="base_color_output" />
  </standard_surface>
  <surfacematerial name="M_Split" type="material">
    <input name="surfaceshader" type="surfaceshader" nodename="N_StandardSurface" />
  </surfacematerial>
</materialx>

When opening this in MaterialXView and changing in_color in Property Editor, only one of the inputs to the split node is updated: image

Version tested: MaterialX main branch as of today (a78d13f6)

jstone-lucasfilm commented 2 years ago

Good catch, @rasmusbonnedal, and I believe that's specifically a limitation of the Property Editor in MaterialXView. It would definitely be worthwhile to fix this.

rasmusbonnedal commented 2 years ago

Big disclaimer that I may have misunderstood this, but I'm not sure it is limited to MaterialXView.

This is an excerpt from the generated glsl ps and osl of the above material:

uniform vec3 node_split_0_valuel = vec3(0.404137, 0.000000, 0.000000);
uniform vec3 node_split_0_valuer = vec3(0.500000, 0.500000, 0.500000);
color node_split_0_valuel = color(0.404137, 0, 0),
color node_split_0_valuer = color(0.5, 0.5, 0.5),

in_color is not present in the generated shader code.

MaterialX inserts the value of in_color in all nodes connected to it. When MaterialXView queries the path to the uniform from MaterialX it gets one of the inserted values. How can I get all paths where in_color has been inserted through the MaterialX api?

niklasharrysson commented 2 years ago

Thanks for reporting the issue @rasmusbonnedal. I had a look today and it's a limitation to code generation of compound nodegraphs.

MaterialX differentiates between compound nodegraphs and functional nodegraphs. A compound nodegraph is an explicit use of a nodegraph to wrap a set of nodes to organize your network, like in your example. A functional nodegraph is a way to specify an implementation of an atomic node (the implementation for a nodedef).

Compound nodegraphs with a specified input interface, like in your example, is a quite new construct and apparently we need to update codegen to take this input interface into account. We'll keep this issue open to track the development of a fix.

Until a fix is done, a workaround is to use a functional nodegraph instead. In this case you specify your input interface through a nodedef, and then set the nodegraph to be the implementation of this nodedef. Here's an updated doc with your example:

<?xml version="1.0"?>
<materialx version="1.38" colorspace="lin_rec709">
  <!-- Definition and implementation of node <splitter> -->
  <nodedef name="splitter_color3" node="splitter" type="color3">
    <input name="in_color" type="color3" value="0.5, 0.5, 0.5" />
    <output name="out" type="color3" />
  </nodedef>
  <nodegraph name="NG_Split" nodedef="splitter_color3">
    <splitlr name="node_split_0" type="color3">
      <input name="valuel" type="color3" interfacename="in_color" />
      <input name="valuer" type="color3" interfacename="in_color" />
    </splitlr>
    <output name="out" type="color3" nodename="node_split_0" />
  </nodegraph>
  <!-- Using an instance of node <splitter> -->
  <splitter name="splitter1" type="color3">
    <input name="in_color" type="color3" value="0.5, 0.5, 0.5" />
  </splitter>
  <standard_surface name="N_StandardSurface" type="surfaceshader">
    <input name="base_color" type="color3" nodename="splitter1" />
  </standard_surface>
  <surfacematerial name="M_Split" type="material">
    <input name="surfaceshader" type="surfaceshader" nodename="N_StandardSurface" />
  </surfacematerial>
</materialx>

This will force codegen to generate a separate function from the nodegraph, thus hiding the internal inputs node_split_0_valuel and node_split_0_valuer, and only publish the single input in_color.

2021-12-14

rasmusbonnedal commented 2 years ago

Great explanation @niklasharrysson, thanks!

We are seeing an issue that might be somewhat related. Is it supposed to be possible to have two compound nodegraphs providing outputs to a shader? We are seeing an issue when there is a node with the same name in both nodegraphs. This shader generates incorrect code for both osl and glsl. Version tag v1.38.3.

<?xml version="1.0"?>
<materialx version="1.38" colorspace="lin_rec709">
  <nodegraph name="NG_1">
    <output name="out_color" type="color3" nodename="c1" />
    <constant name="c1" type="color3">
      <input name="value" type="color3" value="0, 0, 1" />
    </constant>
  </nodegraph>

  <nodegraph name="NG_2">
    <output name="out_float" type="float" nodename="c1" />
    <constant name="c1" type="float">
      <input name="value" type="float" nodename="0.5" />
    </constant>
  </nodegraph>

  <standard_surface name="SR_input" type="surfaceshader">
    <input name="base_color" type="color3" nodegraph="NG_1" output="out_color" />
    <input name="metalness" type="float" nodegraph="NG_2" output="out_float" />
  </standard_surface>

  <surfacematerial name="InputMaterial" type="material">
    <input name="surfaceshader" type="surfaceshader" nodename="SR_input" />
  </surfacematerial>
</materialx>

Also, when the nodes with conflicting names have the same type it may silently choose the wrong node.