MagicBeanDev/Longshot

Watch

Fork 98

fix(vk): draw quads with 8-bit indices on devices without index_uint8 #595

Open

jcabarca wants to merge 1 commit from jcabarca/Longshot:fix/vk-uint8-quads into main

jcabarca commented

2026-11-14 22:03:11 +00:00

Contributor

Type: fix · Risk: low · Tested on: Windows, AMD Radeon RX 9070 XT · Needs: none

Why

On a GPU without VK_EXT_index_type_uint8 (dev.index_uint8 false), every indexed quad draw with 8-bit indices was skipped, so those quads were not drawn. Quads never give 8-bit indices to the GPU (they are read on the CPU and drawn with 32-bit ones), so now they are drawn; no Vulkan rule was broken, this is a functional fix.

How to reproduce

Seen on: not seen in the game: found by reading draw() in gpu/vk/vk_draw.cpp; it needs a GPU without VK_EXT_index_type_uint8
Settings and switches: defaults
Steps: on such a GPU, reach any scene that draws quads with 8-bit indices; game-free: python -I -B tests/test_uint8_quads.py on main (copy the test from this PR)
Expected: the quads are drawn
Actual: the quads are skipped (SKIP_HERE()); the test fails on main

What changed

Indexed quad draws with 8-bit indices are no longer skipped on devices without uint8 index support, because the quads path reads the indices on the CPU and draws with 32-bit ones. Every other primitive with 8-bit indices is still skipped there, as before.

vk: draw() in gpu/vk/vk_draw.cpp: the skip of indexed draws with 8-bit indices on devices without index_uint8 no longer applies to NVN_DRAW_PRIMITIVE_QUADS.
tests: tests/test_uint8_quads.py (new) reads draw() in gpu/vk/vk_draw.cpp and checks that the quads path still reads 8-bit indices on the CPU into 32-bit ones and that the 8-bit skip before it leaves quads out.

The bug, step by step:

draw() picks the index type from d.itype (0 = 8-bit, 1 = 16-bit, 2 = 32-bit).
Right after, if (d.indexed && d.itype == 0 && !dev.index_uint8) SKIP_HERE(); dropped every indexed draw with 8-bit indices when the device has no uint8 index support, before the quads path is reached.
The quads path does not need that support: it reads each index on the CPU with unit = d.itype == 0 ? 1 : d.itype == 1 ? 2 : 4 (one byte for 8-bit ones), writes two triangles per quad into the stream and binds them with bind_index(cmd, cur->stream, at6, VK_INDEX_TYPE_UINT32).
So on such a device, quads with 8-bit indices were dropped although they would draw correctly.

No VUID: VK_INDEX_TYPE_UINT8_EXT is never bound for quads, so nothing in the Vulkan spec was broken; the draw was simply lost.

Before (draw()):

if (d.indexed && d.itype == 0 && !dev.index_uint8) SKIP_HERE();

After:

// (quads read their indices on the CPU and draw with 32-bit ones: they need no uint8 index support)
if (d.indexed && d.itype == 0 && !dev.index_uint8 && d.prim != NVN_DRAW_PRIMITIVE_QUADS) SKIP_HERE();

Start reading at the 8-bit skip in draw(), then the quads block right below it: unit = d.itype == 0 ? 1 : ... reads the indices and bind_index(..., VK_INDEX_TYPE_UINT32) binds the 32-bit ones.

Measurements

Verdict: it fixes the bug: tests/test_uint8_quads.py fails on main and passes with this PR, in both interleaved runs.

Hardware: AMD Radeon RX 9070 XT, Windows; the test itself is game-free (it reads source, it does not use the GPU); Python 3.12
Scene: none, game-free
Method: tests/test_uint8_quads.py from this PR against each side's gpu/vk/vk_draw.cpp (a checkout of each commit, the PR's test copied into main's), 2 runs each, interleaved (main, PR, main, PR); the result is pass/fail, so the median is the result both runs gave
How to reproduce: python -I -B tests/test_uint8_quads.py in a checkout of each side (copy the test into main's)

	Before: main @ `5e072fc8`	After: this PR @ `196a9420`
`tests/test_uint8_quads.py`, run 1	fails	passes
`tests/test_uint8_quads.py`, run 2	fails	passes
Median	fails	passes

run 1, main @ 5e072fc8:
quads with 8-bit indices are skipped without uint8 support, though they draw with 32-bit indices
exit 1
run 1, this PR @ 196a9420:
quads with 8-bit indices are drawn without uint8 index support
exit 0
run 2, main @ 5e072fc8:
quads with 8-bit indices are skipped without uint8 support, though they draw with 32-bit indices
exit 1
run 2, this PR @ 196a9420:
quads with 8-bit indices are drawn without uint8 index support
exit 0

What the test checks:

The quads path in draw() (if (d.prim == NVN_DRAW_PRIMITIVE_QUADS) {) still exists, still reads 8-bit indices one byte each (unit = d.itype == 0 ? 1) and still binds VK_INDEX_TYPE_UINT32; if not, it stops with "this test needs updating", since the fix relies on it.
The skip of 8-bit index draws without uint8 support (!dev.index_uint8 with SKIP_HERE();) before the quads path still exists, and it excludes NVN_DRAW_PRIMITIVE_QUADS.

Flags and settings

None.

Testing

Tested: tests/test_uint8_quads.py on main (fails) and on this branch (passes), Windows, Python 3.12.

Built and played: this commit builds on Windows (llvm-mingw back end) and on Linux (longshot_vk.so, without and with AMD FSR 4). Played on Windows, AMD Radeon RX 9070 XT: the game runs normally.

Not tested: a device without index_uint8: the RX 9070 XT has it, so in the game this path (quads with 8-bit indices on such a device) is never reached. Linux was built, not run.

Must be tested: None: the RX 9070 XT cannot reach this path, and no GPU the team tests on is known to lack VK_EXT_index_type_uint8; the game-free test covers the change.

Risks

What could break: only devices without index_uint8 take the new path, and only for quads with 8-bit indices; those now go through the same quads path that 16-bit and 32-bit quads already use. Every other 8-bit index draw is skipped as before.

Turning it off: revert the commit.

Compatibility: Nothing changes.

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). (no issue fits)
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). (the check that reads the changed code, on this PR, Windows: test_uint8_quads, passing; no other test references this code)
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. (tests/test_uint8_quads.py, new, reads gpu/vk/vk_draw.cpp)
Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. (built on Windows with llvm-mingw and on Linux, longshot_vk.so without and with AMD FSR 4; the change is one Vulkan draw condition, no OS-specific code or paths)

**Type:** fix · **Risk:** low · **Tested on:** Windows, AMD Radeon RX 9070 XT · **Needs:** none ## Why On a GPU without `VK_EXT_index_type_uint8` (`dev.index_uint8` false), every indexed quad draw with 8-bit indices was skipped, so those quads were not drawn. Quads never give 8-bit indices to the GPU (they are read on the CPU and drawn with 32-bit ones), so now they are drawn; no Vulkan rule was broken, this is a functional fix. ## How to reproduce **Seen on:** not seen in the game: found by reading `draw()` in `gpu/vk/vk_draw.cpp`; it needs a GPU without `VK_EXT_index_type_uint8` **Settings and switches:** defaults **Steps:** on such a GPU, reach any scene that draws quads with 8-bit indices; game-free: `python -I -B tests/test_uint8_quads.py` on main (copy the test from this PR) **Expected:** the quads are drawn **Actual:** the quads are skipped (`SKIP_HERE()`); the test fails on main ## What changed Indexed quad draws with 8-bit indices are no longer skipped on devices without uint8 index support, because the quads path reads the indices on the CPU and draws with 32-bit ones. Every other primitive with 8-bit indices is still skipped there, as before. - vk: `draw()` in `gpu/vk/vk_draw.cpp`: the skip of indexed draws with 8-bit indices on devices without `index_uint8` no longer applies to `NVN_DRAW_PRIMITIVE_QUADS`. - tests: `tests/test_uint8_quads.py` (new) reads `draw()` in `gpu/vk/vk_draw.cpp` and checks that the quads path still reads 8-bit indices on the CPU into 32-bit ones and that the 8-bit skip before it leaves quads out. The bug, step by step: 1. `draw()` picks the index type from `d.itype` (0 = 8-bit, 1 = 16-bit, 2 = 32-bit). 2. Right after, `if (d.indexed && d.itype == 0 && !dev.index_uint8) SKIP_HERE();` dropped every indexed draw with 8-bit indices when the device has no uint8 index support, before the quads path is reached. 3. The quads path does not need that support: it reads each index on the CPU with `unit = d.itype == 0 ? 1 : d.itype == 1 ? 2 : 4` (one byte for 8-bit ones), writes two triangles per quad into the stream and binds them with `bind_index(cmd, cur->stream, at6, VK_INDEX_TYPE_UINT32)`. 4. So on such a device, quads with 8-bit indices were dropped although they would draw correctly. No VUID: `VK_INDEX_TYPE_UINT8_EXT` is never bound for quads, so nothing in the Vulkan spec was broken; the draw was simply lost. Before (`draw()`): ```cpp if (d.indexed && d.itype == 0 && !dev.index_uint8) SKIP_HERE(); ``` After: ```cpp // (quads read their indices on the CPU and draw with 32-bit ones: they need no uint8 index support) if (d.indexed && d.itype == 0 && !dev.index_uint8 && d.prim != NVN_DRAW_PRIMITIVE_QUADS) SKIP_HERE(); ``` Start reading at the 8-bit skip in `draw()`, then the quads block right below it: `unit = d.itype == 0 ? 1 : ...` reads the indices and `bind_index(..., VK_INDEX_TYPE_UINT32)` binds the 32-bit ones. ## Measurements **Verdict:** it fixes the bug: `tests/test_uint8_quads.py` fails on main and passes with this PR, in both interleaved runs. Hardware: AMD Radeon RX 9070 XT, Windows; the test itself is game-free (it reads source, it does not use the GPU); Python 3.12 Scene: none, game-free Method: `tests/test_uint8_quads.py` from this PR against each side's `gpu/vk/vk_draw.cpp` (a checkout of each commit, the PR's test copied into main's), 2 runs each, interleaved (main, PR, main, PR); the result is pass/fail, so the median is the result both runs gave How to reproduce: `python -I -B tests/test_uint8_quads.py` in a checkout of each side (copy the test into main's) | | Before: main @ 5e072fc8 | After: this PR @ 196a9420 | |---|---|---| | `tests/test_uint8_quads.py`, run 1 | fails | passes | | `tests/test_uint8_quads.py`, run 2 | fails | passes | | Median | fails | passes | ``` run 1, main @ 5e072fc8: quads with 8-bit indices are skipped without uint8 support, though they draw with 32-bit indices exit 1 run 1, this PR @ 196a9420: quads with 8-bit indices are drawn without uint8 index support exit 0 run 2, main @ 5e072fc8: quads with 8-bit indices are skipped without uint8 support, though they draw with 32-bit indices exit 1 run 2, this PR @ 196a9420: quads with 8-bit indices are drawn without uint8 index support exit 0 ``` What the test checks: - The quads path in `draw()` (`if (d.prim == NVN_DRAW_PRIMITIVE_QUADS) {`) still exists, still reads 8-bit indices one byte each (`unit = d.itype == 0 ? 1`) and still binds `VK_INDEX_TYPE_UINT32`; if not, it stops with "this test needs updating", since the fix relies on it. - The skip of 8-bit index draws without uint8 support (`!dev.index_uint8` with `SKIP_HERE();`) before the quads path still exists, and it excludes `NVN_DRAW_PRIMITIVE_QUADS`. ### Flags and settings None. ## Testing **Tested:** `tests/test_uint8_quads.py` on main (fails) and on this branch (passes), Windows, Python 3.12. **Built and played:** this commit builds on Windows (llvm-mingw back end) and on Linux (`longshot_vk.so`, without and with AMD FSR 4). Played on Windows, AMD Radeon RX 9070 XT: the game runs normally. **Not tested:** a device without `index_uint8`: the RX 9070 XT has it, so in the game this path (quads with 8-bit indices on such a device) is never reached. Linux was built, not run. **Must be tested:** None: the RX 9070 XT cannot reach this path, and no GPU the team tests on is known to lack `VK_EXT_index_type_uint8`; the game-free test covers the change. ## Risks **What could break:** only devices without `index_uint8` take the new path, and only for quads with 8-bit indices; those now go through the same quads path that 16-bit and 32-bit quads already use. Every other 8-bit index draw is skipped as before. **Turning it off:** revert the commit. **Compatibility:** Nothing changes. ## 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). (no issue fits) - [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). (the check that reads the changed code, on this PR, Windows: `test_uint8_quads`, passing; no other test references this code) - [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. (`tests/test_uint8_quads.py`, new, reads `gpu/vk/vk_draw.cpp`) - [x] Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. (built on Windows with llvm-mingw and on Linux, `longshot_vk.so` without and with AMD FSR 4; the change is one Vulkan draw condition, no OS-specific code or paths)

jcabarca added 1 commit

2026-11-14 22:03:11 +00:00

fix(vk): draw quads with 8-bit indices on devices without index_uint8

CI / Conventional Commit title (pull_request) Blocked by required conditions

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions

Details

CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions

Details

Third-party parts / The same as upstream (pull_request) Blocked by required conditions

Details

Third-party parts / As their notes say (pull_request) Blocked by required conditions

Details

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

Details

Website / Website on Cloudflare (pull_request_target) Has been skipped

Details

196a942033

draw() skipped every indexed draw with 8-bit indices when the device lacks VK_EXT_index_type_uint8, before reaching the quads path. Quads never give 8-bit indices to the GPU: they read them on the CPU (one byte each) and bind 32-bit triangle indices made in the stream. The skip now leaves quads out. tests/test_uint8_quads.py fails on main and passes with the fix.

Assisted-by: GitHub Copilot

jcabarca changed title from ~~WIP: fix(vk): draw quads with 8-bit indices on devices without index_uint8~~ to fix(vk): draw quads with 8-bit indices on devices without index_uint8

2026-11-14 22:11:39 +00:00

jcabarca commented

2026-11-14 23:25:15 +00:00

Author

Contributor

Description updated to the new pull request template (#571): status line, How to reproduce, a summary first in What changed, the Verdict and Before/After columns in Measurements, Flags and settings under Measurements, Must be tested, Risks, and the Everyone / When it applies checklist. The code and the measurements are unchanged.