<surfacematerial> without `surfaceshader` declaration will crash code generation (GLSL)

[kwokcb]

Issue

If you create a <surfacematerial> but don't specify the surfaceshader and try to generate code then code generation can crash when it tries to emit the function for a ShaderNode created by default for the surfaceshader input. This is due to the fact that this node has no implementation and code tries to read through these null implementations.

Example

<?xml version="1.0"?>
<materialx version="1.38">
  <displacement name="displacement_float" type="displacementshader">
    <input name="displacement" type="float" value="0.0" />
    <input name="scale" type="float" value="1.0" />
  </displacement>
  <surfacematerial name="material_displacement_float_out" type="material">
    <input name="displacementshader" type="displacementshader" nodename="displacement_float" />
  </surfacematerial>
</materialx>

Initial Triage

• All I could figure out was that a ShaderNode was being created based on the surfacematerial nodedef signature instead of just failed during initla ShaderGraph creation time. This would avoid trying to parse this node to get it's implementation later on inside the MaterialNode implementation.
• Marking for GLSL only as did not test with other code generators.

Note: There are pre-checks for this in integrations but it seems that this check should exist within code-generation to not try and create a dummy surface shader as not sure what it means to have a material with no surface shader but have a displacement shader?

[madmann91]

As of today, on main (c7f01d8), this MaterialX network can be loaded both via MaterialXGraphEditor and MaterialXView, without crashing. Should this perhaps be closed then?

[kwokcb]

Hi @madmann91 , I'm not sure. I don't recall if this was performed via unit tests and there was additional protection added to those apps to prevent a crash. Might be best to add this to the testsuite and run RTS first.

[jstone-lucasfilm]

Although there are likely still improvements needed on the GLSL code generation side, I believe we should close this issue as we no longer have a straightforward repro case. Feel free to write up a new issue if we encounter this again in the future.