Repository navigation
fix(vk): snapshot uniform buffers on Intel when the buffer pools have no VRAM mirror - #2464
Conversation
|
I tested this on a second iGPU, on Linux. My machine doesn't reproduce the veil, so I can't confirm the fix, but I found no regressions, and I have some history on the same root cause that may help. Setup
Result
Earlier report of the same cause That suggests the stale reads are timing-dependent: whether a frame's uniforms get overwritten before the GPU reads them depends on how far the replay falls behind the game. That fits the bug showing on your Core Ultra in Kokiri and not on my Iris Xe today. This PR fixes it more narrowly and more cheaply than mirroring every pool: only the uniform buffers are copied, and you measured no fps cost. That's what I'd want on iGPUs. |
Joey-Magnus
left a comment
There was a problem hiding this comment.
Good investigation, but:
- It was tested on
6c8c1bf, before #2419 landed. #2419 fixes the same class of bug: the iGPU game overwrites uniforms before later parts of a split submission read them (#997). Please retest the veil on current main without this change first. tests/test_env_on.pyfails ongetenv(...) ? env_on(...) : ...atvk_draw.cpp:2780.- "Discrete GPUs are unchanged" is true, but
mirror_on()is off on every non-discrete GPU, so this also switches on per-draw uniform snapshots on the Steam Deck and AMD APUs like the 780M. It was only tested on one Intel laptop. #2343 also turns mirrors off on GCN dGPUs, which would quietly enable this there too.
hckrman101
left a comment
There was a problem hiding this comment.
Thanks, this is a careful write-up, and the depth readback numbers make the mechanism easy to follow. I agree with @Joey-Magnus's three points and won't repeat them in full. Here's where I land on each:
- Retest on current main first. #2419 merged today. It fixes the fence waits that returned after the first ~120-draw part of a split submission, so the game was rewriting uniforms that the later parts still had to read. Your Kokiri prepass alone is 558 draws, so that frame is certainly split, and the "next frame's matrices over the submitted ones" symptom is what #2419 describes. @thomas-montano's note that his Iris Xe artifacts depended on timing points the same way. If the veil is gone on main without this change, the default flip isn't needed, and
RX_VK_UB_SNAPSHOT=1stays as the opt-in it already is. If it's still there, that's a strong result and worth posting. test_env_on.py: see the inline comment for a form that passes the check.- Scope.
!mirror_on()covers more than Intel iGPUs. It's true on every non-discrete GPU (Deck, 780M/8060S APUs), on GCN dGPUs since #2343, and withRX_VK_MIRROR=0. On all of those, every UBO bind becomes a stream allocation plus a copy throughmemory_read_as_submitted, which takespools_muwhen the range spans pages. You measured no fps change on the 155H, but nobody has measured the Deck or an RX 580, where the frame budget is tighter. If the retest still shows the veil, I'd either want a number from one AMD APU and one GCN card or limit the new default to Intel for now.
No CI ran on this branch. It's 52 commits behind main, but the hunk's context is unchanged there, so it should rebase cleanly.
… no VRAM mirror With no VRAM mirror (mirror_on() off) a draw's uniform buffers are bound in the guest pools, and the GPU reads them when it runs the frame: by then the game has written the next frame's per-object matrices over them. On Intel Core Ultra iGPUs the Kokiri depth prepass drew the ground off screen and the fog pass painted sky over it (the white veil, NotRexed#999). Still there on main after NotRexed#2419. RX_VK_UB_SNAPSHOT (copy the uniforms into the stream as submitted) is now on by default on Intel when the pools are not mirrored. Other unmirrored GPUs (AMD APUs, GCN since NotRexed#2343, RX_VK_MIRROR=0) keep it off until measured. RX_VK_UB_SNAPSHOT=0/1 overrides. Assisted-by: Claude Code Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
369dfa3 to
83c18ac
Compare
|
Thanks for the reviews. I retested on current main (3e32949, with #2419), built from source with this branch rebased on it, and ran the same Kokiri scene with The veil is still there on main. Same passes, same order, "far" depth samples out of 57,600:
That is the same profile as on 6c8c1bf (~45,000 vs ~1,000), so the split-submission fence fix doesn't cover this case. Screenshots: main without / with the snapshot. Caveats:
Changes (force-pushed, one commit on current main):
Thanks also to the Iris Xe tester for checking it on Mesa ANV. |
main's NotRexed#2464 copies uniform buffers on Intel without mirrors; this keeps that and extends the default to every integrated GPU without mirrors (the Steam Deck), not to discrete GPUs without them (GCN since NotRexed#2343). The reads use env_on; a copy that could not be read is not kept for reuse. The event-wait default (e5d5054) is taken out here, to go separately with the design note. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…M mirror On Intel Core Ultra iGPUs, Kokiri Forest is covered by a white veil: the ground is missing and the fog is painted over it. With no VRAM mirror (integrated GPUs, RX_VK_MIRROR=0) a draw's uniform buffers are bound straight into the imported guest pools and read when the GPU runs the frame; by then the game has written the next frame's per-object matrices over the ones submitted. The depth prepass drew the terrain with the wrong c4 matrices, off screen, so depth stayed at far and the fog pass treated every pixel as sky. RX_VK_UB_SNAPSHOT already copies a uniform buffer into the stream as submitted (memory_read_as_submitted). It is now on by default on Intel when the pools are not mirrored (vendorID == 0x8086 && !pools_mirrored()). Other GPUs without a mirror (AMD APUs, GCN, RX_VK_MIRROR=0) keep it off until the cost is measured there. RX_VK_UB_SNAPSHOT=0 turns it off; =1 forces it on anywhere. Ported from upstream 018a1d2 (NotRexed#2464). Intel iGPU hardware not available here: the default is checked, the veil is unverified.


This is my first contribution to Hookshot, so I am happy to change anything about the approach, the style or the scope. I did the investigation and the change with the help of AI, as the commit trailers say. I checked the results on my own machine, described below.
On Intel Core Ultra iGPUs, Kokiri Forest is covered by a white veil: the ground is missing and the sky/fog is painted over it. The mechanism is not Intel-specific (any GPU whose buffer pools have no VRAM mirror can read uniform buffers stale), but the new default is limited to Intel for now.
Why
With no VRAM mirror (
mirror_on()off: integrated GPUs, GCN since #2343,RX_VK_MIRROR=0), a draw's uniform buffers are bound straight into the imported guest pools. The GPU reads them when it runs the frame. By then the game has already written the next frame's per-object matrices over the ones that were submitted.In Kokiri, the depth prepass (generic program
ffb53ed102dd9908) drew the terrain with the wrongc4matrices, so the terrain landed off screen. The depth there stayed at far (1.0). The fog/atmosphere composite (1a7482c0ec9991ea) then treats every pixel above 0.9999 as sky and painted fog over the ground. The forward pass draws the ground colour correctly, which is why only the fog layer looked wrong. On a discrete GPU the mirror is copied at submission time, so it never sees the rewrite.What changed
RX_VK_UB_SNAPSHOTalready copies a uniform buffer into the stream as submitted (memory_read_as_submitted). It is now on by default on Intel when the pools are not mirrored (vendorID == 0x8086 && !pools_mirrored()). Other GPUs without a mirror (AMD APUs, Steam Deck, GCN,RX_VK_MIRROR=0) keep it off until someone measures the cost there.RX_VK_UB_SNAPSHOT=0turns it off;=1forces it on anywhere.How it was found
RX_VK_DUMP_EACH=ffb53ed102dd9908+RX_VK_DUMP_ARGSread the depth back after each prepass draw. 551 of 558 draws added no depth at all, including 12k-24k-index terrain meshes drawn while the depth was still empty.c3/c4, put 454 of them on screen with depth ~0.99.Testing
HOS_OUTPUT_RES=3),RX_VK_FIFO=1,RX_VK_PREPARE_BACKGROUND=1.main(3e32949, with fix(vk): a fence wait covers every part of a split submission, not only the first #2419) built from source with this branch rebased on it.Related: #999 (Intel tracker); report and workaround: #999 (comment)
🤖 Generated with Claude Code