Repository navigation
fix(agc): bind UINT views for float16 stores outside compute shaders - #15741
Merged
oneandonlydean merged 3 commits intoNov 4, 2026
Merged
Conversation
The UINT storage view for float16 image stores was chosen only when the global binding number fell in the uint storage image slots of the main stage. Binding numbers are stage * Binding::Count + slot, so a pixel, hull or domain shader's slots never matched: its uint writes went to a float16 view and the store was lost on AMD's Windows driver. The slot is now taken from the binding number within its stage. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VAPyfZR5GPZaWxRz67e9FT
…r the shader cache key It only picks the host view for a binding number and does not change what a recompile produces. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VAPyfZR5GPZaWxRz67e9FT
A full-screen draw stores each pixel's position and two constants to a 16_FLOAT, 16_16_FLOAT and 16_16_16_16_FLOAT image from the pixel shader, in wave64 and wave32, and checks every texel. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VAPyfZR5GPZaWxRz67e9FT
Contributor
✅ NVIDIA GPU test: passedPR head
Failing on main too (3, not counted against this PR)Validated but not compared (1, not in the main baseline's validation pass)Validation IDs already present on main (13 IDs in 545 tests)Skipped (49)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
Contributor
✅ AMD GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (8, not counted against this PR)Validated but not compared (3, not in the main baseline's validation pass)Validation IDs already present on main (8 IDs in 545 tests)Skipped (60)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
12 tasks done
|
Windows and NVIDIA Blackwell run, for head
The failure, The new The retail title's depth-of-field pass was not run. AI-assisted: yes. Claude Code ran the builds and tests and drafted this comment; the numbers are from the runs on this machine. |
Contributor
|
Cross-test on a title: Demon's Souls PPSA01341 v01.007.000, Windows 11 Pro, RTX 5080 (driver 32.0.16.1742), WinLibs GCC 15.2.0 Release, On Binding 103 is a pixel-stage uint storage slot, which is this PR's case. With this PR the
|
12 tasks done
Superseabas
approved these changes
Nov 4, 2026
Superseabas
left a comment
Superseabas
left a comment
There was a problem hiding this comment.
Head b70a3fc6fc merged onto current main b11c7bf84f (clean merge, local only): code read, red/green check and a run on two Windows GPUs, NVIDIA and AMD's Windows driver.
Windows 11 Home 10.0.26200, WinLibs GCC 16.2.0 (x86_64-ucrt-posix-seh, r3), Release, BUILD_TESTING=ON. ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, ctest -R '^(agc_driver_graphics|agc_driver_.*uint.*|agc_driver_pixel_.*|agc_driver_.*float16.*|agc_driver_.*storage_image.*|agc_driver_.*export.*)$', ANYPS5_GPU selecting the device (confirmed by Selected GPU):
| GPU | Driver | main (44 tests) |
merged (45, with agc_driver_pixel_float16_store) |
|---|---|---|---|
NVIDIA GeForce RTX 5070 Ti (10de:2c05) |
621.01 | 43 + 1 skip | 44 + 1 skip |
AMD Radeon Graphics, Ryzen 7 7700X iGPU (1002:164e) |
AMD 26.10.2 | 43 + 1 skip | 44 + 1 skip |
- The skips are the same on both sides. The only
VUID-in either tree isVUID-VkPipelineColorBlendStateCreateInfo-pAttachments-00605(#11990), which is onmaintoo. No new ID appears, and there is no missing-layer line. The new test printspixel float16 store tests passedon both GPUs. - Red check: the merged tree with only this PR's
ShaderResources.cppchange reversed failsagc_driver_pixel_float16_storeon both GPUs withpixel shader wave64 image_store of 16_FLOAT: texel (0, 0) component 0 is 0, expected 14336. The store writes nothing on NVIDIA's Windows driver too, not only on AMD's. check_conventions.py --base mainpasses on the merge.
Read through the change:
UintStorageImageBindingtakes the binding moduloBinding::Count(65) and checks for slots 36..42 (FirstStorageImageBinding + 7 .. + 13) within that stage. It returns false at or aboveStageCount * 65= 260, so the image table bindings 260 and 261 stay out, andShaderDeviceProfilechecks exactly that.- For the main stage (bindings below 65) the result is the same as the old range check.
- The three call sites, storage image table entries, compact storage bindings and multisample storage views, now use it.
- Only the host view changes, which fits the shader-cache review listing.
I found nothing to change.
AI-assisted: yes (Claude Code)
12 tasks done
12 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
027c6f0 and 4a25a65 bind a UINT storage view for write-only float16 images, which the recompiler declares as uint. The driver picks that view only when the binding number is in
FirstStorageImageBinding + 7 .. + 13(36..42). Binding numbers arestage * Binding::Count + slot(RuntimeAbi::BindingNumber), so this matches only the main stage. In a pixel, hull or domain shader the same slots are 101..107, 166..172 and 231..237: the shader writes uint texels through a float16 view (undefined behaviour), and on AMD's Windows driver the store writes nothing.RuntimeAbi::UintStorageImageBindingtakes the slot from the binding number within its stage. The three call sites inShaderResources.cpp(storage images, the compact binding path and multisample storage views) use it. The image table bindings (260, 261) are never uint storage slots. The function only picks the host view, so it is listed inshaderCacheReviewedNameswithout a cache key change.Overlap: #7642, #9508, #10193, #13928 and #13929 make the same correction at the same three call sites (a
UintStorageBindinghelper taking the binding moduloBinding::Count) inside larger image table and descriptor heap work; none is merged and none adds a test for this case. This PR is the fix alone with a GPU test that fails onmain, so it can go first and those would useRuntimeAbi::UintStorageImageBindinginstead of their own helper. #10743 and #13796 change the same lines without this fix and will need a small rebase.Tested
agc_driver_pixel_float16_store: a full-screen draw stores each pixel's position and two constants to 16_FLOAT, 16_16_FLOAT and 16_16_16_16_FLOAT images from the pixel shader (wave64 and wave32) and checks every texel. Onmainit fails (texel (0, 0) component 0 is 0, expected 14336) and the validation layer reportsVUID-vkCmdDraw-format-07753(uint image type through a float16 view); with this change it passes with noVUID-errors.agc_shader_device_profilenow checks every stage × binding against the stage-local uint storage slots, plus the two image table bindings.agc_shader_device_profile,image_store_*,uint*,multisample*,*storage*) onmain(5b1d381) and on this branch, same machine,ctest -V -j 1withVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation,ANYPS5_NO_SHADER_CACHE=1,ANYPS5_REQUIRE_VULKAN=1: 81/81 pass on both. Neither log reportswas not found but was requested by env var VK_INSTANCE_LAYERS. 0VUID-errors on both; the only validation output is the sameWARNING-VkImageSubresourceRange-layerCount-compatibilityon both (tests 1311, 1565, 2022).agc_frame_replaywrites 518400/518400 texels, and the artefact is gone in live play. RX 6750 XT, Windows 11.python3 tools/check_conventions.pypasses.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)Test image of agc_driver_pixel_float16_store (16_16_16_16_FLOAT, wave64; R = x/64, G = y/32, B = 1.0; 8x), dumped by a local uncommitted change to the test.
Main: store lost, all black.
This PR: every texel written.