[fisherjoey]

What

This is the fix from #1133. A comparison-sampled R32_SFLOAT or R16_UNORM surface that a storage image holds is now sampled from a D32_SFLOAT or D16_UNORM image that the GPU refills from the storage image. It no longer stays a CPU snapshot compared against guest memory.

Vulkan only compares depth formats, and this driver reports no depth-comparison sampling on R32_SFLOAT or R16_UNORM, so cachedTexture kept these lookups (depthCompare) off the storage-image view path. After every GPU write-back, the next lookup flushed, waited for the GPU and compared the whole surface under the GPU mutex. In Demon's Souls that is a 16 MiB shadow map (1024x1024, 4 layers), compared 10-20 times a second on queue 0x30, and queue 0 waits behind it.

• Texture(context, source, descriptor, components, Texture::DepthCopyTag) makes a depth image of the surface's shape. RefreshDepthCopy() records a copy from the storage image into a device buffer and from there into the depth aspect, with barriers, whenever StorageTexture::Version() has moved since the last copy.
• cachedTexture sends a depthCompare lookup down this path when the surface is host-imported, R32F or R16 unorm, not 3D, at least 1 MiB, and the storage image has the same format (CanDepthCopyFrom). Everything else stays on the snapshot path as before.
• ShaderResources::Revalidate refreshes the depth copies of a set after a successful proof, because the fast proof repeats no lookup.
• APS5_DEPTH_COPY_MIN_KIB (default 1024) sets the floor. TechnicalDebt has a line for the floor and the 3D exclusion. There is no switch to turn the copies off.

New test: agc_driver_depth_compare_copy has three cases: R32F 512x512, R16 unorm 1024x512 (the D16_UNORM branch), and R32F 256x256 with 4 layers and 3 mips, stored and compare-sampled at layer 0 mip 0, layer 2 mip 1 and layer 3 mip 2. Each case writes the surface with a compute store, compare-samples it, stores again and samples it again through the same compiled program, so the second dispatch reuses the cached descriptor set. Texture::TakeDepthCopyCounts() (depth copy textures made, copies refreshed) must read 1 and 1 after the first samples, and 0 and 1 after the second store, which must also move StorageTexture::Version(). The guest blocks are write-watched so the reuse can take the fast proof.

Closes #1133.

Before/after, this change alone

Demon's Souls (PPSA01341 v1.07), boot through the PS Studios movie into the attract movie, 200 s per run. Same build and settings on both sides; the only difference is APS5_NO_DEPTH_COPY=1 on the "before" side (a switch in that local build, removed from this PR since). 3 runs each, interleaved (before, after, before, ...). Each cell is the median of the per-run medians, per 10 s, with the per-run values in brackets.

	before (copy off)	after (copy on)
PS Studios movie: queue 0x30 GPU mutex held	408 ms (354-520)	34 ms (30-70)
PS Studios movie: queue 0 waiting for the mutex	247 ms (223-317)	65 ms (60-91)
Attract movie: queue 0x30 GPU mutex held	215 ms (209-231)	32-33 ms
Attract movie: queue 0 waiting for the mutex	114 ms (108-115)	30-33 ms
Attract movie: syncs on pending GPU writes ([hooksync])	108 syncs, 104 ms	5-6 syncs, 6-7 ms
Attract movie: presents	106 (105-116)	106-108
Depth copies (refreshes) at the end of the run	0	1 (about 1,700)

On the "before" side, almost all of those syncs are on ranges of up to 16 MiB (the shadow map). With the copy on, none are.

Presents don't move here. On these runs the title presented at about 10-12 fps on both sides, held by something other than this change. In earlier runs on a different build (in the issue), the attract movie went from 384-407 to 429-439 presents per 10 s with this change.

The build is main plus other local fixes and open PRs, not this branch alone. Both sides use that same build. The attract row for "after" has 2 runs: the first "after" run died with SIGSEGV at about 97 s, with no error line in the log and no core dump. 13 earlier runs of the same build with the copy on (it is on by default) had no such crash, so I can't tie it to this change. I'm noting it anyway.

Tested

Latest revision (ec4b473), rebased onto main aac0f1b, RTX 5070 Ti, driver 595.91.07:

• ctest -R agc_ -j1: 351 of 352. The one failure, agc_driver_dcc_keys_first_write, fails on main aac0f1b on this machine too.
• agc_driver_depth_compare_copy with the non-test changes reverted does not build (TakeDepthCopyCounts is missing). With only the ShaderResources.cpp lookup change reverted it fails with 0 depth copy textures and 0 refreshes, expected 1 and 1. With only the refresh loop at the end of Revalidate removed it fails with comparison sample of the same descriptor set after a second store: thread 2 compared to 1.000000, expected 0.000000.
• Before the rebase, the test also passed on RADV (Ryzen 5 7600X iGPU, Mesa 26.2.1). There the reuse takes the full walk instead of the fast proof (the write collection over the host-imported surface fails), so the Revalidate loop is only pinned on NVIDIA.

Earlier revisions:

Nobara 44 / Linux 7.2, GCC 16.2.1, RTX 5070 Ti, driver 595.91.07. Branch rebased onto main a29ef59.

• After that rebase, ctest -R agc_ -j1 (SPIRV tools on): 322/323, including agc_driver_depth_compare_copy. The one failure, agc_driver_dcc_keys_first_write, also fails on main on this setup.
• On the earlier base 9f4bd71, ctest -j1 --timeout 60: agc_driver + guest_agc_driver 184/184, including agc_driver_depth_compare_copy. Full run 463/464; the one failure is guest_dynamic_loader, which also fails on the perf(agc): prove sampled depth surfaces in the fast resource proof #1430 branch on the same main and is not touched here.
• Demon's Souls: numbers above.

Checklist

• Based on current main; no other open PR implements the same functions
• One topic per PR; follow-ups go in a new PR
• No comments in code except technical debt
• Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
• No notes, investigation .md files or images added to the repository
• New third-party code is a submodule built from source
• Generic behaviour, not specific to one title
• Depends on: none
• AI-assisted: yes

[popcoder565]

Native Windows run, since no Windows results were posted. Setup: Windows 10 (19045), RTX 3060 Laptop (NVIDIA 616.92), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release. Head dcffa687, merged onto main 8b393b6 in one build with 31 other PRs that had no Windows run.

• This PR's tests pass: agc_driver_depth_compare_copy.
• Full ctest in that build: 520 of 521. The one failure is guest_offline_imports, broken by feat(libSceRudp): implement sceRudpGetStatus #2359 (reported there).

No title was run.

🤖 Addressed by Claude Code

[adriadam10]

Built and ran this on top of current main: agc_driver_depth_compare_copy passes, and the log shows the new depth copy of storage image path being taken. But I reverted the non-test changes (Texture.hpp/.cpp, ShaderResources.cpp, TechnicalDebt) and the test still passes, so it doesn't pin this change:

• tests/execution/DepthCompareCopy.cpp: the snapshot path on main returns the same comparison results as the depth copy, so the test can't tell them apart. Make it observable: e.g. assert the lookup returned a texture with IsDepthCopy(), or count RefreshDepthCopy() calls / depth-copy lookups (and that the second store bumps StorageTexture::Version() and triggers exactly one more refresh). Per CONTRIBUTING, new tests must fail without the change.
• Coverage: only R32F, 1 layer, 1 mip. The copy has its own offset/extent math per mip and layer (Texture.cpp RefreshDepthCopy) and a separate R16_UNORM -> D16_UNORM branch; please add a case with several mips/layers and one for R16.
• ShaderResources::Revalidate refresh (ShaderResources.cpp, the new loop at the end): the fast-proof path has no test; a reuse of the same set after a store should show the copy refreshed.

Two things I can't verify here and that you may want to state: the SIGSEGV at ~97 s you mention (please say if it reproduces with APS5_NO_DEPTH_COPY=1 too), and that the copy assumes the storage image is in VK_IMAGE_LAYOUT_GENERAL when read from the transfer.

[adriadam10]

The points from the earlier review are still open on 318469b, and the branch now also conflicts with main in docs/dev/TechnicalDebt.md:

• tests/execution/DepthCompareCopy.cpp: still passes with the non-test changes reverted, so it doesn't pin the copy. Assert that the lookup returns a texture with IsDepthCopy(), and that the second store bumps StorageTexture::Version() and triggers exactly one more RefreshDepthCopy().
• Coverage: add a case with several mips and layers (the per-level offset/extent math in RefreshDepthCopy) and one for R16_UNORM -> D16_UNORM.
• ShaderResources::Revalidate (new loop at the end): reuse of the same set after a store should show the refreshed copy.
• Please say whether the ~97 s SIGSEGV reproduces with APS5_NO_DEPTH_COPY=1.

Please rebase on main as well (TechnicalDebt line next to a new one).

[orishamir]

c7110b3d is 318469b5 rebased; the PR's own changes are identical except the extra Mutation().Add(..., true) argument for main's gpu flag in the test. So the review's points are still open on this head:

• The test still does not pin the change: on llvmpipe, agc_driver_depth_compare_copy passes on the merged tree and passes again with Texture.hpp, Texture.cpp and ShaderResources.cpp reverted to main. No IsDepthCopy() or refresh-count assertion, no mips/layers case, no R16_UNORM case, no Revalidate case.
• The TechnicalDebt.md line is again the last line of the file (472/472 merged), the spot where feat(libkernel): implement _fstatfs #2858 and test(libkernel): cover cancellation recovery for #1668 #1739 also insert.
• The ~97 s SIGSEGV question is unanswered.

AI-assisted: yes (Claude Code Fable 5.1 xhigh)

[adriadam10]

The points from the previous reviews are still open on c7110b3d, and the branch now conflicts with main:

• tests/execution/DepthCompareCopy.cpp: still passes with Texture.hpp, Texture.cpp and ShaderResources.cpp reverted, so it doesn't pin the change. Assert that the lookup returns a texture with IsDepthCopy(), and that the second store bumps StorageTexture::Version() and triggers exactly one more RefreshDepthCopy().
• Coverage: add cases for several mips and layers (the per-level offset/extent math in RefreshDepthCopy), for R16_UNORM -> D16_UNORM, and for the Revalidate refresh.
• docs/dev/TechnicalDebt.md: conflicts with main. Rebase, and put the line away from the end of the file, where other open PRs also insert.
• The SIGSEGV at ~97 s with the copy on is unexplained, and the TechnicalDebt line mentions an unexplained intermittent boot crash with small surfaces. Please find out whether the copy is involved (e.g. a run with APS5_NO_DEPTH_COPY=1 reproducing it, or a trace of the crash) before this lands.

Happy to look again once these are addressed.

[fisherjoey]

@adriadam10 Thanks. Rebased onto main aac0f1b (TechnicalDebt conflict resolved) and pushed as ec4b473:

• Pinning the change: the test can't reach the Texture object a dispatch bound, so Texture::TakeDepthCopyCounts() returns how many depth copy textures were made and how many copies RefreshDepthCopy() recorded since the last call. Each case requires 1 and 1 after the first comparison samples, then checks that the second store moves StorageTexture::Version() and that the next samples add exactly one refresh and no new texture. With the non-test changes reverted the test doesn't build. With only the ShaderResources.cpp lookup change reverted it fails: 0 depth copy textures and 0 refreshes, expected 1 and 1.
• Coverage: two more cases. R16 unorm 1024x512 goes through the R16_UNORM -> D16_UNORM branch. R32F 256x256 with 4 layers and 3 mips is written with image_store_mip and read with image_sample_c_l at layer 0 mip 0, layer 2 mip 1 and layer 3 mip 2. Then layer 2 mip 1 is rewritten and both it and layer 0 mip 0 are sampled again.
• Revalidate: the second sample in each case dispatches the same compiled program, so it reuses the cached set. For that to take the fast proof the guest blocks are now write-watched, and the surface is stored and flushed once before the counted part. With the loop at the end of Revalidate removed, the test fails on NVIDIA: comparison sample of the same descriptor set after a second store: thread 2 compared to 1.000000, expected 0.000000. On RADV the write collection over the imported surface fails, so the reuse takes the full walk, whose lookups refresh the copy themselves, and the loop isn't exercised there.
• APS5_NO_DEPTH_COPY is removed, since it only turned the fix off. APS5_DEPTH_COPY_MIN_KIB (the size floor) stays.
• Layout: the copy reads the storage image in VK_IMAGE_LAYOUT_GENERAL. That is where StorageTexture keeps its image between command sequences. Every transition away from it (upload, clear, write-back copy) goes back to GENERAL in the same command buffer, and RefreshDepthCopy records into the active recorder's command buffer after them, or into its own batch when no recorder is active. I checked this by reading the code, not with validation layers, which aren't installed here.
• SIGSEGV at about 97 s with APS5_NO_DEPTH_COPY=1: not tested yet. I couldn't run the game this round. I'll report back after runs with and without the copy.

Tested on RTX 5070 Ti (driver 595.91.07): ctest -R agc_ -j1 passes 351 of 352. The one failure is agc_driver_dcc_keys_first_write, which fails on main aac0f1b on this machine too. check_conventions.py passes. I updated the PR description.

AI-assisted: yes

[adriadam10]

Checked on current main: agc_driver_depth_compare_copy and the rest of the libSceAgcDriver ctest set pass (355/355), the test no longer builds with the non-test changes reverted, and it now pins the copy counts, Version() movement, R16 and the mips/layers case.

Added one commit: moved the TechnicalDebt line off the end of the file (CONTRIBUTING: everyone appends there, so it conflicts).

Merging once the full Build (linux and windows) is green, since the new test opens a Vulkan device.