Prevent a ShaderGraph crash from duplicate color transforms

[LouRohanNV]

Summary

Each filename input on ND_triplanarprojection_color3 can request a color transform for the same shader output. Those requests create transform nodes with the same identifier. The node map replaces the first node, but the node order and downstream connections still point to it. This can crash shader generation, whether the inputs use identical or different color spaces.

This change keeps the first transform request for each output port and ignores later requests for that port. It also makes ShaderGraph::addNode() throw ExceptionShaderGenError on a duplicate identifier, instead of replacing a node with active references. Input transform requests need no deduplication because applyInputTransforms() visits each shader input once.

Fixes #3060.

Tests

The regression test generates shaders from triplanar documents with identical and different input color spaces. Both cases check that generation succeeds and that exactly one output transform remains, using the first input's color space. These document tests exercise the finalize path where the crash occurs.

The focused test passes with 10 assertions. All 11 tests in [genshader] pass, with 251 assertions.

./bin/MaterialXTest 'GenShader: Duplicate Output Color Transforms'
./bin/MaterialXTest '[genshader]'

Limitation

The retained transform still applies after the triplanar blend. For different input color spaces, the first retained conversion applies to the combined result, so the colors can still be incorrect. Correct conversion of each image sample before the blend remains a separate change under #2182.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: LouRohanNV / name: Lou Rohan (d437270)

[jstone-lucasfilm]

Thanks for this proposed fix, @LouRohanNV, as well as for the great overview in #3060. Your diagnosis matches what I see in the code. Each filename input on a color-typed node queues its own output transform in applyInputTransforms, and since every transform node for a given output is named <output>_cm, the second call to ShaderGraph::addNode replaces the first node in _nodeMap while _nodeOrder and the rerouted downstream connections still point to it.

One issue, though, is that the exact-pair check only covers the case where all three inputs share a color space. If the inputs specify different color spaces, as in the configuration described in #2182, the three requests differ in their source color space and survive the check, so the same collision and crash still occur. Ideally I'd like this PR to handle the full class of collisions rather than the single-color-space instance, and I believe two changes would get us there:

• In populateColorTransformMap, deduplicate the output map by shader port alone, retaining the first request. A shader output can carry only a single transform node in the current design, so this matches the behavior of single-filename nodes such as image, and it makes the input-side check unnecessary, since applyInputTransforms visits each shader input exactly once.
• In ShaderGraph::addNode, throw an ExceptionShaderGenError when a node with the given unique ID already exists, rather than replacing the map entry. This turns any future collision of this kind into a clear error rather than a use-after-free.

On the test, I'd suggest a document-level regression test in place of the TestShaderGraph subclass, following the pattern of the "Color Space No-Op" test in GenShader.cpp, since that exercises the finalize path where the crash occurs. Two documents would cover both the identical and differing color space cases above.

More broadly, I agree that the correct long-term behavior for a nodegraph-implemented color node is to apply each transform to the inner image node before the blend, and it seems completely reasonable to defer that improvement to a future PR for #2182.

[jstone-lucasfilm]

Great work, thanks @LouRohanNV, and this fix looks ready to merge.