MagicBeanDev/Hookshot
Archived
Watch
6
0
Fork
You've already forked Hookshot
 0
Code Issues 125 Pull requests 205 Activity

fix(vk): stop requiring vertex strides for mesh fallback draws #2796

Closed
github-sync wants to merge 5 commits from github/pr/2796 into main
pull from: github/pr/2796
merge into: MagicBeanDev:main
Conversation 6 Commits 5 Files changed 4 +345 -1
github-sync commented 2026-11-12 14:41:09 +00:00
Owner

@mooki000 opened this pull request on GitHub · 2026-11-12 13:42 UTC · original
Merged on GitHub in 9b6072cd4b

GPUs using the compute mesh fallback were asked for a dynamic vertex-buffer stride even though those draws bind no vertex buffers. This removes that unused requirement, keeps topology and primitive restart dynamic, and restores dynamic strides before returning to an ordinary draw.

Head 805eb0b3: the five commits rebased onto main 15d4ae8c; the changed lines of both code commits are identical to 5dbdfe0b (on 65a4d874), which is the head the fork's Windows run and the full-suite figures below were made on. The focused test passes on the new base too.

What this is for: the Vulkan state error reported in #2494/#1001 and the ordinary-draw cache transition Joey-Magnus found in review. What is measured: production construction/cache decisions with GCC and Clang, earlier fork Windows source checks, Intel UHD 770 game counters (2,829 omitted restores before; all 2,880 returns rebound after), and stock-build Khronos validation: VUID-04913 968 → 0, with no new message IDs. The existing fragment-output warning remains. Not measured: performance, Windows gameplay, other GPUs' gameplay, or gameplay on the rebased head.

Refs #2494. Part of #1001; this does not close the wider no-mesh-shader tracker.

Problem

  • The fallback's generated vertex shader reads records by vertex index. build_graphics already gives it no vertex bindings or attributes.
  • The same function nevertheless added VK_DYNAMIC_STATE_VERTEX_INPUT_BINDING_STRIDE to every non-native-mesh pipeline.
  • The fallback issues vkCmdDrawIndirect and returns before the ordinary vertex-buffer/stride binding code. This is the unset dynamic state reported as VUID-vkCmdDrawIndirect-pStrides-04913.
  • The owner closed #2494 to collect it in #1001, not because it was fixed. This change does not claim that the validation error caused a crash.
  • Joey-Magnus caught the second step: binding the now-static-stride fallback invalidates previous dynamic strides, while two vertex-bind caches still said they were valid. An ordinary → fallback → ordinary sequence with unchanged buffers could therefore omit the necessary restore. Both cache-validity flags are now cleared at the fallback bind.

What changed

  • gpu/vk/vk_draw.cpp: add dynamic binding stride only for ordinary vertex-input programs. At a fallback graphics-pipeline bind, clear last_vb_valid and vertex_set.on; the next ordinary draw restores stride even with unchanged buffers. Resolved buffer references stay cached. The fallback itself still binds no dummy vertex buffer. Topology/restart and shader translation are unchanged.
  • tests/test_fallback_pipeline_state.py: retain the production-construction checks and add the exact ordinary → fallback → ordinary cache sequence. Replay the actual pipeline-bind and vertex-cache blocks; model Vulkan state lifetime, not a driver. _CRT_SECURE_NO_WARNINGS precedes the first fixture include. The direct fallback library-construction case is extra fixture coverage: runtime fallback programs do not use pipeline libraries.
  • docs/design.md: describe the pipeline-state requirements and ordinary/fallback cache transition under the renderer architecture.
  • docs/tested-configurations.md: retain one stock Intel build row with the validation comparison and its limitations; diagnostic-counter runs stay in this description.

How it was tested

Arch Linux, kernel 7.2.15-1-omarchy, Core i9-13900K; GCC 16.2.1, Clang 23.1.3, Python 3.12.16. Base 51da0464b4cf0c7f56b81ebd6bb309325835e175; product/test commit 20635b20214826d1d6be63ac873b6fc80824414f. Checks run on 2026-11-12.

Initial construction checks — 20635b20

The test extracts build_graphics unmodified and copies its real Vulkan create-info at the pipeline-creation boundary. Program/state/driver collaborators are authored stand-ins; no Vulkan device or game is involved.

Source GCC Clang
Base Fails: fallback whole pipeline and vertex-input library demand dynamic stride Same
Initial change (20635b20) Passes all five pipeline creations Same
Incorrect alternative: omit all three states for fallback Fails: both topology and primitive restart are missing, in whole pipeline and library Same

The ordinary program keeps its binding, attribute and stride. Native mesh retains its task/mesh graphics stages without vertex-input states. The fallback has no vertex bindings/attributes or dynamic stride and keeps topology/restart.

fallback pipeline state: ordinary, native mesh and fallback pass (5 pipeline creations)
476 passed, 0 failed, 20 skipped in 267 s (linux, 8 at once)

The initial Windows-target syntax pass was insufficient: its MinGW headers had Microsoft-compatibility deprecation annotations disabled, and the real Windows job then rejected getenv in the Microsoft UCRT headers. The review fix and stronger check are described below. The original Linux suite result above belongs to 20635b20, not the review commit; no core dumps were left by the checks.

An initial version of the new fixture failed to compile because its direct packed-NVN include needed production-specific compiler conventions and it used a wrong driver enum spelling. That failure is retained separately; the fail-before rows above are the corrected fixture's runtime assertions, not compiler errors.

Initial Intel game runs — before the cache-transition review fix

Same system as above, Intel UHD 770 (Raptor Lake-S), Mesa ANV 26.2.5-arch1.1, Vulkan driver 0x6802005. The device has no mesh shaders, so Automatic uses the compute fallback. Both runs loaded the same copied child save in Kokiri Forest with the same initial cache, then stood still. Output 1920×1080 offscreen, captures 1280×720; Minimum first-start preset, frame generation off. Startup precompile, warm-up preparation and prediction were off (RX_VK_PRECOMPILE=0, RX_VK_NO_WARM=1, RX_VK_NO_PREDICT=1); each process had a 32 GiB memory limit. This avoids repeating the known Intel startup-precompile OOM, not a fix for it.

vk: no mesh shaders: task and mesh programs run as compute shaders (issue 640 fallback)
vk: Intel(R) Graphics (RPL-S), Vulkan 1.4, driver 0x6802005; mesh shaders no, host memory import yes, pipeline libraries yes
vk: driver: Intel open-source Mesa driver Mesa 26.2.5-arch1.1
vk: no window: presents render offscreen (1920x1080)
Build Local interval, 2026-11-12 (UTC−06) Wall / exit Captures Fallback draws in three queried frames Game crash / device loss / driver faults Virtual-core escape lines
Before: 51da0464, source 18ed87b5 07:09:58–07:17:49 471.1 s / 124 (requested stop) 42 43 / 43 / 43 0 / 0 / 0 1
After: 20635b20, source f6f995e7 07:25:15–07:33:06 470.7 s / 124 (requested stop) 45 43 / 42 / 45 0 / 0 / 0 3

The renderer's GPU pipeline-statistics capture reports nonzero vertex invocations and clipping input for fallback draws in all six sampled frames. Before: 63,081 / 74,685 / 70,290 vertex invocations; after: 74,685 / 62,898 / 74,685. Native task/mesh invocations are zero. This establishes that the changed path actually drew, rather than merely being available at startup. Both late control-status samples report play in the same forest, with loading and event false.

Other builds/checks were running. Load averages (1/5/15 minutes) were 10.81/13.73/29.55 → 63.20/60.55/47.84 before, and 48.46/54.11/49.78 → 16.84/35.54/44.43 after. No timing or speed comparison is made, and the differing virtual-core diagnostics are retained.

1 and 3: before the change, frames 4800 and 5040. 2 and 4: after it, the same frames. Kokiri Forest on the Intel UHD 770 with this iGPU's known false colours on both builds

Four labelled panels show frames 4800 and 5040 before and after. The same forest layout, foliage, Link/Navi and HUD remain visible; Intel's existing false colours remain on both builds. Five original captures and the composed panel were inspected. These stills are not a claim of correct colours, identical pixels or no transient defects.

The Khronos validation layer was absent for these initial runs. Available manifests were Mesa anti-lag, Mesa device selection and NVIDIA's implicit layers, not VK_LAYER_KHRONOS_validation. These initial runs therefore show continued fallback drawing, not VUID disappearance. The later stock-build validation comparison below supplies that missing check.

Review correction — 5da2d634

Credit to Joey-Magnus for both findings. The Vulkan dynamic-state lifetime rule requires dynamic state to be set after the last pipeline bind that made it static. Native mesh omits vertex-input state and does not disturb it; the fallback includes that state with no bindings. Among vkCmdBindVertexBuffers2 values, stride is the affected dynamic pipeline state, not buffer handles, offsets or bound sizes. Both skip flags must be invalidated; the resolved-buffer lookup cache, index-buffer cache and unrelated dynamic-state caches remain usable.

Local checks for the correction

  • GCC 16.2.1 and Clang 23.1.3: the reviewed e76c32dd fails the third-draw rebind assertion; 5da2d634 passes. Keeping either one of the two cache flags valid also fails. The original stride and topology/restart negative controls remain and still fail as intended.
  • The sequence also checks that repeated ordinary draws and an intervening native mesh draw do not add redundant binds, and that the resolved buffer is looked up only once. This is an authored state-lifetime model around production cache decisions, not driver validation.
  • Complete local source check: 475 passed, 1 failed, 20 skipped. All four C++ unit tests pass. The one failure is the known release-script race the reviewer named as flaky (test_nightly_two_runs_at_once_end_at_the_newer_commit, checksum publication after five uploads). It is retained, not rerun or called green.
  • The repaired Windows-target check rejects the old generated fixture, including real getenv deprecation errors, and accepts the fix. It enables MinGW's compatibility annotations and treats deprecation warnings as errors, and checks for the CRT define before the first include. It does not silently supply that define itself. Ten new rule tests and eleven existing check-tool tests pass. This local compile is still syntax-only.

Real Windows checks on the fork, before this push

For 5da2d634, the project's Python source checks (windows-2022, part 1/2) and (part 2/2) both passed on the fork's scratch PR #3 before the commit was pushed here. The Windows Vulkan back ends, DLSS/FSR link check, SPIR-V fixtures, Builder package, dav1d and actionlint also passed there. The two Linux parts and AppImage job were still running when those results were read. This pull request's own run is the authoritative check; the fork result is not a claim that this PR's current run is green.

Real Intel cache-transition counters

Same Arch Linux / Core i9-13900K / Intel UHD 770 / Mesa ANV 26.2.5-arch1.1 system and Kokiri Forest child save as above. One ordinary windowless run per renderer, identical warmed save/cache and settings, precompile off, 32 GiB process limit. The host is unchanged 20635b20 in both copies; only the renderer is rebuilt from e76c32dd or 5da2d634, with identical local-only counters. The two renderer sources differ only by the six committed invalidation lines. No counters are included in this commit.

The counters follow an ordinary binding path, a fallback graphics bind, and a return to the same guest buffer addresses/sizes/strides and pool generation within that recording. They count whether the real vertex-buffer bind call is recorded, without changing the cache decision. Values below end at each run's last logged checkpoint, not necessarily its final draw.

Renderer Interval on 2026-11-12 (UTC−06) Fallback binds Same-buffer returns Rebinds recorded Restores skipped Game crash / device loss / GPU faults
e76c32dd 09:14:03–09:18:04 45,087 2,829 0 2,829 0 / 0 / 0
5da2d634 09:18:05–09:22:06 45,992 2,880 2,880 0 0 / 0 / 0

Both stopped at the requested limit (exit 124), recorded 27 captures, and had no virtual-core escape or driver-error lines. Both late status samples report the same forest in play, with loading/event false. The runs were not quiet: load averages were 76.03/36.34/22.38 → 13.47/24.84/21.05 before, and 12.63/24.48/20.95 → 8.53/15.41/17.99 after. Exclusive holds can pause Intel games too, so the verdict uses counters, not wall time; no speed comparison is made.

The inspected frame-3240 pair shows the same forest/HUD with the pre-existing false colours.

Kokiri Forest on the Intel UHD 770: 1, the reviewed renderer; 2, the fixed renderer. The same forest and HUD, with this iGPU's known false colours on both

This shows continued drawing, not a visual cure for the separate Intel colour problem. The binding counters establish the missing restore before and the emitted restores after; they are not validation-layer error counts. The later stock-build validation comparison below checks VUID disappearance directly.

Khronos validation comparison — stock builds, before rebase

The requested check is now complete: VUID-vkCmdDrawIndirect-pStrides-04913 occurs 968 times on main and zero times on the fixed branch. There are no new message IDs on the branch, and no branch error messages. The existing unused-fragment-output warning remains; this is not a claim that the game is free of validation warnings.

These runs used stock host and renderer binaries, without the earlier diagnostic counters: main 51da0464b4cf0c7f56b81ebd6bb309325835e175 (source 18ed87b5) and pre-rebase branch b7d175e4f2e6f5167257cc2e23964ed07b1dd58c (source 43edcbeb). The validation counts belong to those two commits, not the rebased head.

Same Intel UHD 770, Mesa ANV 26.2.5-arch1.1, Arch Linux kernel 7.2.15-1-omarchy and Core i9-13900K as above; same copied warmed child save and initial cache, Kokiri Forest, 1920×1080 offscreen, Minimum preset, FG off, 32 GiB process limit. Precompile, warm-up preparation and prediction stayed off. Each run reached the forest and accumulated 60 seconds of sampled active play with progressing frames and loading/event false before a normal requested quit. Start-up/loading messages are included in the whole-run columns; the scene-only columns isolate those play windows.

Layer ID (MessageID) Severity Main 51da0464, whole run Branch b7d175e4, whole run Main, 60 s of play Branch, 60 s of play
VUID-vkCmdDrawIndirect-pStrides-04913 (0x3252797) Error 968 0 957 0
Undefined-Value-ShaderOutputNotConsumed-DynamicRendering (0x46877e3e) Warning 120,333 145,510 114,916 121,291
CURRENT-VALIDATION-ENABLED (0xfd7b2292) Information 1 1 0 0
WARNING-cache-file-error (0xb8515d13) Information 1 1 0 0

This table contains every distinct layer message, including its two information messages. Counts are of JSON message records, not occurrences of the ID in repeated specification text. All first complete messages were retained. The warning says the fragment shader writes frag_color0 at location 0 without a corresponding colour attachment; it appears on both builds. Despite its name, WARNING-cache-file-error has Information severity: the per-copy shader-validation cache did not yet exist. CURRENT-VALIDATION-ENABLED confirms core, stateless-parameter, object-lifetime and thread-safety checks, plus handle wrapping.

The layer was originally absent from the system and project dependencies. For these runs, Arch's vulkan-validation-layers 1.4.363.0-1 package was downloaded from the distribution mirror, its SHA-256 matched against the local sync database, and it was privately unpacked with no system installation. The loader reported the private Khronos layer inserted for both the instance and device and enabled by the application. The game used RX_VK_VALIDATION=1; the layer settings included khronos_validation.enable_message_limit = false, core validation enabled, and all error/warning/performance/information reports. The 968 repeated target errors in the control are also direct evidence that the default duplicate cap was not hiding them.

Stock run Interval on 2026-11-12 (UTC−06) Exit Game crash / device loss / GPU faults Captures Virtual-core escape lines
Main 51da0464 11:29:37–11:31:45 0 0 / 0 / 0 30 2
Branch b7d175e4 11:41:08–11:42:59 0 0 / 0 / 0 29 0

The final captures on both runs still show the forest and HUD with the known Intel false colours. These were ordinary shared-machine runs, not exclusive performance holds: loads were 126.63/108.22/67.84 → 140.24/118.03/76.42 on main and 31.16/66.61/70.58 → 22.50/52.59/65.06 on the branch. Other exclusive holds can pause Intel games too. The conclusion uses the message counts and sampled active-play windows, not wall time or a speed comparison.

Rebase and documentation review

Rebased onto freshly fetched main 65a4d874fd83a2c8fe2e230befb144f1668b900c on a new local branch, leaving the old branch at b7d175e4 untouched. Only docs/tested-configurations.md conflicted: both sides' independent rows were preserved in both replayed documentation commits. Range-diff shows the two code/test commits unchanged; the two old documentation commits differ only in surrounding table context, with identical added/removed payloads.

A separate documentation commit, 5dbdfe0b860c4033229b6a2b1441c7963eaca91d, keeps just one reproducible stock-build row for this PR, moves the pipeline-state explanation to docs/design.md, and records the validation run. The diagnostic-counter history stays here rather than in the tested-configurations table.

On that rebased head, the fixture passes with GCC 16.2.1 and Clang 23.1.3. With both compilers it rejects the new-main source at the missing-stride assertion and the pre-cache-fix source at the third-draw rebind assertion. The complete game-free suite reports 501 passed, 2 failed, 20 skipped; all four C++ unit tests pass. The two failures are inherited from the fetched base: test_program_cache_one_variant.py fails to compile its extracted cache code because the fixture lacks the new zstd declarations, and test_readme_menu_rows.py finds the newly added “Skip opening” row missing from the README table. The failing fixtures and all compiled/readme inputs are byte-identical to the base. This is not the earlier release-script race, and the full suite is not green.

The pre-push check passes. Main advanced during the checks; the branch still merged cleanly at that check. No game run or Windows CI result is claimed for this rebased head; the runtime evidence remains pinned to the stock pre-rebase builds above, and this pull request's own CI remains authoritative.

Not tested

  • Gameplay on the rebased head: the stock validation comparison is explicitly pinned to pre-rebase 51da0464 and b7d175e4.
  • Windows gameplay. The revised game-free fixture is covered by the fork's Windows source-check jobs described above; this PR's own run remains authoritative.
  • AMD, Pascal/Maxwell, Intel Arc, Steam Deck, native mesh gameplay, a real window/swapchain, physical input, sound or a long session.
  • Performance. These checks do not establish a speed improvement or repair Intel's separate colour/startup-memory problems.

Risks

The change affects fallback pipeline state on every driver. The regression checks preserve the states the fallback still uses, plus ordinary and native mesh behavior, but cannot replace execution on other drivers. Existing pipeline keys and shader translation are unchanged.

Overlaps

At initial submission, no competing pull request naming #2494 or “dynamic stride” was found and no product-code conflict was reported. That initial overlap check found competing additions at the top of docs/tested-configurations.md with #2414, #2715, #2738, #2755, #2756, #2757, #2759, #2769, #2771 and #2788. Each hunk was read: our two Intel rows meet independent measurements, so both sets should be retained when either lands. This does not duplicate the other work on fallback dispatches, program caching or Intel compilation.

Assisted-by: GPT-6 Astra
Co-Authored-By: GPT-6 Astra [email protected]

Head note: 711c6585 is 805eb0b3 rebased onto main 3c2d80cf to pick up main's test fixes (#2861, #2883). All five commits are unchanged (git range-diff marks each =); the measurements above were made on 805eb0b3 and earlier heads as stated, and no game run was repeated for the rebase.

> [@mooki000](https://github.com/mooki000) opened this pull request on GitHub · 2026-11-12 13:42 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2796) > **Merged on GitHub** in https://git.magicbean.dev/MagicBeanDev/Hookshot/commit/9b6072cd4b373bdd9ff3a8e056b7dc2b7dc359a5 GPUs using the compute mesh fallback were asked for a dynamic vertex-buffer stride even though those draws bind no vertex buffers. This removes that unused requirement, keeps topology and primitive restart dynamic, and restores dynamic strides before returning to an ordinary draw. **Head `805eb0b3`:** the five commits rebased onto main `15d4ae8c`; the changed lines of both code commits are identical to `5dbdfe0b` (on `65a4d874`), which is the head the fork's Windows run and the full-suite figures below were made on. The focused test passes on the new base too. **What this is for:** the Vulkan state error reported in #2494/#1001 and the ordinary-draw cache transition Joey-Magnus found in review. **What is measured:** production construction/cache decisions with GCC and Clang, earlier fork Windows source checks, Intel UHD 770 game counters (2,829 omitted restores before; all 2,880 returns rebound after), and stock-build Khronos validation: **VUID-04913 968 → 0, with no new message IDs**. The existing fragment-output warning remains. **Not measured:** performance, Windows gameplay, other GPUs' gameplay, or gameplay on the rebased head. Refs #2494. Part of #1001; this does not close the wider no-mesh-shader tracker. ## Problem - The fallback's generated vertex shader reads records by vertex index. `build_graphics` already gives it no vertex bindings or attributes. - The same function nevertheless added `VK_DYNAMIC_STATE_VERTEX_INPUT_BINDING_STRIDE` to every non-native-mesh pipeline. - The fallback issues `vkCmdDrawIndirect` and returns before the ordinary vertex-buffer/stride binding code. This is the unset dynamic state reported as `VUID-vkCmdDrawIndirect-pStrides-04913`. - The owner closed #2494 to collect it in #1001, not because it was fixed. This change does not claim that the validation error caused a crash. - Joey-Magnus caught the second step: binding the now-static-stride fallback invalidates previous dynamic strides, while two vertex-bind caches still said they were valid. An ordinary → fallback → ordinary sequence with unchanged buffers could therefore omit the necessary restore. Both cache-validity flags are now cleared at the fallback bind. ## What changed - `gpu/vk/vk_draw.cpp`: add dynamic binding stride only for ordinary vertex-input programs. At a fallback graphics-pipeline bind, clear `last_vb_valid` and `vertex_set.on`; the next ordinary draw restores stride even with unchanged buffers. Resolved buffer references stay cached. The fallback itself still binds no dummy vertex buffer. Topology/restart and shader translation are unchanged. - `tests/test_fallback_pipeline_state.py`: retain the production-construction checks and add the exact ordinary → fallback → ordinary cache sequence. Replay the actual pipeline-bind and vertex-cache blocks; model Vulkan state lifetime, not a driver. `_CRT_SECURE_NO_WARNINGS` precedes the first fixture include. The direct fallback library-construction case is extra fixture coverage: runtime fallback programs do not use pipeline libraries. - `docs/design.md`: describe the pipeline-state requirements and ordinary/fallback cache transition under the renderer architecture. - `docs/tested-configurations.md`: retain one stock Intel build row with the validation comparison and its limitations; diagnostic-counter runs stay in this description. ## How it was tested Arch Linux, kernel 7.2.15-1-omarchy, Core i9-13900K; GCC 16.2.1, Clang 23.1.3, Python 3.12.16. Base `51da0464b4cf0c7f56b81ebd6bb309325835e175`; product/test commit `20635b20214826d1d6be63ac873b6fc80824414f`. Checks run on 2026-11-12. ### Initial construction checks — `20635b20` The test extracts `build_graphics` unmodified and copies its real Vulkan create-info at the pipeline-creation boundary. Program/state/driver collaborators are authored stand-ins; no Vulkan device or game is involved. | Source | GCC | Clang | |---|---|---| | Base | Fails: fallback whole pipeline and vertex-input library demand dynamic stride | Same | | Initial change (`20635b20`) | Passes all five pipeline creations | Same | | Incorrect alternative: omit all three states for fallback | Fails: both topology and primitive restart are missing, in whole pipeline and library | Same | The ordinary program keeps its binding, attribute and stride. Native mesh retains its task/mesh graphics stages without vertex-input states. The fallback has no vertex bindings/attributes or dynamic stride and keeps topology/restart. ``` fallback pipeline state: ordinary, native mesh and fallback pass (5 pipeline creations) 476 passed, 0 failed, 20 skipped in 267 s (linux, 8 at once) ``` The initial Windows-target syntax pass was insufficient: its MinGW headers had Microsoft-compatibility deprecation annotations disabled, and the real Windows job then rejected `getenv` in the Microsoft UCRT headers. The review fix and stronger check are described below. The original Linux suite result above belongs to `20635b20`, not the review commit; no core dumps were left by the checks. An initial version of the new fixture failed to compile because its direct packed-NVN include needed production-specific compiler conventions and it used a wrong driver enum spelling. That failure is retained separately; the fail-before rows above are the corrected fixture's **runtime assertions**, not compiler errors. ### Initial Intel game runs — before the cache-transition review fix Same system as above, Intel UHD 770 (Raptor Lake-S), **Mesa ANV 26.2.5-arch1.1**, Vulkan driver `0x6802005`. The device has no mesh shaders, so Automatic uses the compute fallback. Both runs loaded the same copied child save in **Kokiri Forest** with the same initial cache, then stood still. Output 1920×1080 offscreen, captures 1280×720; Minimum first-start preset, frame generation off. Startup precompile, warm-up preparation and prediction were off (`RX_VK_PRECOMPILE=0`, `RX_VK_NO_WARM=1`, `RX_VK_NO_PREDICT=1`); each process had a 32 GiB memory limit. This avoids repeating the known Intel startup-precompile OOM, not a fix for it. ``` vk: no mesh shaders: task and mesh programs run as compute shaders (issue 640 fallback) vk: Intel(R) Graphics (RPL-S), Vulkan 1.4, driver 0x6802005; mesh shaders no, host memory import yes, pipeline libraries yes vk: driver: Intel open-source Mesa driver Mesa 26.2.5-arch1.1 vk: no window: presents render offscreen (1920x1080) ``` | Build | Local interval, 2026-11-12 (UTC−06) | Wall / exit | Captures | Fallback draws in three queried frames | Game crash / device loss / driver faults | Virtual-core escape lines | |---|---|---|---:|---|---|---:| | Before: `51da0464`, source `18ed87b5` | 07:09:58–07:17:49 | 471.1 s / 124 (requested stop) | 42 | 43 / 43 / 43 | 0 / 0 / 0 | 1 | | After: `20635b20`, source `f6f995e7` | 07:25:15–07:33:06 | 470.7 s / 124 (requested stop) | 45 | 43 / 42 / 45 | 0 / 0 / 0 | 3 | The renderer's GPU pipeline-statistics capture reports nonzero vertex invocations and clipping input for fallback draws in **all six sampled frames**. Before: 63,081 / 74,685 / 70,290 vertex invocations; after: 74,685 / 62,898 / 74,685. Native task/mesh invocations are zero. This establishes that the changed path actually drew, rather than merely being available at startup. Both late control-status samples report play in the same forest, with loading and event false. Other builds/checks were running. Load averages (1/5/15 minutes) were 10.81/13.73/29.55 → 63.20/60.55/47.84 before, and 48.46/54.11/49.78 → 16.84/35.54/44.43 after. **No timing or speed comparison is made**, and the differing virtual-core diagnostics are retained. ![1 and 3: before the change, frames 4800 and 5040. 2 and 4: after it, the same frames. Kokiri Forest on the Intel UHD 770 with this iGPU's known false colours on both builds](https://gist.githubusercontent.com/mooki000/b86bd3f35969803fb032f71f035844f8/raw/59dbd892c70426dc97b019940dd03f4ae2084553/hookshot-fallback-stride-intel-before-after.jpg) Four labelled panels show frames 4800 and 5040 before and after. The same forest layout, foliage, Link/Navi and HUD remain visible; Intel's existing false colours remain on both builds. Five original captures and the composed panel were inspected. These stills are not a claim of correct colours, identical pixels or no transient defects. The Khronos validation layer was absent for these initial runs. Available manifests were Mesa anti-lag, Mesa device selection and NVIDIA's implicit layers, not `VK_LAYER_KHRONOS_validation`. These initial runs therefore show continued fallback drawing, not VUID disappearance. The later stock-build validation comparison below supplies that missing check. ## Review correction — `5da2d634` Credit to **Joey-Magnus** for both findings. The [Vulkan dynamic-state lifetime rule](https://docs.vulkan.org/spec/latest/chapters/pipelines.html#dynamic-state-lifetime) requires dynamic state to be set after the last pipeline bind that made it static. Native mesh omits vertex-input state and does not disturb it; the fallback includes that state with no bindings. Among `vkCmdBindVertexBuffers2` values, **stride** is the affected dynamic pipeline state, not buffer handles, offsets or bound sizes. Both skip flags must be invalidated; the resolved-buffer lookup cache, index-buffer cache and unrelated dynamic-state caches remain usable. ### Local checks for the correction - GCC 16.2.1 and Clang 23.1.3: the reviewed `e76c32dd` fails the third-draw rebind assertion; `5da2d634` passes. Keeping either one of the two cache flags valid also fails. The original stride and topology/restart negative controls remain and still fail as intended. - The sequence also checks that repeated ordinary draws and an intervening native mesh draw do not add redundant binds, and that the resolved buffer is looked up only once. This is an authored state-lifetime model around production cache decisions, not driver validation. - Complete local source check: **475 passed, 1 failed, 20 skipped**. All four C++ unit tests pass. The one failure is **the known release-script race the reviewer named as flaky** (`test_nightly_two_runs_at_once_end_at_the_newer_commit`, checksum publication after five uploads). It is retained, not rerun or called green. - The repaired Windows-target check rejects the old generated fixture, including real `getenv` deprecation errors, and accepts the fix. It enables MinGW's compatibility annotations and treats deprecation warnings as errors, and checks for the CRT define before the first include. It does not silently supply that define itself. Ten new rule tests and eleven existing check-tool tests pass. This local compile is still syntax-only. ### Real Windows checks on the fork, before this push For `5da2d634`, the project's **Python source checks (windows-2022, part 1/2)** and **(part 2/2)** both passed on [the fork's scratch PR #3](https://github.com/mooki000/Hookshot/pull/3) before the commit was pushed here. The Windows Vulkan back ends, DLSS/FSR link check, SPIR-V fixtures, Builder package, dav1d and actionlint also passed there. The two Linux parts and AppImage job were still running when those results were read. **This pull request's own run is the authoritative check; the fork result is not a claim that this PR's current run is green.** ### Real Intel cache-transition counters Same Arch Linux / Core i9-13900K / Intel UHD 770 / Mesa ANV 26.2.5-arch1.1 system and Kokiri Forest child save as above. One ordinary windowless run per renderer, identical warmed save/cache and settings, precompile off, 32 GiB process limit. **The host is unchanged `20635b20` in both copies**; only the renderer is rebuilt from `e76c32dd` or `5da2d634`, with identical local-only counters. The two renderer sources differ only by the six committed invalidation lines. No counters are included in this commit. The counters follow an ordinary binding path, a fallback graphics bind, and a return to the same guest buffer addresses/sizes/strides and pool generation within that recording. They count whether the real vertex-buffer bind call is recorded, without changing the cache decision. Values below end at each run's last logged checkpoint, not necessarily its final draw. | Renderer | Interval on 2026-11-12 (UTC−06) | Fallback binds | Same-buffer returns | Rebinds recorded | Restores skipped | Game crash / device loss / GPU faults | |---|---|---:|---:|---:|---:|---| | `e76c32dd` | 09:14:03–09:18:04 | 45,087 | 2,829 | 0 | 2,829 | 0 / 0 / 0 | | `5da2d634` | 09:18:05–09:22:06 | 45,992 | 2,880 | 2,880 | 0 | 0 / 0 / 0 | Both stopped at the requested limit (exit 124), recorded 27 captures, and had no virtual-core escape or driver-error lines. Both late status samples report the same forest in play, with loading/event false. The runs were not quiet: load averages were 76.03/36.34/22.38 → 13.47/24.84/21.05 before, and 12.63/24.48/20.95 → 8.53/15.41/17.99 after. Exclusive holds can pause Intel games too, so **the verdict uses counters, not wall time**; no speed comparison is made. The inspected frame-3240 pair shows the same forest/HUD with the pre-existing false colours. ![Kokiri Forest on the Intel UHD 770: 1, the reviewed renderer; 2, the fixed renderer. The same forest and HUD, with this iGPU's known false colours on both](https://gist.githubusercontent.com/mooki000/b86bd3f35969803fb032f71f035844f8/raw/1eae2688d3d053a1e386b89e35b0fcb9952a7023/hookshot-fallback-stride-review-rebinds.jpg) This shows continued drawing, not a visual cure for the separate Intel colour problem. The binding counters establish the missing restore before and the emitted restores after; **they are not validation-layer error counts**. The later stock-build validation comparison below checks VUID disappearance directly. ## Khronos validation comparison — stock builds, before rebase The requested check is now complete: **VUID-vkCmdDrawIndirect-pStrides-04913 occurs 968 times on main and zero times on the fixed branch**. There are **no new message IDs** on the branch, and no branch error messages. The existing unused-fragment-output warning remains; this is not a claim that the game is free of validation warnings. These runs used stock host and renderer binaries, without the earlier diagnostic counters: main `51da0464b4cf0c7f56b81ebd6bb309325835e175` (source `18ed87b5`) and pre-rebase branch `b7d175e4f2e6f5167257cc2e23964ed07b1dd58c` (source `43edcbeb`). **The validation counts belong to those two commits, not the rebased head.** Same Intel UHD 770, Mesa ANV 26.2.5-arch1.1, Arch Linux kernel 7.2.15-1-omarchy and Core i9-13900K as above; same copied warmed child save and initial cache, Kokiri Forest, 1920×1080 offscreen, Minimum preset, FG off, 32 GiB process limit. Precompile, warm-up preparation and prediction stayed off. Each run reached the forest and accumulated 60 seconds of sampled active play with progressing frames and loading/event false before a normal requested quit. Start-up/loading messages are included in the whole-run columns; the scene-only columns isolate those play windows. | Layer ID (`MessageID`) | Severity | Main `51da0464`, whole run | Branch `b7d175e4`, whole run | Main, 60 s of play | Branch, 60 s of play | |---|---|---:|---:|---:|---:| | `VUID-vkCmdDrawIndirect-pStrides-04913` (`0x3252797`) | Error | 968 | 0 | 957 | 0 | | `Undefined-Value-ShaderOutputNotConsumed-DynamicRendering` (`0x46877e3e`) | Warning | 120,333 | 145,510 | 114,916 | 121,291 | | `CURRENT-VALIDATION-ENABLED` (`0xfd7b2292`) | Information | 1 | 1 | 0 | 0 | | `WARNING-cache-file-error` (`0xb8515d13`) | Information | 1 | 1 | 0 | 0 | This table contains every distinct layer message, including its two information messages. Counts are of JSON message records, not occurrences of the ID in repeated specification text. All first complete messages were retained. The warning says the fragment shader writes `frag_color0` at location 0 without a corresponding colour attachment; it appears on both builds. Despite its name, `WARNING-cache-file-error` has Information severity: the per-copy shader-validation cache did not yet exist. `CURRENT-VALIDATION-ENABLED` confirms core, stateless-parameter, object-lifetime and thread-safety checks, plus handle wrapping. The layer was originally absent from the system and project dependencies. For these runs, Arch's **vulkan-validation-layers 1.4.363.0-1** package was downloaded from the distribution mirror, its SHA-256 matched against the local sync database, and it was privately unpacked with no system installation. The loader reported the private Khronos layer inserted for both the instance and device and enabled **by the application**. The game used `RX_VK_VALIDATION=1`; the layer settings included `khronos_validation.enable_message_limit = false`, core validation enabled, and all error/warning/performance/information reports. The 968 repeated target errors in the control are also direct evidence that the default duplicate cap was not hiding them. | Stock run | Interval on 2026-11-12 (UTC−06) | Exit | Game crash / device loss / GPU faults | Captures | Virtual-core escape lines | |---|---|---:|---|---:|---:| | Main `51da0464` | 11:29:37–11:31:45 | 0 | 0 / 0 / 0 | 30 | 2 | | Branch `b7d175e4` | 11:41:08–11:42:59 | 0 | 0 / 0 / 0 | 29 | 0 | The final captures on both runs still show the forest and HUD with the known Intel false colours. These were ordinary shared-machine runs, not exclusive performance holds: loads were 126.63/108.22/67.84 → 140.24/118.03/76.42 on main and 31.16/66.61/70.58 → 22.50/52.59/65.06 on the branch. Other exclusive holds can pause Intel games too. The conclusion uses the message counts and sampled active-play windows, **not wall time or a speed comparison**. ## Rebase and documentation review Rebased onto freshly fetched main `65a4d874fd83a2c8fe2e230befb144f1668b900c` on a new local branch, leaving the old branch at `b7d175e4` untouched. Only `docs/tested-configurations.md` conflicted: both sides' independent rows were preserved in both replayed documentation commits. Range-diff shows the two code/test commits unchanged; the two old documentation commits differ only in surrounding table context, with identical added/removed payloads. A separate documentation commit, `5dbdfe0b860c4033229b6a2b1441c7963eaca91d`, keeps just one reproducible stock-build row for this PR, moves the pipeline-state explanation to `docs/design.md`, and records the validation run. The diagnostic-counter history stays here rather than in the tested-configurations table. On that rebased head, the fixture passes with GCC 16.2.1 and Clang 23.1.3. With both compilers it rejects the new-main source at the missing-stride assertion and the pre-cache-fix source at the third-draw rebind assertion. The complete game-free suite reports **501 passed, 2 failed, 20 skipped**; all four C++ unit tests pass. The two failures are inherited from the fetched base: `test_program_cache_one_variant.py` fails to compile its extracted cache code because the fixture lacks the new zstd declarations, and `test_readme_menu_rows.py` finds the newly added “Skip opening” row missing from the README table. The failing fixtures and all compiled/readme inputs are byte-identical to the base. This is **not** the earlier release-script race, and the full suite is not green. The pre-push check passes. Main advanced during the checks; the branch still merged cleanly at that check. No game run or Windows CI result is claimed for this rebased head; the runtime evidence remains pinned to the stock pre-rebase builds above, and this pull request's own CI remains authoritative. ## Not tested - Gameplay on the rebased head: the stock validation comparison is explicitly pinned to pre-rebase `51da0464` and `b7d175e4`. - Windows gameplay. The revised game-free fixture is covered by the fork's Windows source-check jobs described above; this PR's own run remains authoritative. - AMD, Pascal/Maxwell, Intel Arc, Steam Deck, native mesh gameplay, a real window/swapchain, physical input, sound or a long session. - Performance. These checks do not establish a speed improvement or repair Intel's separate colour/startup-memory problems. ## Risks The change affects fallback pipeline state on every driver. The regression checks preserve the states the fallback still uses, plus ordinary and native mesh behavior, but cannot replace execution on other drivers. Existing pipeline keys and shader translation are unchanged. ## Overlaps At initial submission, no competing pull request naming #2494 or “dynamic stride” was found and no product-code conflict was reported. That initial overlap check found competing additions at the top of `docs/tested-configurations.md` with #2414, #2715, #2738, #2755, #2756, #2757, #2759, #2769, #2771 and #2788. Each hunk was read: our two Intel rows meet independent measurements, so both sets should be retained when either lands. This does not duplicate the other work on fallback dispatches, program caching or Intel compilation. Assisted-by: GPT-6 Astra Co-Authored-By: GPT-6 Astra <[email protected]> **Head note:** `711c6585` is `805eb0b3` rebased onto main `3c2d80cf` to pick up main's test fixes (#2861, #2883). All five commits are unchanged (`git range-diff` marks each `=`); the measurements above were made on `805eb0b3` and earlier heads as stated, and no game run was repeated for the rebase. <!-- github-sync:item:2796 -->
The fallback vertex shader reads records without vertex bindings. Keep its dynamic topology and primitive restart, but only require dynamic binding stride for ordinary vertex-input programs.

Exercise production pipeline construction for ordinary, native mesh and fallback programs, including the vertex-input library. The test rejects both the previous stride requirement and dropping topology/restart with it.

Refs #2494

Refs #1001

Assisted-by: GPT-6 Astra

Co-Authored-By: GPT-6 Astra <[email protected]>
Record the windowless Kokiri Forest runs before and after the stride-state fix, with real fallback GPU query work. Keep the existing false colours, virtual-core diagnostics and unavailable Vulkan validation layer explicit.

Assisted-by: GPT-6 Astra

Co-Authored-By: GPT-6 Astra <[email protected]>
github-sync commented 2026-11-12 14:48:35 +00:00
Author
Owner

@Joey-Magnus requested changes on GitHub · 2026-11-12 14:35 UTC · original

I merged this into current main. tests/test_fallback_pipeline_state.py passes on Linux but fails on windows-2022, and that's this PR. The fixture includes compat/hookshot_env.h, MSVC marks getenv deprecated, and -Werror stops the build. It needs #define _CRT_SECURE_NO_WARNINGS like the other fixtures (tests/draw_stream_visibility_test.cpp:1). The linux test_release_scripts failure is the known flaky one.

There's also a side effect in vk_draw.cpp. With the stride now static in the fallback pipeline, binding that pipeline invalidates the stride state. The draw path caches the vertex bind (vertex_set_same, and last_vb_valid near line 7010). So after an ordinary draw, a fallback draw, then an ordinary draw with the same buffers, the third draw skips vkCmdBindVertexBuffers2 and runs without valid strides. That's the same VUID-04913, moved to the ordinary draw. Clearing last_vb_valid and vertex_set.on when a fallback pipeline is bound (near line 6781) closes it. The change at line 2158 is right otherwise: fallback programs never use pipeline libraries.

> [@Joey-Magnus](https://github.com/Joey-Magnus) requested changes on GitHub · 2026-11-12 14:35 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2796#pullrequestreview-5876259318) I merged this into current main. tests/test_fallback_pipeline_state.py passes on Linux but fails on windows-2022, and that's this PR. The fixture includes compat/hookshot_env.h, MSVC marks getenv deprecated, and -Werror stops the build. It needs `#define _CRT_SECURE_NO_WARNINGS` like the other fixtures (tests/draw_stream_visibility_test.cpp:1). The linux test_release_scripts failure is the known flaky one. There's also a side effect in vk_draw.cpp. With the stride now static in the fallback pipeline, binding that pipeline invalidates the stride state. The draw path caches the vertex bind (vertex_set_same, and last_vb_valid near line 7010). So after an ordinary draw, a fallback draw, then an ordinary draw with the same buffers, the third draw skips vkCmdBindVertexBuffers2 and runs without valid strides. That's the same VUID-04913, moved to the ordinary draw. Clearing last_vb_valid and vertex_set.on when a fallback pipeline is bound (near line 6781) closes it. The change at line 2158 is right otherwise: fallback programs never use pipeline libraries. <!-- github-sync:r:5876259318 -->
github-sync commented 2026-11-12 14:48:35 +00:00
Author
Owner

@mooki000 commented on GitHub · 2026-11-12 14:38 UTC · original

Thanks, both points are right. The fixture gets the define, and I am fixing the cached vertex bind where a fallback pipeline is bound, with your three-draw sequence (ordinary, fallback, ordinary with the same buffers) added to the test so it fails before the fix. The Windows job will run on my fork before I push here.

> [@mooki000](https://github.com/mooki000) commented on GitHub · 2026-11-12 14:38 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2796#issuecomment-6826614953) Thanks, both points are right. The fixture gets the define, and I am fixing the cached vertex bind where a fallback pipeline is bound, with your three-draw sequence (ordinary, fallback, ordinary with the same buffers) added to the test so it fails before the fix. The Windows job will run on my fork before I push here. <!-- github-sync:c:6826614953 -->
Joey-Magnus caught that binding the static-stride fallback invalidates dynamic strides while both vertex-bind skip caches remained valid. Clear both validity flags at the fallback graphics bind, retaining resolved buffer references.

Replay the production bind/cache decisions for ordinary, fallback, ordinary draws with identical buffers; preserve native mesh and redundant ordinary-bind controls. Define _CRT_SECURE_NO_WARNINGS before fixture headers so the Windows CRT warnings remain compatible with -Werror.

Refs #2796

Assisted-by: GPT-6 Astra

Co-Authored-By: GPT-6 Astra <[email protected]>
Record the paired renderer-only runs for the #2796 cache review: 2829 same-buffer returns skipped the restore before; all 2880 observed returns rebind after. Keep the common host, local-only counters, known Intel colours and unavailable validation layer explicit.

Assisted-by: GPT-6 Astra

Co-Authored-By: GPT-6 Astra <[email protected]>
github-sync commented 2026-11-12 15:53:35 +00:00
Author
Owner

@mooki000 commented on GitHub · 2026-11-12 15:45 UTC · original

Thanks, you were right on both points. 5da2d634:

  • The fixture has _CRT_SECURE_NO_WARNINGS before its includes.
  • The cache transition: both vertex-bind skip flags (last_vb_valid, vertex_set.on) are cleared when a fallback graphics pipeline is bound, so the next ordinary draw binds its buffers and strides again; the resolved buffer references stay cached. Your ordinary, fallback, ordinary sequence is in the test: GCC and Clang reject the reviewed source and either one-flag-only fix, and pass the repair. The test replays the production cache decisions, not a driver.

In the game, Intel UHD 770 (Mesa ANV 26.2.5, Kokiri Forest, the same warmed save and cache, precompile off, local-only counters): the reviewed renderer skipped the bind on all 2,829 same-buffer returns to an ordinary draw after a fallback draw; the fixed one rebound on all 2,880, none skipped. No crash, device loss or GPU fault in either run, and the pictures match (this iGPU's known false colours on both). These are counts of recorded commands, not validation-layer output: whether the message itself is gone still needs someone with the layer (T-VUU4).

Checks: both windows-2022 parts passed on a run on my fork before the push, and this pull request's own run on 5da2d634 has every check green. Locally on Linux: 475 passed, 20 skipped, 1 failed, the release-script race you named as flaky. Our local Windows cross-check had missed the CRT warning because its headers had the deprecation annotations off; it now turns them on as errors.

A docs-only commit since (b7d175e4) adds the two counter runs to the tested-configurations table. The description is rewritten around the review fix.

> [@mooki000](https://github.com/mooki000) commented on GitHub · 2026-11-12 15:45 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2796#issuecomment-6828307252) Thanks, you were right on both points. `5da2d634`: - **The fixture** has `_CRT_SECURE_NO_WARNINGS` before its includes. - **The cache transition:** both vertex-bind skip flags (`last_vb_valid`, `vertex_set.on`) are cleared when a fallback graphics pipeline is bound, so the next ordinary draw binds its buffers and strides again; the resolved buffer references stay cached. Your ordinary, fallback, ordinary sequence is in the test: GCC and Clang reject the reviewed source and either one-flag-only fix, and pass the repair. The test replays the production cache decisions, not a driver. **In the game, Intel UHD 770** (Mesa ANV 26.2.5, Kokiri Forest, the same warmed save and cache, precompile off, local-only counters): the reviewed renderer skipped the bind on **all 2,829** same-buffer returns to an ordinary draw after a fallback draw; the fixed one rebound on **all 2,880, none skipped**. No crash, device loss or GPU fault in either run, and the pictures match (this iGPU's known false colours on both). These are counts of recorded commands, not validation-layer output: whether the message itself is gone still needs someone with the layer (T-VUU4). **Checks:** both `windows-2022` parts passed on a run on my fork before the push, and this pull request's own run on `5da2d634` has every check green. Locally on Linux: 475 passed, 20 skipped, 1 failed, the release-script race you named as flaky. Our local Windows cross-check had missed the CRT warning because its headers had the deprecation annotations off; it now turns them on as errors. A docs-only commit since (`b7d175e4`) adds the two counter runs to the tested-configurations table. The description is rewritten around the review fix. <!-- github-sync:c:6828307252 -->
github-sync commented 2026-11-12 16:56:17 +00:00
Author
Owner

@hckrman101 commented on docs/tested-configurations.md line 17 on GitHub · 2026-11-12 16:04 UTC · original

Nit: four rows from one machine, for one validation fix, is a lot for this table. Two of them use renderers with local-only counters that nobody else can build. The table is for configurations people can reproduce. I'd keep one row, the fixed build in Kokiri Forest (plus the validation-layer count once you have it), and leave the counter runs in the PR description, where they already are. That also makes the conflicts with the other PRs adding rows at the top smaller.

> [@hckrman101](https://github.com/hckrman101) commented on `docs/tested-configurations.md` line 17 on GitHub · 2026-11-12 16:04 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2796#discussion_r4552924232) Nit: four rows from one machine, for one validation fix, is a lot for this table. Two of them use renderers with local-only counters that nobody else can build. The table is for configurations people can reproduce. I'd keep one row, the fixed build in Kokiri Forest (plus the validation-layer count once you have it), and leave the counter runs in the PR description, where they already are. That also makes the conflicts with the other PRs adding rows at the top smaller. <!-- github-sync:rc:4552924232 -->
github-sync commented 2026-11-12 16:56:17 +00:00
Author
Owner

@hckrman101 reviewed on GitHub · 2026-11-12 16:04 UTC · original

Thanks, this is a careful one. The code is right and I'd merge it once there is one validation-layer run.

Joey's points (CHANGES_REQUESTED at e76c32dd): both fixed at 5da2d634.

  • The fixture now has _CRT_SECURE_NO_WARNINGS before its includes. Both windows-2022 parts pass on this PR's own run at b7d175e4, and the log shows test_fallback_pipeline_state.py running there.
  • The ordinary, fallback, ordinary cache hole is closed. Binding a fallback pipeline now clears last_vb_valid and vertex_set.on, so the next ordinary draw goes through vkCmdBindVertexBuffers2 again with its strides. That is the only place a static-stride pipeline enters this path: native mesh pipelines carry no vertex-input state, and the other resets (lines 4617/4714/7543/7570) were already there. Clearing at the bind and not at the draw is also correct when the fallback pipeline stays bound across several fallback draws.

The change itself. Only p.fallback programs lose the dynamic stride. Ordinary vertex-input pipelines keep it, and their bindings and attributes are untouched, so nothing can now draw with stride 0 or a stale stride. The fallback already declared no bindings or attributes, so with a static stride there is nothing left for the layer to check. Topology and primitive restart stay dynamic. Fallback programs never take the GPL path (line 2007), so the library case in the test is extra coverage, as the description says. This affects GPUs without mesh shaders (RDNA1 and older, Pascal/Maxwell, Intel iGPUs) and Navi 22 on Windows, where task + mesh programs use the fallback by default (#996).

The test is a real check, not a text match. It compiles the real build_graphics and the real pipeline-bind and vertex-bind blocks, then replays the three-draw sequence against a small model of when the stride is valid. Its text markers are unique in the file today. Good.

What's still missing (rule 3c). #2494 is a validation-layer finding with no visible symptom, so the only way to show the bug gone is the layer's count before and after. Your counters and screenshots show that the restore now happens and that Intel still draws, which is good evidence, but they are not the VUID count. One run with the reproduction from #2494 would settle it: RX_VK_VALIDATION=1, the Khronos layer with its duplicate-message limit off, about a minute in Kokiri Forest on any GPU without mesh shaders, counting VUID-vkCmdDrawIndirect-pStrides-04913 on main and on this branch. On Arch the layer is the vulkan-validation-layers package, and your UHD 770 takes the fallback path, so it would work there. Please also check that no new VUID shows up for the ordinary draws after a fallback draw. That is the hole Joey found, and the layer would catch it there.

Nit on the docs: see the inline comment on tested-configurations. The development.md paragraph is accurate, but it explains how the renderer works and sits under the environment-variable table. docs/design.md would suit it better, or it can just be the code comment.

CI is all green at b7d175e4, and the branch is up to date with main with no conflicts.

> [@hckrman101](https://github.com/hckrman101) reviewed on GitHub · 2026-11-12 16:04 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2796#pullrequestreview-5877746534) Thanks, this is a careful one. The code is right and I'd merge it once there is one validation-layer run. **Joey's points (CHANGES_REQUESTED at e76c32dd): both fixed at 5da2d634.** - The fixture now has `_CRT_SECURE_NO_WARNINGS` before its includes. Both windows-2022 parts pass on this PR's own run at b7d175e4, and the log shows `test_fallback_pipeline_state.py` running there. - The ordinary, fallback, ordinary cache hole is closed. Binding a fallback pipeline now clears `last_vb_valid` and `vertex_set.on`, so the next ordinary draw goes through `vkCmdBindVertexBuffers2` again with its strides. That is the only place a static-stride pipeline enters this path: native mesh pipelines carry no vertex-input state, and the other resets (lines 4617/4714/7543/7570) were already there. Clearing at the bind and not at the draw is also correct when the fallback pipeline stays bound across several fallback draws. **The change itself.** Only `p.fallback` programs lose the dynamic stride. Ordinary vertex-input pipelines keep it, and their bindings and attributes are untouched, so nothing can now draw with stride 0 or a stale stride. The fallback already declared no bindings or attributes, so with a static stride there is nothing left for the layer to check. Topology and primitive restart stay dynamic. Fallback programs never take the GPL path (line 2007), so the library case in the test is extra coverage, as the description says. This affects GPUs without mesh shaders (RDNA1 and older, Pascal/Maxwell, Intel iGPUs) and Navi 22 on Windows, where task + mesh programs use the fallback by default (#996). **The test** is a real check, not a text match. It compiles the real `build_graphics` and the real pipeline-bind and vertex-bind blocks, then replays the three-draw sequence against a small model of when the stride is valid. Its text markers are unique in the file today. Good. **What's still missing (rule 3c).** #2494 is a validation-layer finding with no visible symptom, so the only way to show the bug gone is the layer's count before and after. Your counters and screenshots show that the restore now happens and that Intel still draws, which is good evidence, but they are not the VUID count. One run with the reproduction from #2494 would settle it: `RX_VK_VALIDATION=1`, the Khronos layer with its duplicate-message limit off, about a minute in Kokiri Forest on any GPU without mesh shaders, counting `VUID-vkCmdDrawIndirect-pStrides-04913` on main and on this branch. On Arch the layer is the `vulkan-validation-layers` package, and your UHD 770 takes the fallback path, so it would work there. Please also check that no new VUID shows up for the ordinary draws after a fallback draw. That is the hole Joey found, and the layer would catch it there. Nit on the docs: see the inline comment on tested-configurations. The development.md paragraph is accurate, but it explains how the renderer works and sits under the environment-variable table. docs/design.md would suit it better, or it can just be the code comment. CI is all green at b7d175e4, and the branch is up to date with main with no conflicts. <!-- github-sync:r:5877746534 -->
github-sync closed this pull request 2026-11-12 19:02:22 +00:00
github-sync commented 2026-11-12 19:02:23 +00:00
Author
Owner

@mooki000 commented on GitHub · 2026-11-12 18:32 UTC · original

@hckrman101 the validation-layer run you asked for: 968 of that error on main, 0 on the branch, and no new message IDs. Counts are from stock builds of 51da0464 and b7d175e4, before the rebase; the second pair of columns is the 60 seconds of confirmed play in Kokiri Forest.

Layer ID (MessageID) Severity Main 51da0464, whole run Branch b7d175e4, whole run Main, 60 s of play Branch, 60 s of play
VUID-vkCmdDrawIndirect-pStrides-04913 (0x3252797) Error 968 0 957 0
Undefined-Value-ShaderOutputNotConsumed-DynamicRendering (0x46877e3e) Warning 120,333 145,510 114,916 121,291
CURRENT-VALIDATION-ENABLED (0xfd7b2292) Information 1 1 0 0
WARNING-cache-file-error (0xb8515d13) Information 1 1 0 0

Those are all the distinct messages the layer printed, not only the one we were after. One existing warning remains on both (an unused fragment output; it grows because more draws now run to completion), and the two information lines are the layer saying it is on and that its cache file did not exist yet.

Setup: Intel UHD 770, Mesa ANV 26.2.5, Arch Linux, the same warmed save and caches for both, precompile off, no window; Khronos validation 1.4.363 with core checks on and the duplicate-message limit off, loaded from an unpacked copy of the distribution's package (nothing installed). Both runs quit normally with no device loss or driver fault. The picture still has this chip's known false colours; that is a separate problem.

Also done: your documentation nits (one stock-build row in the table, the counter runs kept in the description, the implementation paragraph moved under the renderer section of docs/design.md), and a rebase, since today's merges had put rows at the same place in the table. Pushed as 805eb0b3 on main 15d4ae8c. The two code commits' changed lines are identical to the reviewed ones; the fork's Windows run on the same patches (on 65a4d874) fails only in the three tests that were red on main itself today (#2861, #2857, #2868), and tests/test_fallback_pipeline_state.py passes there on both systems. No game run was repeated for the rebase.

> [@mooki000](https://github.com/mooki000) commented on GitHub · 2026-11-12 18:32 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2796#issuecomment-6832431521) @hckrman101 the validation-layer run you asked for: **968 of that error on main, 0 on the branch, and no new message IDs.** Counts are from stock builds of `51da0464` and `b7d175e4`, before the rebase; the second pair of columns is the 60 seconds of confirmed play in Kokiri Forest. | Layer ID (`MessageID`) | Severity | Main `51da0464`, whole run | Branch `b7d175e4`, whole run | Main, 60 s of play | Branch, 60 s of play | |---|---|---:|---:|---:|---:| | `VUID-vkCmdDrawIndirect-pStrides-04913` (`0x3252797`) | Error | 968 | 0 | 957 | 0 | | `Undefined-Value-ShaderOutputNotConsumed-DynamicRendering` (`0x46877e3e`) | Warning | 120,333 | 145,510 | 114,916 | 121,291 | | `CURRENT-VALIDATION-ENABLED` (`0xfd7b2292`) | Information | 1 | 1 | 0 | 0 | | `WARNING-cache-file-error` (`0xb8515d13`) | Information | 1 | 1 | 0 | 0 | Those are all the distinct messages the layer printed, not only the one we were after. One existing warning remains on both (an unused fragment output; it grows because more draws now run to completion), and the two information lines are the layer saying it is on and that its cache file did not exist yet. Setup: Intel UHD 770, Mesa ANV 26.2.5, Arch Linux, the same warmed save and caches for both, precompile off, no window; Khronos validation 1.4.363 with core checks on and the duplicate-message limit off, loaded from an unpacked copy of the distribution's package (nothing installed). Both runs quit normally with no device loss or driver fault. The picture still has this chip's known false colours; that is a separate problem. Also done: your documentation nits (one stock-build row in the table, the counter runs kept in the description, the implementation paragraph moved under the renderer section of `docs/design.md`), and a rebase, since today's merges had put rows at the same place in the table. Pushed as `805eb0b3` on main `15d4ae8c`. The two code commits' changed lines are identical to the reviewed ones; the fork's Windows run on the same patches (on `65a4d874`) fails only in the three tests that were red on main itself today (#2861, #2857, #2868), and `tests/test_fallback_pipeline_state.py` passes there on both systems. No game run was repeated for the rebase. <!-- github-sync:c:6832431521 -->
Commenting is not possible because the repository is archived.
Reviewers
No reviewers
Labels
Clear labels   
accessibility
Barrier affecting people with disabilities

  
autorelease: pending

   
autorelease: tagged

   
bug
Something isn't working

  
dependencies
Pull requests that update a dependency file

  
documentation
Improvements or additions to documentation

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
github_actions
Pull requests that update GitHub Actions code

  
good first issue
Good for newcomers

  
help wanted
Extra attention is needed

  
invalid
This doesn't seem right

  
question
Further information is requested

  
testing wanted

   
wontfix
This will not be worked on

No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
1 participant
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/Hookshot!2796
No description provided.