Repository navigation
fix(agc): give a block-copied BC texture an sRGB view for FORCE_DEGAMMA samplers - #7799
Conversation
|
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.
|
jotitadev
left a comment
There was a problem hiding this comment.
Apple GPU / MoltenVK run, as a second vendor. The block-copy sRGB view works on Metal. agc_driver_bc_block_copy passes with the new FORCE_DEGAMMA cases, and with Texture.cpp reverted it fails with the message from the description.
Machine: Apple M2 Max, macOS 27.2, x86-64 build under Rosetta. Drivers: MoltenVK 1.4.2 (Vulkan SDK 1.4.363.0) and MoltenVK main + SPIRV-Cross #2704. Head d130092, PR diff applied cleanly.
Tested: the tree is main a373e85 + mugurc's macOS series (ci/macos-series-2) + the PRs needed to build on macOS (#6667, #6668, #7524, #6969, and two files of #7332), not plain main (it doesn't build on macOS yet). Applied together in the same build: #7666, #7811, #7799 and #7803, plus a test-only patch that lets nine AGC fixtures load the Vulkan loader on macOS (branch jotitadev:fix/agc-tests-vulkan-loader-macos). Base on that tree: 40 ^agc failures with MoltenVK 1.4.2 and 26 with MoltenVK main + #2704, plus agc_driver_pooled_buffer_range and agc_performance_overlay, which only run because of the loader patch.
- With the PR:
agc_driver_bc_block_copy(BC7 through 32x4 uint blocks, BC1 through 32x2 uint blocks) andagc_driver_image_sample_force_degammapass 3 of 3 on both drivers. The log shows the block copy of the storage image (guest format 75 -> vk 145,62 -> vk 133). - With only
Texture.cppreverted (test kept),agc_driver_bc_block_copyfails on both drivers:a texture of guest format 181, which is not sRGB and has no sRGB view, is sampled through a sampler that forces sRGB decoding, which is not implemented. - Full
^agcsuite with the stack: the base failures on each driver, nothing new and nothing gone.
GPUs tested: Apple M2 Max (MoltenVK 1.4.2; MoltenVK main + SPIRV-Cross #2704)
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
d130092 to
12c62eb
Compare
fogydes
left a comment
There was a problem hiding this comment.
Verified the diff, conventions, and test coverage in PR #7799:
- Logic: In
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp, settingVK_IMAGE_CREATE_MUTABLE_FORMAT_BITwhenSrgbTwinFormatexists and callingprepareSrgbViewfor block-copied compressed textures aligns the block-copy constructor with the upload path, allowing samplers configured withFORCE_DEGAMMAto sample BC1/BC2/BC3/BC7 formats without rejection. - Unit Testing & Debt: Updated test fixtures in
tests/execution/BcBlockCopy.cppverify that pending block copies match stored blocks under forced degamma and remain pending afterwards. Debt entry indocs/dev/debt/libSceAgcDriver.mdupdated cleanly. - Conventions & CI: Zero code comments introduced. All CI checks pass 100% green (
conventions,eligibility,labels,report).
|
AMD and Intel Windows data: with this PR's test change, Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release, every execution test built,
AI-assisted: yes (Claude Code) |
…MA samplers Co-Authored-By: Claude <noreply@anthropic.com>
12c62eb to
3c905ee
Compare
|
@oneandonlydean asked me to bring my results from #8159 (closed as a duplicate of this PR) here, so here they are. I reran them on this PR's current head Setup: Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release, AMD Radeon RX 9070 XT (
None of the validation logs contains a A title: #8159's code change was the same two lines in I did not rerun the UHD 770 on this head. AI-assisted: yes (Claude Code) |
|
@savvaniss thanks for bringing these over and rerunning them on this head. I've added your RX 9070 XT runs and the Trivial Pursuit Live! result to the description, with |
jotitadev
left a comment
There was a problem hiding this comment.
Adds NVIDIA and lavapipe (native Linux) to the devices here: the existing runs are AMD RX 6700 XT / RX 9070 XT and Intel UHD 770 on Windows, plus an Apple M2 Max MoltenVK run. red→green holds on both of mine too.
Tested
Ubuntu 26.04, GCC 15.2.0, Release, BUILD_TESTING=ON, ANYPS5_NO_SHADER_CACHE=1 and ANYPS5_REQUIRE_VULKAN=1, device selected with VK_ICD_FILENAMES, serial CTest, on main a6dcea927 (this PR applies cleanly on it; head 3c905ee2d).
agc_driver_bc_block_copy |
NVIDIA RTX 3070 (595.99.02) | lavapipe (Mesa 26.0.3, LLVM 21.1.8) |
|---|---|---|
main a6dcea927 + the PR's test only (Texture.cpp reverted) |
fail | fail |
| this PR | pass | pass |
With Texture.cpp reverted and the PR's BcBlockCopy.cpp change kept, both devices exit non-zero with the message this PR removes:
AGC graphics: a texture of guest format 181, which is not sRGB and has no sRGB view, is sampled through a sampler that forces sRGB decoding, which is not implemented
The log shows the block copy landing as a storage image (block copy of storage image (guest format 75 -> vk 145)) and the sample then reaching Texture with guest format 181 and no sRGB view. With the full PR both devices pass. agc_driver_image_sample_force_degamma (the pre-existing force-degamma test) passes with and without the change, so the new path is specific to the block-copied BC case.
The rest of the guarded ^agc suite is unchanged from main on both devices (NVIDIA 858/861, lavapipe 860/861; the deltas are the same pre-existing sample_order / multisample_retirement failures main already has here).
GPUs tested: NVIDIA GeForce RTX 3070 (595.99.02, Linux) and lavapipe (Mesa 26.0.3, LLVM 21.1.8), Ubuntu 26.04.
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
Re-checked the interdiff: the test now covers both block patterns through a FORCE_DEGAMMA sampler and the pending-blocks store; the mutable-format flag and |
What
A sampler with FORCE_DEGAMMA samples a BC1, BC2, BC3 or BC7 UNORM texture through an sRGB view of the same image (#4487). That works when the texture is read from guest memory. It does not work while the texture is a block copy of a storage image a shader wrote, before those blocks are stored back. Then the draw is skipped, or the dispatch refused:
The block-copy constructor in
Texture.cppcreates its image withoutVK_IMAGE_CREATE_MUTABLE_FORMAT_BITand never prepares the sRGB view. So the same texture and sampler are refused or served depending only on whether the blocks have been stored back yet. It now does what the upload path does:SrgbTwinFormathas a twin for the block format (BC4, BC5 and BC6H have none and stay as they are);prepareSrgbView.The view is still made on first use, and a plain sampler uses the block copy as before. The technical debt is unchanged: its FORCE_DEGAMMA entry never listed block copies.
I found this while checking Ghost of Tsushima's report on #5166. Its format-56 case (8_8_8_8 UNORM) already gets an sRGB view on
main; the block-copy path is the one left out. Onmain, only BC textures whose blocks are still pending in a 32x2 or 32x4 uint storage image take this path. @savvaniss reports in #8159 (closed as a duplicate of this PR, with the same twoTexture.cpplines) that the PS4 path of #6977 also block-copies a smaller view of a render target, and that Trivial Pursuit Live! (CUSA03312, #7198) samples such an8_8_8_8_UNORMview with FORCE_DEGAMMA before its language menu and stops at the error above.Tested
Windows 11 26200, AMD Radeon RX 6700 XT (gfx1031), driver 32.0.21045.5002, WinLibs GCC 15.2.0 r7, Release,
BUILD_TESTING=ON, onmain1e774a3b2. Runs below useVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation(Vulkan-ValidationLayersvulkan-sdk-1.4.363.0, built locally),ANYPS5_NO_SHADER_CACHE=1,ANYPS5_REQUIRE_VULKAN=1andctest -V, with the implicit overlay layers on this PC switched off (VK_LOADER_LAYERS_DISABLE=~implicit~); no log reports the layer as not found.agc_driver_bc_block_copyhas a FORCE_DEGAMMA case for both of its formats, BC7 through 32x4 uint blocks and BC1 through 32x2 uint blocks:VUID-errors.main'sTexture.cppand this test it fails with the message above (guest format 181, BC7), with noVUID-errors.VUID-VkImageViewCreateInfo-image-12397four times (an sRGB view of an image created withoutVK_IMAGE_CREATE_MUTABLE_FORMAT_BIT). So the validation run covers that line.agc_driver_image_sample_force_degammapasses, with noVUID-errors.ctest --parallel 4 --timeout 120of this head (3c905ee2d,ANYPS5_REQUIRE_VULKAN=1, no validation layer): 1685 tests, 6 fail, none from this change:log_macrosandlog_macros_timestamps, because this local build passes ccache a relative__FILE__;agc_trimmed_import, which fails on unmodifiedmainhere (reported on perf(agc): keep a host import across a split of its guest range #6499; fix(agc): retire host imports before range cuts #7849 replaces it);agc_driver_layered_volume_vertex_no_gpl(AMD Windows: alpha-to-coverage still fails at the original executable path after #6436 #4768),agc_driver_draw_cache_reuse_offandagc_driver_stencil_replay_no_gpl, which hitVK_ERROR_DEVICE_LOSTfrom AMD's per-executable pipeline cache here and pass from renamed copies of their executables.python tools/check_conventions.py --base origin/mainwith this title and body passes.12c62ebc6merged intomain85029a00fon an AMD Radeon RX 9070 XT (driver 32.0.31041.1004) and an Intel UHD Graphics 770 (driver 32.0.101.7088), Windows 11 Pro 26300,ANYPS5_NO_SHADER_CACHE=1, serial CTest:agc_driver_bc_block_copypasses 3 of 3 with the PR and fails with only its test change onmain, with the message above.3c905ee2dmerged intomain56b299c3a) on the RX 9070 XT with the Khronos validation layer,ANYPS5_NO_SHADER_CACHE=1andANYPS5_REQUIRE_VULKAN=1:agc_driver_bc_block_copyandagc_driver_image_sample_force_degammapass 3 of 3 onmainand with the PR, with noVUID-line and no layer-not-found line; this PR's test withmain'sTexture.cppfails with the message above. With fix(agc): give block-copied textures an sRGB view #8159's change on feat: run PS4 games on the AGC driver — Limbo (CUSA01664) in game (integration branch) #6977's branch, Trivial Pursuit Live! reaches its language menu and title screen at 60 fps on the RX 9070 XT; without it, the title stops at the error above.d1300927con an Apple M2 Max, macOS 27.2, through MoltenVK 1.4.2 and through MoltenVK main with SPIRV-Cross Ratchet & Clank: Rift Apart: SIGSEGV since #861, an address-less 64 GiB reservation now takes the addresses the title hints next #2704. The tree wasmaina373e85e5with the macOS series, the PRs needed to build on macOS, fix(shader): round normalized storage image writes from the exact product #7666, fix(shader): keep host interpolation across entry EXEC copies #7811 and fix(shader): read a 2D MSAA T# through DIM 2D as its sample 0 #7803, and a test-only Vulkan loader patch, not plainmain. With this change,agc_driver_bc_block_copyandagc_driver_image_sample_force_degammapass 3 of 3 on both drivers; with onlyTexture.cppreverted,agc_driver_bc_block_copyfails with the message above.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)🤖 Generated with Claude Code