Repository navigation
fix(agc): require full coverage only for metadata pass targets with CMASK or DCC keys - #9694
Conversation
…MASK or DCC keys
A CB fast-clear eliminate or DCC decompress pass whose viewport or scissor
did not cover a color target was refused ("CB metadata pass over part of a
color target") even when that target had no CMASK and no DCC keys. Such a
pass leaves the target's texels unchanged: RunColorMetadataPass touches a
target only through its CMASK clear or its DCC keys, so for a target with
neither, the covered area does not matter.
Marvel's Guardians of the Galaxy (PPSA01748) issues fast-clear eliminates
over three 1920x1080 targets without CMASK or DCC while PA_SC_MODE_CNTL_0
still enables a 480x270 viewport scissor left from an earlier pass, and
stopped in its first level on that refusal.
The coverage check now applies only to targets with a CMASK or DCC keys;
FMASK decompress (which requires a CMASK) and resolves are unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MarcZX10
left a comment
There was a problem hiding this comment.
The one-line relaxation is consistent with RunColorMetadataPass: targets with neither CMASK nor DCC metadata have no metadata work to perform, while the full-coverage refusal remains for targets whose metadata may be changed. The added negative assertions retain the refusal for keyed targets. I ran the current-main conventions check and git diff --check; both passed. A pairwise merge-tree with open #9597 is clean; that PR changes mipmapped DCC target decoding in a separate area, not this coverage condition.
Two repository requirements still need attention before approval:
- This changes the AGC driver. CONTRIBUTING requires the same focused test(s) on
mainand the PR withVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation,ANYPS5_NO_SHADER_CACHE=1andANYPS5_REQUIRE_VULKAN=1, usingctest -V, plus confirmation the layer was loaded and no newVUID-errors were introduced. The report includes the focused tests and GPU, but not those validation-layer runs or the main/change VUID comparison. Please attach both logs/results. The hosted product jobs are skipped on this PR; I could not run the AGC target in this checkout. - The checklist says
Unblocks: PPSA01748 (first level after New Game), but the repo requires the Game bring-up discussion number as well. I could not find a Game bring-up discussion for PPSA01748; #8991 is a merged PR, not that discussion. Please create or locate the title's Game bring-up discussion, link it on this line, and keep its open-PR/blocker summary current.
I found no code defect in the changed condition. These requests are for the required AGC validation evidence and title-tracking checklist.
|
@MarcZX10 thanks for the review. Both items: 1. Validation-layer runs. Same machine as the description (RTX 4090, NVIDIA 615.78.08, Linux).
The one VUID on both sides is at device creation and comes before any of this PR's code runs: Loader and result lines (PR run)2. Game bring-up discussion. There wasn't one for PPSA01748, so I opened it: #9704. The checklist line now links it. Tested and written by Claude (AI assistant) on a contributor's machine. |
|
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.
|
MarcZX10
left a comment
There was a problem hiding this comment.
Thanks for adding the title's Game bring-up discussion and the focused validation-layer comparison. The new report documents same-machine runs on main and this PR, confirms the requested layer was loaded for both GPU tests, and shows no new VUIDs. The checklist now links PPSA01748 to #9704, whose current blocker section records the remaining AMD/RADV issue separately from this AnyPS5 fix. I independently rechecked the exact head: current-main conventions pass, is clean, and a merge-tree with current main is clean. I did not rerun the AGC binaries locally because this checkout has no configured AGC build and the filesystem is full; I am relying on the clearly attributed contributor-machine results for those runs. I found no remaining blocker in the diff or required validation evidence.
MarcZX10
left a comment
There was a problem hiding this comment.
The requested title tracking and validation evidence are now present. I reviewed the code path and focused test additions, verified conventions, diff cleanliness and current-main mergeability, and found no remaining blocker.
|
A Windows run of this head: Windows 11 Home 10.0.26200, WinLibs GCC 15.2.0 r7 (MinGW-w64 UCRT), Release.
Not run: This PR and #9604 both change AI-assisted: yes (Claude Code) |
|
First AMD run, and first NVIDIA-on-Windows run, of the device metadata-pass tests. Windows 11 10.0.26300, WinLibs GCC 15.2.0, Release, full build; this head
AI-assisted: yes (Claude Code); results above were run on this machine. |
Keep boykopovar#9694's coverage check for targets without CMASK or DCC keys and skip it only for a fast-clear eliminate over a CMASK target without DCC keys. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What
With #8991, Marvel's Guardians of the Galaxy (PPSA01748) gets past its menus. After New Game and the first level load, it stops in presentation on:
Cause: the pass is a fast-clear eliminate (
CB_COLOR_CONTROLmode 2) over three 1920x1080 color targets that have no CMASK and no DCC keys. The viewport covers the targets (xs/xo = 960/960,ys/yo = 540/540), butPA_SC_MODE_CNTL_0still enables the viewport scissor, andPA_SC_VPORT_SCISSOR_0is left at 480x270 from an earlier pass.DecodeColorMetadataPassrequires every target to be covered, so it refuses. For these targets the pass changes nothing:RunColorMetadataPasstouches a target only through its CMASK clear or its DCC keys.Fix: the coverage check applies only to targets with a CMASK or DCC keys. A partial pass over such a target is still refused as before, and FMASK decompress (which requires a CMASK) and resolves are unchanged.
Tested
Linux (CachyOS, kernel 7.2.9), GCC 16.2.1, Release,
BUILD_TESTING=ON, RTX 4090 (NVIDIA 615.78.08).metadataPassTests(agc_driver_graphics): the two partial-coverage cases (a scissor and a viewport that miss part of the target) now first check that a target without CMASK or DCC keys decodes with its target. Then they add DCC keys to the same state and require the refusal as before.State.cpprestored tomain,agc_driver_graphicsfails on the new expectation. With the change,agc_driver_graphicsandagc_driver_graphics_maps_fallbackpass.ctest -R '^agc_'(866 tests): 862 pass and 1 skips. The threeagc_driver_sample_order*tests fail ("2x scene with the standard locations: the slopes did not leave depth both at and below the clear value"), and they fail the same way onmaind6eafa5 on this machine.python3 tools/check_conventions.py --base origin/main: passes.main6830c91 plus fix(recompiler): match image-table T# halves offset before and after readfirstlane #8991's change, one run): stops on the refusal above at the end of the first-level load.cmask=0,dcc=0under the 480x270 viewport scissor.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)Tested and written by Claude (AI assistant) on a contributor's machine.