MagicBeanDev/Longshot

Watch

Fork 102

WIP: fix(vk): a direct quad draw's triangles grow the stream under RX_VK_STREAM_GROW_OFF too #853

Draft

mooki wants to merge 1 commit from mooki/Longshot:fix/quad-stream-grow into main

mooki commented

2026-11-15 17:40:29 +00:00

Member

Type: fix · Risk: low · Tested on: macOS 26.7.1, Apple M4 Max (arm64; x86_64 under Rosetta) · Needs: Windows and Linux, any GPU

Why

LS draws NVN's quads by writing each quad's two triangles into the recording's upload stream and drawing them as an indexed
triangle list. The testing switch RX_VK_STREAM_GROW_OFF=1 keeps that stream from growing when a recording needs more than it
holds. With the switch on and the stream full, a direct quad draw found no room for its triangles and was left out of the frame,
on every platform and driver. The switch is there to try a fixed-size stream, not to drop geometry: what does not fit should be
refused only where nothing is drawn wrong because of it. This PR lets the stream grow for the triangles a quad draw cannot be
drawn without, under the switch too. Without the switch nothing changes. Found while the macOS port checked every draw that is
left out for want of stream room (its StreamRoom work). See #420.

How to reproduce

Seen on: every OS and GPU (the same code on every driver); shown on main by this PR's game-free test, on macOS 26.7.1, Apple M4 Max, arm64 and x86_64 under Rosetta
Settings and switches: RX_VK_STREAM_GROW_OFF=1 (with it the first stream is 24 MB; RX_VK_STREAM_MB=1 makes it 1 MB, so it fills sooner)
Steps: start the game with the switch and play until a recording's stream fills (vk: the recording's stream is full (N MB) in longshot_log.txt) with a direct quad draw after it in the same recording. Game-free, from this branch: python -I -B tests/test_stream_needed.py --headers (the Vulkan-Headers include folder) --source (a checkout of main), case grow-off-full
Expected: the quad draw drawn, from a bigger stream made for its triangles (vk: a recording needed more than its N MB stream: one of M MB made)
Actual: the quad draw is left out: counted at the triangles' stream_alloc in the vk: skips a frame by vk_draw.cpp line: report; the test says FAIL grow-off-full: the direct quad draw left out (1 skipped, 0 drawn of 1)

What changed

The stream's allocation takes a needed flag: what a draw cannot be drawn without. For a needed allocation the stream grows
under RX_VK_STREAM_GROW_OFF too; every other use of the stream is refused under the switch as before. Only draw()'s direct
quad draw asks for its triangles as needed. The fix comes from the macOS port's stream work (its commit bbfb465f), applied to
main's code by hand.

gpu/vk/vk_draw.cpp: stream_next(need, needed) makes the next stream under the switch when needed;
stream_reserve(n, align, needed = false) and stream_alloc(n, align, needed = false) pass it along; draw()'s direct quad draw
calls stream_alloc((uint64_t)quads * 6 * 4, 16, true). A quad draw is left out only when no buffer can be made at all. The
other callers (uniform data, stream_copy, uniform_upload) are unchanged: under the switch an upload that does not fit still
reads the null buffer, as before, and no draw is left out for it. The comment above stream_grow_off() says what the switch
refuses and what it does not.
tests/test_stream_needed.py with tests/stream_needed_test.cpp (new): RecordingRes, stream_insert .. stream_alloc and
draw()'s direct quad draw cut out of vk_draw.cpp (and DrawArgs out of vk_draw.h), run on a model of a recording and of
the GPU. Cases: grow-off-full (the switch on and the stream full: a quad draw from its first vertex and one indexed by the
game's 16-bit indices, each drawn from a stream made for its triangles, each quad as {v0, v1, v2, v0, v2, v3});
others-refused (an upload under the switch still refused, no stream made); grow-on (the default: drawn, uploads grow the stream
as before); room; no-memory (no buffer to be made: left out, the stream as it was). And a source check: the direct quad draw
alone asks as needed, and stream_next is reached from stream_reserve alone. --source runs it on another tree.

Measurements

Nothing to measure: without RX_VK_STREAM_GROW_OFF the stream code takes main's path (it grows as before); with the switch the fix is whether a quad draw is drawn, which the new game-free test shows (failing on main, passing here, arm64 and x86_64).

Flags and settings

One existing testing switch changes what it does: RX_VK_STREAM_GROW_OFF no longer leaves a quad draw out.

Flag	What it is	Before this PR	After this PR	Effect	Helps / doesn't help / untested	Recommendation
`RX_VK_STREAM_GROW_OFF`	testing switch: the recording's stream never grows (its first stream 24 MB instead of 2)	unset: the stream grows; `=1`: a direct quad draw that finds the stream full is left out	unset: the stream grows, as before; `=1`: uploads are refused as before, a direct quad draw's triangles grow the stream, so no quad draw is left out	with the switch, quad draws no longer vanish once the stream is full	shown game-free (tests/test_stream_needed.py); not tried in the game with the switch	unset (off): for testing only

Testing

Tested: on macOS 26.7.1, Apple M4 Max, game-free, at this PR's head d0df2ded:

tests/test_stream_needed.py on arm64 and on x86_64 under Rosetta (clang++ -arch x86_64): all five cases and the source
check pass on both. The same test against main's vk_draw.cpp (--source, main aa95d35f): grow-off-full and the source check
fail on both arches, the other four cases pass (as they should: they are main's behaviour too).
The checks that cut the stream functions out of vk_draw.cpp or read them, on this head: test_draw_stream_visibility,
test_recording_admission, test_fsr_uniform_ownership and test_draw_binds on arm64 and x86_64, test_mem_trim and
test_run_all_declarations: all pass. None needed a change.
gpu/vk/vk_draw.cpp compiled whole for arm64 and x86_64 with nx2/build.py's own macOS commands: both exit 0. Main has no macOS
build, so this PR's vk_draw.cpp (byte for byte) was compiled in the macOS port's host tree (topic macos-host 40c1c5f5,
which leaves vk_draw.cpp as main has it and adds the macOS build and cache-file code main lacks); compiled in main's own tree
it stops at posix_fallocate in pipeline_cache_file.h (not in this PR), on both arches.
What Windows and Linux compile (unifdef -U__APPLE__ of every changed C/C++ file, compared as tokens with main's):
vk_draw.cpp only, and the difference is exactly this change (the needed parameter of the three stream functions, its
pass-through, stream_grow_off() && !needed, and the quad draw's true); comments aside, nothing else.
The whole game-free suite (python -I -B tests/run_all.py --jobs 6) on this Mac against main's: 544 passed, 33 failed, 17 skipped (main: 543, 33, 17); by name the one difference is test_stream_needed, new and passing. The 33 failures are main's own on this Mac.
In the game, on macOS arm64: this commit is part of the integration build I41 (78ec1c47), whose run E9a held play 1,197 s
with every draw drawn (12 of 12 must-shows, a clean end, 0 skipped draws), without the switch: the default path is unchanged.

Not tested: Windows and Linux (no PC here); the game with RX_VK_STREAM_GROW_OFF=1 on any platform (the switch is for
testing; the model test covers the draw).

Must be tested:

Windows and Linux, any GPU: build this PR (head d0df2ded) and main (aa95d35f) the same way and run python -I -B tests/run_all.py on each, then start each to the title, expecting test_stream_needed to pass, no other check to change status against main, and the same vk: lines in longshot_log.txt as main's (Test Farm: @Beanstalk windows and linux: build unchanged, run_all, start to the title)

Risks

What could break: only under RX_VK_STREAM_GROW_OFF=1: a recording with direct quad draws after its stream fills now makes
bigger streams (more memory) where before it left those draws out. Without the switch nothing changes: every caller takes
main's path.

Turning it off: revert the commit (the switch is the only path it changes).

Compatibility: Nothing changes: no cache, save, longshot_options.ini key or vk ABI is touched.

Issues

See #420

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.
A game-free test added for the fix, failing on main and passing here.
Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths.

**Type:** fix · **Risk:** low · **Tested on:** macOS 26.7.1, Apple M4 Max (arm64; x86_64 under Rosetta) · **Needs:** Windows and Linux, any GPU ## Why LS draws NVN's quads by writing each quad's two triangles into the recording's upload stream and drawing them as an indexed triangle list. The testing switch `RX_VK_STREAM_GROW_OFF=1` keeps that stream from growing when a recording needs more than it holds. With the switch on and the stream full, a direct quad draw found no room for its triangles and was left out of the frame, on every platform and driver. The switch is there to try a fixed-size stream, not to drop geometry: what does not fit should be refused only where nothing is drawn wrong because of it. This PR lets the stream grow for the triangles a quad draw cannot be drawn without, under the switch too. Without the switch nothing changes. Found while the macOS port checked every draw that is left out for want of stream room (its StreamRoom work). See #420. ## How to reproduce **Seen on:** every OS and GPU (the same code on every driver); shown on main by this PR's game-free test, on macOS 26.7.1, Apple M4 Max, arm64 and x86_64 under Rosetta **Settings and switches:** `RX_VK_STREAM_GROW_OFF=1` (with it the first stream is 24 MB; `RX_VK_STREAM_MB=1` makes it 1 MB, so it fills sooner) **Steps:** start the game with the switch and play until a recording's stream fills (`vk: the recording's stream is full (N MB)` in longshot_log.txt) with a direct quad draw after it in the same recording. Game-free, from this branch: `python -I -B tests/test_stream_needed.py --headers (the Vulkan-Headers include folder) --source (a checkout of main)`, case grow-off-full **Expected:** the quad draw drawn, from a bigger stream made for its triangles (`vk: a recording needed more than its N MB stream: one of M MB made`) **Actual:** the quad draw is left out: counted at the triangles' `stream_alloc` in the `vk: skips a frame by vk_draw.cpp line:` report; the test says `FAIL grow-off-full: the direct quad draw left out (1 skipped, 0 drawn of 1)` ## What changed The stream's allocation takes a `needed` flag: what a draw cannot be drawn without. For a needed allocation the stream grows under `RX_VK_STREAM_GROW_OFF` too; every other use of the stream is refused under the switch as before. Only draw()'s direct quad draw asks for its triangles as needed. The fix comes from the macOS port's stream work (its commit `bbfb465f`), applied to main's code by hand. - `gpu/vk/vk_draw.cpp`: `stream_next(need, needed)` makes the next stream under the switch when `needed`; `stream_reserve(n, align, needed = false)` and `stream_alloc(n, align, needed = false)` pass it along; draw()'s direct quad draw calls `stream_alloc((uint64_t)quads * 6 * 4, 16, true)`. A quad draw is left out only when no buffer can be made at all. The other callers (uniform data, `stream_copy`, `uniform_upload`) are unchanged: under the switch an upload that does not fit still reads the null buffer, as before, and no draw is left out for it. The comment above `stream_grow_off()` says what the switch refuses and what it does not. - `tests/test_stream_needed.py` with `tests/stream_needed_test.cpp` (new): `RecordingRes`, `stream_insert` .. `stream_alloc` and draw()'s direct quad draw cut out of `vk_draw.cpp` (and `DrawArgs` out of `vk_draw.h`), run on a model of a recording and of the GPU. Cases: grow-off-full (the switch on and the stream full: a quad draw from its first vertex and one indexed by the game's 16-bit indices, each drawn from a stream made for its triangles, each quad as {v0, v1, v2, v0, v2, v3}); others-refused (an upload under the switch still refused, no stream made); grow-on (the default: drawn, uploads grow the stream as before); room; no-memory (no buffer to be made: left out, the stream as it was). And a source check: the direct quad draw alone asks as needed, and `stream_next` is reached from `stream_reserve` alone. `--source` runs it on another tree. ## Measurements Nothing to measure: without `RX_VK_STREAM_GROW_OFF` the stream code takes main's path (it grows as before); with the switch the fix is whether a quad draw is drawn, which the new game-free test shows (failing on main, passing here, arm64 and x86_64). ### Flags and settings One existing testing switch changes what it does: `RX_VK_STREAM_GROW_OFF` no longer leaves a quad draw out. | Flag | What it is | Before this PR | After this PR | Effect | Helps / doesn't help / untested | Recommendation | |---|---|---|---|---|---|---| | `RX_VK_STREAM_GROW_OFF` | testing switch: the recording's stream never grows (its first stream 24 MB instead of 2) | unset: the stream grows; `=1`: a direct quad draw that finds the stream full is left out | unset: the stream grows, as before; `=1`: uploads are refused as before, a direct quad draw's triangles grow the stream, so no quad draw is left out | with the switch, quad draws no longer vanish once the stream is full | shown game-free (tests/test_stream_needed.py); not tried in the game with the switch | unset (off): for testing only | ## Testing **Tested:** on macOS 26.7.1, Apple M4 Max, game-free, at this PR's head `d0df2ded`: - `tests/test_stream_needed.py` on arm64 and on x86_64 under Rosetta (`clang++ -arch x86_64`): all five cases and the source check pass on both. The same test against main's `vk_draw.cpp` (`--source`, main `aa95d35f`): grow-off-full and the source check fail on both arches, the other four cases pass (as they should: they are main's behaviour too). - The checks that cut the stream functions out of `vk_draw.cpp` or read them, on this head: `test_draw_stream_visibility`, `test_recording_admission`, `test_fsr_uniform_ownership` and `test_draw_binds` on arm64 and x86_64, `test_mem_trim` and `test_run_all_declarations`: all pass. None needed a change. - `gpu/vk/vk_draw.cpp` compiled whole for arm64 and x86_64 with nx2/build.py's own macOS commands: both exit 0. Main has no macOS build, so this PR's `vk_draw.cpp` (byte for byte) was compiled in the macOS port's host tree (topic macos-host `40c1c5f5`, which leaves `vk_draw.cpp` as main has it and adds the macOS build and cache-file code main lacks); compiled in main's own tree it stops at `posix_fallocate` in `pipeline_cache_file.h` (not in this PR), on both arches. - What Windows and Linux compile (`unifdef -U__APPLE__` of every changed C/C++ file, compared as tokens with main's): `vk_draw.cpp` only, and the difference is exactly this change (the `needed` parameter of the three stream functions, its pass-through, `stream_grow_off() && !needed`, and the quad draw's `true`); comments aside, nothing else. - The whole game-free suite (`python -I -B tests/run_all.py --jobs 6`) on this Mac against main's: 544 passed, 33 failed, 17 skipped (main: 543, 33, 17); by name the one difference is `test_stream_needed`, new and passing. The 33 failures are main's own on this Mac. - In the game, on macOS arm64: this commit is part of the integration build I41 (`78ec1c47`), whose run E9a held play 1,197 s with every draw drawn (12 of 12 must-shows, a clean end, 0 skipped draws), without the switch: the default path is unchanged. **Not tested:** Windows and Linux (no PC here); the game with `RX_VK_STREAM_GROW_OFF=1` on any platform (the switch is for testing; the model test covers the draw). **Must be tested:** - Windows and Linux, any GPU: build this PR (head `d0df2ded`) and main (`aa95d35f`) the same way and run `python -I -B tests/run_all.py` on each, then start each to the title, expecting `test_stream_needed` to pass, no other check to change status against main, and the same `vk:` lines in longshot_log.txt as main's (Test Farm: @Beanstalk windows and linux: build unchanged, run_all, start to the title) ## Risks **What could break:** only under `RX_VK_STREAM_GROW_OFF=1`: a recording with direct quad draws after its stream fills now makes bigger streams (more memory) where before it left those draws out. Without the switch nothing changes: every caller takes main's path. **Turning it off:** revert the commit (the switch is the only path it changes). **Compatibility:** Nothing changes: no cache, save, `longshot_options.ini` key or vk ABI is touched. ## Issues See #420 ## 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`. - [ ] `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] A game-free test added for the fix, failing on `main` and passing here. - [ ] Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths.

mooki added 1 commit

2026-11-15 17:40:29 +00:00

fix(vk, every platform): a direct quad draw's triangles grow the stream under RX_VK_STREAM_GROW_OFF too

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_target) Has been skipped

Details

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

Details

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

Details

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

Details

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

Details

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

Details

d0df2deda5

A direct quad draw (NVN's quads, two triangles each) is drawn from the triangles draw() writes into the recording's stream. With
RX_VK_STREAM_GROW_OFF=1 and the stream full, stream_alloc refused them and the draw was left out (SKIP_HERE), on every driver.
stream_alloc and stream_reserve take needed (false by default, every other caller as before): what a draw cannot be drawn without,
for which stream_next makes the next stream under RX_VK_STREAM_GROW_OFF too. The quads' triangles are needed; a quad draw is left
out only when no buffer can be made at all. Without the switch nothing changes (the stream grows as before). Uploads keep the
switch's refusal (an upload refused reads the null buffer, no draw left out). From the macOS port's StreamRoom work
(reports/2026-11-15-stream-room.md there, its commit bbfb465f), applied to main's code.

tests/test_stream_needed.py (new, with tests/stream_needed_test.cpp): RecordingRes, stream_insert .. stream_alloc and draw()'s
direct quad draw cut out of vk_draw.cpp, on a model of a recording and of the GPU: the stream full under the switch, both a quad
draw from its first vertex and one indexed by the game's 16-bit indices drawn from a stream made for their triangles; an upload
still refused, the stream growing by default, room left, no buffer to be made (left out, the stream as it was); and a source check
that the direct quad draw alone asks as needed. On main (--source) case grow-off-full and the source check fail, on arm64 and on
x86_64 under Rosetta; with this commit all pass on both.

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