fix(vk): occlusion counts stay visible unless known: one query numbering across queues, no count from a draw that didn't run #260

Merged
HumanBean merged 1 commit from HumanBean/Longshot:fix/occlusion-visible-unless-known into main 2026-11-13 18:15:26 +00:00
Owner

Why

#197 gives Ocarina of Time's samples-passed reports the GPU's real occlusion counts, and the game leaves out the objects whose count is 0. Its rule is that a count not known for sure stays "visible" (4096): a wrong 0 makes an object the player can see vanish. A review of #197 found several ways a wrong or unknown count could still reach the game, and one that breaks a Vulkan rule on every frame (the validation layer reported it on the RX 580 and on the RTX 3090). This closes them, keeps the culling, and tests the real code paths.

What changed

  • replay, query numbering (gpu/vk/vk_replay.cpp: occ_begin_buffer, occ_end_buffer, occ_state_copied, occ_room, occ_open_oldest): queries are numbered from one counter for all the game's queues (occ_next_all). It was in NvnState, and each queue has its own state (RX_VK_QUEUE_STATE, on by default), so two queues could hand out the same ring entries: a query begun again without a reset, one queue's 0 in another's report, host resets of queries still in flight, a stale ring tail. Each command buffer's replay copies the counter into its state before the dry pass and the workers copy that state, and writes it back however replay_handle returns. A run left open while another queue took queries is split: what it took is freed and its report stays visible. A queue's state made as a copy of another's drops the other's open run. The ring's room counts every queue's open run and every waiting report, in any order.
  • draw (gpu/vk/vk_draw.cpp: OccGuard, draw()): a query only counts a draw that drew. It is begun just before the draw command (occ.issue()), not when the pass opens. A draw that returns before its command (a missing sampler, a uint8 index buffer without the feature, the quad paths, a negative indirect count or stride, a buffer that could not be bound), or whose vertex input could not all be bound (zero_buffer in its place), never begins its query: it has no result, and the report stays visible. Before, those ended an empty query, which reads an available 0.
  • draw, mesh fallback: a task program split into batches drops its query instead of beginning it once per batch (devices without mesh shaders, e.g. the RX 580).
  • draw: the guard is made after StatsQuery (RX_VK_DRAW_STATS), so it ends first, inside the pass.
  • replay, report value: the reset and report cases record rec_cur_value, the submission the queries go out in, instead of a global that every begin_recording overwrote (a present's and frame generation's included).
  • replay, resets: queries read are reset from the host only once the GPU has finished the submission they went out in (its timeline value), in order (occ_freeing). The counts are still written as soon as the queries are available. Resetting on the availability bit alone gave VUID-vkResetQueryPool-firstQuery-02741, then VUID-vkCmdBeginQuery-None-00807 at the query's next use, every frame (validation: ~25k and 2.6M on the RX 580 lab run of #197; capped thousands of each on the RTX 3090 here).
  • replay, slot sequence (occ_slots, occ_slot_next): each report slot has a report sequence, and a count is written only over the report it belongs to. The timestamp word alone let an older 0 replace a newer "visible" when two reports fell within one tick (~0.5 us at RX_VK_TS_SCALE 0.1). It is a direct-mapped table: a slot that evicts another drops the evicted one's count (the safe side; ~0.2-1% of reports here).
  • replay, held-back work (occ_visible_ahead, occ_slot_visible; run_submit's stop, submit_job's queued submits): the samples-passed reports of work held at an event wait read 4096 at once, so the game never reads an older count in a slot before the replay gets to its report. Each held submission is walked once. With RX_VK_ASYNC=1 (a replay running behind the game) the queries are off.
  • switches: RX_VK_OCC_FIXED now wins over RX_VK_OCC_QUERIES=1, as documented. RX_VK_OCC_LOG also says why reports stayed visible.
  • tests: tests/test_vk_occlusion_queries.py compiles the real ring helpers, OccGuard, the task-batch block and occ_queries_on() against fakes. It covers several queue states, an early return after the pass began, missing vertex input, task batches, the report's value, same-tick slot reuse, slot-table eviction, the visible-ahead write, and the reset rule (the fake vkResetQueryPool fails if any query it resets went out in a submission that is not done). It also checks the wiring: every draw command after the guard preceded by occ.issue() with no way out between, and the guard after StatsQuery. tests/test_draw_side_effects.py and tests/test_fallback_pipeline_state.py cut code next to the change, so their fixtures follow it.
  • docs: docs/development.md, the occlusion paragraph.

Flags and settings

Flag What it does Default now Recommendation Helps on Doesn't help on Untested on
RX_VK_OCC_QUERIES Unchanged; off with RX_VK_ASYNC=1 now on on as #197 - AMD, Intel, the Steam Deck, Windows (this change)
RX_VK_OCC_FIXED=<n> Every report reads n; now also when RX_VK_OCC_QUERIES=1 is set unset unchanged - - -
RX_VK_OCC_LOG Also: queries not begun (draws that returned, vertex input, task batches), runs split by another queue, slots made visible ahead of a stopped queue and those walks' cost off off (a developer switch) - - -

Measurements

main: fdc2840b (2026-11-13), which has #197
this PR: fix/occlusion-visible-unless-known @ cd873617 (measured; the branch is now 794214d4, the same change rebased onto main fbd42024, see below)
Hardware: RTX 3090 24 GB (NVIDIA 620.70.06), Ryzen 9 7900X, 30 GB, Linux (CachyOS, kernel 7.3.3)
Scene: the pristine Kokiri save after the opening event: Kokiri Forest standing 40 s, then the camera turning at full speed 30 s; then Castle Town Market (debug teleport, 30 s to settle), standing 40 s and turning 30 s. 2048x1152, default options, shaders prepared, cap 240, RX_VK_OCC_LOG=1 on both sides.
Method: one game per run in a private headless sway with scripted input, under the PC's timing lock (own CPU cores). Both sides use the same game build (main's longshot), with each side's renderer built the same way (RX_VK_ONLY). Two interleaved sets with the final code, P M M P and M P M P (4 runs each side), and 2 runs of this PR with RX_VK_OCC_QUERIES=0 for the no-culling reference. Draws and GPU time: the medians of the renderer's 120-present lines in each phase; frame times from RX_VK_FRAME_LOG. Each cell: the median of the runs' values. The PC swapped in every run (100k-1.7M pages in a phase).
How to reproduce: as in #197: stand 40 s and turn 30 s in Kokiri and the Market; compare against main, and RX_VK_OCC_QUERIES=0 for no culling.

main @ fdc2840b this PR @ cd873617 this PR @ cd873617, RX_VK_OCC_QUERIES=0 (no culling, reference)
Kokiri standing: draws a frame 2,600 2,614 2,864
Kokiri standing: GPU time a frame 9.05 ms 9.00 ms 8.98 ms
Kokiri standing: frame time median / p99 / max 15.47 / 21.30 / 27.9 ms 15.20 / 20.05 / 25.5 ms 15.32 / 19.95 / 43.2 ms
Kokiri turning: draws 1,753 1,760 1,709
Kokiri turning: frame time median / p99 13.68 / 19.15 ms 13.71 / 20.30 ms 13.37 / 18.55 ms
Market standing: draws 2,433 2,440 2,928
Market standing: GPU time 7.53 ms 7.40 ms 7.95 ms
Market standing: frame time median / p99 / max 15.67 / 25.25 / 47.6 ms 16.12 / 28.95 / 56.4 ms 17.34 / 28.20 / 59.2 ms
Market turning: draws (varies with where the turn ends) 1,144 1,048 990
Market turning: frame time median / p99 13.36 / 28.35 ms 13.60 / 28.90 ms 13.05 / 27.40 ms

Market standing, frame median per run (ms), in run order: P M M P: 16.25 15.37 15.35 16.27; then M P M P: 15.97 16.00 16.16 15.85. In the second set main was as slow as this PR. An earlier set with a build that differed only in the slot table and the visible-ahead writes (M P P M M P): main 15.34 14.53 15.37, this PR 15.41 16.04 15.84. Over all of them main's median is 15.37 ms (7 runs), this PR's 16.00 (9). So this PR may cost ~0.5 ms in the Market standing, or it may be the PC: Kokiri is level, the GPU time is level, the visible-ahead writes are ruled out (two runs with them off: 16.17 and 15.70 ms), and a sampled profile of each side (HOS_PROFILE=5 HOS_SAMPLE_ALL + HOS_CHAIN, scripts/chain.py; 16.53 vs 16.73 ms a frame on the path) shows no single function behind it. The culling itself holds: 2,440 draws against 2,928 with no culling, and the frame median 16.12 against 17.34 ms.

main, Market standing:      vk: 16200 presents, 64.8 fps; a frame: 2427 draws (0 skipped) ...
this PR, RX_VK_OCC_LOG=1:   vk: occlusion queries: ... reports written (... 56.7%), 0 left visible (a draw did not draw), 10 reported again first; written 1.3 recordings after the report; 80 waiting; 84 reads (66 wrote), 6.2 us each; ...; queries not begun: 0 draws returned before drawing, 0 with vertex input not all bound, 0 task batches; 0 runs split by another queue's queries; 2550 slots made visible ahead of a stopped queue (28 walks, 22.0 us each)
main, RX_VK_VALIDATION=1:   VUID-vkResetQueryPool-firstQuery-02741 (2,000, the duplicate cap), VUID-vkCmdBeginQuery-None-00807 (2,000, the cap)
this PR, RX_VK_VALIDATION=1: no VUID

The timed A/B ran on main fdc2840b with this change at cd873617. Rebasing onto fbd42024 changed only one conflicting hunk: #198 renamed pass().draws to ps.draws in lines this change removes. Nothing else this change touches moved, so the numbers should still hold. The rebased build was rechecked in the game under validation (see Testing) rather than timed again.

Testing

Tested:

  • python -I -B tests/run_all.py on Linux at 794214d4 (on main fbd42024): 508 passed, 1 failed, 20 skipped. The failure is test_bake_portability: this PC has no zstandard module. (Before the rebase, at cd873617 on fdc2840b: 501 passed and the same 3 failures as main fdc2840b, two of them fixed on main since.)
  • The new test fails on main: the code it cuts out isn't there. Each fix was also put back to the old behaviour one at a time, and the test fails each time. The bugs put back: per-queue numbering, no split, a room check that ignores other queues, a copied state keeping the other's run, the query begun when the guard is made, the vertex-input drop ignored, task batches keeping the query, a global value, the timestamp-only check, the guard before StatsQuery, no visible-ahead, RX_VK_OCC_FIXED losing to =1, an issue() placed before a way out, reset on availability.
  • Validation layer (Khronos 1.4.363, RX_VK_VALIDATION=1), one short session each on the same route, RTX 3090: main had VUID-vkResetQueryPool-firstQuery-02741 and VUID-vkCmdBeginQuery-None-00807 up to the duplicate cap (2,000 each); this PR had no VUID at all (shader-module and SPIR-V float-mode messages filtered on both sides, as on the lab).
  • After the rebase onto main fbd42024 (794214d4; a full game build, shaders prepared): one short session over the same route with the validation layer and RX_VK_OCC_LOG=1. No VUID and no validation error (only the layer's usual WARNING-vkGetDeviceProcAddr-device). Culling held: Market standing 2,448 draws, ~50% of reports 0. 0 queries left unbegun, 0 runs split. Frame times from this session mean nothing (the layer). The A/B below the Measurements is from before the rebase; #198, the conflicting change, only takes the pass and descriptor writer once per draw, and the conflict was one line (probe_draw now reads ps.draws).
  • The flash detector (RX_VK_FLASH=1, every present compared with its neighbours), one session each over the whole route: 0 flashes on main in 11,733 presents, 0 with this PR in 11,639.
  • RX_VK_OCC_LOG in play (this PR): 0 queries left unbegun by a draw that returned, 0 by vertex input, 0 by task batches, 0 runs split by another queue on this route; 10-57 reports per 5,000 left visible by the slot table; ~2,100-2,550 slots per 5,000 reports made visible ahead of a stopped queue, in 24-28 walks of ~22 us.
  • Mesh fallback forced (RX_VK_FORCE_MESH_FALLBACK=1) on the RTX 3090: one session over the route: culling works on the compute path (54% of reports 0, 0 left visible), and no query ever reached the task-batch path (0 batches): the bounding-box runs are not task-program draws there. The batch rule is covered by the test only; a GPU without mesh shaders runs that path natively (Needs other testers).

Not tested: AMD, Intel, the Steam Deck and Windows with this change (the lab RX 580 run was not repeated with it; the Windows build was not run: the change is portable C++ and Vulkan only); dungeons and interiors; long play.

Needs other testers:

  • RX 580 or another GPU without mesh shaders, under RX_VK_VALIDATION=1: Kokiri and the Market, expecting no vkResetQueryPool-firstQuery-02741 / vkCmdBeginQuery-None-00807, and RX_VK_OCC_LOG showing the task batches left visible (T-VDEY)

Screenshots

Nothing on screen: the same objects are culled as with #197 (draw counts below); no flash in either session.

Risk and rollback

A few more objects may stay drawn where a count is now unknown (a slot-table eviction, a stopped queue's report, a draw that did not draw): culling lost, never an object. RX_VK_OCC_QUERIES=0 turns the queries off; reverting the commit restores #197.

Reviewer notes

Start with occ_begin_buffer / occ_end_buffer in replay_handle, then OccGuard and its issue() calls in draw(), then the deferred resets in occ_collect. On this route no run was split by another queue and no draw returned after its pass began: those fixes close paths that did not show here, though nothing rules them out elsewhere. The resets and the slot sequence are the ones that changed behaviour in play.

Issues

See #197

Checklist

For everyone

  • One topic.
  • The title is a Conventional Commit (type(scope): description).
  • Rebased on current main.
  • python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above).
  • Tested in the game, or what wasn't is named under Testing.
  • Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests).
  • Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported.
  • No game files, dumps, build output or generated files.
  • third_party/ untouched (a new folder with a PROVENANCE.md instead).
  • The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

If applicable

  • Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. The renderer builds on Linux; longshot_vk.dll cross-builds with tools.lock's llvm-mingw as in #197 (not run on Windows).
  • AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
  • Flags noted, and the switch reference updated (docs/development.md, the README).
  • A game-free test added for the fix, failing on main and passing here.
  • A check under tests/ that reads changed source updated in this PR, not weakened.
  • Docs updated.
## Why #197 gives Ocarina of Time's samples-passed reports the GPU's real occlusion counts, and the game leaves out the objects whose count is 0. Its rule is that a count not known for sure stays "visible" (4096): a wrong 0 makes an object the player can see vanish. A review of #197 found several ways a wrong or unknown count could still reach the game, and one that breaks a Vulkan rule on every frame (the validation layer reported it on the RX 580 and on the RTX 3090). This closes them, keeps the culling, and tests the real code paths. ## What changed - replay, query numbering (`gpu/vk/vk_replay.cpp`: `occ_begin_buffer`, `occ_end_buffer`, `occ_state_copied`, `occ_room`, `occ_open_oldest`): queries are numbered from one counter for all the game's queues (`occ_next_all`). It was in `NvnState`, and each queue has its own state (`RX_VK_QUEUE_STATE`, on by default), so two queues could hand out the same ring entries: a query begun again without a reset, one queue's 0 in another's report, host resets of queries still in flight, a stale ring tail. Each command buffer's replay copies the counter into its state before the dry pass and the workers copy that state, and writes it back however `replay_handle` returns. A run left open while another queue took queries is split: what it took is freed and its report stays visible. A queue's state made as a copy of another's drops the other's open run. The ring's room counts every queue's open run and every waiting report, in any order. - draw (`gpu/vk/vk_draw.cpp`: `OccGuard`, `draw()`): a query only counts a draw that drew. It is begun just before the draw command (`occ.issue()`), not when the pass opens. A draw that returns before its command (a missing sampler, a uint8 index buffer without the feature, the quad paths, a negative indirect count or stride, a buffer that could not be bound), or whose vertex input could not all be bound (`zero_buffer` in its place), never begins its query: it has no result, and the report stays visible. Before, those ended an empty query, which reads an available 0. - draw, mesh fallback: a task program split into batches drops its query instead of beginning it once per batch (devices without mesh shaders, e.g. the RX 580). - draw: the guard is made after `StatsQuery` (`RX_VK_DRAW_STATS`), so it ends first, inside the pass. - replay, report value: the reset and report cases record `rec_cur_value`, the submission the queries go out in, instead of a global that every `begin_recording` overwrote (a present's and frame generation's included). - replay, resets: queries read are reset from the host only once the GPU has finished the submission they went out in (its timeline value), in order (`occ_freeing`). The counts are still written as soon as the queries are available. Resetting on the availability bit alone gave `VUID-vkResetQueryPool-firstQuery-02741`, then `VUID-vkCmdBeginQuery-None-00807` at the query's next use, every frame (validation: ~25k and 2.6M on the RX 580 lab run of #197; capped thousands of each on the RTX 3090 here). - replay, slot sequence (`occ_slots`, `occ_slot_next`): each report slot has a report sequence, and a count is written only over the report it belongs to. The timestamp word alone let an older 0 replace a newer "visible" when two reports fell within one tick (~0.5 us at `RX_VK_TS_SCALE` 0.1). It is a direct-mapped table: a slot that evicts another drops the evicted one's count (the safe side; ~0.2-1% of reports here). - replay, held-back work (`occ_visible_ahead`, `occ_slot_visible`; `run_submit`'s stop, `submit_job`'s queued submits): the samples-passed reports of work held at an event wait read 4096 at once, so the game never reads an older count in a slot before the replay gets to its report. Each held submission is walked once. With `RX_VK_ASYNC=1` (a replay running behind the game) the queries are off. - switches: `RX_VK_OCC_FIXED` now wins over `RX_VK_OCC_QUERIES=1`, as documented. `RX_VK_OCC_LOG` also says why reports stayed visible. - tests: `tests/test_vk_occlusion_queries.py` compiles the real ring helpers, `OccGuard`, the task-batch block and `occ_queries_on()` against fakes. It covers several queue states, an early return after the pass began, missing vertex input, task batches, the report's value, same-tick slot reuse, slot-table eviction, the visible-ahead write, and the reset rule (the fake `vkResetQueryPool` fails if any query it resets went out in a submission that is not done). It also checks the wiring: every draw command after the guard preceded by `occ.issue()` with no way out between, and the guard after `StatsQuery`. `tests/test_draw_side_effects.py` and `tests/test_fallback_pipeline_state.py` cut code next to the change, so their fixtures follow it. - docs: `docs/development.md`, the occlusion paragraph. ## Flags and settings | Flag | What it does | Default now | Recommendation | Helps on | Doesn't help on | Untested on | |---|---|---|---|---|---|---| | `RX_VK_OCC_QUERIES` | Unchanged; off with `RX_VK_ASYNC=1` now | on | on | as #197 | - | AMD, Intel, the Steam Deck, Windows (this change) | | `RX_VK_OCC_FIXED=<n>` | Every report reads n; now also when `RX_VK_OCC_QUERIES=1` is set | unset | unchanged | - | - | - | | `RX_VK_OCC_LOG` | Also: queries not begun (draws that returned, vertex input, task batches), runs split by another queue, slots made visible ahead of a stopped queue and those walks' cost | off | off (a developer switch) | - | - | - | ## Measurements main: fdc2840b (2026-11-13), which has #197 this PR: fix/occlusion-visible-unless-known @ cd873617 (measured; the branch is now 794214d4, the same change rebased onto main fbd42024, see below) Hardware: RTX 3090 24 GB (NVIDIA 620.70.06), Ryzen 9 7900X, 30 GB, Linux (CachyOS, kernel 7.3.3) Scene: the pristine Kokiri save after the opening event: Kokiri Forest standing 40 s, then the camera turning at full speed 30 s; then Castle Town Market (debug teleport, 30 s to settle), standing 40 s and turning 30 s. 2048x1152, default options, shaders prepared, cap 240, `RX_VK_OCC_LOG=1` on both sides. Method: one game per run in a private headless sway with scripted input, under the PC's timing lock (own CPU cores). Both sides use the same game build (main's `longshot`), with each side's renderer built the same way (`RX_VK_ONLY`). Two interleaved sets with the final code, P M M P and M P M P (4 runs each side), and 2 runs of this PR with `RX_VK_OCC_QUERIES=0` for the no-culling reference. Draws and GPU time: the medians of the renderer's 120-present lines in each phase; frame times from `RX_VK_FRAME_LOG`. Each cell: the median of the runs' values. The PC swapped in every run (100k-1.7M pages in a phase). How to reproduce: as in #197: stand 40 s and turn 30 s in Kokiri and the Market; compare against main, and `RX_VK_OCC_QUERIES=0` for no culling. | | main @ fdc2840b | this PR @ cd873617 | this PR @ cd873617, `RX_VK_OCC_QUERIES=0` (no culling, reference) | |---|---|---|---| | Kokiri standing: draws a frame | 2,600 | 2,614 | 2,864 | | Kokiri standing: GPU time a frame | 9.05 ms | 9.00 ms | 8.98 ms | | Kokiri standing: frame time median / p99 / max | 15.47 / 21.30 / 27.9 ms | 15.20 / 20.05 / 25.5 ms | 15.32 / 19.95 / 43.2 ms | | Kokiri turning: draws | 1,753 | 1,760 | 1,709 | | Kokiri turning: frame time median / p99 | 13.68 / 19.15 ms | 13.71 / 20.30 ms | 13.37 / 18.55 ms | | Market standing: draws | 2,433 | 2,440 | 2,928 | | Market standing: GPU time | 7.53 ms | 7.40 ms | 7.95 ms | | Market standing: frame time median / p99 / max | 15.67 / 25.25 / 47.6 ms | 16.12 / 28.95 / 56.4 ms | 17.34 / 28.20 / 59.2 ms | | Market turning: draws (varies with where the turn ends) | 1,144 | 1,048 | 990 | | Market turning: frame time median / p99 | 13.36 / 28.35 ms | 13.60 / 28.90 ms | 13.05 / 27.40 ms | Market standing, frame median per run (ms), in run order: P M M P: 16.25 15.37 15.35 16.27; then M P M P: 15.97 16.00 16.16 15.85. In the second set main was as slow as this PR. An earlier set with a build that differed only in the slot table and the visible-ahead writes (M P P M M P): main 15.34 14.53 15.37, this PR 15.41 16.04 15.84. Over all of them main's median is 15.37 ms (7 runs), this PR's 16.00 (9). So this PR may cost ~0.5 ms in the Market standing, or it may be the PC: Kokiri is level, the GPU time is level, the visible-ahead writes are ruled out (two runs with them off: 16.17 and 15.70 ms), and a sampled profile of each side (`HOS_PROFILE=5 HOS_SAMPLE_ALL` + `HOS_CHAIN`, `scripts/chain.py`; 16.53 vs 16.73 ms a frame on the path) shows no single function behind it. The culling itself holds: 2,440 draws against 2,928 with no culling, and the frame median 16.12 against 17.34 ms. ``` main, Market standing: vk: 16200 presents, 64.8 fps; a frame: 2427 draws (0 skipped) ... this PR, RX_VK_OCC_LOG=1: vk: occlusion queries: ... reports written (... 56.7%), 0 left visible (a draw did not draw), 10 reported again first; written 1.3 recordings after the report; 80 waiting; 84 reads (66 wrote), 6.2 us each; ...; queries not begun: 0 draws returned before drawing, 0 with vertex input not all bound, 0 task batches; 0 runs split by another queue's queries; 2550 slots made visible ahead of a stopped queue (28 walks, 22.0 us each) main, RX_VK_VALIDATION=1: VUID-vkResetQueryPool-firstQuery-02741 (2,000, the duplicate cap), VUID-vkCmdBeginQuery-None-00807 (2,000, the cap) this PR, RX_VK_VALIDATION=1: no VUID ``` The timed A/B ran on main fdc2840b with this change at cd873617. Rebasing onto fbd42024 changed only one conflicting hunk: #198 renamed `pass().draws` to `ps.draws` in lines this change removes. Nothing else this change touches moved, so the numbers should still hold. The rebased build was rechecked in the game under validation (see Testing) rather than timed again. ## Testing **Tested:** - `python -I -B tests/run_all.py` on Linux at 794214d4 (on main fbd42024): 508 passed, 1 failed, 20 skipped. The failure is `test_bake_portability`: this PC has no `zstandard` module. (Before the rebase, at cd873617 on fdc2840b: 501 passed and the same 3 failures as main fdc2840b, two of them fixed on main since.) - The new test fails on main: the code it cuts out isn't there. Each fix was also put back to the old behaviour one at a time, and the test fails each time. The bugs put back: per-queue numbering, no split, a room check that ignores other queues, a copied state keeping the other's run, the query begun when the guard is made, the vertex-input drop ignored, task batches keeping the query, a global value, the timestamp-only check, the guard before `StatsQuery`, no visible-ahead, `RX_VK_OCC_FIXED` losing to `=1`, an `issue()` placed before a way out, reset on availability. - Validation layer (Khronos 1.4.363, `RX_VK_VALIDATION=1`), one short session each on the same route, RTX 3090: main had `VUID-vkResetQueryPool-firstQuery-02741` and `VUID-vkCmdBeginQuery-None-00807` up to the duplicate cap (2,000 each); this PR had no VUID at all (shader-module and SPIR-V float-mode messages filtered on both sides, as on the lab). - After the rebase onto main fbd42024 (794214d4; a full game build, shaders prepared): one short session over the same route with the validation layer and `RX_VK_OCC_LOG=1`. No VUID and no validation error (only the layer's usual `WARNING-vkGetDeviceProcAddr-device`). Culling held: Market standing 2,448 draws, ~50% of reports 0. 0 queries left unbegun, 0 runs split. Frame times from this session mean nothing (the layer). The A/B below the Measurements is from before the rebase; #198, the conflicting change, only takes the pass and descriptor writer once per draw, and the conflict was one line (`probe_draw` now reads `ps.draws`). - The flash detector (`RX_VK_FLASH=1`, every present compared with its neighbours), one session each over the whole route: 0 flashes on main in 11,733 presents, 0 with this PR in 11,639. - `RX_VK_OCC_LOG` in play (this PR): 0 queries left unbegun by a draw that returned, 0 by vertex input, 0 by task batches, 0 runs split by another queue on this route; 10-57 reports per 5,000 left visible by the slot table; ~2,100-2,550 slots per 5,000 reports made visible ahead of a stopped queue, in 24-28 walks of ~22 us. - Mesh fallback forced (`RX_VK_FORCE_MESH_FALLBACK=1`) on the RTX 3090: one session over the route: culling works on the compute path (54% of reports 0, 0 left visible), and no query ever reached the task-batch path (0 batches): the bounding-box runs are not task-program draws there. The batch rule is covered by the test only; a GPU without mesh shaders runs that path natively (Needs other testers). **Not tested:** AMD, Intel, the Steam Deck and Windows with this change (the lab RX 580 run was not repeated with it; the Windows build was not run: the change is portable C++ and Vulkan only); dungeons and interiors; long play. **Needs other testers:** - RX 580 or another GPU without mesh shaders, under `RX_VK_VALIDATION=1`: Kokiri and the Market, expecting no `vkResetQueryPool-firstQuery-02741` / `vkCmdBeginQuery-None-00807`, and `RX_VK_OCC_LOG` showing the task batches left visible (T-VDEY) ## Screenshots Nothing on screen: the same objects are culled as with #197 (draw counts below); no flash in either session. ## Risk and rollback A few more objects may stay drawn where a count is now unknown (a slot-table eviction, a stopped queue's report, a draw that did not draw): culling lost, never an object. `RX_VK_OCC_QUERIES=0` turns the queries off; reverting the commit restores #197. ## Reviewer notes Start with `occ_begin_buffer` / `occ_end_buffer` in `replay_handle`, then `OccGuard` and its `issue()` calls in `draw()`, then the deferred resets in `occ_collect`. On this route no run was split by another queue and no draw returned after its pass began: those fixes close paths that did not show here, though nothing rules them out elsewhere. The resets and the slot sequence are the ones that changed behaviour in play. ## Issues See #197 ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). - [x] Tested in the game, or what wasn't is named under Testing. - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). - [x] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### If applicable - [x] Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. The renderer builds on Linux; longshot_vk.dll cross-builds with tools.lock's llvm-mingw as in #197 (not run on Windows). - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] Flags noted, and the switch reference updated (docs/development.md, the README). - [x] A game-free test added for the fix, failing on `main` and passing here. - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. - [x] Docs updated.
fix(vk): occlusion counts stay visible unless known: one query numbering across queues, no count from a draw that didn't run
All checks were successful
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been skipped
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been skipped
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been skipped
Third-party parts / The same as upstream (pull_request) Has been skipped
CI / Conventional Commit title (pull_request) Successful in 4s
CI / Workflow lint (actionlint) (pull_request) Successful in 7s
Third-party parts / As their notes say (pull_request) Successful in 8s
PR description / Description follows the template (pull_request_target) Successful in 7s
794214d466
The rule: a samples-passed report reads 4096 ("visible") unless its count is known for sure; a wrong 0 hides an object
in view. Review findings on #197, each fixed here:

- One query counter for every queue (occ_next_all). Each game queue has its own NvnState, and the counter lived there:
  two queues handed out the same ring entries (a query begun twice without a reset, one queue's 0 in another's report,
  host resets of queries in flight, a stale ring tail). Each command buffer's replay now starts its state's copy from
  the shared counter (occ_begin_buffer; the dry pass and workers copy it from there) and writes it back however it
  returns (occ_end_buffer). A run left open while another queue took queries is split: what it took is freed and its
  report stays visible. A queue state copied from another drops the other's open run. The ring's room counts every
  queue's open run and every report, in any order.
- A query only counts a draw that drew (OccGuard, vk_draw.cpp): it is begun just before the draw command
  (occ.issue()), not when the pass opens. A draw that returns before its command (a missing sampler, a uint8 index
  buffer without the feature, the quad paths, a bad indirect draw, a buffer not bound), or whose vertex input could not
  all be bound (zero_buffer in its place), never begins its query: no result, so the report stays visible.
- The mesh fallback's task batches drop the query instead of beginning it once per batch.
- The report and the reset record rec_cur_value, the submission the queries go out in, not a global every
  begin_recording overwrote (a present's and frame generation's recordings included).
- Each report slot has a report sequence (a direct-mapped table: a slot evicted by another drops its count, the safe
  side); a count lands only over the report it belongs to. The timestamp word alone let an older 0 replace a newer
  "visible" when two reports fell in one tick.
- The guard is made after StatsQuery (RX_VK_DRAW_STATS), so it ends inside the pass.
- Work held at an event wait (a stopped queue, its later submits) has its samples-passed reports written as 4096 at
  once (each submission walked once), so the game never reads an older count there before the replay gets to it.
  With RX_VK_ASYNC=1 the queries are off.
- Queries read are reset only once the GPU has finished their submission (its timeline value); the counts are still
  written as soon as the queries are available. Resetting on the availability bit alone made the validation layer
  report VUID-vkResetQueryPool-firstQuery-02741 and then VUID-vkCmdBeginQuery-None-00807 at each next use.
- RX_VK_OCC_FIXED wins over RX_VK_OCC_QUERIES=1, as documented.
- RX_VK_OCC_LOG also says why reports were left visible: draws that returned, vertex input, task batches, runs split by
  another queue, slots made visible ahead of a stopped queue (and those walks' cost).

tests/test_vk_occlusion_queries.py now compiles the real ring helpers, OccGuard, the task-batch block and
occ_queries_on() against fakes: several queue states, an early return after the pass began, missing vertex input,
task batches, the report's value, the same-tick slot reuse, the deferred resets, the switches. test_draw_side_effects
and test_fallback_pipeline_state cut code next to the changes: their fixtures follow.

Co-Authored-By: Claude Fable 5.2 <[email protected]>
HumanBean changed title from WIP: fix(vk): occlusion counts stay visible unless known: one query numbering across queues, no count from a draw that didn't run to fix(vk): occlusion counts stay visible unless known: one query numbering across queues, no count from a draw that didn't run 2026-11-13 18:12:59 +00:00
HumanBean deleted branch fix/occlusion-visible-unless-known 2026-11-13 18:15:26 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
MagicBeanDev/Longshot!260
No description provided.