MagicBeanDev/Longshot

Watch

Fork 98

WIP: perf(vk): a direct indexed draw keeps its pool's index buffer binding, its offset as the first index #798

Draft

dyst wants to merge 1 commit from dyst/Longshot:perf/vk-index-buffer-bind into main

dyst commented

2026-11-15 12:28:21 +00:00

Member

Type: perf · Risk: low · Tested on: Windows 11 (the game-free checks; the game A/B is next) · Needs: a game run on NVIDIA, AMD and Intel

Why

Each submesh has its own index offset, and the replay bound the index buffer at it: nearly every indexed draw recorded a vkCmdBindIndexBuffer. Found in an audit of the renderer's per-draw path; no issue on the tracker.

What changed

A direct indexed draw binds its pool's buffer at the start and draws from first = offset / index size, so the submeshes of one pool keep one binding. The same indices are read, and robust access's bounds see the same reads in and out of bounds. Indirect draws keep their binding at NVN's address.

draw: index_bind_base and the direct indexed draw in draw() (gpu/vk/vk_draw.cpp); RX_VK_PROF_FINE=1 counts the bind commands recorded (index buffer bind commands)
tests: tests/test_draw_binds.py checks the read addresses and bounds against the old scheme

Measurements

Verdict: not measured yet: A/B runs in the game (this PR against main on one base, interleaved) are next and will replace this section.

Flags and settings

One switch for an A/B.

Flag	What it is	Before this PR	After this PR	Effect	Helps / doesn't help / untested	Recommendation
`RX_VK_INDEX_AT_OFFSET=1`	Each direct draw bound at its own offset, as before	-	unset	On: the old binds	measurement pending	leave unset; `=1` for an A/B

Testing

Tested: Windows 11, llvm-mingw (the builder's v2.0.14 tools): the new and changed checks; every check that reads the
changed code; -fsyntax-only of the changed renderer files with the build's flags and headers; python -I -B tests/run_all.py on this branch and on main @ aa95d35f, same PC and environment: the same results apart from the extended test_draw_binds; the failures main has here are named in the first comment.

Not tested: the game yet. This PR is WIP until its A/B is in (next: this PR against main on one dev_packs base of
main @ aa95d35f, on an RTX 3080 with a Ryzen 9 3900X, Windows 11; AMD and Intel need other testers).

Must be tested:

NVIDIA, AMD and Intel (large first index values are new): Kokiri Forest and Castle Town a few minutes, expecting the same picture and, with RX_VK_PROF_FINE=1, far fewer index buffer bind commands a draw than with RX_VK_INDEX_AT_OFFSET=1 (Test Farm: @Beanstalk amd, intel: those scenes, look for missing or spiky geometry)

Risks

What could break: A driver that mishandles large firstIndex values (up to the pool offset over the index size, ~2^27): missing or spiky geometry on that driver.

Turning it off: RX_VK_INDEX_AT_OFFSET=1

Compatibility: Nothing changes: no caches, settings or vk ABI involved.

Checklist

Everyone

One topic.
The title is a Conventional Commit (type(scope): description) of the status line's type.
Why gives the reason plainly and links the issues that fit (the tracker searched).
Measurements has a verdict, or one line saying why there is nothing to measure.
Testing names what was tested, what wasn't, and what must still be tested, where and how.
Rebased on current main.
python -I -B tests/run_all.py passes (any failure that also fails on main is named).
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.

When it applies

AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
Switches documented (docs/development.md; the README for a menu row) and shipped at their recommended value.
A game-free test added for the fix, failing on main and passing here.

🤖 Generated with Claude Code

**Type:** perf · **Risk:** low · **Tested on:** Windows 11 (the game-free checks; the game A/B is next) · **Needs:** a game run on NVIDIA, AMD and Intel ## Why Each submesh has its own index offset, and the replay bound the index buffer at it: nearly every indexed draw recorded a `vkCmdBindIndexBuffer`. Found in an audit of the renderer's per-draw path; no issue on the tracker. ## What changed A direct indexed draw binds its pool's buffer at the start and draws from `first = offset / index size`, so the submeshes of one pool keep one binding. The same indices are read, and robust access's bounds see the same reads in and out of bounds. Indirect draws keep their binding at NVN's address. - draw: `index_bind_base` and the direct indexed draw in `draw()` (`gpu/vk/vk_draw.cpp`); `RX_VK_PROF_FINE=1` counts the bind commands recorded (`index buffer bind commands`) - tests: `tests/test_draw_binds.py` checks the read addresses and bounds against the old scheme ## Measurements **Verdict:** not measured yet: A/B runs in the game (this PR against main on one base, interleaved) are next and will replace this section. ### Flags and settings One switch for an A/B. | Flag | What it is | Before this PR | After this PR | Effect | Helps / doesn't help / untested | Recommendation | |---|---|---|---|---|---|---| | `RX_VK_INDEX_AT_OFFSET=1` | Each direct draw bound at its own offset, as before | - | unset | On: the old binds | measurement pending | leave unset; `=1` for an A/B | ## Testing **Tested:** Windows 11, llvm-mingw (the builder's v2.0.14 tools): the new and changed checks; every check that reads the changed code; `-fsyntax-only` of the changed renderer files with the build's flags and headers; python -I -B tests/run_all.py on this branch and on main @ aa95d35f, same PC and environment: the same results apart from the extended `test_draw_binds`; the failures main has here are named in the first comment. **Not tested:** the game yet. This PR is WIP until its A/B is in (next: this PR against main on one dev_packs base of main @ aa95d35f, on an RTX 3080 with a Ryzen 9 3900X, Windows 11; AMD and Intel need other testers). **Must be tested:** - NVIDIA, AMD and Intel (large first index values are new): Kokiri Forest and Castle Town a few minutes, expecting the same picture and, with `RX_VK_PROF_FINE=1`, far fewer `index buffer bind commands` a draw than with `RX_VK_INDEX_AT_OFFSET=1` (Test Farm: @Beanstalk amd, intel: those scenes, look for missing or spiky geometry) ## Risks **What could break:** A driver that mishandles large `firstIndex` values (up to the pool offset over the index size, ~2^27): missing or spiky geometry on that driver. **Turning it off:** `RX_VK_INDEX_AT_OFFSET=1` **Compatibility:** Nothing changes: no caches, settings or vk ABI involved. ## Checklist ### Everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`) of the status line's type. - [x] Why gives the reason plainly and links the issues that fit (the tracker searched). - [x] Measurements has a verdict, or one line saying why there is nothing to measure. - [x] Testing names what was tested, what wasn't, and what must still be tested, where and how. - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` passes (any failure that also fails on `main` is named). - [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. ### When it applies - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] Switches documented (docs/development.md; the README for a menu row) and shipped at their recommended value. - [x] A game-free test added for the fix, failing on `main` and passing here. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

dyst added 1 commit

2026-11-15 12:28:21 +00:00

perf(vk): a direct indexed draw keeps its pool's index buffer binding, its offset as the first index

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

Details

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

Details

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

Details

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

Details

PR description / Description follows the template (pull_request_target) Successful in 3s

Details

CI / Conventional Commit title (pull_request) Successful in 4s

Details

CI / Workflow lint (actionlint) (pull_request) Successful in 7s

Details

Guide screenshots / Guide images and local UI source review (pull_request) Failing after 6s

Details

Third-party parts / As their notes say (pull_request) Successful in 8s

Details

Website / Website on Cloudflare (pull_request_target) Has been skipped

Details

bc89e90194

Each submesh has an index offset of its own, and the draw bound the index buffer at it: nearly every indexed draw recorded a
vkCmdBindIndexBuffer. A direct indexed draw now binds the buffer at its start and draws from first = offset / index size
(index_bind_base), so the submeshes of one pool keep one binding. The same indices are read (start + first * size is the
offset); robust access's bounds run from the bound offset to the buffer's end and no read lies below the draw's offset, so
nothing read in or out of bounds changes. An offset that is no multiple of the index size, or a first index and count past
32 bits, binds at the offset as before. Indirect indexed draws keep their binding at NVN's address: the GPU-written first
indices count from there. RX_VK_INDEX_AT_OFFSET=1: every direct draw bound at its offset, as before.

RX_VK_PROF_FINE=1 counts the bind commands recorded ("index buffer bind commands"), not only the bind calls.
tests/test_draw_binds.py checks index_bind_base's results and fallbacks against the old scheme's read addresses and bounds.

Assisted-by: Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

dyst commented

2026-11-15 12:28:22 +00:00

Author

Member

The checks on this PC (Windows 11, RTX 3080, Ryzen 9 3900X, llvm-mingw from the builder's v2.0.14 tools): python -I -B tests/run_all.py on main @ aa95d35f fails these 19 here, and so does this branch: test_appimage_player_build.py, test_bake_portability.py, test_cpm_discovery.py, test_cpp_preflight.py, test_crash_dialog.py, test_env_on.py, test_fpx_sticky.py, test_lift_cleanup_error.py, test_linux_preflight.py, test_make_game_cache.py, test_make_game_prepare.py, test_missing_nup.py, test_oot_state_tap.py, test_options_budget_once.py, test_readback_late.py, test_spawn_emit.py, test_tested_configurations.py, test_vk_pgo.py, test_walk_key.py. Any other failure in a full run on this branch was rerun on its own and passed (the full runs shared the PC with dev_packs builds: timing checks).

Next: an interleaved A/B in the game against main on one dev_packs base of aa95d35f; the numbers will replace the Measurements section.

The checks on this PC (Windows 11, RTX 3080, Ryzen 9 3900X, llvm-mingw from the builder's v2.0.14 tools): `python -I -B tests/run_all.py` on main @ aa95d35f fails these 19 here, and so does this branch: `test_appimage_player_build.py`, `test_bake_portability.py`, `test_cpm_discovery.py`, `test_cpp_preflight.py`, `test_crash_dialog.py`, `test_env_on.py`, `test_fpx_sticky.py`, `test_lift_cleanup_error.py`, `test_linux_preflight.py`, `test_make_game_cache.py`, `test_make_game_prepare.py`, `test_missing_nup.py`, `test_oot_state_tap.py`, `test_options_budget_once.py`, `test_readback_late.py`, `test_spawn_emit.py`, `test_tested_configurations.py`, `test_vk_pgo.py`, `test_walk_key.py`. Any other failure in a full run on this branch was rerun on its own and passed (the full runs shared the PC with dev_packs builds: timing checks). Next: an interleaved A/B in the game against main on one dev_packs base of aa95d35f; the numbers will replace the Measurements section.