Skip to content

fix(vulkan): resolve startup hang and enable dynamic rendering pipelines#2

Merged
jackthepunished merged 1 commit intomasterfrom
fix/vulkan-startup-hang-and-dynamic-rendering
Mar 21, 2026
Merged

fix(vulkan): resolve startup hang and enable dynamic rendering pipelines#2
jackthepunished merged 1 commit intomasterfrom
fix/vulkan-startup-hang-and-dynamic-rendering

Conversation

@jackthepunished
Copy link
Copy Markdown
Owner

@jackthepunished jackthepunished commented Mar 21, 2026

Summary

  • Fix startup hang: Resolved three Vulkan synchronization bugs (fence reset ordering, swapchain semaphore reuse, internal frame fence deadlock) that caused the window to become unresponsive on launch
  • Enable dynamic rendering: Migrated all 6 graphics pipeline definitions from legacy VkRenderPass to Vulkan 1.3 VkPipelineRenderingCreateInfo, fixing the blue-screen-only output caused by renderPass/dynamic-rendering mismatch
  • Minor fixes: Corrected Texture::create() static factory usage, added camera descriptor binding at init, locale-safe path conversion for Turkish Windows

Changes

Area Files What
RHI Pipeline rhi_pipeline.hpp, vk_pipeline.cpp Added color_attachment_formats / depth_attachment_format to GraphicsPipelineDesc; use VkPipelineRenderingCreateInfo when render_pass == nullptr
Frame Sync vk_device.cpp Signal internal frame fence via empty submit in end_frame()
App Render Loop application.cpp, application.hpp Move reset_fences before submit; image-index-based render semaphores; guard early-returns with end_frame()
Renderer deferred_renderer.cpp Switch geometry/lighting/composite/shadow/SSAO/blur pipelines to dynamic rendering formats; bind camera UBO at init
Shader Compiler shader_compiler.cpp, shader_compiler.hpp path_to_string() for locale-safe path handling
Shaders ssao_blur.frag Upgrade to GLSL 450 / Vulkan descriptor layout
Cleanup ssao.vert, ssao.cpp, ssao.hpp Remove legacy OpenGL SSAO module
Entry Point main.cpp Set C locale to prevent MinGW Illegal byte sequence crash

Test Plan

  • cmake --build build --target horizon_game compiles clean (0 errors)
  • Application launches without freezing (Responding: True)
  • Render loop runs at 500-880 FPS
  • "renderPass equal to VK_NULL_HANDLE" validation errors eliminated
  • Manual visual inspection: scene geometry, lighting, shadows render correctly

Summary by CodeRabbit

Release Notes

  • Refactor

    • Refactored ambient occlusion rendering and integrated with the main rendering pipeline
    • Modernized graphics rendering architecture for improved performance
  • Bug Fixes

    • Improved shader compilation compatibility on Windows systems

The application was freezing ("not responding") on startup due to three
synchronization issues in the Vulkan frame loop:

1. Fence reset was called immediately after wait, before any GPU work
   was submitted - causing the next frame's wait to block forever.
   Moved reset_fences() to just before submit().

2. Swapchain semaphores were indexed by frame-in-flight rather than by
   acquired image index, causing semaphore reuse conflicts when the
   swapchain image count (3) exceeded frames-in-flight (2).
   Changed to image-index-based semaphore management.

3. VulkanDevice::end_frame() reset its internal fence but never
   re-signaled it, causing begin_frame() to deadlock on the next
   iteration. Added an empty queue submit to signal the fence.

Additionally, all graphics pipelines were created with VkRenderPass
objects but the rendering code uses vkCmdBeginRendering (Vulkan 1.3
dynamic rendering), making every draw call invalid and producing a
blue screen. Fixed by:

- Adding color_attachment_formats / depth_attachment_format fields to
  GraphicsPipelineDesc for dynamic rendering compatibility
- Using VkPipelineRenderingCreateInfo (pNext chain) when render_pass
  is nullptr in VulkanPipeline creation
- Migrating all 6 pipeline definitions (geometry, lighting, composite,
  shadow, SSAO, SSAO blur) to use format-based dynamic rendering

Other fixes included in this changeset:
- Fixed Texture::create() calls (static factory, not member function)
- Added early camera descriptor set binding after UBO creation
- Guarded command list early-return to always call end_frame()
- Locale fix for MinGW path conversion on Turkish Windows

Made-with: Cursor
@jackthepunished jackthepunished merged commit 7192c55 into master Mar 21, 2026
0 of 3 checks passed
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 21, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 693db3b1-8e62-495d-8140-d01e41bd68a9

📥 Commits

Reviewing files that changed from the base of the PR and between 9fcf393 and 16fee9d.

📒 Files selected for processing (14)
  • assets/shaders/ssao.vert
  • assets/shaders/ssao_blur.frag
  • engine/assets/shader_compiler.cpp
  • engine/assets/shader_compiler.hpp
  • engine/renderer/deferred_renderer.cpp
  • engine/renderer/ssao.cpp
  • engine/renderer/ssao.hpp
  • engine/rhi/rhi_device.cpp
  • engine/rhi/rhi_pipeline.hpp
  • engine/rhi/vulkan/vk_device.cpp
  • engine/rhi/vulkan/vk_pipeline.cpp
  • game/src/application.cpp
  • game/src/application.hpp
  • game/src/main.cpp

📝 Walkthrough

Walkthrough

This pull request removes the legacy OpenGL-based SSAO implementation and migrates the graphics pipeline to Vulkan dynamic rendering. Changes include updating GLSL shaders to version 450 with explicit layout qualifiers, removing the OpenGL SSAO shader and renderer code, restructuring graphics pipeline creation to use format descriptors instead of render passes, adjusting synchronization based on swapchain image counts, and setting the C locale at application startup with improved Windows path encoding for UTF-8.

Changes

Cohort / File(s) Summary
Shader Files
assets/shaders/ssao.vert, assets/shaders/ssao_blur.frag
Removed ssao.vert entirely. Updated ssao_blur.frag to GLSL 450 core with explicit layout qualifiers for inputs/outputs (location = 0) and descriptor bindings (set = 0, binding = 0). Simplified blur normalization constant.
Shader Compilation Infrastructure
engine/assets/shader_compiler.cpp, engine/assets/shader_compiler.hpp
Introduced path_to_string() helper for Windows UTF-8 path conversion. Updated shader compilation error messages and shaderc inputs to use UTF-8 paths. Changed ShaderCompileOptions::filename from std::string_view to owning std::string.
Vulkan RHI Updates
engine/rhi/rhi_pipeline.hpp, engine/rhi/vulkan/vk_pipeline.cpp, engine/rhi/rhi_device.cpp
Added color_attachment_formats and depth_attachment_format fields to GraphicsPipelineDesc for Vulkan dynamic rendering. Updated VulkanPipeline to conditionally build VkPipelineRenderingCreateInfo when render pass is null. Added Windows path-to-UTF-8 conversion in RHI device shader logging and debug naming.
Vulkan Device Synchronization
engine/rhi/vulkan/vk_device.cpp
Added explicit empty queue submission with fence signal in end_frame() to strengthen synchronization of in-flight frame fences.
Renderer Modernization
engine/renderer/deferred_renderer.cpp, engine/renderer/ssao.cpp, engine/renderer/ssao.hpp
Removed complete SSAO implementation files (ssao.cpp, ssao.hpp). Updated deferred renderer pipelines to use explicit attachment formats instead of render passes for all graphics pipelines (geometry, lighting, composite, shadow). Added camera UBO initial write during pipeline creation.
Application & Synchronization
game/src/application.hpp, game/src/application.cpp
Changed m_render_finished_sems from fixed-size std::array to dynamic std::vector based on swapchain image count. Updated semaphore indexing from frame slot to swapchain image index with bounds checking. Added execute_ssao_pass() invocation between geometry and lighting passes. Improved command buffer creation error handling.
Initialization
game/src/main.cpp
Added explicit C locale configuration at startup via std::locale::global() and std::setlocale() with exception handling.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

  • Integrate SSAO into deferred Vulkan pipeline #1 — Directly related; adds SSAO integration into the deferred renderer and updates SSAO shader paths, while this PR removes the legacy OpenGL SSAO implementation and enables Vulkan-based rendering pipeline modernization.

Poem

🐰 Old OpenGL fades to dusk,
Dynamic rendering we trust,
Vulkan pipelines shine so bright,
Shaders reborn in 450 light,
Locale set, synchronization true,
Modern graphics born anew!

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/vulkan-startup-hang-and-dynamic-rendering

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.

Add a .trivyignore file to your project to customize which findings Trivy reports.

@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
21.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

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