Skip to content

Add a Vulkan renderer backend to imgui-lwjgl3 - #427

Open
Enaium wants to merge 3 commits into
SpaiR:mainfrom
Enaium:vulkan
Open

Add a Vulkan renderer backend to imgui-lwjgl3#427
Enaium wants to merge 3 commits into
SpaiR:mainfrom
Enaium:vulkan

Conversation

@Enaium

@Enaium Enaium commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a Vulkan renderer backend (ImGuiImplVulkan, port of imgui_impl_vulkan) to the imgui-lwjgl3 module, along with ImGuiImplVulkanH helper structures and createVulkanSurface() helpers for the GLFW and SDL3 backends. SPIR-V shaders ship as classpath resources. imgui-app gains a VULKAN backend (WindowVulkan) with swapchain, render pass, per-frame sync objects and resize handling, and the example module gets an ExampleVulkan smoke test.

Type of change

  • Minor changes or tweaks (quality of life stuff)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Notes for reviewer

  • This work is AI-assisted: the Vulkan backend port and the imgui-app integration were written with the help of DeepSeek. The commit carries a Co-authored-by: DeepSeek <noreply@deepseek.com> trailer.
  • Multi-viewport is not supported (the binding has no Platform_CreateVkSurface callback yet); the sampler descriptor set is bound once in setupRenderState (no per-draw sampler switching).
  • LWJGL's p* setters do not always set the matching *Count fields (VkPresentInfoKHR.pSwapchains in particular), so counts are set explicitly via native setters.
  • GLFW_SCALE_FRAMEBUFFER is disabled on Wayland to avoid GLFW's viewport interfering with the Vulkan WSI.

Fixes #397

@Enaium

Enaium commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author
Screenshot From 2026-08-04 12-57-08

@SpaiR SpaiR added the feat New feature or request label Aug 4, 2026
@SpaiR

SpaiR commented Aug 4, 2026

Copy link
Copy Markdown
Owner

After a quick glance, I can confidently say that the work generally looks solid. The limited scope, implementation within the app module, and working example are exactly what I haven't typically seen in previous implementations. I'll need a bit more time for a detailed review (as I'm simultaneously undertaking a major rewrite of the internal binding structure), but I don't anticipate your PR will be held up for long. At least from this initial look, I don't see anything that would be a blocker.

P.S. I want to specifically mention one thing. Given current trends, if any part of your work was done with the help of AI, please note it somewhere. Either as "co-authored" in the commit message, or if the commit is already pushed, simply add a line in the PR description. I have no bias against this whatsoever and actively use AI in my own work, but I just want transparency on the matter. If all the code was written manually, then all I can say is "impressive, very nice" 😅 In that case, there's no need to mention anything anywhere.

@Enaium

Enaium commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for the quick look and the kind words! To address your note about AI usage: yes, this work was AI-assisted. The Vulkan backend port and the imgui-app integration were written with the help of DeepSeek, and the commit carries a Co-authored-by: DeepSeek <noreply@deepseek.com> trailer (see commit ad2ffaa). I've also added a note to the PR description for transparency.

Quick status for reference:

  • Verified locally: ExampleVulkan runs on Linux (Wayland + X11) with ImGui content rendering correctly, including swapchain resize handling and the font atlas upload path.
  • Shaders: the embedded SPIR-V arrays were extracted into classpath resources under imgui/vulkan/shaders/; custom shaders via InitInfo.customShaderVert/Frag are still supported.
  • Known limitations (also in the PR body): multi-viewport is not wired up yet (the binding has no Platform_CreateVkSurface callback), and the sampler descriptor set is bound once per frame rather than per draw command.

Port of the Dear ImGui Vulkan renderer (imgui_impl_vulkan) into the
imgui-lwjgl3 module:
- ImGuiImplVulkan renderer with descriptor sets, pipeline creation and
  font atlas upload; shader SPIR-V shipped as classpath resources
- ImGuiImplVulkanH helper structures
- createVulkanSurface() helpers for the GLFW and SDL3 backends
- imgui-app: VULKAN backend with WindowVulkan (swapchain, render pass,
  per-frame sync objects and resize handling)
- example: ExampleVulkan smoke test

Co-authored-by: DeepSeek <noreply@deepseek.com>
@Enaium

Enaium commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author
截屏2026-08-04 19 02 10 Tested on macOS

@SpaiR SpaiR left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — this is solid work overall, and I'd like to get it in. Three things before I merge; everything else I'll take as follow-ups on my side, so don't worry about the long tail.

  1. WindowVulkan breaks the buildAvoidStarImport fires on lines 13, 20, 21, 22 (the rule only excludes GL32 and GLFW). Please expand those imports.

  2. The font texture leaks on shutdown. createFontsTexture() declares fontImage, fontMemory, fontImageView and fontDescriptorSet as locals at ImGuiImplVulkan.java:1234-1237, which shadow the fields at 260-263. The fields stay zero, so destroyFontsTexture() destroys nothing. Deleting the four local declarations should be all it takes.

  3. Please align the descriptor design with our pinned upstream. We're on 1.92.7 (b1bcb12a6), where imgui_impl_vulkan.cpp uses a single descriptor set with COMBINED_IMAGE_SAMPLER at set 0 (1106-1116), one pool (1123-1127), and one bind per draw (683). This PR splits texture and sampler into two sets, and the shipped SPIR-V follows the split — the fragment blob is 900 bytes against upstream's 772, so it isn't upstream's shader. I'd like the port to match the pin instead: it keeps the next submodule bump reviewable, and it means you don't have to maintain shaders at all — copy upstream's __glsl_shader_vert_spv / __glsl_shader_frag_spv byte-for-byte, the way ImGuiImplSdlGpu3Shaders does. The deprecated addTexture overload, the second pool-size constant and the "sampler bound once in setupRenderState" caveat from your description should all fall out with it.

I realize 3 is the largest of the three — happy to talk it through if you'd rather split it off, but I'd prefer not to ship a descriptor layout we can't trace back to a revision.

Once these are in, I'll merge and clean up the rest myself.

Enaium and others added 2 commits August 6, 2026 16:14
Match the pinned Dear ImGui 1.92.7 backend: single descriptor set with
COMBINED_IMAGE_SAMPLER at set 0, one pool size, one bind per draw, and
the upstream shader bytecode shipped as resources. Drops the separate
sampler descriptor set, the deprecated addTexture overload and the
second pool-size constant.

Co-authored-by: DeepSeek <noreply@deepseek.com>
createFontsTexture() declared locals shadowing the fontImage/fontMemory/
fontImageView/fontDescriptorSet fields, so destroyFontsTexture() never
released the atlas resources. Also expand the wildcard Vulkan imports in
WindowVulkan to satisfy AvoidStarImport.

Co-authored-by: DeepSeek <noreply@deepseek.com>
@Enaium

Enaium commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Addressed all three review points:

  1. Star imports in WindowVulkan — expanded the wildcard Vulkan imports (KHRSurface, KHRSwapchain, VK10, org.lwjgl.vulkan.*); GLFW.* kept per the checkstyle exemption. AvoidStarImport is clean now.
  2. Font texture leak — removed the local declarations in createFontsTexture() that shadowed the fontImage/fontMemory/fontImageView/fontDescriptorSet fields; destroyFontsTexture() now actually releases the atlas resources.
  3. Descriptor design aligned with the pinned 1.92.7 (b1bcb12a6) — single descriptor set layout with COMBINED_IMAGE_SAMPLER at set 0, one pool size, one bind per draw; the SPIR-V is now copied byte-for-byte from upstream's __glsl_shader_vert_spv/__glsl_shader_frag_spv (fragment blob now 772 bytes). The deprecated addTexture overload, the second pool-size constant and the sampler-set binding all fell out.

Verified locally: ExampleVulkan still runs cleanly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing Bindings: Vulkan

2 participants