Repository navigation
feat(agc): read individual multisample image samples - #6601
Conversation
|
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
left a comment
jotitadev
left a comment
There was a problem hiding this comment.
First Linux run (the body says Linux/lavapipe is unverified). It passes on NVIDIA, but on lavapipe, which the Linux CI job uses, all six new tests fail.
Machine: NVIDIA GeForce RTX 3070, driver 595.99.02, Vulkan 1.4.329, plus lavapipe (Mesa 26.0.3, LLVM 21.1.8) on the same machine. Ubuntu 26.04 (kernel 7.0.0-38), i7-8700, GCC 15.2, Release, BUILD_TESTING=ON. Head 4681a51bd merged into main 5d9dcc42d (clean merge). It builds with GCC on Linux without the #6452 change.
RTX 3070:
agc_driver_multisample_readbackand the_uint8/_uint16/_uint64/_uint128/_depthcases pass 3/3.ctest -R "^agc|shader|recompiler" -j1(flip, display andlds_atomics64excluded) gives the same result asmainplus the 6 new tests. The only failure isstore_over_untracked, which also fails onmainhere (#6347).
lavapipe (VK_DRIVER_FILES=.../lvp_icd.json ANYPS5_GPU=llvmpipe):
- All six fail right away with
AGC graphics: readback fixture format does not support its sample count or extent. - lavapipe reports
sampledImage*SampleCountsandframebuffer*SampleCounts= 1, 4, 8. There is no 2x, and the test's loop starts at{2u, 4u, 8u}. TheRequireinSurfacethrows, so the test returns 1 instead of skipping that count. - With a local edit that only drops
2ufrom that loop, all six pass on lavapipe, including D32 and the 128x64 4x depth case. So the reader itself works there. - The other multisample tests (
native_multisample_depth,sample_mask_multisample,sample_multisample_target,multisample_fill_host_only,agc_multisample_guest_access) pass on lavapipe.
Skipping sample counts the format doesn't support would fix it, with a message such as "the device does not support 2 samples" that tools/check_skips.py accepts. Mesa 25.x lavapipe on the CI runner probably lacks 2x too, but I only checked 26.0.3.
GPUs tested: NVIDIA GeForce RTX 3070 (595.99.02), lavapipe (Mesa 26.0.3, LLVM 21.1.8), Ubuntu 26.04
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
@jotitadev Thank you for isolating the unsupported 2x count. Fixed in 4b24b3ca0: the fixture reports unsupported format/count/extent combinations and runs the remaining counts; optional 4x cases are checked too. It skips the whole case only when no count can run or per-sample shading is unavailable. Unexpected Vulkan errors and actual readback failures still fail; production code is unchanged. All six modes pass 5/5 on both physical Windows GPUs (RTX 5080 and AMD Radeon(TM) Graphics): 60 fresh processes, no skips, maximum 0.34 s. Full Windows build/checks and selected Cppcheck/clang-tidy diagnostics pass. The PR body records the exact validation composition and credits your Linux results. Local Linux update: exact head Update: thank you for confirming the corrected head on Mesa 26.0.3 and RTX 3070. Your lavapipe run supplies the requested 4x/8x coverage (all six modes 3/3; only 2x unavailable), and RTX 3070 runs 2x/4x/8x. The PR body now records your source pin and machine separately from our local results. The requested retest is complete. Written with AI assistance (Codex); I ran the local checks above. |
jotitadev
left a comment
jotitadev
left a comment
There was a problem hiding this comment.
@Amoenus, as you asked, I re-ran 4b24b3ca0 on the same Mesa 26.0.3 setup. It passes, and both 4x and 8x run there.
Machine: NVIDIA GeForce RTX 3070, driver 595.99.02, Vulkan 1.4.329, plus lavapipe (Mesa 26.0.3, LLVM 21.1.8) on the same machine. Ubuntu 26.04 (kernel 7.0.0-38), i7-8700, GCC 15.2, Release, BUILD_TESTING=ON. Head 4b24b3ca0 merged onto main 0615cd2b8, clean merge (main did not build at the time, #6784).
GPUs tested: NVIDIA GeForce RTX 3070 (595.99.02), lavapipe (Mesa 26.0.3, LLVM 21.1.8), Ubuntu 26.04
lavapipe (ctest -R multisample_readback -V, 3 runs):
- All six pass in every run, about 0.05 s each.
- Every mode prints
samples=4andsamples=8as passed. Each also prints oneskipped, the device does not support readback fixture format=<f> samples=2 extent=17x5 layers=2. So on this Mesa only 2x is missing, not 8x as on your 25.2.8.
RTX 3070:
- All six pass, with 2x, 4x and 8x printed for every mode.
ctest -R "^agc|shader|recompiler" -E "agc_display_buffer_dcc|agc_driver_flip|agc_driver_lds_atomics64|display|flip" -j1 --timeout 60: 619 tests, onlyagc_driver_store_over_untrackedfails, which also fails on unchangedmainhere (#6347).
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
7511810 to
3e3cf1d
Compare
3e3cf1d to
7f53a96
Compare
github-actions
Bot
added
the
unblocks title
|
Checked the reader against the diff: input validation (format, fragments, extent, element and buffer ranges, dispatch limits) throws, pipeline/pool/layout lifetime follows the detiler's ring, pipeline creation holds the cache use lock, and the fixture compares every sample, layer and pixel with sentinels and partial windows. Debt entry is under Functional. Lavapipe and NVIDIA runs by @jotitadev cover the Linux driver paths; merging through the queue. |
What
Shared sample-preserving readback foundation requested in #6570 (Onimusha: Way of the Sword, PPSA27836). Adds
TextureDetiler::DispatchMultisampleReadback, which records per-sample fetches into a linear storage buffer, in layer/y/x/fragment order. Supports unsigned 1/2/4/8/16-byte element views and D32 depth, 2/4/8 samples, array layers and partial element windows. Invalid formats, counts, ranges and dispatch limits throw.Uses a sampled 2D-array view in GENERAL. The depth variant fetches floating-point depth and bitcasts to DWORDs; the unsigned variant preserves element bits and the unused bytes of a final packed word. The caller owns image/view lifetimes, producer/readback barriers and completion. Descriptor/pipeline ownership is lazy and follows the detiler's existing descriptor-ring lifecycle.
Semantics: Vulkan specifies floating-point reads of depth aspects and per-sample OpImageFetch without a sampler. Both generated modules require only Shader capability. Source images need sampled usage; storage usage is not used by the fixtures. This avoids a storage-image prerequisite for native D32 readback.
@simonbronk will consume the reader in the depth publisher discussed in #6570; the reported stop is
AGC graphics: sample-preserving guest publication of native multisample depth is not implemented. Guest-memory publication, ownership/coherence and HTILE remain that follow-up. #6588 provides the complementary CPU layout/retile helper independently. No dependency on #5881 and no title-unblock claim from this utility alone. No duplicate reader found in current source/open PRs and exact-helper/related-readback searches.Tested
Current submitted head: 7f53a96a9f, rebased directly onto main
0bb238b4a447f448e5d6592eae39d43a6a8c7d8c. Only adjacent debt entries conflicted; both are retained, with this entry before the final existing entry. No production resolution was needed. The reader implementation, shaders and fixture are unchanged from the previously tested3e3cf1da06, including cache locking and snapshot exclusion.Require(false)before either dereference. These are inspected advisories, not a clean Cppcheck claim. Conventions and diff checks pass.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)