fix(vk): with the render thread off, a frame is no longer drawn before the data the game writes after its submit (Link dark for a frame) #769

Merged
HumanBean merged 1 commit from GummiNaomi/Longshot:fix/met-wait-capture-pr into main 2026-11-15 18:29:41 +00:00
Contributor

Type: fix · Risk: medium · Tested on: Windows 11, RTX 4070 SUPER (NVIDIA 617.42) · Needs: Linux; AMD; Intel; a PC under 12 logical processors

Why

With Threaded rendering off (what Auto picks under 12 logical processors, and the Off setting), the lit scene and Link go
dark for single frames: 21 to 40 times in a 140 s run at Castle Town's gate here, uncapped. It is the black flash of #366 on
the path the render thread does not cover. The render thread itself shows none in the same runs.

It is still there on nightly 178 (e090cc48) and on main a00c485c, which answers the question on #366 whether a nightly
from 164 on still shows it: with Threaded rendering on, no; with it off, yes.

See #366. Not the same cause as #526 (the late pages' fix writing older words over newer ones): this one is a capture taken
too early, or not at all. The two were not run together. #759 changes the same two files and will need a rebase by
whichever of the two goes in second.

How to reproduce

Seen on: Windows 11 Pro 26200, RTX 4070 SUPER 12 GB, NVIDIA 617.42
Settings and switches: RX_VK_ASYNC=0 (or Threaded rendering: Off), frame rate cap Uncapped; 2560x1440, DLAA
Steps: Castle Town's south gate: walk out to the drawbridge and back through the gate a few times
Expected: Link and the ground lit in every frame
Actual: on main, Link and the lit scene are black for one frame, about once every 600 frames; more right after a loading screen

What changed

Two causes, both of the same kind: the game writes part of a frame's data (the scene buffer's light grid among it) after
it has submitted the frame's work, then signals the event that work waits for, and the replay went ahead without that data.

  • A wait already met takes its capture (gpu/vk/vk_replay.cpp, NOP_WAIT_EVENT, cpu_signal_uncaptured, resume_queue;
    gpu/vk/vk_memory.cpp capture_make, memory_captures_begun). A queue stopped at the wait gets a capture when it goes on.
    A wait the replay found met went on with its submit's capture, and when the signal had come after that capture was begun
    the frame was lit with an empty light grid. Such a wait now stops there once and goes on through resume_queue's capture.
    Whether the signal came after: each capture is counted as begun before it collects the written pages, and
    event_signal_queued notes that count on the signalling thread. A signal an older capture already follows stops nothing.
  • Stopped queues are not made to go while the game waits for the GPU (gpu/vk/hold_gpu.h, sync_watch and wait() in
    gpu/vk/vk_replay.cpp). The game's main thread waits once a frame for the GPU to finish the frame before, and makes its
    signal after that wait. When the GPU took over RX_VK_HOLD_MS (50) for that frame, as after a load, the watch thread
    made the stopped queues go past their waits and they drew without the data. They are now held on while such a wait is
    under way and for RX_VK_HOLD_MS after it, 1000 ms at most.

Reviewers: start with the comment above ev_cpu_captures in vk_replay.cpp, then hold_gpu.h.

Measurements

Verdict: dark frames on the submitting thread go from 126 in four runs (median 32.5 a run) to 0 in four runs; no cost
in frame rate or in long frames that these runs can show.

Hardware: Windows 11 Pro 26200, RTX 4070 SUPER 12 GB, NVIDIA 617.42, Ryzen 9 9950X3D (16 cores, SMT off), 32 GB
Scene: Castle Town's south gate, out to the drawbridge and back several times (each run crosses a few loading screens), day,
then night at 90 s; 2560x1440, DLAA, no frame generation, uncapped
Method: 140 s scripted runs, three sides in turn, four rounds. A dark frame is counted by a local counter (not in this PR):
a frame in which the main scene buffer (the 0x3500-byte c3) had an empty light grid word at every one of its bindings;
single frames or pairs only. Frame rate: mean over the run from HOS_PRESENT_TRACE, loads included. Hold timeouts: the
50 ms count of the log's queues ordered by their events line.
How to reproduce: save set "First Step into Hyrule Town", the route above, the switches in the column headers.

Builds: this PR's source (the working tree that became 8d516bf5) plus the local counter. Rounds 1 to 3 on main e090cc48,
round 4 on main a00c485c. The commit itself is on main aa95d35f (#761 came in between and also changes vk_replay.cpp):
that build was run once on the same route to see it start and play, without the counter, so it has no dark-frame count.
Baseline: the same build with both changes switched off, because the count needs the local counter in the build; main
without this PR was run the same way on 6c9edffe (six rounds, below).

Before, RX_VK_ASYNC=0 RX_VK_MET_WAIT_CAPTURE=0 RX_VK_HOLD_GPU_WAIT=0: this PR @ 8d516bf5 After, RX_VK_ASYNC=0: this PR @ 8d516bf5 After, RX_VK_ASYNC=1 (the render thread): this PR @ 8d516bf5
dark frames, rounds 1 / 2 / 3 / 4 40 / 28 / 21 / 37 (median 32.5) 0 / 0 / 0 / 0 0 / 0 / 0 / 0
hold timeouts 3 / 2 / 1 / 2 0 / 0 / 0 / 0 0 / 0 / 1 / 0
mean fps 121.3 / 125.1 / 123.2 / 141.3 (median 124.2) 118.5 / 132.2 / 129.5 / 130.5 (median 130.0) 143.7 / 138.5 / 146.5 / 197.7 (median 145.1)
frames over 50 ms 35 / 37 / 42 / 48 35 / 37 / 40 / 36 33 / 34 / 29 / 54
waits met on arrival that stopped for a capture 0 526 / 743 / 478 / 408 0
times the stopped queues were held for a GPU wait 0 1 / 2 / 1 / 2 0

Round 4 is on a newer main than rounds 1 to 3, so compare its three sides with each other, not with the earlier rounds
(the render thread's 197.7 fps is that round's; why it is higher than rounds 1 to 3 was not looked into).

Each change alone, on main 6c9edffe plus the changes (the same route and method):

main's behaviour the capture alone both
dark frames 156 in six runs (20, 31, 25, 25, 31, 24) 13 in six runs (7, 1, 1, 1, 1, 2) 0 in five runs
mean fps 124.0 121.3 129.5 (against 127.6 with the capture alone, in turn)

The 13 left with the capture alone were the hold timeouts, run by run.

Round 1's log lines, before and after:

vk: queues ordered by their events: 90000 submissions stopped at a wait, 89966 went on once it was reached, 32 waits gone past; made to go by presents 1138, releases 0, waits for the replay 0, 50 ms 3 (stopped 51.01 ms at most); waits met 216467, not met 0; captures at a CPU event's wait 6080, 0 of them at a wait met on arrival
vk: queues ordered by their events: 90000 submissions stopped at a wait, 89999 went on once it was reached, 0 waits gone past; made to go by presents 1067, releases 0, waits for the replay 0, 50 ms 0 (stopped 54.00 ms at most); waits met 217613, not met 0; captures at a CPU event's wait 6710, 526 of them at a wait met on arrival
vk: stopped queues held past 50 ms while a thread waits for the GPU: 1 times so far (0 of them made to go at 1000 ms)

Flags and settings

Three switches, all for testing; no menu row, no ini key.

Flag What it is Before this PR After this PR Effect Helps / doesn't help / untested Recommendation
RX_VK_MET_WAIT_CAPTURE=0 A met wait for a CPU event goes on with its submit's capture whenever it is met (the behaviour) off unless set: the capture is taken dark frames back helps: the submitting thread; no effect with the render thread; untested: AMD, Intel, Linux leave unset; set only to compare
RX_VK_HOLD_GPU_WAIT=0 Stopped queues are made to go at RX_VK_HOLD_MS whoever waits (the behaviour) off unless set: held while the game waits for the GPU a dark frame at a long GPU frame back as above leave unset; set only to compare
RX_VK_HOLD_GPU_MS=<ms> The most the stopped queues are held for such a wait - 1000 a longer or shorter bound untested at other values leave

All three are in docs/development.md.

Testing

Tested:

  • Windows 11, RTX 4070 SUPER (617.42): the runs above; 11 more runs of the capture alone and 16 of the hold on 6c9edffe.

  • python -I -B tests/run_all.py on this PR's commit: 323 passed, 80 failed, 192 skipped (no gcc or g++ on this PC). On untouched main aa95d35f: 321 passed, 80 failed, 192 skipped. The 80 are the same 80 on both (several are link errors of this PC's clang; they were not looked into one by one); the two more that pass here are this PR's new tests.

    The 80 that fail on both

    test_alt_enter, test_appimage_player_build, test_audio_deadline, test_audio_output_errors, test_audio_wave_buffer_release, test_bake_portability, test_bc_levels, test_bind_memo, test_capture_copy_helpers, test_collect_wide, test_cpm_discovery, test_crash_dialog, test_dlss_actor, test_dlss_checks, test_dlss_math, test_draw_binds, test_env_file, test_env_on, test_fps_overlay_order, test_fps_rate, test_fps_stats, test_frame_whole, test_framegen_auto, test_framegen_gate, test_fsr3_math, test_game_font, test_gpu_choice, test_hd_rumble, test_high_fps_fixes, test_import_span, test_jobs_wait_runs_queued, test_lift_cleanup_error, test_linux_preflight, test_make_game_cache, test_make_game_prepare, test_menu_mouse_scroll, test_mirror_inherit, test_missing_nup, test_movie_upscale_policy, test_oot_jitter, test_options_budget_once, test_pass_hazard, test_pool_span, test_precompile_compute_guard, test_present_ahead, test_present_thread, test_prof_clock, test_program_cache_cleanup, test_prompt_auto_face, test_psychov35_default, test_psychov35_dlss, test_push_state, test_quick_pipelines, test_readback_late, test_report_words, test_rumble_mapping, test_rumble_menu, test_rumble_refresh, test_same_bind, test_same_draw, test_same_dyn, test_seed_reader, test_shader_module_guard, test_skip_code, test_status_line, test_submitted_page, test_table_track, test_task_mesh_fallback_devices, test_tested_configurations, test_tex_spans_once_a_present, test_texture_layout_sizes, test_ub_gap, test_upscale_independent, test_upscale_label, test_utf8_paths, test_vertex_bind, test_vk_occlusion_queries, test_vk_pgo, test_walk_key, test_wb_pages

  • With this PC's clang wrappers in place of g++, the 137 checks that read the changed files (on main a00c485c): 125 pass; 12 fail, and the same 12 fail the same way on untouched main: test_audio_volume, test_crash_dialog, test_crash_record, test_oot_fov, test_oot_state_tap, test_present_aspect_vulkan, test_present_thread, test_readback_late, test_readback_wait_bounded, test_text_size, test_vk_pgo, test_walk_key.

  • After the rebase onto aa95d35f: the two new tests and the eight checks nearest #761's change (test_vk_occlusion_queries, test_recorder_sync, test_draw_stream_visibility, test_fsr_image_retirement, test_flush_nothing_held, test_recorded_wait_flush, test_late_fix_build, test_readback_part) pass; the build links and plays the route.

  • New: tests/test_met_wait_capture.py (source), tests/test_hold_gpu.py with tests/hold_gpu_test.cpp (the rule compiled and run). Both fail on main (the code they read is not there).

  • tests/test_tested_configurations.py: 24 of 25 pass here; the one failure is its CLI pipe test's line endings on Windows, which reads none of this PR's files.

  • scripts/check_guide_screenshots.py --review-sources already asks for a review on main aa95d35f (14 files, gpu/vk/vk_internal.h among them), and asks for the same 14 with this PR. This PR adds one declaration to that header and changes no menu.

Not tested:

  • Linux, AMD (RADV and Windows), Intel: no hardware here beyond NVIDIA on Windows.
  • A PC under 12 logical processors: the submitting thread was forced with RX_VK_ASYNC=0 on a 16-core PC.
  • The picture itself in these runs: dark frames were counted by the light grid's state, which was matched to the dark-Link
    frames by eye on an older base (150 recorded frames, the block empty in exactly the 3 dark ones), not on this one.
  • A frame rate cap of 60: all runs were uncapped.
  • Any area but the gate and its two sides (Castle Town, Hyrule Field at the drawbridge).
  • This change together with #526.

Must be tested:

  • Linux, any GPU, Threaded rendering Off (or under 12 logical processors): Castle Town's south gate, in and out of the gate for a few minutes, expecting no single dark frame and the log's 50 ms count at 0 (Test Farm: the gate with RX_VK_ASYNC=0).
  • AMD, Windows or Linux, Threaded rendering Off: the same gate, in and out for a few minutes, expecting no single dark frame and the log's 50 ms count at 0 (by hand: watch Link each time the gate's loading screen ends).
  • A PC with 4 to 8 cores: the same, with Threaded rendering on Auto, expecting no dark frame and no new hitch at loads (by hand: watch Link through three area changes).

Risks

What could break: a queue held longer than before at a long GPU frame: the frame that would have been dark comes up to
RX_VK_HOLD_GPU_MS later instead (in the runs: 14 holds, none reached the bound). A game thread that waits for the GPU for
ever would hold stopped queues for 1000 ms each time instead of 50. The met-wait stop adds 400 to 750 captures a run.

Turning it off: RX_VK_MET_WAIT_CAPTURE=0, RX_VK_HOLD_GPU_WAIT=0; or revert the commit.

Compatibility: Nothing changes: no cache, save, option or ABI is touched.

Issues

See #366

Checklist

Everyone

  • One topic.
  • The title is a Conventional Commit (type(scope): description) of the status line's type.
  • Why gives the reason plainly and links the issues that fit (the tracker searched).
  • Measurements has a verdict, or one line saying why there is nothing to measure.
  • Testing names what was tested, what wasn't, and what must still be tested, where and how.
  • Rebased on current main.
  • python -I -B tests/run_all.py passes (any failure that also fails on main is named). (80 fail here and the same 80 on main, named under Testing)
  • No game files, dumps, build output or generated files.
  • third_party/ untouched (a new folder with a PROVENANCE.md instead).
  • The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

When it applies

  • AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
  • Numbers from the same PC, scene and settings, at least 2 interleaved runs each, the median reported.
  • Switches documented (docs/development.md; the README for a menu row) and shipped at their recommended value.
  • A file in docs/tested/ for each new measured run (format in docs/tested-configurations.md): twelve files, 2026-11-15-met-wait-hold-gumminaomi-{ta,sa,sb}{1,2,3,4}.md.
  • A game-free test added for the fix, failing on main and passing here.
  • Docs updated.

🤖 Generated with Claude Code

**Type:** fix · **Risk:** medium · **Tested on:** Windows 11, RTX 4070 SUPER (NVIDIA 617.42) · **Needs:** Linux; AMD; Intel; a PC under 12 logical processors ## Why With Threaded rendering off (what Auto picks under 12 logical processors, and the Off setting), the lit scene and Link go dark for single frames: 21 to 40 times in a 140 s run at Castle Town's gate here, uncapped. It is the black flash of #366 on the path the render thread does not cover. The render thread itself shows none in the same runs. It is still there on nightly 178 (e090cc48) and on main a00c485c, which answers the question on #366 whether a nightly from 164 on still shows it: with Threaded rendering on, no; with it off, yes. See #366. Not the same cause as #526 (the late pages' fix writing older words over newer ones): this one is a capture taken too early, or not at all. The two were not run together. #759 changes the same two files and will need a rebase by whichever of the two goes in second. ## How to reproduce **Seen on:** Windows 11 Pro 26200, RTX 4070 SUPER 12 GB, NVIDIA 617.42 **Settings and switches:** `RX_VK_ASYNC=0` (or Threaded rendering: Off), frame rate cap Uncapped; 2560x1440, DLAA **Steps:** Castle Town's south gate: walk out to the drawbridge and back through the gate a few times **Expected:** Link and the ground lit in every frame **Actual:** on main, Link and the lit scene are black for one frame, about once every 600 frames; more right after a loading screen ## What changed Two causes, both of the same kind: the game writes part of a frame's data (the scene buffer's light grid among it) after it has submitted the frame's work, then signals the event that work waits for, and the replay went ahead without that data. - **A wait already met takes its capture** (`gpu/vk/vk_replay.cpp`, `NOP_WAIT_EVENT`, `cpu_signal_uncaptured`, `resume_queue`; `gpu/vk/vk_memory.cpp` `capture_make`, `memory_captures_begun`). A queue stopped at the wait gets a capture when it goes on. A wait the replay found met went on with its submit's capture, and when the signal had come after that capture was begun the frame was lit with an empty light grid. Such a wait now stops there once and goes on through `resume_queue`'s capture. Whether the signal came after: each capture is counted as begun before it collects the written pages, and `event_signal_queued` notes that count on the signalling thread. A signal an older capture already follows stops nothing. - **Stopped queues are not made to go while the game waits for the GPU** (`gpu/vk/hold_gpu.h`, `sync_watch` and `wait()` in `gpu/vk/vk_replay.cpp`). The game's main thread waits once a frame for the GPU to finish the frame before, and makes its signal after that wait. When the GPU took over `RX_VK_HOLD_MS` (50) for that frame, as after a load, the watch thread made the stopped queues go past their waits and they drew without the data. They are now held on while such a wait is under way and for `RX_VK_HOLD_MS` after it, 1000 ms at most. Reviewers: start with the comment above `ev_cpu_captures` in `vk_replay.cpp`, then `hold_gpu.h`. ## Measurements **Verdict:** dark frames on the submitting thread go from 126 in four runs (median 32.5 a run) to 0 in four runs; no cost in frame rate or in long frames that these runs can show. Hardware: Windows 11 Pro 26200, RTX 4070 SUPER 12 GB, NVIDIA 617.42, Ryzen 9 9950X3D (16 cores, SMT off), 32 GB Scene: Castle Town's south gate, out to the drawbridge and back several times (each run crosses a few loading screens), day, then night at 90 s; 2560x1440, DLAA, no frame generation, uncapped Method: 140 s scripted runs, three sides in turn, four rounds. A dark frame is counted by a local counter (not in this PR): a frame in which the main scene buffer (the 0x3500-byte `c3`) had an empty light grid word at every one of its bindings; single frames or pairs only. Frame rate: mean over the run from `HOS_PRESENT_TRACE`, loads included. Hold timeouts: the `50 ms` count of the log's `queues ordered by their events` line. How to reproduce: save set "First Step into Hyrule Town", the route above, the switches in the column headers. Builds: this PR's source (the working tree that became `8d516bf5`) plus the local counter. Rounds 1 to 3 on main e090cc48, round 4 on main a00c485c. The commit itself is on main aa95d35f (#761 came in between and also changes `vk_replay.cpp`): that build was run once on the same route to see it start and play, without the counter, so it has no dark-frame count. Baseline: the same build with both changes switched off, because the count needs the local counter in the build; main without this PR was run the same way on 6c9edffe (six rounds, below). | | Before, `RX_VK_ASYNC=0 RX_VK_MET_WAIT_CAPTURE=0 RX_VK_HOLD_GPU_WAIT=0`: this PR @ 8d516bf5 | After, `RX_VK_ASYNC=0`: this PR @ 8d516bf5 | After, `RX_VK_ASYNC=1` (the render thread): this PR @ 8d516bf5 | |---|---|---|---| | dark frames, rounds 1 / 2 / 3 / 4 | 40 / 28 / 21 / 37 (median 32.5) | 0 / 0 / 0 / 0 | 0 / 0 / 0 / 0 | | hold timeouts | 3 / 2 / 1 / 2 | 0 / 0 / 0 / 0 | 0 / 0 / 1 / 0 | | mean fps | 121.3 / 125.1 / 123.2 / 141.3 (median 124.2) | 118.5 / 132.2 / 129.5 / 130.5 (median 130.0) | 143.7 / 138.5 / 146.5 / 197.7 (median 145.1) | | frames over 50 ms | 35 / 37 / 42 / 48 | 35 / 37 / 40 / 36 | 33 / 34 / 29 / 54 | | waits met on arrival that stopped for a capture | 0 | 526 / 743 / 478 / 408 | 0 | | times the stopped queues were held for a GPU wait | 0 | 1 / 2 / 1 / 2 | 0 | Round 4 is on a newer main than rounds 1 to 3, so compare its three sides with each other, not with the earlier rounds (the render thread's 197.7 fps is that round's; why it is higher than rounds 1 to 3 was not looked into). Each change alone, on main 6c9edffe plus the changes (the same route and method): | | main's behaviour | the capture alone | both | |---|---|---|---| | dark frames | 156 in six runs (20, 31, 25, 25, 31, 24) | 13 in six runs (7, 1, 1, 1, 1, 2) | 0 in five runs | | mean fps | 124.0 | 121.3 | 129.5 (against 127.6 with the capture alone, in turn) | The 13 left with the capture alone were the hold timeouts, run by run. Round 1's log lines, before and after: ``` vk: queues ordered by their events: 90000 submissions stopped at a wait, 89966 went on once it was reached, 32 waits gone past; made to go by presents 1138, releases 0, waits for the replay 0, 50 ms 3 (stopped 51.01 ms at most); waits met 216467, not met 0; captures at a CPU event's wait 6080, 0 of them at a wait met on arrival vk: queues ordered by their events: 90000 submissions stopped at a wait, 89999 went on once it was reached, 0 waits gone past; made to go by presents 1067, releases 0, waits for the replay 0, 50 ms 0 (stopped 54.00 ms at most); waits met 217613, not met 0; captures at a CPU event's wait 6710, 526 of them at a wait met on arrival vk: stopped queues held past 50 ms while a thread waits for the GPU: 1 times so far (0 of them made to go at 1000 ms) ``` ### Flags and settings Three switches, all for testing; no menu row, no ini key. | Flag | What it is | Before this PR | After this PR | Effect | Helps / doesn't help / untested | Recommendation | |---|---|---|---|---|---|---| | `RX_VK_MET_WAIT_CAPTURE=0` | A met wait for a CPU event goes on with its submit's capture whenever it is met | (the behaviour) | off unless set: the capture is taken | dark frames back | helps: the submitting thread; no effect with the render thread; untested: AMD, Intel, Linux | leave unset; set only to compare | | `RX_VK_HOLD_GPU_WAIT=0` | Stopped queues are made to go at `RX_VK_HOLD_MS` whoever waits | (the behaviour) | off unless set: held while the game waits for the GPU | a dark frame at a long GPU frame back | as above | leave unset; set only to compare | | `RX_VK_HOLD_GPU_MS=<ms>` | The most the stopped queues are held for such a wait | - | 1000 | a longer or shorter bound | untested at other values | leave | All three are in `docs/development.md`. ## Testing **Tested:** - Windows 11, RTX 4070 SUPER (617.42): the runs above; 11 more runs of the capture alone and 16 of the hold on 6c9edffe. - `python -I -B tests/run_all.py` on this PR's commit: 323 passed, 80 failed, 192 skipped (no gcc or g++ on this PC). On untouched main aa95d35f: 321 passed, 80 failed, 192 skipped. The 80 are the same 80 on both (several are link errors of this PC's clang; they were not looked into one by one); the two more that pass here are this PR's new tests. <details><summary>The 80 that fail on both</summary> `test_alt_enter`, `test_appimage_player_build`, `test_audio_deadline`, `test_audio_output_errors`, `test_audio_wave_buffer_release`, `test_bake_portability`, `test_bc_levels`, `test_bind_memo`, `test_capture_copy_helpers`, `test_collect_wide`, `test_cpm_discovery`, `test_crash_dialog`, `test_dlss_actor`, `test_dlss_checks`, `test_dlss_math`, `test_draw_binds`, `test_env_file`, `test_env_on`, `test_fps_overlay_order`, `test_fps_rate`, `test_fps_stats`, `test_frame_whole`, `test_framegen_auto`, `test_framegen_gate`, `test_fsr3_math`, `test_game_font`, `test_gpu_choice`, `test_hd_rumble`, `test_high_fps_fixes`, `test_import_span`, `test_jobs_wait_runs_queued`, `test_lift_cleanup_error`, `test_linux_preflight`, `test_make_game_cache`, `test_make_game_prepare`, `test_menu_mouse_scroll`, `test_mirror_inherit`, `test_missing_nup`, `test_movie_upscale_policy`, `test_oot_jitter`, `test_options_budget_once`, `test_pass_hazard`, `test_pool_span`, `test_precompile_compute_guard`, `test_present_ahead`, `test_present_thread`, `test_prof_clock`, `test_program_cache_cleanup`, `test_prompt_auto_face`, `test_psychov35_default`, `test_psychov35_dlss`, `test_push_state`, `test_quick_pipelines`, `test_readback_late`, `test_report_words`, `test_rumble_mapping`, `test_rumble_menu`, `test_rumble_refresh`, `test_same_bind`, `test_same_draw`, `test_same_dyn`, `test_seed_reader`, `test_shader_module_guard`, `test_skip_code`, `test_status_line`, `test_submitted_page`, `test_table_track`, `test_task_mesh_fallback_devices`, `test_tested_configurations`, `test_tex_spans_once_a_present`, `test_texture_layout_sizes`, `test_ub_gap`, `test_upscale_independent`, `test_upscale_label`, `test_utf8_paths`, `test_vertex_bind`, `test_vk_occlusion_queries`, `test_vk_pgo`, `test_walk_key`, `test_wb_pages` </details> - With this PC's clang wrappers in place of g++, the 137 checks that read the changed files (on main a00c485c): 125 pass; 12 fail, and the same 12 fail the same way on untouched main: `test_audio_volume`, `test_crash_dialog`, `test_crash_record`, `test_oot_fov`, `test_oot_state_tap`, `test_present_aspect_vulkan`, `test_present_thread`, `test_readback_late`, `test_readback_wait_bounded`, `test_text_size`, `test_vk_pgo`, `test_walk_key`. - After the rebase onto aa95d35f: the two new tests and the eight checks nearest #761's change (`test_vk_occlusion_queries`, `test_recorder_sync`, `test_draw_stream_visibility`, `test_fsr_image_retirement`, `test_flush_nothing_held`, `test_recorded_wait_flush`, `test_late_fix_build`, `test_readback_part`) pass; the build links and plays the route. - New: `tests/test_met_wait_capture.py` (source), `tests/test_hold_gpu.py` with `tests/hold_gpu_test.cpp` (the rule compiled and run). Both fail on main (the code they read is not there). - `tests/test_tested_configurations.py`: 24 of 25 pass here; the one failure is its CLI pipe test's line endings on Windows, which reads none of this PR's files. - `scripts/check_guide_screenshots.py --review-sources` already asks for a review on main aa95d35f (14 files, `gpu/vk/vk_internal.h` among them), and asks for the same 14 with this PR. This PR adds one declaration to that header and changes no menu. **Not tested:** - Linux, AMD (RADV and Windows), Intel: no hardware here beyond NVIDIA on Windows. - A PC under 12 logical processors: the submitting thread was forced with `RX_VK_ASYNC=0` on a 16-core PC. - The picture itself in these runs: dark frames were counted by the light grid's state, which was matched to the dark-Link frames by eye on an older base (150 recorded frames, the block empty in exactly the 3 dark ones), not on this one. - A frame rate cap of 60: all runs were uncapped. - Any area but the gate and its two sides (Castle Town, Hyrule Field at the drawbridge). - This change together with #526. **Must be tested:** - Linux, any GPU, Threaded rendering Off (or under 12 logical processors): Castle Town's south gate, in and out of the gate for a few minutes, expecting no single dark frame and the log's `50 ms` count at 0 (Test Farm: the gate with `RX_VK_ASYNC=0`). - AMD, Windows or Linux, Threaded rendering Off: the same gate, in and out for a few minutes, expecting no single dark frame and the log's `50 ms` count at 0 (by hand: watch Link each time the gate's loading screen ends). - A PC with 4 to 8 cores: the same, with Threaded rendering on Auto, expecting no dark frame and no new hitch at loads (by hand: watch Link through three area changes). ## Risks **What could break:** a queue held longer than before at a long GPU frame: the frame that would have been dark comes up to `RX_VK_HOLD_GPU_MS` later instead (in the runs: 14 holds, none reached the bound). A game thread that waits for the GPU for ever would hold stopped queues for 1000 ms each time instead of 50. The met-wait stop adds 400 to 750 captures a run. **Turning it off:** `RX_VK_MET_WAIT_CAPTURE=0`, `RX_VK_HOLD_GPU_WAIT=0`; or revert the commit. **Compatibility:** Nothing changes: no cache, save, option or ABI is touched. ## Issues See #366 ## Checklist ### Everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`) of the status line's type. - [x] Why gives the reason plainly and links the issues that fit (the tracker searched). - [x] Measurements has a verdict, or one line saying why there is nothing to measure. - [x] Testing names what was tested, what wasn't, and what must still be tested, where and how. - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` passes (any failure that also fails on `main` is named). (80 fail here and the same 80 on `main`, named under Testing) - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### When it applies - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] Numbers from the same PC, scene and settings, at least 2 interleaved runs each, the median reported. - [x] Switches documented (docs/development.md; the README for a menu row) and shipped at their recommended value. - [x] A file in `docs/tested/` for each new measured run (format in `docs/tested-configurations.md`): twelve files, `2026-11-15-met-wait-hold-gumminaomi-{ta,sa,sb}{1,2,3,4}.md`. - [x] A game-free test added for the fix, failing on `main` and passing here. - [x] Docs updated. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
fix(vk): with the render thread off, a frame is no longer drawn before the data the game writes after its submit (Link dark for a frame)
Some checks are pending
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
CI / Conventional Commit title (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
Guide screenshots / Guide images and local UI source review (pull_request) Blocked by required conditions
Third-party parts / As their notes say (pull_request) Blocked by required conditions
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
Website / Website on Cloudflare (pull_request) Blocked by required conditions
PR description / Description follows the template (pull_request_target) Successful in 16s
Website / Website on Cloudflare (pull_request_target) Has been skipped
8d516bf5f9
With Threaded rendering off (Auto under 12 logical processors, and the Off
setting) the lit scene and Link went dark for single frames: the black flash
of #366 on the path the render thread does not cover. The game writes part of
a frame's data (the scene buffer's light grid among it) after it has submitted
the frame's work, then signals the event that work waits for, and the replay
went ahead without that data in two cases:

- A wait for a CPU event that the replay found met went on with its submit's
  capture. When the CPU's signal came after that capture was begun, the wait
  now stops there once and goes on through resume_queue's capture
  (cpu_signal_uncaptured, memory_captures_begun). RX_VK_MET_WAIT_CAPTURE=0:
  as before.
- Queues stopped at a wait were made to go after RX_VK_HOLD_MS (50) even while
  the game's main thread sat in its once-a-frame wait for the GPU, after which
  it makes the signal they wait for (a GPU frame of ~100 ms after a load).
  They are now held on while such a wait is under way and for RX_VK_HOLD_MS
  after it, RX_VK_HOLD_GPU_MS (1000) at most; the rule is gpu/vk/hold_gpu.h.
  RX_VK_HOLD_GPU_WAIT=0: as before.

Castle Town's south gate, RTX 4070 SUPER, Windows 11, 140 s runs, the
submitting thread: 89 dark frames in three runs before, 0 in three after; no
cost in frame rate or long frames that the runs show. Not run on Linux, AMD or
Intel.

Tests: tests/test_met_wait_capture.py (source), tests/test_hold_gpu.py with
tests/hold_gpu_test.cpp (the rule compiled and run). The three switches are in
docs/development.md; the measured runs in docs/tested/.

See #366

Assisted-by: Claude Mythos 5.2
Co-Authored-By: Claude Mythos 5.2 <[email protected]>
GummiNaomi changed title from WIP: fix(vk): with the render thread off, a frame is no longer drawn before the data the game writes after its submit (Link dark for a frame) to fix(vk): with the render thread off, a frame is no longer drawn before the data the game writes after its submit (Link dark for a frame) 2026-11-15 11:05:31 +00:00
HumanBean left a comment

First review, on code only (no tests run here). The fix looks correct and I found nothing that blocks it. I'm posting a comment rather than an approval for one reason: the hardware runs the PR lists under Must be tested (Linux; AMD; Intel or a PC under 12 logical processors) haven't happened yet, and I found no test request for them.

What I checked

  • The met-wait stop, ordering and deadlock. When a wait is met on arrival and the CPU's signal came after the newest capture was begun, the replay stops at that wait. The stop never waits on the game: the wait is already met, so the schedule() that every stopping caller runs next (submit_job, drain, sync_watch) resumes it straight away through resume_queue. That takes memory_capture_now, and met_resumed_at lets the wait through on the resumed run, so each wait stops at most once. A later wait in the same resumed run can only stop again if a new signal came after that capture.
  • Capture accounting. The count is bumped before the written pages are collected, and the signal records the count under ev_cpu_mu. So a capture that began before the signal is never treated as covering it, which errs on the safe side. Captures on the submitting thread are taken under sync_order_mu and their memory work is applied in submit order, so "begun after the signal" does mean the data is in the mirrors before the stopped work replays.
  • The GPU-wait hold, what bounds it. GpuWaiter counts a thread only in the mine branch, where none of the queue's work up to f is stopped. The Vulkan work that wait needs therefore cannot depend on the held queues, and wait_rec_seq flushes it first, so there is no cycle. Three things bound the hold: RX_VK_HOLD_GPU_MS (1000 ms, never under hold_ms) counted from when the queues stopped; presents and releases, whose drains are unchanged; and a wait that needs stopped work, which wait() still forces after RX_VK_WAIT_HOLD_US. A poll (timeout of 1 ms or less) is not counted. The remaining cost is the one Risks names: a thread that only polls for stopped work, while another thread waits on the GPU, now waits up to 1000 ms instead of 50.
  • The render-thread path is unchanged. met_wait_capture_on() is false when async_on(). ev_cpu_captures is written only in the submitting thread's branch of event_signal_queued. GpuWaiter sits inside the !async_on() block, and sync_watch only runs in sync mode. The resume_capture change is a straight move of the same static into resume_capture_on(). Setting met_resumed_at on the render thread is harmless.
  • Frame-time cost on the path that now waits. The stop adds 400 to 750 recording splits and captures per 140 s run, about 3 to 5 a second. Most of the bytes would otherwise have gone into the next submit's capture. Each met wait also does one ev_cpu_mu lookup. The measured fps (median 124.2 before, 130.0 after) and the frames over 50 ms show no cost. That is within noise, as the PR says.
  • Switches. Both fixes are on by default and the switches only turn them off for comparison; there are no player knobs. Fine.

Two non-blocking nits are inline: the met-wait stop is still on with RX_VK_CAPTURE=0, and the "made to go at 1000 ms" count also includes holds that ended normally.

Rebase: main has moved since aa95d35f (#526, #552, #489 and others touch vk_memory.cpp, vk_replay.cpp and vk_internal.h). The PR still rebases onto main ced37d40 without conflicts and Forgejo reports it mergeable, so I didn't push anything. Note that #526 is now on main and your runs predate it. The PR already lists "this change together with #526" as not tested, so the runs below should be on a build of this PR on current main.

Before merge (a game run): Castle Town's south gate with Threaded rendering Off (RX_VK_ASYNC=0), going in and out through the gate's loads for a few minutes, expecting no single dark frame and the log's 50 ms count at 0. Needed on Linux (any GPU) and on AMD, plus one PC with 4 to 8 cores on Auto, as the PR's Testing section says. Please post a test request for these (scripts/test_requests.py request --pr 769 ...) or ask for the testing wanted label. With those runs in the comments, this can be approved as it is.

First review, on code only (no tests run here). The fix looks correct and I found nothing that blocks it. I'm posting a comment rather than an approval for one reason: the hardware runs the PR lists under **Must be tested** (Linux; AMD; Intel or a PC under 12 logical processors) haven't happened yet, and I found no test request for them. **What I checked** - **The met-wait stop, ordering and deadlock.** When a wait is met on arrival and the CPU's signal came after the newest capture was begun, the replay stops at that wait. The stop never waits on the game: the wait is already met, so the `schedule()` that every stopping caller runs next (`submit_job`, `drain`, `sync_watch`) resumes it straight away through `resume_queue`. That takes `memory_capture_now`, and `met_resumed_at` lets the wait through on the resumed run, so each wait stops at most once. A later wait in the same resumed run can only stop again if a new signal came after that capture. - **Capture accounting.** The count is bumped before the written pages are collected, and the signal records the count under `ev_cpu_mu`. So a capture that began before the signal is never treated as covering it, which errs on the safe side. Captures on the submitting thread are taken under `sync_order_mu` and their memory work is applied in submit order, so "begun after the signal" does mean the data is in the mirrors before the stopped work replays. - **The GPU-wait hold, what bounds it.** `GpuWaiter` counts a thread only in the `mine` branch, where none of the queue's work up to `f` is stopped. The Vulkan work that wait needs therefore cannot depend on the held queues, and `wait_rec_seq` flushes it first, so there is no cycle. Three things bound the hold: `RX_VK_HOLD_GPU_MS` (1000 ms, never under `hold_ms`) counted from when the queues stopped; presents and releases, whose drains are unchanged; and a wait that needs stopped work, which `wait()` still forces after `RX_VK_WAIT_HOLD_US`. A poll (timeout of 1 ms or less) is not counted. The remaining cost is the one Risks names: a thread that only polls for stopped work, while another thread waits on the GPU, now waits up to 1000 ms instead of 50. - **The render-thread path is unchanged.** `met_wait_capture_on()` is false when `async_on()`. `ev_cpu_captures` is written only in the submitting thread's branch of `event_signal_queued`. `GpuWaiter` sits inside the `!async_on()` block, and `sync_watch` only runs in sync mode. The `resume_capture` change is a straight move of the same static into `resume_capture_on()`. Setting `met_resumed_at` on the render thread is harmless. - **Frame-time cost on the path that now waits.** The stop adds 400 to 750 recording splits and captures per 140 s run, about 3 to 5 a second. Most of the bytes would otherwise have gone into the next submit's capture. Each met wait also does one `ev_cpu_mu` lookup. The measured fps (median 124.2 before, 130.0 after) and the frames over 50 ms show no cost. That is within noise, as the PR says. - **Switches.** Both fixes are on by default and the switches only turn them off for comparison; there are no player knobs. Fine. Two non-blocking nits are inline: the met-wait stop is still on with `RX_VK_CAPTURE=0`, and the "made to go at 1000 ms" count also includes holds that ended normally. **Rebase:** main has moved since `aa95d35f` (#526, #552, #489 and others touch `vk_memory.cpp`, `vk_replay.cpp` and `vk_internal.h`). The PR still rebases onto main `ced37d40` without conflicts and Forgejo reports it mergeable, so I didn't push anything. Note that https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/526 is now on main and your runs predate it. The PR already lists "this change together with #526" as not tested, so the runs below should be on a build of this PR on current main. **Before merge (a game run):** Castle Town's south gate with Threaded rendering Off (`RX_VK_ASYNC=0`), going in and out through the gate's loads for a few minutes, expecting no single dark frame and the log's `50 ms` count at 0. Needed on Linux (any GPU) and on AMD, plus one PC with 4 to 8 cores on Auto, as the PR's Testing section says. Please post a test request for these (`scripts/test_requests.py request --pr 769 ...`) or ask for the `testing wanted` label. With those runs in the comments, this can be approved as it is.
@ -1000,0 +1016,4 @@
static const bool v = !getenv("RX_VK_RESUME_CAPTURE") || atoi(getenv("RX_VK_RESUME_CAPTURE")) != 0;
return v;
}
bool met_wait_capture_on() {
Owner

Nit, not blocking: this is on with RX_VK_CAPTURE=0 too. Then capture_make returns before captures_begun moves, so cpu_signal_uncaptured stays true for every event the CPU has signalled. Every met wait for a CPU event stops, and resume_queue splits the recording (finish_recording) for a memory_capture_now that makes nothing. met_resumed_at stops it from looping, so it only costs time, and only under a debug switch. Fix: export capture_on() from vk_memory.cpp's anonymous namespace (say bool memory_capture_on(); in vk_internal.h) and add && memory_capture_on() here, as you already do for resume_capture_on().

Nit, not blocking: this is on with `RX_VK_CAPTURE=0` too. Then `capture_make` returns before `captures_begun` moves, so `cpu_signal_uncaptured` stays true for every event the CPU has signalled. Every met wait for a CPU event stops, and `resume_queue` splits the recording (`finish_recording`) for a `memory_capture_now` that makes nothing. `met_resumed_at` stops it from looping, so it only costs time, and only under a debug switch. Fix: export `capture_on()` from `vk_memory.cpp`'s anonymous namespace (say `bool memory_capture_on();` in `vk_internal.h`) and add `&& memory_capture_on()` here, as you already do for `resume_capture_on()`.
@ -2804,0 +2870,4 @@
}
break;
case hold_gpu::Choice::Force:
if (holding) st_hold_gpu_forced++;
Owner

Nit, not blocking: this counts every drain that ends a hold. That includes the normal end, where the GPU wait is over, hold_ms has passed after it, and no signal came, at well under the bound. The log line reports the count as "made to go at 1000 ms", which is only true when the bound was hit. The Risks section reads it that way ("none reached the bound"). For example, a 100 ms GPU wait with no signal in the 50 ms after it is counted here at about 150 ms. Fix: if (holding && ms_since(blocked_since) >= most_ms) st_hold_gpu_forced++;. Or keep two counts (ended after the wait, hit the bound) so the log tells them apart.

Nit, not blocking: this counts every drain that ends a hold. That includes the normal end, where the GPU wait is over, `hold_ms` has passed after it, and no signal came, at well under the bound. The log line reports the count as "made to go at 1000 ms", which is only true when the bound was hit. The Risks section reads it that way ("none reached the bound"). For example, a 100 ms GPU wait with no signal in the 50 ms after it is counted here at about 150 ms. Fix: `if (holding && ms_since(blocked_since) >= most_ms) st_hold_gpu_forced++;`. Or keep two counts (ended after the wait, hit the bound) so the log tells them apart.
HumanBean deleted branch fix/met-wait-capture-pr 2026-11-15 18:29:41 +00:00
Some checks are pending
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
CI / Conventional Commit title (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
Guide screenshots / Guide images and local UI source review (pull_request) Blocked by required conditions
Third-party parts / As their notes say (pull_request) Blocked by required conditions
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
Website / Website on Cloudflare (pull_request) Blocked by required conditions
PR description / Description follows the template (pull_request_target) Successful in 16s
Website / Website on Cloudflare (pull_request_target) Has been skipped
Some workflows are waiting to be reviewed.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
MagicBeanDev/Longshot!769
No description provided.