Skip to content

feat(shader): gather converted unorm, converted float and R8/R8G8 sRGB images - #4905

Closed
Dextroid17 wants to merge 1 commit into
boykopovar:mainfrom
Dextroid17:feat/gather-converted-formats
Closed

Dextroid17 wants to merge 1 commit into
boykopovar:mainfrom
Dextroid17:feat/gather-converted-formats

Conversation

@Dextroid17

@Dextroid17 Dextroid17 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

What

A non-compare image_gather4 of a 10_11_11 UNORM, 10_11_11 FLOAT, 8_SRGB or 8_8_SRGB image used to throw in the recompiler. A gather returns whole texels and needs no filtering, so this lets it through:

  • ResourceTracker.cpp: a gather no longer clears srgbDecodeCompatible.
  • docs/dev/debt/shader.md: updated for the new coverage and the caveat below.
  • SpirvImageEmitter.cpp: the converted unorm, converted float and sRGB refusals skip ImageGatherRaw. UnpackImageGather converts the fetched fields, including the constant-1 channel, and the sRGB decode runs on the gathered texels (also on the 1D gather path, which has no test).
  • Samples and LOD queries of these images still throw. Compare gathers (image_gather4_c_lz) are still refused, by ResourceMaterializer.cpp ("sampled image descriptor uses an unsupported format" for the 10_11_11 UNORM image in the test, "incompatible with the static runtime image interface" for the 8_8_SRGB one), not by the checks changed here.

Related to #3751 (not a fix).

Tested

Local, lavapipe (llvmpipe, LLVM 23.1.1), Linux, on main b1328fc0f (main has since gained only CMake and tools/ changes, no rebuild after that): agc_driver_image_converted_unorm_tests and agc_driver_image_srgb_load_tests pass. Title bring-up was not run for this PR.

ImageConvertedUnorm.cpp and ImageSrgbLoad.cpp replace the old "gather is refused" checks with value checks:

  • 10_11_11 UNORM and FLOAT: left texels of the 2x2 footprint 3, 4, 100, 1023, 2046 of a 2048-wide row, all four components with X Y Z 1, plus Z Y X 1 and (UNORM only) X 0 1 1. A two-row UNORM image (1024 wide, left texels 0, 5, 500, 1022, components 0 to 2) checks the lane order (x0,y1), (x1,y1), (x1,y0), (x0,y0).
  • 8_SRGB and 8_8_SRGB: texel positions 0, 1, 100, 254, 255 of a 256-wide row.
    • 8_8_SRGB: components 0 to 3 (including the constant 0 and 1 channels) with X Y 0 1, plus component 0 with Y X 1 0.
    • 8_SRGB: component 0 with X X X 1.
  • image_gather4_c_lz is still refused on a 10_11_11 UNORM image and on an 8_8_SRGB image. The old compare-then-gather refusal check in ImageSrgbLoad.cpp was removed.

Only these two test binaries were run for this PR on lavapipe; the other agc_driver_image_*_tests were not re-run.

Windows (RTX 4080 SUPER, MinGW GCC 15.2): reported by xXPBSXx in this thread: built on main 88107b7de with thirteen other open GPU PRs cherry-picked, both tests passing 3 of 3 runs.

Not covered:

  • At the last texel of a row of the 2048-wide UNORM/FLOAT image, the lanes past the right edge read 0 instead of repeating the last texel, with a clamp-to-edge sampler. The 256-wide sRGB image does not show it (its test includes the last texel, 255). I did not find the cause; no other device was tried. The tests stop at texel 2046 and the debt doc says so.
  • Gathers with offsets or LOD variants, and emitted SPIR-V values on any other device.

Checklist

@MarcZX10

Copy link
Copy Markdown
Contributor

I checked the failing conventions run 38341276634. It reports five findings: the PR title and commit subject need Conventional Commit prefixes, and the PR body is missing the template sections What, Tested, and Checklist. Updating those should address the current conventions failure; the related build checks are skipped rather than failed.

@xXPBSXx

xXPBSXx commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Windows test (MinGW GCC 15.2, Release, Windows 10, RTX 4080 SUPER). Built on main 88107b7de together with thirteen other open GPU PRs (their own commits cherry-picked, no merge commits), so only the tests that belong to this PR say something about it:

  • tests/execution/ImageConvertedUnorm.cpp (agc_driver_image_converted_unorm): 3 of 3 runs passed.
  • tests/execution/ImageSrgbLoad.cpp (agc_driver_image_srgb_load): 3 of 3 runs passed.

Not tested: no game run. The batch was built once, not per PR; nothing failed, so no bisect was needed.

AI-assisted (Claude Code).

…B images

image_gather4 returns whole texels, so it needs no filtering: the converted
unorm/float and R8/R8G8 sRGB refusals now let a non-compare gather through.
UnpackImageGather converts the fetched fields the way UnpackImageTexel does
for loads, and the sRGB decode runs on the gathered texels. Samples and LOD
queries still throw. Compare gathers are not exercised by these tests.
Tests run on lavapipe (llvmpipe, LLVM 23.1.1) only.
@Dextroid17 Dextroid17 changed the title shader: gather converted unorm, converted float and R8/R8G8 sRGB images feat(shader): gather converted unorm, converted float and R8/R8G8 sRGB images Oct 13, 2026
@Dextroid17
Dextroid17 force-pushed the feat/gather-converted-formats branch from 11fd3c1 to 6e7bbdd Compare October 13, 2026 11:05
@Dextroid17

Copy link
Copy Markdown
Contributor Author

@MarcZX100 thanks for checking the failed run. I retitled the PR and the commit subject with a Conventional Commit prefix and rewrote the body to What / Tested / Checklist. The conventions check now passes on 6e7bbdd. Nothing failed in the other jobs: linux and windows are skipped because full builds are disabled for pull-request runs, and relinker, register-context and write-watch are skipped because this PR touches none of the files that trigger them.

@xXPBSXx thank you for the Windows run. I added it to the Tested section, with your base (main 88107b7 plus thirteen other open GPU PRs cherry-picked) and the 3 of 3 results for agc_driver_image_converted_unorm and agc_driver_image_srgb_load.

Still open on my side, as the body says: the two tests were last run locally on lavapipe on main b1328fc, not again after the rebase onto c8eb1f5, which only changed CMake and tools/ files. Samples and LOD queries of these images still throw.

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
#757 core/shader/recompiler/SpirvBackend/src/SpirvImageEmitter.cpp L1191-1192, L1230-1265
#758 core/shader/recompiler/SpirvBackend/src/SpirvImageEmitter.cpp L1191-1192, L1230-1265
#5371 core/libs/prx/libSceAgcDriver/tests/execution/ImageSrgbLoad.cpp L80-83, L245-248
core/shader/recompiler/SpirvBackend/src/SpirvImageEmitter.cpp L1570-1571
docs/dev/debt/shader.md L43

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Closing: no title is known to reach this. With the queue at 700+ open PRs, please hold off on new PRs that don't name a title needing the change until it's down. If a title stops on this on current main, reopen with the title ID and the log line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* docs shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants