rhi: vulkan: Simplify frame sync

imageFence seems to have no purpose. And some of the
comments seem to be out of date / misleading.

More importantly, do the command buffer completion wait
before the acquireNextImage. Otherwise frame.imageSem
may have wait operations pending if the queueSubmit
did not get to wait on it yet. The expectation is that
this potential issue is what triggers the recent
validation layer releases' enhanced synchronization
checkers.

Pick-to: 6.8
Fixes: QTBUG-132808
Change-Id: Ib8e3884ccea343f5a839aa2ee17cd358fafeac69
Reviewed-by: Andy Nichols <andy.nichols@qt.io>
(cherry picked from commit 24d62ffd720b5bec5d07b07b8d2c9dda7635f3c0)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Laszlo Agocs 2025-01-15 15:29:32 +01:00 committed by Qt Cherry-pick Bot
parent c4eacb70fd
commit b96994c9eb
2 changed files with 12 additions and 34 deletions

View File

@ -2357,9 +2357,6 @@ bool QRhiVulkan::recreateSwapChain(QRhiSwapChain *swapChain)
frame.imageAcquired = false; frame.imageAcquired = false;
frame.imageSemWaitable = false; frame.imageSemWaitable = false;
df->vkCreateFence(dev, &fenceInfo, nullptr, &frame.imageFence);
frame.imageFenceWaitable = true; // fence was created in signaled state
df->vkCreateSemaphore(dev, &semInfo, nullptr, &frame.imageSem); df->vkCreateSemaphore(dev, &semInfo, nullptr, &frame.imageSem);
df->vkCreateSemaphore(dev, &semInfo, nullptr, &frame.drawSem); df->vkCreateSemaphore(dev, &semInfo, nullptr, &frame.drawSem);
@ -2395,13 +2392,6 @@ void QRhiVulkan::releaseSwapChainResources(QRhiSwapChain *swapChain)
frame.cmdFence = VK_NULL_HANDLE; frame.cmdFence = VK_NULL_HANDLE;
frame.cmdFenceWaitable = false; frame.cmdFenceWaitable = false;
} }
if (frame.imageFence) {
if (frame.imageFenceWaitable)
df->vkWaitForFences(dev, 1, &frame.imageFence, VK_TRUE, UINT64_MAX);
df->vkDestroyFence(dev, frame.imageFence, nullptr);
frame.imageFence = VK_NULL_HANDLE;
frame.imageFenceWaitable = false;
}
if (frame.imageSem) { if (frame.imageSem) {
df->vkDestroySemaphore(dev, frame.imageSem, nullptr); df->vkDestroySemaphore(dev, frame.imageSem, nullptr);
frame.imageSem = VK_NULL_HANDLE; frame.imageSem = VK_NULL_HANDLE;
@ -2488,24 +2478,26 @@ QRhi::FrameOpResult QRhiVulkan::beginFrame(QRhiSwapChain *swapChain, QRhi::Begin
inst->handle()->beginFrame(swapChainD->window); inst->handle()->beginFrame(swapChainD->window);
if (!frame.imageAcquired) { // Make sure the previous commands for the same frame slot have finished.
// Wait if we are too far ahead, i.e. the thread gets throttled based on the presentation rate //
// (note that we are using FIFO mode -> vsync) // Do this also for any other swapchain's commands with the same frame slot
if (frame.imageFenceWaitable) { // While this reduces concurrency, it keeps resource usage safe: swapchain
df->vkWaitForFences(dev, 1, &frame.imageFence, VK_TRUE, UINT64_MAX); // A starting its frame 0, followed by swapchain B starting its own frame 0
df->vkResetFences(dev, 1, &frame.imageFence); // will make B wait for A's frame 0 commands, so if a resource is written
frame.imageFenceWaitable = false; // in B's frame or when B checks for pending resource releases, that won't
} // mess up A's in-flight commands (as they are not in flight anymore).
waitCommandCompletion(frameResIndex);
if (!frame.imageAcquired) {
// move on to next swapchain image // move on to next swapchain image
uint32_t imageIndex = 0; uint32_t imageIndex = 0;
VkResult err = vkAcquireNextImageKHR(dev, swapChainD->sc, UINT64_MAX, VkResult err = vkAcquireNextImageKHR(dev, swapChainD->sc, UINT64_MAX,
frame.imageSem, frame.imageFence, &imageIndex); frame.imageSem, VK_NULL_HANDLE, &imageIndex);
if (err == VK_SUCCESS || err == VK_SUBOPTIMAL_KHR) { if (err == VK_SUCCESS || err == VK_SUBOPTIMAL_KHR) {
swapChainD->currentImageIndex = imageIndex; swapChainD->currentImageIndex = imageIndex;
frame.imageSemWaitable = true; frame.imageSemWaitable = true;
frame.imageAcquired = true; frame.imageAcquired = true;
frame.imageFenceWaitable = true;
} else if (err == VK_ERROR_OUT_OF_DATE_KHR) { } else if (err == VK_ERROR_OUT_OF_DATE_KHR) {
return QRhi::FrameOpSwapChainOutOfDate; return QRhi::FrameOpSwapChainOutOfDate;
} else { } else {
@ -2519,18 +2511,6 @@ QRhi::FrameOpResult QRhiVulkan::beginFrame(QRhiSwapChain *swapChain, QRhi::Begin
} }
} }
// Make sure the previous commands for the same image have finished. (note
// that this is based on the fence from the command buffer submit, nothing
// to do with the Present)
//
// Do this also for any other swapchain's commands with the same frame slot
// While this reduces concurrency, it keeps resource usage safe: swapchain
// A starting its frame 0, followed by swapchain B starting its own frame 0
// will make B wait for A's frame 0 commands, so if a resource is written
// in B's frame or when B checks for pending resource releases, that won't
// mess up A's in-flight commands (as they are not in flight anymore).
waitCommandCompletion(frameResIndex);
currentFrameSlot = int(swapChainD->currentFrameSlot); currentFrameSlot = int(swapChainD->currentFrameSlot);
currentSwapChain = swapChainD; currentSwapChain = swapChainD;
if (swapChainD->ds) if (swapChainD->ds)

View File

@ -653,8 +653,6 @@ struct QVkSwapChain : public QRhiSwapChain
QVarLengthArray<ImageResources, EXPECTED_MAX_BUFFER_COUNT> imageRes; QVarLengthArray<ImageResources, EXPECTED_MAX_BUFFER_COUNT> imageRes;
struct FrameResources { struct FrameResources {
VkFence imageFence = VK_NULL_HANDLE;
bool imageFenceWaitable = false;
VkSemaphore imageSem = VK_NULL_HANDLE; VkSemaphore imageSem = VK_NULL_HANDLE;
VkSemaphore drawSem = VK_NULL_HANDLE; VkSemaphore drawSem = VK_NULL_HANDLE;
bool imageAcquired = false; bool imageAcquired = false;