Repository navigation
fix(agc): read FMASK of up to 8 samples in every storage width - #8429
milescoviello wants to merge 1 commit into
Conversation
|
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.
|
|
Windows data point: RTX 3080 (NVIDIA 617.42) and AMD Radeon RDNA 2 iGPU ( AI-assisted: yes (Claude Code). |
|
Thanks @TheSovietWaffle. The AMD run is useful here, since the expansion now runs on both vendors' compilers. I added it to the description. AI-assisted: yes (Claude Code) |
|
Windows run of this head (
Both builds succeeded. AI-assisted: yes (Claude Code) |
4660cdd to
643bb61
Compare
Since the synthetic FMASK mode was removed, a direct FMASK load reads the stored word and only Fmask32_S8_F8 is accepted. A load from any other FMASK format of up to 8 samples now reads the 8-, 16- or 32-bit surface and expands it in the shader to 4 bits per sample, an index past the fragment count reading 8 (unknown). One runtime image mode handles every such format, reading the format from the descriptor at run time or from a specialization constant. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
643bb61 to
4feda34
Compare
|
Rebased over #7948, now on
The 81 FMASK, MSAA and image tests pass 3 of 3. Under the 1.4.363 layer they match AI-assisted: yes (Claude Code) |
|
@milescoviello yes: I checked
I also built and tested literal main
20 test processes total: 4 CPU and 16 GPU. All pass, with no skips/timeouts or Vulkan validation warnings/errors/VUIDs. In particular, the existing eight-sample native-owner regression still passes on both vendors. Main does not contain the new narrow-format cases. Environment: Windows 11 Enterprise build 26200; WinLibs GCC 15.2.0 r7/UCRT, Release; NVIDIA GeForce RTX 5080 driver 610.62 (32.0.16.1062), AMD Radeon(TM) Graphics driver 32.0.21036.18. Verbose serial CTest, 30-second limit; This is focused preservation/capability validation, not a repeat of your full 81-test matrix, a title run, or independent confirmation of console semantics. No further guard change requested from this review. AI-assisted review and validation: yes (Codex). |
|
@Amoenus thanks for checking the combined guards, and for the NVIDIA and AMD runs on Windows. I added them to Tested. AI-assisted: yes (Claude Code) |
What
Since #6674, a direct FMASK image load reads the FMASK stored in guest memory, but only for
Fmask32_S8_F8(8 samples, 8 fragments). The other 12 FMASK formats throwFMASK format is not implemented; before #6674 they all read a synthetic identity. This PR reads every uncompressed FMASK format of up to 8 samples:DecodeTextureResourcedecodes such a T# as an integerZ_64KB_Xsurface of the format's storage width:R8_UINT,R16_UINTorR32_UINT. That is the same surface conversion fix(agc): read uncompressed FMASK images from guest memory #6674 uses for 32 bits, following PAL's FMASK-to-surface conversion.nir_lower_ms_txf_to_fragment_fetchreadsubfe(fmask, sample * 4, 3)for every MSAA mode, and its comment notes that EQAA can return 8 for an unknown fragment. Each format stores 1, 2 or 4 bits per sample. The layouts come from PAL's packed expanded FMASK values:0xE4forS4_F4(2 bits),0xA4forS4_F2(2 bits, with 2 meaning unknown),0x44443210forS8_F4(4 bits, with 4 meaning unknown), and so on. A stored index at or past the fragment count reads 8. Nibbles of samples past the sample count read 0, which is not measured on hardware.conversionFormat=FmaskExpansion). So a 32-bit image load gains one mode, the same count it had before fix(agc): read uncompressed FMASK images from guest memory #6674.FmaskFormatBase+ image. The constant is supplied for every image that has the mode, and it is 0 unless the FMASK mode is the selected one, so other draws don't gain specialized modules.Fmask32_S8_F8keeps fix(agc): read uncompressed FMASK images from guest memory #6674's path. The 16-sample formats, whose expanded value is 64 bits, still throw, as do packed accesses.main: without readable metadata it throws, and with it only an 8-sample, 8-fragment FMASK goes to feat(agc): expose expanded FMASK for tracked native owners #7948's native owner path. A compressed FMASK in one of this PR's narrower formats now throwsFMASK CMASK compression is implemented only for 8-sample, 8-fragment FMASK, both inDecodeTextureResourceand in the materializer, since nothing decodes its compressed indices. The shader debt entry states the new limits.Why
Ruffle (PPSA68091, 1.2.0) renders through its own RADV and resolves 4x MSAA in a fragment shader through
FMASK8_S4_F4. Onmaina9c93a0 with #7897 (its start-up fix), the run stops 2 of 2 at 10 s, when the demo starts:A diagnostic print of the refused T#: format 161 (
Fmask8_S4_F4),dword6compression bits clear, 2DZ_64KB_X,40043880 ca100000 810dc1df 91800924 00000000 00000000 00080000 00000000. Ruffle ran past this point on 10-18, before #6674.With this PR, the run continues to the 120 s limit 2 of 2.
Tested
Rebased onto
mainf6075c2 as4feda344e6, after #7948 merged. The conflicts were the two FMASK checks that #7948 changed:GuestTextureResource.cppandResourceMaterializer.cpp, the format check is this PR's (any FMASK layout of up to 8 samples), and the CMASK checks are feat(agc): expose expanded FMASK for tracked native owners #7948's (readable, pipe-aligned metadata).agc_driver_graphics' decode test now asks that refusal of each of the eight narrow formats.On that head (Ubuntu 26.04.1, GCC 15.2, RTX 4070 Ti, 615.71.09), the 81 tests matching
^agc_driver_(fmask|sample_multisample|msaa|multisample|graphics$|image_)pass 3 of 3. Under Khronos 1.4.363 the same set passes onmainand on this head; both report the sameVUID-VkImageViewCreateInfo-usage-02275x2 as before, and nothing is new in head.4feda344e6and found that the combined guards match feat(agc): expose expanded FMASK for tracked native owners #7948's support boundary. They also ran it, composed withmain9e568a3, on Windows 11 under the 1.4.363 layer, on an NVIDIA RTX 5080 (610.62) and an AMD Radeon iGPU (32.0.21036.18).agc_shader_memory,agc_driver_graphics,agc_driver_fmask_image_reads(3 of 3) andagc_driver_native_fmaskpass on both, with noVUID-message, so feat(agc): expose expanded FMASK for tracked native owners #7948's eight-sample native-owner regression still passes.Earlier: rebased onto
main8b3a6ba as643bb61490. The one conflict was an include line inRdnaDescriptorFormat.hpp, where #9430 added<span>and this PR adds<optional>; both are kept, and the change is otherwise line for line the same. On that head, Ubuntu 26.04.1 (kernel 7.0), GCC 15.2, NVIDIA RTX 4070 Ti, driver 615.71.09, Release:agc_driver_fmask_image_reads,agc_driver_graphicsandagc_shader_memorypass 3 of 3. The results below are from the pre-rebase head.Ubuntu 24.04 (Linux 6.8), GCC 13.3, NVIDIA RTX 4070 Ti, driver 595.99.02, on
maind50ec76.agc_driver_fmask_image_readsgains a pass over the eight new formats:S2_F1,S4_F1,S8_F1,S2_F2,S4_F2,S4_F4,S8_F2(16-bit) andS8_F4(32-bit).0xE4→0x3210,0xA4→0x8810,0x44443210→0x88883210). A second round stores varied values.main's production code it fails 3 of 3 (FMASK format is not implemented). With this PR it passes 3 of 3.agc_shader_memorychecks that the eight formats select the one expansion mode, that the 16-sample formats still throw, and that a Y selector is refused.agc_driver_graphicschecks each format's surface width, that identity selectors are accepted on an expanded view and refused onFmask32_S8_F8, and that the 16-sample formats are refused. Both pass 3 of 3.ctest -R '^(agc_|shader_cache)': 828 of 828 pass. The usual three skip themselves (agc_host_import_sparse,agc_driver_sparse_dma_buf_import,agc_driver_stencil_ref_export).Khronos validation layer (LunarG SDK 1.4.363), same machine:
VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation ANYPS5_NO_SHADER_CACHE=1 ANYPS5_REQUIRE_VULKAN=1 ctest -R '^agc_driver_(fmask|sample_multisample|msaa|multisample|graphics$|image_)' -Vonmaind50ec76 and on this PR (4660cdd). 73 of 73 pass on each. Neither log reports the layer missing. Both have the same VUID,VUID-VkImageViewCreateInfo-usage-02275×2, and none is new.PPSA68091 on
maina9c93a0 + fix(libc): adopt the guest SDK allocator only when the title replaces the heap #7897 + this PR: the demo runs to the 120 s limit 2 of 2. The demo is an empty SWF, so I also ran Ruffle's own visual tests, the 36 that check frame 1, each injected as the only library game for 32 s.output.expected.png, 29 of the 32 match at a coarse (32×24 block) difference under 2, with the same per-test values as on 10-18.define_bits_lossless2_rgb15(26.5) andsimple_shapes__gradients__repeat(2.5) score the same as on 10-18.filters__bevelvaries between runs on these libs (6.7, 14.2 and 14.6); in the worse frames two of its rows haven't been drawn yet.SPIR-V size, from the shader replay corpus (466 dumped requests from MXGP 26 and Super Meat Boy 3D),
agc_shader_replay --spvatmaind50ec76 and at this PR: the same 462 modules come out; 197 of them grow, by at most 7.2 KB each (the expansion is emitted for each FMASK-capable 32-bit load), at most 10.3% on one 69 KB module, and 0.12% in total (150,120,600 to 150,298,672 bytes). Before fix(agc): read uncompressed FMASK images from guest memory #6674 the same loads carried a constant FMASK mode instead.tools/check_conventions.py --base upstream/mainpasses. Two open PRs edit the same lines:GuestTextureResource.cpp,ResourceMaterializer.cpp).I moved two edits so they don't collide with fix(agc): support packed 16-bit storage images #6218 and fix(shader): round normalized storage image writes from the exact product #7666. Whichever of these lands second needs a short rebase.
Windows 10 Pro 22H2, WinLibs GCC 15.2.0, Release, by @TheSovietWaffle on
4660cdd42:agc_driver_fmask_image_reads,agc_shader_memoryandagc_driver_graphicspass 3 of 3 on an NVIDIA RTX 3080 (617.42) and on an AMD Radeon RDNA 2 iGPU (1002:164e, 32.0.21045.5002).Not tested: the console itself.
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