WIP: fix(vk): give samples-passed reports the GPU's counts (RX_VK_OCC_QUERIES) #23

Draft
dyst wants to merge 1 commit from dyst/Longshot:fix/vk-occlusion-queries into main
First-time contributor

Why

On Vulkan every occlusion report reads 4096 (since NotRexed/Hookshot#487), so Ocarina of Time never culls an object hidden behind something: it draws everything, every frame. This gives the game the GPU's real counts again, behind an opt-in switch, so it can be tested on more hardware before it is on by default.

What changed

  • replay: occlusion queries in gpu/vk/vk_replay.cpp (RX_VK_OCC_QUERIES=1, off by default):
    • occ_queries_on() makes a 65,536-entry query pool (kOccPool); it needs hostQueryReset (core in Vulkan 1.2), and without it the switch logs and stays off;
    • occ_take() gives each draw from a samples-passed reset (NOP_RESET_COUNTER) to its report (NOP_REPORT_COUNTER) a query, numbered in NvnState in every mode, so a parallel replay's dry pass and its workers agree on them; NOP_DRAW, NOP_DRAW_IDX, NOP_DRAW_INDIRECT and the mesh draw take one;
    • the report is still written as 4096 at replay, as before. occ_collect(), called as each recording begins (begin_recording), reads the reports whose queries are done and writes their sum into the slot, unless the slot's timestamp word shows it was reported into again;
    • a report whose draw never ran, or that took more than 256 draws (kOccRun), stays 4096, so a late or unknown count costs culling, never a missing object;
    • replay_handle sets the command buffer's query limit (occ_room(), kOccBuffer) before its dry pass's start state is copied;
    • RX_VK_OCC_LOG=1 logs, every ~5,000 reports, how many were written, how many read 0 and how many recordings late they were.
  • draw: draw() in gpu/vk/vk_draw.cpp begins the query once its pass is open and ends it as it returns (OccGuard), before each_guard can end the pass; DrawArgs (gpu/vk/vk_draw.h) carries occ_pool and occ_query.
  • device: create_device() in gpu/vk/vk_main.cpp records occlusionQueryPrecise and hostQueryReset (Device::occlusion_precise, host_query_reset in gpu/vk/vk_internal.h); queries are precise where the device has it.
  • state: NvnState (gpu/vk/vk_state.h) gains occ_next, occ_first, occ_limit, occ_on, occ_overflow.
  • tests: new tests/test_vk_occlusion_queries.py runs the ring's code (cut from vk_replay.cpp) against a fake query pool and guest memory (sum when done; "visible" when a draw never ran, overflowed or was reported again; in order; ring reset once read; dry-pass numbering) and checks the wiring in the source; it fails without the change. tests/fsr_uniform_ownership_test.cpp stubs occ_collect / occ_value, which begin_recording now uses.

Flags and settings

Flag New or changed What it does Default now Goal: on by default? Helps on Doesn't help on Untested on Untested, but should help on
RX_VK_OCC_QUERIES new the game's samples-passed reports get the GPU's occlusion counts, so it culls hidden objects off after more testing: AMD, the Steam Deck, integrated GPUs and Linux first RTX 3080, Kokiri Forest standing still (~1,330 draws a frame against ~1,570) - AMD, the Steam Deck, integrated GPUs, Linux; a moving camera, indoor scenes indoor scenes (bigger savings where more is hidden)
RX_VK_OCC_LOG new logs, every ~5,000 reports, how many were written, how many read 0, and how late off no (a developer switch) - - AMD, the Steam Deck, integrated GPUs, Linux -

Measurements

Measured on the GitHub version (NotRexed/Hookshot#2871), not yet on Longshot: this PR stays WIP until it is.

Hardware: RTX 3080 (NVIDIA 621.14), Ryzen 9 3900X, 64 GB, Windows 11
Scene: Kokiri Forest, first save, standing still ~100 s after loading; headless, 2048x1152.
Method: 200 s each, back to back on a quiet machine: the default (reports read 4096), every report forced to 0 (RX_VK_OCC_FIXED=0, the ceiling), and the GPU's counts (RX_VK_OCC_QUERIES=1, this PR).
Baseline: GitHub, default (reports read 4096)

GitHub, default (reports read 4096) GitHub, RX_VK_OCC_FIXED=0 this PR (GitHub), RX_VK_OCC_QUERIES=1
Reports read 4096 0 the GPU's counts
Draws a frame ~1,570 ~1,010 ~1,330
GPU time a frame ~11.5 ms ~7.5 ms ~11.0 ms
Frame rate 26-28 fps ~30 fps 27.5-29 fps
Picture normal nearly everything gone same as default

About 32% of reports read 0, each written ~1 recording (about a sixth of a frame) after its report; none had to stay "visible".

Also measured: LONGSHOT_OCCLUSION_OFF=1 gives the same picture and the same GPU time and frame rate as the default on Vulkan, as expected since nothing was culled before.

In a dumped Kokiri Forest frame there are 491 samples-passed reports and no conditional render at all; with every report forced to 0 the game makes ~565 fewer draws a frame and none are "skipped" by a condition. So the counts only have to reach guest memory within a frame or two; no VK_EXT_conditional_rendering is needed.

Checks: python -I -B tests/run_all.py on Windows: the failure left is test_bake_portability (no zstandard in that Python), the same without this change.

Testing

Tested: the runs under Measurements (on the GitHub version) and the game-free checks this description names. Checked but not measured: screenshots at the same present, compared with the default run (below).

Not tested: a moving camera and indoor scenes (pop-in when something comes out from behind cover; bigger savings where more is hidden); the sun with real counts (its haze should now dim when the sun is hidden); AMD (Tier 1, half the reports), the Steam Deck, integrated GPUs and Linux. Testing wanted there before this is on by default.

Needs other testers:

  • AMD (Windows or Linux): Kokiri Forest and Castle Town Market with RX_VK_OCC_QUERIES=1, expecting no objects missing and no flicker against the switch unset (request to open)
  • Steam Deck: the same scenes with RX_VK_OCC_QUERIES=1, expecting no hang and no objects missing (request to open)

Screenshots

Nothing on screen: no visual change is expected. Screenshots at the same present matched the default run apart from the falling leaves and fairies; none are attached (the pair can be added).

Risk and rollback

Off by default: unset, the replay takes no queries and the reports read 4096 as before. Switched on, an object could be culled that should be drawn (a count written to the wrong slot, or pop-in as something comes out from behind cover), and the queries add work to the replay thread and the GPU on hardware not tried yet; a count that is late or unknown stays "visible". Unsetting RX_VK_OCC_QUERIES turns it off; reverting the commit is enough.

Reviewer notes

Carried over from NotRexed/Hookshot#2871 with the move to Longshot: the same change on Longshot's main, with its rename applied. Its review and discussion are there.

A draft: the occlusion queries run in the game; it stays a draft as on GitHub.

How the game culls: the z-prepass draws each object between a samples-passed reset and a report into the object's slot. The game reads those slots back on the CPU a frame or two later and leaves the objects whose count was 0 out of its draw lists.

The checks and the in-game runs above were done on the GitHub version; this branch is the same change with the rename applied (the same files and line counts). Forgejo runs no checks, and they were not run again here.

Issues

See NotRexed/Hookshot#487 (where the reports started reading 4096).

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

  • AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
  • Flags table filled in; README / docs/development.md switch reference updated.
  • Screenshots for a change that shows on screen.
  • A row in docs/tested-configurations.md for each measured run.
  • MKWVK_API_VERSION bumped (gpu/vk/vk_api.h's MkwVkHost or MkwVkRenderer changed).
  • The SPIR-V headers regenerated with glslang (a shader changed).
  • 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.
  • Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths.
  • Docs updated.

🤖 Generated with Claude Code

## Why On Vulkan every occlusion report reads 4096 (since [NotRexed/Hookshot#487](https://github.com/NotRexed/Hookshot/pull/487)), so Ocarina of Time never culls an object hidden behind something: it draws everything, every frame. This gives the game the GPU's real counts again, behind an opt-in switch, so it can be tested on more hardware before it is on by default. ## What changed - replay: occlusion queries in `gpu/vk/vk_replay.cpp` (`RX_VK_OCC_QUERIES=1`, off by default): - `occ_queries_on()` makes a 65,536-entry query pool (`kOccPool`); it needs `hostQueryReset` (core in Vulkan 1.2), and without it the switch logs and stays off; - `occ_take()` gives each draw from a samples-passed reset (`NOP_RESET_COUNTER`) to its report (`NOP_REPORT_COUNTER`) a query, numbered in `NvnState` in every mode, so a parallel replay's dry pass and its workers agree on them; `NOP_DRAW`, `NOP_DRAW_IDX`, `NOP_DRAW_INDIRECT` and the mesh draw take one; - the report is still written as 4096 at replay, as before. `occ_collect()`, called as each recording begins (`begin_recording`), reads the reports whose queries are done and writes their sum into the slot, unless the slot's timestamp word shows it was reported into again; - a report whose draw never ran, or that took more than 256 draws (`kOccRun`), stays 4096, so a late or unknown count costs culling, never a missing object; - `replay_handle` sets the command buffer's query limit (`occ_room()`, `kOccBuffer`) before its dry pass's start state is copied; - `RX_VK_OCC_LOG=1` logs, every ~5,000 reports, how many were written, how many read 0 and how many recordings late they were. - draw: `draw()` in `gpu/vk/vk_draw.cpp` begins the query once its pass is open and ends it as it returns (`OccGuard`), before `each_guard` can end the pass; `DrawArgs` (`gpu/vk/vk_draw.h`) carries `occ_pool` and `occ_query`. - device: `create_device()` in `gpu/vk/vk_main.cpp` records `occlusionQueryPrecise` and `hostQueryReset` (`Device::occlusion_precise`, `host_query_reset` in `gpu/vk/vk_internal.h`); queries are precise where the device has it. - state: `NvnState` (`gpu/vk/vk_state.h`) gains `occ_next`, `occ_first`, `occ_limit`, `occ_on`, `occ_overflow`. - tests: new `tests/test_vk_occlusion_queries.py` runs the ring's code (cut from `vk_replay.cpp`) against a fake query pool and guest memory (sum when done; "visible" when a draw never ran, overflowed or was reported again; in order; ring reset once read; dry-pass numbering) and checks the wiring in the source; it fails without the change. `tests/fsr_uniform_ownership_test.cpp` stubs `occ_collect` / `occ_value`, which `begin_recording` now uses. ## Flags and settings | Flag | New or changed | What it does | Default now | Goal: on by default? | Helps on | Doesn't help on | Untested on | Untested, but should help on | |---|---|---|---|---|---|---|---|---| | `RX_VK_OCC_QUERIES` | new | the game's samples-passed reports get the GPU's occlusion counts, so it culls hidden objects | off | after more testing: AMD, the Steam Deck, integrated GPUs and Linux first | RTX 3080, Kokiri Forest standing still (~1,330 draws a frame against ~1,570) | - | AMD, the Steam Deck, integrated GPUs, Linux; a moving camera, indoor scenes | indoor scenes (bigger savings where more is hidden) | | `RX_VK_OCC_LOG` | new | logs, every ~5,000 reports, how many were written, how many read 0, and how late | off | no (a developer switch) | - | - | AMD, the Steam Deck, integrated GPUs, Linux | - | ## Measurements Measured on the GitHub version (NotRexed/Hookshot#2871), not yet on Longshot: this PR stays WIP until it is. Hardware: RTX 3080 (NVIDIA 621.14), Ryzen 9 3900X, 64 GB, Windows 11 Scene: Kokiri Forest, first save, standing still ~100 s after loading; headless, 2048x1152. Method: 200 s each, back to back on a quiet machine: the default (reports read 4096), every report forced to 0 (`RX_VK_OCC_FIXED=0`, the ceiling), and the GPU's counts (`RX_VK_OCC_QUERIES=1`, this PR). Baseline: GitHub, default (reports read 4096) | | GitHub, default (reports read 4096) | GitHub, `RX_VK_OCC_FIXED=0` | this PR (GitHub), `RX_VK_OCC_QUERIES=1` | |---|---|---|---| | Reports read | 4096 | 0 | the GPU's counts | | Draws a frame | ~1,570 | ~1,010 | ~1,330 | | GPU time a frame | ~11.5 ms | ~7.5 ms | ~11.0 ms | | Frame rate | 26-28 fps | ~30 fps | 27.5-29 fps | | Picture | normal | nearly everything gone | same as default | About 32% of reports read 0, each written ~1 recording (about a sixth of a frame) after its report; none had to stay "visible". Also measured: `LONGSHOT_OCCLUSION_OFF=1` gives the same picture and the same GPU time and frame rate as the default on Vulkan, as expected since nothing was culled before. In a dumped Kokiri Forest frame there are 491 samples-passed reports and no conditional render at all; with every report forced to 0 the game makes ~565 fewer draws a frame and none are "skipped" by a condition. So the counts only have to reach guest memory within a frame or two; no `VK_EXT_conditional_rendering` is needed. Checks: `python -I -B tests/run_all.py` on Windows: the failure left is `test_bake_portability` (no `zstandard` in that Python), the same without this change. ## Testing **Tested:** the runs under Measurements (on the GitHub version) and the game-free checks this description names. Checked but not measured: screenshots at the same present, compared with the default run (below). **Not tested:** a moving camera and indoor scenes (pop-in when something comes out from behind cover; bigger savings where more is hidden); the sun with real counts (its haze should now dim when the sun is hidden); AMD (Tier 1, half the reports), the Steam Deck, integrated GPUs and Linux. Testing wanted there before this is on by default. **Needs other testers:** - AMD (Windows or Linux): Kokiri Forest and Castle Town Market with `RX_VK_OCC_QUERIES=1`, expecting no objects missing and no flicker against the switch unset (request to open) - Steam Deck: the same scenes with `RX_VK_OCC_QUERIES=1`, expecting no hang and no objects missing (request to open) ## Screenshots Nothing on screen: no visual change is expected. Screenshots at the same present matched the default run apart from the falling leaves and fairies; none are attached (the pair can be added). ## Risk and rollback Off by default: unset, the replay takes no queries and the reports read 4096 as before. Switched on, an object could be culled that should be drawn (a count written to the wrong slot, or pop-in as something comes out from behind cover), and the queries add work to the replay thread and the GPU on hardware not tried yet; a count that is late or unknown stays "visible". Unsetting `RX_VK_OCC_QUERIES` turns it off; reverting the commit is enough. ## Reviewer notes Carried over from [NotRexed/Hookshot#2871](https://github.com/NotRexed/Hookshot/pull/2871) with the move to Longshot: the same change on Longshot's `main`, with its rename applied. Its review and discussion are there. **A draft:** the occlusion queries run in the game; it stays a draft as on GitHub. How the game culls: the z-prepass draws each object between a samples-passed reset and a report into the object's slot. The game reads those slots back on the CPU a frame or two later and leaves the objects whose count was 0 out of its draw lists. The checks and the in-game runs above were done on the GitHub version; this branch is the same change with the rename applied (the same files and line counts). Forgejo runs no checks, and they were not run again here. ## Issues See [NotRexed/Hookshot#487](https://github.com/NotRexed/Hookshot/pull/487) (where the reports started reading 4096). ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [ ] 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). - [ ] 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] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [ ] Flags table filled in; README / docs/development.md switch reference updated. - [ ] Screenshots for a change that shows on screen. - [ ] A row in `docs/tested-configurations.md` for each measured run. - [ ] `MKWVK_API_VERSION` bumped (`gpu/vk/vk_api.h`'s `MkwVkHost` or `MkwVkRenderer` changed). - [ ] The SPIR-V headers regenerated with glslang (a shader changed). - [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. - [ ] Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. - [ ] Docs updated. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Ocarina of Time reads its z-prepass occlusion reports back on the CPU a frame
or two later and leaves out of its draw lists the objects whose count was 0.
The Vulkan renderer writes every samples-passed report as 4096 (PR #487), so
the game never culled anything.

With RX_VK_OCC_QUERIES=1 each draw from a samples-passed reset to its report
gets a Vulkan occlusion query, numbered in the replay state so a parallel
replay's dry pass and its workers agree. The report is still written as 4096
at replay; once its queries are done the replay thread writes their sum over
it, unless the slot was reported into again since. A report whose draw did not
run, or that took more than 256 draws, stays 4096. Off by default.

Kokiri Forest, standing still, RTX 3080: ~1,570 -> ~1,330 draws a frame,
~11.5 -> ~11.0 ms of GPU time, 26-28 -> 27.5-29 fps, the same picture; ~32% of
reports read 0, each written about one recording after the report.

tests/test_vk_occlusion_queries.py runs the ring's code against a fake query
pool and checks the wiring in the source; tests/fsr_uniform_ownership_test.cpp
stubs the harvest begin_recording now calls.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Author
First-time contributor

Back to WIP under #48's process: this was carried over before the new template. Its description will be brought to the template and measured on Longshot (main against this PR, at named commits) when it's wanted for review; ask here if you'd like it sooner.

Back to WIP under #48's process: this was carried over before the new template. Its description will be brought to the template and measured on Longshot (main against this PR, at named commits) when it's wanted for review; ask here if you'd like it sooner.
Author
First-time contributor

Description brought to Longshot's PR template (#48). Still WIP: its numbers are from the GitHub version, and it isn't rebased on current main; it gets measured on Longshot (main against this PR, at named commits) before it is marked ready.

Description brought to Longshot's PR template (#48). Still WIP: its numbers are from the GitHub version, and it isn't rebased on current main; it gets measured on Longshot (main against this PR, at named commits) before it is marked ready.
Author
First-time contributor

Description moved to the template's Testing section and whole checklist (#72): what was tested, what wasn't, and what needs other testers (their test requests to open while this is WIP).

Description moved to the template's Testing section and whole checklist (#72): what was tested, what wasn't, and what needs other testers (their test requests to open while this is WIP).
This pull request is marked as a work in progress.
This branch is out-of-date with the base branch
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u 'fix/vk-occlusion-queries:dyst-fix/vk-occlusion-queries'
git switch 'dyst-fix/vk-occlusion-queries'
Sign in to join this conversation.
No description provided.