Repository navigation
fix(agc): store an image-copied buffer copy's destination before later labels - #6324
Conversation
…r labels A buffer copy whose source is exactly a live storage image becomes a device copy between images (AliasCopy). Its results stayed pending in the destination image and reached guest memory only at a later flush, so a guest that waits for a label recorded after the copy and reads the destination on the CPU (a readback) read the bytes from before the copy. Store the destination's results in the same batch, behind the image copy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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.
|
|
Tested on Linux x86-64 (Release build, lavapipe / llvmpipe LLVM 20.1.2), with 21cdb8f merged into main ce991f1:
AI-assisted: yes (Claude Code) |
Hot Wheels (PPSA22408) at head
|
|
Thanks for the runs. On Windows (RTX 3080, NVIDIA 610.74) the crash this PR addresses looks different: without the change the title dies at 115-295 s on the RenderThread with a wild read in a loop that walks UE's GPUMessage readback stream (a record length read back as float bits, e.g. 0xc43fd1d5, sends the index negative). With the change, three runs stayed alive for 420, 420 and 300 s with no fault and reached the first-run settings screen at about 5 FPS. Your fault (a null read after a virtual call that returns zero from an empty lookup table) does not match that signature, so it is probably a separate stop further on; it would help to know whether AI-assisted (Claude Code); numbers from runs on my machine. |
Hot Wheels (PPSA22408): correction, the crash we reported here was caused by our setupThe null read at Relinking the title's own For this PR: no regression on Linux, and the crash above happens with or without it. AI-assisted: this comment was drafted with the help of Claude Code and checked against the logs by us. |
|
Intel and AMD Windows data: the new test passes on both GPUs with this change and fails on both without it, as on the RTX 3080. Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release, every execution test built,
Full CTest ( AI-assisted: yes (Claude Code) |
github-actions
Bot
added
the
unblocks title
5 participants
What
VulkanDevice::CopyBufferturns a copy whose source range is exactly a live storage image into a device copy between storage images (AliasCopy,StorageTexture::CopyFrom). The copied bytes then stay in the destination image as pending results, like a shader's: they reach guest memory only when something in the driver reads that range (FlushPendingthrough the flush hook), at the idle store, or at a flip/label whenAPS5_STORE_AT_LABEL/APS5_STORE_AT_FLIPis set. A guest that reads the destination on the CPU after waiting for a label that follows the copy, which is how a GPU readback works, reads the bytes that were there before the copy: the CPU read does not go through the driver.HOT WHEELS Infinite Rush (PPSA22408, Unreal Engine) copies its 256 KiB GPU message buffer into a readback buffer every frame with a DMA copy, waits for the frame's label and walks the records (
[count][id, payload dwords, payload...]). The message buffer's range is also exactly the surface of a live storage image the driver still caches (transient memory the engine reuses), so the copy takes the image path, the readback buffer keeps old vertex data, the walker reads a float as a record length and faults (RenderThread 0,app.exe+0x93f044, read of0x125...4758) at 115-140 s into every run. With a temporary trace of copies into the readback range:The source holds an empty message buffer (count 0); the destination still holds floats when the CPU reads it.
After
AliasCopysucceeds,CopyBuffernow stores the destination image's results at once (FlushPending(destination, bytes, nullptr, "buffer copy destination", PublishScope::Whole)). The store is recorded in the same batch behind the image copy, so a label recorded after the copy is ordered behind it, as it is for the transfer path. The destination image stays cached, so GPU readers of the destination keep using it.Tested
New test
agc_driver_alias_copy_label(tests/execution/AliasCopyLabel.cpp): a compute store leaves a pending 256x256 R32_UINT image over a 256 KiB source,CopyBuffercopies it to a second 256 KiB range (asserts the image-copy path), a label is recorded on the GPU after it and submitted, the test spins on the label with plain CPU reads and then compares the destination's raw bytes with the source. On main: 262144 of 262144 bytes still unwritten (5 of 5 runs). With the change: 0 (5 of 5 runs).ctest -R agc_589/589 passed (3 skipped for device features);tools/check_conventions.py --base origin/mainandtools/check_test_registration.pypass.APS5_NO_COPY_ALIAS=1, 2.0-2.9 withAPS5_STORE_AT_LABEL=1, both of which also avoid the crash); "PRESS ANY BUTTON", then the first-run settings screen. Gameplay not reached yet (the title runs at ~5 FPS).Related: #6249 (alias copies of surfaces with padding lose bytes outside the image) changes
AliasCopy's eligibility; this PR does not touch it and the two apply independently. #5480 moves alias selection intoPlanAlias; the store here belongs after whichever alias path succeeds.Checklist
main; no other open PR implements the same functions.mdfiles or images added to the repository🤖 Generated with Claude Code