[FIocker]

On AMD, the water in the Royal Family Tomb shows a red dotted line. Separately, when sirit meets an undefined operand it aborts the whole game without logging why. This PR fixes both. It's part of the "super build" series, but it builds and passes the tests on its own, on current main.

Part	Branch	Builds
Shader fixes (this PR)	super-build/shaders (#1830)	on its own
Upscaling and frame generation	super-build/upscaling-framegen (#1831)	only with #1833 and #1834
DLSS 5 neural rendering	super-build/neural (#1833)	only with #1831 and #1834
Menu, renderer and build plumbing	super-build/core (#1834)	only with #1831 and #1833

What changed since the first review

The branch is rebuilt on main at d7a7306. Main now has its own versions of most of what this PR used to carry, so those parts are dropped:

• The R2UR span (fix(shaders): R2UR's source is one register, not a pair (a vertex program refused) #1806).
• The bit-32 immediate and compute local memory (fix(shaders): two large compute programs lower (an immediate is not R254) and get their local memory #1800).
• LDG/STG bit 78 with its MEMBAR.GL fences (fix(shaders): LDG/STG [URn+imm] with ordering bit 78 lower (2 compute programs) #1778).
• The closing self-branch (fix(shaders): a loop that leaves by predicated EXITs ends at its closing self-branch (the last program that did not lower) #1810).
• SULD.1D, LEA.HI with a constant base, and ISBERD.O.

lower_to_maxwell.h and its tests are now exactly main's. Also dropped:

• on_big_stack and the RX_VK_TRACE_KEYS tracing, with its process-wide set_terminate.
• The vk_texture.cpp changes (memory_guest_contained, the unswizzle threshold, texture_touch_misses). They moved to feat: menu, renderer and build plumbing for #1831/#1833; upload hitches; lock diagnostics (builds with #1831, #1833) #1834, which describes them.
• The test_shader_stamp_cache.py change, which also moved to feat: menu, renderer and build plumbing for #1831/#1833; upload hitches; lock diagnostics (builds with #1831, #1833) #1834.

What it does

• Noperspective varyings on AMD (vk_shader.cpp; Eden's SPIR-V back end: runtime_info.h, spirv_emit_context.*, emit_spirv_special.cpp, emit_spirv_context_get_set.cpp).
  • The bug: the Royal Family Tomb's water writes clip.xy/clip.w as a noperspective varying. When a vertex with w close to zero is clipped, AMD interpolates the wrong depth, and its exp2 overflows into a red dotted line across the water.
  • The fix: the vertex stage writes those outputs multiplied by Position.w and lets them interpolate perspective-correct; the fragment stage multiplies by FragCoord.w to get the screen-linear value back. A geometry stage restores the unscaled values after EmitVertex.
  • Where it applies: every VS+FS pipeline whose fragment shader has a noperspective input, on every vendor. The result is the same value, interpolated a different way.
  • Where it doesn't: it's skipped when the fragment shader reads inputs by index (the multiply can't be undone there), when transform feedback is on (it would capture the scaled value), and for mesh pipelines.
• sirit: an operand with no SPIR-V definition (a null Id) now throws instead of std::abort(). EmitSPIRV's catch names the IR op and its arguments, and the program is reported as not translated instead of ending the game with nothing in the log.
• RX_VK_LOWER_SKIP=<key prefix,...>: leaves those programs unlowered, for bisecting. It's a diagnostic and does nothing unless set.

Testing

• Builds: the changed files compile on this branch alone with clang and nx2/build.py's renderer flags.
• Tests: tests/run_all.py on this branch, locally (clang targeting MSVC, the CPM Vulkan headers, glslang 11.12), gives 324 passed. The 18 failures are the same 18 main at d7a7306 has on this machine, all down to this PC's toolchain, so there are no new failures.
• In game: Windows 11, NVIDIA RTX, OoT v131072, as part of the combined build: no pipeline changes visible on NVIDIA. The AMD fix is from the fork's AMD testing; I have no AMD GPU to confirm it here.

🤖 Generated with Claude Code

[ghost]

Thanks for splitting the super build up. Several of these fixes are real, but this branch doesn't stand on its own yet, and about half of the lowering changes overlap with open or merged PRs in the same file.

Blocking

• Build break: upload_unswizzle_gpu calls memory_guest_contained(), but nothing on this branch declares or defines it. It's declared in vk_internal.h on super-build/all, which isn't part of this PR (see inline).
• Three registered tests fail as written. run_all.py picks up every tests/test_*.py. These are probably the "tests that need a compiler" that didn't run on your machine:
  • test_lower_suld_1d.py expects a 1D coordinate in R254 to lower, and 1D_BUFFER / 1D_ARRAY to be refused. Both checks now go the other way.
  • test_lower_lea_hi_constant.py expects a high source (Rc not RZ) and a zero shift to be refused. The new LEA.HI block accepts both.
  • test_shader_stamp_cache.py now asserts that nx2/build.py has streamline_sdk_path and opennr_jobs. Neither is on this branch; they come from #1833/#1834.
• No CI has run on this branch. It's 26 commits behind main.

Overlap with other PRs (please rebase and drop the duplicates):

• R2UR span: #1806 is merged (9edb3a5) with the same FieldSpan line, so drop this copy. Main has it after the extended_renaming check, and yours comes before it, so git may keep both.
• Immediate at bit 32 + compute local memory: #1800 (approved) fixes both. Its bit-32 skip is narrowed to form 4 of the ALU families, and it sanity-checks +0x68 against +0x50 (≤64 KB). The !is_register[f] skip here is the broad version. It drops the bit-32 registers of TLD4 0xb63/0xf63, TMML 0xb69/0xf69, TEX 0xf60, SHFL.IDX 0x589 and STL 0x987, none of which is in the register_field list. Their Rb then stops getting its REGMAP prefix whenever another field is high. That's a silent miscompile of programs that lower fine today. Please keep #1800's version and drop these hunks.
• LDG/STG bit 78: #1778 widens the same masks and brackets the access with MEMBAR.GL when bit 78 is set. Bits 77-79 all set is .STRONG.GPU, as on the RED/ATOMG cases in this file. Here the qualifier is just dropped. The fences need to stay, so please take #1778's (approved) version. The LDG mask here also lets bit 78 through without bit 77.
• Closing self-branch: #1810 changes the same line. Your version (no pending forward branch or call target) is more general than #1810's "previous instruction is an unconditional BRA/EXIT", and I think it's sound: an unconditional self-branch never falls through, and finish re-checks the relocations. Only one of them should land, though. Two small differences from the EXIT path: it doesn't check barrier_tokens, and it allows a call target ≤ index before main_exited, which the EXIT path refuses. It also has no test.

Lowering details

• SULD: main already lowers SULD.P.1D, which covers those 17 programs. This hunk adds 1D_BUFFER and 1D_ARRAY, which main's comment says OoT never uses, and there's no test for them. It also gets the coordinate limits wrong. 1D and 1D_BUFFER need one register, so R254 should stay allowed (this is the regression the test catches). 1D_ARRAY needs two (x, layer), so its limit is 253, not 252.
• LEA.HI constant base: the Rc/.X decoding mirrors the 0x211 register form, so the new paths look right. But main's mask also required bits 84-86 and 91-95 to be zero, and this check no longer looks at them, so unknown qualifiers there now pass silently. Please keep the mask, widened only by bit 74 and Rc. Also update the test for what is now accepted. Note that main's comment already says the 40 no-carry programs lower.
• ISBERD.O: the lowering and frontend look fine. Nit: IsbeReadOutput ignores .32 (bit 44), so a U8 read would return the whole word. PayloadWord already clamps, so the extra Clamped in the task branch is redundant.

Not in the description (worth their own PR, or at least a mention):

• The noperspective-varying rewrite (RuntimeInfo::noperspective_outputs, the vertex outputs scaled by Position.w, fragment inputs multiplied by FragCoord.w). The math checks out, but it changes every VS+FS pipeline with a noperspective input on every vendor, not only AMD.
• upload_unswizzle_gpu no longer imports a pool by default, and the multithreaded CPU unswizzle threshold drops from 16 MB to 4 MB. Both are behaviour and performance changes.
• texture_touch_misses() has no caller on this branch.

On "70 programs": per the description, that number comes from the in-game programs (0 failed) log on the combined super-build/all branch, from one Kokiri Forest run on NVIDIA. It wasn't measured on this branch against current main. Main already lowers the 40 LEA.HI and 17 SULD.1D programs, and the R2UR one is merged too. It would help to say which programs still fail on main and which hunk fixes each one.

Nits: on_big_stack is described as a precaution that turned out not to be needed, and it's Windows-only. Its trace path calls std::set_terminate from a worker thread, which replaces the handler for the whole process. The LOWER_DEBUG block and the RX_VK_LOWER_SKIP / RX_VK_TRACE_KEYS hooks are fine as diagnostics.

[ghost]

memory_guest_contained isn't declared or defined anywhere on this branch (it's in vk_internal.h / vk_memory.cpp on super-build/all), so vk_texture.cpp won't compile on its own.

[ghost]

nx2/build.py on this branch has neither streamline_sdk_path nor opennr_jobs, so this assert fails. This change belongs with #1833/#1834.

[ghost]

Duplicate of #1806, which is now merged. Main has this line after the extended_renaming check, so please drop this one when you rebase.

[ghost]

This skips every bit-32 field that isn't in the register_field list, and that list misses opcodes whose lowering reads a register at 32: TLD4 0xb63/0xf63, TMML 0xb69/0xf69, TEX 0xf60, SHFL.IDX 0x589 and STL 0x987. Before, those still got marked whenever another field set any_high. Now their Rb loses the REGMAP prefix. #1800 (approved) skips only bit 32 of form 4 of the ALU families, the actual immediate. Please keep that version and drop this one.

[ghost]

1D and 1D_BUFFER read one coordinate register, so R254 should stay allowed. Main allows it and test_lower_suld_1d.py checks for it. 1D_ARRAY reads two (x, layer), so its limit is 253. Accepting 1D_BUFFER and 1D_ARRAY also flips that test's refusal checks, and OoT doesn't seem to use either. Maybe leave them refused until a program needs them.

[ghost]

Bits 77-79 all set is .STRONG.GPU (as on RED/ATOMG here), so dropping it loses the ordering. #1778 covers the same mask and fences the access with MEMBAR.GL when bit 78 is set, so I'd keep that version. This mask also lets 78 through without 77.

[ghost]

Main's mask also required bits 84-86 and 91-95 to be zero. This check doesn't test them, so an unknown qualifier there now passes silently. Could you keep the old mask, widened only by bit 74 (.X) and Rc? Also, test_lower_lea_hi_constant.py expects Rc≠RZ and a zero shift to be refused, so it fails now and needs updating.

[ghost]

#1810 changes this same spot with a narrower rule (the previous instruction must be an unconditional BRA/EXIT). I think yours is sound, but only one should land. Small gaps compared with the EXIT path: no barrier_tokens check, and call targets ≤ index are allowed before main_exited. A test would help.

[ghost]

#1800 reads this field too, and cross-checks it against +0x50 (== 32 × +0x68) with a 64 KB cap and a log line. A 1 MB cap with no check is much looser, so please take #1800's version.

[ghost]

This noperspective rewrite isn't mentioned in the description. It affects every VS+FS pipeline with a noperspective input on every vendor. Worth its own PR, or at least a line in the description.

[FIocker]

Thanks for the thorough review; you were right on all counts. I rebuilt the branch on main at d7a7306, and it now builds and tests on its own. The description is rewritten.

Blocking:

• memory_guest_contained: the vk_texture.cpp changes moved to feat: menu, renderer and build plumbing for #1831/#1833; upload hitches; lock diagnostics (builds with #1831, #1833) #1834, together with the function and its description. Nothing on this branch references them now.
• The three failing tests:
  • test_lower_suld_1d.py and test_lower_lea_hi_constant.py: lower_to_maxwell.h is now exactly main's, so both pass unchanged.
  • test_shader_stamp_cache.py: the change moved to feat: menu, renderer and build plumbing for #1831/#1833; upload hitches; lock diagnostics (builds with #1831, #1833) #1834, where streamline_sdk_path and opennr_jobs live.
• No CI: I can't approve the runs, but I ran the whole tests/run_all.py on this branch locally with clang targeting MSVC, the CPM Vulkan headers and glslang 11.12. It gives 324 passed; the 18 failures are the same 18 main has on this machine, all down to the toolchain, so there are no new ones.

Overlap: since your review, #1806, #1800, #1778 and #1810 all merged, and main also has SULD.1D, LEA.HI with a constant base and ISBERD.O. I dropped all of ours in their favour, including the broad !is_register[f] skip, the 1 MB local-memory cap, the unfenced LDG/STG and our closing self-branch.

Not in the description before:

• The noperspective rewrite is now the PR's main subject, with the reason: the red dotted line over the Royal Family Tomb water on AMD, where a clipped vertex with w near zero gets the wrong depth interpolated. It also lists where it applies (every VS+FS pipeline with a noperspective input, all vendors) and where it's skipped.
• The unswizzle import and the 4 MB threshold moved to feat: menu, renderer and build plumbing for #1831/#1833; upload hitches; lock diagnostics (builds with #1831, #1833) #1834. That description explains both hitches they fix (22-25 ms per import, three at the Deku Tree turn).
• texture_touch_misses moved with its caller.

"70 programs": no longer claimed. Main now lowers everything those hunks did.

Nits: on_big_stack and RX_VK_TRACE_KEYS are gone, and with them the worker thread's std::set_terminate. RX_VK_LOWER_SKIP stays as a diagnostic. IsbeReadOutput's .32 and the redundant Clamped went with our ISBERD.O, since main's is used now.

[Joey-Magnus]

The rebase onto d7a7306 handled the earlier review, but main has moved again and vk_shader.cpp conflicts in translate_into. When you resolve it, note that #2042 removed oot_visual(), so the line you moved up with the fragment translation won't compile any more. Main's block now just translates and fills frt. The math is right: perspective interpolation of v*w, times FragCoord.w (1/w), gives back the screen-linear value. But it rewrites every VS+FS pipeline with a noperspective input on every vendor, and it hasn't run on AMD, the GPU it's for. Could you get one AMD run of the Royal Family Tomb with before/after screenshots, plus a quick NVIDIA check that nothing else with noperspective varyings moved? RX_VK_LOWER_SKIP isn't in translator_stamp, so programs translated while it was set stay unlowered in the cache after it's unset. That's fine for a diagnostic, but say so in the comment. The "(super build)" tags in comments can go.

[ghost]

This now conflicts with main in gpu/vk/vk_shader.cpp. Could you merge main into the branch (or rebase) when you get a chance? I'll take another look once it's up to date.

[omgitsgela]

Steam Deck (RADV, VanGogh): the red particles and the red water both clear

Independent hardware confirmation that this fix addresses the red artifacts reported in #1634 / #1000, not just the Royal Family Tomb water.

Setup. Hookshot v2.1.0-rc.3-52-gebbba2e (upstream ebba2e), Steam Deck LCD, RADV on VanGogh, FSR 3.1 from the Handheld picture (1536x864) at 50% render resolution.

Patch. This PR's noperspective work only: the six hunks in runtime_info.h, spirv_emit_context.{h,cpp}, emit_spirv_context_get_set.cpp, emit_spirv_special.cpp and vk_shader.cpp. The RX_VK_LOWER_SKIP bisection and the sirit / emit_spirv error-reporting changes were left out, so this is the interpolation change alone. It applies to ebba2e with offsets only and builds with the release toolchain (clang 23.1.5, renderer-only rebuild, 50 s, 0 failed).

Before. Blinking red particle boxes over most of the screen, everywhere in Kokiri Village, and red-tinted sun highlights on the water — the same red as the particles. Reproduced consistently, and more obvious at 50% render resolution because upscaling magnifies the wrong pixels (a 1440p frame downscaled to the 800p panel averaged them away).

After. Same build, same save, same settings, only the renderer swapped (original 7b32684f…, patched 1f37986a…): the red particles are gone and the water's sun highlights render in their normal colour. Reported by the person playing, confirmed twice.

Mechanism, for the record. Main already emits NoPerspective; what it lacks is this PR's clipped-vertex workaround, so a varying whose vertex clips near w = 0 still interpolates wrongly on AMD and the consuming exp2 saturates to red. On an RTX 3090 the same build is clean, which is why this looked Deck-specific.

Not tested here: geometry-shader and mesh-program paths, and any effect that reads indexed attributes (the PR leaves those alone by design). No game files, saves or logs attached.

[ghost]

Heads-up first: I merged main into the branch (d85bd1b) to clear the vk_shader.cpp conflict, so please pull before you push again. The conflict came from #2042 removing oot_visual() (Joey-Magnus pointed it out). The merge keeps your fragment translation moved ahead of the vertex stage and drops the oot_visual call. On my side the shader tests pass with it and the Vulkan back end builds.

Everything from my last review is handled. The rebuilt branch no longer touches lower_to_maxwell.h, vk_texture.cpp or the tests, so the build break, the three failing tests and the overlaps with #1806/#1800/#1778/#1810 are all gone, and so are on_big_stack and the set_terminate. The noperspective rewrite is now the PR's subject, and the description says what it changes on every vendor.

I went through the noperspective path again:

• The math holds, clipping included. Perspective-correct interpolation of v·w, times FragCoord.w (the interpolated 1/w), gives the screen-linear value. On a clipped edge, the clip-space lerp of v·w divided by the lerp of w is the same as the screen-space lerp of v.
• The vertex side covers every output element, including split variables (dedup by id). The fragment side only marks float, non-per-primitive inputs. The scaling runs after ConvertDepthMode, which only touches z.
• Indexed reads, XFB and the mesh path (its own frt) are left alone. The GS EmitVertex hooks are a no-op for now, because only the VS+FS branch sets noperspective_outputs.
• No manual cache bump is needed. HOOKSHOT_SHADER_STAMP hashes eden/, sirit/ and vk_shader.cpp, so the new SPIR-V gets a new stamp by itself.
• omgitsgela's Steam Deck run (RADV, VanGogh) is the AMD confirmation Joey asked for. With only the noperspective hunks applied, the red particles and the red water highlights from #1634/#1000 go away, and the same build is clean on an RTX 3090. Together with your NVIDIA run, that's enough for me.

Left, none of it blocking:

• Joey's point about RX_VK_LOWER_SKIP still stands: it isn't in translator_stamp, so a bisection only works on programs that aren't cached yet. The easiest fix is to add "RX_VK_LOWER_SKIP" to the prefix list. bake_ds.cpp needs the same entry, because test_program_stamp_knobs.py checks that the two lists match. Otherwise, say it in the comment.
• nx2/bake_ds/bake_ds.cpp copies this VS+FS path but doesn't set noperspective_outputs / scaled_noperspective_inputs, so an offline-baked bank would still have the old interpolation. It's self-consistent and not broken, just without the fix. It already leaves out other live-only bits, so a follow-up is fine.
• Nits: the (super build) tags are still in emit_spirv.cpp, stream.h and vk_shader.cpp. A before/after screenshot pair in the description would be good, since this shows on screen (CONTRIBUTING.md). omgitsgela's would do if they're OK with that.

CI: only the title check has run on this branch, and the build and test workflows haven't. That's why GitHub shows it as unstable.

[ghost]

Nit: loads is already info.loads.mask | info.passthrough.mask (top of DefineInputs), so the info.passthrough.AnyComponent(PositionX) term repeats loads.AnyComponent(PositionX). || noperspective_as_smooth is the only part that does something.

[FIocker]

Thanks, both of you, for merging main in and for the review.

@omgitsgela: thank you for the Steam Deck run. Good to know it clears the red particles from #1634/#1000 too, not just the tomb water.

The non-blocking notes are in #2782:

• bake_ds.cpp now bakes the noperspective workaround as live translation does.
• RX_VK_LOWER_SKIP is in translator_stamp and bake_ds.cpp's list.
• The redundant passthrough term is gone.
• The (super build) tags are gone.