Skip to content

fix(agc): read direct FMASK images from metadata snapshot - #4790

Closed
MarcZX10 wants to merge 4 commits into
boykopovar:mainfrom
MarcZX10:codex/fmask-image-read-3219
Closed

MarcZX10 wants to merge 4 commits into
boykopovar:mainfrom
MarcZX10:codex/fmask-image-read-3219

Conversation

@MarcZX10

Copy link
Copy Markdown
Contributor

What

Replace the hard-coded direct FMASK IMAGE_LOAD result with a snapshot that reads the guest FMASK and its CMASK metadata, resolves supported fast-clear and expanded-tile states, and exposes the result as a linear 32-bit image. Unsupported FMASK formats, layouts, selectors, mip states, fragment indices, and CMASK codes still throw.

This carries the two FMASK layout/implementation commits from Romans Pokrovskis, preserving their original commit authorship. A separate MarcZX100 compatibility commit adapts the carried implementation to the current GuestTextureResource model and retains the current multisample-target guard.

The CPU regression covers FMASK/CMASK addressing and snapshot values, including clear tiles, expanded tiles, mixed tiles/layers, and rejected metadata states. The existing Vulkan-backed FmaskImages test is included for hardware execution.

Evidence and limits

Issue #3219 includes a Windows RTX 5080 synthetic IMAGE_LOAD reproduction: raw FMASK bytes remain unchanged while CMASK switches between expanded and fast-clear states, and the old shader output remains constant. The issue reports validation of its hardware fixture, but this PR’s Linux run did not execute the Vulkan FMASK integration test or validate GPU decoding. The console configuration, exact DCC extent, and full hardware mapping remain unconfirmed.

Tested

  • Linux Release: agc_driver_graphics passed (1/1) on current main base ee496219007b05e054a469fe12330f7e38e5451a.
  • python3 tools/check_conventions.py --base upstream/main passed.
  • git diff --check upstream/main...HEAD passed.
  • The Vulkan-backed FmaskImages test and PS5/GPU validation were not run here.

Checklist

  • Based on current main; no open PR implements direct FMASK reads with CMASK interpretation
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unsupported paths throw or use NotImplemented_nid_no_patch
  • No local investigation files or captures added to the repository
  • No new third-party code
  • Generic behavior
  • Depends on: none
  • AI-assisted: yes

@kr4y4

kr4y4 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

A hardware run of FmaskImages, since the description says the Vulkan test and GPU decoding weren't run.

Setup: Windows 11 (26200), WinLibs GCC 15.2.0 r7, Release, CI configure flags, an RTX 4070 Laptop GPU and an Intel Raptor Lake-S iGPU. The head 4f0c50c merged into main 284a1c3, which passes 1020 of 1020 on both GPUs.

  • agc_driver_fmask_images, agc_driver_graphics, agc_descriptor_pool_reset_failure and agc_compute_descriptor_layout_flags pass on both GPUs with --repeat until-fail:3. The full suite (ctest --timeout 120 --parallel 4) passes 1021 of 1021 on both.
  • agc_driver_fmask_images reports 13 read cases, 5 unsupported-input refusals, negative control on both GPUs, with the same values on each. With the FMASK bytes left at 76543210, cmask-fast-clear-same-address (CMASK cc) reads 0, and expanded-restored-same-address (CMASK ff) reads 76543210 again. That is the Shader: compressed FMASK/CMASK image reads remain unsupported (PPSA99902) #3219 case.
  • On main with only FmaskImages.cpp added, the test fails on both GPUs at its first FMASK case: expanded-fmask observed output differs from diagnostic prediction: lane=0 component=1 value=4275878552. That value is the 0xfedcba98 that SpirvImageEmitter.cpp:1637 on main puts in the second component. The production files can't be reverted on their own, because the new FmaskTexture.cpp needs this branch's ColorTargetLayout.hpp.

Written with AI assistance.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Which title reads an FMASK image? #3219 is a synthetic reproduction. cachedTexture now rebuilds the snapshot on every lookup with a per-texel CPU loop (ShaderResources.cpp:522-540) and throws for CMASK codes other than 0xC and 0xF, where main returned a constant, so this needs a run of a title that reaches it: the ID, that it gets as far as on main, and the frame time before and after.

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Thanks for running the exact head on both the RTX 4070 Laptop and Intel iGPU, including the full-suite runs and the failing-main control. That materially strengthens the hardware validation. The requested retail-title evidence is still missing: #3219's PPSA99902 data came from a diagnostic stack on a non-current base, and I do not have a current-main title run or before/after frame-time measurement for this PR. I am not claiming title compatibility or a performance result until that is measured.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
@MarcZX10
MarcZX10 force-pushed the codex/fmask-image-read-3219 branch from 4f0c50c to 41d933f Compare October 13, 2026 19:07
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
@MarcZX10

Copy link
Copy Markdown
Contributor Author

Thanks for the dual-GPU run. This confirms the focused synthetic regression on both devices and shows the main-only control fails at the targeted FMASK case. I do not have a title that reaches this path or before/after frame-time measurements, so I cannot answer those two production-impact questions yet. The per-lookup snapshot/CPU-loop cost and unsupported CMASK modes remain limitations in the PR; the focused correctness evidence does not establish title coverage or performance.

@blawesom

Copy link
Copy Markdown
Contributor

Rebase handoff — rebased onto current main (docs-only conflict, both entries kept), authorship unchanged.

Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release, BUILD_TESTING=ON, AMD RX 9070 XT / RADV):

To pick it up:

git fetch https://github.com/blawesom/AnyPS5 rebase/4790
git checkout <your branch>
git merge --ff-only FETCH_HEAD

Head is 48e5ed990.

Amoenus and others added 4 commits October 14, 2026 01:17
Preserve prior local diagnostic guards and changes. Integrate the separately reviewed FMASK read topic. Written with AI assistance; local validation only.
@MarcZX10
MarcZX10 force-pushed the codex/fmask-image-read-3219 branch from 41d933f to 30aca28 Compare October 14, 2026 01:20
@MarcZX10

Copy link
Copy Markdown
Contributor Author

Rebased onto main at ee9037d85154d0c840517c46c61087a736fe0641. The two documentation conflicts were resolved by retaining the updated main entries and the FMASK documentation. The existing four commits were replayed with their original authors preserved; head is 30aca286c86d85e25169984fd6c53450a762d769.

Linux verification on the rebased head:

  • cmake --build build --target agc_driver_graphics_tests --parallel 4 — passed
  • ctest --test-dir build -R '^agc_driver_graphics$' --output-on-failure — passed (1/1)
  • ctest --test-dir build -R '^agc_driver_fmask_images$' --output-on-failure — passed (1/1), using llvmpipe software Vulkan
  • git diff --check upstream/main...HEAD and python3 tools/check_conventions.py --base upstream/main — passed

These Linux checks do not supply the requested retail-title run or before/after frame-time evidence. I still cannot claim title compatibility or performance results; the previously reported Windows dual-GPU synthetic test remains the hardware evidence.

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

Copy link
Copy Markdown
Collaborator

Closing: no title is known to reach this. With the queue at 480+ open PRs, please hold off on new PRs that don't name a title needing the change until it's down. If a title stops on this on current main, reopen with the title ID and the log line.

@Amoenus

Amoenus commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Native-owner-only follow-up qualified (2026-10-19): one focused fork commit, main88a6, exposes expanded indices only for an exact tracked native8x DCC/CMASK owner. It preserves your attribution/coauthor trailer for shared descriptor normalization and excludes the standalone compressed decoder. New direct/registered/prepared/invalidation checks pass three processes per actual NVIDIA/AMD GPU. Full builds pass; the total matrix has36 passes and nine unchanged AMD #7936 baseline failures, zero VUID/errors/warnings.

Native-owner-only PR #7948 is now open after the same creation request succeeded on retry. It is separate from your standalone decoder; no decoder PR/reopen was created. Your standalone metadata-decoding topic remains separate. Maintained evidence, remaining gaps and conditional Sonic context. AI-assisted: yes (Codex).

Local adaptation and title attempt — 2026-10-19

@MarcZX10 @oneandonlydean the metadata-aware subset is now adapted locally on main5c6c and merged #6674, preserving Romans Pokrovskis' original work and MarcZX10's compatibility attribution. Fork branch; no competing upstream PR or reopen here.

Main/focused/integration:36 repeated passes (18 host-GPU,18 CPU), plus6 expected main capability refusals, on verified RTX5080/Radeon with Khronos validation and no VUID/errors/warnings. The focused fixture checks unchanged-address CMASK transitions, mixed tiles/layers and unsupported codes.

The same-parent Sonic title attempt gets past the FMASK compiler refusal but stops at our native color-snapshot stack's unsupported-tiling/metadata guard. Both parent/child end with black captures and no menu; the exact snapshot condition remains unidentified. This does not yet establish a title unblock, completed title FMASK decoding or before/after frame-time evidence. Complete pins, logs, attribution, analysis limits and results. The per-lookup CPU snapshot cost and console layout evidence remain limitations. AI-assisted: yes, Codex.

Prior mainb7 title evidence and coordination

@MarcZX10 @oneandonlydean following the closing request for a title ID/log: Sonic Unleashed Recompiled PPSA99902 / 01.003.000 now reaches this compressed-FMASK path with unmodified mainb7a75819f3 runtime libraries, from a clean separate source/build:

[videoout] presentation failed: shader 0x20be36e00 stage 5: FMASK CMASK compression is not implemented

Natural0xC0000409 after22.334s; inspected startup logos and final black capture, no menu/gameplay. RTX5080, driver610.62/32.0.16.1062, Windows11 Enterprise26200; Khronos1.4.363 loaded. All127 archived/build libraries match;145 prior runtime/user-data files restored,562 expected hashes independently verified. This retains oura515 converted application/guest libc and original assets/configs; it is not a fresh main conversion. Main also logs10mipLevels-00958 errors and10fragment-output warnings, so no validation-clean claim. Log SHA256 032e204589003ffa11dabc710b7500f60a39d342e13d89071082834bd9249870.

The composed 115d integration, incorporating the same main, reaches the same refusal at NOW LOADING after30.448s with0VUID/errors/warnings. Both replays end before the menu without input or a harness timeout. The elapsed times are not a performance comparison.

Merged #6674 now handles uncompressed FMASK and explicitly rejects CMASK compression, so the older implementation needs a focused adaptation on top of that work. We updated #3219 to describe the remaining gap and #7260 with exact pins/results. Mainc82ffb0f20 landed after the tests without changing this materializer; it was not replayed.

Would you like to revive/adapt the compressed-metadata subset here? I can validate an updated head against this title. I have not reopened your PR or published a competing copy. Romans Pokrovskis' original work and your compatibility work should retain their attribution. These observations establish the startup path only; they do not test the old4790 head, its runtime cost, or before/after frame times.

AI-assisted validation/report: yes, Codex.

Amoenus added a commit to Amoenus/AnyPS5 that referenced this pull request Oct 19, 2026
Import an eight-fragment DCC-cleared owner with disjoint watched metadata, capture its independent CB FMASK address and XOR, and expose expanded indices only for an exact matching native view. Reject standalone compressed reads, descriptor mismatches and metadata writes. Exercise direct/registered/prepared dispatches and invalidation with a cold GPU fixture.

Descriptor metadata normalization builds on Romans Pokrovskis and MarcZX100 work in boykopovar#4790; standalone CPU decoding remains outside this change. Expanded indices follow AMD PAL c5e800072a32f68b6ccc4422936d96167c6e0728. Written with AI assistance.

Co-authored-by: MarcZX100 <marczxrr@gmail.com>
Amoenus added a commit to Amoenus/AnyPS5 that referenced this pull request Oct 20, 2026
Import an eight-fragment DCC-cleared owner with disjoint watched metadata, capture its independent CB FMASK address and XOR, and expose expanded indices only for an exact matching native view. Reject standalone compressed reads, descriptor mismatches and metadata writes. Exercise direct/registered/prepared dispatches and invalidation with a cold GPU fixture.

Descriptor metadata normalization builds on Romans Pokrovskis and MarcZX100 work in boykopovar#4790; standalone CPU decoding remains outside this change. Expanded indices follow AMD PAL c5e800072a32f68b6ccc4422936d96167c6e0728. Written with AI assistance.

Co-authored-by: MarcZX100 <marczxrr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* build docs needs rebase Conflicts with main, rebase needed shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants