Graph Editor: Reproducible crash after copying and connecting node
Version: dfffe83f74e742edc3743f004da359c35cef5652 on windows
When I was debugging another issue, I found a crash accidently:
Repoduce step:
- Launch the Graph Editor with its default marble material.
- Click on the
NG_marble1node and copy it. - Break the connect of he out of the origin
NG_marble1node toSR_marble1'ssubsurface_color - Connect the out of the origin
NG_marble1node to the new created one'sbase_color_1 - Connect the new created one's
base_color_1toSR_marble1'ssubsurface_color
Hello, I would like to work on this ticket for ASWF Dev Days 2024.
Thanks to @fnRaihanKibria for adding new checks in #2029, which reduce the severity of this error when it occurs, and I'm copying the notes from @niklasharrysson that describe next steps for fully resolving the error:
The cause of this issue is a limitation in the support for cascading nodegraphs.
When there are multiple nodegraphs connected to each other these may be flattened into a single large ShaderGraph for codegen. But there are no handling of the case where nodes internal to these graphs may share the same name. So when connections are constructed and nodes are referenced by name this breaks down..
A typical case is copy/paste of a graph as in https://github.com/AcademySoftwareFoundation/MaterialX/issues/1932, where all nodes in the copy share names with the original.
So the symptom is that a cycle is found in the graph, but it's actually caused by nodes not being uniquely named.
I think the best fix for this is to stop flattening nodegraphs, and preserving the nodegraph hierarchies, when creating the codegen ShaderGraph. There are other benefits of this as well, for example an issue we've discussed before with limiting the number of uniforms that are generated for a shader (if nodegraphs are not flattened it's much easier to keep internal parameters private).
I will take a closer look at getting this work started as soon as possible.