fix(vk): mark a clear's targets in the parallel replay's dry pass (the one-frame black scenes) #1639

Merged

SadTransGirl merged 2 commits from fix/parallel-replay-clears into main

2026-11-10 16:05:21 +00:00

SadTransGirl commented

2026-11-10 11:49:26 +00:00

(Migrated from github.com)

With RX_VK_PARALLEL (the multi-threaded replay, off by default) the whole 3D scene goes black for one frame now and then, with the HUD still drawn over it. This fixes the cause I found; the mode stays off by default.

What goes wrong

RX_VK_PARALLEL records a command buffer's segments on several threads, out of order, after a dry pass on the replay thread. A worker that samples a texture nobody has marked as the GPU's uploads what its memory holds, and that upload runs after the earlier segments' work.

The dry pass marks the targets of draws and dispatches for that reason (dry_mark_written), but it left clears out, and a clear opens a pass on the worker just as a draw does. So a target the game clears in one segment and samples in a later one was overwritten with stale memory whenever the later segment was recorded first. The result is the whole 3D scene black for one frame, with the HUD over it.

The existing warning names it every time: #... 853x480 NVN 0x29 written in segment N, uploaded by the later segment N+2, a target made that frame.

Changes

The dry pass marks the targets of NOP_CLEAR_COLOR and NOP_CLEAR_DS (RX_VK_NO_DRY_CLEARS=1: as before).
A worker counts a uniform update only when the dry pass appended it (the same size test). One of no bytes, or of more than the stream holds, moved every later draw of the segment one update ahead.
dry_mark_written skips "the same targets as the draw before" within one dry pass only (dry_marks_begin). Its first is the pass's first draw or dispatch, so a command buffer that began with a dispatch compared its first draw with the last draw of the dry pass before. This one is by reading; I could not make it happen on demand (RX_VK_DRY_LAST_KEPT=1: as before).
The order warning now says what wrote the texture (written by: a pass's colour target).

Numbers

Unattended runs (HOS_HEADLESS, scripted input), Ryzen 9 9950X3D, RTX 4070 SUPER, built from 4a7ae1b. Castle Town's market with the camera turning, about 2,450 draws.

8 threads and segments of 32, to make the fault frequent; 5,520 presents a run:

	order warnings
before (three runs)	7, 5, 4
with this (three runs)	0, 0, 0

The same with RX_VK_FLASH (a shot every present), 4,200 presents a run:

	order warnings	whole-scene black frames
before (two runs)	12, 3	14, 3
with this (two runs)	0, 0	0, 0

On 3 threads, eight other save points (Kokiri Forest, the Deku Tree, Dodongo's Cavern, Jabu-Jabu, Hyrule Castle, Hyrule Field, the Temple of Time, and one made after meeting Zelda and Impa), about 3,800 presents each with RX_VK_FLASH: no flash, no warning.

The replay's wall time a frame in the market:

threads	1	2	3	4	6	8
ms	10.90	8.64	7.75	8.02	8.27	8.33

Not done

The Kakariko walk of the comment at par_threads() was not run again (I have no save there), so the mode stays off by default.
Copy and CLEAR_TEX destinations are still not marked in the dry pass. None showed up in these runs' warnings.
#1473 refuses RX_VK_PARALLEL>=2; this does not touch that.

Test

python -I -B tests/test_parallel_dry_marks.py (new, source checks; fails on main). run_all texture_handle, texture_init, mem_trim pass.

🤖 Generated with Claude Code

With `RX_VK_PARALLEL` (the multi-threaded replay, off by default) the whole 3D scene goes black for one frame now and then, with the HUD still drawn over it. This fixes the cause I found; the mode stays off by default. ### What goes wrong `RX_VK_PARALLEL` records a command buffer's segments on several threads, out of order, after a dry pass on the replay thread. A worker that samples a texture nobody has marked as the GPU's uploads what its memory holds, and that upload runs after the earlier segments' work. The dry pass marks the targets of draws and dispatches for that reason (`dry_mark_written`), but it left clears out, and a clear opens a pass on the worker just as a draw does. So a target the game clears in one segment and samples in a later one was overwritten with stale memory whenever the later segment was recorded first. The result is the whole 3D scene black for one frame, with the HUD over it. The existing warning names it every time: `#... 853x480 NVN 0x29 written in segment N, uploaded by the later segment N+2`, a target made that frame. ### Changes - The dry pass marks the targets of `NOP_CLEAR_COLOR` and `NOP_CLEAR_DS` (`RX_VK_NO_DRY_CLEARS=1`: as before). - A worker counts a uniform update only when the dry pass appended it (the same size test). One of no bytes, or of more than the stream holds, moved every later draw of the segment one update ahead. - `dry_mark_written` skips "the same targets as the draw before" within one dry pass only (`dry_marks_begin`). Its `first` is the pass's first draw or dispatch, so a command buffer that began with a dispatch compared its first draw with the last draw of the dry pass before. This one is by reading; I could not make it happen on demand (`RX_VK_DRY_LAST_KEPT=1`: as before). - The order warning now says what wrote the texture (`written by: a pass's colour target`). ### Numbers Unattended runs (`HOS_HEADLESS`, scripted input), Ryzen 9 9950X3D, RTX 4070 SUPER, built from 4a7ae1b. Castle Town's market with the camera turning, about 2,450 draws. 8 threads and segments of 32, to make the fault frequent; 5,520 presents a run: | | order warnings | |---|---| | before (three runs) | 7, 5, 4 | | with this (three runs) | 0, 0, 0 | The same with `RX_VK_FLASH` (a shot every present), 4,200 presents a run: | | order warnings | whole-scene black frames | |---|---|---| | before (two runs) | 12, 3 | 14, 3 | | with this (two runs) | 0, 0 | 0, 0 | On 3 threads, eight other save points (Kokiri Forest, the Deku Tree, Dodongo's Cavern, Jabu-Jabu, Hyrule Castle, Hyrule Field, the Temple of Time, and one made after meeting Zelda and Impa), about 3,800 presents each with `RX_VK_FLASH`: no flash, no warning. The replay's wall time a frame in the market: | threads | 1 | 2 | 3 | 4 | 6 | 8 | |---|---|---|---|---|---|---| | ms | 10.90 | 8.64 | 7.75 | 8.02 | 8.27 | 8.33 | ### Not done - The Kakariko walk of the comment at `par_threads()` was not run again (I have no save there), so the mode stays off by default. - Copy and `CLEAR_TEX` destinations are still not marked in the dry pass. None showed up in these runs' warnings. - #1473 refuses `RX_VK_PARALLEL>=2`; this does not touch that. ### Test `python -I -B tests/test_parallel_dry_marks.py` (new, source checks; fails on main). `run_all` texture_handle, texture_init, mem_trim pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

hckrman101 (Migrated from github.com) approved these changes

2026-11-10 16:02:09 +00:00

hckrman101 (Migrated from github.com) left a comment

Looks right to me. A clear on the worker goes through begin_pass(cmd, targets(s)), which marks every colour target plus depth, so marking targets(s) in the dry pass is exactly what the worker will do later, just earlier and in stream order. Partial, scissored and layered clears mark the whole texture there, the same as the serial path does through begin_pass, so nothing changes in behaviour beyond the ordering. Passing first = true and setting dry_last_valid means a draw on the same targets right after the clear is still skipped correctly.

The NOP_UPDATE_UB fix is correct too: the worker now counts an update under the same u.size && u.size <= avail - sizeof u test the dry pass appends by, so ub_limit no longer drifts ahead of ub_updates. dry_marks_begin() is thread_local and runs on the same thread as the dry pass, so it's fine.

The serial path only gains a dry && test per clear, and the dry pass gains one targets() lookup (cached by rt_serial) per clear, so the cost is negligible. The why string is only formatted when the warning fires.

Nits, none blocking:

Two inline notes about over-marking when the worker's clear returns early (below).
The test only checks the source text. It gets picked up by run_all's tests/test_*.py glob, so it is registered, but it will break on any rewording and doesn't exercise the marking itself. That's acceptable for this path, since there's no GPU in CI.
With dry_marks_begin() in place, the draw's first argument (dry_work == 0) is now redundant for draws. You could leave it or drop it.
As you say, COPY_B2T/COPY_T2T/CLEAR_TEX destinations are still unmarked in the dry pass, so the same kind of order warning could still come from a copy target. A follow-up could mark those too.

CI is green. No overlap with #1473 or the other open vk_replay PRs on these lines.

Looks right to me. A clear on the worker goes through `begin_pass(cmd, targets(s))`, which marks every colour target plus depth, so marking `targets(s)` in the dry pass is exactly what the worker will do later, just earlier and in stream order. Partial, scissored and layered clears mark the whole texture there, the same as the serial path does through `begin_pass`, so nothing changes in behaviour beyond the ordering. Passing `first = true` and setting `dry_last_valid` means a draw on the same targets right after the clear is still skipped correctly. The `NOP_UPDATE_UB` fix is correct too: the worker now counts an update under the same `u.size && u.size <= avail - sizeof u` test the dry pass appends by, so `ub_limit` no longer drifts ahead of `ub_updates`. `dry_marks_begin()` is thread_local and runs on the same thread as the dry pass, so it's fine. The serial path only gains a `dry &&` test per clear, and the dry pass gains one `targets()` lookup (cached by `rt_serial`) per clear, so the cost is negligible. The `why` string is only formatted when the warning fires. Nits, none blocking: - Two inline notes about over-marking when the worker's clear returns early (below). - The test only checks the source text. It gets picked up by run_all's `tests/test_*.py` glob, so it is registered, but it will break on any rewording and doesn't exercise the marking itself. That's acceptable for this path, since there's no GPU in CI. - With `dry_marks_begin()` in place, the draw's `first` argument (`dry_work == 0`) is now redundant for draws. You could leave it or drop it. - As you say, `COPY_B2T`/`COPY_T2T`/`CLEAR_TEX` destinations are still unmarked in the dry pass, so the same kind of order warning could still come from a copy target. A follow-up could mark those too. CI is green. No overlap with #1473 or the other open vk_replay PRs on these lines.

gpu/vk/vk_replay.cpp

Nit: clear_color_target returns before begin_pass when mask & 0xF is 0 or index has no view, so in that case the worker marks nothing, but the dry pass still marks every target here. Over-marking sets gpu_written, and that can stop a later segment from uploading a CPU-written target that the serial replay would have uploaded. It's rare, but gating on the payload's mask (e.g. payload<nvn_p_clear_color>(a, avail).mask & 0xF) would match the worker exactly.

Nit: `clear_color_target` returns before `begin_pass` when `mask & 0xF` is 0 or `index` has no view, so in that case the worker marks nothing, but the dry pass still marks every target here. Over-marking sets `gpu_written`, and that can stop a later segment from uploading a CPU-written target that the serial replay would have uploaded. It's rare, but gating on the payload's mask (e.g. `payload<nvn_p_clear_color>(a, avail).mask & 0xF`) would match the worker exactly.

gpu/vk/vk_replay.cpp

Same here: clear_depth_stencil returns early when no depth is bound, or when neither aspect is masked in. A depth-only clear with no depth target then marks the colour targets in the dry pass, although the worker never opens a pass for them. Checking s.rt.depth (and the masks) first would keep the two in step.

Same here: `clear_depth_stencil` returns early when no depth is bound, or when neither aspect is masked in. A depth-only clear with no depth target then marks the colour targets in the dry pass, although the worker never opens a pass for them. Checking `s.rt.depth` (and the masks) first would keep the two in step.