fix(shaders): the noperspective workaround baked offline too; RX_VK_LOWER_SKIP in the program stamp #2782

Open
github-sync wants to merge 1 commit from github/pr/2782 into main
Owner

@FIocker opened this pull request on GitHub · 2026-11-12 12:49 UTC · original

#1830 merged with a few non-blocking notes left from hckrman101's and Joey-Magnus's reviews; this PR is those. The main one: an offline-baked program bank didn't get the AMD noperspective workaround that live translation now has.

What it does

  • The workaround baked offline too (nx2/bake_ds/bake_ds.cpp):
    • The VS+FS path now translates the fragment stage before it emits the vertex stage, as vk_shader.cpp translate_into does.
    • It sets noperspective_outputs / scaled_noperspective_inputs the same way: not with indexed fragment reads and not with transform feedback.
    • Without this, a bank baked offline kept the old interpolation, so the red dotted line and particles on AMD (#1634, #1000) came back for programs taken from it.
  • RX_VK_LOWER_SKIP is in the program stamp: it's in translator_stamp's prefix list and in bake_ds.cpp's, which test_program_stamp_knobs.py keeps equal. Programs translated while it's set are kept apart in the cache, so a bisection also sees programs that were already cached. The comment says so.
  • One redundant term removed: DefineInputs' loads is already info.loads.mask | info.passthrough.mask, so FragCoord's condition is just loads.AnyComponent(PositionX) || noperspective_as_smooth.
  • The "(super build)" tags are gone from emit_spirv.cpp, stream.h and vk_shader.cpp.

Testing

  • Builds: vk_shader.cpp, spirv_emit_context.cpp and emit_spirv.cpp compile with clang using nx2/build.py's renderer flags. bake_ds.cpp is POSIX-only; I syntax-checked it on Windows with stand-in unistd.h/dlfcn.h, so Linux CI is its real build.
  • Tests: the stamp, bake and shader checks pass locally, test_program_stamp_knobs.py among them. test_bake_portability.py fails on this PC for main as well.
  • Screenshots for #1830: omgitsgela's Steam Deck run there is the before/after. If they're OK with it, their pair would go well in #1830's description.

🤖 Generated with Claude Code

> [@FIocker](https://github.com/FIocker) opened this pull request on GitHub · 2026-11-12 12:49 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2782) #1830 merged with a few non-blocking notes left from hckrman101's and Joey-Magnus's reviews; this PR is those. The main one: an offline-baked program bank didn't get the AMD noperspective workaround that live translation now has. ## What it does - **The workaround baked offline too** (`nx2/bake_ds/bake_ds.cpp`): - The VS+FS path now translates the fragment stage before it emits the vertex stage, as `vk_shader.cpp translate_into` does. - It sets `noperspective_outputs` / `scaled_noperspective_inputs` the same way: not with indexed fragment reads and not with transform feedback. - Without this, a bank baked offline kept the old interpolation, so the red dotted line and particles on AMD (#1634, #1000) came back for programs taken from it. - **`RX_VK_LOWER_SKIP` is in the program stamp**: it's in `translator_stamp`'s prefix list and in `bake_ds.cpp`'s, which `test_program_stamp_knobs.py` keeps equal. Programs translated while it's set are kept apart in the cache, so a bisection also sees programs that were already cached. The comment says so. - **One redundant term removed**: `DefineInputs`' `loads` is already `info.loads.mask | info.passthrough.mask`, so FragCoord's condition is just `loads.AnyComponent(PositionX) || noperspective_as_smooth`. - **The "(super build)" tags** are gone from `emit_spirv.cpp`, `stream.h` and `vk_shader.cpp`. ## Testing - **Builds**: `vk_shader.cpp`, `spirv_emit_context.cpp` and `emit_spirv.cpp` compile with clang using `nx2/build.py`'s renderer flags. `bake_ds.cpp` is POSIX-only; I syntax-checked it on Windows with stand-in `unistd.h`/`dlfcn.h`, so Linux CI is its real build. - **Tests**: the stamp, bake and shader checks pass locally, `test_program_stamp_knobs.py` among them. `test_bake_portability.py` fails on this PC for main as well. - **Screenshots** for #1830: omgitsgela's Steam Deck run there is the before/after. If they're OK with it, their pair would go well in #1830's description. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- github-sync:item:2782 -->
- nx2/bake_ds/bake_ds.cpp translates the fragment stage before it emits the vertex stage, as vk_shader.cpp translate_into does,
  and sets noperspective_outputs / scaled_noperspective_inputs the same way (not with indexed fragment reads or transform
  feedback): an offline-baked bank now has the AMD clipped-vertex workaround as live translation does
- RX_VK_LOWER_SKIP is in translator_stamp's prefix list and bake_ds.cpp's (test_program_stamp_knobs keeps the two equal):
  programs translated while it is set are kept apart in the cache, so a bisection sees cached programs too
- spirv_emit_context.cpp: loads already holds passthrough's mask, so FragCoord's condition is loads or noperspective_as_smooth
- the (super build) tags gone from emit_spirv.cpp, stream.h and vk_shader.cpp

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Author
Owner

@Joey-Magnus reviewed on GitHub · 2026-11-12 14:35 UTC · original

I merged this into current main and ran the bake, stamp and shader checks: test_bake_ds, test_bake_portability, test_bake_skeleton, test_program_stamp_knobs, test_shader_stamp_*, test_prepared_stamps and the vk/shader/program subset of run_all. All pass. The bake_ds.cpp change matches vk_shader.cpp translate_into:1341-1348 line for line: fragment first, no indexed attribute reads, xfb_count == 0, and scaled_noperspective_inputs set from the vertex mask. Dropping the passthrough term in DefineInputs is right, since loads already ORs in passthrough. RX_VK_LOWER_SKIP is now in both prefix lists, and test_program_stamp_knobs keeps them equal. One correction to the description: no CI job builds nx2/bake_ds/bake_ds.cpp. The Linux jobs only run the source checks, and nothing in .github compiles it through nx2/bake_ds/build.sh. So it is still unbuilt after CI. I couldn't build it here either (no fmt/boost externals). Please run nx2/bake_ds/build.sh once on Linux before this merges, or ask someone who has the externals to.

> [@Joey-Magnus](https://github.com/Joey-Magnus) reviewed on GitHub · 2026-11-12 14:35 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2782#pullrequestreview-5876255123) I merged this into current main and ran the bake, stamp and shader checks: test_bake_ds, test_bake_portability, test_bake_skeleton, test_program_stamp_knobs, test_shader_stamp_*, test_prepared_stamps and the vk/shader/program subset of run_all. All pass. The bake_ds.cpp change matches vk_shader.cpp translate_into:1341-1348 line for line: fragment first, no indexed attribute reads, xfb_count == 0, and scaled_noperspective_inputs set from the vertex mask. Dropping the passthrough term in DefineInputs is right, since loads already ORs in passthrough. RX_VK_LOWER_SKIP is now in both prefix lists, and test_program_stamp_knobs keeps them equal. One correction to the description: no CI job builds nx2/bake_ds/bake_ds.cpp. The Linux jobs only run the source checks, and nothing in .github compiles it through nx2/bake_ds/build.sh. So it is still unbuilt after CI. I couldn't build it here either (no fmt/boost externals). Please run nx2/bake_ds/build.sh once on Linux before this merges, or ask someone who has the externals to. <!-- github-sync:r:5876255123 -->
Commenting is not possible because the repository is archived.
No description provided.