fix(vk): Keep memory-writing draws while pipelines compile #1573

Merged

klimovich008 merged 18 commits from codex/preserve-graphics-storage-writes into main 2026-11-11 16:34:19 +00:00

Conversation 11 Commits 18 Files changed 4 +191 -10

klimovich008 commented 2026-11-10 09:07:45 +00:00 (Migrated from github.com)

• Copy link
• Copy Markdown

What this fixes

A draw producing data for later draws can be skipped while its shader or pipeline compiles. An older framebuffer cannot preserve separate buffer or image outputs.

Why merge

• Read published metadata before checking writes from every stage; ready pipelines avoid the scan.
• Keep attachment-only pipeline skipping and RX_VK_PIPE_SKIP=0. Mode 2 restores the old untranslated-draw skip.
• Preserve the API, cache formats and shader instructions. Document the first-visit waiting cost.

Change

• gpu/vk/vk_draw.cpp: preserve memory-writing draws during compilation.
• tests/test_draw_side_effects.py: execute the production decision in four modes.
• docs/development.md: document modes 0, 1 and 2.
• docs/tested-configurations.md: record the three Hyrule checks.

Tested

• python -I -B tests/run_all.py --jobs 1 --strict draw_side_effects shader_stamp_cache run_all_declarations: 23 cases per mode, shader stamp and runner checks.
• Fails on the old code, passes now. Four broken variants rejected.
• Full suite: 340 passed, four failures reproduced on unchanged main, 30 skipped.
• RX 9070 XT, three 96-second Hyrule checks at 32:9: intact terrain, no fatal error. Candidate summed readiness waits: 3.12 s; repeated control: 0.92 s. No speedup claim. No game data is included.

Not covered

• Tested on AMD only with matched API13 binaries; current main's API14 runtime and Linux gameplay untested.
• No native writer replay; implicit driver/OS caches uncontrolled. First-visit pauses remain, and mode 2 can omit unknown writes.

## What this fixes A draw producing data for later draws can be skipped while its shader or pipeline compiles. An older framebuffer cannot preserve separate buffer or image outputs. ## Why merge - Read published metadata before checking writes from every stage; ready pipelines avoid the scan. - Keep attachment-only pipeline skipping and `RX_VK_PIPE_SKIP=0`. Mode `2` restores the old untranslated-draw skip. - Preserve the API, cache formats and shader instructions. Document the first-visit waiting cost. ## Change - `gpu/vk/vk_draw.cpp`: preserve memory-writing draws during compilation. - `tests/test_draw_side_effects.py`: execute the production decision in four modes. - `docs/development.md`: document modes `0`, `1` and `2`. - `docs/tested-configurations.md`: record the three Hyrule checks. ## Tested - `python -I -B tests/run_all.py --jobs 1 --strict draw_side_effects shader_stamp_cache run_all_declarations`: 23 cases per mode, shader stamp and runner checks. - Fails on the old code, passes now. Four broken variants rejected. - Full suite: 340 passed, four failures reproduced on unchanged main, 30 skipped. - RX 9070 XT, three 96-second Hyrule checks at 32:9: intact terrain, no fatal error. Candidate summed readiness waits: 3.12 s; repeated control: 0.92 s. No speedup claim. No game data is included. ## Not covered - Tested on AMD only with matched API13 binaries; current main's API14 runtime and Linux gameplay untested. - No native writer replay; implicit driver/OS caches uncontrolled. First-visit pauses remain, and mode `2` can omit unknown writes.

hckrman101 (Migrated from github.com) reviewed 2026-11-10 15:56:23 +00:00

hckrman101 (Migrated from github.com) left a comment

Thanks, the pipeline-pending part looks right to me. Once translation is published, stores_global_memory plus is_written on storage buffers, image buffers and images covers global stores, SSBO writes and image stores/atomics. Those are the same flags the binding code already uses for write-back (memory_note_gpu_write, texture_written). Ready pipelines never reach the scan, so the hot path is unchanged. Waiting there goes through the same jobs_wait the !may_skip path already used, so I don't see any new lock or deadlock risk. The test pulls the real block out of vk_draw.cpp, checks for early metadata reads, and is registered in run_all.py with {cxx}. CI is green.

The part I'd like settled before merge is the translation-pending change (inline). It's a bigger behaviour change than the title suggests. The old code skipped every not-yet-translated draw with persistent targets. The comment this PR deletes says that was hookshot's deliberate default, because waiting "froze Ocarina 0.2-3 s at each area's first new programs, the most reported stutter". After this PR every untranslated draw blocks the render thread, color-only ones too, not only the memory writers. You list this under "Not covered", but the routes you tested don't say whether the program cache was cold. Could you post st_waits / st_wait_ms (or just felt stutter) for a cold-cache first visit to a couple of areas, main vs. this branch? If it's noticeably worse, an opt-out like RX_VK_PIPE_SKIP=2 (old translation skip, new pipeline rule) would keep the fix while giving people a way back.

Nits:

• Please keep a line of the removed history in the new comment, i.e. that waiting on translation trades correctness against first-visit freezes. Otherwise the next person flips it back without the context.
• The branch is 28 commits behind main. It merges cleanly, so this is just a heads-up.

Thanks, the pipeline-pending part looks right to me. Once translation is published, `stores_global_memory` plus `is_written` on storage buffers, image buffers and images covers global stores, SSBO writes and image stores/atomics. Those are the same flags the binding code already uses for write-back (`memory_note_gpu_write`, `texture_written`). Ready pipelines never reach the scan, so the hot path is unchanged. Waiting there goes through the same `jobs_wait` the `!may_skip` path already used, so I don't see any new lock or deadlock risk. The test pulls the real block out of `vk_draw.cpp`, checks for early metadata reads, and is registered in `run_all.py` with `{cxx}`. CI is green. The part I'd like settled before merge is the translation-pending change (inline). It's a bigger behaviour change than the title suggests. The old code skipped *every* not-yet-translated draw with persistent targets. The comment this PR deletes says that was hookshot's deliberate default, because waiting "froze Ocarina 0.2-3 s at each area's first new programs, the most reported stutter". After this PR every untranslated draw blocks the render thread, color-only ones too, not only the memory writers. You list this under "Not covered", but the routes you tested don't say whether the program cache was cold. Could you post `st_waits` / `st_wait_ms` (or just felt stutter) for a cold-cache first visit to a couple of areas, main vs. this branch? If it's noticeably worse, an opt-out like `RX_VK_PIPE_SKIP=2` (old translation skip, new pipeline rule) would keep the fix while giving people a way back. Nits: - Please keep a line of the removed history in the new comment, i.e. that waiting on translation trades correctness against first-visit freezes. Otherwise the next person flips it back without the context. - The branch is 28 commits behind main. It merges cleanly, so this is just a heads-up.

gpu/vk/vk_draw.cpp

hckrman101 (Migrated from github.com) commented 2026-11-10 15:56:23 +00:00

This now waits for every program still being translated, including color-only draws that used to be skipped when their targets were persistent. That's the 0.2-3 s first-visit freeze the old comment described as the most reported stutter. Waiting here is the only way to know whether a stage writes memory, so I get why it's needed. But it would be good to see cold-cache wait numbers vs. main, and maybe an env opt-out for the old translation skip.

This now waits for every program still being translated, including color-only draws that used to be skipped when their targets were persistent. That's the 0.2-3 s first-visit freeze the old comment described as the most reported stutter. Waiting here is the only way to know whether a stage writes memory, so I get why it's needed. But it would be good to see cold-cache wait numbers vs. main, and maybe an env opt-out for the old translation skip.

hckrman101 commented 2026-11-10 17:49:41 +00:00 (Migrated from github.com)

• Copy link
• Copy Markdown

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!

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!

klimovich008 commented 2026-11-10 18:20:26 +00:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Addressed the source changes in the refreshed branch:

1. Merged current main and moved compiler arguments into the test's RUN_ALL_ARGS, matching the new runner.
2. Restored the comment explaining the first-visit pause tradeoff and added/documented RX_VK_PIPE_SKIP=2. It restores the old skip only for unknown translations with persistent targets; published memory-writing pipelines still wait. Defaults and =0 retain the correctness fix.
3. Expanded the actual-source fixture to 23 cases per mode (unset/0/1/2), including unknown translations with nonpersistent targets. Main and four broken variants fail, including a variant dropping the mandatory =0/nonpersistent pipeline waits. Three focused checks pass.

Your cache question is valid: the earlier routes tested first-use graphics pipelines with an already prepared program bank. They do not measure cold translation. The description now says that explicitly. I am keeping this draft until the cold-program main/candidate comparison is done; current main also needs its API13 host for the new DLLs, so the earlier API12 game runs are not being presented as a new-head hardware check.

Addressed the source changes in the refreshed branch: 1. Merged current main and moved compiler arguments into the test's `RUN_ALL_ARGS`, matching the new runner. 2. Restored the comment explaining the first-visit pause tradeoff and added/documented `RX_VK_PIPE_SKIP=2`. It restores the old skip only for unknown translations with persistent targets; published memory-writing pipelines still wait. Defaults and `=0` retain the correctness fix. 3. Expanded the actual-source fixture to 23 cases per mode (unset/0/1/2), including unknown translations with nonpersistent targets. Main and four broken variants fail, including a variant dropping the mandatory `=0`/nonpersistent pipeline waits. Three focused checks pass. Your cache question is valid: the earlier routes tested first-use graphics pipelines with an already prepared program bank. They do not measure cold translation. The description now says that explicitly. I am keeping this draft until the cold-program main/candidate comparison is done; current main also needs its API13 host for the new DLLs, so the earlier API12 game runs are not being presented as a new-head hardware check.

klimovich008 commented 2026-11-10 20:22:27 +00:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Cold-program tests are now run: one 96-second launch per arm in Hyrule and the Kokiri house, RX 9070 XT / Ryzen 7 5700X3D, 32:9, API13, serial replay. Each started from the same visually checked title/save-menu program bank; area programs were absent. Play-generation cache files were reset too. Validation was off; original saves were unchanged.

Scene	Main: waits / wait time	PR: waits / wait time	Longest complete native-present interval, main / PR
Hyrule	Incomplete: only 27.2 s of trace	~931 / 48.3 s	Not comparable / 37.1 s
Kokiri house	~316 / 9.2 s	~619 / 2.1 s	6.97 / 1.55 s

Wait totals are approximate sums of rounded 120-frame summaries, including compute-program waits. Intervals include menus/loading and screenshot costs; they are application-present times, not display times.

This does not establish a latency improvement: driver/OS caches were not cleared, worker compute compiles reached 18.7–22.0 seconds, and the first Hyrule control did not capture gameplay. Candidate shots show Hyrule; both Kokiri arms show intact house geometry. No unexpected translation errors appeared; the reported network omissions are the card policy.

I am keeping the PR draft while isolating those common compute costs. The documented RX_VK_PIPE_SKIP=2 fallback and first-visit history comment are present, and the refreshed Windows/Linux CI is green.

Cold-program tests are now run: one 96-second launch per arm in Hyrule and the Kokiri house, RX 9070 XT / Ryzen 7 5700X3D, 32:9, API13, serial replay. Each started from the same visually checked title/save-menu program bank; area programs were absent. Play-generation cache files were reset too. Validation was off; original saves were unchanged. | Scene | Main: waits / wait time | PR: waits / wait time | Longest complete native-present interval, main / PR | |---|---:|---:|---:| | Hyrule | Incomplete: only 27.2 s of trace | ~931 / 48.3 s | Not comparable / 37.1 s | | Kokiri house | ~316 / 9.2 s | ~619 / 2.1 s | 6.97 / 1.55 s | Wait totals are approximate sums of rounded 120-frame summaries, including compute-program waits. Intervals include menus/loading and screenshot costs; they are application-present times, not display times. This does **not** establish a latency improvement: driver/OS caches were not cleared, worker compute compiles reached 18.7–22.0 seconds, and the first Hyrule control did not capture gameplay. Candidate shots show Hyrule; both Kokiri arms show intact house geometry. No unexpected translation errors appeared; the reported network omissions are the card policy. I am keeping the PR draft while isolating those common compute costs. The documented `RX_VK_PIPE_SKIP=2` fallback and first-visit history comment are present, and the refreshed Windows/Linux CI is green.

klimovich008 (Migrated from github.com) reviewed 2026-11-11 00:06:08 +00:00

gpu/vk/vk_draw.cpp

klimovich008 (Migrated from github.com) commented 2026-11-11 00:06:08 +00:00

The cold-program Hyrule check now includes complete gameplay in both arms. Three 96-second launches used matched API13 binaries on RX 9070 XT / Ryzen 7 5700X3D, 32:9, serial replay. Each restored identical saved inputs: a menu-only program bank and warmed application pipeline caches, including play generations. Area CPU programs were absent. Validation and GPU profiling were off; sparse screenshots stayed enabled.

Run order	Readiness waits	Summed wait time	Longest native-present interval
Main control, first	557	30.725 s	32.568 s
PR	815	3.120 s	2.114 s
Main control, repeated	557	0.923 s	2.128 s

Repeating unchanged control removed its long freeze. This is not a PR speedup: implicit driver/OS caches and successful initial driver-cache admission were uncontrolled. Compared with the later control, the PR accumulated 2.20 seconds more readiness waiting. These totals include compute and graphics; they do not isolate translation CPU time. The first control's empty 40–60-second measurement window was rejected.

All three reached Hyrule Field without a fatal error; checked screenshots show intact terrain, grass and castle. Original saves were unchanged. This supplies first-visit cost evidence, not native memory-writer output proof.

The documented RX_VK_PIPE_SKIP=2 fallback and first-visit history comment remain. The refreshed readiness block is identical to the hardware-tested block. Three focused checks pass. Full suite: 321 passed, 23 skipped; all four failures reproduce on unchanged current main (symlink privilege, builder UI fixture, FPX DLL loading, MinGW NOMINMAX). Both independent source reviews found no defects.

The cold-program Hyrule check now includes complete gameplay in both arms. Three 96-second launches used matched API13 binaries on RX 9070 XT / Ryzen 7 5700X3D, 32:9, serial replay. Each restored identical saved inputs: a menu-only program bank and warmed application pipeline caches, including play generations. Area CPU programs were absent. Validation and GPU profiling were off; sparse screenshots stayed enabled. | Run order | Readiness waits | Summed wait time | Longest native-present interval | |---|---:|---:|---:| | Main control, first | 557 | 30.725 s | 32.568 s | | PR | 815 | 3.120 s | 2.114 s | | Main control, repeated | 557 | 0.923 s | 2.128 s | Repeating unchanged control removed its long freeze. This is **not** a PR speedup: implicit driver/OS caches and successful initial driver-cache admission were uncontrolled. Compared with the later control, the PR accumulated 2.20 seconds more readiness waiting. These totals include compute and graphics; they do not isolate translation CPU time. The first control's empty 40–60-second measurement window was rejected. All three reached Hyrule Field without a fatal error; checked screenshots show intact terrain, grass and castle. Original saves were unchanged. This supplies first-visit cost evidence, not native memory-writer output proof. The documented `RX_VK_PIPE_SKIP=2` fallback and first-visit history comment remain. The refreshed readiness block is identical to the hardware-tested block. Three focused checks pass. Full suite: 321 passed, 23 skipped; all four failures reproduce on unchanged current main (symlink privilege, builder UI fixture, FPX DLL loading, MinGW `NOMINMAX`). Both independent source reviews found no defects.

klimovich008 commented 2026-11-11 00:36:23 +00:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Linux source checks, Linux AppImage and Windows Builder pass on the refreshed head. Windows source checks finished with 325 passed, 1 failed and 22 skipped: only test_bake_skeleton timed out at 600 seconds. The draw-side-effects check passes on both platforms.

The same skeleton timeout occurs in the base main run, which also failed the log-batch timing assertion. The PR's log-batch check passes. No claim that the entire CI suite is green.

Linux source checks, Linux AppImage and Windows Builder pass on the refreshed head. Windows source checks finished with 325 passed, 1 failed and 22 skipped: only `test_bake_skeleton` timed out at 600 seconds. The draw-side-effects check passes on both platforms. The same skeleton timeout occurs in [the base main run](https://github.com/NotRexed/Hookshot/actions/runs/42924663471), which also failed the log-batch timing assertion. The PR's log-batch check passes. No claim that the entire CI suite is green.

hckrman101 (Migrated from github.com) approved these changes 2026-11-11 02:16:38 +00:00

hckrman101 (Migrated from github.com) left a comment

Thanks for the follow-up, this settles what I asked for.

• Translation-pending wait: RX_VK_PIPE_SKIP=2 brings back the old skip for untranslated draws with persistent targets. Since the pipeline check runs after translation, published memory-writers still wait in that mode, which is the right split. Defaults, 1 and 0 keep the fix.
• History comment: restored. The new comment says waiting on unknown translations can bring back the 0.2-3 s first-visit pauses and points at mode 2.
• Cold-cache numbers: thanks for being careful with these. The run order dominates, as you say (main went from 30.7 s to 0.9 s just by being repeated). Against the warm control the PR adds about 2.2 s of summed waiting over 96 s, and the longest present interval is about the same (2.11 vs 2.13 s). Kokiri house points the same way. That's a cost I'm fine with for not losing memory writes.

I re-read the new block. p.stages is only read after cp->ready was acquired (or after a wait on it). Mode 2 returns before it gets there, so there are no early reads. The fixture covers all four modes.

CI: the only red check is Windows source checks, where test_bake_skeleton timed out at 600 s. That's unrelated, and main's latest source-check runs fail too. No conflicts with main.

One nit inline (a mis-encoded dash in the docs). It's fine to merge as is.

Thanks for the follow-up, this settles what I asked for. - **Translation-pending wait:** `RX_VK_PIPE_SKIP=2` brings back the old skip for untranslated draws with persistent targets. Since the pipeline check runs after translation, published memory-writers still wait in that mode, which is the right split. Defaults, `1` and `0` keep the fix. - **History comment:** restored. The new comment says waiting on unknown translations can bring back the 0.2-3 s first-visit pauses and points at mode 2. - **Cold-cache numbers:** thanks for being careful with these. The run order dominates, as you say (main went from 30.7 s to 0.9 s just by being repeated). Against the warm control the PR adds about 2.2 s of summed waiting over 96 s, and the longest present interval is about the same (2.11 vs 2.13 s). Kokiri house points the same way. That's a cost I'm fine with for not losing memory writes. I re-read the new block. `p.stages` is only read after `cp->ready` was acquired (or after a wait on it). Mode 2 returns before it gets there, so there are no early reads. The fixture covers all four modes. CI: the only red check is Windows source checks, where `test_bake_skeleton` timed out at 600 s. That's unrelated, and main's latest source-check runs fail too. No conflicts with main. One nit inline (a mis-encoded dash in the docs). It's fine to merge as is.

docs/tested-configurations.md

hckrman101 (Migrated from github.com) commented 2026-11-11 02:16:38 +00:00

Nit: 40–60 is a double-encoded en dash (UTF-8 read as cp1252). A plain 40-60 would do.

Nit: `40–60` is a double-encoded en dash (UTF-8 read as cp1252). A plain `40-60` would do.

hckrman101 (Migrated from github.com) reviewed 2026-11-11 02:16:47 +00:00

gpu/vk/vk_draw.cpp

hckrman101 (Migrated from github.com) commented 2026-11-11 02:16:47 +00:00

Thanks, that answers it. The mode 2 opt-out and the restored comment cover the concern, and the waiting it adds against the warm control (~2.2 s over 96 s, about the same longest interval) is acceptable. Resolved from my side.

Thanks, that answers it. The mode 2 opt-out and the restored comment cover the concern, and the waiting it adds against the warm control (~2.2 s over 96 s, about the same longest interval) is acceptable. Resolved from my side.

klimovich008 (Migrated from github.com) reviewed 2026-11-11 04:06:46 +00:00

docs/tested-configurations.md

klimovich008 (Migrated from github.com) commented 2026-11-11 04:06:46 +00:00

Fixed the range text to 40-60 in b0c03b4. The refreshed table also keeps the AMD and NVIDIA “as above” groups separate (6041ea0).

Fixed the range text to `40-60` in b0c03b4. The refreshed table also keeps the AMD and NVIDIA “as above” groups separate (6041ea0).

mooki000 commented 2026-11-11 15:52:34 +00:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Linux run with NVIDIA's driver (the part you list as untested): no regression, and no visible gain on this route. RTX 4090, NVIDIA 615.78.08, i9-13900K, Arch Linux (kernel 7.2.15). Head 6041ea0 merged onto main 364d199 (10 Nov; it still merges cleanly onto today's main), against main 364d199 itself. Headless (offscreen 1920x1080, the game's 2048x1152), defaults.

To make pipelines build in play, both builds started with the Vulkan pipeline cache and the driver's shader cache deleted and RX_VK_PRECOMPILE=0; the translated programs were kept (the log says translation 0 ms in all on every one of its 32 to 37 such lines in each run, so your first hunk, the wait for translation, never ran). Then a save in Kokiri Forest and 45 s of camera turns by script. Runs in turn in one hold of the card, load average 10-37 (other jobs on the machine; no frame-rate claim).

run	load avg	longest gap in the save's load, s	draws left out, whole run	waits for shaders or pipelines, whole run	their time, s	frames over 50 ms in 44 s of play	FATAL
main, 1	36.7 -> 16.4	24.2	610200	210	23.4	5	0
main + #1573, 1	16.4 -> 12.8	18.9	423132	215	18.7	2	0
main, 2	12.8 -> 11.0	19.3	448776	209	19.0	1	0
main + #1573, 2	10.4 -> 10.4	19.0	415872	210	18.5	3	0

(The totals are the log's 120-present lines a frame: N draws left out (not ready), M waits for shaders or pipelines (T ms), each times 120 and summed.)

• No added pause. The waits are 209-210 on main and 210-215 with the PR, and their time (nearly all inside the save's load) is 18.5-18.7 s against main's 19.0-23.4 s. Main's first run had the higher load.
• Slightly fewer draws left out (416-423 thousand against 449-610 thousand), which is the direction the change should have; with main's two runs 36% apart I would not call it measured.
• The picture is the same. Both builds show a washed-out forest for the first seconds after this cold load (4-5 of the once-a-second shots have more than 5% white pixels, on main and with the PR, the last one 300-360 presents after the first), then a normal picture. So the wash on a cold pipeline cache is not from the draws this PR keeps, at least here. Pictures below.
• Both builds ran without a fault (no FATAL, no device lost, no driver line in the kernel log).

Not tested: a folder without the program cache, where draws wait for translation (your first hunk); RX_VK_PIPE_SKIP=2; AMD, Intel, Windows; a window.

**Linux run with NVIDIA's driver (the part you list as untested): no regression, and no visible gain on this route.** RTX 4090, NVIDIA 615.78.08, i9-13900K, Arch Linux (kernel 7.2.15). Head 6041ea0 merged onto main 364d199 (10 Nov; it still merges cleanly onto today's main), against main 364d199 itself. Headless (offscreen 1920x1080, the game's 2048x1152), defaults. To make pipelines build in play, both builds started with the Vulkan pipeline cache and the driver's shader cache deleted and `RX_VK_PRECOMPILE=0`; the translated programs were kept (the log says `translation 0 ms in all` on every one of its 32 to 37 such lines in each run, so your first hunk, the wait for translation, never ran). Then a save in Kokiri Forest and 45 s of camera turns by script. Runs in turn in one hold of the card, load average 10-37 (other jobs on the machine; no frame-rate claim). | run | load avg | longest gap in the save's load, s | draws left out, whole run | waits for shaders or pipelines, whole run | their time, s | frames over 50 ms in 44 s of play | FATAL | |---|---|---|---|---|---|---|---| | main, 1 | 36.7 -> 16.4 | 24.2 | 610200 | 210 | 23.4 | 5 | 0 | | main + #1573, 1 | 16.4 -> 12.8 | 18.9 | 423132 | 215 | 18.7 | 2 | 0 | | main, 2 | 12.8 -> 11.0 | 19.3 | 448776 | 209 | 19.0 | 1 | 0 | | main + #1573, 2 | 10.4 -> 10.4 | 19.0 | 415872 | 210 | 18.5 | 3 | 0 | (The totals are the log's 120-present lines `a frame: N draws left out (not ready), M waits for shaders or pipelines (T ms)`, each times 120 and summed.) - **No added pause.** The waits are 209-210 on main and 210-215 with the PR, and their time (nearly all inside the save's load) is 18.5-18.7 s against main's 19.0-23.4 s. Main's first run had the higher load. - **Slightly fewer draws left out** (416-423 thousand against 449-610 thousand), which is the direction the change should have; with main's two runs 36% apart I would not call it measured. - **The picture is the same.** Both builds show a washed-out forest for the first seconds after this cold load (4-5 of the once-a-second shots have more than 5% white pixels, on main and with the PR, the last one 300-360 presents after the first), then a normal picture. So the wash on a cold pipeline cache is not from the draws this PR keeps, at least here. Pictures below. - Both builds ran without a fault (no FATAL, no device lost, no driver line in the kernel log). Not tested: a folder without the program cache, where draws wait for translation (your first hunk); `RX_VK_PIPE_SKIP=2`; AMD, Intel, Windows; a window. 

hckrman101 (Migrated from github.com) approved these changes 2026-11-11 16:33:47 +00:00

hckrman101 (Migrated from github.com) left a comment

Re-checked at 6041ea0. Since my last review the branch took main (65 commits, no conflicts) and added two small commits of its own:

• The vk_draw.cpp change is the same as the one I approved. The fixture got a pipeline_topology stub for main's topology normalization (#2046), which is what that change needed.
• The 40–60 nit is fixed, and the cold-cache rows in tested-configurations.md now sit together.

@mooki000's Linux NVIDIA run (RTX 4090, cold pipeline and driver caches) covers part of what you listed as untested: wait counts and times about the same as main's, slightly fewer draws left out, no fault. The white wash on a cold load shows on main too, so it doesn't come from the draws this PR keeps. That run had translated programs ready, so the translation wait wasn't exercised there. Your AMD cold-program runs cover that part.

Overlap with #1946: it changes the same skip decision. It adds an "Asynchronous" mode that never waits, and makes targets_persistent false for targets cleared whole this frame. Whichever PR lands second has to merge the two rules. I'd expect the memory-writer check from this PR to still hold in #1946's default mode. Its author already said they'll rebase if this one goes first.

CI is all green on this head, Windows source checks included. Good to merge.

Re-checked at 6041ea0. Since my last review the branch took main (65 commits, no conflicts) and added two small commits of its own: - The `vk_draw.cpp` change is the same as the one I approved. The fixture got a `pipeline_topology` stub for main's topology normalization (#2046), which is what that change needed. - The `40–60` nit is fixed, and the cold-cache rows in `tested-configurations.md` now sit together. @mooki000's Linux NVIDIA run (RTX 4090, cold pipeline and driver caches) covers part of what you listed as untested: wait counts and times about the same as main's, slightly fewer draws left out, no fault. The white wash on a cold load shows on main too, so it doesn't come from the draws this PR keeps. That run had translated programs ready, so the translation wait wasn't exercised there. Your AMD cold-program runs cover that part. Overlap with #1946: it changes the same skip decision. It adds an "Asynchronous" mode that never waits, and makes `targets_persistent` false for targets cleared whole this frame. Whichever PR lands second has to merge the two rules. I'd expect the memory-writer check from this PR to still hold in #1946's default mode. Its author already said they'll rebase if this one goes first. CI is all green on this head, Windows source checks included. Good to merge.

github-sync referenced this pull request 2026-11-12 06:22:54 +00:00

chore(main): release 2.2.0-rc.1 #2071

github-sync referenced this pull request 2026-11-12 08:31:16 +00:00

fix(vk): bound the wait for shaders still translating, then leave the… #2586

github-sync referenced this pull request 2026-11-12 08:31:17 +00:00

fix(vk): bound the wait for shaders still translating, then leave the… #2586

github-sync referenced this pull request 2026-11-12 09:32:31 +00:00

fix(vk): bound the wait for shaders still translating, then leave the… #2586

github-sync referenced this pull request 2026-11-12 10:34:41 +00:00

fix(vk): bound the wait for shaders still translating, then leave the… #2586

github-sync referenced this pull request 2026-11-12 10:34:41 +00:00

fix(vk): bound the wait for shaders still translating, then leave the… #2586

github-sync referenced this pull request 2026-11-12 15:49:48 +00:00

perf(vk): a draw that has to wait for its pipeline gets one at once, not the full build #1946

github-sync referenced this pull request 2026-11-12 16:55:12 +00:00

fix(vk): bound the wait for shaders still translating, then leave the… #2586

github-sync referenced this pull request from a commit 2026-11-12 17:57:25 +00:00

fix(vk): bound the wait for shaders still translating, then leave the… (#2586)

github-sync referenced this pull request 2026-11-12 17:58:11 +00:00

fix(vk): bound the wait for shaders still translating, then leave the… #2586

Commenting is not possible because the repository is archived.

Reviewers

No reviewers

hckrman101

Labels

Clear labels   

accessibility

Barrier affecting people with disabilities

  

autorelease: pending

  

autorelease: tagged

  

bug

Something isn't working

  

dependencies

Pull requests that update a dependency file

  

documentation

Improvements or additions to documentation

  

duplicate

This issue or pull request already exists

  

enhancement

New feature or request

  

github_actions

Pull requests that update GitHub Actions code

  

good first issue

Good for newcomers

  

help wanted

Extra attention is needed

  

invalid

This doesn't seem right

  

question

Further information is requested

  

testing wanted

  

wontfix

This will not be worked on

No labels

accessibility

autorelease: pending

autorelease: tagged

bug

dependencies

documentation

duplicate

enhancement

github_actions

good first issue

help wanted

invalid

question

testing wanted

wontfix

Milestone

Clear milestone

No items

No milestone

Assignees

Clear assignees

No assignees

1 participant

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/Hookshot!1573

No description provided.