fix(vk): AMD/Intel shader translation fixes (out-of-bounds fetches, gathers, NaN tests) #2457

Merged

hckrman101 merged 5 commits from fix/vendor-translation into main

2026-11-11 22:55:09 +00:00

hckrman101 commented

2026-11-11 19:50:26 +00:00

(Migrated from github.com)

What changed

Which fixes a GPU gets is decided once, from its Vulkan properties, in gpu/vk/vk_vendor.h. The log prints the result as one vk: vendor fixes line. Each fix has an RX_VK_ switch that forces it on or off on any GPU (listed in docs/development.md).

Fix	GPUs	Switch
Out-of-bounds texel fetches from images return zero, as NVIDIA's GPUs return them (`EmitImageFetch`). The fetch itself reads in-range coordinates, level and sample, so it never leaves the image. Texel buffers are checked only when robustBufferAccess2 is off: Robust buffers set to Off, `RX_VK_ROBUST=0`, or a driver without it. When it is on (the default), it already returns zero for their out-of-bounds reads. Whether they are checked is part of the program cache's stamp.	Every GPU except NVIDIA's	`RX_VK_FETCH_BOUNDS=0/1`
Eden's 1/512-texel gather offset is now enabled, for 2D, rect and 2D array textures. It was already in the translator but Hookshot never turned it on. Eden and Lichii enable it for these drivers. Cubes are left as they are: Eden's cube case emitted invalid SPIR-V (a three-component size query on a cube), and a nudge means nothing for a direction.	AMD's and Intel's drivers (proprietary, RADV, AMDVLK, ANV)	`RX_VK_GATHER_OFFSET=0/1`
NaN tests check the value's bits instead of using `OpIsNan`. This covers `FPIsNan` 16/32/64, the colour outputs' NaN guard and the f32→f16 conversion.	AMD's driver whose driverInfo names the "AMD proprietary shader compiler": the Windows branch for RDNA 1/2 (RX 5000/6000). RDNA 3/4 (LLPC) is unchanged. AMDGPU-PRO on Linux would get it too if it reports the same compiler, which is the case the fix is for.	`RX_VK_NAN_BITS=0/1`

Re-prepare. The three flags are in the program cache's stamp. Like any translator change, every player prepares the shaders again once, NVIDIA players included, even though their shaders come out the same.
One known difference from NVIDIA. Out of bounds, NVIDIA returns alpha 1 for formats without an alpha channel (0 for formats with one), and this fix always returns alpha 0. The translator doesn't know the texture's format. The fog shader reads only .x, and a shader would have to read the alpha of an out-of-bounds fetch from a one-to-three-channel texture to see the difference.
Why shader checks rather than robustImageAccess2. robustImageAccess2 (off in Hookshot since its first release) would match NVIDIA exactly, including alpha 1 for formats without alpha, and needs no re-translation. But it is device-wide: it bounds-checks every image load, store and fetch in every shader on the device. Its cost is unmeasured on the AMD and Intel cards that would get it, and none of us can measure it here. The shader check is limited to the 520 programs that fetch from images. The trade-off is the alpha difference above. If the lab or players find that the alpha matters, or that the shader check costs more than expected, robustImageAccess2 for non-NVIDIA devices is a small follow-up.
Overlap with the Intel Windows runtime PR. Both PRs add gpu/vk/vk_vendor.h, the vk: vendor fixes block in vk_main.cpp, its switches row in docs/development.md and tests/test_vendor_fixes.py. Each carries only its own fixes, so either can merge first. Whichever merges second gets a trivial conflict in those four files. The resolution: keep both sets of VendorFixes fields, their vendor_fixes() lines and RX_VK_ overrides; merge the two logf calls into one vk: vendor fixes line with all five values; join the two docs rows; and in the test, concatenate the FIELDS lists, each device's digit strings (Intel's two first) and the switch names, combine the two check_source bodies, and keep #2456's static_a2c inversion in the expected-value line (RX_VK_DYN_A2C=1 turns that fix off). This PR's copy of the test dropped that branch because it has no such field.
Not included. AMD storage-buffer alignment and ASTC bounds are in the separate Fritchii port. DCC off on old RADV (Lichii #950) doesn't apply: Hookshot keeps game images in GENERAL, so it doesn't pay the decompress cost that change removes.

Credit

The cause of the fog box. An out-of-bounds texelFetch of depth in the ground-fog shader, found by the author of Ocarina of Lain on an RX 9070. This is the idea only, reimplemented here in the translator; no code was copied.
The AMD shader-compiler test on driverInfo. From totk-pc-recomp's runtime/vk_caps.c (Valiantsin Dzerakh, MIT).
The gather offset. Eden's, and Lichii's choice of drivers.

Verified on NVIDIA (RTX 3090, Linux, driver 620.70.06)

Gates, on made-up devices. The new tests/test_vendor_fixes.py gives 16 sets of device properties to the real vendor_fixes(): NVIDIA on Windows and Linux, NVK, AMD's Windows driver in both branches and with no driverInfo, RADV on a desktop card and a Steam Deck, AMDVLK, Intel Windows on integrated and Arc, ANV, and llvmpipe.
- Each fix turns on only for the GPUs it is meant for. NVIDIA's driver gets none. One row is an AMD driver that names another shader compiler: it does not get the NaN fix.
- Every switch forces its fix both ways.
- The test also checks that the translator is told the flags and that the stamp includes them.
Translation, game-free. The new tests/test_vendor_translation.py builds a Sirit compute shader with the real FetchInBounds and IsNan. It fetches from every texture type a fetch can read (texel buffer, 1D, 1D array, 2D, 2D rect, 2D array, 3D, 2D multisampled) and tests f16/f32/f64 NaNs. It gathers too, as above.
- With the fixes off, it emits exactly the old instructions. The off path of EmitImageFetch is checked to be the old code, word for word.
- With the fixes on, every image fetch is checked against its size, level and sample, and no OpIsNan is left. The texel buffer's fetch is checked only in a third module made as if robustBufferAccess2 were off.
- It also gathers from 2D, rect, 2D array and cube textures: the offset is applied to the first three, and the cube is unchanged. The old cube case fails spirv-val here.
- Both modules pass spirv-val.
What NVIDIA actually returns. I ran the same two modules on the 3090 with robustness off.
- Out-of-bounds fetches return zero on every texture type: past each edge, past the last level and past the last sample.
- The bounds-checked shader gave the same answer as NVIDIA's own hardware in all 96 cases. That run was before the review, when texel buffers were still checked; NVIDIA returned zero for them too.
- The bit-test NaN check matched OpIsNan for quiet, signalling and negative NaNs, ±infinity, denormals, FLT_MAX, ±0 and a value that overflows f16.
The game's own shaders, NVIDIA unchanged. I prepared every shader of the game with main and with this branch, then compared the SPIR-V of all 53,412 translated program variants (46,676 programs). They are byte-identical, so NVIDIA's rendering cannot change.
The game's shaders with the fetch fix forced on (RX_VK_FETCH_BOUNDS=1). It changes 520 of the 46,676 programs (2,533 of 53,412 variants). All 2,518 changed SPIR-V modules pass spirv-val, with the device's uniform-buffer standard layout and scalar block layout.
The whole game's shaders with all three fixes forced on (RX_VK_FETCH_BOUNDS=1 RX_VK_GATHER_OFFSET=1 RX_VK_NAN_BITS=1, this branch after the review fixes). All 38,809 programs translate, the same counts as without the fixes. Every unique SPIR-V module was run through spirv-val with the device's layout rules: 50,808 modules, 2 invalid. Those two (programs 743906830fa2e5d1 and 3aa6798b83cf18a2, a dominance error in the control flow) are just as invalid in main's own translation without the fixes, so they come from elsewhere in the translator. None of the three fixes adds blocks.
One headless run with the fetch fix forced on NVIDIA, in Kokiri Forest from a save. It drew 2,434 draws a frame with 0 skipped. The picture was normal: the ground haze is intact and there is no box.
Test suite. tests/run_all.py passes, apart from checks that fail on this PC without the change too (no zstandard or tkinter here).

Not tested: please help if you have an AMD or Intel GPU

I have no AMD or Intel GPU, so none of the fixes has been seen working on the hardware it is for. The NaN and gather fixes were not run with the full game here; only the fixture tests above cover them. Reports with hookshot_log.txt are very welcome, especially from:

Any AMD card (RX 9070 reported it). Is the pale stepped box at the left, right and top edges of the ground fog gone? Kokiri Forest's haze shows it. Compare with RX_VK_FETCH_BOUNDS=0.
AMD and Intel. Shadow and wall edges, compared with RX_VK_GATHER_OFFSET=0.
AMD RX 5000 / RX 6000 on Windows. Are the black or white blocks gone? Compare with RX_VK_NAN_BITS=0.

Also not measured: the GPU cost of the bounds checks on AMD and Intel. The checks are a few instructions per texel fetch, in 520 of the game's programs.

On AMD, the ground fog draws a pale stepped box at the left, right and top edges of the screen. AMD's older Windows compiler can let NaN tests fail, which shows as black and white blocks. And gathers on AMD and Intel can pick different texels from the console's at texel edges. This PR changes the shader translation to work around these, only on the drivers that need it. NVIDIA's driver gets none of it: the game's translated shaders on NVIDIA are byte for byte what they were. ## What changed Which fixes a GPU gets is decided once, from its Vulkan properties, in `gpu/vk/vk_vendor.h`. The log prints the result as one `vk: vendor fixes` line. Each fix has an `RX_VK_` switch that forces it on or off on any GPU (listed in docs/development.md). | Fix | GPUs | Switch | |---|---|---| | Out-of-bounds texel fetches from images return zero, as NVIDIA's GPUs return them (`EmitImageFetch`). The fetch itself reads in-range coordinates, level and sample, so it never leaves the image. Texel buffers are checked only when robustBufferAccess2 is off: Robust buffers set to Off, `RX_VK_ROBUST=0`, or a driver without it. When it is on (the default), it already returns zero for their out-of-bounds reads. Whether they are checked is part of the program cache's stamp. | Every GPU except NVIDIA's | `RX_VK_FETCH_BOUNDS=0/1` | | Eden's 1/512-texel gather offset is now enabled, for 2D, rect and 2D array textures. It was already in the translator but Hookshot never turned it on. Eden and Lichii enable it for these drivers. Cubes are left as they are: Eden's cube case emitted invalid SPIR-V (a three-component size query on a cube), and a nudge means nothing for a direction. | AMD's and Intel's drivers (proprietary, RADV, AMDVLK, ANV) | `RX_VK_GATHER_OFFSET=0/1` | | NaN tests check the value's bits instead of using `OpIsNan`. This covers `FPIsNan` 16/32/64, the colour outputs' NaN guard and the f32→f16 conversion. | AMD's driver whose driverInfo names the "AMD proprietary shader compiler": the Windows branch for RDNA 1/2 (RX 5000/6000). RDNA 3/4 (LLPC) is unchanged. AMDGPU-PRO on Linux would get it too if it reports the same compiler, which is the case the fix is for. | `RX_VK_NAN_BITS=0/1` | - **Re-prepare.** The three flags are in the program cache's stamp. Like any translator change, every player prepares the shaders again once, NVIDIA players included, even though their shaders come out the same. - **One known difference from NVIDIA.** Out of bounds, NVIDIA returns alpha 1 for formats without an alpha channel (0 for formats with one), and this fix always returns alpha 0. The translator doesn't know the texture's format. The fog shader reads only `.x`, and a shader would have to read the alpha of an out-of-bounds fetch from a one-to-three-channel texture to see the difference. - **Why shader checks rather than `robustImageAccess2`.** robustImageAccess2 (off in Hookshot since its first release) would match NVIDIA exactly, including alpha 1 for formats without alpha, and needs no re-translation. But it is device-wide: it bounds-checks every image load, store and fetch in every shader on the device. Its cost is unmeasured on the AMD and Intel cards that would get it, and none of us can measure it here. The shader check is limited to the 520 programs that fetch from images. The trade-off is the alpha difference above. If the lab or players find that the alpha matters, or that the shader check costs more than expected, robustImageAccess2 for non-NVIDIA devices is a small follow-up. - **Overlap with the Intel Windows runtime PR.** Both PRs add `gpu/vk/vk_vendor.h`, the `vk: vendor fixes` block in vk_main.cpp, its switches row in docs/development.md and `tests/test_vendor_fixes.py`. Each carries only its own fixes, so either can merge first. Whichever merges second gets a trivial conflict in those four files. The resolution: keep both sets of `VendorFixes` fields, their `vendor_fixes()` lines and `RX_VK_` overrides; merge the two `logf` calls into one `vk: vendor fixes` line with all five values; join the two docs rows; and in the test, concatenate the FIELDS lists, each device's digit strings (Intel's two first) and the switch names, combine the two `check_source` bodies, and keep #2456's `static_a2c` inversion in the expected-value line (`RX_VK_DYN_A2C=1` turns that fix off). This PR's copy of the test dropped that branch because it has no such field. - **Not included.** AMD storage-buffer alignment and ASTC bounds are in the separate Fritchii port. DCC off on old RADV (Lichii #950) doesn't apply: Hookshot keeps game images in `GENERAL`, so it doesn't pay the decompress cost that change removes. ## Credit - **The cause of the fog box.** An out-of-bounds `texelFetch` of depth in the ground-fog shader, found by the author of [Ocarina of Lain](https://github.com/jabroni1kanobi/Ocarina-of-Lain) on an RX 9070. This is the idea only, reimplemented here in the translator; no code was copied. - **The AMD shader-compiler test on driverInfo.** From totk-pc-recomp's `runtime/vk_caps.c` (Valiantsin Dzerakh, MIT). - **The gather offset.** Eden's, and Lichii's choice of drivers. ## Verified on NVIDIA (RTX 3090, Linux, driver 620.70.06) - **Gates, on made-up devices.** The new `tests/test_vendor_fixes.py` gives 16 sets of device properties to the real `vendor_fixes()`: NVIDIA on Windows and Linux, NVK, AMD's Windows driver in both branches and with no driverInfo, RADV on a desktop card and a Steam Deck, AMDVLK, Intel Windows on integrated and Arc, ANV, and llvmpipe. - Each fix turns on only for the GPUs it is meant for. NVIDIA's driver gets none. One row is an AMD driver that names another shader compiler: it does not get the NaN fix. - Every switch forces its fix both ways. - The test also checks that the translator is told the flags and that the stamp includes them. - **Translation, game-free.** The new `tests/test_vendor_translation.py` builds a Sirit compute shader with the real `FetchInBounds` and `IsNan`. It fetches from every texture type a fetch can read (texel buffer, 1D, 1D array, 2D, 2D rect, 2D array, 3D, 2D multisampled) and tests f16/f32/f64 NaNs. It gathers too, as above. - With the fixes off, it emits exactly the old instructions. The off path of `EmitImageFetch` is checked to be the old code, word for word. - With the fixes on, every image fetch is checked against its size, level and sample, and no `OpIsNan` is left. The texel buffer's fetch is checked only in a third module made as if robustBufferAccess2 were off. - It also gathers from 2D, rect, 2D array and cube textures: the offset is applied to the first three, and the cube is unchanged. The old cube case fails spirv-val here. - Both modules pass spirv-val. - **What NVIDIA actually returns.** I ran the same two modules on the 3090 with robustness off. - Out-of-bounds fetches return zero on every texture type: past each edge, past the last level and past the last sample. - The bounds-checked shader gave the same answer as NVIDIA's own hardware in all 96 cases. That run was before the review, when texel buffers were still checked; NVIDIA returned zero for them too. - The bit-test NaN check matched `OpIsNan` for quiet, signalling and negative NaNs, ±infinity, denormals, FLT_MAX, ±0 and a value that overflows f16. - **The game's own shaders, NVIDIA unchanged.** I prepared every shader of the game with main and with this branch, then compared the SPIR-V of all 53,412 translated program variants (46,676 programs). They are byte-identical, so NVIDIA's rendering cannot change. - **The game's shaders with the fetch fix forced on** (`RX_VK_FETCH_BOUNDS=1`). It changes 520 of the 46,676 programs (2,533 of 53,412 variants). All 2,518 changed SPIR-V modules pass spirv-val, with the device's uniform-buffer standard layout and scalar block layout. - **The whole game's shaders with all three fixes forced on** (`RX_VK_FETCH_BOUNDS=1 RX_VK_GATHER_OFFSET=1 RX_VK_NAN_BITS=1`, this branch after the review fixes). All 38,809 programs translate, the same counts as without the fixes. Every unique SPIR-V module was run through spirv-val with the device's layout rules: 50,808 modules, 2 invalid. Those two (programs `743906830fa2e5d1` and `3aa6798b83cf18a2`, a dominance error in the control flow) are just as invalid in main's own translation without the fixes, so they come from elsewhere in the translator. None of the three fixes adds blocks. - **One headless run with the fetch fix forced on NVIDIA,** in Kokiri Forest from a save. It drew 2,434 draws a frame with 0 skipped. The picture was normal: the ground haze is intact and there is no box. - **Test suite.** `tests/run_all.py` passes, apart from checks that fail on this PC without the change too (no `zstandard` or `tkinter` here). ## Not tested: please help if you have an AMD or Intel GPU I have no AMD or Intel GPU, so none of the fixes has been seen working on the hardware it is for. The NaN and gather fixes were not run with the full game here; only the fixture tests above cover them. Reports with `hookshot_log.txt` are very welcome, especially from: - **Any AMD card (RX 9070 reported it).** Is the pale stepped box at the left, right and top edges of the ground fog gone? Kokiri Forest's haze shows it. Compare with `RX_VK_FETCH_BOUNDS=0`. - **AMD and Intel.** Shadow and wall edges, compared with `RX_VK_GATHER_OFFSET=0`. - **AMD RX 5000 / RX 6000 on Windows.** Are the black or white blocks gone? Compare with `RX_VK_NAN_BITS=0`. Also not measured: the GPU cost of the bounds checks on AMD and Intel. The checks are a few instructions per texel fetch, in 520 of the game's programs.