WIP: fix(vk): give samples-passed reports the GPU's counts (RX_VK_OCC_QUERIES) #23
No reviewers
Labels
No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
needs measurements
question
testing wanted
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!23
Loading…
Reference in a new issue
No description provided.
Delete branch "dyst/Longshot:fix/vk-occlusion-queries"
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
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
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 needshostQueryReset(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 inNvnStatein every mode, so a parallel replay's dry pass and its workers agree on them;NOP_DRAW,NOP_DRAW_IDX,NOP_DRAW_INDIRECTand the mesh draw take one;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;kOccRun), stays 4096, so a late or unknown count costs culling, never a missing object;replay_handlesets the command buffer's query limit (occ_room(),kOccBuffer) before its dry pass's start state is copied;RX_VK_OCC_LOG=1logs, every ~5,000 reports, how many were written, how many read 0 and how many recordings late they were.draw()ingpu/vk/vk_draw.cppbegins the query once its pass is open and ends it as it returns (OccGuard), beforeeach_guardcan end the pass;DrawArgs(gpu/vk/vk_draw.h) carriesocc_poolandocc_query.create_device()ingpu/vk/vk_main.cpprecordsocclusionQueryPreciseandhostQueryReset(Device::occlusion_precise,host_query_resetingpu/vk/vk_internal.h); queries are precise where the device has it.NvnState(gpu/vk/vk_state.h) gainsocc_next,occ_first,occ_limit,occ_on,occ_overflow.tests/test_vk_occlusion_queries.pyruns the ring's code (cut fromvk_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.cppstubsocc_collect/occ_value, whichbegin_recordingnow uses.Flags and settings
RX_VK_OCC_QUERIESRX_VK_OCC_LOGMeasurements
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)
RX_VK_OCC_FIXED=0RX_VK_OCC_QUERIES=1About 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=1gives 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_renderingis needed.Checks:
python -I -B tests/run_all.pyon Windows: the failure left istest_bake_portability(nozstandardin 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:
RX_VK_OCC_QUERIES=1, expecting no objects missing and no flicker against the switch unset (request to open)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_QUERIESturns 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
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.docs/tested-configurations.mdfor each measured run.MKWVK_API_VERSIONbumped (gpu/vk/vk_api.h'sMkwVkHostorMkwVkRendererchanged).mainand passing here.tests/that reads changed source updated in this PR, not weakened.🤖 Generated with Claude Code
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.
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 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).
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.