MagicBeanDev/Longshot

Watch

Fork 98

WIP: fix(vk): give each light its own shadow map in the water's light loop (for testing against #107) #545

Closed

kingo wants to merge 2 commits from kingo/Longshot:test/water-handle-table-v3 into main

kingo commented

2026-11-14 18:58:22 +00:00

First-time contributor

Why

Light squares on water, ice and snow (the "Navi light" blocks, https://github.com/NotRexed/Hookshot/issues/2509): a player still sees them in the Water Temple. This is our Hookshot fix for them, opened so someone can test it against main's own fix (#107). It predates #107 and conflicts with it on purpose: it is on main 9eae480e, the commit before #107 merged, so the two can be compared.

What changed

vk: gpu/vk/eden/shader_recompiler/ir_opt/texture_pass.cpp: FindHandleTableIndex() finds a light's index in the address of its 96-byte record (a plain multiply, or an XMAD pair's 16-bit halves in either order) and builds the shadow-map handle as a table (HandleOp::Index), so each light samples its own shadow map instead of the dummy; ir_opt/passes.h, shader_info.h: the index operation and the array.
vk: gpu/vk/vk_shader.cpp, vk_shader.h, vk_draw.cpp: the draw evaluates the handle once per element; RX_VK_HANDLE_TABLE and RX_VK_HANDLE_TABLE_COUNT in the program cache's stamp.
tests: tests/test_handle_table.py (new); test_texture_handle_banks.py, test_translation_pools.py, test_program_answer_domains.py follow the new operation.
docs: two rows in docs/development.md.

Flags and settings

Flag	What it does	Default now	Recommendation	Helps on	Doesn't help on	Untested on
`RX_VK_HANDLE_TABLE=0`	The old search (one texture for every light)	on	On, if it tests better than #107	Water, ice, snow lit by several lights (Hookshot, RTX 5060 Ti)	-	Longshot in game, AMD, Intel, Linux
`RX_VK_HANDLE_TABLE_COUNT=<n>`	The table's size (2 to 256)	64	Leave at 64	-	-	-

Measurements

main: 9eae480e (2026-11-13), the commit before #107
this PR: test/water-handle-table-v3 @ 63ad82d2
Hardware: RTX 5060 Ti 8 GB, driver 32.0.16.1088; Intel Core Ultra 7 265F; 64 GB; Windows 11
Scene: Zora's Domain and Zora's River (child, day), Chamber of Sages; 1920x1080
Method: on Hookshot (main 8c359bca + these two commits), in game, screenshots with and without the fix; on this branch, the game-free checks and a renderer build only.
How to reproduce: Zora's Domain or the Water Temple, standing by lit water; RX_VK_HANDLE_TABLE=0 brings the squares back.

	main @ `9eae480e`	this PR @ `63ad82d2`
Squares on the water (Zora's Domain, Hookshot)	yes	none
`tests/test_handle_table.py`	fails (not there)	passes

Not measured on Longshot: no fps or VRAM numbers; asked for testing first.

Testing

Tested: on Hookshot in game: Zora's Domain (child and adult), Zora's River and the Chamber of Sages clean with the fix. On this branch: tests/test_handle_table.py, test_texture_handle_banks.py, test_translation_pools.py pass with llvm-mingw on Windows; the renderer builds (RX_VK_ONLY).

Not tested: this branch in game on Longshot; the full run_all.py; against #107 side by side.

Needs other testers:

A player who still sees the squares (Water Temple): build this PR, compare with the newest nightly (which has #107), expecting no squares here (T-X4WT)

Screenshots

Nothing from this branch yet; the Hookshot before/after is on the issue linked above.

Risk and rollback

Conflicts with main (#107 replaced the same code): this is for testing, not to merge as it is. If it fixes a case #107 misses, the difference would be ported onto #107 in a new PR. RX_VK_HANDLE_TABLE=0 turns it off.

Issues

See https://github.com/NotRexed/Hookshot/issues/2509

Checklist

For everyone

One topic.
The title is a Conventional Commit (type(scope): description).
Rebased on current main. (No, on purpose: based on 9eae480e, before #107, for comparison.)
python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above). (Only the touched checks run.)
Tested in the game, or what wasn't is named under Testing.
Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests).
Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. (Not measured: for testing.)
No game files, dumps, build output or generated files.
third_party/ untouched (a new folder with a PROVENANCE.md instead).
The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

If applicable

AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
Flags noted, and the switch reference updated (docs/development.md; not a menu row).
A game-free test added for the fix, failing on main and passing here.
A check under tests/ that reads changed source updated in this PR, not weakened.

🤖 Generated with Claude Code

## Why Light squares on water, ice and snow (the "Navi light" blocks, https://github.com/NotRexed/Hookshot/issues/2509): a player still sees them in the Water Temple. This is our Hookshot fix for them, opened so someone can test it against main's own fix (#107). It predates #107 and conflicts with it on purpose: it is on main 9eae480e, the commit before #107 merged, so the two can be compared. ## What changed - vk: `gpu/vk/eden/shader_recompiler/ir_opt/texture_pass.cpp`: `FindHandleTableIndex()` finds a light's index in the address of its 96-byte record (a plain multiply, or an XMAD pair's 16-bit halves in either order) and builds the shadow-map handle as a table (`HandleOp::Index`), so each light samples its own shadow map instead of the dummy; `ir_opt/passes.h`, `shader_info.h`: the index operation and the array. - vk: `gpu/vk/vk_shader.cpp`, `vk_shader.h`, `vk_draw.cpp`: the draw evaluates the handle once per element; `RX_VK_HANDLE_TABLE` and `RX_VK_HANDLE_TABLE_COUNT` in the program cache's stamp. - tests: `tests/test_handle_table.py` (new); `test_texture_handle_banks.py`, `test_translation_pools.py`, `test_program_answer_domains.py` follow the new operation. - docs: two rows in `docs/development.md`. ## Flags and settings | Flag | What it does | Default now | Recommendation | Helps on | Doesn't help on | Untested on | |---|---|---|---|---|---|---| | `RX_VK_HANDLE_TABLE=0` | The old search (one texture for every light) | on | On, if it tests better than #107 | Water, ice, snow lit by several lights (Hookshot, RTX 5060 Ti) | - | Longshot in game, AMD, Intel, Linux | | `RX_VK_HANDLE_TABLE_COUNT=<n>` | The table's size (2 to 256) | 64 | Leave at 64 | - | - | - | ## Measurements main: 9eae480e (2026-11-13), the commit before #107 this PR: test/water-handle-table-v3 @ 63ad82d2 Hardware: RTX 5060 Ti 8 GB, driver 32.0.16.1088; Intel Core Ultra 7 265F; 64 GB; Windows 11 Scene: Zora's Domain and Zora's River (child, day), Chamber of Sages; 1920x1080 Method: on Hookshot (main 8c359bca + these two commits), in game, screenshots with and without the fix; on this branch, the game-free checks and a renderer build only. How to reproduce: Zora's Domain or the Water Temple, standing by lit water; `RX_VK_HANDLE_TABLE=0` brings the squares back. | | main @ 9eae480e | this PR @ 63ad82d2 | |---|---|---| | Squares on the water (Zora's Domain, Hookshot) | yes | none | | `tests/test_handle_table.py` | fails (not there) | passes | Not measured on Longshot: no fps or VRAM numbers; asked for testing first. ## Testing **Tested:** on Hookshot in game: Zora's Domain (child and adult), Zora's River and the Chamber of Sages clean with the fix. On this branch: `tests/test_handle_table.py`, `test_texture_handle_banks.py`, `test_translation_pools.py` pass with llvm-mingw on Windows; the renderer builds (`RX_VK_ONLY`). **Not tested:** this branch in game on Longshot; the full `run_all.py`; against #107 side by side. **Needs other testers:** - A player who still sees the squares (Water Temple): build this PR, compare with the newest nightly (which has #107), expecting no squares here (T-X4WT) ## Screenshots Nothing from this branch yet; the Hookshot before/after is on the issue linked above. ## Risk and rollback Conflicts with main (#107 replaced the same code): this is for testing, not to merge as it is. If it fixes a case #107 misses, the difference would be ported onto #107 in a new PR. `RX_VK_HANDLE_TABLE=0` turns it off. ## Issues See https://github.com/NotRexed/Hookshot/issues/2509 ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [ ] Rebased on current `main`. (No, on purpose: based on 9eae480e, before #107, for comparison.) - [ ] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). (Only the touched checks run.) - [x] Tested in the game, or what wasn't is named under Testing. - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). - [ ] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. (Not measured: for testing.) - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### If applicable - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] Flags noted, and the switch reference updated (docs/development.md; not a menu row). - [x] A game-free test added for the fix, failing on `main` and passing here. - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

kingo added 2 commits

2026-11-14 18:58:23 +00:00

fix(vk): give each light its own shadow map in the water's light loop (a handle table read per pixel) 03eae3bfda

The water and wet floors light each pixel with its screen tile's lights (clustered
lighting). A projected light's shadow map handle is word 0 of its 96-byte record,
read per pixel. No handle program could follow it, and the fallback took the light
buffer's address, which binds the dummy: every light filled its tiles and stopped
at their edges, the squares on the water (https://github.com/NotRexed/Hookshot/issues/2509).

The texture pass now builds such a handle as a table: the element index becomes
HandleOp::Index, the texture an array the shader indexes by it, and the draw
evaluates the handle once per element. Both forms of the index multiply the
game's shaders use are followed (a plain multiply and an XMAD pair's halves).
Handles still not followed are logged once per translation.

RX_VK_HANDLE_TABLE=0 goes back to the old search; RX_VK_HANDLE_TABLE_COUNT sets
the array's size (64). Both are in the program cache's stamp.

For testing against main's own handle tables (#107), which this predates: it is on main
9eae480e, the commit before #107 merged. From Hookshot (YakuzaFanA/Hookshot, fix/light-texture-table).

Assisted-by: Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 <[email protected]>

fix(vk): an XMAD pair's index is found whichever half's multiply comes first

PR description / Description follows the template (pull_request_target) Successful in 17s

Details

Third-party parts / As their notes say (pull_request) Has been cancelled

Details

Third-party parts / The same as upstream (pull_request) Has been cancelled

Details

CI / Conventional Commit title (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled

Details

CI / Workflow lint (actionlint) (pull_request) Has been cancelled

Details

beanstalk/approval awaiting maintainer approval

Details

63ad82d2bb

The handle-table search returned on the first multiply by a stride it met in the
record's address. An XMAD pair reads the index in two 16-bit halves, and only the
low half's extract was unwrapped back to the index. Had the search met the high
half's multiply first, it took the extract as the index, the handle program could
not be built, and the shader fell back to the dummy. It worked only because of
the order the address's sums are walked in.

Both halves (BitFieldUExtract x, 0 or 16, 16) now give x, which the handle program
builds as HandleOp::Index in both. The shift form unwraps the same way. A plain
multiply is unchanged.

Checked: every shader translated at start-up; the shaders still on the fallback
are the same single program as before. Zora's Domain (child and adult): its light
shaders get their tables, no texture fails to bind, no blocks.

Assisted-by: Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 <[email protected]>

kingo commented

2026-11-14 20:59:11 +00:00

Author

First-time contributor

Closing this. A player who sees the squares at the Water Temple entrance ran this PR's build (log: longshot v3.0.0-rc.3-13-g63ad82d2, RTX 4070 SUPER) and still saw them, so this change does not fix their case. I've asked them to try the newest nightly (with #107) at the same spot; if it still shows there, an issue with their log and screenshot is the next step. T-X4WT is cancelled.

Closing this. A player who sees the squares at the Water Temple entrance ran this PR's build (log: `longshot v3.0.0-rc.3-13-g63ad82d2`, RTX 4070 SUPER) and still saw them, so this change does not fix their case. I've asked them to try the newest nightly (with #107) at the same spot; if it still shows there, an issue with their log and screenshot is the next step. T-X4WT is cancelled.

kingo closed this pull request

2026-11-14 20:59:14 +00:00