Route swapchain present synchronization through RHIExecutor#157
Route swapchain present synchronization through RHIExecutor#157Copilot wants to merge 13 commits into
Conversation
Agent-Logs-Url: https://github.com/NJUCG/MoerEngine/sessions/98b0e8f7-c6e3-45ad-b5e4-2236b363bf86 Co-authored-by: Valer1ie <61175238+Valer1ie@users.noreply.github.com>
Agent-Logs-Url: https://github.com/NJUCG/MoerEngine/sessions/98b0e8f7-c6e3-45ad-b5e4-2236b363bf86 Co-authored-by: Valer1ie <61175238+Valer1ie@users.noreply.github.com>
Agent-Logs-Url: https://github.com/NJUCG/MoerEngine/sessions/98b0e8f7-c6e3-45ad-b5e4-2236b363bf86 Co-authored-by: Valer1ie <61175238+Valer1ie@users.noreply.github.com>
Agent-Logs-Url: https://github.com/NJUCG/MoerEngine/sessions/98b0e8f7-c6e3-45ad-b5e4-2236b363bf86 Co-authored-by: Valer1ie <61175238+Valer1ie@users.noreply.github.com>
Agent-Logs-Url: https://github.com/NJUCG/MoerEngine/sessions/98b0e8f7-c6e3-45ad-b5e4-2236b363bf86 Co-authored-by: Valer1ie <61175238+Valer1ie@users.noreply.github.com>
Agent-Logs-Url: https://github.com/NJUCG/MoerEngine/sessions/98b0e8f7-c6e3-45ad-b5e4-2236b363bf86 Co-authored-by: Valer1ie <61175238+Valer1ie@users.noreply.github.com>
Agent-Logs-Url: https://github.com/NJUCG/MoerEngine/sessions/98b0e8f7-c6e3-45ad-b5e4-2236b363bf86 Co-authored-by: Valer1ie <61175238+Valer1ie@users.noreply.github.com>
Agent-Logs-Url: https://github.com/NJUCG/MoerEngine/sessions/504f8916-ef36-4b64-bc4d-215c366a9c60 Co-authored-by: Valer1ie <61175238+Valer1ie@users.noreply.github.com>
Agent-Logs-Url: https://github.com/NJUCG/MoerEngine/sessions/504f8916-ef36-4b64-bc4d-215c366a9c60 Co-authored-by: Valer1ie <61175238+Valer1ie@users.noreply.github.com>
Agent-Logs-Url: https://github.com/NJUCG/MoerEngine/sessions/504f8916-ef36-4b64-bc4d-215c366a9c60 Co-authored-by: Valer1ie <61175238+Valer1ie@users.noreply.github.com>
Agent-Logs-Url: https://github.com/NJUCG/MoerEngine/sessions/504f8916-ef36-4b64-bc4d-215c366a9c60 Co-authored-by: Valer1ie <61175238+Valer1ie@users.noreply.github.com>
Agent-Logs-Url: https://github.com/NJUCG/MoerEngine/sessions/504f8916-ef36-4b64-bc4d-215c366a9c60 Co-authored-by: Valer1ie <61175238+Valer1ie@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR migrates present-facing lifecycle code (resize/destroy/release) away from caller-owned queue synchronization and toward RHIExecutor-owned synchronization, while also refactoring swapchain surface creation to flow through a new WindowSurfaceSource abstraction and documenting the follow-up design for viewport-scoped present sync.
Changes:
- Replaced direct
GraphicsQueue::Sync()waits in present-facing paths withRHIExecutor::Sync(ERHISyncDepth::Present)(andERHISyncDepth::RHIfor non-present cleanup). - Refactored
SwapchainSurfaceInfoto carry aWindowSurfaceSourceRefand introducedRenderDevice::CreateSwapchainSurfaceInfo(window)to standardize surface plumbing. - Added a checkpoint document describing the next-step contract for per-viewport/swapchain present synchronization.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| source/test/RHITest/rhi_translate_multiqueue_test.cpp | Updates tests to use device.CreateSwapchainSurfaceInfo and new SurfaceInitInfo signature. |
| source/runtime/render/window/glfw/GLFWWindowImpl.h | Removes Vulkan-surface-specific API and introduces interop-handle retrieval. |
| source/runtime/render/window/glfw/GLFWWindowImpl.cpp | Implements GetInteropHandle and removes old Vulkan init/surface creation paths. |
| source/runtime/render/window/WindowContextImpl.h | Adds EWindowInteropHandleType and new interop-handle API surface for window impls. |
| source/runtime/render/window/WindowContext.h | Removes SurfaceInitInfo::rhi_type and drops old native window / Vulkan surface helpers. |
| source/runtime/render/window/WindowContext.cpp | Removes old native window/surface wrapper APIs; adds GetWindowInteropHandle free function. |
| source/runtime/render/rhi/vulkan/VulkanSwapChain.cpp | Swapchain recreation now validates and routes surface creation via WindowSurfaceSource. |
| source/runtime/render/rhi/vulkan/VulkanDevice.h | Adds CreateSwapchainSurfaceInfo override. |
| source/runtime/render/rhi/vulkan/VulkanDevice.cpp | Implements CreateSwapchainSurfaceInfo via platform helper. |
| source/runtime/render/rhi/d3d12/D3D12Device.h | Adds CreateSwapchainSurfaceInfo override. |
| source/runtime/render/rhi/d3d12/D3D12Device.cpp | Implements CreateSwapchainSurfaceInfo via platform helper. |
| source/runtime/render/rhi/RHIResource.h | Introduces WindowSurfaceSource + updates SwapchainSurfaceInfo to hold source. |
| source/runtime/render/rhi/RHIImpl.h | Extends device impl interface for swapchain surface creation; adds platform helper declaration. |
| source/runtime/render/rhi/RHIImpl.cpp | Implements GLFW-backed WindowSurfaceSource and platform surface-info factory. |
| source/runtime/render/rhi/RHI.h | Exposes RenderDevice::CreateSwapchainSurfaceInfo. |
| source/runtime/render/renderer/common/ui/ImGUIRenderer.cpp | Routes viewport destroy/resize sync through RHIExecutor::Sync(Present). |
| source/runtime/render/renderer/common/PresentationSurface.h | Removes private surface-info builder helper. |
| source/runtime/render/renderer/common/PresentationSurface.cpp | Uses device.CreateSwapchainSurfaceInfo(window) for swapchain creation. |
| source/runtime/render/renderer/Renderer.cpp | Replaces explicit queue sync with executor-owned present/RHI sync. |
| source/runtime/Engine.cpp | Updates window init for new SurfaceInitInfo ctor signature. |
| source/profiler/main.cpp | Updates window init signature and present resize sync through RHIExecutor. |
| docs/checkpoints/swapchain_present_sync_checkpoint.json | Documents follow-up design for viewport-scoped present synchronization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #include "window/WindowContextImpl.h" | ||
| #include "shader/ShaderResourceManager.h" | ||
|
|
||
| #include "rhi/plugin/NrdPlugin.h" |
There was a problem hiding this comment.
This TU uses Vulkan types and glfwCreateWindowSurface but doesn’t explicitly include a Vulkan core header (or define GLFW_INCLUDE_VULKAN before including glfw3.h). That can make VkResult/VkInstance/VkSurfaceKHR undefined depending on include order/build flags. Please add an explicit Vulkan include or the GLFW_INCLUDE_VULKAN define here to guarantee compilation.
| #include "rhi/plugin/NrdPlugin.h" | |
| #include "rhi/plugin/NrdPlugin.h" | |
| #define GLFW_INCLUDE_VULKAN |
There was a problem hiding this comment.
已修复,提交在 3b0adf5。/home/runner/work/MoerEngine/MoerEngine/source/runtime/render/rhi/RHIImpl.cpp 现在显式包含了 <vulkan/vulkan_core.h>,不再依赖 glfw3.h 或 include 顺序来提供 Vulkan 类型定义。
| const WindowSurfaceSource* existing_surface_source = surface_info.source.get(); | ||
| const WindowSurfaceSource* incoming_surface_source = _info.surface.source.get(); | ||
| MOER_ASSERT( | ||
| incoming_surface_source != nullptr, | ||
| "Vulkan swapchain recreate requires a valid incoming surface source from SwapchainCreateInfo" |
There was a problem hiding this comment.
CreateOrRecreate already hard-asserts incoming_surface_source via MOER_ASSERT here, but later also uses a plain assert(incoming_surface_source && ...) before CreateSurface. It’s redundant and mixes assertion mechanisms; consider removing the duplicate assert (or converting it to MOER_ASSERT) to keep failure behavior consistent across builds.
There was a problem hiding this comment.
已处理,提交在 3b0adf5:保留了 RHIImpl.cpp 的显式 Vulkan core 头包含,并删除了 VulkanSwapChain.cpp 里与前置 MOER_ASSERT 重复的 plain assert。
Agent-Logs-Url: https://github.com/NJUCG/MoerEngine/sessions/c174d796-74d4-4c35-92e8-55333bdab298 Co-authored-by: Valer1ie <61175238+Valer1ie@users.noreply.github.com>
RHIImpl.cpp和VulkanSwapChain.cpp,确认需要处理的是 Vulkan 头显式包含和重复 assert 两项dev_parallel_rhiworkflow runs;当前未发现相关 runs,可继续按代码 review 修正RHIImpl.cpp显式包含 Vulkan core 头,避免依赖 include 顺序让VkResult/VkInstance/VkSurfaceKHR隐式可见VulkanSwapChain.cpp删除与前置MOER_ASSERT重复的 plainassert,统一断言语义cmake -B build;当前 Linux 配置下未暴露可用的 render/RHI 目标脚本覆盖这两个 TU,仓库现有验证覆盖存在缺口