Skip to content

Fix Vulkan descriptor bindings on first-frame draws - #19

Merged
themuffinator merged 1 commit into
mainfrom
codex/fix-17-vulkan-descriptors
Oct 1, 2026
Merged

themuffinator merged 1 commit into
mainfrom
codex/fix-17-vulkan-descriptors

Conversation

@themuffinator

Copy link
Copy Markdown
Owner

Single-texture menu draws bind the uniform buffer and diffuse texture but can leave descriptor set 2 unbound. The fragment shader still statically uses its depth-fade sampler, including when the specialization constant disables sampling. This matches the reporter's LLDB trace, which faults in MoltenVK's resource binding while encoding an indexed draw.

The main descriptor binder now supplies the missing sampler, fills leading and trailing gaps with descriptors of the correct type, restores the current dynamic uniform offset, and bounds restores to the device's pipeline layout. Existing active samplers and unchanged-state caching are preserved. The RTX backend already supplies a depth-sampler fallback; this fixes the raster Vulkan path.

Refs #17. This is a draft pending native Apple Silicon confirmation.

Validation:

  • The new Meson regression test compiles the actual production binder with a mock Vulkan command sink. It fails on the original code and passes with this patch, covering first-frame draws, sampler preservation, uniform restores, descriptor limits, and caching.
  • Windows x86_64 / NVIDIA RTX 4060 Laptop / Vulkan validation layer 1.4.313.1: the original startup produces 10 VUID-vkCmdDrawIndexed-None-08600 errors for unbound set 2. Patched startup, 4x MSAA, and depth fade disabled each produce zero of those errors.
  • Windowed q3dm1 loading and vid_restart pass with validation disabled; engine screenshots verify rendering.
  • Meson build passes. The full Meson run has 38 passing tests, one optional audio skip, and one packaging fixture failure: that test assumes OS temporary files live outside the repository, while this task keeps them under .tmp. The external-output case passes with its repository boundary simulated. All 33 Vulkan sweep script tests pass.

Separate pre-existing findings: startup still reports three depth/stencil aspect-mask validation errors and one invalid debug image-view name. With validation enabled, map loading also reuses a stale depth-fade descriptor and crashes in the NVIDIA driver; both the original and patched builds reproduce it. These are outside this descriptor-binding fix.

Apple Silicon retest: build this branch and repeat the original windowed launch:

./fnquake3 +set cl_renderer vk +set r_fullscreen 0 +set r_hdr 0

Native macOS execution was unavailable during this work, so resolution of the reported SIGSEGV remains to be confirmed.

@themuffinator
themuffinator marked this pull request as ready for review October 1, 2026 15:47
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@themuffinator
themuffinator merged commit 99e7984 into main Oct 1, 2026
7 checks passed
@themuffinator
themuffinator deleted the codex/fix-17-vulkan-descriptors branch October 1, 2026 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant