Repository navigation
Conversation
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 NotRexed#2494 Refs NotRexed#1001 Assisted-by: GPT-6 Astra Co-Authored-By: GPT-6 Astra <noreply@openai.com>
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 <noreply@openai.com>
Joey-Magnus
left a comment
There was a problem hiding this comment.
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.
|
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. |
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 NotRexed#2796 Assisted-by: GPT-6 Astra Co-Authored-By: GPT-6 Astra <noreply@openai.com>
Record the paired renderer-only runs for the NotRexed#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 <noreply@openai.com>
|
Thanks, you were right on both points.
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 A docs-only commit since ( |
hckrman101
left a comment
There was a problem hiding this comment.
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 e76c32d): both fixed at 5da2d63.
- The fixture now has
_CRT_SECURE_NO_WARNINGSbefore its includes. Both windows-2022 parts pass on this PR's own run at b7d175e, and the log showstest_fallback_pipeline_state.pyrunning there. - The ordinary, fallback, ordinary cache hole is closed. Binding a fallback pipeline now clears
last_vb_validandvertex_set.on, so the next ordinary draw goes throughvkCmdBindVertexBuffers2again 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 b7d175e, and the branch is up to date with main with no conflicts.
|
|
||
| | Date | Build | OS / desktop | GPU / driver | CPU / RAM | Settings | Where | Result | | ||
| |---|---|---|---|---|---|---|---| | ||
| | 2026-11-12 | Host `20635b20` (source f6f995e7); renderer `5da2d634` with local-only stride counters, after #2796's cache review fix | Arch Linux, kernel 7.2.15-1-omarchy, headless | Intel UHD 770 (RPL-S), Mesa ANV 26.2.5-arch1.1 | Core i9-13900K, 93 GB reported by the game | 1920x1080, Minimum, FG off, precompile/warm/prediction off, 32 GiB process limit; identical warmed save/cache and counters to the control below | Kokiri Forest, child, standing; 09:18:05–09:22:06 UTC−06 | Last counter checkpoint: 45,992 fallback pipeline binds; 2,880 same-buffer ordinary returns after fallback, all 2,880 rebound, 0 skipped. 27 captures; forest/HUD and existing false colours remain. Exit 124 at the requested limit; no game crash, device loss, virtual-core escape or GPU fault. Ordinary run: counts only, no timing or validation-layer claim | |
There was a problem hiding this comment.
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.
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.
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, fork Windows source checks, and Intel UHD 770 game counters: 2,829 omitted restores before, all 2,880 observed returns rebound after. Not measured: validation-layer errors (layer unavailable), performance, Windows gameplay, or other GPUs' gameplay.
Refs #2494. Part of #1001; this does not close the wider no-mesh-shader tracker.
Problem
build_graphicsalready gives it no vertex bindings or attributes.VK_DYNAMIC_STATE_VERTEX_INPUT_BINDING_STRIDEto every non-native-mesh pipeline.vkCmdDrawIndirectand returns before the ordinary vertex-buffer/stride binding code. This is the unset dynamic state reported asVUID-vkCmdDrawIndirect-pStrides-04913.What changed
gpu/vk/vk_draw.cpp: add dynamic binding stride only for ordinary vertex-input programs. At a fallback graphics-pipeline bind, clearlast_vb_validandvertex_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_WARNINGSprecedes the first fixture include. The direct fallback library-construction case is extra fixture coverage: runtime fallback programs do not use pipeline libraries.docs/development.md: describe which pipeline types require these states.docs/tested-configurations.md: record both Intel scene runs, including their limitations.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 commit20635b20214826d1d6be63ac873b6fc80824414f. Checks run on 2026-11-12.Initial construction checks —
20635b20The test extracts
build_graphicsunmodified 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.20635b20)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.
The initial Windows-target syntax pass was insufficient: its MinGW headers had Microsoft-compatibility deprecation annotations disabled, and the real Windows job then rejected
getenvin the Microsoft UCRT headers. The review fix and stronger check are described below. The original Linux suite result above belongs to20635b20, 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.51da0464, source18ed87b520635b20, sourcef6f995e7The 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.
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. Available layer manifests were Mesa anti-lag, Mesa device selection and NVIDIA's implicit layers; none was
VK_LAYER_KHRONOS_validation. Therefore VUID-04913 present-before/absent-after is not established here. The test above proves the production constructor stops declaring the unused state; the game runs show continued fallback drawing without device loss.Review correction —
5da2d634Credit 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
vkCmdBindVertexBuffers2values, 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
e76c32ddfails the third-draw rebind assertion;5da2d634passes. 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.test_nightly_two_runs_at_once_end_at_the_newer_commit, checksum publication after five uploads). It is retained, not rerun or called green.getenvdeprecation 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
20635b20in both copies; only the renderer is rebuilt frome76c32ddor5da2d634, 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.
e76c32dd5da2d634Both 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.
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. VUID disappearance remains untested locally.
Not tested
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.mdwith #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 noreply@openai.com