Repository navigation
feat(agc): run DB depth copies into a single-sample R32 float target - #8975
Conversation
A draw with DB_RENDER_CONTROL.DEPTH_COPY copies one sample of the Z_32_FLOAT depth surface into color target 0. It now runs as a compute pass that fetches that sample and stores it into the R32 float target, when the draw covers the whole surface without depth or stencil tests. With COPY_CENTROID the sample is the first one the draw covers, which is sample 0 under the full sample mask the driver requires, as radeonsi programs its per-sample DB to CB copies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A one-sample depth surface was bound to the copy shader's sampler2DMS, which Vulkan only allows for a multisampled view. The shader is now built in a single-sample and a multisample variant, and the copy picks one by the surface's sample count. The copy also no longer requires the guest-memory import state of native multisample depth sampling: it reads the host image the draws rendered into. Its barriers name the host stage their host access bits need, as the CMASK clear's do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
added
the
unblocks title
|
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.
|
|
Tested on AMD Ryzen 7 9800X3D and AMD Radeon RX 7900 XTX (RADV NAVI31) with Fedora Linux (Mesa 26.2.5):
|
|
Thanks @clairtonmichel, that's the first AMD run of this. Heads-up: the next push replaces the compute copy you tested ( AI-assisted: yes (Claude Code) |
|
@milescoviello an update from the maintainer side: the whole-surface copy has been accepted as it is, with its debt line, so #8975 doesn't need to wait for the draw-based copy. If you mark it ready as it stands, we'll run full CI and merge it, then close #5499 as carried. The draw-based copy can follow as its own PR whenever you get to it. |
- Drain the draw pipeline before recording the copy, as the CB metadata pass does, so the copy follows the queued draws that render the plane. - Address the plane as decodeDepth registers it: a multisampled plane's pipe-bank XOR bits are stripped and a DB_DEPTH_VIEW slice is added. Mipmapped or partially resident planes are refused. - Refuse a color target with CMASK fast clears, as the CB resolve does. - Find the source through the depth owner (planeSurface) and require a 32-bit float surface with the decoded sample count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks @oneandonlydean. Before marking it ready I went over the rebase once more and fixed four things in
The decode tests cover 2: XOR stripping, slice addressing and the mip refusal. On The draw-based copy will follow as its own PR. I have a first version of it locally: the title's draw with a driver pixel program, and a coverage test against a reference draw of the same triangle. AI-assisted: yes (Claude Code) |
|
Tested final rebase (127aba3) on AMD Ryzen 7 9800X3D and AMD Radeon RX 7900 XTX (RADV, Fedora Linux, Mesa 26.2.5):
|
|
Thanks @clairtonmichel, that's this head on AMD. I added it to the Tested section and the GPU line. AI-assisted: yes (Claude Code) |
|
The draw-based copy is up as #8999, stacked on this PR. It runs the copy as the title's draw with a driver pixel program, and a new execution test checks that only the texels a reference draw of the same triangle covers are written. It's for after this one merges. AI-assisted: yes (Claude Code) |
|
@milescoviello thanks for marking it ready. Full CI found one problem on Linux (run 39574666009, llvmpipe; Windows is green): So on a device that can't sample the depth surface at its sample count, the refusal path crashes instead of refusing cleanly (llvmpipe hits it; the GPUs you tested probably don't). Could you take a look? We're reproducing it locally too and will add the line if we find it first. |
The DB depth copy case drew a two-sample depth surface, which lavapipe cannot sample (it samples depth at 1 and 4 samples only). The copy then threw outside the expected-failure checks and the test crashed on exit. Pick 2 samples when the device samples them, else 4, else skip the multisample half, and copy every sample of that count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@oneandonlydean found it, it's the test. lavapipe samples depth only at 1 and 4 samples: llvmpipe 20.1 reports The fix only touches the test. It uses 2 samples when the device can sample them, otherwise 4, and otherwise skips the multisample half. It also copies every sample of that count, not just 0 and 1. I'll push it as a new commit once it passes here under lavapipe and on the NVIDIA card. A title perf run has the machine for about another 15 minutes. AI-assisted: yes (Claude Code) |
|
@oneandonlydean pushed as Under lavapipe (llvmpipe, LLVM 20.1.2), #8999 is rebased on top of it. Its tree doesn't change, since it removes that test case anyway. AI-assisted: yes (Claude Code) |
|
@milescoviello thanks, that matches what we see: lavapipe gives 1 | 4 for both depth sample counts, so the two-sample copy hit the refusal outside an expected-failure check. One small addition, if you agree: with this change no device runs the new refusal itself. AMD and NVIDIA sample depth at 2, and lavapipe now takes the 4-sample path, so |
|
I tested Dean's requested unsupported-count case at exact head
Ubuntu 24.04.3, GCC 13.3, Release; lavapipe Mesa 25.2.8 / LLVM 20.1.2; RTX 3090 / NVIDIA 595.99.02; gfx1036 ( This can be folded into the existing PR; it adds the requested expected-refusal coverage without changing production code. It does not make arbitrary unexpected exceptions in this fixture safe to tear down. diff --git a/core/libs/prx/libSceAgcDriver/tests/execution/DepthStorage.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/DepthStorage.cpp
index 4aba6f4a2..27236f419 100644
--- a/core/libs/prx/libSceAgcDriver/tests/execution/DepthStorage.cpp
+++ b/core/libs/prx/libSceAgcDriver/tests/execution/DepthStorage.cpp
@@ -355,6 +355,12 @@ void DepthCopyCase(Context context, std::uint8_t* block) {
ExpectFailure([&] { copy(samples, 0, {Side / 2, Side / 2}); }, "of another size", "a DB depth copy of another extent than the depth surface was accepted");
ClearDepthSurfaces(context.device);
}
+ if ((context.limits.sampledImageDepthSampleCounts & VK_SAMPLE_COUNT_2_BIT) == 0) {
+ Require(DepthSurfaceView(context, DepthTarget{depthAddress, 0, {Side, Side}, VK_FORMAT_D32_SFLOAT, 0.375f, 0, 0, false, 2}) != VK_NULL_HANDLE, "the unsampleable two-sample depth surface was not created");
+ ExpectFailure([&] { copy(2, 0, {Side, Side}); }, "cannot sample at its sample count", "a DB depth copy from an unsampleable two-sample depth surface was accepted");
+ ClearDepthSurfaces(context.device);
+ std::puts("DB depth copy: unsupported two-sample source refused cleanly");
+ }
Require(DepthSurfaceView(context, DepthTarget{depthAddress, 0, {Side, Side}, VK_FORMAT_D32_SFLOAT, 0.625f, 0, 0, false, 1}) != VK_NULL_HANDLE, "the one-sample depth surface was not created");
std::fill(block, block + BlockBytes, std::uint8_t{0});
copy(1, 0, {Side, Side}); |
…not sample On a device that cannot sample two-sample depth, as lavapipe, the DB depth copy case now also creates a two-sample surface and expects the copy's sampling refusal, so that refusal is covered where it applies. Devices that sample two-sample depth skip it. Co-authored-by: simonbbby <159568960+simonbbby@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@oneandonlydean agreed. Thanks @simonbbby for the diff and for the GDB trace: the crash is the cache teardown after AI-assisted: yes (Claude Code) |
|
Pushed AI-assisted: yes (Claude Code) |
What
This carries #5499 by @codingreapermaps, rebased onto current
main, with a fix for one-sample depth surfaces and the review fixes below on top. Their commit keeps its authorship. @oneandonlydean offered this route on #5499: a new PR crediting them, with #5499 closed in its favour.The change (codingreapermaps's commit,
191c8a8d2b). A draw withDB_RENDER_CONTROL.DEPTH_COPYthrewDB_RENDER_CONTROL depth copy, resummarize or decompress is unsupported. PPSA12361 (Transport Fever 2) issues one in every frame of a map:DB_RENDER_CONTROL = 0x84(DEPTH_COPYwithCOPY_CENTROID), a two-sampleZ_32_FLOATdepth surface and a single-sample R32 float color target of the same size, which its ambient occlusion pass samples next.DecodeDepthCopyPassrecognises the draw before shader compilation. It accepts only:Z_32_FLOATdepth plane;PA_SC_AA_MASK;Anything else throws, and draws without
DEPTH_COPYare unchanged.The copy runs as a compute pass (
DepthSample.comp). It fetches one sample of the host depth image and stores it into the target's resident storage image, which is then marked dirty like any other GPU write.With
COPY_CENTROID, the copied sample is the first one the draw covers. radeonsi programs its DB-to-CB copies withCOPY_CENTROIDand a one-sample mask per sample; under the full sample mask required here, that is sample 0. WithoutCOPY_CENTROID,COPY_SAMPLEselects it. The console is not measured;docs/dev/debt/libSceAgcDriver.mdrecords this.Rebase.
CMakeLists.txt,TextureDetiler.hpp/.cpp,DepthSurface.cppand the debt file conflicted.CopySamplereads through it, as the texture path does. TheSAMPLED-usage hunk is gone, becausemainnow covers it.DescriptorRing, asmain's other detiler passes do now.One-sample surfaces (
120900a342, mine). PPSA34501 and PPSA34502 (#6304) copy from a one-sample D32 surface. Before, its single-sample view was bound to the shader'ssampler2DMS, which Vulkan allows only for multisampled views (VUID-RuntimeSpirv-samples-08726; it happened to work on NVIDIA).DepthSample.compis now built in asampler2Dvariant and asampler2DMSvariant, and the copy picks one by the surface's sample count.samplingActive). It reads the host image the draws rendered into.VK_PIPELINE_STAGE_HOST_BITbesideALL_COMMANDS, as the CMASK clear's do, because their access masks include host bits.Review fixes (
127aba3ed6, mine).decodeDepthdecodes it: a multisampled plane's pipe-bank XOR bits are stripped and aDB_DEPTH_VIEWslice is added. Mipmapped and partially resident planes are refused.planeSurface), and it must be a 32-bit float surface with the decoded sample count.The copy is still chosen from the viewport and scissor. The maintainers accepted that with its debt line, so the draw-based copy (the title's draw with a driver pixel program) will be its own PR.
Tested
Ubuntu 24.04, GCC 13.3, NVIDIA RTX 4070 Ti, driver 595.99.02.
On
223e90289b, the lavapipe fix for the CI failure:DepthCopyCasenow copies at 2 samples when the device can sample them, otherwise 4, and skips the multisample half when neither works.agc_driver_depth_storagepasses 3 of 3 under lavapipe (llvmpipe, LLVM 20.1.2, which samples depth at 1 and 4 samples only).On
67cea1c314, @simonbbby's addition (co-author), which @oneandonlydean asked for: on a device that can't sample two-sample depth,DepthCopyCasecreates a two-sample surface and expects thecannot sample at its sample countrefusal.agc_driver_depth_storagepasses 3 of 3 and printsDB depth copy: unsupported two-sample source refused cleanlyeach time.On
127aba3ed6, onmainf894ba6: the 30 tests matchingdepth|Depth|db_copy|DbCopy|^agc_driver_graphics$pass 3 of 3.The decode tests now also cover XOR stripping, slice addressing and the mip refusal.
On
120900a342, before the review fixes, the 29 depth tests passed.The tests for the one-sample fix:
depthCopyPassTestsdecodes a one-sample plane and refuses its sample 1.DepthCopyCase, after the multisample checks, copies from a one-sample D32 surface, expects its clear value in every texel, and refuses sample 1.Khronos validation layer (LunarG SDK 1.4.363) over
^agc_driver_(depth_storage|graphics):127aba3ed6,120900a342andmainf894ba6 each pass 5 of 5 with no VUID.The same layer at the previous head (815c622, the same two commits on an older
main), overagc_driver_depth_storage*andagc_driver_graphics:VUID-RuntimeSpirv-samples-08726×2, plus-02815×6 and-02816×12 (host bits onALL_COMMANDS).CoD Black Ops II (PPSA34502):
mainstops at 15 s onregister 0x0 = 0x00000084.120900a342(release libs, 12 GB cap): the scripted 203 s run reaches the war-room menu, which renders at about 0.7 FPS without the performance PRs. The log has no0x00000084stop and no driver failure.@alexpournaras: PPSA34501 gets past the
0x00000084stop with this branch (feat(agc): run DB depth copies into a single-sample R32 float target #5499).codingreapermaps's original testing (Windows 11, MinGW-w64 GCC 15.2.0, NVIDIA MX450): PPSA12361's map renders, and a RenderDoc capture shows the R32 float target holding the copied depth that the ambient occlusion pass reads.
@clairtonmichel, AMD Radeon RX 7900 XTX (RADV, Mesa 26.2.5, Fedora): the same 30 tests pass at
127aba3ed6.Not tested: Intel at this head.
Checklist
main; no other open PR implements the same functions (supersedes feat(agc): run DB depth copies into a single-sample R32 float target #5499; overlaps the draft feat(agc): add experimental depth clear and copy recognition #1395)NotImplemented_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)0x84stop; untested here)🤖 Generated with Claude Code