Repository navigation
fix(agc): import a native multisample D32S8 target over uneven planes once its first draw wrote every sample - #16363
Open
hugodearaujo wants to merge 4 commits into
Conversation
…ILE to its depth and stencil planes
…ication # Conflicts: # docs/dev/debt/libSceAgcDriver.md
… stay unimplemented
… once its first draw wrote every sample
Contributor
|
Correction (rerun): my first run rebuilt only the main test target, but the test pattern also matched Windows check of head Setup: Windows 11 Enterprise 22621, WinLibs GCC 15.2 UCRT, Release, Radeon RX 6700 XT, AMD driver 26.10.1. Run through the Windows test queue with
AI-assisted: yes. |
Contributor
✅ NVIDIA GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (4, not counted against this PR)Validation IDs already present on main (9 IDs in 588 tests)Skipped (53)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
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
Assassin's Creed Mirage (PPSA13960) reuses the memory of its 4-sample D32S8 depth target (no HTILE) every frame:
prepareUniformPlaneSampling) finds planes that do not hold one value each and recordsnative multisample depth plane without HTILE does not hold one depth value in [0, 1]. The target is still drawn.DepthSurfaceTexture.The first draw of each frame on that target is a full-screen triangle. It has depth test ALWAYS with depth writes on, and on both faces stencil compare ALWAYS, pass op REPLACE, reference 0 and write mask 0xff.
DB_RENDER_CONTROLis 0x60 and the scissor is the target extent. On the console that draw overwrites every sample, so the guest bytes under the planes no longer matter. But the registers alone cannot show that the geometry covers every sample.This change proves it after the draw instead:
DepthSurfaceViewWithStencilWriteIntenttakes the stencil value that the draw writes to every sample it covers.Draw.cpppasses it only for depth ALWAYS with writes and no depth bounds, plus stencil ALWAYS/REPLACE with write mask 0xff and one reference on both faces. If such a draw creates an owner whose import was refused only because the planes are uneven, the owner starts with stencil~referencein every sample. The refusal stays recorded.CheckCurrent, or a guest access over its planes inRequireNoResidentDepthRead_nid_no_patch. The driver syncs the recorder and stores pending storage results over the planes. Then it reads the stencil samples back, with the readbackPublishToGuestuses (now a shared helper). If every sample holds the reference, every sample passed that draw's tests, so its depth was written too.~reference), another stencil value, guest writes, and storage results stored over the planes since the import.The debt entry for D32S8 without HTILE now says which uneven planes are imported.
Cost: one proof per created owner, which is a recorder sync plus a readback of the stencil plane (7 MB at 1772x996x4).
Tested
Linux 7.2.11, AMD Radeon RX 9070 XT, RADV (Mesa 26.2.6), Release with tests.
agc_driver_native_depth_overwritten_planes(--overwritten-planes), at 2, 4 and 8 samples, on a 200x130 D32S8 target without HTILE. Its planes hold mixed words (5.5e7 and NaN patterns among them) and mixed stencil bytes.RequireNoMultisampleTarget) comes first. It publishes the first draw's samples.DepthSurfaceViewWithStencilWriteIntent, so it cannot build on fix(agc): publish a written native multisample D32S8 owner without HTILE to its depth and stencil planes #15688 alone. A mutation of the fix stands in for the base instead. Each was made locally and reverted, and each fails the test at the expected case:VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation,ANYPS5_NO_SHADER_CACHE=1,ANYPS5_REQUIRE_VULKAN=1,ctest -V -R "native_depth|native_multisample|depth_storage|depth_surface|agc_storage_|multisample_guest|repeat_storage_guard|cached_image|storage_flush_ordering|storage_one_slice|storage_read_during|storage_write_back|msaa_address_on_plane"):VUID-line or "was not found but was requested". Both have the same 20WARNING-VkImageSubresourceRange-layerCount-compatibilitylines.tools/check_conventions.py --base fa590e435a: passes.main5299939 + fix(agc): store storage results pending under a multisampled color target before it takes the memory #13963 (7d54822) + fix(agc): release a published native multisample depth owner whose plane a written storage view covers #15707 (a8c5df4, with fix(agc): attach a smaller native multisample depth extent with the same Z_X layout to its owner #15690 and fix(agc): publish a written native multisample D32S8 owner without HTILE to its depth and stencil planes #15688), 720 s each:compute shader 0x10b4c8ad00: AGC graphics: native multisample depth plane without HTILE does not hold one depth value in [0, 1].AGC graphics: a native multisample depth attachment over the planes of a host-written owner of another identity is not implemented, a separate stop;compute shader 0x10009c9600: AGC graphics: a texture read of 0x13bc1a0000+0x800000 overlaps multisampled color target 0x13bada0000+0x1860000 (4 samples), whose samples are only on the host: guest access to it is not implemented, a separate multisampled colour stop;DepthReadCause::Reuseat that access, our case for it passes without the fourth change.main5b1d381: stops at 497 s and 456 s on the attachment message, the VRAM guard at 470 s, and the multisampled colour stop at 485 s. A rerun of that earlier build here stopped at 464 s on the colour stop.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)