Repository navigation
Conversation
github-actions
Bot
added
the
unblocks title
|
Tested on AMD Ryzen 7 9800X3D and AMD Radeon RX 7900 XTX (Fedora Linux, Mesa 26.2.5):
|
12 tasks done
Merged
12 tasks done
|
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.
|
jotitadev
reviewed
Oct 22, 2026
jotitadev
left a comment
jotitadev
left a comment
Contributor
There was a problem hiding this comment.
Apple GPU run of 22766ab: the new test passes on MoltenVK and fails without the change.
Setup: Apple M2 Max, macOS 27.2, x86-64 under Rosetta 2, Release. Base is #8658 (feat/macos-port) @ f48446a (main plus the macOS port), not main; your diff from 7e2ba73 applies cleanly on it. Two drivers: MoltenVK 1.4.2 (Vulkan SDK 1.4.363) and MoltenVK main + SPIRV-Cross #2704. ANYPS5_NO_SHADER_CACHE=1, -j1 --timeout 30.
- This PR:
agc_driver_eqaa_color_targetpasses 3 of 3 on both drivers:4-sample 2-fragment draws resolve like 4x MSAA (262144 bytes), 410 quarter-coverage channels, FMASK guarded. That is the same 410 channels as on your 4090, so Metal's 4x sample positions give the same edge coverage. Your focused set (32 ctest entries:agc_driver_graphics, the new test, and the MSAA, FMASK, CMASK and journal tests) with--repeat until-fail:3: the only failures aremultisample_retirementandsample_mask_multisample. Both fail the same way on the base without this PR. - Test only, production files at the base (new
EqaaColorTarget.cppand its CMake line): fails on both drivers withAGC graphics: color targets with fewer fragments than samples (EQAA) are unsupported, the same as yourmaincontrol. - Full
^agc(857 entries, without flip/display/lds_atomics64): MoltenVK main + #2704 gives exactly the base's 16 failures. MoltenVK 1.4.2 gives the base's 17, plusagc_driver_uint8x4_color_target, which is intermittent on Apple without this PR too (it fails in 19 of 150 runs on the base). No new failures. - The
sizeof(State) == 792assert is under__linux__ && __GLIBCXX__, so it doesn't apply to the macOS libc++ build. The macOS build compiles with no errors.
GPUs tested: Apple M2 Max (MoltenVK 1.4.2; MoltenVK main + SPIRV-Cross #2704).
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
12 tasks done
12 tasks done
12 tasks done
MarcZX10
reviewed
Oct 22, 2026
|
|
||
| bool sameTarget(const ColorTarget& a, const ColorTarget& b) { | ||
| return a.surfaceAddress == b.surfaceAddress && a.format == b.format && a.extent.width == b.extent.width && a.extent.height == b.extent.height && a.samples == b.samples; | ||
| return a.surfaceAddress == b.surfaceAddress && a.format == b.format && a.extent.width == b.extent.width && a.extent.height == b.extent.height && a.samples == b.samples && a.eqaaFragments == b.eqaaFragments; |
Contributor
There was a problem hiding this comment.
This cache key distinguishes EQAA fragment count but omits the newly guarded FMASK address. If the same color surface is rendered again with a different fmaskAddress, acquire() finds the existing target and returns it without refreshing guarded(), so the old FMASK range remains protected while the current FMASK range is not. Please include the FMASK address in target identity/recreation or explicitly replace that guard when it changes, and add a regression that renders the same color target with two FMASK bases and verifies the guard follows the active base.
Contributor
Author
There was a problem hiding this comment.
Thanks, that was a real gap. Fixed in 53d00af: when acquire() reuses an EQAA target whose FMASK base changed, it replaces the target's FMASK guard with one at the new base, and 81eca78 keeps the FMASK pipe/bank XOR with it. I kept the FMASK base out of the target's identity, so the host target and its samples carry over to the next draw.
The regression is in EqaaColorTarget.cpp: it draws the same color surface again with a second FMASK block, then requires a read of the new base to be refused by name and the old base to be unguarded. Without the change in acquire() it fails with a read of the FMASK base the EQAA target was last drawn with was not refused by name. Results are in the body.
AI-assisted: yes (Claude Code)
MarcZX10
suggested changes
Oct 22, 2026
MarcZX10
left a comment
MarcZX10
left a comment
Contributor
There was a problem hiding this comment.
I reviewed the decode, fragment-sized guest footprint, FMASK guard, resolve path, and the execution test. The author reports 62/62 focused tests on an RTX 4090 with three repeats and validation-layer comparison; an independent Apple/MoltenVK report also says the new test passes. I could not independently build or run the C++ tests in this Linux checkout because no configured product build is available.
There is a guard-lifetime issue at sameTarget (I left an inline comment): the cache key includes eqaaFragments but omits fmaskAddress. If the same color surface is reused with a different FMASK base, acquire() can return the existing target without refreshing its guard, leaving the new FMASK range unprotected. Please make the target/guard follow the current FMASK address and add a regression that changes the FMASK base for the same color surface.
Please also update this branch to current main (8b3a6ba88d8fc74e768f1a7e8e9e36fb69842768); the current head is based on 7e2ba73e4c, 13 commits behind. Rerun the focused tests after rebasing.
The overlap report lists shared changes with #7948 and #9450, which this description already discusses, and newly open #9672 through #9675, which touch State.hpp's register ranges and/or State.cpp's target decode. The repository asks authors to agree on landing order for overlapping code and declare Depends on when a stack is required. Please coordinate with those authors and update the checklist if this change depends on a particular order. I am treating these as shared-file coordination needs, not assuming the separate PS4/PS5 behavior is duplicate work.
Local checks: git diff --check is clean. GitHub conventions, eligibility, intake, labels and report checks pass; product CI jobs are skipped by the repository gates. I did not independently reproduce the author-reported GPU results.
12 tasks done
Contributor
|
@frankea a merge-order note: #9533, with #9717 and #9742 stacked on it, moves the guarded byte count out of |
This was referenced Oct 22, 2026
github-actions
Bot
added
build
docs
agc
Contributor
Author
|
@EagleBearerYS thank you for working that out. I've recorded it in the checklist: #9533, with #9717 and #9742, goes after this PR and takes the fragment count in AI-assisted: yes (Claude Code) |
This was referenced Oct 23, 2026
fix(agc): draw a one-fragment multisampled color target as one sample with sample 0's coverage
#9450
Open
Contributor
Author
|
@MarcZX10 the FMASK guard follows the active FMASK base since 53d00af, with the regression you asked for, and the branch now merges AI-assisted: yes (Claude Code) |
12 tasks done
12 tasks done
12 tasks done
A color target with fewer fragments than samples (CB_COLOR_ATTRIB NUM_FRAGMENTS below NUM_SAMPLES) with FMASK is drawn as a host multisampled target at its coverage sample count. Its guest footprint and guard are sized by its fragments, its FMASK plane is guarded, and sampling it, storage access to it, DCC on it and EQAA without FMASK throw.
adriadam10
force-pushed
the
eqaa-color-targets
branch
from
October 24, 2026 00:39
d902416 to
a2ed56e
Compare
github-actions
Bot
added
needs rebase
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
DecodeColorBufferrefused every color target whoseCB_COLOR_ATTRIB.NUM_FRAGMENTSis belowNUM_SAMPLES(EQAA): "color targets with fewer fragments than samples (EQAA) are unsupported". This accepts such a target when it has FMASK (CB_COLOR_INFO.COMPRESSIONand aCB_COLOR*_FMASKbase) and no DCC, and draws it as the existing host multisampled target at its coverage sample count.ColorTargetgainseqaaFragments(0 when the fragments equal the samples). The FMASK base is feat(agc): expose expanded FMASK for tracked native owners #7948'sfmaskAddress, whichmaindecodes for every multisampled target;CB_COLOR*_FMASK_BASE_EXT(context 0x3a0 + slot) is now inDrawKeyRegisters.samplesstaysNUM_SAMPLES, so pipelines, draws, CMASK fast clears, the FMASK decompress and the CB resolve are the existing MSAA path.eqaaFragmentsfits in existing padding, sosizeof(State)and the pipeline journal are unchanged; a pipeline depends on the coverage sample count, not the fragment count.MultisampledColorBytes, the guarded range, color fills) uses the fragment count, as addrlib sizes the surface: a 4-sample 2-fragment 512x512 RGBA8 target occupies 2 MiB, not 4.Z_64KB_Xsurface of 1, 2 or 4 bytes per pixel, so a T#, CPU or DMA read of it throws by name ("overlaps the FMASK of EQAA color target") instead of reading memory the host never wrote. An unmap or allocation removal of either plane retires all of the target's guards; the second pass ofretireUnmappednow drops every inactive range, not only metadata ranges.CB_COLOR*_FMASKbase,acquire()replaces its FMASK guard with one at the new base and keeps the target and its samples. The target's identity stays surface, format, extent, samples and fragments, so its contents carry over.GPU semantics, with sources:
NUM_SAMPLESis the coverage sample count andNUM_FRAGMENTSthe number of colors stored per pixel. PAL and Mesa program them that way: PAL gfx9ColorTargetView.cpp and Mesa ac_descriptors.c.NUM_FRAGMENTSdistinct colors. Where more primitives meet in a pixel, the console marks samples unknown and the CB resolve drops them (radeonsi si_state.c). Which samples it drops is not documented or measured, so a resolve here can differ at those pixels. The debt entry says so.GL_AMD_framebuffer_multisample_advanced, RDNA2) show the split: with 2 or 4 storage samples, draws that cover part of a pixel resolve to blends, as a host color per sample does; with 1 storage sample only what sample 0 covers survives.Why: Gran Turismo 7 (PPSA01317) stops on this refusal when it loads the World Map and when it delivers a purchased car, at a 512x512 RGBA8 target with 4 samples and 2 fragments (
CB_COLOR_INFO0x0000e028,CB_COLOR_ATTRIB0x0000a000, CMASK and FMASK). The title consumes the target only through a CB resolve.Overlap with other PRs:
FMASK8_S4_F2included. With this PR, such a read of a host EQAA target's FMASK throws through the guard until the driver serves FMASK from the host target, as feat(agc): expose expanded FMASK for tracked native owners #7948 does for 8-fragment targets.guardedColorBytes(target). It goes after this PR and takes the fragment count there, as @EagleBearerYS noted.NUM_FRAGMENTSbelowNUM_SAMPLESinDecodeColorBuffer.Tested
Refreshed onto
main0db47b2 (head d902416; no conflicts), RTX 4090 (NVIDIA 616.92, Windows 11): the 64 ctest entries of the focused set below (withagc_driver_eqaa_color_target) pass 64/64, each test run three times, and 64/64 under the validation layer (VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation,ANYPS5_NO_SHADER_CACHE=1,ANYPS5_REQUIRE_VULKAN=1,ctest -V), with noVUID-line; its only messages aremain's threeUndefined-Value-ShaderOutputNotConsumed-DynamicRenderingwarnings fromagc_driver_alpha_to_coverage.main0db47b2 passes the union of the refreshed sets, 115/115, under the same layer settings with noVUID-line.Windows 11 (build 26200), NVIDIA GeForce RTX 4090 (driver 616.92), MinGW GCC Release build with
BUILD_TESTING=ON, all on one machine.Focused set: the 63 ctest entries of 19 targets (62 before the
mainmerge). These areagc_driver_graphics_tests(the CPU decode tests, with a new EQAA case), the newagc_driver_eqaa_color_target_tests, and the MSAA, FMASK, CMASK and journal tests: alpha_to_coverage, cached_image_multisample_guard, cmask_fast_clear, color565_target, fmask_image_reads, image_msaa_address_on_plane, msaa_enable_single_sample, multisample_fill_host_only, multisample_readback, multisample_retirement, multisampled_pass_order, native_multisample_depth, pipeline_journal, pixel_sample_coverage, sample_mask_multisample, sample_multisample_target, staged_clear_sampled_read.This PR (43a9181, which merges
main7e69be6): 63/63 passed, each test run three times (ctest --repeat until-fail:3).After the merge,
main(feat(agc): expose expanded FMASK for tracked native owners #7948) decodes an FMASK base for every multisampled target, so 43a9181 drops the decode test's check that a 4-sample 4-fragment target has none; it still checks that such a target is not EQAA.Control for the FMASK guard move: this head without the move in
acquire(). The new test fails witha read of the FMASK base the EQAA target was last drawn with was not refused by name.Validation layer:
VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation(Vulkan SDK 1.4.363.0),ANYPS5_NO_SHADER_CACHE=1andANYPS5_REQUIRE_VULKAN=1withctest -V, onmain83462cc (7e69be6 plus eight commits totools/only) (62/62; the new test isn't onmain) and on this PR (63/63). Neither log haswas not found but was requested by env var VK_INSTANCE_LAYERS. Neither log has aVUID-line. Both have the same threeUndefined-Value-ShaderOutputNotConsumed-DynamicRenderingwarnings, frommain'sagc_driver_alpha_to_coverage.Before the
mainmerge (22766ab onmain7e2ba73): 62/62 three times.maincontrol there:main6ef5a1c plus only the new GPU test and its CMake line; the test fails withAGC graphics: color targets with fewer fragments than samples (EQAA) are unsupported.The GPU test draws two slanted triangles, red and green, into a 256x256 RGBA8 target with 4 samples and 2 fragments and into the same target at 4 fragments, then CB-resolves both into one-sample targets. It requires byte-identical results, with edge texels at one-quarter and three-quarter coverage (410 such channels). It also checks that a read of the EQAA FMASK range is refused by name, that the color guard ends at the 2-fragment footprint (512 KiB, not 1 MiB), and that the 4x target's FMASK is not guarded. Then it draws the EQAA color surface again with a second FMASK base and checks that a read of the new base is refused by name and the old base is no longer guarded.
The CPU tests decode GT7's registers (
PA_SC_AA_CONFIG0x0020c002,CB_COLOR_INFO0xe028,CB_COLOR_ATTRIB0xa000, an FMASK base with pipe-bank bits) through the draw-key register facade (0x3a0 now covered), as well as 8 samples with 4 fragments. They check the refusals: one fragment, more fragments than samples, noCOMPRESSION, no FMASK base, a badFMASK_BASE_EXT, and DCC. They also check the CB resolve's source decode and the 4x, 4s/2f and 8s/4f footprints.Not run on Linux.
sizeof(State)stays atmain's 808: a probe built on macOS gives 784, 792 and 808 formain7e2ba73, this PR's previous head andmain7e69be6, the values of their Linux guards, and 808 for this head.tools/check_conventions.pyreports no errors; its one warning is the length of the existing "Multisampled target guest access" debt entry, which this PR extends.Gran Turismo 7 (PPSA01317) on a local stack of
mainwith the open PRs it needs before this stop (fix(agc): write a replace blend into a packed color target without blending #7243, feat(agc): render layered draws into a depth view of several slices #7256, fix(shader): answer image_get_resinfo on any T# dimension #7550, fix(agc): raise no end-of-pipe event from flips and carry the release's interrupt context #7978, feat(agc): merge partial writes into 4_4_4_4 UNORM color targets #8564, feat(agc): blend into 11_11_10 float color targets in the pixel shader #9020, feat(agc): draw alpha-tested packed-target merges that write depth as a depth pass and an EQUAL merge pass #9335, fix(shader): link table callees that together exceed s_call_b64 range #9068), with its diagnostic switches that skip refused compute dispatches (at the revision before the one-fragment refusal; GT7's target has two fragments): the run no longer stops on the EQAA refusal, and nothing hits the FMASK or color guard. After CROSS at the main menu it renders the car loading screens, then stops on the next pass:a one-sample depth target under a multisampled rasterizer without MSAA_ENABLE is unsupported for points or lines and with ... DB_EQAA Z interpolation or overrasterization. That pass keeps the EQAA pass's 4xPA_SC_AA_CONFIGwithDB_EQAA0x00050000 (INTERPOLATE_COMP_Z); it is a separate topic for a follow-up PR.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)State.hpp,State.cppandMultisampleTarget.cpp; I've proposed that fix(agc): draw a one-fragment multisampled color target as one sample with sample 0's coverage #9450 goes first and this PR then mergesmainand keeps both (asked on fix(agc): draw a one-fragment multisampled color target as one sample with sample 0's coverage #9450). feat(libSceAgcDriver): draw PS4 (GCN) command buffers #9431, under ntamas94's fix(libSceAgcDriver): sample a PS4 depth buffer through a T# smaller than its padded extent #9672 to feat(libSceAgcDriver): render into one array slice of a PS4 color target #9675, also extendsDrawKeyRegistersinState.hpp; whichever lands second keeps both ranges (agreed on fix(libSceAgcDriver): sample a PS4 depth buffer through a T# smaller than its padded extent #9672).