fix(vk): restore Deku Tree floor web lighting on NVIDIA #1442

github-sync · 2026-11-10T02:46:54Z

RoksTK commented

2026-11-10 02:46:54 +00:00

(Migrated from github.com)

Summary

Restore lighting on the circular floor web in the Great Deku Tree on NVIDIA. Request SPIR-V Inline for the 128-bit global-load helper in vertex shader b449bcdb69b36a8a (draw 927f31eddda79218). This reuses the existing EDEN_NX2_GM_INLINE switch and applies the same workaround used for a related task and mesh shader miscompile in #1334.

The original report is #455. The floor-web follow-up in #1002 reports that the underground web still renders black.

Cause

The grid dimensions and origin matched the values in memory, and the emitted SPIR-V used the dimensions load for the X and Z coordinate clamps. The NVIDIA shader readback showed those clamps using the origin load instead. That produced coarse cell index 0x4ccffff9 where the captured position should select cell 0. Requesting inlining restored cell 0, subdivision 32, and all eight expected fine-probe offsets.

Testing

In-game A/B on Windows 11, RTX 5070 Ti, NVIDIA driver 621.01, Vulkan, with the v2.0.11 host and my fork's renderer. Load a Great Deku Tree save with the round floor web intact and stand at its edge. The web was nearly black before the change. The strands looked normally lit after the change with the shader probes off. Manual coverage was limited to that setup and focused on visual correctness.
Built the Windows Vulkan renderer from this PR branch on upstream main at 9a2e80e, using LLVM-MinGW and RX_VK_ONLY=1. The game-free IR regression verifies that the workaround switches the vertex shader's 128-bit helper to Inline while the 32-bit helper stays DontInline. All 22 emitted fixture modules passed spirv-val for Vulkan 1.3.
Ran python -I -B tests/run_all.py locally on Windows. 187 passed, 16 were skipped, and the same five failures reproduced on unmodified upstream main in this environment.

AI assistance

OpenAI Codex assisted with diagnosis, implementation, the game-free test, and this write-up. The investigation used existing Claude-assisted IR probes. I ran the in-game checks and confirmed the visual result. The commit includes both AI attribution trailers.

Before the fix

After the fix

## Summary Restore lighting on the circular floor web in the Great Deku Tree on NVIDIA. Request SPIR-V `Inline` for the 128-bit global-load helper in vertex shader `b449bcdb69b36a8a` (draw `927f31eddda79218`). This reuses the existing `EDEN_NX2_GM_INLINE` switch and applies the same workaround used for a related task and mesh shader miscompile in #1334. The original report is #455. The [floor-web follow-up in #1002](https://git.magicbean.dev/MagicBeanDev/Hookshot/issues/1002) reports that the underground web still renders black. ## Cause The grid dimensions and origin matched the values in memory, and the emitted SPIR-V used the dimensions load for the X and Z coordinate clamps. The NVIDIA shader readback showed those clamps using the origin load instead. That produced coarse cell index `0x4ccffff9` where the captured position should select cell `0`. Requesting inlining restored cell `0`, subdivision `32`, and all eight expected fine-probe offsets. ## Testing - In-game A/B on Windows 11, RTX 5070 Ti, NVIDIA driver 621.01, Vulkan, with the v2.0.11 host and my fork's renderer. Load a Great Deku Tree save with the round floor web intact and stand at its edge. The web was nearly black before the change. The strands looked normally lit after the change with the shader probes off. Manual coverage was limited to that setup and focused on visual correctness. - Built the Windows Vulkan renderer from this PR branch on upstream main at `9a2e80e`, using LLVM-MinGW and `RX_VK_ONLY=1`. The game-free IR regression verifies that the workaround switches the vertex shader's 128-bit helper to `Inline` while the 32-bit helper stays `DontInline`. All 22 emitted fixture modules passed `spirv-val` for Vulkan 1.3. - Ran `python -I -B tests/run_all.py` locally on Windows. 187 passed, 16 were skipped, and the same five failures reproduced on unmodified upstream main in this environment. ## AI assistance OpenAI Codex assisted with diagnosis, implementation, the game-free test, and this write-up. The investigation used existing Claude-assisted IR probes. I ran the in-game checks and confirmed the visual result. The commit includes both AI attribution trailers. ## Before the fix <img width="1544" height="784" alt="Nearly black floor web in the Great Deku Tree before the fix" src="https://github.com/user-attachments/assets/239a1106-4b78-45e9-88b3-d22f639824e5" /> ## After the fix <img width="2557" height="1437" alt="Lit floor web in the Great Deku Tree after the fix" src="https://github.com/user-attachments/assets/5fa789d9-cb59-4964-83fb-9be9a4e25c54" />

NotRexed commented

2026-11-10 03:49:44 +00:00

(Migrated from github.com)

This landed in the v2.0.12 branch (merged there for testing; it reaches main with that release), so closing. Thanks!

This landed in the v2.0.12 branch (merged there for testing; it reaches `main` with that release), so closing. Thanks!

NotRexed commented

2026-11-10 03:55:42 +00:00

(Migrated from github.com)

Reopened: closing this was a mistake on my side. It was tested in the v2.0.12 branch, and it will be merged into main here, so it shows as merged. Sorry for the noise.

hckrman101 (Migrated from github.com) approved these changes

2026-11-10 04:15:19 +00:00

hckrman101 (Migrated from github.com) left a comment

Thanks, this is a clean, well-scoped workaround and the write-up is very good.

On AMD/Intel: no effect at all. inline_global_128_loads is only set when vendorID == 0x10DE and the vertex key is b449bcdb69b36a8a, so other vendors (and every other shader on NVIDIA) still emit the same SPIR-V they do today. Inline is only a hint and doesn't change semantics, so even if it reached another vendor it wouldn't break anything. The cache side is fine as well, since vk_shader.cpp and the eden tree are both in HOOKSHOT_SHADER_STAMP, and EDEN_NX2_GM_INLINE=0 still turns it off.

On root cause: this works around a driver miscompile, it doesn't fix one. I couldn't find anything in the helper bodies (no shared Private state, nothing call-order dependent) that would let a correct compiler swap one call's result for another's. It's the same failure as #1334 (one call's result used in place of another's), so I think treating it as an NVIDIA bug is right. The open question is whether to scope it this narrowly, see the inline note.

The new fixture looks right: the 32-bit helper stays DontInline and only the 128-bit one changes. It runs in the existing source-regressions job. No CI ran on this PR (fork workflows waiting for approval), so a maintainer should kick it off before merging. No overlap with what's landed on main since the base.

Thanks, this is a clean, well-scoped workaround and the write-up is very good. On AMD/Intel: no effect at all. `inline_global_128_loads` is only set when `vendorID == 0x10DE` and the vertex key is `b449bcdb69b36a8a`, so other vendors (and every other shader on NVIDIA) still emit the same SPIR-V they do today. `Inline` is only a hint and doesn't change semantics, so even if it reached another vendor it wouldn't break anything. The cache side is fine as well, since `vk_shader.cpp` and the eden tree are both in `HOOKSHOT_SHADER_STAMP`, and `EDEN_NX2_GM_INLINE=0` still turns it off. On root cause: this works around a driver miscompile, it doesn't fix one. I couldn't find anything in the helper bodies (no shared Private state, nothing call-order dependent) that would let a correct compiler swap one call's result for another's. It's the same failure as #1334 (one call's result used in place of another's), so I think treating it as an NVIDIA bug is right. The open question is whether to scope it this narrowly, see the inline note. The new fixture looks right: the 32-bit helper stays `DontInline` and only the 128-bit one changes. It runs in the existing source-regressions job. No CI ran on this PR (fork workflows waiting for approval), so a maintainer should kick it off before merging. No overlap with what's landed on main since the base.

gpu/vk/vk_shader.cpp

					
				@ -1520,0 +1520,4 @@

				            // Deku Tree web (#455): with DontInline, NVIDIA's compiled vertex shader substitutes the grid-origin

				            // load result for the grid dimensions in the coordinate clamps. The SPIR-V names the right result.

				            constexpr uint64_t kDekuWebVertexCode = 0xb449bcdb69b36a8aull;

				            vrt.inline_global_128_loads = dev.props.vendorID == 0x10DE && vsc->key == kDekuWebVertexCode;

Non-blocking: gating on one shader hash fixes this draw, but any other vertex shader with the same pattern (several 128-bit global loads feeding each other) will still miscompile on NVIDIA, and nobody will notice until someone reports a black mesh. The 41 s compile-time problem that kept DontInline only came from the culling compute programs with hundreds of loads. Vertex shaders have very few, so turning on inlining for every vertex stage on NVIDIA (or every vendor, since it's only a hint) should be cheap and would cover the whole class. If you'd rather keep it narrow for now, that's fine, maybe just add a short comment saying so.

Non-blocking: gating on one shader hash fixes this draw, but any other vertex shader with the same pattern (several 128-bit global loads feeding each other) will still miscompile on NVIDIA, and nobody will notice until someone reports a black mesh. The 41 s compile-time problem that kept `DontInline` only came from the culling compute programs with hundreds of loads. Vertex shaders have very few, so turning on inlining for every vertex stage on NVIDIA (or every vendor, since it's only a hint) should be cheap and would cover the whole class. If you'd rather keep it narrow for now, that's fine, maybe just add a short comment saying so.