Skip to content

perf(agc): skip the depth plane copy when no slice changed - #2491

Merged
oneandonlydean merged 2 commits into
boykopovar:mainfrom
oneandonlydean:up639/depth-plane-unchanged-copy-on-425
Oct 13, 2026
Merged

oneandonlydean merged 2 commits into
boykopovar:mainfrom
oneandonlydean:up639/depth-plane-unchanged-copy-on-425

Conversation

@oneandonlydean

@oneandonlydean oneandonlydean commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

What

Follow-up to #425. Stacked on #2490 (keep unchanged guest layers of a depth plane copy), which this builds on; the first commit here is #2490's.

  • Each depth surface carries a write serial, bumped when a draw binds it as its depth target and when a pending fast clear is applied.
  • DepthPlaneCopy keeps the image and serial of every slice it copied. A use copies only the slices whose surface or serial changed, plus the guest layers that changed (as before), and records nothing at all when none did.
  • A slice whose surface is the depth target of the latest depth draw is copied at every use until another depth target is bound, since that draw may still be recorded after the copy.
  • When only some slices changed, the copy image keeps its other layers (it goes from GENERAL to transfer and back instead of from UNDEFINED).

#425 is merged, so this head is rebased onto main 1c5e628, on top of #2490's new head af716047. Both commits applied without conflicts and the change is the same.

  • Depth surface images are written only through DepthSurfaceView (draws) and ApplyFastClear on main, HTILE and memory value fills included (they set the pending clear), so the serial covers every writer.
  • The copy's cached textures view the copy image, which lives as long as the copy, so the 2D view of the first slice that feat(agc): depth planes sampled as 2D arrays, cubes and integer views #425 added stays valid and sees each refill.
  • feat(libSceAgcDriver): storage image access to depth surfaces #4500 adds a store-back from a storage image into a depth surface. With both in the tree, that store-back has to bump the serial (written = ++depthWrites; after its copyPlane(plane, stencil, false)); whichever lands second needs that one line.

It cannot merge before #2490.

Why

Every sampled use of a depth array, cube or R16_UINT view copied every slice a depth surface holds into the staging buffer and the whole staging buffer into the copy image, behind three ALL_COMMANDS barriers, although nothing had written the slices since the previous use. A title that renders its shadow cascades once per frame and samples them a few hundred times spent most of its GPU frame in these copies.

Tested

Linux (CachyOS, GCC 16.2.1, Release with CI's configure flags and ANYPS5_ENABLE_SPIRV_TOOLS=ON, full build and libs), RTX 4090, NVIDIA 610.57.04.

Not measured on this base: the level numbers of the earlier base (0.174 fps without, 2.275 fps with, on main 99c1c26 + #425's earlier head + #454). Not tested on Windows or on AMD.

Checklist

This was referenced Oct 9, 2026
@github-actions github-actions Bot added docs agc libSceAgc* needs rebase Conflicts with main, rebase needed labels Oct 9, 2026
@NahuelGenchi

Copy link
Copy Markdown
Contributor

Tested on a second title and vendor: Sonic Origins Plus (PPSA05324), Linux, Radeon R9 380 2 GB (RADV, Mesa 26.0.8), 960x540 render size. Its main menu draws a 4-slice 1024x1024 D32 shadow cascade array once per frame and samples it in 43 draws.

Main menu, 10 s windows with APS5_PROFILE_GPU=1. Both builds are the same tree (#425 at 62b1e55, #454, and the fixes the title needs to reach the menu), without and with this PR's head 8085dda:

Per presented frame Without With
Presents per 10 s 100–104 146–151
GPU busy 98.7 ms 65.1 ms
Uses of the plane copy 43 43
Uses that record a copy 43, 4 slices each 1, 4 slices
GPU time outside every timed range 22–23 ms 1.4 ms
Blended passes that sample the cascades 45 passes, 15.1 ms 4.2 passes, 3.8 ms
  • The copies were the untimed GPU time. I counted the uses and the recorded copies with a local counter in Refresh.
  • The draws that sample the array now share passes. Each copy used to end the pass before it.
  • The menu frame looks the same in both builds.
  • agc_driver_depth_plane_guest_layers passes on this card.

One note for the rebase: with #454 in the same tree, DepthSurfaceView conflicts. I kept #454's shape, bumped written where it finds the bound surface, and set drawnDepth = bound before the return.

@Mouhand-Kaddo

Copy link
Copy Markdown
Contributor

Additional platform validation on standalone PR head 8085ddae5d8326194483da312e50ed9a3d96c702:

Windows: passed — Windows 11, supported WinLibs GCC 15.2.0 UCRT/POSIX/SEH r7; CMake 4.3.1 / Ninja.

Release / BUILD_TESTING=ON: built the selected test targets; 2 CTest cases, 0 failures, 0 CTest skips.

Tests: agc_driver_graphics, agc_driver_depth_plane_guest_layers.

Reproduce after building the test targets: ctest --test-dir <build> --verbose --no-tests=error --timeout 30 -R "^agc_driver_(depth_plane_guest_layers|graphics)$".

Vulkan: native AMD RX 6900 XT, driver 32.0.21045.5002.

Integration limitation: a local merge against main 948a526c2 has source conflicts; these results validate the standalone PR head, not a resolved merge.

@oneandonlydean
oneandonlydean force-pushed the up639/depth-plane-unchanged-copy-on-425 branch from 8085dda to 1360f6f Compare October 11, 2026 18:32
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 11, 2026
@oneandonlydean
oneandonlydean force-pushed the up639/depth-plane-unchanged-copy-on-425 branch from 1360f6f to ae3b551 Compare October 11, 2026 23:32
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 11, 2026
snowzhangy pushed a commit to snowzhangy/AnyPS5 that referenced this pull request Oct 12, 2026
…opies only for changed layers and slices) into integ/head

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@oneandonlydean oneandonlydean self-assigned this Oct 12, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 12, 2026
@oneandonlydean
oneandonlydean force-pushed the up639/depth-plane-unchanged-copy-on-425 branch from ae3b551 to dcd0b74 Compare October 12, 2026 19:30
Sampling a depth surface as a 2D array, a cube or its integer bits
copies its planes into a per-use image. Each use read every slice no
depth surface holds from guest memory and detiled it again, even when
nothing had written it since: in one level of a heavier title that was
about 165,000 layer reads, about 650 GiB, in a five-minute run.

A slice read from guest memory now stays in the copy's staging buffer
with the write generation it was read at. The next use reads and
detiles it again only when its address, size or tiling changed, the
write tracker reports a write since, or recorded GPU work writes the
range; a slice a depth surface held in between is read again. Without
write watching every use reads, as before.
@oneandonlydean
oneandonlydean force-pushed the up639/depth-plane-unchanged-copy-on-425 branch from dcd0b74 to e691d03 Compare October 13, 2026 00:25
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
@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
#4500 docs/dev/debt/libSceAgcDriver.md L29-30
#4873 core/libs/prx/libSceAgcDriver/Graphics/src/DepthSurface.cpp L209-210, L227-234, L277-287

@GustavoGamarra95

Copy link
Copy Markdown
Contributor

@oneandonlydean #4873 overlaps this PR in DepthSurface.cpp (the bot flagged it). It is a follow-up to #425 for padded linear slices: Beast of Reincarnation (PPSA29343) samples a 32x32 D16 surface as a six-face cube, and DescribeSurface gives 16384-byte slices while the copy expects 2048, so the title stops with depth plane copy geometry does not match its layers.

This PR keeps the same tight-slice assumption in record(): region() uses sliceBytes() * layer as the staging offset, while the detiler writes at geometry.LinearLayerOffset(layer). Whichever lands second can resolve it like this:

  • Refresh: accept geometry.sliceLinearBytes >= sliceBytes() and grow the staging buffer to sliceLinearBytes * layers when needed;
  • region(): bufferOffset = geometry.LinearLayerOffset(layer) + mip.linearOffset and bufferRowLength = mip.pitchBytes / elementBytes.

Growing the staging buffer does not affect this PR's partial copy: every copied layer rewrites its part of the staging buffer before the image copy, and unchanged layers are kept in the image, not in the staging buffer. Happy to rebase #4873 onto this one if it lands first, or the other way around.

@oneandonlydean

Copy link
Copy Markdown
Collaborator Author

This one goes in first, after #2490. Please rebase #4873 on top once it is merged, with the Refresh and region() changes you describe, so the partial copy uses the padded slice offsets as well.

Sampling a depth array, cube or R16_UINT view of depth surfaces went
through DepthPlaneCopy::Refresh on every use. Each use copied every slice
a depth surface holds into the staging buffer and the whole staging buffer
into the copy image, with three ALL_COMMANDS barriers, although nothing
had written the slices since the previous use.

Each depth surface now carries a write serial, bumped when a draw binds it
as its depth target and when a pending fast clear is applied. The copy
keeps the image and serial of every slice it copied. A use copies only the
slices whose surface or serial changed and the guest layers that changed
(as before), and records nothing when none did. A slice whose surface is
the depth target of the latest depth draw is copied at every use until
another depth target is bound, since that draw may still be recorded after
the copy.

Test: agc_driver_depth_plane_guest_layers counts the recorded slice copies
and copy fills; a second use with nothing written records none (it records
one of each without this change), a draw to a slice's surface or a CPU write
to a guest layer copies only that layer.
@oneandonlydean
oneandonlydean force-pushed the up639/depth-plane-unchanged-copy-on-425 branch from e691d03 to 50e5f08 Compare October 13, 2026 01:34
@GustavoGamarra95

Copy link
Copy Markdown
Contributor

Sounds good. I'll wait for #2490 and #2491 to land, then rebase #4873 onto the updated main and adapt Refresh and region() to preserve the padded linear slice offsets with partial copies. Thanks for coordinating the merge order.

@oneandonlydean
oneandonlydean merged commit 2a55cfe into boykopovar:main Oct 13, 2026
8 checks passed
@Scramblrrrr

Copy link
Copy Markdown
Contributor

Windows RTX 5070 Ti cross-test completed for the #2490/#2491 port, now codex/first-light-depth-plane-copy-cross-test at 2086c53, base 861fe25 (experimental 007 integration).

Native guest-layer/copy-count, depth compare-copy, fast-clear, fast-clear-submit, storage-reuse, last-writer and full mocked graphics: 7/7 passed initially (7.50 s). I also imported Michon's sampled-value fixture dc0c2cf preserving authorship. It exposed a pre-existing integration omission: the extracted planesAccepted helper lost upstream's depth-bits alternative. Both this port and production baseline 861fe25 failed the D16 depth-bits array refusal identically. Restoring the same upstream gate made all eight checks pass (8.05 s).

Then I removed only #2491's production change, keeping #2490, the restored format gate and both fixtures: copy-count fails with 1 depth surface copy + 1 plane fill where 0 + 0 are expected; sampled-value control passes. Restored and rebuilt: 8/8 passed (7.98 s), without skips (ANYPS5_REQUIRE_VULKAN=1). Final fixture-only comment cleanup follows this integration's conventions; rebuilt sampled-value test passes again. Scoped conventions/diff checks pass.

The integration also has storage-to-depth write-back, so Transfer(..., false, ...) bumps the new write serial as required; retired-surface/address filters and TakeWrites are preserved. No game performance claim yet. The new game comparison stages only patched graphics, with libc/kernel unchanged, normal asynchronous draws, the same warm cache/profile/cache and memory limits as 861fe25. Graphics SHA256 FCB1B8FBE97FD95D817653C882B307D2296D81EEA04C913F3474C056016DF166.

AI-assisted (Codex). No title code, captures or assets uploaded.

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

Labels

agc libSceAgc* docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants