Skip to content

Vulkan backend (docking): Fix VUIDs 03868/00067 (unsafe render semaphore usage) #8842

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: docking
Choose a base branch
from
Open

Vulkan backend (docking): Fix VUIDs 03868/00067 (unsafe render semaphore usage) #8842

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 38 additions & 42 deletions backends/imgui_impl_vulkan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ void ImGui_ImplVulkan_DestroyDeviceObjects();
void ImGui_ImplVulkan_DestroyFrameRenderBuffers(VkDevice device, ImGui_ImplVulkan_FrameRenderBuffers* buffers, const VkAllocationCallbacks* allocator);
void ImGui_ImplVulkan_DestroyWindowRenderBuffers(VkDevice device, ImGui_ImplVulkan_WindowRenderBuffers* buffers, const VkAllocationCallbacks* allocator);
void ImGui_ImplVulkanH_DestroyFrame(VkDevice device, ImGui_ImplVulkanH_Frame* fd, const VkAllocationCallbacks* allocator);
void ImGui_ImplVulkanH_DestroyFrameSemaphores(VkDevice device, ImGui_ImplVulkanH_FrameSemaphores* fsd, const VkAllocationCallbacks* allocator);
void ImGui_ImplVulkanH_DestroyAllViewportsRenderBuffers(VkDevice device, const VkAllocationCallbacks* allocator);
void ImGui_ImplVulkanH_CreateWindowSwapChain(VkPhysicalDevice physical_device, VkDevice device, ImGui_ImplVulkanH_Window* wd, const VkAllocationCallbacks* allocator, int w, int h, uint32_t min_image_count);
void ImGui_ImplVulkanH_CreateWindowCommandBuffers(VkPhysicalDevice physical_device, VkDevice device, ImGui_ImplVulkanH_Window* wd, uint32_t queue_family, const VkAllocationCallbacks* allocator);
Expand Down Expand Up @@ -1570,20 +1569,19 @@ void ImGui_ImplVulkanH_CreateWindowCommandBuffers(VkPhysicalDevice physical_devi
VkFenceCreateInfo info = {};
info.sType = VK_STRUCTURE_TYPE_FENCE_CREATE_INFO;
info.flags = VK_FENCE_CREATE_SIGNALED_BIT;
err = vkCreateFence(device, &info, allocator, &fd->Fence);
err = vkCreateFence(device, &info, allocator, &fd->RenderFence);
check_vk_result(err);
}
}

for (uint32_t i = 0; i < wd->SemaphoreCount; i++)
{
ImGui_ImplVulkanH_FrameSemaphores* fsd = &wd->FrameSemaphores[i];
{
VkSemaphoreCreateInfo info = {};
info.sType = VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO;
err = vkCreateSemaphore(device, &info, allocator, &fsd->ImageAcquiredSemaphore);
err = vkCreateSemaphore(device, &info, allocator, &fd->ImageAcquiredSemaphore);
check_vk_result(err);
err = vkCreateSemaphore(device, &info, allocator, &fsd->RenderCompleteSemaphore);
}
{
VkSemaphoreCreateInfo info = {};
info.sType = VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO;
err = vkCreateSemaphore(device, &info, allocator, &wd->RenderSemaphores[i]);
check_vk_result(err);
}
}
Expand Down Expand Up @@ -1613,11 +1611,14 @@ void ImGui_ImplVulkanH_CreateWindowSwapChain(VkPhysicalDevice physical_device, V
// We don't use ImGui_ImplVulkanH_DestroyWindow() because we want to preserve the old swapchain to create the new one.
// Destroy old Framebuffer
for (uint32_t i = 0; i < wd->ImageCount; i++)
{
ImGui_ImplVulkanH_DestroyFrame(device, &wd->Frames[i], allocator);
for (uint32_t i = 0; i < wd->SemaphoreCount; i++)
ImGui_ImplVulkanH_DestroyFrameSemaphores(device, &wd->FrameSemaphores[i], allocator);
vkDestroySemaphore(device, wd->RenderSemaphores[i], allocator);
wd->RenderSemaphores[i] = VK_NULL_HANDLE;
}

wd->Frames.clear();
wd->FrameSemaphores.clear();
wd->RenderSemaphores.clear();
wd->ImageCount = 0;
if (wd->RenderPass)
vkDestroyRenderPass(device, wd->RenderPass, allocator);
Expand Down Expand Up @@ -1670,11 +1671,10 @@ void ImGui_ImplVulkanH_CreateWindowSwapChain(VkPhysicalDevice physical_device, V
err = vkGetSwapchainImagesKHR(device, wd->Swapchain, &wd->ImageCount, backbuffers);
check_vk_result(err);

wd->SemaphoreCount = wd->ImageCount + 1;
wd->Frames.resize(wd->ImageCount);
wd->FrameSemaphores.resize(wd->SemaphoreCount);
wd->RenderSemaphores.resize(wd->ImageCount);
memset(wd->Frames.Data, 0, wd->Frames.size_in_bytes());
memset(wd->FrameSemaphores.Data, 0, wd->FrameSemaphores.size_in_bytes());
memset(wd->RenderSemaphores.Data, 0, wd->RenderSemaphores.size_in_bytes());
for (uint32_t i = 0; i < wd->ImageCount; i++)
wd->Frames[i].Backbuffer = backbuffers[i];
}
Expand Down Expand Up @@ -1858,11 +1858,14 @@ void ImGui_ImplVulkanH_DestroyWindow(VkInstance instance, VkDevice device, ImGui
//vkQueueWaitIdle(bd->Queue);

for (uint32_t i = 0; i < wd->ImageCount; i++)
{
ImGui_ImplVulkanH_DestroyFrame(device, &wd->Frames[i], allocator);
for (uint32_t i = 0; i < wd->SemaphoreCount; i++)
ImGui_ImplVulkanH_DestroyFrameSemaphores(device, &wd->FrameSemaphores[i], allocator);
vkDestroySemaphore(device, wd->RenderSemaphores[i], allocator);
wd->RenderSemaphores[i] = VK_NULL_HANDLE;
}

wd->Frames.clear();
wd->FrameSemaphores.clear();
wd->RenderSemaphores.clear();
vkDestroyRenderPass(device, wd->RenderPass, allocator);
vkDestroySwapchainKHR(device, wd->Swapchain, allocator);
vkDestroySurfaceKHR(instance, wd->Surface, allocator);
Expand All @@ -1872,24 +1875,19 @@ void ImGui_ImplVulkanH_DestroyWindow(VkInstance instance, VkDevice device, ImGui

void ImGui_ImplVulkanH_DestroyFrame(VkDevice device, ImGui_ImplVulkanH_Frame* fd, const VkAllocationCallbacks* allocator)
{
vkDestroyFence(device, fd->Fence, allocator);
vkDestroySemaphore(device, fd->ImageAcquiredSemaphore, allocator);
vkDestroyFence(device, fd->RenderFence, allocator);
vkFreeCommandBuffers(device, fd->CommandPool, 1, &fd->CommandBuffer);
vkDestroyCommandPool(device, fd->CommandPool, allocator);
fd->Fence = VK_NULL_HANDLE;
fd->ImageAcquiredSemaphore = VK_NULL_HANDLE;
fd->RenderFence = VK_NULL_HANDLE;
fd->CommandBuffer = VK_NULL_HANDLE;
fd->CommandPool = VK_NULL_HANDLE;

vkDestroyImageView(device, fd->BackbufferView, allocator);
vkDestroyFramebuffer(device, fd->Framebuffer, allocator);
}

void ImGui_ImplVulkanH_DestroyFrameSemaphores(VkDevice device, ImGui_ImplVulkanH_FrameSemaphores* fsd, const VkAllocationCallbacks* allocator)
{
vkDestroySemaphore(device, fsd->ImageAcquiredSemaphore, allocator);
vkDestroySemaphore(device, fsd->RenderCompleteSemaphore, allocator);
fsd->ImageAcquiredSemaphore = fsd->RenderCompleteSemaphore = VK_NULL_HANDLE;
}

void ImGui_ImplVulkanH_DestroyAllViewportsRenderBuffers(VkDevice device, const VkAllocationCallbacks* allocator)
{
ImGuiPlatformIO& platform_io = ImGui::GetPlatformIO();
Expand Down Expand Up @@ -1996,11 +1994,11 @@ static void ImGui_ImplVulkan_RenderWindow(ImGuiViewport* viewport, void*)
vd->SwapChainNeedRebuild = vd->SwapChainSuboptimal = false;
}

ImGui_ImplVulkanH_Frame* fd = nullptr;
ImGui_ImplVulkanH_FrameSemaphores* fsd = &wd->FrameSemaphores[wd->SemaphoreIndex];
ImGui_ImplVulkanH_Frame* fd = &wd->Frames[wd->CurrentFrameIndex];
{
{
err = vkAcquireNextImageKHR(v->Device, wd->Swapchain, UINT64_MAX, fsd->ImageAcquiredSemaphore, VK_NULL_HANDLE, &wd->FrameIndex);
err = vkAcquireNextImageKHR(v->Device, wd->Swapchain, UINT64_MAX, fd->ImageAcquiredSemaphore, VK_NULL_HANDLE, &wd->SwapchainImageIndex);

if (err == VK_ERROR_OUT_OF_DATE_KHR)
{
vd->SwapChainNeedRebuild = true; // Since we are not going to swap this frame anyway, it's ok that recreation happens on next frame.
Expand All @@ -2010,11 +2008,10 @@ static void ImGui_ImplVulkan_RenderWindow(ImGuiViewport* viewport, void*)
vd->SwapChainSuboptimal = true;
else
check_vk_result(err);
fd = &wd->Frames[wd->FrameIndex];
}
for (;;)
{
err = vkWaitForFences(v->Device, 1, &fd->Fence, VK_TRUE, 100);
err = vkWaitForFences(v->Device, 1, &fd->RenderFence, VK_TRUE, 100);
if (err == VK_SUCCESS) break;
if (err == VK_TIMEOUT) continue;
check_vk_result(err);
Expand Down Expand Up @@ -2112,18 +2109,18 @@ static void ImGui_ImplVulkan_RenderWindow(ImGuiViewport* viewport, void*)
VkSubmitInfo info = {};
info.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO;
info.waitSemaphoreCount = 1;
info.pWaitSemaphores = &fsd->ImageAcquiredSemaphore;
info.pWaitSemaphores = &fd->ImageAcquiredSemaphore;
info.pWaitDstStageMask = &wait_stage;
info.commandBufferCount = 1;
info.pCommandBuffers = &fd->CommandBuffer;
info.signalSemaphoreCount = 1;
info.pSignalSemaphores = &fsd->RenderCompleteSemaphore;
info.pSignalSemaphores = &wd->RenderSemaphores[wd->SwapchainImageIndex];

err = vkEndCommandBuffer(fd->CommandBuffer);
check_vk_result(err);
err = vkResetFences(v->Device, 1, &fd->Fence);
err = vkResetFences(v->Device, 1, &fd->RenderFence);
check_vk_result(err);
err = vkQueueSubmit(v->Queue, 1, &info, fd->Fence);
err = vkQueueSubmit(v->Queue, 1, &info, fd->RenderFence);
check_vk_result(err);
}
}
Expand All @@ -2140,16 +2137,15 @@ static void ImGui_ImplVulkan_SwapBuffers(ImGuiViewport* viewport, void*)
return;

VkResult err;
uint32_t present_index = wd->FrameIndex;

ImGui_ImplVulkanH_FrameSemaphores* fsd = &wd->FrameSemaphores[wd->SemaphoreIndex];
ImGui_ImplVulkanH_Frame* fd = &wd->Frames[wd->CurrentFrameIndex];

VkPresentInfoKHR info = {};
info.sType = VK_STRUCTURE_TYPE_PRESENT_INFO_KHR;
info.waitSemaphoreCount = 1;
info.pWaitSemaphores = &fsd->RenderCompleteSemaphore;
info.pWaitSemaphores = &wd->RenderSemaphores[wd->SwapchainImageIndex];
info.swapchainCount = 1;
info.pSwapchains = &wd->Swapchain;
info.pImageIndices = &present_index;
info.pImageIndices = &wd->SwapchainImageIndex;
err = vkQueuePresentKHR(v->Queue, &info);
if (err == VK_ERROR_OUT_OF_DATE_KHR)
{
Expand All @@ -2160,7 +2156,7 @@ static void ImGui_ImplVulkan_SwapBuffers(ImGuiViewport* viewport, void*)
vd->SwapChainSuboptimal = true;
else
check_vk_result(err);
wd->SemaphoreIndex = (wd->SemaphoreIndex + 1) % wd->SemaphoreCount; // Now we can use the next set of semaphores
wd->CurrentFrameIndex = (wd->CurrentFrameIndex + 1) % wd->ImageCount; // Now we can use the next set of frame data
}

void ImGui_ImplVulkan_InitMultiViewportSupport()
Expand Down
16 changes: 5 additions & 11 deletions backends/imgui_impl_vulkan.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,18 +179,13 @@ struct ImGui_ImplVulkanH_Frame
{
VkCommandPool CommandPool;
VkCommandBuffer CommandBuffer;
VkFence Fence;
VkSemaphore ImageAcquiredSemaphore;
VkFence RenderFence;
VkImage Backbuffer;
VkImageView BackbufferView;
VkFramebuffer Framebuffer;
};

struct ImGui_ImplVulkanH_FrameSemaphores
{
VkSemaphore ImageAcquiredSemaphore;
VkSemaphore RenderCompleteSemaphore;
};

// Helper structure to hold the data needed by one rendering context into one OS window
// (Used by example's main.cpp. Used by multi-viewport features. Probably NOT used by your own engine/app.)
struct ImGui_ImplVulkanH_Window
Expand All @@ -205,12 +200,11 @@ struct ImGui_ImplVulkanH_Window
bool UseDynamicRendering;
bool ClearEnable;
VkClearValue ClearValue;
uint32_t FrameIndex; // Current frame being rendered to (0 <= FrameIndex < FrameInFlightCount)
uint32_t SwapchainImageIndex; // Current swapchain image index being rendered to (0 <= SwapchainImageIndex < ImageCount)
uint32_t ImageCount; // Number of simultaneous in-flight frames (returned by vkGetSwapchainImagesKHR, usually derived from min_image_count)
uint32_t SemaphoreCount; // Number of simultaneous in-flight frames + 1, to be able to use it in vkAcquireNextImageKHR
uint32_t SemaphoreIndex; // Current set of swapchain wait semaphores we're using (needs to be distinct from per frame data)
uint32_t CurrentFrameIndex; // Current frame-in-flight index
ImVector<ImGui_ImplVulkanH_Frame> Frames;
ImVector<ImGui_ImplVulkanH_FrameSemaphores> FrameSemaphores;
ImVector<VkSemaphore> RenderSemaphores; // Semaphores to signal when rendering is done for each frame, indexed by SwapchainImageIndex to be sure the semaphore is safe to use again for that swapchain image.

ImGui_ImplVulkanH_Window()
{
Expand Down