fix(vk): an indexed indirect draw binds the indices it can read, a whole vertex stream its pool #434

Open

mooki wants to merge 1 commit from mooki/Longshot:fix/indexed-indirect-extent into main

pull from: mooki/Longshot:fix/indexed-indirect-extent

merge into: MagicBeanDev:main

MagicBeanDev:main

Conversation 0 Commits 1 Files changed 4 +411 -4

mooki commented 2026-11-14 13:49:37 +00:00

Member

• Copy link
• Copy Markdown

Type: fix · Risk: medium · Tested on: macOS 26.7.1, Apple M4 Max (game-free checks, arm64 and x86_64 object compiles; no game or GPU run) · Needs: Windows or Linux, a discrete AMD or NVIDIA GPU

Why

An indexed indirect draw reads its first index and its count from records on the GPU, but the renderer bound only 4 bytes
of its index buffer. A vertex stream bound whole (size 0, VK_WHOLE_SIZE) was bound by 1 byte. The import holding those
bytes ends where it happens to, a 4 MB chunk on with VRAM mirrors and 16 MB on without. Indices or vertices past that end
read 0 where robust buffer access is on and undefined bytes where it is off: MoltenVK, the 'Robust buffers' Off row, and
drivers without robustBufferAccess2. Found by reading the code; nobody has reported it from the game.

How to reproduce

Seen on: found by reading the code; nobody has reported it from the game. The game-free test shows it on macOS
26.7.1, Apple M4 Max, against main 6c9edffe's vk_draw.cpp. The binding is the same code on every OS; undefined bytes
(rather than 0) are read where robust buffer access is off.
Settings and switches: Robust buffers (Debug tab, Developer) Off, MoltenVK, or a driver without robustBufferAccess2;
with VRAM mirrors on, the import ends 4 MB on, 16 MB without.
Steps: game-free, from this branch's checkout: python -I -B tests/test_draw_extent.py --cxx clang++ --headers <Vulkan include> --source <a tree of main>. In the game: an indexed indirect draw whose indices, or a stream bound whole
whose vertices, run past the import holding their first bytes (whether the game has one is unknown).
Expected: the bound buffer holds every byte the draw may read.
Actual: on main the bound index buffer holds 64 of the 122,683,456 bytes the draw may read, and a stream bound whole
16 of 118,489,104; past that, indices or vertices read 0 or undefined bytes. The test fails 6 checks on main.

What changed

An indexed indirect draw now binds every index it may read, and a vertex stream bound whole binds up to its game pool's
end, where one import can hold that; otherwise the binding is main's, with a log line. Bindings past 1 GB or whose import
fails stay as before.

• Read first: index_extent, vertex_whole_extent, kImportMax and extent_bind (vk_draw.cpp, beside
  vertex_bind_reuse), then their two call sites in draw().
• vk: gpu/vk/vk_draw.cpp, three helpers beside vertex_bind_reuse, plus a constant.
  • index_extent(d): the bytes an indexed indirect draw may read of its index buffer. Any index in
    [ibuf, ibuf + 2^33 units) may be read (a 32-bit first index plus a 32-bit count), up to the end of the game pool
    holding ibuf (gpu_pool_end); 4 where no pool holds it.
  • vertex_whole_extent(addr): the bytes a stream bound whole may read, up to its game pool's end; 1 where no pool
    holds it.
  • kImportMax (1 GB): the most one import holds. pool_lookup merges no more into one, mirrors are refused above it,
    and the import table's scans assume none is larger.
  • extent_bind(addr, extent, token, what) binds the whole extent where one import may hold it and does. Otherwise it
    binds the first token bytes, as before, and logs a line (at most 16 lines):
    vk: index buffer 0x2000000100: no single import takes the 8589934592 bytes a draw may read there; bound by its first 4 as before.
    That happens when the extent is over 1 GB or its import fails (the device's allocation or buffer limit).
• vk: draw() binds through them.
  • The indexed indirect branch binds index_extent(d) instead of 4 bytes.
  • The vertex-stream loop binds vertex_whole_extent for a stream bound whole. A stream with a size binds that size, as
    before. The Vulkan binding size is unchanged (VK_WHOLE_SIZE or the stream's size).
  • A kept stream (vertex_bind_reuse) is still taken without a lookup. Its pool's end is asked only when the stream is
    looked up again.
• tests: tests/test_draw_extent.py (new).
  • It runs draw()'s own vertex-stream loop and indirect branch, with the structs and helpers they use, cut from the
    source.
  • The stand-ins: game pools, imports made by 4 MB chunk around a bound range in one pool (pool_lookup with mirrors on),
    a device limit on an import's size, and recorded Vulkan binds.
  • For each binding it compares the bytes the bound buffer holds from the binding's offset with what the draw may read,
    which the test works out on its own.
  • Cases: u16 indices 64 bytes before a chunk's end in a 120 MB pool; u8 and u32 indices in a 10 GB pool (over 1 GB:
    bound as before, logged); an import refused by a 64 MB device limit (as before, logged); an address above 1 TB with
    no pool end (4 bytes, as before); a non-indexed indirect draw; a stream with a size, a stream bound whole 16 bytes
    before a chunk's end, a stream bound whole above 1 TB, and a stream bound whole in the 10 GB pool. A kept stream is
    not looked up again, and no binding asks for an import over 1 GB.
  • It fails on main.
• tests: tests/test_vertex_bind.py and tests/test_fallback_pipeline_state.py follow the stream loop's binding into
  extent_bind.
  • test_vertex_bind.py still checks that the kept stream is asked before any lookup, now extent_bind and
    vertex_whole_extent.
  • test_fallback_pipeline_state.py pastes the loop, so it now also pastes vertex_whole_extent, kImportMax and
    extent_bind when the source has them, with a pool end of 0. Its checks are unchanged and still pass on main's
    source.
• Design: index_extent is the formula the macOS port's admission demands for these draws (SegBind), with the same name
  and body. There, segment gathers take an extent across a segment's cut. Here an extent no single import holds keeps
  main's binding; the pieces for that are the read-side fallback work, a separate change.
• Design: the 2^33-unit bound never decides a binding here: it is at least 8 GB, past kImportMax. It sets the number in
  the log line and keeps the formula the port's.
• Unchanged: the direct indexed draw (d.count indices) and the quad path (indices read on the CPU): they already bind or
  read what they draw.
• Left out: fetching vertices past a stream's own size (a stream with a size) is a separate gap: what the GPU returns
  without robust access is unknown, and it is left for a measurement first.

Measurements

Verdict: fixed in the game-free test: it fails on main (6 checks; the index buffer holds 64 of 122,683,456 bytes,
the whole stream 16 of 118,489,104) and passes here. The full game-free suite changes only by the new test. The in-game
cost of the larger bindings (fps, VRAM, hitches) is not measured yet: it is the test request (Testing).

Commits: main 6c9edffe5d (2026-11-14); this PR fix/indexed-indirect-extent @
5bde466843.
Hardware: Apple M4 Max (16 CPUs), macOS 26.7.1, Apple clang 21. Game-free: no GPU or game involved
Scene: none. This PR's proof is the test, as CONTRIBUTING's fix row allows. The in-game cost is a test request
(Testing).
Method: tests/test_draw_extent.py against main's gpu/vk/vk_draw.cpp, vk_draw.h and vk_internal.h (6c9edffe)
and against this PR; the full game-free suite on both
How to reproduce: python -I -B tests/test_draw_extent.py --cxx clang++ --headers <Vulkan include> (add
--source <a tree of main> for main's side)

	Before: main @ 6c9edffe	After: this PR @ 5bde4668
tests/test_draw_extent.py	FAIL: 6 checks; the index buffer holds 64 of 122,683,456 bytes, the whole stream 16 of 118,489,104	PASS
full game-free suite on macOS (tests/run_all.py --jobs 6)	531 passed, 32 failed, 17 skipped	532 passed, 32 failed, 17 skipped

main @ 6c9edffe:
FAIL: an indexed indirect draw's indices past the import holding its first bytes: the bound index buffer holds 64 of the 122683456 bytes the draw may read
FAIL: an index extent of 2^33 bytes (u8) is bound by 4 bytes as before, with one line: 1 draws, 0 lines logged, no import over 1 GB asked for
FAIL: a u32 extent ends at the pool's end, 10 GB on: bound by 4 bytes as before, with one line: 0 lines logged
FAIL: an extent whose import fails is bound by 4 bytes as before, with one line: 1 draws, 0 skipped, 0 lines logged
FAIL: a vertex stream bound whole, its vertices past the import holding its first byte: the bound buffer holds 16 of the 118489104 bytes the stream may read
FAIL: a stream bound whole past 1 GB of its pool is bound by 1 byte as before, with one line: 0 lines logged, no import over 1 GB asked for
FAIL: 6 of the checks above

this PR @ 5bde4668:
PASS: an indexed indirect draw binds the indices it may read and a stream bound whole its pool, both in one import; past 1 GB or a failed import as before, with a line:
  vk: index buffer 0x2000000100: no single import takes the 8589934592 bytes a draw may read there; bound by its first 4 as before

On main, four of the six failures are only the missing log line: there the binding is the one main makes. The other two
are the bug: the bound buffer ends before what the draw may read.

Screenshots: none; nothing on screen in the game-free tests.

Flags and settings

None.

Testing

Tested:

• On an Apple M4 Max with macOS 26.7.1, game-free only.
• tests/test_draw_extent.py fails on main and passes here.
• tests/test_vertex_bind.py, tests/test_draw_binds.py and tests/test_fallback_pipeline_state.py pass.
  test_fallback_pipeline_state.py also passes against main's vk_draw.cpp (--source, 6c9edffe).
• The full suite on both sides, compared by test name.
  • By test name, against main 6c9edffe's run on the same Mac: only one status changed: test_draw_extent, absent on
    main, passes here.
  • These 32 checks fail here and on main on this Mac, by name (not triaged here: some need the macOS port's other pull
    requests, some are checks main added or changed since 5e072fc8): test_appimage_package_retry,
    test_appimage_player_build, test_collect_wide, test_control, test_cpm_discovery, test_crash_dialog,
    test_crash_record, test_env_on, test_game_layout, test_lift_cleanup_error, test_linux_preflight,
    test_longshot_ctl, test_low_memory_notice, test_make_game_cache, test_mem_gate, test_mem_gate_children,
    test_mirror_trim, test_oot_state_tap, test_pcache_header_read, test_pipeline_cache_file,
    test_pipeline_cache_save, test_pipeline_play_cache, test_pipeline_tour, test_prof_symbols,
    test_program_cache_cleanup, test_program_cache_one_variant, test_readback_late, test_report_run,
    test_split_giants, test_vk_pgo, test_vram_budget_row, test_walk_key.
  • The same 17 checks skipped as on main.
  • Crash reports during the run: none (main's run: fault-arm64-2026-11-14-180704.ips,
    recorder_sync_test.exe-2026-11-14-180744.ips).
• Object compiles of gpu/vk/vk_draw.cpp with the renderer's flags (nx2/build.py), Apple clang 21, arm64 and
  x86_64, on main and here.
  • Main's renderer does not build on macOS as it is: vk_draw.cpp stops at posix_fallocate
    (pipeline_cache_file.h).
  • With a compile-only prelude declaring environ and posix_fallocate, all 4 objects build, 2 on main and 2 here.
• After the rebase onto main aa95d35f, at 4b31c5bd (5bde4668 before the rebases onto e090cc48 and aa95d35f):
  Sweep3's checks of this head (the C/C++ it changes compiled, and its own tests run, on arm64 and x86_64 where they
  apply) gave the same results as before; python -I -B tests/run_all.py on wave S1's merge 11a5b4e7, which carries
  this head: 568/22/17 (passed/failed/skipped) against main aa95d35f's 543/33/17 on this Mac, no new failure
  (reports/2026-11-15-rebase-sweep-3.md).

Not tested:

• No game run and no GPU run, on any platform.
• A build on Windows (llvm-mingw) or Linux. The change uses only gpu_pool_end, gpu_buffer_bind, logf and
  std::atomic, as the code around it does.
• Larger bindings mean larger imports. An index buffer or a stream bound whole now takes an import up to the end of its
  game pool, where before it took a chunk around its first bytes. On a discrete GPU each import gets a VRAM mirror of
  its whole range (mirror_on), within the mirrors' budget. A pool whose merged import passes 1 GB gets no mirror, and
  the GPU reads it over PCIe. The first binding of a larger range merges the chunks already imported into one new
  import on the replay thread. Main's comments in vk_memory.cpp put an import at 5-45 ms, and one that took two
  chunks of a pool as one at 130-147 ms, the one hitch of a panning run in Kokiri. How often that happens with this PR,
  and what it costs in fps, VRAM and hitches, is unmeasured.
• Whether the game has indexed indirect draws or streams bound whole whose reads pass the old import's end, and so
  whether anything changes on screen, is unknown.
• No timing: the measurement in the game (main and this PR at named commits, on the same PC, scene and settings, at
  least 2 interleaved runs each, the median) is the test request below.

Must be tested:

• Windows or Linux, a discrete AMD or NVIDIA GPU (VRAM mirrors on), any driver: build this PR @ 4b31c5bd and main
  aa95d35f the same way; in Kokiri Forest and on a Hyrule Field walk, the same settings, 2 interleaved runs of 120 s each
  after a warm-up, the FPS counter at Breakdown, report fps (average and 1% low), median and p99 frame time, and the GPU's
  longest frame, and from longshot_log.txt the import lines (vk: pool … imported in … ms as … (N imports, M ms in all)), the last vk: N pools mirrored in VRAM (M MB), any … stays in system memory: the mirrors have … MB of … and
  would take in … pools: left overlapping, and this PR's no single import takes … lines (count them: each is a
  binding left as main makes it), expecting fps and frame times within run-to-run noise and the import count and mirror
  total within a few percent of main's; then the same scenes once each on main and this PR with Robust buffers (Debug
  tab, Developer) Off, looking for stretched or missing geometry on main that this PR fixes, and for none new, expecting
  the same picture or a fix (by hand: test request T-CL66).

Risks

What could break:

• Medium.
  • The bindings can be much larger: up to a game pool's remaining bytes, at most 1 GB. The imports and mirrors grow
    with them.
  • A range whose merge with neighbouring imports passes 1 GB takes main's existing escape (would take in … pools: left overlapping), as any binding near 1 GB does today.
  • Past 1 GB, and where an import fails, the binding is main's and one of at most 16 lines says so.
• Landing order: no conflict with main or with the other macOS-port PRs rebased on 6c9edffe (git merge-tree of the
  new heads, 19:49 CST).

Turning it off: no switch. Reverting the commit is enough. RX_VK_POOL_END_IMPORTED=1 (an existing A/B switch) makes
gpu_pool_end answer the import's end, which narrows these extents to the import already there. It also changes the
storage-buffer bindings that ask the pool's end, so it is not an off switch for this PR.

Compatibility: Nothing changes: vk_draw.cpp is not in the translator stamp (nx2/translator_files.py), and saves,
the options file, the shader and pipeline caches and the vk ABI are unchanged.

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: on aa95d35f (#761, 2026-11-15), head 4b31c5bd, without a conflict; this PR's own
  commits, old → new: 5bde4668 → 4b31c5bd; the commit names elsewhere in this description are from before the
  rebase.
• python -I -B tests/run_all.py passes (any failure that also fails on main is named). On macOS: 532 passed, 17
  skipped, and 32 failures that also fail on main 6c9edffe on this Mac, named under Testing. After the rebase onto main
  aa95d35f (head 4b31c5bd): wave S1's merge 11a5b4e7, 568/22/17 against main aa95d35f's 543/33/17, no new failure.
• 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.
• 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.

**Type:** fix · **Risk:** medium · **Tested on:** macOS 26.7.1, Apple M4 Max (game-free checks, arm64 and x86_64 object compiles; no game or GPU run) · **Needs:** Windows or Linux, a discrete AMD or NVIDIA GPU ## Why An indexed indirect draw reads its first index and its count from records on the GPU, but the renderer bound only 4 bytes of its index buffer. A vertex stream bound whole (size 0, `VK_WHOLE_SIZE`) was bound by 1 byte. The import holding those bytes ends where it happens to, a 4 MB chunk on with VRAM mirrors and 16 MB on without. Indices or vertices past that end read 0 where robust buffer access is on and undefined bytes where it is off: MoltenVK, the 'Robust buffers' Off row, and drivers without `robustBufferAccess2`. Found by reading the code; nobody has reported it from the game. ## How to reproduce **Seen on:** found by reading the code; nobody has reported it from the game. The game-free test shows it on macOS 26.7.1, Apple M4 Max, against main 6c9edffe's `vk_draw.cpp`. The binding is the same code on every OS; undefined bytes (rather than 0) are read where robust buffer access is off. **Settings and switches:** Robust buffers (Debug tab, Developer) Off, MoltenVK, or a driver without `robustBufferAccess2`; with VRAM mirrors on, the import ends 4 MB on, 16 MB without. **Steps:** game-free, from this branch's checkout: `python -I -B tests/test_draw_extent.py --cxx clang++ --headers <Vulkan include> --source <a tree of main>`. In the game: an indexed indirect draw whose indices, or a stream bound whole whose vertices, run past the import holding their first bytes (whether the game has one is unknown). **Expected:** the bound buffer holds every byte the draw may read. **Actual:** on main the bound index buffer holds 64 of the 122,683,456 bytes the draw may read, and a stream bound whole 16 of 118,489,104; past that, indices or vertices read 0 or undefined bytes. The test fails 6 checks on main. ## What changed An indexed indirect draw now binds every index it may read, and a vertex stream bound whole binds up to its game pool's end, where one import can hold that; otherwise the binding is main's, with a log line. Bindings past 1 GB or whose import fails stay as before. - Read first: `index_extent`, `vertex_whole_extent`, `kImportMax` and `extent_bind` (`vk_draw.cpp`, beside `vertex_bind_reuse`), then their two call sites in `draw()`. - vk: `gpu/vk/vk_draw.cpp`, three helpers beside `vertex_bind_reuse`, plus a constant. - `index_extent(d)`: the bytes an indexed indirect draw may read of its index buffer. Any index in [ibuf, ibuf + 2^33 units) may be read (a 32-bit first index plus a 32-bit count), up to the end of the game pool holding ibuf (`gpu_pool_end`); 4 where no pool holds it. - `vertex_whole_extent(addr)`: the bytes a stream bound whole may read, up to its game pool's end; 1 where no pool holds it. - `kImportMax` (1 GB): the most one import holds. `pool_lookup` merges no more into one, mirrors are refused above it, and the import table's scans assume none is larger. - `extent_bind(addr, extent, token, what)` binds the whole extent where one import may hold it and does. Otherwise it binds the first `token` bytes, as before, and logs a line (at most 16 lines): `vk: index buffer 0x2000000100: no single import takes the 8589934592 bytes a draw may read there; bound by its first 4 as before`. That happens when the extent is over 1 GB or its import fails (the device's allocation or buffer limit). - vk: `draw()` binds through them. - The indexed indirect branch binds `index_extent(d)` instead of 4 bytes. - The vertex-stream loop binds `vertex_whole_extent` for a stream bound whole. A stream with a size binds that size, as before. The Vulkan binding size is unchanged (`VK_WHOLE_SIZE` or the stream's size). - A kept stream (`vertex_bind_reuse`) is still taken without a lookup. Its pool's end is asked only when the stream is looked up again. - tests: `tests/test_draw_extent.py` (new). - It runs `draw()`'s own vertex-stream loop and indirect branch, with the structs and helpers they use, cut from the source. - The stand-ins: game pools, imports made by 4 MB chunk around a bound range in one pool (`pool_lookup` with mirrors on), a device limit on an import's size, and recorded Vulkan binds. - For each binding it compares the bytes the bound buffer holds from the binding's offset with what the draw may read, which the test works out on its own. - Cases: u16 indices 64 bytes before a chunk's end in a 120 MB pool; u8 and u32 indices in a 10 GB pool (over 1 GB: bound as before, logged); an import refused by a 64 MB device limit (as before, logged); an address above 1 TB with no pool end (4 bytes, as before); a non-indexed indirect draw; a stream with a size, a stream bound whole 16 bytes before a chunk's end, a stream bound whole above 1 TB, and a stream bound whole in the 10 GB pool. A kept stream is not looked up again, and no binding asks for an import over 1 GB. - It fails on main. - tests: `tests/test_vertex_bind.py` and `tests/test_fallback_pipeline_state.py` follow the stream loop's binding into `extent_bind`. - `test_vertex_bind.py` still checks that the kept stream is asked before any lookup, now `extent_bind` and `vertex_whole_extent`. - `test_fallback_pipeline_state.py` pastes the loop, so it now also pastes `vertex_whole_extent`, `kImportMax` and `extent_bind` when the source has them, with a pool end of 0. Its checks are unchanged and still pass on main's source. - Design: `index_extent` is the formula the macOS port's admission demands for these draws (SegBind), with the same name and body. There, segment gathers take an extent across a segment's cut. Here an extent no single import holds keeps main's binding; the pieces for that are the read-side fallback work, a separate change. - Design: the 2^33-unit bound never decides a binding here: it is at least 8 GB, past `kImportMax`. It sets the number in the log line and keeps the formula the port's. - Unchanged: the direct indexed draw (`d.count` indices) and the quad path (indices read on the CPU): they already bind or read what they draw. - Left out: fetching vertices past a stream's own size (a stream with a size) is a separate gap: what the GPU returns without robust access is unknown, and it is left for a measurement first. ## Measurements **Verdict:** fixed in the game-free test: it fails on main (6 checks; the index buffer holds 64 of 122,683,456 bytes, the whole stream 16 of 118,489,104) and passes here. The full game-free suite changes only by the new test. The in-game cost of the larger bindings (fps, VRAM, hitches) is not measured yet: it is the test request (Testing). Commits: main 6c9edffe5da1f5b5383977aeab3f544c0de25d4c (2026-11-14); this PR fix/indexed-indirect-extent @ 5bde46684300ecb32e2bfdd645b000a7290ac37f. Hardware: Apple M4 Max (16 CPUs), macOS 26.7.1, Apple clang 21. Game-free: no GPU or game involved Scene: none. This PR's proof is the test, as CONTRIBUTING's `fix` row allows. The in-game cost is a test request (Testing). Method: `tests/test_draw_extent.py` against main's `gpu/vk/vk_draw.cpp`, `vk_draw.h` and `vk_internal.h` (6c9edffe) and against this PR; the full game-free suite on both How to reproduce: `python -I -B tests/test_draw_extent.py --cxx clang++ --headers <Vulkan include>` (add `--source <a tree of main>` for main's side) | | Before: main @ 6c9edffe | After: this PR @ 5bde4668 | |---|---|---| | `tests/test_draw_extent.py` | FAIL: 6 checks; the index buffer holds 64 of 122,683,456 bytes, the whole stream 16 of 118,489,104 | PASS | | full game-free suite on macOS (`tests/run_all.py --jobs 6`) | 531 passed, 32 failed, 17 skipped | 532 passed, 32 failed, 17 skipped | ``` main @ 6c9edffe: FAIL: an indexed indirect draw's indices past the import holding its first bytes: the bound index buffer holds 64 of the 122683456 bytes the draw may read FAIL: an index extent of 2^33 bytes (u8) is bound by 4 bytes as before, with one line: 1 draws, 0 lines logged, no import over 1 GB asked for FAIL: a u32 extent ends at the pool's end, 10 GB on: bound by 4 bytes as before, with one line: 0 lines logged FAIL: an extent whose import fails is bound by 4 bytes as before, with one line: 1 draws, 0 skipped, 0 lines logged FAIL: a vertex stream bound whole, its vertices past the import holding its first byte: the bound buffer holds 16 of the 118489104 bytes the stream may read FAIL: a stream bound whole past 1 GB of its pool is bound by 1 byte as before, with one line: 0 lines logged, no import over 1 GB asked for FAIL: 6 of the checks above this PR @ 5bde4668: PASS: an indexed indirect draw binds the indices it may read and a stream bound whole its pool, both in one import; past 1 GB or a failed import as before, with a line: vk: index buffer 0x2000000100: no single import takes the 8589934592 bytes a draw may read there; bound by its first 4 as before ``` On main, four of the six failures are only the missing log line: there the binding is the one main makes. The other two are the bug: the bound buffer ends before what the draw may read. Screenshots: none; nothing on screen in the game-free tests. ### Flags and settings None. ## Testing **Tested:** - On an Apple M4 Max with macOS 26.7.1, game-free only. - `tests/test_draw_extent.py` fails on main and passes here. - `tests/test_vertex_bind.py`, `tests/test_draw_binds.py` and `tests/test_fallback_pipeline_state.py` pass. `test_fallback_pipeline_state.py` also passes against main's `vk_draw.cpp` (`--source`, 6c9edffe). - The full suite on both sides, compared by test name. - By test name, against main 6c9edffe's run on the same Mac: only one status changed: `test_draw_extent`, absent on main, passes here. - These 32 checks fail here and on main on this Mac, by name (not triaged here: some need the macOS port's other pull requests, some are checks main added or changed since 5e072fc8): `test_appimage_package_retry`, `test_appimage_player_build`, `test_collect_wide`, `test_control`, `test_cpm_discovery`, `test_crash_dialog`, `test_crash_record`, `test_env_on`, `test_game_layout`, `test_lift_cleanup_error`, `test_linux_preflight`, `test_longshot_ctl`, `test_low_memory_notice`, `test_make_game_cache`, `test_mem_gate`, `test_mem_gate_children`, `test_mirror_trim`, `test_oot_state_tap`, `test_pcache_header_read`, `test_pipeline_cache_file`, `test_pipeline_cache_save`, `test_pipeline_play_cache`, `test_pipeline_tour`, `test_prof_symbols`, `test_program_cache_cleanup`, `test_program_cache_one_variant`, `test_readback_late`, `test_report_run`, `test_split_giants`, `test_vk_pgo`, `test_vram_budget_row`, `test_walk_key`. - The same 17 checks skipped as on main. - Crash reports during the run: none (main's run: fault-arm64-2026-11-14-180704.ips, recorder_sync_test.exe-2026-11-14-180744.ips). - Object compiles of `gpu/vk/vk_draw.cpp` with the renderer's flags (`nx2/build.py`), Apple clang 21, arm64 and x86_64, on main and here. - Main's renderer does not build on macOS as it is: `vk_draw.cpp` stops at `posix_fallocate` (`pipeline_cache_file.h`). - With a compile-only prelude declaring `environ` and `posix_fallocate`, all 4 objects build, 2 on main and 2 here. - **After the rebase onto main aa95d35f**, at `4b31c5bd` (`5bde4668` before the rebases onto e090cc48 and aa95d35f): Sweep3's checks of this head (the C/C++ it changes compiled, and its own tests run, on arm64 and x86_64 where they apply) gave the same results as before; `python -I -B tests/run_all.py` on wave S1's merge `11a5b4e7`, which carries this head: 568/22/17 (passed/failed/skipped) against main aa95d35f's 543/33/17 on this Mac, no new failure (`reports/2026-11-15-rebase-sweep-3.md`). **Not tested:** - No game run and no GPU run, on any platform. - A build on Windows (llvm-mingw) or Linux. The change uses only `gpu_pool_end`, `gpu_buffer_bind`, `logf` and `std::atomic`, as the code around it does. - Larger bindings mean larger imports. An index buffer or a stream bound whole now takes an import up to the end of its game pool, where before it took a chunk around its first bytes. On a discrete GPU each import gets a VRAM mirror of its whole range (`mirror_on`), within the mirrors' budget. A pool whose merged import passes 1 GB gets no mirror, and the GPU reads it over PCIe. The first binding of a larger range merges the chunks already imported into one new import on the replay thread. Main's comments in `vk_memory.cpp` put an import at 5-45 ms, and one that took two chunks of a pool as one at 130-147 ms, the one hitch of a panning run in Kokiri. How often that happens with this PR, and what it costs in fps, VRAM and hitches, is unmeasured. - Whether the game has indexed indirect draws or streams bound whole whose reads pass the old import's end, and so whether anything changes on screen, is unknown. - No timing: the measurement in the game (main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median) is the test request below. **Must be tested:** - Windows or Linux, a discrete AMD or NVIDIA GPU (VRAM mirrors on), any driver: build this PR @ 4b31c5bd and main aa95d35f the same way; in Kokiri Forest and on a Hyrule Field walk, the same settings, 2 interleaved runs of 120 s each after a warm-up, the FPS counter at Breakdown, report fps (average and 1% low), median and p99 frame time, and the GPU's longest frame, and from `longshot_log.txt` the import lines (`vk: pool … imported in … ms as … (N imports, M ms in all)`), the last `vk: N pools mirrored in VRAM (M MB)`, any `… stays in system memory: the mirrors have … MB of …` and `would take in … pools: left overlapping`, and this PR's `no single import takes …` lines (count them: each is a binding left as main makes it), expecting fps and frame times within run-to-run noise and the import count and mirror total within a few percent of main's; then the same scenes once each on main and this PR with Robust buffers (Debug tab, Developer) Off, looking for stretched or missing geometry on main that this PR fixes, and for none new, expecting the same picture or a fix (by hand: test request T-CL66). ## Risks **What could break:** - Medium. - The bindings can be much larger: up to a game pool's remaining bytes, at most 1 GB. The imports and mirrors grow with them. - A range whose merge with neighbouring imports passes 1 GB takes main's existing escape (`would take in … pools: left overlapping`), as any binding near 1 GB does today. - Past 1 GB, and where an import fails, the binding is main's and one of at most 16 lines says so. - Landing order: no conflict with `main` or with the other macOS-port PRs rebased on 6c9edffe (`git merge-tree` of the new heads, 19:49 CST). **Turning it off:** no switch. Reverting the commit is enough. `RX_VK_POOL_END_IMPORTED=1` (an existing A/B switch) makes `gpu_pool_end` answer the import's end, which narrows these extents to the import already there. It also changes the storage-buffer bindings that ask the pool's end, so it is not an off switch for this PR. **Compatibility:** Nothing changes: `vk_draw.cpp` is not in the translator stamp (`nx2/translator_files.py`), and saves, the options file, the shader and pipeline caches and the vk ABI are unchanged. ## 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`: on aa95d35f (#761, 2026-11-15), head `4b31c5bd`, without a conflict; this PR's own commits, old → new: `5bde4668` → `4b31c5bd`; the commit names elsewhere in this description are from before the rebase. - [x] `python -I -B tests/run_all.py` passes (any failure that also fails on `main` is named). On macOS: 532 passed, 17 skipped, and 32 failures that also fail on main 6c9edffe on this Mac, named under Testing. After the rebase onto main aa95d35f (head `4b31c5bd`): wave S1's merge `11a5b4e7`, 568/22/17 against main aa95d35f's 543/33/17, no new failure. - [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] 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.

mooki added 1 commit 2026-11-14 13:49:38 +00:00

fix(vk): an indexed indirect draw binds the indices it can read, a whole vertex stream its pool ... 05c373755e

An indexed indirect draw bound 4 bytes of its index buffer and a vertex stream bound whole (size 0, VK_WHOLE_SIZE)
1 byte, so the import holding them ended where it happened to, a chunk on: indices and vertices past it read 0 with
robust buffer access and anything without. Both now bind what the draw may read, to the end of the game pool holding
them (an index in [ibuf, ibuf + 2^33 units): index_extent; vertex_whole_extent). An extent no single import can hold
(over 1 GB, or a failed import) keeps the old binding and logs a line (extent_bind).

tests/test_draw_extent.py runs draw()'s vertex-stream loop and indirect branch against stand-in pools and imports: it
fails on main and passes here. test_vertex_bind.py and test_fallback_pipeline_state.py follow the stream loop's binding
through extent_bind.

Assisted-by: Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 <[email protected]>

mooki added the

kind

bug

priority

high

labels 2026-11-14 14:01:45 +00:00

mooki added the

area/renderer

label 2026-11-14 16:27:59 +00:00

mooki added the

platform/linux

platform/windows

labels 2026-11-14 18:23:56 +00:00

Yeetbot90 referenced this pull request 2026-11-14 18:51:26 +00:00

fix(vk): draws with 8-bit indices are drawn on GPUs without 8-bit index buffers (RADV on GCN 1-2) #541

mooki force-pushed fix/indexed-indirect-extent from 05c373755e to 5bde466843 2026-11-15 01:51:51 +00:00 Compare

mooki changed title from WIP: fix(vk): an indexed indirect draw binds the indices it can read, a whole vertex stream its pool to fix(vk): an indexed indirect draw binds the indices it can read, a whole vertex stream its pool 2026-11-15 03:33:51 +00:00

mooki force-pushed fix/indexed-indirect-extent from 5bde466843 to 99c1327a5c 2026-11-15 07:01:18 +00:00 Compare

mooki force-pushed fix/indexed-indirect-extent from 99c1327a5c to 4b31c5bdba 2026-11-15 13:02:40 +00:00 Compare

MaikelJSQ referenced this pull request 2026-11-15 18:05:02 +00:00

Game: Longshot closed unexpectedly while you were playing #859

Some checks failed

Hide checks

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

Website / Website on Cloudflare (pull_request) Has been skipped

Details

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

Details

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

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 9s

Details

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

Details

This pull request doesn't have enough approvals yet. 0 of 1 approvals granted.

This branch is out-of-date with the base branch

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u 'fix/indexed-indirect-extent:mooki-fix/indexed-indirect-extent'

git switch 'mooki-fix/indexed-indirect-extent'

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

area/accessibility

Barriers for people with disabilities

  

area/audio

Sound and music

  

area/input

Controllers, keyboard, mouse

  

area/install

Installing, updating, building from your game

  

area/menu

In-game menu and settings

  

area/renderer

Vulkan renderer, shaders, effects

  

area/saves

Save files

  

area/upscaling

DLSS, FSR, frame generation

  

beanstalk: approved @ 1697603

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 2c5a62e

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 4fd82b7

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 64d6d09

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 6f91fae

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 70a78f9

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 8d361dc

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 9d8356e

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ abf2227

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ d82f202

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ e5f1e40

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ ee8ab5f

Set by Beanstalk for show; the farm's database is what counts

  

ci: approved

A maintainer approved CircleCI for this fork PR (sticky; removed after 14 quiet days)

  

good first issue

A good start for a new contributor

  

gpu/amd

AMD GPUs

  

gpu/igpu

Integrated GPUs

  

gpu/intel

Intel GPUs

  

gpu/nvidia

NVIDIA GPUs

  

help wanted

Extra hands or knowledge wanted

  

kind

bug

Something doesn't work as it should

  

kind

chore

Build, CI, dependencies, tidying

  

kind

docs

Documentation

  

kind

feature

A new feature or an improvement

  

kind

performance

Frame rate, frame time, loading, memory

  

kind

question

A question rather than a change

  

needs measurements

Waiting for before/after measurements

  

platform/linux

Affects Linux (a change for every system carries both Windows and Linux)

  

platform/macos

macOS (Apple silicon or Intel Macs)

  

platform/steam-deck

Steam Deck

  

platform/windows

Affects Windows (a change for every system carries both Windows and Linux)

  

preview

Maintainers: deploy a preview of this PR's website (read its changes first). Removed after the run.

  

priority

critical

Crashes, data loss, or blocks most players

  

priority

high

Important: next in line

  

priority

low

Nice to have

  

regression

Worked in an earlier build, broken in a newer one: highest priority, above other bugs

  

status

blocked

Waits on something else first

  

status

confirmed

Reproduced or agreed on

  

status

duplicate

Already reported or proposed

  

status

invalid

Not a problem in this project

  

status

needs-info

Waiting for more information from the reporter

  

status

wontfix

Will not be worked on

  

testing wanted

Wanted: a test on other hardware (posts a test request in #test-requests)

  

ci: approved

No labels

area/accessibility

area/audio

area/input

area/install

area/menu

area/renderer

area/saves

area/upscaling

beanstalk: approved @ 1697603

beanstalk: approved @ 2c5a62e

beanstalk: approved @ 4fd82b7

beanstalk: approved @ 64d6d09

beanstalk: approved @ 6f91fae

beanstalk: approved @ 70a78f9

beanstalk: approved @ 8d361dc

beanstalk: approved @ 9d8356e

beanstalk: approved @ abf2227

beanstalk: approved @ d82f202

beanstalk: approved @ e5f1e40

beanstalk: approved @ ee8ab5f

ci: approved

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/macos

platform/steam-deck

platform/windows

preview

priority

critical

priority

high

priority

low

regression

status

blocked

status

confirmed

status

duplicate

status

invalid

status

needs-info

status

wontfix

testing wanted

ci: approved

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

MagicBeanDev/Longshot!434

No description provided.