Skip to content

fix(agc): retire fully unmapped multisample targets - #4474

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
JJenkx:fix/multisample-target-retirement
Oct 13, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
JJenkx:fix/multisample-target-retirement

Conversation

@JJenkx

@JJenkx JJenkx commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

What

After a multisampled color target is freed, its guest-read guard and cached host image survive. A texture reusing that allocation is rejected as a read of live multisampled data. Notify the graphics registry after successful allocation removal or unmap, retire targets fully covered by the released range, and skip their cached images in later draws and metadata updates.

Protection changes and failed unmaps retain the guard. Retired Vulkan images remain allocated until device teardown so outstanding commands retain valid resources. Partial unmap/reuse and reads of live multisampled data remain unsupported. No new guest system exports.

Rebased onto upstream main b478fbbb1c on October 13. A subsequent merge-tree check against upstream 61b8742b6b4 is clean, including its newer multisample mutex initialization. The allocation callback follows the newer successful applyPatch mutation. Retirement preserves the upstream fragment-block guard sizing and checks target activity before the newer color-fill notification. The regression retains the upstream 2x/4x/8x guard-boundary cases alongside allocation removal, unmap, address reuse, protection and failed-unmap checks.

Tested

The full fork workflow passes with published head f7fd7569fa merged into current main 0214069e33c. Linux: 1,031 passed, 46 skipped, zero failed of 1,077; its multisample regression passes. Windows: 708 passed, 387 skipped, zero failed of 1,095. Windows multisample guest access and copied-direct ordering are skipped, so their Windows runtime behavior remains unvalidated. These are the repository-defined full workflow and test selections; skipped cases are untested. Upstream-maintainer CI remains pending.

Independent full Linux Release build of f7fd7569fa, BUILD_TESTING=ON, bundled pinned dependencies, default SPIRV-Tools option OFF, also passes. Nine selected checks pass on Radeon RX 6650 XT without skips: patched_libraries, guest_memory_pool_stats, agc_copied_direct_ordering, agc_host_import_unmap, agc_parked_import_unmap, agc_multisample_guest_access, agc_driver_graphics, guest_memory and guest_module_info. Multisample guest access and copied-direct ordering also pass on Mesa lavapipe without skips. Conventions pass. Removing only the unmap-observer installation makes the multisample regression fail on both Radeon and lavapipe at the stale texture-read guard after allocation reuse; exact restoration and rebuilding make both pass.

The earlier complete local lavapipe suite on this standalone head had 1,007 passed, 44 skipped and six failures of 1,057 with a 30-second timeout and parallelism 4, running as root. The five timeout cases pass individually at the upstream 120-second timeout with GALLIUM_OVERRIDE_CPU_CAPS=avx; the named-shared-memory permission case passes as normal user jjenkx. These follow-up results are not a single full-green local run. The first fork CI attempt on older main 091c136ba6 failed only guest_math, also failing on untouched main. Current upstream fixes that acosf NaN-bit issue, and the new full workflow above passes without adding unrelated math code to this PR.

Crazy Chicken Shooter Bundle (PPSA03117) motivated this fix. Earlier combined integration runs reached gameplay near 60 FPS and closed normally; they include other fixes and are not standalone validation of this rebased PR. No title run or game performance improvement is claimed for the current rebase.

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; no silent stubs added
  • No investigation reports, screenshots or game assets added to the repository
  • No new third-party code
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: PPSA03117 startup guard failure; no new game compatibility claim
  • GPUs tested: AMD Radeon RX 6650 XT, RADV / Mesa 26.2.4-arch1.1, Arch Linux; Mesa lavapipe 26.2.4-arch1.1 / LLVM 23.1.1, Arch Linux
  • AI-assisted: yes

@kr4y4

kr4y4 commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Windows run and the full suite, which the description lists as not run.

Setup: Windows 11 (26200), WinLibs GCC 15.2.0 r7, Release, CI configure flags, an RTX 4070 Laptop GPU and an Intel Raptor Lake-S iGPU. The head 012379f merged into main 2a5eca1.

  • It builds and links on Windows, including libSceAgcDriver's call into the new GuestAllocationsSetUnmapObserver_nid_postfix in libc.
  • agc_multisample_guest_access, agc_host_import_unmap, agc_parked_import_unmap, agc_driver_graphics, guest_memory, guest_memory_pool_stats and patched_libraries pass on both GPUs with --repeat until-fail:3. agc_multisample_guest_access prints Multisampled target guest access tests passed on both.
  • With main's MultisampleTarget.cpp and GuestAllocations.cpp/.hpp and the new test, agc_multisample_guest_access fails on both GPUs: a texture read of 0x…+0x50000 overlaps multisampled color target 0x…+0x40000 (4 samples), whose samples are only on the host: guest access to it is not implemented.
  • Full suite (ctest --timeout 120 --parallel 4):
    • iGPU: 972 of 973. The one failure, agc_driver_f32_dot2_rounding, fails the same way on main (no shaderFloat64, test(agc): skip the f32 dot2 rounding test on devices without shaderFloat64 #4528).
    • RTX 4070: in the first run agc_driver_graphics failed after 0.16 s (it normally takes about 8 s), and I didn't capture its output. Two more full runs passed 973 of 973, and 60 runs of agc_driver_graphics_tests.exe, four at a time, all passed. So I can't tie that exit to this change. main 2a5eca1 passed 973 of 973 in its one run.

Written with AI assistance.

@JJenkx
JJenkx force-pushed the fix/multisample-target-retirement branch from 012379f to 7b5b433 Compare October 12, 2026 18:42
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 12, 2026
@JJenkx
JJenkx force-pushed the fix/multisample-target-retirement branch from 7b5b433 to 026c198 Compare October 12, 2026 18:49
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 12, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Needs a rebase: #4199 is merged and this now conflicts with it in MultisampleTarget.cpp (guard()) and tests/Recorder.cpp. It gets a full build once it is clean.

@JJenkx
JJenkx force-pushed the fix/multisample-target-retirement branch from 026c198 to f7fd756 Compare October 13, 2026 15:52
@JJenkx

JJenkx commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

The rebase is now pushed as f7fd7569fa; GitHub reports MERGEABLE. It includes the #4199 guard changes and the newer Recorder regression cases. A merge-tree check against current main 091c136ba6 is clean.

The independent full Linux Release build passes. Nine selected checks pass on Radeon RX 6650 XT; multisample guest access and copied-direct ordering also pass on lavapipe, without skips. Removing only the unmap-observer installation makes the regression fail on both drivers; restoring it makes both pass. The PR description includes the broader suite results and their limitations.

Ready for the full build you offered once the conflicts were resolved. Thank you.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
@JJenkx

JJenkx commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor Author

The published rebase has completed full fork build results: PR build. Windows passes: 705 passed, 386 skipped, 0 failed of 1,091. Linux builds successfully: 1,026 passed, 46 skipped, 1 failed of 1,073; the multisample regression passes. Windows multisample guest access and copied-direct ordering are among the skipped cases, so no Windows runtime validation is claimed for them.

The single Linux failure is guest_math, which also aborts in the untouched upstream-main build at the same base 091c136ba6. An Ubuntu 24.04/GCC 13.3 probe of that production libc reproduces the failure at the acosf(2.0f) NaN-bit assertion. Current upstream already fixes it in 0265f336368; the same probe passes on 0214069e33c. This is a reduced libc/test build, not the entire runner environment.

A fresh full PR build against current main is queued. No unrelated math changes were added to this PR. Skipped cases remain unvalidated.

@JJenkx

JJenkx commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

The published head f7fd7569fa now passes the full fork workflow against current main 0214069e33c.

  • Linux: 1,031 passed, 46 skipped, zero failures of 1,077. The multisample regression passes.
  • Windows: 708 passed, 387 skipped, zero failures of 1,095. Its multisample guest-access and copied-ordering checks are skipped, so their Windows runtime paths remain unvalidated.

The earlier Linux failure on base 091c136ba6 was the upstream acosf NaN-bit issue; current main includes its existing fix. No unrelated math changes were added to this PR. Ready for the upstream full build/review you offered once the rebase was clean.

@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
#5003 docs/dev/debt/libSceAgcDriver.md L29-30
#5347 docs/dev/debt/libSceAgcDriver.md L29
#5462 core/libs/prx/libSceAgcDriver/Graphics/src/MultisampleTarget.cpp L7, L177, L207-217, L219, L235-237, L242-246, L390
core/libs/prx/libSceAgcDriver/tests/Recorder.cpp L5124-5125
#5466 docs/dev/debt/libSceAgcDriver.md L29
#5496 core/libs/prx/libSceAgcDriver/Graphics/src/MultisampleTarget.cpp L221-222

@oneandonlydean
oneandonlydean merged commit 8d98e88 into boykopovar:main Oct 13, 2026
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants