Repository navigation
fix(agc): accept ANCILLARY without PS_ITER_SAMPLE for a one-sample color target without MSAA_ENABLE - #13991
Open
JJenkx wants to merge 2 commits into
Open
fix(agc): accept ANCILLARY without PS_ITER_SAMPLE for a one-sample color target without MSAA_ENABLE#13991JJenkx wants to merge 2 commits into
JJenkx wants to merge 2 commits into
Conversation
…lor target without MSAA_ENABLE Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
added
unblocks title
MarcZX10
approved these changes
Oct 31, 2026
MarcZX10
left a comment
MarcZX10
left a comment
Contributor
There was a problem hiding this comment.
Reviewed — looks good
The exact-head code and regression cases are consistent with the reported Linux validation, and the applicable hosted checks pass.
What I ran
Reviewed head d9506e308ff0f932b413fd13f244693aa70f3e6d against base 73f4f7e06e4f210a8c6570e335fd654dc0a86134; no build or merge-preview was run. Environment: Debian Linux 6.1 x86_64, Python 3.11.2; no C++ compiler invocation.
git diff --check 73f4f7e06e4f210a8c6570e335fd654dc0a86134...d9506e308ff0f932b413fd13f244693aa70f3e6d
gh pr checks 13991 --repo boykopovar/AnyPS5I also inspected the exact-head State.cpp and Graphics.cpp diff and the PR checklist. The author reports same-machine Linux main/change CTest runs and a PPSA14408 title run; those runs were not performed by me.
Results
| Check | Result |
|---|---|
git diff --check |
Passed; no output |
| Windows CI | Passed |
| Conventions, eligibility, intake, labels, library portability, report | Passed |
| Linux and specialized product jobs | Skipped by workflow |
| Source/test inspection | The non-PS_ITER_SAMPLE ANCILLARY case is accepted, the PS_ITER_SAMPLE case remains rejected, and coverage/sample-mask cases remain rejected. |
| Author-reported matched Linux CTest and title run | 3060 tests on each head with the same one failure and same 47 skips; validation output reported identical. PPSA14408 reaches the Bluepoint logo before a later NGS2 refusal. Not independently run. |
Unedited local diff-check output
Unedited hosted check summary
windows pass 6m7s https://github.com/boykopovar/AnyPS5/actions/runs/41210557699/job/124564472729
linux skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/41210557699/job/124564473863
register-context skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/41210557699/job/124564473962
relinker skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/41210557699/job/124564473679
relinker-asan skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/41210557699/job/124564474325
write-watch skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/41210557699/job/124564473980
conventions pass 17s https://github.com/boykopovar/AnyPS5/actions/runs/41210557447/job/124564448916
eligibility pass 4s https://github.com/boykopovar/AnyPS5/actions/runs/41210557699/job/124564448089
intake pass 4s https://github.com/boykopovar/AnyPS5/actions/runs/41210558048/job/124564450280
labels pass 9s https://github.com/boykopovar/AnyPS5/actions/runs/41210557498/job/124564448843
library-portability pass 1m23s https://github.com/boykopovar/AnyPS5/actions/runs/41210557562/job/124564447545
report pass 1m5s https://github.com/boykopovar/AnyPS5/actions/runs/41210557495/job/124564447253
Negative control
The author reports that the focused test fails with the base implementation and passes with this change; I did not independently run that control.
Findings
Blocking
None found in the reviewed diff.
Non-blocking
The console behavior of ANCILLARY's sample index without PS_ITER_SAMPLE remains an inference, as the PR states. The title run establishes progress through the first frames, not full game compatibility.
Sources
Not verified
I did not build AnyPS5 or run CTest locally, test other operating systems or GPUs, or verify behavior on a physical PS5. The Linux and title results above are author-reported; product Linux and specialized hosted jobs were skipped.
|
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.
|
Contributor
✅ NVIDIA GPU test: passedPR head
Unstable on main (2, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (4, not counted against this PR)Validation IDs already present on main (4 IDs in 387 tests)Skipped (48)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
Contributor
|
Windows result for head Host: Windows 11 Pro 26200, i7-9750H, WinLibs GCC 15.2.0 (
Only this test target was built, not the full suite, and no title was run. AI-assisted. |
github-actions
Bot
added
review queue
XairRrR
approved these changes
Nov 2, 2026
XairRrR
left a comment
XairRrR
left a comment
Contributor
There was a problem hiding this comment.
Reviewed — approved
The change gives one-sample color targets the rule #9988 gave one-sample depth targets: ANCILLARY without PS_ITER_SAMPLE is accepted. The code matches the depth path, the test pins both the new acceptance and the refusals that remain, and the negative control fails on the old refusal.
What I ran
Head d9506e308ff0f932b413fd13f244693aa70f3e6d, main 9090972b7e91fd266819cbd77a8b058e177b5682, merged with --no-ff into tree ea394590bb7434fec715354c4083a84b9406f4d9. Debian 13 VM, GCC 14.2.0, Release, Vulkan through lavapipe (Mesa 26.1.6).
git diff --check 9090972b7e91fd266819cbd77a8b058e177b5682...d9506e308ff0f932b413fd13f244693aa70f3e6d
python3 tools/check_conventions.py --base 9090972b7e91fd266819cbd77a8b058e177b5682
ctest --test-dir build -R '^agc_driver_graphics$' --output-on-failure
# negative control
git show 9090972b7e91fd266819cbd77a8b058e177b5682:core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp > core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp
ninja -C build tests/agc_driver_graphics_tests
ctest --test-dir build -R '^agc_driver_graphics$' --output-on-failure
git show ea394590bb7434fec715354c4083a84b9406f4d9:core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp > core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp
ninja -C build tests/agc_driver_graphics_tests
ctest --test-dir build -R '^agc_driver_graphics$'
git status --porcelainResults
| Check | Result |
|---|---|
git diff --check |
Clean |
check_conventions.py |
0 errors. 1 debt-length warning on the edited entry (4335 characters); the entry was already well over 1000 characters on main |
| Build (script) | Configure, build and libs OK |
| Selected tests (script) | 2031 run, 1913 passed, 111 expected skips, 0 new failures, 0 timeouts. 7 known failures that also fail on main (agc_driver_division, the denormal tests, unwrap_self, vector_constant_factor_tininess) |
| Validation (script) | 757 tests, layer loaded in 756 (none listed as missing), 0 tests with new VUID- |
agc_driver_graphics on the merged tree |
Passed (34.5 s; 35.3 s after the restore) |
| Negative control | Failed as expected (see below) |
git status --porcelain after the restore |
Empty |
Negative control
I restored only core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp from main, kept this PR's test, rebuilt and ran agc_driver_graphics. It failed with the refusal this PR narrows:
Output
All 256 MSAA CMASK byte classifications passed
AGC graphics: a one-sample color target under a multisampled rasterizer without MSAA_ENABLE is unsupported with a sample index or coverage input, or a sample-mask export
0% tests passed, 1 tests failed out of 1
After restoring the merged file, the test passes again and the worktree matches the merged tree. This confirms @jacknieland's red/green result on Windows, now on Linux with GCC 14.
Code review
core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp:1686-1688uses the samesampleIndexmask as the depth path atState.cpp:1587-1589. SoANCILLARYis refused only withPA_SC_MODE_CNTL_1.PS_ITER_SAMPLE, andSAMPLE_COVERAGEand theDB_SHADER_CONTROLsample-mask export still throw. The two paths stay consistent.- The claim that the sample index reads as 0 holds.
core/shader/recompiler/Optimization/src/ConstantFolder.cpp:815lowers the packed-ancillary sample field toConstant(0)unlesspixelIteratesSamplesis set. HerePS_ITER_SAMPLEis clear, so the recompiler gives sample 0, which is the sample the one-sample draw keeps. perSampleShadingis computed atState.cpp:1608before the color loop, and the branch forces it to false atState.cpp:1691. The test checks this attests/Graphics.cpp:854.- The test keeps the refusals for
SAMPLE_COVERAGE(tests/Graphics.cpp:855-857), forANCILLARYunderPS_ITER_SAMPLE(tests/Graphics.cpp:863-864) and for the sample-mask export (tests/Graphics.cpp:865-868). All of them match the new message. - The behaviour is general, not tied to one title, and no code comments were added. The debt entry is updated: the depth-only qualifier is removed from both the throw rule and the sample-index inference, and PPSA14408 is named.
- The checklist lines (Depends on, Fixes, Unblocks, GPUs tested, AI-assisted) are present. The description's Not verified section correctly says the console's sample index is inferred, as in #9988.
Findings
Blocking
None.
Non-blocking
agc_driver_graphicstook 34.5–35.3 s here, over the 30 s budget. TechLevelZero's NVIDIA CI measured 30.4 s for it. This PR only adds oneDecodeStatecall, so the time is not caused by the PR, and I did not timemainon this machine. The test should be split separately.check_conventions.pywarns that the "A one-sample target under a multisampled rasterizer without MSAA_ENABLE" entry indocs/dev/debt/libSceAgcDriver.md:79is 4335 characters long. The entry was already far over the limit onmain, and this PR adds one sentence, so this is not a reason to hold the PR.
Not verified
- The console's ancillary sample index on this path. It is inferred, not measured, as the PR states.
- The PPSA14408 title run. I did not launch titles.
- Behaviour on hardware GPUs. I ran on lavapipe only; the author reports RADV, and others report NVIDIA (RTX 2000 Ada, GTX 1650).
github-actions
Bot
added
review queue
github-actions
Bot
added
the
review queue
|
Windows and NVIDIA Blackwell run, for head
Both failures fail on Timing: Not run: the Khronos validation layer is not installed on this machine, so this says nothing about VUIDs. PPSA14408 was not run. AI-assisted: yes. Claude Code ran the builds and tests and drafted this comment; the numbers are from the runs on this machine. |
Contributor
Author
|
Still needed on current This PR removes that refusal. With this PR and #15538, the same title ran to the scheduled close of a 300 s run, as reported on #15538 and in discussion #8737. This PR's head AI-assisted: yes |
Collaborator
|
Hi I pushed one small commit on top of your branch: it removes the SAMPLE_COVERAGE |
Contributor
✅ NVIDIA GPU test: passedPR head
Failing on main too (1, not counted against this PR)Validation IDs already present on main (13 IDs in 548 tests)Skipped (48)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
Contributor
✅ AMD GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (7, not counted against this PR)Validation IDs already present on main (4 IDs in 556 tests)Skipped (58)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
Contributor
Author
|
Current-main check of this PR on Metal Gear / Metal Gear 2 (PPSA14408), Linux, RX 6650 XT (RADV), Release. Tree: main
AI-assisted: yes |
12 tasks done
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Metal Gear 1 in Metal Gear & Metal Gear 2: Solid Snake (PPSA14408) stops before its first frame:
The draw has these registers:
PA_SC_AA_CONFIG0x30e003: eight samples.PA_SC_MODE_CNTL_00x2:MSAA_ENABLEis clear.SPI_PS_INPUT_ENAandSPI_PS_INPUT_ADDR0x2002:PERSP_CENTERandANCILLARY.DB_SHADER_CONTROL0x50: no sample-mask export.The only refused input is
ANCILLARY.#9988 accepted
ANCILLARYwithoutPS_ITER_SAMPLEfor a one-sample depth target on this path, and left the one-sample color rule unchanged. This PR applies the same rule to one-sample color targets.ANCILLARYnow throws on this path only withPA_SC_MODE_CNTL_1.PS_ITER_SAMPLE.SAMPLE_COVERAGEand a sample-mask export still throw. #9988's reasoning carries over:PS_ITER_SAMPLEthe guest does not iterate per sample, and the recompiler already reads the sample index as 0. The one-sample draw keeps sample 0 at the pixel center.PS_ITER_SAMPLEthe console runs one invocation per sample, each with its own index, so that case still throws.The debt entry "A one-sample target under a multisampled rasterizer without MSAA_ENABLE" now states the rule for both target kinds and names the title.
Tested
Linux 7.2.11, Ryzen 9 7950X, Radeon RX 6650 XT, RADV Mesa 26.2.6, Release.
agc_driver_graphics: a one-sample color draw whose program readsANCILLARYwithoutPS_ITER_SAMPLEis drawn as one sample without sample shading.SAMPLE_COVERAGE,ANCILLARYunderPS_ITER_SAMPLEand a sample-mask export still throw, with the new message.main's rule inState.cppand this PR's test,agc_driver_graphicsfails on the old refusal. With the change it passes.ctest -j8, onmain73f4f7e06eand on this change (d9506e308f, on the same base). Both ran withVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation,ANYPS5_REQUIRE_VULKAN=1andANYPS5_NO_SHADER_CACHE=1. Results:agc_idle_publish_wait("the idle publish's bytes are wrong"), which also fails onmain.VUID-VkPipelineColorBlendStateCreateInfo-pAttachments-00605errors, and the same warnings by ID and count.PPSA14408, natively, content version 01.500.000. The launcher starts MG1 through
sceSystemServiceLoadExec(/app0/PS/eboot.bin -mgst mg1 …). Relinked with--module-dir Media/Modules --module-dir Media/Plugins --module-dir . --executable PS/eboot.bin. Run withAPS5_PLAYGO_UNLISTED_CHUNKS=installed.main60bdb06135d9506e308f)NGS2: waveform block 0 does not fit its data.The DB depth copy that stopped MG1 in #8737 is no longer reached as a stop on
main.Independent reports on head
d9506e308f(their runs, summarised here; see their comments):agc_driver_graphicspasses on this PR and onmainwith 0VUID-lines. This PR's test onmain'sState.cppfails with the refusal it narrows. Only that target was built; no title was run.main73f4f7e: 2,334 passed, 0 new failures, withVK_LAYER_KHRONOS_validation.Not verified
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)🤖 Generated with Claude Code