GPUOpen-LibrariesAndSDKs / BlenderUSDHydraAddon

This add-on allows you to assemble and compose USD data with Blender data and render it all using various renderers via Hydra.
Apache License 2.0
363 stars 37 forks source link

MaterialX importer: Shader inputs precedency #264

Open bowald opened 2 years ago

bowald commented 2 years ago

Hi

I have found a case when the MaterialX importer gets a different visual result than MaterialXViewer. However, It's a bit of an edge case and I can't find a precedency in the MaterialX specs for inputs when multiple valid connections exists. So I'm not sure if this should be considered a bug or not.

Consider a MaterialX file containing:

Syntax for the input would be: <input name="base_color" type="color3" value="1, 0, 0" nodename="blue" output="green" nodegraph="NG_green" />

This means that we have multiple valid values for the input, but which one to chose?

In MaterialXViewer and OSL, the precedency seems to be: nodegraph > nodename > value I.e the expected base_color will be green

But in this importer it seems like the precedency is value > nodename > nodegraph The base_color is red, and no other nodes are imported.

If I render the attached material in MaterialX I get this output

But when importing it in Blender using the add-on I got this image

Where I'm expecting to get an input similar to this:

As stated in the beginning. I'm not sure what the standard says when it comes to precedency, I have asked a question regarding this on the ASWF Slack., so I'm not entirely sure if this is a bug or not. But it can be good to look into as currently RPR have a different result when importing a mtlx file such as the one described.

Tested versions:

The attached material

<materialx version="1.38">
  <nodegraph name="NG_green">
    <output name="green" type="color3" nodename="green_node" />
    <constant name="green_node" type="color3" nodedef="ND_constant_color3">
      <input name="value" type="color3" value="0, 1, 0" />
    </constant>
  </nodegraph>

  <constant name="blue" type="color3" nodedef="ND_constant_color3">
      <input name="value" type="color3" value="0, 0, 1" />
  </constant>

  <standard_surface name="SR" type="surfaceshader">
    <input name="base_color" type="color3" value="1, 0, 0" nodename="blue" output="green" nodegraph="NG_green" />
  </standard_surface>

  <surfacematerial name="Mat" type="material">
    <input name="surfaceshader" type="surfaceshader" nodename="SR" />
  </surfacematerial>
</materialx>
bsavery commented 2 years ago

Interesting. I think the importer is probably hooking up whatever it finds in order so what should the importer too? Give a warning that multiple connections are there? Or just take the last it finds?

bowald commented 2 years ago

Based on the discussion on ASFW Slack it seems that the current precedence for all the generators (GLSL/OSL/MDL) is confirmed to be nodegraph > nodename > value

That being said, the standard states that input could be value or connection to an output or node. So even if the case above currently is passing validation, it may not be in a future version of MaterialX.

From a MaterialX editor standpoint, I would argue that the current precedency in the generators is quite good. Because the "value" could then be used as a default, but when a user connects an output or a node to that input, it would then be overridden. Since the old value would still be present in MaterialX XML it can then be used again if the connection later would be dropped by the user.

Anyway, we could either: