fix(vk): submit a 2x set's copies together so its presents keep their times #1720

github-sync · 2026-11-10T14:32:48Z

mayathaim-hue commented

2026-11-10 14:32:48 +00:00

(Migrated from github.com)

Summary

With Frame generation 2x and Frame pacing on, frames reached the screen alternately ~5.7 and ~11 ms apart instead of every 8.3 ms, so a VRR display's refresh changed on every frame (flicker on OLED). Addresses #1702.

The pacer's times were right. Each frame's copy into its swapchain image was submitted at its due time, and by the game frame's turn the next game frame's rendering (~11 ms here) was already on the same queue, so its copy and present waited for that, while the made frame's went out at once.

Present ahead: for a paced set of two, display_main now submits both copies together (with present_defer_into set, present_image returns after vkQueueSubmit2 with what present_commit needs), waits for the first copy's fence, and keeps only vkQueuePresentKHR on the pacer's times. RX_VK_PRESENT_AHEAD=0 turns it off.

It holds two swapchain images acquired at once, so it runs only where the swapchain has two to spare: dev.swap_spare, the image count less the surface's minImageCount, set when the swapchain is made. Past that an acquire has to time out (the Vulkan spec's limit), and on Linux the 100 ms acquire would take the window as not shown. The swapchain asks for 4 images, so with NVIDIA's minimum of 2 it runs. 3x and up, and unpaced output, are unchanged.
A swapchain made again between a copy and its present (a resize, HDR switched) is checked in present_commit, and that image is not presented.
The separate present queue stays opt-in and untouched. On the default single queue the presents themselves still queue behind the next frame, so the gain there is partial; with RX_VK_PRESENT_QUEUE=1 the uneven delivery is gone (below).

Validation

Intel PresentMon 2.3.1 (MsBetweenDisplayChange), 60 s of running and turning in Hyrule Field. RTX 5080, NVIDIA 617.42, Ryzen 7 7800X3D, Windows 11, 3840x2160 at 165 Hz with G-Sync, DLSS Quality, Frame generation 2x (the default, AMD FSR 3.1's made frames), the game at its 60. These runs used v2.0.12 with this change, the same code in these files as main; the swapchain guard and the test were added for this PR.

Run	Frames shifting > 4 ms from the one before	Average change frame to frame	1% low on screen	Worst
v2.0.12	82% (95% with warm pipelines)	4.7 ms (5.1)	67 fps (74)	24 ms
+ present ahead, default single queue	61%	4.1 ms	70 fps	31 ms
+ present ahead, `RX_VK_PRESENT_QUEUE=1` (4 runs)	0.1-0.2%	0.4-0.7 ms	96-98 fps	20 ms
same, `RX_VK_FSR_FG=0`, 60 s of normal play	0.0%	0.6 ms	103 fps	10 ms
`RX_VK_PRESENT_QUEUE=1` without present ahead (v2.0.10)	96%	5.2 ms	72 fps	38 ms

tests/test_present_ahead.py (new) compiles present_ahead_wanted into a fixture (on only for a paced 2x set with two spare images; off with the switch, unpaced, 1x, 3x, one or no spare image) and checks that present_image stops after the submit when deferred, that present_commit skips a swapchain made again, and that the spare count comes from minImageCount. It fails on main and on four mutations (one spare image allowed, 3x allowed, no swapchain check, no deferral).
17 related checks pass with LLVM-MinGW on Windows (framegen gate, auto and preset, frame wait bounds, device lost exit, capture, flash sampling, framelog integrity, Reflex swapchain lifetime, swapchain compat and colorspace, parallel); 5 are skipped here for missing Vulkan headers.
About 1.5 hours of play on v2.0.12 with present ahead and RX_VK_PRESENT_QUEUE=1 on this PC, with no device loss or black screen.

Limits: measured on one NVIDIA PC. The swapchain guard and the test are new since those runs and have not been run in game yet. Not tested on AMD, Intel or Linux, where the guard decides whether it runs. Keeping the presents fully on time on the single queue would need another way to keep them out of the next frame's queue order (present timing or metering, for example); not attempted here.

Found, patched and measured with the help of Claude (an AI assistant). No game files, generated code, binaries or saves are included.

## Summary With Frame generation 2x and Frame pacing on, frames reached the screen alternately ~5.7 and ~11 ms apart instead of every 8.3 ms, so a VRR display's refresh changed on every frame (flicker on OLED). Addresses #1702. The pacer's times were right. Each frame's copy into its swapchain image was submitted at its due time, and by the game frame's turn the next game frame's rendering (~11 ms here) was already on the same queue, so its copy and present waited for that, while the made frame's went out at once. Present ahead: for a paced set of two, `display_main` now submits both copies together (with `present_defer_into` set, `present_image` returns after `vkQueueSubmit2` with what `present_commit` needs), waits for the first copy's fence, and keeps only `vkQueuePresentKHR` on the pacer's times. `RX_VK_PRESENT_AHEAD=0` turns it off. - It holds two swapchain images acquired at once, so it runs only where the swapchain has two to spare: `dev.swap_spare`, the image count less the surface's `minImageCount`, set when the swapchain is made. Past that an acquire has to time out (the Vulkan spec's limit), and on Linux the 100 ms acquire would take the window as not shown. The swapchain asks for 4 images, so with NVIDIA's minimum of 2 it runs. 3x and up, and unpaced output, are unchanged. - A swapchain made again between a copy and its present (a resize, HDR switched) is checked in `present_commit`, and that image is not presented. - The separate present queue stays opt-in and untouched. On the default single queue the presents themselves still queue behind the next frame, so the gain there is partial; with `RX_VK_PRESENT_QUEUE=1` the uneven delivery is gone (below). ## Validation Intel PresentMon 2.3.1 (`MsBetweenDisplayChange`), 60 s of running and turning in Hyrule Field. RTX 5080, NVIDIA 617.42, Ryzen 7 7800X3D, Windows 11, 3840x2160 at 165 Hz with G-Sync, DLSS Quality, Frame generation 2x (the default, AMD FSR 3.1's made frames), the game at its 60. These runs used v2.0.12 with this change, the same code in these files as main; the swapchain guard and the test were added for this PR. | Run | Frames shifting > 4 ms from the one before | Average change frame to frame | 1% low on screen | Worst | |---|---|---|---|---| | v2.0.12 | 82% (95% with warm pipelines) | 4.7 ms (5.1) | 67 fps (74) | 24 ms | | + present ahead, default single queue | 61% | 4.1 ms | 70 fps | 31 ms | | + present ahead, `RX_VK_PRESENT_QUEUE=1` (4 runs) | 0.1-0.2% | 0.4-0.7 ms | 96-98 fps | 20 ms | | same, `RX_VK_FSR_FG=0`, 60 s of normal play | 0.0% | 0.6 ms | 103 fps | 10 ms | | `RX_VK_PRESENT_QUEUE=1` without present ahead (v2.0.10) | 96% | 5.2 ms | 72 fps | 38 ms | - `tests/test_present_ahead.py` (new) compiles `present_ahead_wanted` into a fixture (on only for a paced 2x set with two spare images; off with the switch, unpaced, 1x, 3x, one or no spare image) and checks that `present_image` stops after the submit when deferred, that `present_commit` skips a swapchain made again, and that the spare count comes from `minImageCount`. It fails on main and on four mutations (one spare image allowed, 3x allowed, no swapchain check, no deferral). - 17 related checks pass with LLVM-MinGW on Windows (framegen gate, auto and preset, frame wait bounds, device lost exit, capture, flash sampling, framelog integrity, Reflex swapchain lifetime, swapchain compat and colorspace, parallel); 5 are skipped here for missing Vulkan headers. - About 1.5 hours of play on v2.0.12 with present ahead and `RX_VK_PRESENT_QUEUE=1` on this PC, with no device loss or black screen. Limits: measured on one NVIDIA PC. The swapchain guard and the test are new since those runs and have not been run in game yet. Not tested on AMD, Intel or Linux, where the guard decides whether it runs. Keeping the presents fully on time on the single queue would need another way to keep them out of the next frame's queue order (present timing or metering, for example); not attempted here. Found, patched and measured with the help of Claude (an AI assistant). No game files, generated code, binaries or saves are included.

hckrman101 (Migrated from github.com) approved these changes

2026-11-10 16:41:23 +00:00

hckrman101 (Migrated from github.com) left a comment

Read through the diff and the surrounding present_image / display_main code. The approach holds up, and the numbers in the PR back it.

Things I checked:

The lock order in present_commit (present_image_mu, then dev.present_mu) matches present_image, and present_out keeps the qk.owns_lock() handling for the separate present queue.
The frame fences are fine. The 3-deep frames[] ring is used for two copies per set, and the next set's first present_image waits on a different slot's fence.
An early return in present_image (acquire timeout while the window is unseen, out-of-date, failed swapchain create) leaves pending[k].used false, so present_commit skips it. A swapchain made again between copy and present is caught by the swapchain handle compare.
present_defer_into is thread_local and only set on the display thread, so the game thread's direct present_image call is unaffected.
swap_spare is computed after n is final, and the gate (paced, shown == 2, spare >= 2) keeps 3x and up and unpaced output on the old path.
tests/test_present_ahead.py is picked up by run_all.py's test_*.py discovery, so no registration is needed.

Nits, none blocking:

test_present_ahead.py is mostly source-text ordering checks plus the small gate fixture. That is fine as a guard, but it won't catch a regression in how the pieces behave together.
As the PR says, the swapchain guard and the single-queue case are only measured on one NVIDIA box. The RX_VK_PRESENT_AHEAD=0 escape hatch covers AMD, Intel and Linux.
When a present is skipped in the commit loop, menu_note_frame and the shown-at stats still count it as shown. That only matters around a resize, so it's cosmetic.

CI hadn't reported on the head sha when I looked (status pending, no check runs).

Read through the diff and the surrounding present_image / display_main code. The approach holds up, and the numbers in the PR back it. Things I checked: - The lock order in `present_commit` (`present_image_mu`, then `dev.present_mu`) matches `present_image`, and `present_out` keeps the `qk.owns_lock()` handling for the separate present queue. - The frame fences are fine. The 3-deep `frames[]` ring is used for two copies per set, and the next set's first `present_image` waits on a different slot's fence. - An early return in `present_image` (acquire timeout while the window is unseen, out-of-date, failed swapchain create) leaves `pending[k].used` false, so `present_commit` skips it. A swapchain made again between copy and present is caught by the swapchain handle compare. - `present_defer_into` is thread_local and only set on the display thread, so the game thread's direct `present_image` call is unaffected. - `swap_spare` is computed after `n` is final, and the gate (paced, shown == 2, spare >= 2) keeps 3x and up and unpaced output on the old path. - `tests/test_present_ahead.py` is picked up by `run_all.py`'s `test_*.py` discovery, so no registration is needed. Nits, none blocking: - `test_present_ahead.py` is mostly source-text ordering checks plus the small gate fixture. That is fine as a guard, but it won't catch a regression in how the pieces behave together. - As the PR says, the swapchain guard and the single-queue case are only measured on one NVIDIA box. The `RX_VK_PRESENT_AHEAD=0` escape hatch covers AMD, Intel and Linux. - When a present is skipped in the commit loop, `menu_note_frame` and the shown-at stats still count it as shown. That only matters around a resize, so it's cosmetic. CI hadn't reported on the head sha when I looked (status pending, no check runs).

mayathaim-hue commented

2026-11-10 17:09:32 +00:00

(Migrated from github.com)

Thanks for the review. Two commits on top of the merge:

CI fix (tests/test_present_semaphores.py, the "Python source checks" failure on both OSes): the test read pi.pWaitSemaphores from present_image, and the present itself is now present_out, so it reads it from there. The test also compiles the swapchain's making, where dev.swap_spare is now set, so its stand-in Device has the field and recreate() a minImageCount of 2, with new checks: 4 images leave 2 spare, 2 leave 0, 5 leave 3, and 0 after a failed creation. swap_spare is now reset with the swapchain, so a failed re-creation leaves no stale count.
Review note 3: present_commit returns whether it presented. A skipped present keeps the pacer's line (next_show) and stays out of menu_note_frame and the shown/late counts.

Checked here:

test_present_semaphores.py and test_present_ahead.py pass, and fail on mutations (no reset, spare = image count, plus the earlier four).
Full tests/run_all.py on the branch head: 236 passed, 5 failed, 37 skipped. The same 5 fail on main 0d8e755 on this PC: four need tools it lacks (zstandard, a .NET SDK, C++ and Windows SDK headers for clang), and nx2/test_nup_stand_in.py fails with TypeError: <lambda>() takes 1 positional argument but 2 were given in make_game.stand_in_damaged_files, which looks like it came with #1764, not this PR.
In game: these renderer files built as hookshot_vk.dll (RX_VK_ONLY, on v2.0.13's tree) on the RTX 5080 / 617.42, DLSS Quality, 2x at 60 fps, RX_VK_PRESENT_QUEUE=1: the log has the present-ahead line, and 60 s of PresentMon gave 0.1% of frames shifting over 4 ms, a 1% low of 99 fps and a worst frame of 20 ms, the same as the first version.

Thanks for the review. Two commits on top of the merge: - **CI fix** (`tests/test_present_semaphores.py`, the "Python source checks" failure on both OSes): the test read `pi.pWaitSemaphores` from `present_image`, and the present itself is now `present_out`, so it reads it from there. The test also compiles the swapchain's making, where `dev.swap_spare` is now set, so its stand-in `Device` has the field and `recreate()` a `minImageCount` of 2, with new checks: 4 images leave 2 spare, 2 leave 0, 5 leave 3, and 0 after a failed creation. `swap_spare` is now reset with the swapchain, so a failed re-creation leaves no stale count. - **Review note 3**: `present_commit` returns whether it presented. A skipped present keeps the pacer's line (`next_show`) and stays out of `menu_note_frame` and the shown/late counts. Checked here: - `test_present_semaphores.py` and `test_present_ahead.py` pass, and fail on mutations (no reset, spare = image count, plus the earlier four). - Full `tests/run_all.py` on the branch head: 236 passed, 5 failed, 37 skipped. The same 5 fail on main 0d8e755 on this PC: four need tools it lacks (zstandard, a .NET SDK, C++ and Windows SDK headers for clang), and `nx2/test_nup_stand_in.py` fails with `TypeError: <lambda>() takes 1 positional argument but 2 were given` in `make_game.stand_in_damaged_files`, which looks like it came with #1764, not this PR. - In game: these renderer files built as `hookshot_vk.dll` (RX_VK_ONLY, on v2.0.13's tree) on the RTX 5080 / 617.42, DLSS Quality, 2x at 60 fps, `RX_VK_PRESENT_QUEUE=1`: the log has the present-ahead line, and 60 s of PresentMon gave 0.1% of frames shifting over 4 ms, a 1% low of 99 fps and a worst frame of 20 ms, the same as the first version.

hckrman101 commented

2026-11-10 17:12:16 +00:00

(Migrated from github.com)

Fix conflicts, but also check with #1730

Fix conflicts, but also check with https://git.magicbean.dev/MagicBeanDev/Hookshot/pulls/1730

hckrman101 commented

2026-11-10 17:50:25 +00:00

(Migrated from github.com)

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!

hckrman101 (Migrated from github.com) approved these changes

2026-11-10 18:13:10 +00:00

hckrman101 (Migrated from github.com) left a comment

Re-checked at 7006ea0. Both follow-ups look right:

present_commit now returns false for a skipped present, and the loop moves next_show on and continues past menu_note_frame and the counts. That closes my third nit.
swap_spare = 0 is now in the retire block of create_swapchain, so a failed re-create leaves no stale count. test_present_semaphores.py reads pWaitSemaphores from present_out now and checks the 2/0/3/0 spare counts. That fixes the earlier "Python source checks" failure.

One thing blocks the merge. Main moved again after your merge: #1515 (presentation device loss, merged 18:11) landed and the PR shows conflicts again. The conflict is in gpu/vk/vk_replay.cpp, where #1515 changed the tail of present_image (res = vkQueueSubmit2(...) + device_lost(), and if (res == VK_ERROR_DEVICE_LOST) device_lost(); after the two vkQueuePresentKHR calls). When you resolve it, keep the submit check in present_image and move the present check into present_out. tests/test_present_device_loss.py scans from present_image to the next namespace {, so it will pick up present_out as long as it stays in that range.

Nit: per #1822, tests/test_present_ahead.py should declare RUN_ALL_ARGS = ['--cxx', '{cxx}'] (see inline).

CI hasn't reported any checks on this head. Approving again, assuming a clean merge of main.

Re-checked at 7006ea0. Both follow-ups look right: - `present_commit` now returns false for a skipped present, and the loop moves `next_show` on and `continue`s past `menu_note_frame` and the counts. That closes my third nit. - `swap_spare = 0` is now in the retire block of `create_swapchain`, so a failed re-create leaves no stale count. `test_present_semaphores.py` reads `pWaitSemaphores` from `present_out` now and checks the 2/0/3/0 spare counts. That fixes the earlier "Python source checks" failure. One thing blocks the merge. Main moved again after your merge: #1515 (presentation device loss, merged 18:11) landed and the PR shows conflicts again. The conflict is in `gpu/vk/vk_replay.cpp`, where #1515 changed the tail of `present_image` (`res = vkQueueSubmit2(...)` + `device_lost()`, and `if (res == VK_ERROR_DEVICE_LOST) device_lost();` after the two `vkQueuePresentKHR` calls). When you resolve it, keep the submit check in `present_image` and move the present check into `present_out`. `tests/test_present_device_loss.py` scans from `present_image` to the next `namespace {`, so it will pick up `present_out` as long as it stays in that range. Nit: per #1822, `tests/test_present_ahead.py` should declare `RUN_ALL_ARGS = ['--cxx', '{cxx}']` (see inline). CI hasn't reported any checks on this head. Approving again, assuming a clean merge of main.

tests/test_present_ahead.py

					
				@ -0,0 +33,4 @@

				def main():

				    p = argparse.ArgumentParser(description=__doc__)

				    p.add_argument("--cxx", default="g++")

Nit: since #1822, run_all.py takes a test's arguments from its own RUN_ALL_ARGS. Without RUN_ALL_ARGS = ['--cxx', '{cxx}'] at module level, this runs with a hard-coded g++, not the compiler run_all found, and won't skip cleanly on a machine that has none (the same way test_present_device_loss.py declares it).

Nit: since #1822, run_all.py takes a test's arguments from its own `RUN_ALL_ARGS`. Without `RUN_ALL_ARGS = ['--cxx', '{cxx}']` at module level, this runs with a hard-coded `g++`, not the compiler run_all found, and won't skip cleanly on a machine that has none (the same way `test_present_device_loss.py` declares it).

mayathaim-hue commented

2026-11-10 18:18:44 +00:00

(Migrated from github.com)

Merged main into the branch (7006ea0). The one conflict was tests/test_present_semaphores.py: it now reads present_image and present_out each as a complete function. The related checks pass.

Checked with #1730 (2x now uses the renderer's own blend by default). Same spot as before (a dungeon entrance, 60 s of running and turning), RTX 5080 / 617.42, DLSS Quality, 2x with the game at 60, v2.0.13 with this PR's renderer files, the own blend, and the default single queue (RX_VK_PRESENT_QUEUE unset):

Run	Frames shifting > 4 ms from the one before	Average change frame to frame	1% low on screen	Worst
`RX_VK_PRESENT_AHEAD=0`	88%	5.2 ms	62 fps	247 ms (one hitch)
present ahead	6%	1.8 ms	75 fps	21 ms
present ahead, repeat	28%	2.7 ms	82 fps	18 ms
present ahead + `RX_VK_PRESENT_QUEUE=1`, for reference	0.1%	0.8 ms	99 fps	20 ms

So with the own blend, the default single queue gets most of the gain; the separate queue removes the rest. With FSR 3.1's interpolation (RX_VK_FSR_FG=1) on the single queue, present ahead helped little in three runs here (82-97% of frames shifting), so that combination still needs the present queue.

Merged main into the branch (7006ea0). The one conflict was `tests/test_present_semaphores.py`: it now reads `present_image` and `present_out` each as a complete function. The related checks pass. Checked with #1730 (2x now uses the renderer's own blend by default). Same spot as before (a dungeon entrance, 60 s of running and turning), RTX 5080 / 617.42, DLSS Quality, 2x with the game at 60, v2.0.13 with this PR's renderer files, the own blend, and the **default single queue** (`RX_VK_PRESENT_QUEUE` unset): | Run | Frames shifting > 4 ms from the one before | Average change frame to frame | 1% low on screen | Worst | |---|---|---|---|---| | `RX_VK_PRESENT_AHEAD=0` | 88% | 5.2 ms | 62 fps | 247 ms (one hitch) | | present ahead | 6% | 1.8 ms | 75 fps | 21 ms | | present ahead, repeat | 28% | 2.7 ms | 82 fps | 18 ms | | present ahead + `RX_VK_PRESENT_QUEUE=1`, for reference | 0.1% | 0.8 ms | 99 fps | 20 ms | So with the own blend, the default single queue gets most of the gain; the separate queue removes the rest. With FSR 3.1's interpolation (`RX_VK_FSR_FG=1`) on the single queue, present ahead helped little in three runs here (82-97% of frames shifting), so that combination still needs the present queue.

mayathaim-hue commented

2026-11-10 18:31:03 +00:00

(Migrated from github.com)

Heads-up: this was merged at a259978, a one-step commit where gpu/vk/vk_replay.cpp was briefly back at main's version while I worked around the conflict (the re-apply was the next commit). So main is missing that file's half: present_commit and present_defer_into have no definition (unresolved link), and test_present_ahead.py / test_present_semaphores.py fail. #1853 puts the same 24 lines back on top of main. Sorry about that.

Heads-up: this was merged at a259978, a one-step commit where `gpu/vk/vk_replay.cpp` was briefly back at main's version while I worked around the conflict (the re-apply was the next commit). So main is missing that file's half: `present_commit` and `present_defer_into` have no definition (unresolved link), and `test_present_ahead.py` / `test_present_semaphores.py` fail. #1853 puts the same 24 lines back on top of main. Sorry about that.

NotRexed commented

2026-11-10 18:44:08 +00:00

(Migrated from github.com)

@mayathaim-hue @hckrman101: heads-up. The last commit before this merge had reset gpu/vk/vk_replay.cpp to main's copy, and the re-apply never landed. So present_commit and present_defer_into are declared and called, but not defined on main, and the Vulkan back end does not link (Windows and the AppImage). #1858 puts the 24 missing lines back on current main, and will be merged once its CI is green. Until then, a red AppImage or Windows Vulkan check on a PR updated from main is this, not the PR. Thanks!

@mayathaim-hue @hckrman101: heads-up. The last commit before this merge had reset `gpu/vk/vk_replay.cpp` to main's copy, and the re-apply never landed. So `present_commit` and `present_defer_into` are declared and called, but not defined on `main`, and the Vulkan back end does not link (Windows and the AppImage). #1858 puts the 24 missing lines back on current `main`, and will be merged once its CI is green. Until then, a red AppImage or Windows Vulkan check on a PR updated from `main` is this, not the PR. Thanks!

github-sync referenced this pull request

2026-11-12 06:26:38 +00:00

chore(main): release 2.1.0-rc.1 #2026

Commenting is not possible because the repository is archived.

No reviewers

hckrman101