Skip to content

Fix GUI buffer update with deferred allocation - #1557

Open
swayam8624 wants to merge 1 commit into
KhronosGroup:mainfrom
swayam8624:fix/gui-update-buffers-null
Open

Fix GUI buffer update with deferred allocation#1557
swayam8624 wants to merge 1 commit into
KhronosGroup:mainfrom
swayam8624:fix/gui-update-buffers-null

Conversation

@swayam8624

Copy link
Copy Markdown

Summary

Fixes #1508.

Gui::update_buffers() can be called while explicit_update is false. In that mode the persistent vertex and index buffers are intentionally null until the first update, but the method dereferenced both pointers before allocating them.

This change short-circuits the two conditions when the corresponding buffer is null, allowing the existing allocation and upload path to initialize both buffers.

Reproduction

On macOS Apple Silicon (M2 Pro, macOS 26.6, Apple Clang 21, Vulkan SDK 1.4.341), an AddressSanitizer harness using the real GuiCpp::update_buffers() implementation and non-empty ImGui draw data reproduced a deterministic null dereference in the parent implementation:

SEGV on unknown address 0x000000000028
vk::Buffer::operator!()
vkb::Gui<(vkb::BindingType)1>::update_buffers()

Validation

  • cmake --build build/repro --config Debug --target framework vulkan_samples -j4 — passed
  • ctest --test-dir build/repro --output-on-failure — no tests found in this repository configuration
  • python3 scripts/copyright.py main --fix — passed
  • git diff --check — passed
  • ./scripts/clang_format.py main — could not run because clang-format is not installed

The diff is limited to framework/gui.h; no public API or dependency changes are included.

@CLAassistant

CLAassistant commented Jul 29, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@asuessenbach

Copy link
Copy Markdown
Contributor

Your fix might be correct. But it might as well just hide the actual error.

That is: does anybody know what that explicit_update flag is supposed to control?
The comment says "If true, update buffers every frame". But none of the two update_buffers functions in Gui seem to care about that flag.
In the Gui-constructor, it controls the initialization of the vertex_buffer and the index_buffer. But both are used unconditionally at several places.

@swayam8624

Copy link
Copy Markdown
Author

Thanks for checking the lifecycle semantics. The explicit_update flag selects the persistent-buffer path: the constructor creates placeholder buffers, update_buffers() replaces/resizes and uploads them, and draw_impl() binds them. When the flag is false, draw_impl() instead allocates per-frame buffers through update_buffers(command_buffer). The two overloads therefore do not need to inspect the flag themselves; the caller/path selection does.\n\nThe null guards in this PR address the reported deferred-allocation case where a zero-sized ImGui frame leaves the persistent buffers null before a later draw. I agree the flag comment could be clearer, but I would prefer to keep that separate from this narrowly scoped fix unless you recommend combining them.

@swayam8624
swayam8624 marked this pull request as ready for review July 29, 2026 11:01
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.

Bug: framework/gui.h/update_buffers() crashes when "explicit_update == false" because vertex_buffer/index_buffer are null

3 participants