Skip to content

Zero-initialize struct-typed function-local variables (fixes Area Lights, OpenPBR Analytic Lights)#1720

Closed
bkaradzic-microsoft wants to merge 1 commit into
BabylonJS:masterfrom
bkaradzic-microsoft:fix-tpc-1654-zero-init-struct-locals
Closed

Zero-initialize struct-typed function-local variables (fixes Area Lights, OpenPBR Analytic Lights)#1720
bkaradzic-microsoft wants to merge 1 commit into
BabylonJS:masterfrom
bkaradzic-microsoft:fix-tpc-1654-zero-init-struct-locals

Conversation

@bkaradzic-microsoft
Copy link
Copy Markdown
Contributor

Summary

Babylon.js v6+ shaders for Area Lights (computeAreaLighting) and a number of other PBR/OpenPBR helpers declare a struct local without an initializer, then immediately do read-modify-write on its fields:

lightingInfo result;
result.specular += value;
result.diffuse  += value;
return result;

This is undefined behavior under the GLSL spec (locals are not zero-initialized), but every WebGL driver in practice tolerates it. SPIRV-Cross faithfully reproduces the pattern in its HLSL output, and D3DCompile correctly rejects it with error X4000: variable used without having been completely initialized. Result: Area Lights - Standard Material and any other test that exercises this code path fails to compile its fragment shader on D3D11 / D3D12 today.

Fix

Add a new AST traverser, ZeroInitializeStructLocals, that walks every function body and prepends EOpAssign(symbol, EOpConstructStruct(0, 0, ...)) for each unique struct-typed EvqTemporary symbol it references. The SPIR-V emitter then sees a normal store; SPIRV-Cross emits HLSL with the struct local definitely initialized; D3DCompile accepts it. Any subsequent real assignment in the body is dead-code eliminated by DXC's optimizer, so functional behavior is unchanged.

Wired into the DXBC, DXIL, Metal, and Vulkan compile pipelines (same as the existing sampler-handling and dFdy traversers). Arrays of structs are intentionally skipped — they are uncommon in BJS shaders and would require constructing a synthetic array constructor.

Verification

End-to-end on build/win32-default (D3D11, Chakra, RelWithDebInfo) and build/win32-gl (OpenGL/ANGLE, V8, Release):

Test D3D11 OpenGL
idx 191 Area Lights - Standard Material PASS (1027 px diff) PASS (1131 px diff)
idx 194 Area Lights - PBR (regression check for #1718) PASS PASS
idx 558 OpenPBR Fuzz Weight vs Fuzz Roughness - Analytic Lights PASS PASS
idx 559 OpenPBR Fuzz Weight vs Coat Weight w/ Normal Maps - Analytic Lights PASS PASS
idx 574 OpenPBR Transmission Dispersion VS IOR - Analytic Lights PASS PASS
idx 0/39/47/165 (baseline regression checks) PASS PASS

Wide regression sweep:

  • D3D11: 135 indices (1-30, 100-115, 160-180, 285-305, 380-405, 540-560) → 134 PASS / 1 FAIL. The single failure is idx 22 Outline with Error: Unsupported alpha mode: -1 — pre-existing, unrelated to shader pipeline.
  • OpenGL: 113 indices (1-50, 100-120, 250-270, 400-420) → 52 PASS / 60 SKIP / 1 FAIL. Same idx 22 failure.

No new regressions on either backend.

Why a BN traverser (not a SPIRV-Cross fix)

Both the SPIRV-Cross fork-extension route and the BN traverser route would solve this. We chose the traverser because:

  1. It avoids extending the BabylonJS/SPIRV-Cross fork further.
  2. The fix lives next to the existing sampler/dFdy traversers, which use the exact same pattern.
  3. It applies uniformly to all four backends (DXBC, DXIL, Metal, Vulkan) with one change.

Out of scope

  • idx 192/193 (Textured - Area Lights - *) still fail with Unsupported texture format or type: format 5, type 3553. This is a NativeEngine::PrepareImage whitelist gap and orthogonal to this PR.
  • A separate cluster of OpenPBR-IBL tests (idx 557/561/575) fails with a JS-side TypeError: 'FLOAT' undefined before any shader compile. Will be filed as a separate issue.

Babylon.js emits shaders such as `computeAreaLighting` (inlined into the
fragment shader by every Area Lights / Standard Material scene) that
declare a local struct without an initializer and immediately do
read-modify-write on its fields, e.g.

    lightingInfo result;
    result.specular += value;
    result.diffuse  += value;
    return result;

This is undefined behavior under the GLSL spec (locals are not
zero-initialized), but every WebGL driver in practice tolerates it. The
HLSL that SPIRV-Cross emits faithfully reproduces the pattern, and
D3DCompile correctly rejects it with `error X4000: variable used without
having been completely initialized`. The result is that Area Lights -
Standard Material (and any other test that exercises this code path)
fails to compile its fragment shader on D3D11 / D3D12.

Add a new AST traverser, `ZeroInitializeStructLocals`, that walks every
function body and prepends an `EOpAssign(symbol, EOpConstructStruct(0,
0, ...))` for each unique struct-typed `EvqTemporary` symbol it
references. The SPIR-V emitter then sees a normal store; SPIRV-Cross
emits HLSL with the struct local definitely initialized; D3DCompile
accepts it. Any subsequent real assignment in the body is dead-code
eliminated by DXC's optimizer, so functional behavior is unchanged.

Wired into the DXBC, DXIL, Metal and Vulkan compile pipelines (same as
the existing sampler-handling and dFdy traversers). Arrays of structs
are intentionally skipped — they are uncommon in BJS shaders and would
require constructing a synthetic array constructor that does not match
any GLSL syntax.

Verified end-to-end on Win32 D3D11 RelWithDebInfo / Chakra:

  idx 191 Area Lights - Standard Material : PASS  (was X4000 in
                                                   D3DCompile)
  idx 194 Area Lights - PBR               : PASS  (regression check
                                                   for BabylonJS#1718)
  idx 0/39/47/165                          : PASS  (baseline)
  134/135 wide regression sweep            : PASS  (only failure is
                                                   the pre-existing
                                                   idx 22 Outline JS
                                                   error, unrelated)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 28, 2026 16:40
@bkaradzic-microsoft bkaradzic-microsoft deleted the fix-tpc-1654-zero-init-struct-locals branch May 28, 2026 16:42
@bkaradzic-microsoft
Copy link
Copy Markdown
Contributor Author

Closed and superseded by #1721 (branch was renamed for housekeeping).

@bkaradzic-microsoft bkaradzic-microsoft review requested due to automatic review settings May 28, 2026 17:03
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.

1 participant