Skip to content

feat(agc): multisampled color and depth targets with CB resolves - #419

Merged
adriadam10 merged 8 commits into
boykopovar:mainfrom
MichonGoddijn231849:feat/msaa-targets
Oct 11, 2026
Merged

adriadam10 merged 8 commits into
boykopovar:mainfrom
MichonGoddijn231849:feat/msaa-targets

Conversation

@MichonGoddijn231849

@MichonGoddijn231849 MichonGoddijn231849 commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

What

Draws with multisampling enabled were refused (multisampling or coverage conversion is unsupported). They now render:

  • State: PA_SC_AA_CONFIG with 2, 4 or 8 samples is accepted when all of the following hold:
    • every color target has that many samples and fragments (CB_COLOR_ATTRIB);
    • the depth target has that many samples (DB_Z_INFO);
    • MSAA_ENABLE is set;
    • the sample locations of all four quad pixels are the standard ones Vulkan rasterizes with.
  • Targets: a multisampled color target is a host-only multisampled image, and a multisampled depth target is a multisampled depth surface. The pipeline and render pass use the draw's sample count. The guest sample planes and FMASK are never read or written. Sampling a multisampled surface is still refused, and MSAA image types already were.
  • Resolve: a CB_COLOR_CONTROL resolve over a multisampled CB0 runs as a render pass whose resolve attachment is CB1's resident image. A single-sample resolve is still refused as before.
  • Fast clear: a zero fill of a target's CMASK (fill kernel or CP DMA, next to the HTILE hook) fast-clears it. Its next use clears it to that use's CB_COLOR_CLEAR_WORD.

EQAA (fewer fragments than samples), coverage conversion, nonstandard sample locations and multisampled mips stay refused. The draw key gains the sample-location and CMASK_BASE_EXT registers.

GTA V renders its reflection cube's faces into a 256x256 2x target and resolves each face into the cube.

Tested

Windows 11 Pro, AMD Ryzen 5 7600, NVIDIA GeForce RTX 5080 (driver 32.0.16.1074), 32 GB DDR5-5600, MinGW-w64 GCC 15.2.0.

  • ctest: all tests pass. New cases cover:
    • 2x decode and the precheck;
    • nonstandard sample locations, EQAA, a single-sample target under 2x, MSAA_ENABLE cleared, coverage conversion;
    • the CMASK address;
    • CMASK compression on a single-sample target, still refused;
    • the resolve pass and a resolve between different formats.
  • GTA V (PPSA04264), Prologue: about 13k multisampled draws and resolves per 10 s were skipped before. They now render with no rejections, the CMASK fast clears are taken, and fps is unchanged. This was tested on my branch that combines this change with my other open PRs, not on this branch alone.
  • Dreaming Sarah and Demon's Souls were not re-run; neither multisamples in the scenes I know.

Checklist

🤖 Generated with Claude Code

@ThisIsAkill

Copy link
Copy Markdown
Contributor

A data point on the fast-clear part, in case it helps scope it: PPSA21567 (Astro Bot) fast-clears single-sample color targets through their CMASK, which this PR's multisampleInfo (applied only when samples > 1) leaves refused on main and here.

What the title's draws show (Linux, main with other open PRs and local changes, not clean main):

  • CB_COLOR_INFO = 0x00042730 and 0x10042730 (with DCC) on 11 distinct targets. The only bit the State.cpp mask rejects is 13 (FAST_CLEAR); DCC (28) and ROUND_MODE (18) are already accepted.
  • CB_COLOR_ATTRIB = 0 on all of them (1 sample, 1 fragment).
  • Every target with FAST_CLEAR set has a non-zero CB_COLOR_CMASK; the two targets without it have none.
  • The clear words are non-zero on several, for example CB_COLOR_CLEAR_WORD0/1 = 0x38003800 / 0x00005650 and 0x00000000 / 0xbc003c00.

These draws are currently skipped (color compression, DCC, ... is unsupported (CB_COLOR_INFO 0x00042730)), so the title's main targets are never rendered. I have not yet checked whether its CMASK clears are whole-target zero fills like the ones this PR hooks.

Do you plan to cover single-sample targets in this PR, or would you welcome that as a follow-up on top of it? I'm trying the obvious extension locally (the same zero-fill hook and clear-on-next-use for 1-sample targets, anything else refused) and can report how it behaves.

@ThisIsAkill

Copy link
Copy Markdown
Contributor

Follow-up on the single-sample case, with what Mesa's AMD drivers say about the CMASK codes (Mesa e5f06878):

  • src/amd/common/ac_descriptors.h:57-58: single-sample CMASK codes are CMASK_NOAA_COLOR_CLEAR_REG = 0x0 (fast-cleared to the clear register, "illegal with DCC") and CMASK_NOAA_COLOR_EXPANDED = 0xF (0xffffffff as a fill: expanded, not fast-cleared). Caveat: both are commented "only valid on gfx6-9".
  • radv initializes a CMASK to COLOR_EXPANDED (radv_init_color_image_metadata in radv_cmd_buffer.c). When it fast-clears an image that has both DCC and CMASK, it writes COLOR_EXPANDED to the CMASK and carries the clear in the DCC keys (radv_get_cmask_fast_clear_value, meta/radv_meta_clear.c:771-786).
  • radeonsi does use the zero code on GFX10: in the gfx_level >= GFX10 path of si_clear.c (around lines 654-707) it fast-clears a single-sample texture without DCC by filling a separately allocated CMASK with CMASK_NOAA_COLOR_CLEAR_REG. So the "gfx6-9" comment does not hold for that code, at least.

That matches what PPSA21567 does. Its non-DCC targets (CB_COLOR_INFO 0x00042730) get zero fills over the whole CMASK. Its DCC targets (0x10042730) get 0xffffffff fills, with the clear expected in the DCC keys, which main already models.

I tried the extension locally on top of this PR (Linux, with other open PRs and local changes, not clean main). A single-sample target may set FAST_CLEAR when it has a CMASK. A full zero fill fast-clears it to CB_COLOR_CLEAR_WORD on its next use or eliminate pass, a full 0xffffffff fill cancels a pending clear, and any other pattern or partial fill throws. Result: 8 targets were fast-cleared through their CMASK, the 0xffffffff fills cancel without error, and the title's distinct skipped-draw reasons went from 23 to 3. The run then stops at an unrelated depth storage-image path (#425's area). The branch is ready if you'd like it as a follow-up.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 4, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

@boykopovar this one needs your call: should multisampled colour targets be host-only images? With this PR their guest sample planes and FMASK are never written or read, so guest memory never holds an MSAA target's contents. Sampling one stays refused, and a CB resolve lands in the single-sample destination's resident image. That is the model main already uses for depth surfaces (a host image with no write-back), so it's an architecture choice more than a correctness question.

My read: fine as a step, because every shader path that would read the sample planes still throws, but a CPU or DMA read of that memory would see stale bytes without an error, as with depth today. Either way it needs a rebase: it conflicts with main in Graphics/src/Draw.cpp now.

@MichonGoddijn231849

MichonGoddijn231849 commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor Author

Thanks. Yes, host-only multisampled color targets are meant as a step. Every shader path that would read the sample planes or FMASK still throws, so the gap is the one you name: a CPU or DMA read of that memory sees stale bytes, as with depth surfaces on main. If @boykopovar wants write-back for both, I'd do depth and MSAA together in one follow-up, since they need the same machinery (see #425). I'll rebase this today.

@ThisIsAkill, thanks for the CMASK data and the Mesa references. Single-sample CMASK fast clears fit better as a follow-up on top of this than in it. Your rules (a whole zero fill fast-clears to CB_COLOR_CLEAR_WORD, a whole 0xffffffff fill cancels, anything else throws) match how I'd scope it, so please open it as a PR once this lands.

@MichonGoddijn231849

MichonGoddijn231849 commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor Author

Rebased onto main and the updated #374 (which it builds on). The Draw.cpp conflict was with the draw templates' moved-buffer rebinding from #424; multisampled draws still take no recipe. Build and ctest are clean apart from the failures main has locally.

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

Copy link
Copy Markdown
Collaborator

Thanks for the rebase. The #374 commits it carries now match #374's head, and the Draw.cpp resolution keeps multisampled draws out of the recipe path next to #424's moved-buffer check. The host-only question is still with @boykopovar.

@MichonGoddijn231849

Copy link
Copy Markdown
Contributor Author

Rebased onto current main and today's rebase of #374. The conflicts were ColorTarget's fields and the colour-target loop in State.cpp, against main's 3D colour targets and compacted exports. Both sides stay: depth, depthSlice and exportIndex from main, samples and cmaskAddress from this PR, with the sample-count check after exportIndex is set. Build and ctest are clean apart from the failures main has locally.

@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 5, 2026
@adriadam10

Copy link
Copy Markdown
Collaborator

Rebased again on current main (it conflicted with the new agc_draw_snapshot_cap test registration in libSceAgcDriver/CMakeLists.txt; both tests are kept, no logic changed). Build + ctest of the touched components pass, agc_driver_graphics and agc_multisample_guest_access run, not skipped.

Our commits on your branch are unchanged: duplicate TechnicalDebt lines removed, the guest-access guard covering CB bytes * samples, the guard test registered in ctest, and the TechnicalDebt/ARCHITECTURE notes.

Not covered by any test here: an actual MSAA draw and ResolveMultisampleTarget on a GPU; the title reports in the thread are the evidence for that part.

Merging once CI is green.

@github-actions

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
#2076 core/libs/prx/libSceAgcDriver/Graphics/src/DepthSurface.cpp L44
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp L832-838
#2490 core/libs/prx/libSceAgcDriver/Graphics/src/DepthSurface.cpp L44
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp L832-838
#2491 core/libs/prx/libSceAgcDriver/Graphics/src/DepthSurface.cpp L44
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp L832-838
#2904 core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp L833-836
#3846 core/libs/prx/libSceAgcDriver/Graphics/src/Pipeline.cpp L290-291
#4029 core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp L4

@adriadam10
adriadam10 merged commit 9093395 into boykopovar:main Oct 11, 2026
14 checks passed
@MichonGoddijn231849

Copy link
Copy Markdown
Contributor Author

@billythemenace2 thanks, that reading is right: the guard ranges only go away at device teardown, so once Unity hands a former 2x target's memory to another resource, every read of it throws. With #419 merged that is on main now, so I'll do the follow-up PR: a multisampled target is retired (its guard range and host image) when its memory is written as something else, for example a single-sample color or depth target bound over it, a fill or DMA_DATA, or a CPU write the write tracking sees. The guard then throws only for reads of bytes the multisampled target still owns.

It would help to know which of those wrote the 1280x431 texture in PPSA06888. If you can add a print, the cheapest place to start is the single-sample color/depth target setup over the range: that's the hook I'd put first.

The sample-location stops (New Joe & Mac, PPSA06888) are #2169's path, as you found.

@xXPBSXx

xXPBSXx commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Source for how another PS5 emulator treats the PA_SC_AA_SAMPLE_LOCS_* registers (addition to my comment above on the registers and Mesa): KytyPS5 stores the 16 registers 0x2FE..0x30D (pm4.h) as locations[16] of a context struct (HwCtxSetAaSampleControl in pm4Handlers.cpp, which also takes them as a 20-dword block together with PA_SC_CENTROID_PRIORITY_0/1), and renderer/debug.cpp (AaCheck) does not render with them: for any non-zero location, non-zero centroid priority or sample count it only logs, at most 16 times, warning: unsupported PS5 sample locations use native Vulkan locations: samples=..., exposed=..., max_dist=.... So Kyty's behavior is to ignore the guest locations and use Vulkan's native ones with a warning. That is a precedent, not a statement about what is correct for New Joe & Mac (its 2-sample locations are the mirrored diagonal from my earlier comment); if this PR or a follow-up wanted the same tolerance, it would be a deliberate approximation and belongs in TechnicalDebt.md, not a silent accept. shadPS4 only has the register names in its devtools (gcn_context_regs.cpp), no handling.

AI-assisted (Claude Code).

@xXPBSXx

xXPBSXx commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Summary of the registers behind nonstandard sample locations are unsupported, with the origin of each statement:

  • PA_SC_AA_SAMPLE_LOCS_PIXEL_X0Y0_0 .. X1Y1_3 = 0x2fe .. 0x30d (16 registers; four per pixel of the 2x2 quad; each register has four samples as 4-bit signed x/y fields in 1/16 pixel). Origin: Linux gc_10_1_0_offset.h / gc_10_1_0_sh_mask.h (non-PS5). KytyPS5 (pm4.h) uses the same block of 16 starting at 0x2FE.
  • PA_SC_CENTROID_PRIORITY_0/1 = 0x2f5 / 0x2f6 (same Linux headers, 8 four-bit DISTANCE_n fields per register). Mesa radv_cmd_buffer.c (radv_compute_centroid_priority) derives them from the sample locations: it sorts the samples by squared distance from the pixel center and writes the order. They only affect which sample is used for centroid-interpolated inputs, not coverage.
  • Mesa radv_device.c has the standard tables (2x: (4,4), (-4,-4)); the title here programs (-4,+4), (+4,-4).
  • KytyPS5 ignores the guest locations and uses Vulkan's native ones with a log line (renderer/debug.cpp, AaCheck); shadPS4 only names the registers (gcn_context_regs.cpp).

What a minimum treatment could be, as far as these sources support it: (1) keep rejecting unless the 16 location registers equal a standard table, as this PR does; (2) if an approximation is accepted, accept the mirrored 2x pattern only, explicitly, and list in TechnicalDebt.md that the host uses Vulkan's native positions (same trade-off as Kyty), because with equal-weight resolves the sample positions change only where geometry edges fall within the pixel, and with centroid inputs; the sources do not say how visible this is on the console. I would not accept arbitrary locations silently. No code from me.

AI-assisted (Claude Code).

@xXPBSXx

xXPBSXx commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Update for New Joe & Mac: Caveman Ninja (PPSA02801), the title that stopped on nonstandard sample locations are unsupported in my earlier comment: on current main e7c70a2 + #425 + #4408 + #3840 (plus local diagnostic prints, not committed), relinked with 0 absent imports, Windows, RTX 4080 SUPER, 240 s limit: no stop. The run reached the time limit; apart from my diagnostic lines the log has only the [SAVEDATA:native] memory calls and the usual start-up lines, no presentation failed, no FATAL. The log shows thousands of indexed draws and dword fills, so the title is drawing. I have not looked at the image. The sample-locations stop is therefore gone with the changes that landed after this PR (custom sample locations from #2169 and #4309), at least for this title.

AI-assisted (Claude Code).

@KingDarkHN

Copy link
Copy Markdown

Following up on the retire-on-write idea from #4199/#1189: I implemented and tested it locally against Fallen Legion (PPSA07645), and wanted to share the results.

Implementation

  • A write guard mirroring the read guard (SetWriteGuard) that runs before every store that flows through storeOwn (the innermost guest store: the game's memory-operation shims and the driver's own writes).
  • The same retire happens for GPU-side writes that bypass storeOwn: DMA_DATA destinations, WRITE_DATA/COPY_DATA stores (ResolveStore) and buffer fills.
  • On overlap, the multisampled target is retired: the host image and its guard are dropped (the samples live host-side; the guest has overwritten the memory, so the target is dead).
  • Added a recorder test: a store over the target's memory retires it, so subsequent CPU reads and the DMA guard no longer refuse.

Result in the game. Fallen Legion aborts on a real macro that reuses a multisampled color target's memory after a resolve (the same 4x 3840x2160 target that #2121's resolve involves). With this change the store now retires that target instead of leaving a stale guard that crashes the next frame:

[agc] multisampled color target 0x2270d0000+0x870000 retired after a guest write of 0x2271561e8+0x4 reused its memory

One observation for the follow-up. The remaining overlapping reads in the title do not have a tracked preceding write: parts of the reuse come from GPU shader stores (or writes that happen before the target/guard exists). Retiring on writes alone therefore does not fully un-hang the game. Since true MSAA sampling/readback is not implemented, refusing those reads only ever aborts the title; letting them read the (now foreign) memory would turn a hard crash into a minor visual artifact while a real read-side test proves the region was repurposed.

Happy to turn the write-guard core + test into a PR if useful, or to adjust the read-side behavior to match what you prefer.

MichonGoddijn231849 added a commit to MichonGoddijn231849/AnyPS5-fork that referenced this pull request Oct 13, 2026
With APS5_PIPELINE_DMA (on by default since the previous commit) a DMA_DATA was enqueued before the
packet path's metadata notes ran, so a fill of 0 over a depth surface's HTILE (boykopovar#374) or a color
target's CMASK (boykopovar#419) never reached the surface: agc_driver_depth_fast_clear_submit failed. The
queued commit now notes the fill in order before it stores it, and a DMA copy is checked against
multisampled targets at enqueue, as on the packet path.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
MichonGoddijn231849 added a commit to MichonGoddijn231849/AnyPS5-fork that referenced this pull request Oct 13, 2026
With APS5_PIPELINE_DMA (on by default since the previous commit) a DMA_DATA was enqueued before the
packet path's metadata notes ran, so a fill of 0 over a depth surface's HTILE (boykopovar#374) or a color
target's CMASK (boykopovar#419) never reached the surface: agc_driver_depth_fast_clear_submit failed. The
queued commit now notes the fill in order before it stores it, and a DMA copy is checked against
multisampled targets at enqueue, as on the packet path.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
JJenkx pushed a commit to JJenkx/AnyPS5 that referenced this pull request Oct 13, 2026
With APS5_PIPELINE_DMA (on by default since the previous commit) a DMA_DATA was enqueued before the
packet path's metadata notes ran, so a fill of 0 over a depth surface's HTILE (boykopovar#374) or a color
target's CMASK (boykopovar#419) never reached the surface: agc_driver_depth_fast_clear_submit failed. The
queued commit now notes the fill in order before it stores it, and a DMA copy is checked against
multisampled targets at enqueue, as on the packet path.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
adriadam10 pushed a commit to MichonGoddijn231849/AnyPS5-fork that referenced this pull request Oct 15, 2026
With APS5_PIPELINE_DMA=1 a DMA_DATA was enqueued before the
packet path's metadata notes ran, so a fill of 0 over a depth surface's HTILE (boykopovar#374) or a color
target's CMASK (boykopovar#419) never reached the surface: agc_driver_depth_fast_clear_submit failed. The
queued commit now notes the fill in order before it stores it, and a DMA copy is checked against
multisampled targets at enqueue, as on the packet path.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
(cherry picked from commit fda7dbf)
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.