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.87k stars 352 forks source link

Graph Editor: working with color nodes (add/convert?) seems unreliable #1460

Closed marwie closed 1 year ago

marwie commented 1 year ago

Hello, I'm currently having issues with color nodes sometimes not working as expected / seemingly not being evaluated anymore.

The following video shows the nodegraph as it should work (as expected)

https://github.com/AcademySoftwareFoundation/MaterialX/assets/5083203/7b0470f5-dfb4-41c3-bfee-d0a9a10ae2c7

But for some reason the graph below doesn't evaluate (I guess something

https://github.com/AcademySoftwareFoundation/MaterialX/assets/5083203/6bd1e256-e8d6-45c2-80dd-8d63d42297f1

https://github.com/AcademySoftwareFoundation/MaterialX/assets/5083203/4aa3457b-52be-4ab7-aac6-9b588febc3fc

Issue 1

I just noticed is that I deleted the nested nodegraph (in the editor) but e.g. metalness still has the nodegraph/output reference. So deleting the input graph didn't properly cleanup the shader input connection rendering the whole graph invalid

Here is graph from the video after having deleted the nested nodegraph.

<?xml version="1.0"?>
<materialx version="1.38">
  <standard_surface name="needle_standard_surface" type="surfaceshader" xpos="5.282609" ypos="-0.827586">
    <input name="base_color" type="color3" value="0.8, 0.8, 0.8" />
    <input name="metalness" type="float" nodegraph="needle_shadergraph" output="out_1" />
    <input name="specular_roughness" type="float" nodegraph="needle_shadergraph" output="out_2" />
  </standard_surface>
  <surfacematerial name="Default" type="material" xpos="7.144928" ypos="-1.000000">
    <input name="surfaceshader" type="surfaceshader" nodename="needle_standard_surface" />
  </surfacematerial>
  <convert name="add_to_color3" type="color3" xpos="3.949275" ypos="-1.034483">
    <input name="in" type="color4" nodename="add" />
  </convert>
  <dot name="const__Color" type="color4" xpos="0.000000" ypos="-3.931035">
    <input name="in" type="color4" value="1, 1, 1, 0" />
  </dot>
  <add name="add" type="color4" xpos="2.644928" ypos="-1.862069">
    <input name="in1" type="color4" value="0, 0, 0, 0" />
    <input name="in2" type="color4" value="0, 0, 0, 0" />
  </add>
  <convert name="vector4_to_color4" type="color4" xpos="0.934783" ypos="-1.724138">
    <input name="in" type="vector4" interfacename="input_vector4" />
  </convert>
  <input name="input_vector4" type="vector4" value="9.2, 0, 0, 0" xpos="-0.478261" ypos="-1.827586" />
  <input name="_Color" type="color4" value="0, 0, 0, 1" xpos="-0.992754" ypos="-3.482759" />
  <constant name="constant_color4" type="color4" xpos="1.115942" ypos="-2.689655">
    <input name="value" type="color4" value="1, 0, 0, 0" />
  </constant>
</materialx>

Original graph

Here is the original graph where this structure was nested in a nodegraph element I'm currently trying to understand why the color values are clamped

<?xml version="1.0"?>
<materialx name="" version="1.38" >
    <nodegraph name="needle_shadergraph" >
        <!-- Color -->
        <input name="_Color" type="color4" value="1.0, 1.0, 1.0, 1.0" />
        <dot name="const__Color" type="color4" >
            <input name="in" type="color4" interfacename="_Color" />
        </dot>
        <convert name="vector4_to_color4" type="color4" >
            <input name="in" type="vector4" value="3.28, 0.0, 0.0, 0.0" />
        </convert>
        <add name="add" type="color4" >
            <input name="in1" type="color4" nodename="const__Color" />
            <input name="in2" type="color4" nodename="vector4_to_color4" />
        </add>
        <convert name="add_to_color3" type="color3" >
            <input name="in" type="color4" nodename="add" />
        </convert>
        <output name="out" type="color3" nodename="add_to_color3" />
        <constant name="Integer" type="integer" >
            <input name="value" type="integer" value="1" />
        </constant>
        <convert name="Integer_to_float" type="float" >
            <input name="in" type="integer" nodename="Integer" />
        </convert>
        <output name="out_1" type="float" nodename="Integer_to_float" />
        <!-- Smoothness -->
        <input name="_Smoothness" type="float" value="0.0" />
        <dot name="const__Smoothness" type="float" >
            <input name="in" type="float" interfacename="_Smoothness" />
        </dot>
        <convert name="const__Smoothness_to_vector2" type="vector2" >
            <input name="in" type="float" nodename="const__Smoothness" />
        </convert>
        <constant name="Vector_2" type="vector2" >
            <input name="value" type="vector2" nodename="const__Smoothness_to_vector2" />
        </constant>
        <swizzle name="swizzle_0_Vector_2" type="float" >
            <input name="in" type="vector2" nodename="Vector_2" />
        </swizzle>
        <output name="out_2" type="float" nodename="invert" />
        <invert name="invert" type="float" >
            <input name="in" type="float" nodename="swizzle_0_Vector_2" />
        </invert>
    </nodegraph>
    <standard_surface name="needle_standard_surface" type="surfaceshader" >
        <!-- BaseColor -->
        <input name="base_color" type="color3" nodegraph="needle_shadergraph" output="out" />
        <!-- Metallic -->
        <input name="metalness" type="float" nodegraph="needle_shadergraph" output="out_1" />
        <!-- Smoothness -->
        <input name="specular_roughness" type="float" nodegraph="needle_shadergraph" output="out_2" />
    </standard_surface>
    <surfacematerial name="Default" type="material" >
        <input name="surfaceshader" type="surfaceshader" nodename="needle_standard_surface" />
    </surfacematerial>
</materialx>
kwokcb commented 1 year ago

Hi @marwie , @jstone-lucasfilm I tested this with the downstream update fix and it seems to work okay now (with the original graph). BTW, you can get rid of the constant nodes and dot in between other nodes as they are not needed.

Here's a video of my tests with the latest: https://github.com/AcademySoftwareFoundation/MaterialX/assets/49369885/feccc319-464e-43ce-b542-ab653fcc195a

jstone-lucasfilm commented 1 year ago

Looks good to me, @kwokcb, and thanks for the fix!