Skip to content

Stricter check for eliding for Shader Optimization#2754

Merged
jstone-lucasfilm merged 3 commits intoAcademySoftwareFoundation:mainfrom
bowald:fix/shadergen-bypass-issues
Jan 30, 2026
Merged

Stricter check for eliding for Shader Optimization#2754
jstone-lucasfilm merged 3 commits intoAcademySoftwareFoundation:mainfrom
bowald:fix/shadergen-bypass-issues

Conversation

@bowald
Copy link
Contributor

@bowald bowald commented Jan 19, 2026

This is a follow up to the PR #2746.

Me and @rasmustilljander continued to look into issues related to upgrading our MaterialX 1.38 materials to MaterialX 1.39.
The PR #2746 solved an issue for custom nodes getting classified as CONSTANTS but without inputs. However, we discover some more appearances having shaders generated with syntax errors not fixed by last PR. This PR tightens the check to make sure only constant/dot nodes are elided.

Here is a "simplified Material" that fails shader code generation on main, where the custom nodes is considered for bypassing even if they have 2 outputs (i.e. not a constant node).

<?xml version="1.0"?>
<materialx version="1.39" xmlns:xi="http://www.w3.org/2001/XInclude">
  <nodedef name="ND_custom_a" node="custom_a">
    <input name="specular_roughness_raw_in" type="float" value="0.5" />
    <output name="specular_roughness_out" type="float" />
    <output name="specular_ior_out" type="float" />
  </nodedef>
  <nodegraph name="NG_custom_a" nodedef="ND_custom_a">
    <constant name="constant_0" type="float">
      <input name="value" type="float" interfacename="specular_roughness_raw_in"/>
    </constant>
    <output name="specular_roughness_out" type="float" nodename="constant_0" />
    <output name="specular_ior_out" type="float" nodename="constant_0" />
  </nodegraph>
  <nodedef name="ND_custom_b" node="custom_b">
    <output name="specular_roughness_raw_out" type="float" />
    <output name="specular_anisotropy_out" type="float" />
    <input name="normal_raw_in" type="vector3" value="0.5, 0.5, 1.0" />
  </nodedef>
  <nodegraph name="custom_b" nodedef="ND_custom_b">
    <output name="specular_roughness_raw_out" type="float" nodename="constant0_ST0054" />
    <output name="specular_anisotropy_out" type="float" nodename="spec_anisotropy_ST0054" />
    <constant name="spec_anisotropy_ST0054" type="float">
      <input name="value" type="float" value="0.37" />
    </constant>
    <constant name="spec_rotation_ST0054" type="float">
      <input name="value" type="float" value="0.37" />
    </constant>
    <constant name="constant0_ST0054" type="float">
      <input name="value" type="float" value="0.37" />
    </constant>
  </nodegraph>
  <nodegraph name="NG_APPEARANCE">
    <output name="specular_roughness" type="float" nodename="custom_a_inst01" output="specular_roughness_out" />
    <output name="specular_anisotropy" type="float" nodename="custom_b_inst01" output="specular_anisotropy_out" />
    <custom_a name="custom_a_inst01" type="multioutput">
      <input name="specular_roughness_raw_in" type="float" nodename="custom_b_inst01" output="specular_roughness_raw_out" />
    </custom_a>
    <custom_b name="custom_b_inst01" type="multioutput">
    </custom_b>
  </nodegraph>
  <standard_surface name="SR_APPEARANCE" type="surfaceshader">
    <input name="base_color" type="color3" value="0.73, 0.68, 0.57" />
    <input name="specular_roughness" type="float" nodegraph="NG_APPEARANCE" output="specular_roughness" />
    <input name="specular_anisotropy" type="float" nodegraph="NG_APPEARANCE" output="specular_anisotropy" />
  </standard_surface>
  <surfacematerial name="APPEARANCE" type="material">
    <input name="surfaceshader" type="surfaceshader" nodename="SR_APPEARANCE" />
  </surfacematerial>
</materialx>

This is the content of the nodegraph NG_APPEARANCE, where both custom_a_inst01, custom_b_inst01 are considered constant and on main they will be elided even though that is incorrect.
image

Copy link
Member

@jstone-lucasfilm jstone-lucasfilm left a comment

Choose a reason for hiding this comment

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

This looks like a good improvement, thanks @bowald, and I had just a few minor notes for clarity.

Copy link
Member

Choose a reason for hiding this comment

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

Let's just inline these two tests, rather than creating an intermediate constant, as I believe that will read more clearly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps "Constant node doesn't follow expected interface, cannot elide"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hehe, I need to include a spell checker in my IDE. Thanks for addressing this.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps "Dot note doesn't follow expected interface, cannot elide", matching the comment above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Member

@jstone-lucasfilm jstone-lucasfilm left a comment

Choose a reason for hiding this comment

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

Thanks for the fix, @bowald!

@jstone-lucasfilm jstone-lucasfilm merged commit adef8cd into AcademySoftwareFoundation:main Jan 30, 2026
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments