MagicBeanDev/Longshot

Watch

Fork 98

fix(vk): a draw left out after it took its occlusion query no longer reports "hidden" #253

Closed

jarynclouatre wants to merge 1 commit from jarynclouatre/Longshot:fix/occlusion-skipped-draw into main

jarynclouatre commented

2026-11-13 17:38:13 +00:00

Contributor

Why

A draw that is left out partway through draw() could tell the game its object is hidden. With the occlusion counts of #197, the game leaves out of its draw lists the objects whose report reads 0. A draw whose query was begun and that then returned before its draw command (descriptors that could not be bound, an index buffer or the indirect records not found, a negative count from the game) ended a query that had counted nothing: available, with a count of 0. The object was then left out though nothing had tested it. The rule for such a draw is "visible", as the comment at the guard says; it only held for a draw that returned before the query was begun.

What changed

vk (gpu/vk/vk_draw.cpp, draw): OccGuard no longer begins the query where it is made. It has a begin() that begins it once, and occ_guard.begin() stands before each vkCmdDraw* of draw() (nine places covering its thirteen draw commands). The destructor ends the query only if it was begun. A draw that takes any SKIP_HERE() after the guard leaves its query unused, and occ_collect (gpu/vk/vk_replay.cpp, unchanged) keeps its report "visible" once the submission is done.
tests (tests/test_vk_occlusion_queries.py): the guard is cut from vk_draw.cpp and run against the ring with spies for the two query commands. A draw that reaches its command begins once (asked twice) and ends; one that does not begins nothing, and the report of the run stays 4096. The source check now reads draw(): no vkCmdBeginQuery after the guard, every draw command has occ_guard.begin() ahead of it, and nothing returns between the two.

Flags and settings

None.

Measurements

main: fbd42024 (2026-11-13)
this PR: fix/occlusion-skipped-draw @ b3075e9f
Hardware: the game-free checks on a Linux x86-64 PC (g++ 15, Python 3.14.4); the Windows back end cross-built with llvm-mingw 20261104; the game run on Windows 11, RTX 4070 Ti SUPER, Ryzen 7 9800X3D
Scene: Castle Town's gate and the guard house, by scripted input (see Testing)
Method: python3 -I -B tests/run_all.py, two runs each, interleaved (main, PR, main, PR), both runs of a side gave the same counts; this PR's tests/test_vk_occlusion_queries.py also run against main's sources
How to reproduce: the commands above in a checkout of each commit

	main @ `fbd42024`	this PR @ `b3075e9f`
`tests/run_all.py` (run 1, run 2)	449 passed, 1 failed, 79 skipped (both)	449 passed, 1 failed, 79 skipped (both)
failing check	`test_bake_portability.py`	the same one
this PR's `test_vk_occlusion_queries.py`	fails (the guard begins the query where it is made)	passes
`longshot_vk.dll` for Windows (`RX_VK_ONLY=1 RX_RELEASE=1 nx2/build.py --standalone`)	not built	builds, 0 failed

suite main run 1: 449 passed, 1 failed, 79 skipped in 71 s (linux, 16 at once); failing: tests/test_bake_portability.py
suite PR   run 1: 449 passed, 1 failed, 79 skipped in 70 s (linux, 16 at once); failing: tests/test_bake_portability.py
suite main run 2: 449 passed, 1 failed, 79 skipped in 70 s (linux, 16 at once); failing: tests/test_bake_portability.py
suite PR   run 2: 449 passed, 1 failed, 79 skipped in 69 s (linux, 16 at once); failing: tests/test_bake_portability.py
PASS: occlusion counts: summed once their draws are done, visible when unknown (a draw left out never begins its query), overflowed, with a colour draw or reported again, in order, read and reset in one batch (two across the ring's end), the dry pass numbering as the workers do

test_bake_portability.py fails here for a missing Python module (zstandard), on main as well.

Testing

Tested: the game-free suite on a Linux PC, as in Measurements. The game on Windows 11 (RTX 4070 Ti SUPER, Ryzen 7 9800X3D): main @ fbd42024 against b3075e9f, two runs each, interleaved, the same scripted input of about 22,400 presents (start-up, a save loaded, Castle Town's gate, the guard house) with the occlusion queries on. The log's occlusion lines agree: 56.9% and 56.8% of the reports read 0 on main, 56.8% and 56.4% with the fix, none left visible, 60.0 fps on both; the last frame of each run shows the same room with the same objects. The figures are in a comment below.

Not tested: a draw left out after its query in the game. No draw was skipped in the four runs, so that path is covered by the fixture only. The Linux back end was not built here.

Needs other testers: None.

Screenshots

Nothing on screen.

Risk and rollback

The query now covers the draw command alone, not the binds before it; only draw commands produce samples, so the counts are the same. A draw command added to draw() later without occ_guard.begin() would leave its query unused, which costs culling for that report and never an object; the source check fails on it. RX_VK_OCC_QUERIES=0 turns the occlusion counts off as before, and reverting the commit puts the old guard back.

Reviewer notes

Found while reading #197's code, not from a player's report: I know of no missing object traced to this.
git diff main...HEAD -- gpu/vk/vk_draw.cpp is the guard and nine one-line additions.

Issues

None.

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

A check under tests/ that reads changed source updated in this PR, not weakened.
Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths.

## Why A draw that is left out partway through `draw()` could tell the game its object is hidden. With the occlusion counts of #197, the game leaves out of its draw lists the objects whose report reads 0. A draw whose query was begun and that then returned before its draw command (descriptors that could not be bound, an index buffer or the indirect records not found, a negative count from the game) ended a query that had counted nothing: available, with a count of 0. The object was then left out though nothing had tested it. The rule for such a draw is "visible", as the comment at the guard says; it only held for a draw that returned before the query was begun. ## What changed - vk (`gpu/vk/vk_draw.cpp`, `draw`): `OccGuard` no longer begins the query where it is made. It has a `begin()` that begins it once, and `occ_guard.begin()` stands before each `vkCmdDraw*` of `draw()` (nine places covering its thirteen draw commands). The destructor ends the query only if it was begun. A draw that takes any `SKIP_HERE()` after the guard leaves its query unused, and `occ_collect` (`gpu/vk/vk_replay.cpp`, unchanged) keeps its report "visible" once the submission is done. - tests (`tests/test_vk_occlusion_queries.py`): the guard is cut from `vk_draw.cpp` and run against the ring with spies for the two query commands. A draw that reaches its command begins once (asked twice) and ends; one that does not begins nothing, and the report of the run stays 4096. The source check now reads `draw()`: no `vkCmdBeginQuery` after the guard, every draw command has `occ_guard.begin()` ahead of it, and nothing returns between the two. ## Flags and settings None. ## Measurements main: fbd42024 (2026-11-13) this PR: fix/occlusion-skipped-draw @ b3075e9f Hardware: the game-free checks on a Linux x86-64 PC (g++ 15, Python 3.14.4); the Windows back end cross-built with llvm-mingw 20261104; the game run on Windows 11, RTX 4070 Ti SUPER, Ryzen 7 9800X3D Scene: Castle Town's gate and the guard house, by scripted input (see Testing) Method: `python3 -I -B tests/run_all.py`, two runs each, interleaved (main, PR, main, PR), both runs of a side gave the same counts; this PR's `tests/test_vk_occlusion_queries.py` also run against main's sources How to reproduce: the commands above in a checkout of each commit | | main @ fbd42024 | this PR @ b3075e9f | |---|---|---| | `tests/run_all.py` (run 1, run 2) | 449 passed, 1 failed, 79 skipped (both) | 449 passed, 1 failed, 79 skipped (both) | | failing check | `test_bake_portability.py` | the same one | | this PR's `test_vk_occlusion_queries.py` | fails (the guard begins the query where it is made) | passes | | `longshot_vk.dll` for Windows (`RX_VK_ONLY=1 RX_RELEASE=1 nx2/build.py --standalone`) | not built | builds, 0 failed | ``` suite main run 1: 449 passed, 1 failed, 79 skipped in 71 s (linux, 16 at once); failing: tests/test_bake_portability.py suite PR run 1: 449 passed, 1 failed, 79 skipped in 70 s (linux, 16 at once); failing: tests/test_bake_portability.py suite main run 2: 449 passed, 1 failed, 79 skipped in 70 s (linux, 16 at once); failing: tests/test_bake_portability.py suite PR run 2: 449 passed, 1 failed, 79 skipped in 69 s (linux, 16 at once); failing: tests/test_bake_portability.py PASS: occlusion counts: summed once their draws are done, visible when unknown (a draw left out never begins its query), overflowed, with a colour draw or reported again, in order, read and reset in one batch (two across the ring's end), the dry pass numbering as the workers do ``` `test_bake_portability.py` fails here for a missing Python module (zstandard), on main as well. ## Testing **Tested:** the game-free suite on a Linux PC, as in Measurements. The game on Windows 11 (RTX 4070 Ti SUPER, Ryzen 7 9800X3D): main @ fbd42024 against b3075e9f, two runs each, interleaved, the same scripted input of about 22,400 presents (start-up, a save loaded, Castle Town's gate, the guard house) with the occlusion queries on. The log's occlusion lines agree: 56.9% and 56.8% of the reports read 0 on main, 56.8% and 56.4% with the fix, none left visible, 60.0 fps on both; the last frame of each run shows the same room with the same objects. The figures are in a comment below. **Not tested:** a draw left out after its query in the game. No draw was skipped in the four runs, so that path is covered by the fixture only. The Linux back end was not built here. **Needs other testers:** None. ## Screenshots Nothing on screen. ## Risk and rollback The query now covers the draw command alone, not the binds before it; only draw commands produce samples, so the counts are the same. A draw command added to `draw()` later without `occ_guard.begin()` would leave its query unused, which costs culling for that report and never an object; the source check fails on it. `RX_VK_OCC_QUERIES=0` turns the occlusion counts off as before, and reverting the commit puts the old guard back. ## Reviewer notes - Found while reading #197's code, not from a player's report: I know of no missing object traced to this. - `git diff main...HEAD -- gpu/vk/vk_draw.cpp` is the guard and nine one-line additions. ## Issues None. ## 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] A check under `tests/` that reads changed source updated in this PR, not weakened. - [ ] Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths.

jarynclouatre added 1 commit

2026-11-13 17:38:13 +00:00

fix(vk): a draw left out after it took its occlusion query no longer reports "hidden"

PR description / Description follows the template (pull_request_target) Failing after 4s

Details

CI / Conventional Commit title (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled

Details

CI / Workflow lint (actionlint) (pull_request) Has been cancelled

Details

Third-party parts / As their notes say (pull_request) Has been cancelled

Details

Third-party parts / The same as upstream (pull_request) Has been cancelled

Details

Website / Website on Cloudflare (pull_request_target) Has been skipped

Details

b3075e9f65

draw() began the draw's occlusion query as soon as its pass was begun, and
ended it when draw() returned. A draw left out further down (descriptors that
could not be bound, an index buffer or the indirect records not found, a
count the game gave as negative) returned through the same guard, so its
query ended having counted nothing. The query was then available with a
count of 0, the report's sum could come to 0, and the game left the object
out of its draw lists though nothing had tested it. The collector's rule for
an unknown draw, "visible", only covers a query that was never begun.

The guard now begins the query at the draw command: occ_guard.begin() stands
before each vkCmdDraw* of draw(), with no way out of the function between
the two. A draw left out on the way never begins its query, and its report
stays "visible" as the comment at the guard already promised.

tests/test_vk_occlusion_queries.py runs the guard cut from vk_draw.cpp
against the ring: a draw that reaches its command begins once and ends, one
that does not begins nothing and the report stays 4096. It also reads
draw(): every draw command has its begin, and nothing returns in between.

jarynclouatre commented

2026-11-13 22:59:43 +00:00

Author

Contributor

The game run is done, on Windows 11 with an RTX 4070 Ti SUPER and a Ryzen 7 9800X3D.

main @ fbd42024 against b3075e9f, two runs each, interleaved (main, PR, main, PR). Each run is the same scripted input: start-up, a save loaded, a teleport to Castle Town's gate and a walk into the guard house, about 22,400 presents. Occlusion queries at their default (on), RX_VK_OCC_LOG=1 for the log's occlusion lines. The same executable, settings and prepared shader state on both sides, a prepare of its own for each DLL.

run	reports written	of them 0	fps (median of the log's lines)
main 1	5,733,849	3,262,709 (56.9%)	60.0
PR 1	5,720,844	3,247,822 (56.8%)	60.0
main 2	5,769,473	3,278,510 (56.8%)	60.0
PR 2	5,791,391	3,266,692 (56.4%)	60.0

The counts are the sums of each run's occlusion lines. The share of reports that read 0 is the same on both sides within what two runs of one side differ by, so the query that now begins at the draw command counts what it counted before. The last frame of each run shows the same room with the same objects; the runs differ only in where the idle animation stands.

Not shown by this run: a draw left out after its query. No draw was skipped in any of the four runs, so that path is still covered by the fixture only.

The game run is done, on Windows 11 with an RTX 4070 Ti SUPER and a Ryzen 7 9800X3D. main @ fbd42024 against b3075e9f, two runs each, interleaved (main, PR, main, PR). Each run is the same scripted input: start-up, a save loaded, a teleport to Castle Town's gate and a walk into the guard house, about 22,400 presents. Occlusion queries at their default (on), `RX_VK_OCC_LOG=1` for the log's occlusion lines. The same executable, settings and prepared shader state on both sides, a prepare of its own for each DLL. | run | reports written | of them 0 | left visible | draws skipped | fps (median of the log's lines) | |---|---|---|---|---|---| | main 1 | 5,733,849 | 3,262,709 (56.9%) | 0 | 0 | 60.0 | | PR 1 | 5,720,844 | 3,247,822 (56.8%) | 0 | 0 | 60.0 | | main 2 | 5,769,473 | 3,278,510 (56.8%) | 0 | 0 | 60.0 | | PR 2 | 5,791,391 | 3,266,692 (56.4%) | 0 | 0 | 60.0 | The counts are the sums of each run's occlusion lines. The share of reports that read 0 is the same on both sides within what two runs of one side differ by, so the query that now begins at the draw command counts what it counted before. The last frame of each run shows the same room with the same objects; the runs differ only in where the idle animation stands. Not shown by this run: a draw left out after its query. No draw was skipped in any of the four runs, so that path is still covered by the fixture only.

jarynclouatre changed title from ~~WIP: fix(vk): a draw left out after it took its occlusion query no longer reports "hidden"~~ to fix(vk): a draw left out after it took its occlusion query no longer reports "hidden"

2026-11-13 22:59:48 +00:00

jarynclouatre commented

2026-11-13 23:00:41 +00:00

Author

Contributor

#260 (f3fe1763) put the same rule on main while this branch waited for its game run: the query begins at the draw command there (occ.issue()), and it also drops the query of a draw whose vertex input could not all be bound, which this branch does not. The two now conflict in draw() and nothing of this branch is left to add, so I am closing it. The run above was main before #260 against this branch.

#260 (f3fe1763) put the same rule on main while this branch waited for its game run: the query begins at the draw command there (`occ.issue()`), and it also drops the query of a draw whose vertex input could not all be bound, which this branch does not. The two now conflict in `draw()` and nothing of this branch is left to add, so I am closing it. The run above was main before #260 against this branch.

jarynclouatre closed this pull request

2026-11-13 23:00:42 +00:00