Skip to content

fix(agc): keep stores after occlusion dumps after them in a queued run - #5232

Merged
oneandonlydean merged 2 commits into
boykopovar:mainfrom
johnmaia:fix/occlusion-dump-order
Oct 14, 2026
Merged

oneandonlydean merged 2 commits into
boykopovar:mainfrom
johnmaia:fix/occlusion-dump-order

Conversation

@johnmaia

Copy link
Copy Markdown
Contributor

What

Fixes #5215.

Recorder::QueuedWriteGroups splits a queued store run into groups. Each group records its plain stores first and its occlusion counter dumps after them. A store started a new group only when it overlapped an earlier dump, so a store after the dumps that touched other memory joined their group and was recorded before them. That's fine for the final memory contents, but a title polling that store sees it early. The usual case is the label it writes after its occlusion queries to say "results ready". The title then reads the counters before the dumps land.

Now any store after a dump starts a new group, so a group's stores never follow its dumps in program order. The cost is one more dump dispatch per dump-to-store transition. APS5_LABEL_RUNS_INLINE=1, which records every dump at once, was not slower on the title below.

The QueuedWriteGroups tests only ran in the recorder binary's full mode, which ctest doesn't register. They now also run as agc_queued_write_groups (--queued-write-groups-only, no device needed). Updated expectations:

  • A clear of the next query slot waits for the dumps before it.
  • A store after the dumps starts a new group.
  • New case: a label after two dumps lands after them.

Tested

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 under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: PPSA15657 Disney Epic Mickey: Rebrushed (PPSA15657) on Linux #2255
  • AI-assisted: yes

🤖 Generated with Claude Code

@xXPBSXx

xXPBSXx commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Thanks for the fix and for registering queuedWriteGroupTests with CTest. One thing I noticed in the updated expectations: the per-query-slot pattern in the test (clear, begin dump, end dump, next slot's clear) was one group before (Groups{6}) and is now Groups{3, 6}, so a run with N query slots now produces N groups, i.e. N dump dispatches instead of one. The description says the cost is one more dump dispatch per dump-to-store transition and that APS5_LABEL_RUNS_INLINE=1 was not slower on the title used; a number for a title with many occlusion queries per frame would show whether the per-slot split matters.

I only read the diff; I did not build or run it.

AI-assisted (Claude Code).

@johnmaia

Copy link
Copy Markdown
Contributor Author

@xXPBSXx numbers from a title with many occlusion queries: Epic Mickey: Rebrushed in Dark Beauty Castle. Linux, RX 7600 (RADV, Mesa 25.2.8), main ce5c839 + #5030, with and without this PR, APS5_PROFILE_DRAW=1. Per-10-s counters averaged over the last 15 reports (in level), FPS over 30-300 s after Continue:

label-run barriers / 10 s occlusion dumps queued / 10 s in-level FPS
without this PR 3804 7321 4.84
with this PR (3 runs) 8212 / 9663 / 8815 4529 / 5943 / 5324 4.81 / 4.58 / 4.81

At about 48 frames per 10 s, the split adds about 100 label-run barriers per frame: each extra group is one barrier plus one fold dispatch. That is about 10% of all recorded barriers (39k-48k per 10 s). FPS moves within the run-to-run spread (0.23 between runs with the PR). The run without the PR is also the one where draws pop, so it isn't a like-for-like frame.

If the barriers become a cost on other titles, a narrower rule is possible: split only before a non-computed store that something later reads as a result. That needs knowing which store is the label, which the recorder doesn't track today. I'd keep it for a follow-up.

AI-assisted (Claude Code).

@Tamalero

Tamalero commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Tested on High on Life (PPSA15388, UE4), the title from #5488, at the same spot as my earlier traces there. This PR fixes most of that title's occlusion flicker on the normal GPU path, and makes it much faster.

Flicker. Walls, doors and windows in the home base and on the stairs now stay in place on the normal path (DumpSamplesOnGpu, no workaround switch). Before, they flickered frame to frame. One thing is left: about once in 50 frames, a run of queries still reads 0 and a visible object drops out for one frame (see below).

Speed, window-title FPS, same save and machine. The earlier numbers come from several runs, not a strict A/B, but the gap is large:

Scene Before (my earlier runs) + #5232, normal path
Home base living room about 2 on the normal path; 6–9 with the CPU-path workaround from #5488 6–12
Facing a wall 5 or less 14
Loading screen about 40 56, and its animated stencil text no longer stutters

Trace at the upstairs window, with the same local ZPASS_DONE trace as on #5488 (begin dump at A, end at A + 8, sum of the 16 DB slots, all ready):

Before (19 frames) With #5232 (50 frames)
Large steady queries (0.6–1.6 M samples) the 10.4 k-sample slot read 0 in 3 of 19 frames correct in 49 of 50 frames
Bad frame ~20 consecutive queries share one unchanged total the same shape, once

In the one bad frame (#48 of the trace), the three largest queries read 0 together. Their begin and end dumps (#4965–#4980) all show the same total, so no samples were counted between them. The neighbouring frames read about 1.45 M, 0.62 M and 1.58 M. The video shows the matching symptom: an object vanishes for exactly one game frame and comes back. Which object varies from blink to blink: the purple window trim, the rug hung on the right wall, the wooden frames, or a cup on the far window sill. Sometimes it is a larger blink where the window trim and the rugs all vanish at once. That fits a run of queries dropping out together, with a varying length, not one bad query.

So #5232 fixes the ordering against the polled label, which was most of the flicker and most of the cost. What's left looks like the GPU dump/count path that @MarcZX100 pointed at. I'll try #5213 on top of this next.

Details

Tested and written by Claude (AI assistant) on a contributor's machine.

Video:
https://youtu.be/PUh-uERlvts

@Tamalero

Copy link
Copy Markdown
Contributor

Follow-up on the remaining blink: with #5213 on top of this PR, it's gone in High on Life. That's 55 of 55 traced frames correct, no visible dropouts, objects no longer vanish behind the weapon, and about 12 FPS at the window, so no cost over this PR alone. Details are on #5213. Together the two PRs fix this title's occlusion problem from #5488.

Tested and written by Claude (AI assistant) on a contributor's machine.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
johnmaia and others added 2 commits October 14, 2026 18:44
A queued store run records each group's plain stores first and its
occlusion counter dumps after them. A store started a new group only
when it overlapped an earlier dump, so a label the title writes after
its dumps was recorded before them. A title that polls that label then
reads counters from before the dumps land. Any store after a dump now
starts a new group.

Epic Mickey: Rebrushed (PPSA15657) read stale or zero occlusion results
every other frame and left visible geometry undrawn on alternate frames.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Rebased onto main to resolve the test-list conflict in core/libs/prx/libSceAgcDriver/CMakeLists.txt (kept both entries); no other change.

@oneandonlydean
oneandonlydean force-pushed the fix/occlusion-dump-order branch from 1486a18 to 9a1ac6a Compare October 14, 2026 05:45
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
@oneandonlydean
oneandonlydean merged commit 9013e89 into boykopovar:main Oct 14, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* build

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Queued occlusion counter dumps give alternating visibility in PPSA15657 (objects pop every other frame)

4 participants