Repository navigation
fix(agc): rasterize a single-sample scan into a multisampled depth target at the pixel center - #5915
Conversation
|
Real-title check for the Peace Walker case. METAL GEAR SOLID: Peace Walker (PPSA18216), Linux, RTX 3080 Ti (NVIDIA 615.71.09), AI-assisted; the runs are from my machine. |
|
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.
|
|
Checked, as asked in the description. Peace Walker (PPSA18216), Linux, RTX 3080 Ti (NVIDIA 615.71.09),
AI-assisted; the runs are from my machine. |
|
Overlap note: this PR and #5907 (mine) both edit the depth sample-count check in They handle opposite cases:
They combine without changing either: keep #5907's Require(!result.depth || depthKeepsSampleZero || result.depth->samples == result.samples, "the depth target's sample count differs from the rasterizer's");I ran that combination with Onimusha: Way of the Sword (PPSA27836) on |
|
@simonbronk Thanks, agreed! I'd come to the same resolution. I rebuilt it on current The debt file conflicts too. #5907 rewrites the "Custom MSAA sample locations" entry and this PR adds its entry right after it; keeping both lines resolves it. With that, |
|
Re-checked on the PR merged onto current |
|
@adriadam10 Thanks for the re-checks! In case it helps break the loop: GitHub shows no merge queue for |
2c8dda1 to
4355eb4
Compare
|
Rebased onto main f474fb7 now that #5907 is in, as agreed with @simonbronk: #5907’s depthKeepsSampleZero path runs first, then this PR’s single-sample-scan path, then one sample-count check that accepts both. agc_driver_graphics passes 3 of 3 on an RTX 3050 Ti and on lavapipe, and the new tests still fail with main’s State.cpp. The debt file keeps both entries. |
|
Re-checked the rebased head (4355eb4) on |
…rget at the pixel center A draw with PA_SC_AA_CONFIG.MSAA_NUM_SAMPLES 0 and MSAA_ENABLE clear into a 4x depth target threw "the depth target's sample count differs from the rasterizer's", which stops METAL GEAR SOLID: Peace Walker (PPSA18216) at its first presentation. Such a draw now rasterizes the depth target's sample count with every sample at the pixel center, as a single-sample scan into a multisampled color target already does. radeonsi programs this MSAA_ENABLE and PA_SC_AA_CONFIG with the depth buffer's own sample count when GL multisampling is disabled, and GL then gives full coverage with every depth and stencil sample set from the pixel center. Triangles and rectangles are accepted. Points, lines, alpha-to-coverage, DB_EQAA Z interpolation or overrasterization, a pixel program reading the sample index and a sample-mask export throw on this path, as does a single-sample color target next to the multisampled depth target. The primitive check is shared with the single-sample MSAA_ENABLE rule, whose behaviour and messages are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The debt entry cited ARB_multisample's centermost-sample shortcut as what a single-sample scan into a multisampled depth target draws. The draw tests each depth and stencil sample with the depth at the pixel center, which is the rule above that shortcut (L185-L190 and L422-L425); the shortcut is now named as an unmeasured alternative. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Rebased onto current |
4355eb4 to
237fe9c
Compare
What
METAL GEAR SOLID: Peace Walker (PPSA18216) stops at its first presentation with:
mrfelek reported it on #5268, on
main69e7138 with #5422 and #5439, and posted the registers of that draw (draw A):PA_SC_AA_CONFIGPA_SC_MODE_CNTL_0MSAA_ENABLEclear)DB_Z_INFODB_EQAADB_DEPTH_CONTROLThis is the depth version of #4414. #4414 rasterizes a single-sample scan into a multisampled color target with every sample at the pixel center, but it leaves out draws with a depth target. Such a draw now rasterizes the depth target's sample count with every sample at the pixel center, using the custom sample locations from #2169. A 4x color target next to the depth target works the same way.
radeonsi programs this state when GL multisampling is disabled on a multisampled framebuffer:
MSAA_ENABLEfollows the GL multisample state (si_state.cL1066-L1068).PA_SC_AA_CONFIGstays 0, and the depth buffer keeps its own sample count (L3485-L3512).Two differences from that state:
DB_EQAA.MAX_ANCHOR_SAMPLESto the Z sample count, where the game setsDB_EQAAto 0. This change does not depend onMAX_ANCHOR_SAMPLES.ARB_multisample defines this state: rasterization is single-sample with full coverage, and every sample may take the single-sample depth (L185-L190), with the fragment treated as in multisampling with full coverage (L422-L425). Every sample at the pixel center gives exactly that: each depth and stencil sample is tested and written with the depth at the pixel center. GL also lets an implementation test only a "centermost sample" and set every depth and stencil sample from it (L425-L434), which differs when a pixel's stored samples differ.
Triangles and rectangles are accepted; draw A's depth and stencil fill may well be a rect list. On this path, these throw, most with a new message:
DB_EQAAZ interpolation or overrasterization (bits 18-19 and 24-27). These could change the per-sample depth;ANCILLARY). Vulkan would run it once per sample, with an index the console does not give. Sample-interpolated inputs are accepted, because the recompiler reads them through barycentrics, which give the center value for every sample here.#4414's color-only path does not refuse these, and this PR leaves it unchanged. The primitive check moves out of
singleSampleMsaaRejectioninto a helper that both rules use; that rule's behaviour and messages are unchanged.Not measured on hardware, and recorded in the new debt entry:
Tested
Ubuntu 24.04, GCC 13.3, Release with
BUILD_TESTING=ON, onmain84b01f9. Locally, the libusb lines were removed fromCMakeLists.txt, because the libusb submodule isn't checked out here.agc_driver_graphics(tests/Graphics.cpp) gainsmultisampledDepthTests. Unless stated, each case usesPA_SC_AA_CONFIG0,MSAA_ENABLEclear and a 4x Z32 depth target.These now decode, as 4 samples with every location at the pixel center, no sample-location grid and no per-sample shading:
DB_Z_INFO0xa290098b,DB_DEPTH_CONTROL0x777,DB_EQAA0, no color target). The draw precheck accepts it;ANCILLARYinput, when the draw skips its pixel program;PS_ITER_SAMPLEdoes not turn on sample shading.These now throw:
DB_EQAAZ interpolation;ANCILLARYinput with a pixel program;Pinned unchanged (these pass on
maintoo):MSAA_ENABLEset, with sample 0 at the center;On
mainthe test fails at draw A with the message above. With the change it passes, 5 runs out of 5. The existing single-sampleMSAA_ENABLEchecks still pass, inagc_driver_graphicsandagc_driver_msaa_enable_single_sample.Other checks:
cteston the RTX 3050 Ti: 1,159 of 1,161 pass, 12 of them skipping themselves:agc_driver_stencil_ref_exporton this GPU, and the VideoOut flip tests, which need a display. The other 2,patched_librariesandguest_usbd, are Not Run because the libusb submodule isn't checked out. All 401agc_drivertests pass apart from that skip.PA_SC_AA_CONFIGwithMSAA_ENABLEclear, over a 4x depth and a 4x color target. fix(agc): rasterize a multisampled target without MSAA_ENABLE at the pixel center #5882 (merged) handles that state without a depth target, and fix(agc): rasterize a multisampled depth target without MSAA_ENABLE at the pixel center #6014 adds the depth target.PA_SC_AA_CONFIG, handled by fix(agc): rasterize into one-sample targets as one sample when MSAA_ENABLE is clear #6196.State.cppedit.mainf474fb7 after feat(agc): rasterize a multisampled draw into one-sample targets at sample 0's locations #5907 merged. On that head,agc_driver_graphicspasses 3 runs out of 3 on the RTX 3050 Ti and on lavapipe. It still fails withmain'sState.cpp(the depth target's sample count differs from the rasterizer's).MSAA_ENABLEset, drawing into one-sample targets. It rewrote the same depth sample-count check, so this PR is rebased onto it: feat(agc): rasterize a multisampled draw into one-sample targets at sample 0's locations #5907'sdepthKeepsSampleZeropath runs first, then this PR's single-sample-scan path, then one sample-count check that accepts both. The two rules don't overlap: this one needs a single-sample rasterizer withMSAA_ENABLEclear, and feat(agc): rasterize a multisampled draw into one-sample targets at sample 0's locations #5907 needs a one-sample depth target. The debt file keeps feat(agc): rasterize a multisampled draw into one-sample targets at sample 0's locations #5907'sCustom MSAA sample locationsentry, with this PR's entry after it.tools/check_conventions.pyfrommainpasses with this title and body.@mrfelek, thanks for the registers and the runs.
Checklist
main; no other open PR implements the same functionsNotImplemented_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)ctest; the new checks are host-side decode tests)🤖 Generated with Claude Code