Repository navigation
feat(agc): sample 8 and 8_8 sRGB textures through an RGBA8 sRGB copy - #6054
Conversation
|
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.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
Status check on the current head 9130522, merged into main 6239cef (66 commits ahead of this PR's base): no conflicts, and the merge builds. Run on lavapipe (llvmpipe, Mesa 26.2.4, LLVM 23.1.1; CachyOS, Linux 7.2.9; default CMake build with CMAKE_BUILD_TYPE empty, so unoptimized), 3 runs each, all exit 0:
By reading, not run: #4487 (already in this PR's base) gives R8/R8G8 no sRGB twin when The logs show lavapipe here decodes 8_SRGB/8_8_SRGB in the shader (textures created as R8_UNORM/R8G8_UNORM), and the storage case logs "sRGB copy of storage image", so the copy path ran. Not checked: a run with the test's source reverted, any other GPU or OS, and any title. AI-assisted: yes |
|
Tested on Windows hardware. There's one build break on Windows. Otherwise the PR passes on both GPUs here, but neither GPU takes the new copy path. Setup: Windows 11 Pro 10.0.26200, WinLibs GCC 15.2.0 r7, Release,
1.
|
| RTX 4090 | Radeon 610M | |
|---|---|---|
agc_driver_image_srgb_sample_copy |
pass 5/5 | pass 5/5 |
agc_driver_image_srgb_load, agc_driver_srgb8_color_target, agc_driver_emulated_color_compare_srgb_decode |
pass 5/5 | pass 5/5 |
new test with APS5_SRGB_SHADER_DECODE=1 |
exit 1 (samples or gathers an sRGB image the device cannot sample, which is not implemented) |
same |
All runs used ANYPS5_NO_SHADER_CACHE=1.
Full suite on the 4090: 1,272 of 1,274 passed. The same 15 skips as unmodified main on this machine (display, Linux-only and extension skips). The two failures don't look related to this PR:
check_main_sync: thePYTHONIOENCODINGcase from fix(tools): preserve main sync reports on limited encodings #5818agc_shader_disk_cache: timed out at 120 s under--parallel 4, and passed alone in 2.25 s
3. Coverage caveat
Both devices report SAMPLED_IMAGE and SAMPLED_IMAGE_FILTER_LINEAR with optimal tiling for R8_SRGB and R8G8_SRGB. So SrgbDecodeFormats returns 0 and SrgbCopyFormats returns 0 on both. The runs above cover:
- This PR on the native-sampling path: no regression.
- The new test's expected values against hardware sRGB filtering: they match, including the 0.5 bilinear midpoint.
They don't cover the RGBA8 copy itself on hardware. As far as I can tell, there's no switch that forces the copy path on a device that samples these formats natively. APS5_SRGB_SHADER_DECODE=1 only forces the old shader decode. If you want hardware coverage of the copy path, a switch that forces it would let any GPU run it.
Tested with Claude Code.
|
@Jacmd thanks for testing on Windows, and for the fix. Both points are now on the branch:
Results on lavapipe (llvmpipe, Mesa 26.2.4, LLVM 23.1.1). The tests ran on
On your RTX 4090 or Radeon 610M, AI-assisted: yes |
jotitadev
left a comment
jotitadev
left a comment
There was a problem hiding this comment.
Hardware run of the copy path, for the request above. On this NVIDIA device the copy is taken even without the switch, for 8_8_SRGB.
Ubuntu 26.04 (kernel 7.0.0-38), NVIDIA GeForce RTX 3070, driver 595.99.02, GCC 15.2, Release, BUILD_TESTING=ON, ANYPS5_GPU set to the 3070 (the log shows Selected GPU name=NVIDIA GeForce RTX 3070 vendor=0x10de). Head b29b0476f merged onto main 44df21632, no conflicts.
vulkaninfo --show-formatson this driver listsR8_SRGBwithSAMPLED_IMAGEandSAMPLED_IMAGE_FILTER_LINEAR, and listsR8G8_SRGBunder "Unsupported Formats". This device is the mixed case: native sampling for 8_SRGB, none for 8_8_SRGB.agc_driver_image_srgb_sample_copyandagc_driver_image_srgb_sample_copy_forced: pass, 3 of 3 runs each.- In the verbose log, the unforced entry creates the 8_SRGB texture as
vk=15(R8_SRGB) and the 8_8_SRGB textures asvk=16(R8G8_UNORM). WithAPS5_SRGB_SHADER_DECODE=copy, the 8_SRGB texture becomesvk=9(R8_UNORM), and the storage case logssRGB copy of storage image (guest format 128). Going bySrgbCopyFormats, 8_8_SRGB takes the RGBA8 copy here in both entries, becauseR8G8_SRGBcannot be sampled andR8G8_UNORMcan be blitted. - With only the test applied on
main, the test does not compile (SrgbCopyFormatsis not a member ofAgcDriver::Graphics), as expected for a new function. ctest -R "^agc" -E "agc_display_buffer_dcc|agc_driver_flip|agc_driver_lds_atomics64|display" -j1: 594 of 595 pass, 3 skipped. The failure isagc_driver_store_over_untracked, which also fails on unmodifiedmainon this machine: 9 of 10 runs with other builds running, all withthe result did not reach a block the CPU left alone. It is the same test as #6347 and #6383, which report it on Windows. On an idle machine,mainpasses 593 of 593 with the same 3 skips.
Not run: any title.
GPUs tested: NVIDIA GeForce RTX 3070, driver 595.99.02, Vulkan 1.4.329, Ubuntu 26.04 (kernel 7.0.0-38)
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
b29b047 to
4a6f4df
Compare
|
@jotitadev, thanks for the 3070 run. On the 3070, 8_SRGB samples natively and 8_8_SRGB takes the RGBA8 copy without the switch (going by The branch is now rebased onto Re-run on Linux, lavapipe (llvmpipe, Mesa 26.2.4, LLVM 23.1.1), Release build, clean environment ( AI-assisted: yes |
|
Checked on lavapipe: |
4a6f4df to
4339f73
Compare
A device that cannot sample R8_SRGB or R8G8_SRGB had its sRGB textures decoded in the shader, which cannot serve filtered samples: the decode must come before the filter. The texture is now an RGBA8 UNORM image viewed as RGBA8 sRGB, filled by a one-to-one blit of the R8 or RG8 texels (a snapshot's upload, or a storage image's texels refreshed when its Version changes), so the sampler decodes and then filters, and every sample, gather and load of these images is an ordinary sRGB one. The recompiler is given the decode mask only for formats the device cannot blit, or when APS5_SRGB_SHADER_DECODE=1 forces the shader decode. Tests run on lavapipe only.
Co-authored-by: Jacmd <59657462+Jacmd@users.noreply.github.com>
… sRGB through the copy
4339f73 to
c81cfce
Compare
|
Fresh lavapipe run of the current head, after today's push ( Setup: head sRGB tests,
The switch decides the path. Same binary,
So the forced test only passes through the copy, which matches the lavapipe result above from @adriadam10. Full suite: 1447/1448 pass. The one failure is |
4 participants
What
On a Vulkan device that cannot sample R8_SRGB or R8G8_SRGB, 8_SRGB and 8_8_SRGB textures were decoded in the shader. A shader decode runs after the sampler has filtered, so bilinear and gather results were wrong: the sRGB decode has to come before the filter. This samples those textures through an RGBA8 UNORM image viewed as RGBA8 sRGB, filled by a one-to-one blit of the R8 or RG8 texels (a snapshot's upload, or a storage image's texels, refreshed when its Version changes). The sampler then decodes and filters in the right order.
APS5_SRGB_SHADER_DECODE=1forces the old path.CanSrgbExpandFrom).SampledTextureMemory.Relation to #5371: it decodes in the shader for direct single-mip 2D images. This PR decodes before filtering through the device for the same formats. They can coexist: the shader path stays for devices that cannot blit and under the flag. Happy to rebase or drop whichever the maintainers prefer.
Docs:
docs/dev/debt/shader.mdandlibSceAgcDriver.mdupdated.Tested
Linux (CachyOS, kernel 7.2.9), lavapipe only. The CI device is lavapipe; no real GPU and no title or homebrew were run.
ImageSrgbSampleCopy: point, bilinear and gather on 8_SRGB and 8_8_SRGB including the second channel. Bilinear of a 0/255 pair must be 0.5 (filtering before decoding would give 0.21). A pending-storage-image case (store, sample, second store with different data, sample) checks the Version refresh.APS5_SRGB_SHADER_DECODE=1makes the new test exit 1. DisablingrefreshSrgbExpandmakes the storage case fail ("thread 4 sampled 0.0, expected 0.0144").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)