Repository navigation
Conversation
On integrated GPUs (no buffer mirrors) a uniform buffer bound in place is guest memory the game rewrites right after the submit while the GPU still runs. On the Steam Deck this drew giant HUD hearts, white and orange slabs and stretched geometry in 8-10 of 40 gameplay screenshots; with the copy, none of 40. RX_VK_UB_SNAPSHOT=0 restores the in-place binding. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A range copied as submitted serves later bindings of the same address and size in the recording while the capture view is unchanged, so a fresh read would return the same bytes. RX_VK_UB_SNAPSHOT_REUSE=0: copy every binding. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
On integrated GPUs the replay runs far behind the game, and a submission stopped at an event wait resolved texture handles against registrations made since. On the Steam Deck hearts and button glyphs came out black, misplaced or as other HUD text in most frames and the exposure stepped; in submit order every heart was right in 80 of 80 shots and the luminance spread fell from 8.6 to 1.4, at about 1.5 fps less. RX_VK_EVENT_WAITS=1 restores event ordering. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@NotRexed When you have time, could you please review this? It fixes HUD and lighting artifacts on integrated GPUs (measured on a Steam Deck); discrete GPUs keep their current behavior. Thank you! |
|
Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks! |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> # Conflicts: # tests/run_all.py
|
Merged current main (f8efe05) into the branch. The only conflict was tests/run_all.py: main now reads each test's own declarations, so the two new tests declare |
hckrman101
left a comment
There was a problem hiding this comment.
Thanks for the merge with main and for the Steam Deck numbers, they make this easy to follow.
Uniform-buffer snapshot (commits 1-2): looks right to me. Binding a UB in place on a no-mirror device reads live guest memory, which breaks design rule 2 ("nothing the replay reads comes from live guest memory"). Copying through memory_read_as_submitted is the fix that rule asks for, and discrete GPUs keep their path. On the reuse cache, I checked:
- the key is safe:
cap_view_genis bumped on every page stamp, every collected write and every capture publish, so a hit at the same view gives the bytes a fresh read would. That's the same rule the existingsubmitted_pagecache uses. ub_overlayruns before the cache, so inline UB updates later in the recording still win.- streams that are moved off stay alive for the recording (as
c1_atrelies on), and the map is cleared per recording indraw_begin_recording. curis thread_local, so there's no sharing between threads.
Nit: when memory_read_as_submitted returns false, the unread copy is now cached and served to later bindings. That's harmless in practice, since each binding used to get its own unread copy anyway.
Event waits off without mirrors (commit 3): I think this needs NotRexed's call, and I'd rather it went in separately.
docs/design.md§4 rule 1 says cross-queue waits are honoured andRX_VK_EVENT_WAITS=0is "for A/B only". This makes it the default on every no-mirror device, including Windows iGPUs nobody has tested, and the doc isn't updated. The comment abovesync_waits_onlists what the ordering fixed: lit surfaces black in 3-9% of frames, and passes reading another queue's output before it was written (55% of waits). Your 80-shot luminance numbers suggest that didn't come back in Kokiri, but that's one scene on one device.- I think your diagnosis is right, and it points at a bug for every GPU, not just iGPUs. On the default (non-async) path,
register_texture_queued/register_sampler_queuedapply at once on the game's thread. So a submission that stops at a wait and resumes later resolves handles against registrations made in the meantime, which breaks rule 3. Discrete GPUs only hit it less because the replay is less far behind. A fix that holds registrations while a queue is stopped, the wayheld_tex_releasesalready holds releases, would keep the ordering everywhere and fix the cause. Gating onmemory_mirrors_on()uses "has mirrors" as a stand-in for "the replay runs far behind": a discrete user withRX_VK_MIRROR=0silently loses event ordering, and a Deck user withRX_VK_MIRROR=1gets the bug back.
CI: the three failing Python jobs fail on test_make_game_ngx.py and test_pipeline_state_portable.py, which fail on main too (run 42942398373). The two new tests pass on Linux and are declared with RUN_ALL_ARGS / RUN_ALL_SKIP. The Windows back-end build is green.
My suggestion: land commits 1-2 as they are, and either split commit 3 out or get NotRexed's sign-off on it, with docs/design.md rule 1 updated if it stays.
| // ~1.5 fps less. RX_VK_EVENT_WAITS=1 / 0: on / off anywhere) | ||
| static const bool v = [] { | ||
| const char* e = getenv("RX_VK_EVENT_WAITS"); | ||
| const bool on = e ? atoi(e) != 0 : memory_mirrors_on(); |
There was a problem hiding this comment.
This ties the queue-ordering default to the mirror setting, so RX_VK_MIRROR now changes two unrelated things. The cause you describe (a stopped submission seeing registrations made since) looks like register_texture_queued applying immediately when !async_on(). Holding registrations while blocked_n > 0, as releases already are, might fix it for every GPU without dropping the ordering. Either way, this goes against design.md §4 rule 1, so that rule needs updating if this stays.
| if (at == UINT64_MAX) return bi; | ||
| FINE_N(FC_UBO_STREAM, 1); | ||
| // (as the submission had it, not as the game has rewritten it since: the captures' pages) | ||
| memory_read_as_submitted(u.addr, cur->mapped + at, n); |
There was a problem hiding this comment.
Nit: the return value is ignored, so with reuse on, an unreadable range's copy gets cached and reused. Not a regression (each binding used to get its own unread copy), but if (memory_read_as_submitted(...) && view) on the insert below would avoid it.
|
Related finding from Steam Deck testing with this PR's defaults (no mirrors, so |
…ly the first (#2419) On AMD integrated GPUs (ROG Ally, Legion Go, Steam Deck) the picture breaks for one frame at a time: the HUD drawn stretched across the screen, grass flat white, the whole screen flashing. The game rewrites memory the GPU is still reading, because its fence waits return after the first part of a submission. ## Cause A submission longer than `RX_VK_CHUNK` draws (default 120), or one stopped at an event wait and finished later, is recorded in several recordings that all carry its value. `submit_recording` maps the value to its recording with `value_rec.emplace`, which keeps the **first** one: when the last part finishes the submission, the key is already there and nothing changes. So `wait_recorded` (the game's fence waits in `wait()`, and the exposure readback wait) returns once the first ~120 draws are done. With no VRAM mirror of the buffer pools (integrated GPUs), the later parts read guest memory in place, and the game has by then written the next frame's uniforms, vertices and exposure meter into it. The HUD is drawn late in the frame, so it gets the wrong transforms most often; the stale exposure readback gives the whole-screen flashes. myylkn found the same root cause on a Radeon 8060S (#997, with a patch in a comment). This PR is a variant of that patch: - `find` and an update in place rather than `value_rec[v]`, so it builds against `tests/draw_stream_visibility_test.cpp`, which models `value_rec` as a `std::multimap`. - It keeps registering `r.value` when the recording does not signal (the parts split off before the last), raised to the latest seq. A wait on a stopped submission still finds its recording, instead of falling back to the timeline wait, which forces stopped work past its event waits. ## Change - `gpu/vk/vk_replay.cpp`, `submit_recording`: each value maps to the latest recording that carries it. - `tests/test_value_rec_last_part.py`: runs the statements cut from `submit_recording`, for a submission split in three parts and for a stopped submission finished in a later recording. It fails on `main` (`value 10 -> recording 1 (expected 3)`, `value 11 -> recording 4 (expected 5)`) and passes with the change. - `docs/tested-configurations.md`: the three runs below. ## Tested ROG Ally X (Ryzen Z1 Extreme, RADV PHOENIX, Mesa 26.2.3), Linux (Anatase, Fedora-based), counting one-frame luminance spikes in screen recordings (a frame brighter or darker than both its neighbours): | v2.0.13, Hyrule Field | One-frame spikes | 15/255 or more | `GPU waits in 5 s` | |---|---|---|---| | without the change (69 s) | 104 | 15 | 4–109 sync waits, 31–232 ms | | with the change (42 s) | 3 | 0 (the largest is a sword hit) | 103–137 sync waits, 1.4–1.8 s | Then about a day and a half of play with the change, rebuilt for v2.0.14 and v2.1.0 (the renderer alone, `RX_VK_ONLY=1`): no flashes or stray HUD seen in Hyrule Field, Death Mountain Trail and Goron City, in Desktop Mode and in Steam's Gaming Mode. A 57 s Gaming Mode recording on v2.0.14 had no one-frame spikes at all. **It costs frame rate on this device.** The FPS overlay in the field read roughly 45–70 before and 38–49 after (two recordings, not a controlled benchmark): part of the old frame rate was the game running ahead of the GPU. The waits now last as long as the GPU work they stand for. `python -I -B tests/run_all.py` with the Vulkan headers: the same results on a clean checkout of `main` with and without the change, apart from the new test, which passes. `test_draw_stream_visibility`, `test_capture_slot_ownership` and `test_parallel_rb_refresh`, which compile `submit_recording`, pass. **Not tested:** Windows, NVIDIA and discrete AMD cards (the early wait is there too, but their VRAM mirror is synced at the submit, which should hide it; not measured), `RX_VK_PARALLEL` segments (off by default; the same mapping applies to them), and the Steam Deck. ## Related - Part of #997 (AMD handhelds and APUs tracker). - #1795 copies uniform buffers on GPUs without mirrors for the same symptoms. The two can coexist: that one keeps a late write from reaching the GPU; this one makes the fence waits cover the work they stand for. An A/B of #1795 with this change in and `RX_VK_UB_SNAPSHOT=0` would show how much of its effect was this wait. - #2125 (merged) lets stopped work finish before a CPU readback. Complementary: the readback's wait that follows goes through `value_rec`, which this corrects. ## Screenshots Before (v2.0.13, Hyrule Field and Castle Town; each strip is the frame before, the broken frame, the frame after): <img width="1920" height="335" alt="before-1" src="https://github.com/user-attachments/assets/4d3ee32e-8575-462e-b972-b3bb16ec2f3e" /> <img width="1920" height="335" alt="before-2" src="https://github.com/user-attachments/assets/2231f72b-3394-46b0-b67d-647674712a29" /> <img width="1920" height="335" alt="before-3" src="https://github.com/user-attachments/assets/d2d645a7-bf2a-41c3-979a-19b2a36562ff" /> <img width="1920" height="335" alt="before-4" src="https://github.com/user-attachments/assets/366157ae-41e2-48a4-b4cd-7fc96f817eaa" /> After (v2.0.13 with the change, Hyrule Field, fights included; the last strip is the largest one-frame spike left, a sword hit): <img width="1920" height="333" alt="after-1" src="https://github.com/user-attachments/assets/9f025025-1034-47ab-bfe9-0ea1476db075" /> <img width="1920" height="333" alt="after-2-largest-spike-is-a-sword-hit" src="https://github.com/user-attachments/assets/e7e26d16-ba97-4982-bed0-670306352113" /> The analysis and the change were made with AI assistance (Claude); the testing and recordings are mine. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: hckrman101 <239247847+hckrman101@users.noreply.github.com>
Joey-Magnus
left a comment
There was a problem hiding this comment.
Two things on main change the picture since the Deck numbers were taken. #2419 (merged) fixed fence waits returning after the first part of a split submission, and its description lists the same Deck symptoms (stretched HUD, white grass, whole-screen flashes). Could you re-run the 40-shot test on current main with RX_VK_UB_SNAPSHOT unset? If the slabs are gone, the default flip may not be needed at all. Second, #2343 turned mirrors off by default on GCN discrete cards (vk_memory.cpp:2102). Keying on memory_mirrors_on() now also flips both defaults on RX 400/500 and Vega, and nobody has measured that. The rebase conflicts with main's env_on read at vk_draw.cpp:2774. The new reads use getenv/atoi and treat an empty RX_VK_UB_SNAPSHOT as on, which is the opposite of env_on (#2042), so please switch them to env_on. hckrman101's point on commit 3 (event waits off without mirrors, against docs/design.md rule 1) is still open. I'd split that commit out.
A dGPU has its own fast memory and a slower bus to system RAM; stock mirrors every guest pool into VRAM and keeps both copies coherent with captures, shadows and write-backs. At 4K that coherence is CPU-bound: the write-back diff compares ~3.1 GB per 600 submits and mirror syncs move ~137k dirty pages, for 35.6 fps where 1080p runs 60 (issue NotRexed#2099). Under RX_VK_DGPU=1 a pool's first binding makes no mirror (MIRROR_REFUSED, the state iGPU runs use today): reads take the lock-free TLS path and GPU reads/writes land in the imported guest memory, coherent by construction - no mirror sync, no write-back. Every 600-submit window the heaviest guest-memory-read pool with >=1 MB of reads and not write-dominated runs a measured trial (120 frames direct -> 12 settle, held until the mirror is really on -> 120 mirrored, on the recordings' own timestamps); the mirror stays only when the win is both >10% (RX_VK_DGPU_GAIN) and >0.25 ms (RX_VK_DGPU_FLOOR) on present-to-present frame time, so the verdict sees the mirror's CPU cost and its PCIe benefit alike. Trials whose windows did not measure the same workload (zero binds, a >2x bind shift, a mirror that never came) abort with no verdict and no skip line; a pool that aborts three times sits out the session, and iGPUs run no trials. A lost verdict persists to hookshot_mirror_skip.txt beside the game's own state files, keyed to the device and driver (a foreign or header-less file is ignored and re-trialled). RX_VK_DGPU_TRIALS=0 is pure direct. Trials need serial replay (documented); under RX_VK_PARALLEL the policy runs trial-less. Unset = stock exactly: the added counters and logging are gated on the switch. Refused pools read uniform buffers as submitted (NotRexed#1795's per-pool form: the snapshot engages for mirror-less pools at the read, trial-won pools keep in-place mirror binds), so the artifact class the issue documents on mirror-less GPUs is covered under the policy too. Measured on RX 9070 XT / RADV, 4K, Uncapped frame cap, warp-normalized adjacent pairs (Hyrule Field): 70.2->78.0, 71.0->77.0, 59.6->76.6 fps (+9.7% median), draw parity 705 vs 709, GPU 9.2 ms of a ~14 ms frame. Mirror-sync dirty pages 136,872 -> 0; write-back diff 3.06-3.13 GB -> 330 MB or less per 600 submits. Review round 2 (external): RX_VK_DGPU_TRIALS=0's predicate was inverted and now matches the docs (=0 is pure direct); the window's read counter moved off a shared map into the pool (dgpu_read, both direct paths); Settle holds until the mirror is actually live; zero-bind baselines and >2x bind drift between the direct and mirrored windows abort the trial un-blacklisted; keep_line refuses signed and trailing-garbage lines. Review round 3 (hckrman101, Joey-Magnus): refused pools read UBs as submitted (NotRexed#1795 per-pool, live-smoked at 1.26M copies fault-free); verdicts decide on present-to-present frame time instead of GPU-only time (the mirror's CPU and upload-buffer cost is what the frame pays); the policy runs trial-less under RX_VK_PARALLEL and its fast-path counters are parallel-safe; no trials on iGPUs and a 3-abort cap; the skip list carries a device+driver header and ignores foreign files; the test driver uses {cxx}. Rebased onto main's env_on switch rule (NotRexed#2042) and the split-submission fence work (NotRexed#2419). Co-authored-by: GLM-5.3-Flash <glm-5.3-flash@opencode.noreply.github.com>
A dGPU has its own fast memory and a slower bus to system RAM; stock mirrors every guest pool into VRAM and keeps both copies coherent with captures, shadows and write-backs. At 4K that coherence is CPU-bound: the write-back diff compares ~3.1 GB per 600 submits and mirror syncs move ~137k dirty pages, for 35.6 fps where 1080p runs 60 (issue NotRexed#2099). Under RX_VK_DGPU=1 a pool's first binding makes no mirror (MIRROR_REFUSED, the state iGPU runs use today): reads take the lock-free TLS path and GPU reads/writes land in the imported guest memory, coherent by construction - no mirror sync, no write-back. Every 600-submit window the heaviest guest-memory-read pool with >=1 MB of reads and not write-dominated runs a measured trial (120 frames direct -> 12 settle, held until the mirror is really on -> 120 mirrored, on the recordings' own timestamps); the mirror stays only when the win is both >10% (RX_VK_DGPU_GAIN) and >0.25 ms (RX_VK_DGPU_FLOOR) on present-to-present frame time, so the verdict sees the mirror's CPU cost and its PCIe benefit alike. Trials whose windows did not measure the same workload (zero binds, a >2x bind shift, a mirror that never came) abort with no verdict and no skip line; a pool that aborts three times sits out the session, and iGPUs run no trials. A lost verdict persists to hookshot_mirror_skip.txt beside the game's own state files, keyed to the device and driver (a foreign or header-less file is ignored and re-trialled). RX_VK_DGPU_TRIALS=0 is pure direct. Trials need serial replay (documented); under RX_VK_PARALLEL the policy runs trial-less. Unset = stock exactly: the added counters and logging are gated on the switch. Refused pools read uniform buffers as submitted (NotRexed#1795's per-pool form: the snapshot engages for mirror-less pools at the read, trial-won pools keep in-place mirror binds), so the artifact class the issue documents on mirror-less GPUs is covered under the policy too. Measured on RX 9070 XT / RADV, 4K, Uncapped frame cap, warp-normalized adjacent pairs (Hyrule Field): 70.2->78.0, 71.0->77.0, 59.6->76.6 fps (+9.7% median), draw parity 705 vs 709, GPU 9.2 ms of a ~14 ms frame. Mirror-sync dirty pages 136,872 -> 0; write-back diff 3.06-3.13 GB -> 330 MB or less per 600 submits. Review round 2 (external): RX_VK_DGPU_TRIALS=0's predicate was inverted and now matches the docs (=0 is pure direct); the window's read counter moved off a shared map into the pool (dgpu_read, both direct paths); Settle holds until the mirror is actually live; zero-bind baselines and >2x bind drift between the direct and mirrored windows abort the trial un-blacklisted; keep_line refuses signed and trailing-garbage lines. Review round 3 (hckrman101, Joey-Magnus): refused pools read UBs as submitted (NotRexed#1795 per-pool, live-smoked at 1.26M copies fault-free); verdicts decide on present-to-present frame time instead of GPU-only time (the mirror's CPU and upload-buffer cost is what the frame pays); the policy runs trial-less under RX_VK_PARALLEL and its fast-path counters are parallel-safe; no trials on iGPUs and a 3-abort cap; the skip list carries a device+driver header and ignores foreign files; the test driver uses {cxx}. Rebased onto main's env_on switch rule (NotRexed#2042) and the split-submission fence work (NotRexed#2419). Co-authored-by: GLM-5.3-Flash <glm-5.3-flash@opencode.noreply.github.com>
A dGPU has its own fast memory and a slower bus to system RAM; stock mirrors every guest pool into VRAM and keeps both copies coherent with captures, shadows and write-backs. At 4K that coherence is CPU-bound: the write-back diff compares ~3.1 GB per 600 submits and mirror syncs move ~137k dirty pages, for 35.6 fps where 1080p runs 60 (issue NotRexed#2099). Under RX_VK_DGPU=1 a pool's first binding makes no mirror (MIRROR_REFUSED, the state iGPU runs use today): reads take the lock-free TLS path and GPU reads/writes land in the imported guest memory, coherent by construction - no mirror sync, no write-back. Every 600-submit window the heaviest guest-memory-read pool with >=1 MB of reads and not write-dominated runs a measured trial (120 frames direct -> 12 settle, held until the mirror is really on -> 120 mirrored, on the recordings' own timestamps); the mirror stays only when the win is both >10% (RX_VK_DGPU_GAIN) and >0.25 ms (RX_VK_DGPU_FLOOR) on present-to-present frame time, so the verdict sees the mirror's CPU cost and its PCIe benefit alike. Trials whose windows did not measure the same workload (zero binds, a >2x bind shift, a mirror that never came) abort with no verdict and no skip line; a pool that aborts three times sits out the session, and iGPUs run no trials. A lost verdict persists to hookshot_mirror_skip.txt beside the game's own state files, keyed to the device and driver (a foreign or header-less file is ignored and re-trialled). RX_VK_DGPU_TRIALS=0 is pure direct. Trials need serial replay (documented); under RX_VK_PARALLEL the policy runs trial-less. Unset = stock exactly: the added counters and logging are gated on the switch. Refused pools read uniform buffers as submitted (NotRexed#1795's per-pool form: the snapshot engages for mirror-less pools at the read, trial-won pools keep in-place mirror binds), so the artifact class the issue documents on mirror-less GPUs is covered under the policy too. Measured on RX 9070 XT / RADV, 4K, Uncapped frame cap, warp-normalized adjacent pairs (Hyrule Field): 70.2->78.0, 71.0->77.0, 59.6->76.6 fps (+9.7% median), draw parity 705 vs 709, GPU 9.2 ms of a ~14 ms frame. Mirror-sync dirty pages 136,872 -> 0; write-back diff 3.06-3.13 GB -> 330 MB or less per 600 submits. Review round 2 (external): RX_VK_DGPU_TRIALS=0's predicate was inverted and now matches the docs (=0 is pure direct); the window's read counter moved off a shared map into the pool (dgpu_read, both direct paths); Settle holds until the mirror is actually live; zero-bind baselines and >2x bind drift between the direct and mirrored windows abort the trial un-blacklisted; keep_line refuses signed and trailing-garbage lines. Review round 3 (hckrman101, Joey-Magnus): refused pools read UBs as submitted (NotRexed#1795 per-pool, live-smoked at 1.26M copies fault-free); verdicts decide on present-to-present frame time instead of GPU-only time (the mirror's CPU and upload-buffer cost is what the frame pays); the policy runs trial-less under RX_VK_PARALLEL and its fast-path counters are parallel-safe; no trials on iGPUs and a 3-abort cap; the skip list carries a device+driver header and ignores foreign files; the test driver uses {cxx}. Rebased onto main's env_on switch rule (NotRexed#2042) and the split-submission fence work (NotRexed#2419); the trials' functions sit outside the write-back observation's compile-span check (NotRexed#2131) and the GCN mirror-less default (NotRexed#2343) composes unchanged (on GCN the switch runs trial-less direct plus the UB snapshot). Co-authored-by: GLM-5.3-Flash <glm-5.3-flash@opencode.noreply.github.com>
…ly the first (NotRexed#2419) On AMD integrated GPUs (ROG Ally, Legion Go, Steam Deck) the picture breaks for one frame at a time: the HUD drawn stretched across the screen, grass flat white, the whole screen flashing. The game rewrites memory the GPU is still reading, because its fence waits return after the first part of a submission. ## Cause A submission longer than `RX_VK_CHUNK` draws (default 120), or one stopped at an event wait and finished later, is recorded in several recordings that all carry its value. `submit_recording` maps the value to its recording with `value_rec.emplace`, which keeps the **first** one: when the last part finishes the submission, the key is already there and nothing changes. So `wait_recorded` (the game's fence waits in `wait()`, and the exposure readback wait) returns once the first ~120 draws are done. With no VRAM mirror of the buffer pools (integrated GPUs), the later parts read guest memory in place, and the game has by then written the next frame's uniforms, vertices and exposure meter into it. The HUD is drawn late in the frame, so it gets the wrong transforms most often; the stale exposure readback gives the whole-screen flashes. myylkn found the same root cause on a Radeon 8060S (NotRexed#997, with a patch in a comment). This PR is a variant of that patch: - `find` and an update in place rather than `value_rec[v]`, so it builds against `tests/draw_stream_visibility_test.cpp`, which models `value_rec` as a `std::multimap`. - It keeps registering `r.value` when the recording does not signal (the parts split off before the last), raised to the latest seq. A wait on a stopped submission still finds its recording, instead of falling back to the timeline wait, which forces stopped work past its event waits. ## Change - `gpu/vk/vk_replay.cpp`, `submit_recording`: each value maps to the latest recording that carries it. - `tests/test_value_rec_last_part.py`: runs the statements cut from `submit_recording`, for a submission split in three parts and for a stopped submission finished in a later recording. It fails on `main` (`value 10 -> recording 1 (expected 3)`, `value 11 -> recording 4 (expected 5)`) and passes with the change. - `docs/tested-configurations.md`: the three runs below. ## Tested ROG Ally X (Ryzen Z1 Extreme, RADV PHOENIX, Mesa 26.2.3), Linux (Anatase, Fedora-based), counting one-frame luminance spikes in screen recordings (a frame brighter or darker than both its neighbours): | v2.0.13, Hyrule Field | One-frame spikes | 15/255 or more | `GPU waits in 5 s` | |---|---|---|---| | without the change (69 s) | 104 | 15 | 4–109 sync waits, 31–232 ms | | with the change (42 s) | 3 | 0 (the largest is a sword hit) | 103–137 sync waits, 1.4–1.8 s | Then about a day and a half of play with the change, rebuilt for v2.0.14 and v2.1.0 (the renderer alone, `RX_VK_ONLY=1`): no flashes or stray HUD seen in Hyrule Field, Death Mountain Trail and Goron City, in Desktop Mode and in Steam's Gaming Mode. A 57 s Gaming Mode recording on v2.0.14 had no one-frame spikes at all. **It costs frame rate on this device.** The FPS overlay in the field read roughly 45–70 before and 38–49 after (two recordings, not a controlled benchmark): part of the old frame rate was the game running ahead of the GPU. The waits now last as long as the GPU work they stand for. `python -I -B tests/run_all.py` with the Vulkan headers: the same results on a clean checkout of `main` with and without the change, apart from the new test, which passes. `test_draw_stream_visibility`, `test_capture_slot_ownership` and `test_parallel_rb_refresh`, which compile `submit_recording`, pass. **Not tested:** Windows, NVIDIA and discrete AMD cards (the early wait is there too, but their VRAM mirror is synced at the submit, which should hide it; not measured), `RX_VK_PARALLEL` segments (off by default; the same mapping applies to them), and the Steam Deck. ## Related - Part of NotRexed#997 (AMD handhelds and APUs tracker). - NotRexed#1795 copies uniform buffers on GPUs without mirrors for the same symptoms. The two can coexist: that one keeps a late write from reaching the GPU; this one makes the fence waits cover the work they stand for. An A/B of NotRexed#1795 with this change in and `RX_VK_UB_SNAPSHOT=0` would show how much of its effect was this wait. - NotRexed#2125 (merged) lets stopped work finish before a CPU readback. Complementary: the readback's wait that follows goes through `value_rec`, which this corrects. ## Screenshots Before (v2.0.13, Hyrule Field and Castle Town; each strip is the frame before, the broken frame, the frame after): <img width="1920" height="335" alt="before-1" src="https://github.com/user-attachments/assets/4d3ee32e-8575-462e-b972-b3bb16ec2f3e" /> <img width="1920" height="335" alt="before-2" src="https://github.com/user-attachments/assets/2231f72b-3394-46b0-b67d-647674712a29" /> <img width="1920" height="335" alt="before-3" src="https://github.com/user-attachments/assets/d2d645a7-bf2a-41c3-979a-19b2a36562ff" /> <img width="1920" height="335" alt="before-4" src="https://github.com/user-attachments/assets/366157ae-41e2-48a4-b4cd-7fc96f817eaa" /> After (v2.0.13 with the change, Hyrule Field, fights included; the last strip is the largest one-frame spike left, a sword hit): <img width="1920" height="333" alt="after-1" src="https://github.com/user-attachments/assets/9f025025-1034-47ab-bfe9-0ea1476db075" /> <img width="1920" height="333" alt="after-2-largest-spike-is-a-sword-hit" src="https://github.com/user-attachments/assets/e7e26d16-ba97-4982-bed0-670306352113" /> The analysis and the change were made with AI assistance (Claude); the testing and recordings are mine. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: hckrman101 <239247847+hckrman101@users.noreply.github.com> (cherry picked from commit e0dbf40)
|
This now conflicts with main in |
The review round's bind-drift invalidation read the mirror doing its job as the workload moving: dgpu_binds counted only the binds served from guest memory, so a working mirror collapsed them (~70x, measured: 12420 before, 180 after) and the check voided the trial - 42-44 voids a run, only pools the mirror does not help ever reached a verdict, the 3-abort cap never fired (it covered only mirror-never-on), and the make/upload/ void/free churn repeated every run without converging. - dgpu_binds now counts every bind of the pool at its serving path - the fast path's direct and mirror returns, the locked direct paths, the locked mirror return - so the volume guard compares the game's actual binding work, before and after. The window reset skips the trial pool (its count is phase-owned); every other pool's counts the trailing window for the scene set. - The scene moving is its own signal: the pools each window bound meaningfully (>= 64 binds of the trailing ~600-submit window) are compared as a set at the verdict - a scene change swaps them, a working mirror does not. dgpu::ab_voided(base, binds, scene_moved) is the rule, with the measured case pinned in the fixture. - Three no-verdict endings (voids and settle give-ups share the count) sit a pool out for the session, like the abort cap: the churn cannot repeat even where detection is imperfect. Measured live: binds 240 before, 240 after with the mirror serving (was 12420 -> 180), and the third no-verdict ending sits the pool out. - The tests follow: the fixture pins the rule, the source checks read the new shapes, and the two cut-and-compile fixtures that carry the binding paths build against the changed tail (dgpu_on stubbed where the cut has no trial state; the now-unused trial stub dropped). The NotRexed#1795 UB snapshot is left as the measured-informed review found it: its only consumer is the uniform-buffer bind - exactly the class NotRexed#1795's own fix covers - and with mirrors earning again, the heavy readers' binds serve from mirrors the snapshot never sees. The re-measurement gives the honest number. Co-authored-by: GLM-5.3-Flash <glm-5.3-flash@opencode.noreply.github.com>
A dGPU has its own fast memory and a slower bus to system RAM; stock mirrors every guest pool into VRAM and keeps both copies coherent with captures, shadows and write-backs. At 4K that coherence is CPU-bound: the write-back diff compares ~3.1 GB per 600 submits and mirror syncs move ~137k dirty pages, for 35.6 fps where 1080p runs 60 (issue NotRexed#2099). Under RX_VK_DGPU=1 a pool's first binding makes no mirror (MIRROR_REFUSED, the state iGPU runs use today): reads take the lock-free TLS path and GPU reads/writes land in the imported guest memory, coherent by construction - no mirror sync, no write-back. Every 600-submit window the heaviest guest-memory-read pool with >=1 MB of reads and not write-dominated runs a measured trial (120 frames direct -> 12 settle, held until the mirror is really on -> 120 mirrored, on the recordings' own timestamps); the mirror stays only when the win is both >10% (RX_VK_DGPU_GAIN) and >0.25 ms (RX_VK_DGPU_FLOOR) on present-to-present frame time, so the verdict sees the mirror's CPU cost and its PCIe benefit alike. Trials whose windows did not measure the same workload (zero binds, a >2x bind shift, a mirror that never came) abort with no verdict and no skip line; a pool that aborts three times sits out the session, and iGPUs run no trials. A lost verdict persists to hookshot_mirror_skip.txt beside the game's own state files, keyed to the device and driver (a foreign or header-less file is ignored and re-trialled). RX_VK_DGPU_TRIALS=0 is pure direct. Trials need serial replay (documented); under RX_VK_PARALLEL the policy runs trial-less. Unset = stock exactly: the added counters and logging are gated on the switch. Refused pools read uniform buffers as submitted (NotRexed#1795's per-pool form: the snapshot engages for mirror-less pools at the read, trial-won pools keep in-place mirror binds), so the artifact class the issue documents on mirror-less GPUs is covered under the policy too. Measured on RX 9070 XT / RADV, 4K, Uncapped frame cap, warp-normalized adjacent pairs (Hyrule Field): 70.2->78.0, 71.0->77.0, 59.6->76.6 fps (+9.7% median), draw parity 705 vs 709, GPU 9.2 ms of a ~14 ms frame. Mirror-sync dirty pages 136,872 -> 0; write-back diff 3.06-3.13 GB -> 330 MB or less per 600 submits. Review round 2 (external): RX_VK_DGPU_TRIALS=0's predicate was inverted and now matches the docs (=0 is pure direct); the window's read counter moved off a shared map into the pool (dgpu_read, both direct paths); Settle holds until the mirror is actually live; zero-bind baselines and >2x bind drift between the direct and mirrored windows abort the trial un-blacklisted; keep_line refuses signed and trailing-garbage lines. Review round 3 (hckrman101, Joey-Magnus): refused pools read UBs as submitted (NotRexed#1795 per-pool, live-smoked at 1.26M copies fault-free); verdicts decide on present-to-present frame time instead of GPU-only time (the mirror's CPU and upload-buffer cost is what the frame pays); the policy runs trial-less under RX_VK_PARALLEL and its fast-path counters are parallel-safe; no trials on iGPUs and a 3-abort cap; the skip list carries a device+driver header and ignores foreign files; the test driver uses {cxx}. Follow-up fix: the drift check counted served volume, so a working mirror's read collapse voided 40+ trials a run and trials never converged; binds now count once at the serving path and the drift rule compares the scene's pool set, with a three-no-verdict cap. Rebased onto main's env_on switch rule (NotRexed#2042) and the split-submission fence work (NotRexed#2419); the trials' functions sit outside the write-back observation's compile-span check (NotRexed#2131) and the GCN mirror-less default (NotRexed#2343) composes unchanged (on GCN the switch runs trial-less direct plus the UB snapshot). Co-authored-by: GLM-5.3-Flash <glm-5.3-flash@opencode.noreply.github.com>
The review round's bind-drift invalidation read the mirror doing its job as the workload moving: dgpu_binds counted only the binds served from guest memory, so a working mirror collapsed them (~70x, measured: 12420 before, 180 after) and the check voided the trial - 42-44 voids a run, only pools the mirror does not help ever reached a verdict, the 3-abort cap never fired (it covered only mirror-never-on), and the make/upload/ void/free churn repeated every run without converging. - dgpu_binds now counts every bind of the pool at its serving path - the fast path's direct and mirror returns, the locked direct paths, the locked mirror return - so the volume guard compares the game's actual binding work, before and after. The window reset skips the trial pool (its count is phase-owned); every other pool's counts the trailing window for the scene set. - The scene moving is its own signal: the pools each window bound meaningfully (>= 64 binds of the trailing ~600-submit window) are compared as a set at the verdict - a scene change swaps them, a working mirror does not. dgpu::ab_voided(base, binds, scene_moved) is the rule, with the measured case pinned in the fixture. - Three no-verdict endings (voids and settle give-ups share the count) sit a pool out for the session, like the abort cap: the churn cannot repeat even where detection is imperfect. Measured live: binds 240 before, 240 after with the mirror serving (was 12420 -> 180), and the third no-verdict ending sits the pool out. - The tests follow: the fixture pins the rule, the source checks read the new shapes, and the two cut-and-compile fixtures that carry the binding paths build against the changed tail (dgpu_on stubbed where the cut has no trial state; the now-unused trial stub dropped). The NotRexed#1795 UB snapshot is left as the measured-informed review found it: its only consumer is the uniform-buffer bind - exactly the class NotRexed#1795's own fix covers - and with mirrors earning again, the heavy readers' binds serve from mirrors the snapshot never sees. The re-measurement gives the honest number. Co-authored-by: GLM-5.3-Flash <glm-5.3-flash@opencode.noreply.github.com>
A dGPU has its own fast memory and a slower bus to system RAM; stock mirrors every guest pool into VRAM and keeps both copies coherent with captures, shadows and write-backs. At 4K that coherence is CPU-bound: the write-back diff compares ~3.1 GB per 600 submits and mirror syncs move ~137k dirty pages, for 35.6 fps where 1080p runs 60 (issue NotRexed#2099). Under RX_VK_DGPU=1 a pool's first binding makes no mirror (MIRROR_REFUSED, the state iGPU runs use today): reads take the lock-free TLS path and GPU reads/writes land in the imported guest memory, coherent by construction - no mirror sync, no write-back. Every 600-submit window the heaviest guest-memory-read pool with >=1 MB of reads and not write-dominated runs a measured trial (120 frames direct -> 12 settle, held until the mirror is really on -> 120 mirrored, on the recordings' own timestamps); the mirror stays only when the win is both >10% (RX_VK_DGPU_GAIN) and >0.25 ms (RX_VK_DGPU_FLOOR) on present-to-present frame time, so the verdict sees the mirror's CPU cost and its PCIe benefit alike. Trials whose windows did not measure the same workload (zero binds, a >2x bind shift, a mirror that never came) abort with no verdict and no skip line; a pool that aborts three times sits out the session, and iGPUs run no trials. A lost verdict persists to hookshot_mirror_skip.txt beside the game's own state files, keyed to the device and driver (a foreign or header-less file is ignored and re-trialled). RX_VK_DGPU_TRIALS=0 is pure direct. Trials need serial replay (documented); under RX_VK_PARALLEL the policy runs trial-less. Unset = stock exactly: the added counters and logging are gated on the switch. Refused pools read uniform buffers as submitted (NotRexed#1795's per-pool form: the snapshot engages for mirror-less pools at the read, trial-won pools keep in-place mirror binds), so the artifact class the issue documents on mirror-less GPUs is covered under the policy too. Measured on RX 9070 XT / RADV, 4K, Uncapped frame cap, warp-normalized adjacent pairs (Hyrule Field): 70.2->78.0, 71.0->77.0, 59.6->76.6 fps (+9.7% median), draw parity 705 vs 709, GPU 9.2 ms of a ~14 ms frame. Mirror-sync dirty pages 136,872 -> 0; write-back diff 3.06-3.13 GB -> 330 MB or less per 600 submits. Review round 2 (external): RX_VK_DGPU_TRIALS=0's predicate was inverted and now matches the docs (=0 is pure direct); the window's read counter moved off a shared map into the pool (dgpu_read, both direct paths); Settle holds until the mirror is actually live; zero-bind baselines and >2x bind drift between the direct and mirrored windows abort the trial un-blacklisted; keep_line refuses signed and trailing-garbage lines. Review round 3 (hckrman101, Joey-Magnus): refused pools read UBs as submitted (NotRexed#1795 per-pool, live-smoked at 1.26M copies fault-free); verdicts decide on present-to-present frame time instead of GPU-only time (the mirror's CPU and upload-buffer cost is what the frame pays); the policy runs trial-less under RX_VK_PARALLEL and its fast-path counters are parallel-safe; no trials on iGPUs and a 3-abort cap; the skip list carries a device+driver header and ignores foreign files; the test driver uses {cxx}. Follow-up fix: the drift check counted served volume, so a working mirror's read collapse voided 40+ trials a run and trials never converged; binds now count once at the serving path and the drift rule compares the scene's pool set, with a three-no-verdict cap. Rebased onto main's env_on switch rule (NotRexed#2042) and the split-submission fence work (NotRexed#2419); the trials' functions sit outside the write-back observation's compile-span check (NotRexed#2131) and the GCN mirror-less default (NotRexed#2343) composes unchanged (on GCN the switch runs trial-less direct plus the UB snapshot). After the presenter-thread refactor (NotRexed#2325) the present-to-present tick lives in present_finish, which every made present passes exactly once in either threading mode, with a headless twin so offscreen runs stay measurable. Co-authored-by: GLM-5.3-Flash <glm-5.3-flash@opencode.noreply.github.com>
A dGPU has its own fast memory and a slower bus to system RAM; stock mirrors every guest pool into VRAM and keeps both copies coherent with captures, shadows and write-backs. At 4K that coherence is CPU-bound: the write-back diff compares ~3.1 GB per 600 submits and mirror syncs move ~137k dirty pages, for 35.6 fps where 1080p runs 60 (issue NotRexed#2099). Under RX_VK_DGPU=1 a pool's first binding makes no mirror (MIRROR_REFUSED, the state iGPU runs use today): reads take the lock-free TLS path and GPU reads/writes land in the imported guest memory, coherent by construction - no mirror sync, no write-back. Every 600-submit window the heaviest guest-memory-read pool with >=1 MB of reads and not write-dominated runs a measured trial (120 frames direct -> 12 settle, held until the mirror is really on -> 120 mirrored, on the recordings' own timestamps); the mirror stays only when the win is both >10% (RX_VK_DGPU_GAIN) and >0.25 ms (RX_VK_DGPU_FLOOR) on present-to-present frame time, so the verdict sees the mirror's CPU cost and its PCIe benefit alike. Trials whose windows did not measure the same workload (zero binds, a >2x bind shift, a mirror that never came) abort with no verdict and no skip line; a pool that aborts three times sits out the session, and iGPUs run no trials. A lost verdict persists to hookshot_mirror_skip.txt beside the game's own state files, keyed to the device and driver (a foreign or header-less file is ignored and re-trialled). RX_VK_DGPU_TRIALS=0 is pure direct. Trials need serial replay (documented); under RX_VK_PARALLEL the policy runs trial-less. Unset = stock exactly: the added counters and logging are gated on the switch. Refused pools read uniform buffers as submitted (NotRexed#1795's per-pool form: the snapshot engages for mirror-less pools at the read, trial-won pools keep in-place mirror binds), so the artifact class the issue documents on mirror-less GPUs is covered under the policy too. Measured on RX 9070 XT / RADV, 4K, Uncapped frame cap, warp-normalized adjacent pairs (Hyrule Field): 70.2->78.0, 71.0->77.0, 59.6->76.6 fps (+9.7% median), draw parity 705 vs 709, GPU 9.2 ms of a ~14 ms frame. Mirror-sync dirty pages 136,872 -> 0; write-back diff 3.06-3.13 GB -> 330 MB or less per 600 submits. Review round 2 (external): RX_VK_DGPU_TRIALS=0's predicate was inverted and now matches the docs (=0 is pure direct); the window's read counter moved off a shared map into the pool (dgpu_read, both direct paths); Settle holds until the mirror is actually live; zero-bind baselines and >2x bind drift between the direct and mirrored windows abort the trial un-blacklisted; keep_line refuses signed and trailing-garbage lines. Review round 3 (hckrman101, Joey-Magnus): refused pools read UBs as submitted (NotRexed#1795 per-pool, live-smoked at 1.26M copies fault-free); verdicts decide on present-to-present frame time instead of GPU-only time (the mirror's CPU and upload-buffer cost is what the frame pays); the policy runs trial-less under RX_VK_PARALLEL and its fast-path counters are parallel-safe; no trials on iGPUs and a 3-abort cap; the skip list carries a device+driver header and ignores foreign files; the test driver uses {cxx}. Follow-up fix: the drift check counted served volume, so a working mirror's read collapse voided 40+ trials a run and trials never converged; binds now count once at the serving path and the drift rule compares the scene's pool set, with a three-no-verdict cap. The set rule still voided every trial in a streaming scene, so anchors now match by guest range (a recycled pool is the same anchor), anchor at 4x the floor, void only when a substantial fraction of the scene moves, and the candidate floor rose to 256 MB of reads a window (the verdict floor is unreachable below it - a trial there is churn by construction). Rebased onto main's env_on switch rule (NotRexed#2042) and the split-submission fence work (NotRexed#2419); the trials' functions sit outside the write-back observation's compile-span check (NotRexed#2131) and the GCN mirror-less default (NotRexed#2343) composes unchanged (on GCN the switch runs trial-less direct plus the UB snapshot). After the presenter-thread refactor (NotRexed#2325) the present-to-present tick lives in present_finish, which every made present passes exactly once in either threading mode, with a headless twin so offscreen runs stay measurable. Co-authored-by: GLM-5.3-Flash <glm-5.3-flash@opencode.noreply.github.com>
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>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
@Joey-Magnus @hckrman101 Thanks for the reviews. I merged current main (
|
On integrated GPUs without VRAM mirrors, a uniform buffer bound in place is the game's own memory, and the game rewrites it right after the submit while the GPU still runs. #2464 fixed that for Intel; this PR extends the same copy to the other integrated GPUs (measured on a Steam Deck) and reuses the copy within a recording.
What it changes
RX_VK_UB_SNAPSHOTis on when the pools have no mirror and the GPU is Intel (fix(vk): snapshot uniform buffers on Intel when the buffer pools have no VRAM mirror #2464's rule, kept) or integrated (VK_PHYSICAL_DEVICE_TYPE_INTEGRATED_GPU). Discrete GPUs without mirrors (GCN since perf(vk): GCN (RX 400/500): f16 lowered where the GPU has none; no VRAM mirrors or batched submits by default (from hookshot-k10) #2343,RX_VK_MIRROR=0) keep main's default (off) until someone measures them.RX_VK_UB_SNAPSHOT_REUSE=0turns it off). A copy whose read failed is not kept.env_onreads a switch (unset uses the default; 0/off/no/false/empty is off).Steam Deck, current main (
6749ba21+ this)Kokiri Forest, Game Mode, 30 s frame logs, 80 screenshots a start. Frame generation was on, so FPS is presented; the game ran ~14-15 real.
With the snapshot off on current main, the screenshots are clean: #2419 did remove the slabs and stretched HUD seen before. The copy is still needed on the Deck for stability, at ~1 fps. The Linux suite in an Ubuntu 22.04 builder fails only what main fails there (
test_prof_clock,test_cpp_preflight: the container's g++ 11).