Fix shape conversion for uniforms used as return expressions

[VicenteCartas]

🤖 This PR was created by the create-pr skill.

Summary

• handle TIntermBranch parents in injectShapeConversion, so a uniform used directly as a return expression can be rewritten
• skip uniforms that are already vec4 in UniformTypeChangeTraverser as a fast path
• add regression tests for a returned uniform and for an existing vec4 uniform array

Root cause

UniformTypeChangeTraverser widens loose scalar, vec2, and vec3 uniforms to vec4, then injects a shape conversion and replaces the original node in its parent. injectShapeConversion only handled aggregate, binary, unary, and selection parents, and threw for anything else:

Cannot replace symbol: node type handler unimplemented

The failing trigger is the parent node type, not the vector width. For return someUniform; the parent is a TIntermBranch, which was unhandled. Adding a branch handler is the actual fix.

An earlier revision of this PR attributed the failure to vec4 array reshaping. That was wrong: return someVec3; fails identically, and a vec3 must be widened so it cannot be skipped. Thanks to @bghgary for catching this.

Fast path (not the fix)

The type.getVectorSize() < 4 guard is retained because widening an existing vec4 is a no-op — the type is unchanged and addShapeConversion returns the node as-is, so the rewrite is pointless AST churn. It is an optimization, not the fix, and it does not resolve the underlying bug on its own.

Known remaining gaps

The same class of failure still exists for parents that glslang exposes without a setter, so they cannot be fixed here:

Construct	Parent node	Status
return u;	TIntermBranch	fixed by this PR
while (u)	TIntermLoop	still throws — TIntermLoop has getTest() but no setter
switch (u)	TIntermSwitch	still throws — TIntermSwitch has getCondition() but no setter

These need an upstream glslang change to expose setters, so the underlying bug should not be considered fully closed.

Validation

Built UnitTests from a clean worktree based on upstream master. Win32 D3D11 Debug.

Run	ExistingVec4UniformArrayIsNotReshaped	ReturnedUniformIsShapeConverted
This PR (branch handler + guard)	pass	pass
Guard reverted, branch handler kept	pass	pass
Branch handler reverted, guard kept	pass	fail — Vec3 return effect should compile synchronously

Row 2 shows the guard is not what fixes the bug. Row 3 shows the branch handler is. ShaderCompilation.CompileComprehensiveGLSL passes in all three configurations.

[VicenteCartas]

Self code review completed by the code-review workflow. No issues found.

Representative inputs were traced through the guard: scalar, vec2, and vec3 uniforms remain widened; matrices, samplers, structs, blocks, and existing vec4/ivec4/uvec4/bvec4 uniforms remain untouched. The focused vec4-array regression and existing comprehensive shader compilation test both pass locally on D3D11/Win32.

[Copilot]

Pull request overview

This PR fixes an issue in the shader AST normalization pass where uniforms that are already declared as vec4 (including vec4 arrays) could be unnecessarily rewritten during uniform widening, potentially producing an invalid shader AST and failing compilation. It also adds a regression test that compiles a shader containing an existing uniform vec4 values[2]; to ensure the traverser leaves it untouched.

Changes:

• Add a type.getVectorSize() < 4 guard to prevent rewriting existing vec4 uniforms during uniform widening.
• Add a regression unit test that compiles a shader containing a vec4 uniform array to validate the behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
Plugins/ShaderCompiler/Source/ShaderCompilerTraversers.cpp	Prevents the uniform-widening traverser from touching already-vec4 uniforms by gating on vector size.
Apps/UnitTests/Source/Tests.ShaderCompilation.cpp	Adds a regression test to ensure shaders with existing vec4 uniform arrays compile successfully.

[bghgary]

[Reviewed by Copilot on behalf of @bghgary]

The change is worth keeping, but the description and title attribute the failure to the wrong thing — please update them. The bug is not vec4-array reshaping: return someVec3; fails identically, and this PR does not fix that. Details inline.

[bghgary]

[Reviewed by Copilot on behalf of @bghgary]

LGTM. Verified locally at 406ac46: both tests pass on Win32 D3D11 RelWithDebInfo.