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

SadTransGirl commented

2026-11-12 00:23:26 +00:00

(Migrated from github.com)

On a cold or changed program bank, a draw whose shader is still translating waits it out (#1573), which brings back the 0.2-3 s pauses at an area's first new programs. This adds an opt-in bound on that wait, RX_VK_TRANSLATE_WAIT_MS, for people who see those first-visit freezes. Nothing changes by default.

Change (off unless set):

With RX_VK_TRANSLATE_WAIT_MS=<ms> (8 is the measured value), a draw whose shader is still translating and whose targets hold an earlier frame's picture waits for it at most that long a frame in all, on each recording thread, and is left out of the frame if it's still not done. The wait (jobs_wait_for) doesn't take the queued job onto the waiting thread: running it inline would block for its full 0.2-3 s and defeat the bound.
A target made this frame still waits it out, translated memory-writing draws still wait for their pipelines, and RX_VK_PIPE_SKIP=0 / 2 keep their meanings.
The value is parsed strictly (gpu/vk/translate_wait.h, strtod with an end check): anything but a number (8ms, off) is ignored with a warning instead of reading as 0, and the value in use is logged.
Once a frame's bound is spent, a draw is left out without counting a wait, so "waits for shaders or pipelines" stays comparable with #1573's. The bounded wait has PROF_EXACT(), as wait_for does.

Why it's opt-in. An untranslated program can't be classified, so a translation slower than the bound is left out whether or not it writes memory: the case #1573 was merged to fix. Nothing visible came of it in the runs below, but that's one route on one NVIDIA card, and a missing write usually shows later and somewhere else. Off by default, testers on other machines and routes can try it, and the default can change once there's more than one machine's data.

Measured. RTX 4070 SUPER 12 GB (617.42), Windows 11, Ryzen 9 9950X3D, DLAA 2560x1440, unattended, from the title into Castle Town and out to Hyrule Field, two runs each. The build was this change's first version on main c544828f (with #2585), where 8 ms was the default, against RX_VK_TRANSLATE_WAIT_MS=-1:

	Waited out (as main)	8 ms bound
Cold program bank (`RX_VK_NO_PROGRAM_CACHE=1`): fps	44.9, 45.3	50.0, 47.4
the same: frames over 100 ms / over 250 ms	39 / 11, 34 / 11	22 / 8, 23 / 6
the same: time waiting for shaders or pipelines, summed	4.94 s, 4.85 s	1.80 s, 1.70 s
the same: draws left out (not ready)	4,404, 4,368	20,592, 23,208
Warm program bank: fps; waits; draws left out	47.2; 0; 0	47.2; 0; 0

Most of the extra draws left out are in two bursts, the load into the field and the fade at its gate, where the screenshots are black either way. Screenshots every 60 frames showed nothing missing. At the first frame after the load, the run that waited out was missing the HUD (main's own skip of pipelines still building), and the bounded run had it. Those runs' wait counts (1,499-1,505 waited out against 7,999-9,409 bounded) still counted draws after the bound was spent as waits, which this version no longer does, so the summed time is the number to compare. Rows in docs/tested-configurations.md.

Tests. tests/test_draw_side_effects.py compiles the real draw decision and runs every RX_VK_PIPE_SKIP mode with the switch unset, 8, 0 and 8ms: waited out unless set, bounded and then left out when set, no wait counted with a bound of 0, a non-number ignored. It also checks the parser. The checks that read the changed files pass, except two that fail the same way on plain main on this PC.

Not changed. Compute dispatches still wait their translation out (the jobs_wait(cp->ready) on the dispatch path in vk_draw.cpp): a follow-up. It may be part of why frames over 250 ms only drop from 11 to 6-8.

Not tested. AMD, Intel, Linux; other routes; whether a left-out draw's memory write is ever missed.

Part of #995.

Assisted-by: Claude Opus 5.5, Claude Mythos 5.2

🤖 Generated with Claude Code

On a cold or changed program bank, a draw whose shader is still translating waits it out (#1573), which brings back the 0.2-3 s pauses at an area's first new programs. This adds an opt-in bound on that wait, `RX_VK_TRANSLATE_WAIT_MS`, for people who see those first-visit freezes. Nothing changes by default. **Change** (off unless set): - With `RX_VK_TRANSLATE_WAIT_MS=<ms>` (8 is the measured value), a draw whose shader is still translating and whose targets hold an earlier frame's picture waits for it at most that long a frame in all, on each recording thread, and is left out of the frame if it's still not done. The wait (`jobs_wait_for`) doesn't take the queued job onto the waiting thread: running it inline would block for its full 0.2-3 s and defeat the bound. - A target made this frame still waits it out, translated memory-writing draws still wait for their pipelines, and `RX_VK_PIPE_SKIP=0` / `2` keep their meanings. - The value is parsed strictly (`gpu/vk/translate_wait.h`, `strtod` with an end check): anything but a number (`8ms`, `off`) is ignored with a warning instead of reading as 0, and the value in use is logged. - Once a frame's bound is spent, a draw is left out without counting a wait, so "waits for shaders or pipelines" stays comparable with #1573's. The bounded wait has `PROF_EXACT()`, as `wait_for` does. **Why it's opt-in.** An untranslated program can't be classified, so a translation slower than the bound is left out whether or not it writes memory: the case #1573 was merged to fix. Nothing visible came of it in the runs below, but that's one route on one NVIDIA card, and a missing write usually shows later and somewhere else. Off by default, testers on other machines and routes can try it, and the default can change once there's more than one machine's data. **Measured.** RTX 4070 SUPER 12 GB (617.42), Windows 11, Ryzen 9 9950X3D, DLAA 2560x1440, unattended, from the title into Castle Town and out to Hyrule Field, two runs each. The build was this change's first version on main c544828f (with #2585), where 8 ms was the default, against `RX_VK_TRANSLATE_WAIT_MS=-1`: | | Waited out (as main) | 8 ms bound | |---|---|---| | Cold program bank (`RX_VK_NO_PROGRAM_CACHE=1`): fps | 44.9, 45.3 | 50.0, 47.4 | | the same: frames over 100 ms / over 250 ms | 39 / 11, 34 / 11 | 22 / 8, 23 / 6 | | the same: time waiting for shaders or pipelines, summed | 4.94 s, 4.85 s | 1.80 s, 1.70 s | | the same: draws left out (not ready) | 4,404, 4,368 | 20,592, 23,208 | | Warm program bank: fps; waits; draws left out | 47.2; 0; 0 | 47.2; 0; 0 | Most of the extra draws left out are in two bursts, the load into the field and the fade at its gate, where the screenshots are black either way. Screenshots every 60 frames showed nothing missing. At the first frame after the load, the run that waited out was missing the HUD (main's own skip of pipelines still building), and the bounded run had it. Those runs' wait counts (1,499-1,505 waited out against 7,999-9,409 bounded) still counted draws after the bound was spent as waits, which this version no longer does, so the summed time is the number to compare. Rows in `docs/tested-configurations.md`. **Tests.** `tests/test_draw_side_effects.py` compiles the real draw decision and runs every `RX_VK_PIPE_SKIP` mode with the switch unset, `8`, `0` and `8ms`: waited out unless set, bounded and then left out when set, no wait counted with a bound of 0, a non-number ignored. It also checks the parser. The checks that read the changed files pass, except two that fail the same way on plain main on this PC. **Not changed.** Compute dispatches still wait their translation out (the `jobs_wait(cp->ready)` on the dispatch path in `vk_draw.cpp`): a follow-up. It may be part of why frames over 250 ms only drop from 11 to 6-8. **Not tested.** AMD, Intel, Linux; other routes; whether a left-out draw's memory write is ever missed. Part of #995. Assisted-by: Claude Opus 5.5, Claude Mythos 5.2 🤖 Generated with [Claude Code](https://claude.com/claude-code)

github-sync changed title from ~~WIP: fix(vk): bound the wait for shaders still translating, then leave the…~~ to fix(vk): bound the wait for shaders still translating, then leave the…

2026-11-12 08:31:16 +00:00

github-sync commented

2026-11-12 08:31:17 +00:00

Owner

@hckrman101 commented on GitHub · 2026-11-12 08:27 UTC · original

Thanks for picking this up: the first-visit pauses since #1573 are real, and a bound is a reasonable direction. I read the diff against main and I don't think it should go in as it is. The points, most important first:

The lost writes come back for any translation slower than the bound. An untranslated program can't be classified, so when the 8 ms runs out the draw is left out whether or not it writes memory. That is the case #1573 fixed. The bound makes it rarer than RX_VK_PIPE_SKIP=2, but a translation that takes 0.2-3 s (the pauses this targets) is always over it.
Once the frame's budget is spent, every later untranslated draw is left out at once. jobs_wait_for returns false for ms <= 0, so a new area with many new programs behaves like RX_VK_PIPE_SKIP=2 after the first slow one.
The budget is renewed every frame. A translation that takes 500 ms costs up to 8 ms on each recording thread in every frame until it is done, with the draw missing the whole time. That is a slowdown of several frames in place of one pause, and it should be measured rather than assumed better.
The waiting thread no longer helps. jobs_wait runs the queued job on the waiting thread when it is still in the queue (the job-steal path, added because a new area queues a hundred jobs). jobs_wait_for only waits, so with busy workers the translation itself finishes later than it does today.
Dispatches are untouched. The compute path has its own unbounded jobs_wait(cp->ready) in vk_draw.cpp, so the same pause remains there.
Not run in the game. The fixture checks the decision only. Small: RX_VK_TRANSLATE_WAIT_MS goes through atof, so a typo reads as 0 and skips at once.

One data point from my side: on a prepared install (NVIDIA, Linux) the "waits for shaders or pipelines" line reads 0.00 in 120 of 124 intervals of a long play session, so this only matters for a cold or changed program bank. That is also the test I'd want to see before this changes the default: one run with a cold program bank each way, comparing the summed waits, the draws left out, the frame times, and whether anything flashes or goes missing.

Ideas that would keep #1573's guarantee:

Keep helping: take the queued job onto the waiting thread first (as jobs_wait does), and bound only the wait for a job another worker already runs.
Leave a draw out only when its targets are persistent and the previous translation of the same stages (or the file's own reflection) says it writes no memory; wait it out otherwise.
Start with the bound off by default (-1) so people with the freezes can try it and report.

> [@hckrman101](https://github.com/hckrman101) commented on GitHub · 2026-11-12 08:27 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2586#issuecomment-6818645282) Thanks for picking this up: the first-visit pauses since #1573 are real, and a bound is a reasonable direction. I read the diff against main and I don't think it should go in as it is. The points, most important first: 1. **The lost writes come back for any translation slower than the bound.** An untranslated program can't be classified, so when the 8 ms runs out the draw is left out whether or not it writes memory. That is the case #1573 fixed. The bound makes it rarer than `RX_VK_PIPE_SKIP=2`, but a translation that takes 0.2-3 s (the pauses this targets) is always over it. 2. **Once the frame's budget is spent, every later untranslated draw is left out at once.** `jobs_wait_for` returns false for `ms <= 0`, so a new area with many new programs behaves like `RX_VK_PIPE_SKIP=2` after the first slow one. 3. **The budget is renewed every frame.** A translation that takes 500 ms costs up to 8 ms on each recording thread in every frame until it is done, with the draw missing the whole time. That is a slowdown of several frames in place of one pause, and it should be measured rather than assumed better. 4. **The waiting thread no longer helps.** `jobs_wait` runs the queued job on the waiting thread when it is still in the queue (the job-steal path, added because a new area queues a hundred jobs). `jobs_wait_for` only waits, so with busy workers the translation itself finishes later than it does today. 5. **Dispatches are untouched.** The compute path has its own unbounded `jobs_wait(cp->ready)` in `vk_draw.cpp`, so the same pause remains there. 6. **Not run in the game.** The fixture checks the decision only. Small: `RX_VK_TRANSLATE_WAIT_MS` goes through `atof`, so a typo reads as 0 and skips at once. One data point from my side: on a prepared install (NVIDIA, Linux) the "waits for shaders or pipelines" line reads 0.00 in 120 of 124 intervals of a long play session, so this only matters for a cold or changed program bank. That is also the test I'd want to see before this changes the default: one run with a cold program bank each way, comparing the summed waits, the draws left out, the frame times, and whether anything flashes or goes missing. Ideas that would keep #1573's guarantee: - Keep helping: take the queued job onto the waiting thread first (as `jobs_wait` does), and bound only the wait for a job another worker already runs. - Leave a draw out only when its targets are persistent and the previous translation of the same stages (or the file's own reflection) says it writes no memory; wait it out otherwise. - Start with the bound off by default (`-1`) so people with the freezes can try it and report.

github-sync commented

2026-11-12 09:32:31 +00:00

Owner

@SadTransGirl commented on GitHub · 2026-11-12 09:10 UTC · original

Tested #2586 on the same PC: main c544828f + #2586, Castle Town to Hyrule Field, two runs each, RX_VK_TRANSLATE_WAIT_MS=-1 (as #1573) against the default 8 ms:

•	Cold cache (RX_VK_NO_PROGRAM_CACHE=1): 50.0 / 47.4 fps against 44.9 / 45.3; frames over 100 ms 22 / 23 against 39 / 34; over 250 ms 8 / 6 against 11 / 11.
•	Draws left out: 20.6k / 23.2k in 110 / 112 frames, against 4.4k / 4.4k in 102 / 100 frames (main already leaves some out for pipelines still building). Most of the extra is in two bursts: the load into the field and the fade at its gate, where the screenshots are black either way.
•	Screenshots every 60 frames: no missing geometry seen. At the first frame after the load, main's run was missing the HUD (its own pending-pipeline skip) and #2586's run had it.
•	Warm cache: no difference (47.2 / 47.2 fps, no draws left out either way).
•	Not covered: a draw whose shader writes memory can be left out too, since that isn't known before translation; nothing visible came of it here, but this is one route on NVIDIA only.

> [@SadTransGirl](https://github.com/SadTransGirl) commented on GitHub · 2026-11-12 09:10 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2586#issuecomment-6819513523) Tested #2586 on the same PC: main c544828f + #2586, Castle Town to Hyrule Field, two runs each, RX_VK_TRANSLATE_WAIT_MS=-1 (as #1573) against the default 8 ms: • Cold cache (RX_VK_NO_PROGRAM_CACHE=1): 50.0 / 47.4 fps against 44.9 / 45.3; frames over 100 ms 22 / 23 against 39 / 34; over 250 ms 8 / 6 against 11 / 11. • Draws left out: 20.6k / 23.2k in 110 / 112 frames, against 4.4k / 4.4k in 102 / 100 frames (main already leaves some out for pipelines still building). Most of the extra is in two bursts: the load into the field and the fade at its gate, where the screenshots are black either way. • Screenshots every 60 frames: no missing geometry seen. At the first frame after the load, main's run was missing the HUD (its own pending-pipeline skip) and #2586's run had it. • Warm cache: no difference (47.2 / 47.2 fps, no draws left out either way). • Not covered: a draw whose shader writes memory can be left out too, since that isn't known before translation; nothing visible came of it here, but this is one route on NVIDIA only.

github-sync commented

2026-11-12 10:34:41 +00:00

Owner

@hckrman101 commented on gpu/vk/vk_draw.cpp line 6519 on GitHub · 2026-11-12 09:37 UTC · original

atof reads a typo (8ms, off) as 0, which means the bound is spent immediately and every untranslated draw on persistent targets gets left out, i.e. RX_VK_PIPE_SKIP=2 without telling anyone. Parse it with strtod and an end-pointer check, fall back to the default with a warning, and log the value in use once, like RX_VK_FALLBACK_PHI_LIMIT does.

> [@hckrman101](https://github.com/hckrman101) commented on `gpu/vk/vk_draw.cpp` line 6519 on GitHub · 2026-11-12 09:37 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2586#discussion_r4548681781) `atof` reads a typo (`8ms`, `off`) as 0, which means the bound is spent immediately and every untranslated draw on persistent targets gets left out, i.e. `RX_VK_PIPE_SKIP=2` without telling anyone. Parse it with `strtod` and an end-pointer check, fall back to the default with a warning, and log the value in use once, like `RX_VK_FALLBACK_PHI_LIMIT` does.

github-sync commented

2026-11-12 10:34:41 +00:00

Owner

@hckrman101 commented on gpu/vk/vk_draw.cpp line 6532 on GitHub · 2026-11-12 09:37 UTC · original

Nit: once the budget is spent, jobs_wait_for returns at once and the draw is counted as a wait (and as a skip). That inflates the "waits for shaders or pipelines" count we compare against #1573. Counting it only when time was actually spent (budget_left > 0 before the call) would keep the stat honest. PROF_EXACT() is also missing on this path, unlike wait_for.

> [@hckrman101](https://github.com/hckrman101) commented on `gpu/vk/vk_draw.cpp` line 6532 on GitHub · 2026-11-12 09:37 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2586#discussion_r4548681794) Nit: once the budget is spent, `jobs_wait_for` returns at once and the draw is counted as a wait (and as a skip). That inflates the "waits for shaders or pipelines" count we compare against #1573. Counting it only when time was actually spent (`budget_left > 0` before the call) would keep the stat honest. `PROF_EXACT()` is also missing on this path, unlike `wait_for`.

github-sync commented

2026-11-12 10:34:41 +00:00

Owner

@hckrman101 reviewed on GitHub · 2026-11-12 09:37 UTC · original

Thanks for the runs, that's the test I asked for and the numbers are encouraging: on a cold bank the bound gives about 8% more fps and about 40% fewer frames over 100 ms on that route, with nothing visibly missing (and the HUD at the first frame after the load is a nice catch). The code is unchanged since my comment, so here's where my points stand:

Lost writes (1): still open, and you name it yourself under "Not covered". 20.6k/23.2k draws left out against 4.4k is about five times as many, and none of them can be checked for memory writes before translation. One route on one NVIDIA card showing nothing wrong doesn't clear the case #1573 was merged to fix, since a missing write usually shows up later and somewhere else, not as missing geometry in that frame.
Spent budget / per-frame renewal (2, 3): answered by measurement. The extra skips go along with better frame times, so on this run the trade pays.
Not helping on the waiting thread (4): I take this one back. Running the queued translation inline would block for its full 0.2-3 s and defeat the bound, so not stealing is the right call here.
Dispatches (5): still unbounded in vk_draw.cpp. That's fine to leave for a follow-up, but please say so in the description. It may be part of why frames over 250 ms only drop from 11 to 6-8.
atof (6): still open, see inline.

What I'd like before this goes in:

Ship it with the bound off by default (RX_VK_TRANSLATE_WAIT_MS defaulting to -1) and keep 8 as the documented setting for people with first-visit freezes. Then testers on AMD/Intel and on other routes can report, and we change the default once there's more than one machine's data. The other route would be the narrower skip from my comment: leave the draw out only when the previous translation of the same stages says it writes no memory, and keep 8 ms as the default.
Put the measurements in the PR description in place of "Not run in the game", with the GPU, driver and OS of that PC. Please add the summed waits per run too (the "waits for shaders or pipelines" line), since that's the number #1573 was judged on.
Merge main. The red test_env_on checks on both platforms are about HOS_FPS_LOG in host/oot_high_fps.c, which this PR doesn't touch, and main's runs are green now. They aren't this PR's fault.

Unrelated to #2585 (the VRAM squeeze in vk_texture.cpp): no shared code, and either can land without the other.

> [@hckrman101](https://github.com/hckrman101) reviewed on GitHub · 2026-11-12 09:37 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2586#pullrequestreview-5872182715) Thanks for the runs, that's the test I asked for and the numbers are encouraging: on a cold bank the bound gives about 8% more fps and about 40% fewer frames over 100 ms on that route, with nothing visibly missing (and the HUD at the first frame after the load is a nice catch). The code is unchanged since my comment, so here's where my points stand: - **Lost writes (1)**: still open, and you name it yourself under "Not covered". 20.6k/23.2k draws left out against 4.4k is about five times as many, and none of them can be checked for memory writes before translation. One route on one NVIDIA card showing nothing wrong doesn't clear the case #1573 was merged to fix, since a missing write usually shows up later and somewhere else, not as missing geometry in that frame. - **Spent budget / per-frame renewal (2, 3)**: answered by measurement. The extra skips go along with better frame times, so on this run the trade pays. - **Not helping on the waiting thread (4)**: I take this one back. Running the queued translation inline would block for its full 0.2-3 s and defeat the bound, so not stealing is the right call here. - **Dispatches (5)**: still unbounded in `vk_draw.cpp`. That's fine to leave for a follow-up, but please say so in the description. It may be part of why frames over 250 ms only drop from 11 to 6-8. - **`atof` (6)**: still open, see inline. What I'd like before this goes in: 1. Ship it with the bound **off by default** (`RX_VK_TRANSLATE_WAIT_MS` defaulting to -1) and keep 8 as the documented setting for people with first-visit freezes. Then testers on AMD/Intel and on other routes can report, and we change the default once there's more than one machine's data. The other route would be the narrower skip from my comment: leave the draw out only when the previous translation of the same stages says it writes no memory, and keep 8 ms as the default. 2. Put the measurements in the PR description in place of "Not run in the game", with the GPU, driver and OS of that PC. Please add the summed waits per run too (the "waits for shaders or pipelines" line), since that's the number #1573 was judged on. 3. Merge main. The red `test_env_on` checks on both platforms are about `HOS_FPS_LOG` in `host/oot_high_fps.c`, which this PR doesn't touch, and main's runs are green now. They aren't this PR's fault. Unrelated to #2585 (the VRAM squeeze in `vk_texture.cpp`): no shared code, and either can land without the other.

github-sync commented

2026-11-12 16:55:12 +00:00

Owner

@SadTransGirl commented on GitHub · 2026-11-12 16:49 UTC · original

Thanks, all done in 80e43f44 (with main merged first):

Off by default. RX_VK_TRANSLATE_WAIT_MS unset waits each translation out, as #1573; 8 is the documented value for people with first-visit freezes. The lost-write risk is in the description as the reason it's opt-in.
Measurements are in the description in place of "Not run in the game", with the GPU, driver and OS and the summed waits per run: on a cold bank 4.94 / 4.85 s waiting out against 1.80 / 1.70 s with 8 ms. Those runs' wait counts still had the inflation from your nit, so the summed time is the number to compare.
atof is replaced by translate_wait::parse (strtod with an end check, gpu/vk/translate_wait.h): a non-number is ignored with a warning, and the value in use is logged once.
The wait count: once the frame's bound is spent, a draw is left out without counting a wait, and the bounded wait has PROF_EXACT(). The test now checks the count, with a bound of 0 among the cases.
Dispatches are named in the description as a follow-up.
Main is merged, so the HOS_FPS_LOG checks should be green now.

The narrower skip from your first comment (leave a draw out only when the previous translation of the same stages says it writes no memory) would be a good default later. I've kept this PR to the opt-in bound.

> [@SadTransGirl](https://github.com/SadTransGirl) commented on GitHub · 2026-11-12 16:49 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2586#issuecomment-6829975206) Thanks, all done in 80e43f44 (with main merged first): 1. **Off by default.** `RX_VK_TRANSLATE_WAIT_MS` unset waits each translation out, as #1573; 8 is the documented value for people with first-visit freezes. The lost-write risk is in the description as the reason it's opt-in. 2. **Measurements** are in the description in place of "Not run in the game", with the GPU, driver and OS and the summed waits per run: on a cold bank 4.94 / 4.85 s waiting out against 1.80 / 1.70 s with 8 ms. Those runs' wait *counts* still had the inflation from your nit, so the summed time is the number to compare. 3. **`atof`** is replaced by `translate_wait::parse` (`strtod` with an end check, `gpu/vk/translate_wait.h`): a non-number is ignored with a warning, and the value in use is logged once. 4. **The wait count:** once the frame's bound is spent, a draw is left out without counting a wait, and the bounded wait has `PROF_EXACT()`. The test now checks the count, with a bound of 0 among the cases. 5. **Dispatches** are named in the description as a follow-up. 6. **Main is merged**, so the `HOS_FPS_LOG` checks should be green now. The narrower skip from your first comment (leave a draw out only when the previous translation of the same stages says it writes no memory) would be a good default later. I've kept this PR to the opt-in bound.

github-sync referenced this pull request from a commit

2026-11-12 17:57:24 +00:00

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

github-sync closed this pull request

2026-11-12 17:58:09 +00:00

github-sync commented

2026-11-12 17:58:11 +00:00

Owner

@Joey-Magnus approved on GitHub · 2026-11-12 17:02 UTC · original

I merged 26ae0e7e into current main (clean) and ran tests/test_draw_side_effects.py, test_env_on, test_jobs_wait_runs_queued, test_jobs_failure, test_gpl_cold_library, test_ub_threads, test_fallback_depth_hold, test_shadow_mask_guard and test_reactive_mask. All pass.

hckrman101's points are handled in 80e43f44:

The bound is off unless RX_VK_TRANSLATE_WAIT_MS is set. With it unset, the code takes the same wait_for(cp->ready) path as main, so nobody's frames change by default.
translate_wait::parse rejects "8ms", "off", nan and inf instead of reading them as 0, and the value in use is logged once.
A wait is counted only when there was budget left to wait, and the bounded wait has PROF_EXACT(), so the waits line stays comparable with #1573's.
The cold-bank runs, with the summed waits per run, are in the description. Compute dispatches are named as a follow-up.

jobs_wait_for waits on the same done_cv that worker_loop notifies after each job, so a translation that finishes inside the bound wakes the draw at once. Leaving the job queued instead of running it inline is right here, as hckrman101 agreed.

One thing worth knowing, already in the docs row: RX_VK_TRANSLATE_WAIT_MS=0 behaves like RX_VK_PIPE_SKIP=2 for persistent targets, because the budget is spent before it starts. The lost-write risk stays the reason it's opt-in, and that is stated plainly. Approving as the opt-in it now is.

> [@Joey-Magnus](https://github.com/Joey-Magnus) approved on GitHub · 2026-11-12 17:02 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2586#pullrequestreview-5878651689) I merged 26ae0e7e into current main (clean) and ran tests/test_draw_side_effects.py, test_env_on, test_jobs_wait_runs_queued, test_jobs_failure, test_gpl_cold_library, test_ub_threads, test_fallback_depth_hold, test_shadow_mask_guard and test_reactive_mask. All pass. hckrman101's points are handled in 80e43f44: - The bound is off unless RX_VK_TRANSLATE_WAIT_MS is set. With it unset, the code takes the same wait_for(cp->ready) path as main, so nobody's frames change by default. - translate_wait::parse rejects "8ms", "off", nan and inf instead of reading them as 0, and the value in use is logged once. - A wait is counted only when there was budget left to wait, and the bounded wait has PROF_EXACT(), so the waits line stays comparable with #1573's. - The cold-bank runs, with the summed waits per run, are in the description. Compute dispatches are named as a follow-up. jobs_wait_for waits on the same done_cv that worker_loop notifies after each job, so a translation that finishes inside the bound wakes the draw at once. Leaving the job queued instead of running it inline is right here, as hckrman101 agreed. One thing worth knowing, already in the docs row: RX_VK_TRANSLATE_WAIT_MS=0 behaves like RX_VK_PIPE_SKIP=2 for persistent targets, because the budget is spent before it starts. The lost-write risk stays the reason it's opt-in, and that is stated plainly. Approving as the opt-in it now is.

Rows
Columns

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