Skip to content

Commit a85f748

Browse files
authored
Merge pull request #20376 from hrydgard/vulkan-semaphore-fix
Vulkan semaphore fix
2 parents 3b47a22 + 7792e01 commit a85f748

File tree

7 files changed

+81
-78
lines changed

7 files changed

+81
-78
lines changed

Common/GPU/Vulkan/VulkanContext.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1415,7 +1415,7 @@ bool VulkanContext::InitSwapchain() {
14151415
desiredNumberOfSwapChainImages = surfCapabilities_.maxImageCount;
14161416
}
14171417

1418-
INFO_LOG(Log::G3D, "Chosen present mode: %d (%s). numSwapChainImages: %d/%d",
1418+
INFO_LOG(Log::G3D, "Chosen present mode: %d (%s). numSwapChainImages: %d (max: %d)",
14191419
swapchainPresentMode, VulkanPresentModeToString(swapchainPresentMode),
14201420
desiredNumberOfSwapChainImages, surfCapabilities_.maxImageCount);
14211421

Common/GPU/Vulkan/VulkanFrameData.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,9 @@ void FrameData::Init(VulkanContext *vulkan, int index) {
2121
this->index = index;
2222
VkDevice device = vulkan->GetDevice();
2323

24-
VkSemaphoreCreateInfo semaphoreCreateInfo = { VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO };
25-
semaphoreCreateInfo.flags = 0;
24+
static const VkSemaphoreCreateInfo semaphoreCreateInfo = { VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO };
2625
VkResult res = vkCreateSemaphore(vulkan->GetDevice(), &semaphoreCreateInfo, nullptr, &acquireSemaphore);
2726
_dbg_assert_(res == VK_SUCCESS);
28-
res = vkCreateSemaphore(vulkan->GetDevice(), &semaphoreCreateInfo, nullptr, &renderingCompleteSemaphore);
29-
_dbg_assert_(res == VK_SUCCESS);
3027

3128
VkCommandPoolCreateInfo cmd_pool_info = { VK_STRUCTURE_TYPE_COMMAND_POOL_CREATE_INFO };
3229
cmd_pool_info.queueFamilyIndex = vulkan->GetGraphicsQueueFamilyIndex();
@@ -69,7 +66,6 @@ void FrameData::Destroy(VulkanContext *vulkan) {
6966
vkDestroyFence(device, fence, nullptr);
7067
vkDestroyQueryPool(device, profile.queryPool, nullptr);
7168
vkDestroySemaphore(device, acquireSemaphore, nullptr);
72-
vkDestroySemaphore(device, renderingCompleteSemaphore, nullptr);
7369

7470
readbacks_.IterateMut([=](const ReadbackKey &key, CachedReadback *value) {
7571
value->Destroy(vulkan);
@@ -120,7 +116,7 @@ VkResult FrameData::QueuePresent(VulkanContext *vulkan, FrameDataShared &shared)
120116
present.swapchainCount = 1;
121117
present.pSwapchains = &swapchain;
122118
present.pImageIndices = &curSwapchainImage;
123-
present.pWaitSemaphores = &renderingCompleteSemaphore;
119+
present.pWaitSemaphores = &shared.swapchainImages_[curSwapchainImage].renderingCompleteSemaphore;
124120
present.waitSemaphoreCount = 1;
125121

126122
// Can't move these into the if.
@@ -232,7 +228,7 @@ void FrameData::Submit(VulkanContext *vulkan, FrameSubmitType type, FrameDataSha
232228
submit_info.pCommandBuffers = cmdBufs;
233229
if (type == FrameSubmitType::FinishFrame && !skipSwap) {
234230
submit_info.signalSemaphoreCount = 1;
235-
submit_info.pSignalSemaphores = &renderingCompleteSemaphore;
231+
submit_info.pSignalSemaphores = &sharedData.swapchainImages_[curSwapchainImage].renderingCompleteSemaphore;
236232
}
237233

238234
VkResult res;

Common/GPU/Vulkan/VulkanFrameData.h

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,22 @@ struct CachedReadback {
5252
void Destroy(VulkanContext *vulkan);
5353
};
5454

55+
// Swap chain management
56+
struct SwapchainImageData {
57+
VkImage image;
58+
VkImageView view;
59+
VkSemaphore renderingCompleteSemaphore = VK_NULL_HANDLE;
60+
};
61+
5562
struct FrameDataShared {
5663
// For synchronous readbacks.
5764
VkFence readbackFence = VK_NULL_HANDLE;
5865
bool useMultiThreading = false;
5966
bool measurePresentTime = false;
6067

68+
std::vector<SwapchainImageData> swapchainImages_;
69+
uint32_t swapchainImageCount_ = 0;
70+
6171
void Init(VulkanContext *vulkan, bool useMultiThreading, bool measurePresentTime);
6272
void Destroy(VulkanContext *vulkan);
6373
};
@@ -78,7 +88,6 @@ struct FrameData {
7888

7989
VkFence fence = VK_NULL_HANDLE;
8090
VkSemaphore acquireSemaphore = VK_NULL_HANDLE;
81-
VkSemaphore renderingCompleteSemaphore = VK_NULL_HANDLE;
8291

8392
// These are on different threads so need separate pools.
8493
VkCommandPool cmdPoolInit = VK_NULL_HANDLE; // Written to from main thread

Common/GPU/Vulkan/VulkanQueueRunner.cpp

Lines changed: 5 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -79,56 +79,7 @@ void VulkanQueueRunner::DestroyDeviceObjects() {
7979
renderPasses_.Clear();
8080
}
8181

82-
bool VulkanQueueRunner::CreateSwapchain(VkCommandBuffer cmdInit, VulkanBarrierBatch *barriers) {
83-
VkResult res = vkGetSwapchainImagesKHR(vulkan_->GetDevice(), vulkan_->GetSwapchain(), &swapchainImageCount_, nullptr);
84-
_dbg_assert_(res == VK_SUCCESS);
85-
86-
VkImage *swapchainImages = new VkImage[swapchainImageCount_];
87-
res = vkGetSwapchainImagesKHR(vulkan_->GetDevice(), vulkan_->GetSwapchain(), &swapchainImageCount_, swapchainImages);
88-
if (res != VK_SUCCESS) {
89-
ERROR_LOG(Log::G3D, "vkGetSwapchainImagesKHR failed");
90-
delete[] swapchainImages;
91-
return false;
92-
}
93-
94-
for (uint32_t i = 0; i < swapchainImageCount_; i++) {
95-
SwapchainImageData sc_buffer{};
96-
sc_buffer.image = swapchainImages[i];
97-
98-
VkImageViewCreateInfo color_image_view = { VK_STRUCTURE_TYPE_IMAGE_VIEW_CREATE_INFO };
99-
color_image_view.format = vulkan_->GetSwapchainFormat();
100-
color_image_view.components.r = VK_COMPONENT_SWIZZLE_IDENTITY;
101-
color_image_view.components.g = VK_COMPONENT_SWIZZLE_IDENTITY;
102-
color_image_view.components.b = VK_COMPONENT_SWIZZLE_IDENTITY;
103-
color_image_view.components.a = VK_COMPONENT_SWIZZLE_IDENTITY;
104-
color_image_view.subresourceRange.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT;
105-
color_image_view.subresourceRange.baseMipLevel = 0;
106-
color_image_view.subresourceRange.levelCount = 1;
107-
color_image_view.subresourceRange.baseArrayLayer = 0;
108-
color_image_view.subresourceRange.layerCount = 1; // TODO: Investigate hw-assisted stereo.
109-
color_image_view.viewType = VK_IMAGE_VIEW_TYPE_2D;
110-
color_image_view.flags = 0;
111-
color_image_view.image = sc_buffer.image;
112-
113-
// We leave the images as UNDEFINED, there's no need to pre-transition them as
114-
// the backbuffer renderpass starts out with them being auto-transitioned from UNDEFINED anyway.
115-
// Also, turns out it's illegal to transition un-acquired images, thanks Hans-Kristian. See #11417.
116-
117-
res = vkCreateImageView(vulkan_->GetDevice(), &color_image_view, nullptr, &sc_buffer.view);
118-
vulkan_->SetDebugName(sc_buffer.view, VK_OBJECT_TYPE_IMAGE_VIEW, "swapchain_view");
119-
swapchainImages_.push_back(sc_buffer);
120-
_dbg_assert_(res == VK_SUCCESS);
121-
}
122-
delete[] swapchainImages;
123-
124-
// Must be before InitBackbufferRenderPass.
125-
if (InitDepthStencilBuffer(cmdInit, barriers)) {
126-
InitBackbufferFramebuffers(vulkan_->GetBackbufferWidth(), vulkan_->GetBackbufferHeight());
127-
}
128-
return true;
129-
}
130-
131-
bool VulkanQueueRunner::InitBackbufferFramebuffers(int width, int height) {
82+
bool VulkanQueueRunner::InitBackbufferFramebuffers(int width, int height, FrameDataShared &frameDataShared) {
13283
VkResult res;
13384
// We share the same depth buffer but have multiple color buffers, see the loop below.
13485
VkImageView attachments[2] = { VK_NULL_HANDLE, depth_.view };
@@ -141,10 +92,10 @@ bool VulkanQueueRunner::InitBackbufferFramebuffers(int width, int height) {
14192
fb_info.height = height;
14293
fb_info.layers = 1;
14394

144-
framebuffers_.resize(swapchainImageCount_);
95+
framebuffers_.resize(frameDataShared.swapchainImageCount_);
14596

146-
for (uint32_t i = 0; i < swapchainImageCount_; i++) {
147-
attachments[0] = swapchainImages_[i].view;
97+
for (uint32_t i = 0; i < frameDataShared.swapchainImageCount_; i++) {
98+
attachments[0] = frameDataShared.swapchainImages_[i].view;
14899
res = vkCreateFramebuffer(vulkan_->GetDevice(), &fb_info, nullptr, &framebuffers_[i]);
149100
_dbg_assert_(res == VK_SUCCESS);
150101
if (res != VK_SUCCESS) {
@@ -225,11 +176,6 @@ bool VulkanQueueRunner::InitDepthStencilBuffer(VkCommandBuffer cmd, VulkanBarrie
225176

226177

227178
void VulkanQueueRunner::DestroyBackBuffers() {
228-
for (auto &image : swapchainImages_) {
229-
vulkan_->Delete().QueueDeleteImageView(image.view);
230-
}
231-
swapchainImages_.clear();
232-
233179
if (depth_.view) {
234180
vulkan_->Delete().QueueDeleteImageView(depth_.view);
235181
}
@@ -373,7 +319,7 @@ void VulkanQueueRunner::RunSteps(std::vector<VKRStep *> &steps, int curFrame, Fr
373319
// So only acquire once.
374320
if (!frameData.hasAcquired) {
375321
frameData.AcquireNextImage(vulkan_);
376-
SetBackbuffer(framebuffers_[frameData.curSwapchainImage], swapchainImages_[frameData.curSwapchainImage].image);
322+
SetBackbuffer(framebuffers_[frameData.curSwapchainImage], frameDataShared.swapchainImages_[frameData.curSwapchainImage].image);
377323
}
378324

379325
if (!frameData.hasPresentCommands) {

Common/GPU/Vulkan/VulkanQueueRunner.h

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -267,9 +267,9 @@ class VulkanQueueRunner {
267267
hacksEnabled_ = hacks;
268268
}
269269

270-
private:
271-
bool InitBackbufferFramebuffers(int width, int height);
270+
bool InitBackbufferFramebuffers(int width, int height, FrameDataShared &frameDataShared);
272271
bool InitDepthStencilBuffer(VkCommandBuffer cmd, VulkanBarrierBatch *barriers); // Used for non-buffered rendering.
272+
private:
273273

274274
VKRRenderPass *PerformBindFramebufferAsRenderTarget(const VKRStep &pass, VkCommandBuffer cmd);
275275
void PerformRenderPass(const VKRStep &pass, VkCommandBuffer cmd, int curFrame, QueueProfileContext &profile);
@@ -316,14 +316,7 @@ class VulkanQueueRunner {
316316

317317
VulkanBarrierBatch recordBarrier_;
318318

319-
// Swap chain management
320-
struct SwapchainImageData {
321-
VkImage image;
322-
VkImageView view;
323-
};
324319
std::vector<VkFramebuffer> framebuffers_;
325-
std::vector<SwapchainImageData> swapchainImages_;
326-
uint32_t swapchainImageCount_ = 0;
327320
struct DepthBufferInfo {
328321
VkFormat format = VK_FORMAT_UNDEFINED;
329322
VkImage image = VK_NULL_HANDLE;

Common/GPU/Vulkan/VulkanRenderManager.cpp

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ bool VulkanRenderManager::CreateBackbuffers() {
323323

324324
VkCommandBuffer cmdInit = GetInitCmd();
325325

326-
if (!queueRunner_.CreateSwapchain(cmdInit, &postInitBarrier_)) {
326+
if (!CreateSwapchain(cmdInit, &postInitBarrier_, frameDataShared_)) {
327327
return false;
328328
}
329329

@@ -354,6 +354,58 @@ bool VulkanRenderManager::CreateBackbuffers() {
354354
return true;
355355
}
356356

357+
bool VulkanRenderManager::CreateSwapchain(VkCommandBuffer cmdInit, VulkanBarrierBatch *barriers, FrameDataShared &frameDataShared) {
358+
VkResult res = vkGetSwapchainImagesKHR(vulkan_->GetDevice(), vulkan_->GetSwapchain(), &frameDataShared.swapchainImageCount_, nullptr);
359+
_dbg_assert_(res == VK_SUCCESS);
360+
361+
VkImage *swapchainImages = new VkImage[frameDataShared.swapchainImageCount_];
362+
res = vkGetSwapchainImagesKHR(vulkan_->GetDevice(), vulkan_->GetSwapchain(), &frameDataShared.swapchainImageCount_, swapchainImages);
363+
if (res != VK_SUCCESS) {
364+
ERROR_LOG(Log::G3D, "vkGetSwapchainImagesKHR failed");
365+
delete[] swapchainImages;
366+
return false;
367+
}
368+
369+
static const VkSemaphoreCreateInfo semaphoreCreateInfo = { VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO };
370+
for (uint32_t i = 0; i < frameDataShared.swapchainImageCount_; i++) {
371+
SwapchainImageData sc_buffer{};
372+
sc_buffer.image = swapchainImages[i];
373+
res = vkCreateSemaphore(vulkan_->GetDevice(), &semaphoreCreateInfo, nullptr, &sc_buffer.renderingCompleteSemaphore);
374+
_dbg_assert_(res == VK_SUCCESS);
375+
376+
VkImageViewCreateInfo color_image_view = { VK_STRUCTURE_TYPE_IMAGE_VIEW_CREATE_INFO };
377+
color_image_view.format = vulkan_->GetSwapchainFormat();
378+
color_image_view.components.r = VK_COMPONENT_SWIZZLE_IDENTITY;
379+
color_image_view.components.g = VK_COMPONENT_SWIZZLE_IDENTITY;
380+
color_image_view.components.b = VK_COMPONENT_SWIZZLE_IDENTITY;
381+
color_image_view.components.a = VK_COMPONENT_SWIZZLE_IDENTITY;
382+
color_image_view.subresourceRange.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT;
383+
color_image_view.subresourceRange.baseMipLevel = 0;
384+
color_image_view.subresourceRange.levelCount = 1;
385+
color_image_view.subresourceRange.baseArrayLayer = 0;
386+
color_image_view.subresourceRange.layerCount = 1; // TODO: Investigate hw-assisted stereo.
387+
color_image_view.viewType = VK_IMAGE_VIEW_TYPE_2D;
388+
color_image_view.flags = 0;
389+
color_image_view.image = sc_buffer.image;
390+
391+
// We leave the images as UNDEFINED, there's no need to pre-transition them as
392+
// the backbuffer renderpass starts out with them being auto-transitioned from UNDEFINED anyway.
393+
// Also, turns out it's illegal to transition un-acquired images, thanks Hans-Kristian. See #11417.
394+
395+
res = vkCreateImageView(vulkan_->GetDevice(), &color_image_view, nullptr, &sc_buffer.view);
396+
vulkan_->SetDebugName(sc_buffer.view, VK_OBJECT_TYPE_IMAGE_VIEW, "swapchain_view");
397+
frameDataShared.swapchainImages_.push_back(sc_buffer);
398+
_dbg_assert_(res == VK_SUCCESS);
399+
}
400+
delete[] swapchainImages;
401+
402+
// Must be before InitBackbufferRenderPass.
403+
if (queueRunner_.InitDepthStencilBuffer(cmdInit, barriers)) {
404+
queueRunner_.InitBackbufferFramebuffers(vulkan_->GetBackbufferWidth(), vulkan_->GetBackbufferHeight(), frameDataShared);
405+
}
406+
return true;
407+
}
408+
357409
void VulkanRenderManager::StartThreads() {
358410
{
359411
std::unique_lock<std::mutex> lock(compileQueueMutex_);
@@ -430,6 +482,12 @@ void VulkanRenderManager::DestroyBackbuffers() {
430482
StopThreads();
431483
vulkan_->WaitUntilQueueIdle();
432484

485+
for (auto &image : frameDataShared_.swapchainImages_) {
486+
vulkan_->Delete().QueueDeleteImageView(image.view);
487+
vkDestroySemaphore(vulkan_->GetDevice(), image.renderingCompleteSemaphore, nullptr);
488+
}
489+
frameDataShared_.swapchainImages_.clear();
490+
433491
queueRunner_.DestroyBackBuffers();
434492
}
435493

Common/GPU/Vulkan/VulkanRenderManager.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,7 @@ class VulkanRenderManager {
561561
void FlushDescriptors(int frame);
562562

563563
void SanityCheckPassesOnAdd();
564+
bool CreateSwapchain(VkCommandBuffer cmdInit, VulkanBarrierBatch *barriers, FrameDataShared &frameDataShared);
564565

565566
FrameDataShared frameDataShared_;
566567

0 commit comments

Comments
 (0)