fix(vk): occlusion counts stay visible unless known: one query numbering across queues, no count from a draw that didn't run #260
No reviewers
Labels
No labels
area/accessibility
area/audio
area/input
area/install
area/menu
area/renderer
area/saves
area/upscaling
good first issue
gpu/amd
gpu/igpu
gpu/intel
gpu/nvidia
help wanted
kind
bug
kind
chore
kind
docs
kind
feature
kind
performance
kind
question
needs measurements
platform/linux
platform/steam-deck
platform/windows
preview
priority
critical
priority
high
priority
low
status
blocked
status
confirmed
status
duplicate
status
invalid
status
needs-info
status
wontfix
testing wanted
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!260
Loading…
Reference in a new issue
No description provided.
Delete branch "HumanBean/Longshot:fix/occlusion-visible-unless-known"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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
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 inNvnState, 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 howeverreplay_handlereturns. 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.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_bufferin 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.StatsQuery(RX_VK_DRAW_STATS), so it ends first, inside the pass.rec_cur_value, the submission the queries go out in, instead of a global that everybegin_recordingoverwrote (a present's and frame generation's included).occ_freeing). The counts are still written as soon as the queries are available. Resetting on the availability bit alone gaveVUID-vkResetQueryPool-firstQuery-02741, thenVUID-vkCmdBeginQuery-None-00807at 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).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 atRX_VK_TS_SCALE0.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).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. WithRX_VK_ASYNC=1(a replay running behind the game) the queries are off.RX_VK_OCC_FIXEDnow wins overRX_VK_OCC_QUERIES=1, as documented.RX_VK_OCC_LOGalso says why reports stayed visible.tests/test_vk_occlusion_queries.pycompiles the real ring helpers,OccGuard, the task-batch block andocc_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 fakevkResetQueryPoolfails 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 byocc.issue()with no way out between, and the guard afterStatsQuery.tests/test_draw_side_effects.pyandtests/test_fallback_pipeline_state.pycut code next to the change, so their fixtures follow it.docs/development.md, the occlusion paragraph.Flags and settings
RX_VK_OCC_QUERIESRX_VK_ASYNC=1nowRX_VK_OCC_FIXED=<n>RX_VK_OCC_QUERIES=1is setRX_VK_OCC_LOGMeasurements
main:
fdc2840b(2026-11-13), which has #197this PR: fix/occlusion-visible-unless-known @ cd873617 (measured; the branch is now
794214d4, the same change rebased onto mainfbd42024, 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=1on 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 withRX_VK_OCC_QUERIES=0for the no-culling reference. Draws and GPU time: the medians of the renderer's 120-present lines in each phase; frame times fromRX_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=0for no culling.fdc2840bRX_VK_OCC_QUERIES=0(no culling, reference)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.The timed A/B ran on main
fdc2840bwith this change at cd873617. Rebasing ontofbd42024changed only one conflicting hunk: #198 renamedpass().drawstops.drawsin 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.pyon Linux at794214d4(on mainfbd42024): 508 passed, 1 failed, 20 skipped. The failure istest_bake_portability: this PC has nozstandardmodule. (Before the rebase, at cd873617 onfdc2840b: 501 passed and the same 3 failures as mainfdc2840b, two of them fixed on main since.)StatsQuery, no visible-ahead,RX_VK_OCC_FIXEDlosing to=1, anissue()placed before a way out, reset on availability.RX_VK_VALIDATION=1), one short session each on the same route, RTX 3090: main hadVUID-vkResetQueryPool-firstQuery-02741andVUID-vkCmdBeginQuery-None-00807up 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).fbd42024(794214d4; a full game build, shaders prepared): one short session over the same route with the validation layer andRX_VK_OCC_LOG=1. No VUID and no validation error (only the layer's usualWARNING-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_drawnow readsps.draws).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_LOGin 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.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_VK_VALIDATION=1: Kokiri and the Market, expecting novkResetQueryPool-firstQuery-02741/vkCmdBeginQuery-None-00807, andRX_VK_OCC_LOGshowing 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=0turns the queries off; reverting the commit restores #197.Reviewer notes
Start with
occ_begin_buffer/occ_end_bufferinreplay_handle, thenOccGuardand itsissue()calls indraw(), then the deferred resets inocc_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
type(scope): description).main.python -I -B tests/run_all.pyrun and passing (any failure that also fails onmainis named above).third_party/untouched (a new folder with aPROVENANCE.mdinstead).If applicable
Assisted-by:/Co-Authored-By:trailers.mainand passing here.tests/that reads changed source updated in this PR, not weakened.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]>WIP: fix(vk): occlusion counts stay visible unless known: one query numbering across queues, no count from a draw that didn't runto fix(vk): occlusion counts stay visible unless known: one query numbering across queues, no count from a draw that didn't run