Repository navigation
feat(agc): publish a written native multisample D32 depth surface to its guest planes on access - #9950
feat(agc): publish a written native multisample D32 depth surface to its guest planes on access#9950ebarr24 wants to merge 1 commit into
Conversation
…its guest planes on access A guest read, DMA copy source or texture, storage, index or vertex read over a native multisample depth surface that a draw had written threw sample-preserving guest publication of native multisample depth is not implemented (Sonic Superstars PPSA06888 boykopovar#5382, Monster Hunter Wilds PPSA24191 boykopovar#7972, Onimusha PPSA27836 boykopovar#6570). The access now completes the pending fast clear and recorder work, reads the samples back in the guest layer, row, column, fragment order with the merged multisample readback, retiles them into the Z_X plane with the descriptor pipe-bank XOR through MultisampleTextureLayout, and rewrites every HTILE word to the expanded clear word (Mesa radv_get_htile_initial_value: 0xfffc000f without stencil, ZMask 0xf with the full Z range otherwise) before retiring the surface like an unwritten one. Written D16 surfaces, surfaces the device stores in another format and surfaces it cannot sample still throw with their own messages. The console HTILE after a DB decompress and its compressed plane bytes are not measured.
github-actions
Bot
added
the
unblocks title
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the complete implementation, retirement flow, regression tests and debt update. Publication waits for recorder work, reads D32 samples with the descriptor layout/XOR, preserves untouched guest padding, expands HTILE metadata, then retires the guarded surface. The added cases cover 2/4/8 samples, non-aligned extents, depth-only metadata, XOR, sample values, padding and clear transitions. The author reports the focused Vulkan test passing on Windows/NVIDIA and a paired validation-layer run with zero VUIDs on both the test branch and baseline. I checked the seven commits from that baseline (fcd4558) to current main (895f47f); none changes the implementation, tests or layout files reviewed here. Hosted policy checks pass; product build jobs are skipped by repository policy. I found no blocking defect.
|
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.
|
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the publication path and regression coverage. The access path syncs pending recorder work, reads the native multisample image into a linear staging buffer, retiles samples using the guest Z_X layout and pipe-bank XOR, expands the HTILE words, writes guest memory, then retires the resident surface. The implementation explicitly keeps D16, host-format conversion, and unsampleable devices unsupported. The test checks 2x/4x/8x D32 publication, both HTILE shapes, a nonzero pipe-bank XOR, every sample location, and untouched padding; the author reports zero validation-layer VUIDs on both main and the change. The required policy/conventions/intake/labels/report checks pass; hosted product jobs are skipped. I could not run the GPU test here because this environment has no /dev/dri device. The overlap with #9533/#9717 is distinct D32S8 import/stencil sampling work; if either merges first, please rebase and preserve those changes alongside this D32 guest-publication path. Approved at head 875fa29.
Hardware & Environment
Test Methodology & Verification
LGTM! AI-assisted: yes. Written with AI assistance and reviewed by me. |
|
@ebarr24 a merge-order note from #9533, ours, which #9717 and #9742 stack on. Both PRs edit
They compose: keep both includes and merge the two sentences. We'd propose that this PR goes first, and #9533 rebases over it and lists it under Depends on. If you'd rather go second, say so. |
jotitadev
left a comment
jotitadev
left a comment
There was a problem hiding this comment.
First run on Apple silicon. It goes from red to green with both drivers I tried.
Setup: Apple M2 Max (32 GB), macOS 27.2, x86_64 build under Rosetta, Release, BUILD_TESTING=ON. Method: #8658 (feat/macos-port) @ 4e0468722f plus this PR's diff from its merge base (head 875fa29033), not main, since main does not build on macOS yet. Two drivers: MoltenVK 1.4.2 (Vulkan SDK 1.4.363.0), and MoltenVK main with SPIRV-Cross #2704. ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1, ctest -R 'native_multisample_depth|native_depth' -j1 --timeout 30 -V.
| MoltenVK 1.4.2 | MoltenVK main + #2704 | |
|---|---|---|
| this PR's test changes only | 1 of 5 pass. agc_driver_native_multisample_depth fails with sample-preserving guest publication of native multisample depth is not implemented; _clears, _clear_refusals and _retirement fail because they expect the new refusal messages |
same |
| this PR | 5 of 5 pass | 5 of 5 pass |
With this PR, the new publication case checks samples, HTILE and padding for 2 and 4 samples, at 5x3 (depthOnly=0) and 70x67 (depthOnly=1, xor=0x8c00).
GPUs tested: Apple M2 Max (MoltenVK 1.4.2 from Vulkan SDK 1.4.363.0, and MoltenVK main + SPIRV-Cross #2704), macOS 27.2.
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
Windows run on NVIDIA and Intel. Head 875fa29 was merged into Setup: Windows 11 Pro 10.0.26300, WinLibs GCC 15.2.0 r7, Release. Environment:
AI-assisted: yes (Claude Code) |
What
A guest read, DMA copy source or texture, storage, index or vertex read over a native multisample depth surface that a draw had written threw sample-preserving guest publication of native multisample depth is not implemented (Sonic Superstars PPSA06888 #5382, Monster Hunter Wilds PPSA24191 #7972, Onimusha PPSA27836 #6570). The access now completes the pending fast clear and recorder work, reads the samples back in the guest layer, row, column, fragment order with the merged multisample readback, retiles them into the Z_X plane with the descriptor pipe-bank XOR through MultisampleTextureLayout, and rewrites every HTILE word to the expanded clear word (Mesa radv_get_htile_initial_value: 0xfffc000f without stencil, ZMask 0xf with the full Z range otherwise) before retiring the surface like an unwritten one. Written D16 surfaces, surfaces the device stores in another format and surfaces it cannot sample still throw with their own messages. The console HTILE after a DB decompress and its compressed plane bytes are not measured.
Tested
Windows 11, MinGW-w64 GCC 15.2, Release build with tests.
agc_driver_native_multisample_depthpasses with this change; the new test fails onmain.Validation layer (vulkan-sdk-1.4.363.0), on
mainfcd4558 and on this change, withVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation,VK_ADD_LAYER_PATHset to the layer,ANYPS5_NO_SHADER_CACHE=1andANYPS5_REQUIRE_VULKAN=1:ctest -V -R "agc_driver_native_multisample_depth$". Neither log containswas not found but was requested by env var VK_INSTANCE_LAYERS; the change adds noVUID-errors (0 with the change, 0 onmain).ctest -V log, main
The
***Failedresult and the failure line at its end are the new regression test failing onmain, as intended; they are not an unrelated baseline failure.ctest -V log, this change
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)mainand this change, no new VUIDs🤖 Generated with Claude Code