From 8edc6eaf5efa62f365445a0ac51aac58686859b5 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 4 Nov 2017 20:44:11 -0700 Subject: [PATCH 01/11] Vulkan: Fix segfault on swapchain fail. This at least allows us to detect that the backend failed to init. Happens when switching backends with debugger attached (probably driver bug?) --- Common/Vulkan/VulkanContext.cpp | 1 + Common/Vulkan/VulkanContext.h | 2 +- Windows/GPU/WindowsVulkanContext.cpp | 3 ++- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Common/Vulkan/VulkanContext.cpp b/Common/Vulkan/VulkanContext.cpp index 3fcfcc49cb..de0000c382 100644 --- a/Common/Vulkan/VulkanContext.cpp +++ b/Common/Vulkan/VulkanContext.cpp @@ -137,6 +137,7 @@ VkResult VulkanContext::CreateInstance(const char *app_name, int app_ver, uint32 if (res != VK_SUCCESS) { init_error_ = "Failed to enumerate physical devices"; vkDestroyInstance(instance_, nullptr); + instance_ = nullptr; return res; } diff --git a/Common/Vulkan/VulkanContext.h b/Common/Vulkan/VulkanContext.h index f647f4d0f2..64b17f229e 100644 --- a/Common/Vulkan/VulkanContext.h +++ b/Common/Vulkan/VulkanContext.h @@ -378,7 +378,7 @@ private: std::vector msg_callbacks; - VkSwapchainKHR swapchain_; + VkSwapchainKHR swapchain_ = VK_NULL_HANDLE; VkFormat swapchainFormat_; uint32_t queue_count = 0; diff --git a/Windows/GPU/WindowsVulkanContext.cpp b/Windows/GPU/WindowsVulkanContext.cpp index 21dcb026ea..f8263b565e 100644 --- a/Windows/GPU/WindowsVulkanContext.cpp +++ b/Windows/GPU/WindowsVulkanContext.cpp @@ -208,7 +208,8 @@ bool WindowsVulkanContext::Init(HINSTANCE hInst, HWND hWnd, std::string *error_m } void WindowsVulkanContext::Shutdown() { - draw_->HandleEvent(Draw::Event::LOST_BACKBUFFER, g_Vulkan->GetBackbufferWidth(), g_Vulkan->GetBackbufferHeight()); + if (draw_) + draw_->HandleEvent(Draw::Event::LOST_BACKBUFFER, g_Vulkan->GetBackbufferWidth(), g_Vulkan->GetBackbufferHeight()); delete draw_; draw_ = nullptr; From ec0f640f569a2cc6367403f8bfde7f5e5782253b Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 4 Nov 2017 21:04:07 -0700 Subject: [PATCH 02/11] Vulkan: Trigger condvar only at frame end. --- ext/native/thin3d/VulkanRenderManager.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index bd9692c55e..4e54b21de4 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -640,7 +640,8 @@ void VulkanRenderManager::Submit(int frame, bool triggerFence) { assert(res == VK_SUCCESS); } - if (useThread) { + // TODO: If !triggerFence, we're actually not using the thread right now. + if (useThread && triggerFence) { VLOG("PULL: Frame %d.readyForFence = true", frame); std::unique_lock lock(frameData.push_mutex); frameData.readyForFence = true; From 8b55940a3de0f5a0dcdc32bbd1a0596f59353d79 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 4 Nov 2017 22:21:47 -0700 Subject: [PATCH 03/11] Vulkan: Fix out-of-sync frames on threading. We end up with a second thread start at frame 1, so the thread needs to start at frame 1 too. --- ext/native/thin3d/VulkanRenderManager.cpp | 23 +++++++++++++++++++---- ext/native/thin3d/VulkanRenderManager.h | 1 + 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index 4e54b21de4..c2fbf46f93 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -179,7 +179,15 @@ void VulkanRenderManager::CreateBackbuffers() { // Start the thread. if (useThread) { + for (int i = 0; i < vulkan_->GetInflightFrames(); i++) { + // Reset all the frameData. Might be dirty from previous thread stop. + frameData_[i].readyForRun = false; + frameData_[i].readyForFence = true; + } + run_ = true; + // Won't necessarily be 0. + threadInitFrame_ = vulkan_->GetCurFrame(); thread_ = std::thread(&VulkanRenderManager::ThreadFunc, this); } } @@ -236,12 +244,15 @@ VulkanRenderManager::~VulkanRenderManager() { // TODO: Activate this code. void VulkanRenderManager::ThreadFunc() { setCurrentThreadName("RenderMan"); - int threadFrame = -1; // Increment first, start at 0. + int threadFrame = threadInitFrame_; + bool nextFrame = false; while (run_) { { - threadFrame++; - if (threadFrame >= vulkan_->GetInflightFrames()) - threadFrame = 0; + if (nextFrame) { + threadFrame++; + if (threadFrame >= vulkan_->GetInflightFrames()) + threadFrame = 0; + } FrameData &frameData = frameData_[threadFrame]; std::unique_lock lock(frameData.pull_mutex); while (!frameData.readyForRun && run_) { @@ -252,10 +263,14 @@ void VulkanRenderManager::ThreadFunc() { frameData.readyForRun = false; if (!run_) // quick exit if bailing. return; + + nextFrame = true; } VLOG("PULL: Running frame %d", threadFrame); Run(threadFrame); + VLOG("PULL: Finished frame %d", threadFrame); } + VLOG("PULL: Quitting"); } void VulkanRenderManager::BeginFrame() { diff --git a/ext/native/thin3d/VulkanRenderManager.h b/ext/native/thin3d/VulkanRenderManager.h index 44d34c6cc1..53a90a1bcc 100644 --- a/ext/native/thin3d/VulkanRenderManager.h +++ b/ext/native/thin3d/VulkanRenderManager.h @@ -249,6 +249,7 @@ private: VulkanContext *vulkan_; std::thread thread_; std::mutex mutex_; + int threadInitFrame_ = 0; VulkanQueueRunner queueRunner_; // Swap chain management From 97fa0a7461eff5cea48748bf3b056db5e7245435 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 4 Nov 2017 22:23:01 -0700 Subject: [PATCH 04/11] Vulkan: Allow sync when using threading. --- ext/native/thin3d/VulkanRenderManager.cpp | 68 ++++++++++++++++++----- ext/native/thin3d/VulkanRenderManager.h | 9 ++- 2 files changed, 61 insertions(+), 16 deletions(-) diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index c2fbf46f93..e701d12f12 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -264,7 +264,9 @@ void VulkanRenderManager::ThreadFunc() { if (!run_) // quick exit if bailing. return; - nextFrame = true; + // Only increment next time if we're done. + nextFrame = frameData.type == VKRRunType::END; + assert(frameData.type == VKRRunType::END || frameData.type == VKRRunType::SYNC); } VLOG("PULL: Running frame %d", threadFrame); Run(threadFrame); @@ -560,12 +562,14 @@ void VulkanRenderManager::Finish() { FrameData &frameData = frameData_[curFrame]; if (!useThread) { frameData.steps = std::move(steps_); + frameData.type = VKRRunType::END; Run(curFrame); } else { std::unique_lock lock(frameData.pull_mutex); VLOG("PUSH: Frame[%d].readyForRun = true", curFrame); frameData.steps = std::move(steps_); frameData.readyForRun = true; + frameData.type = VKRRunType::END; frameData.pull_condVar.notify_all(); } vulkan_->EndFrame(); @@ -690,8 +694,6 @@ void VulkanRenderManager::EndSubmitFrame(int frame) { } void VulkanRenderManager::Run(int frame) { - VkDevice device = vulkan_->GetDevice(); - BeginSubmitFrame(frame); FrameData &frameData = frameData_[frame]; @@ -701,23 +703,24 @@ void VulkanRenderManager::Run(int frame) { queueRunner_.RunSteps(cmd, stepsOnThread); stepsOnThread.clear(); - EndSubmitFrame(frame); + switch (frameData.type) { + case VKRRunType::END: + EndSubmitFrame(frame); + break; + + case VKRRunType::SYNC: + EndSyncFrame(frame); + break; + + default: + assert(false); + } VLOG("PULL: Finished running frame %d", frame); } -void VulkanRenderManager::FlushSync() { - int frame = vulkan_->GetCurFrame(); - BeginSubmitFrame(frame); - +void VulkanRenderManager::EndSyncFrame(int frame) { FrameData &frameData = frameData_[frame]; - frameData.steps = std::move(steps_); - - auto &stepsOnThread = frameData_[frame].steps; - VkCommandBuffer cmd = frameData.mainCmd; - queueRunner_.RunSteps(cmd, stepsOnThread); - stepsOnThread.clear(); - Submit(frame, false); // This is brutal! Should probably wait for a fence instead, not that it'll matter much since we'll @@ -734,4 +737,39 @@ void VulkanRenderManager::FlushSync() { }; VkResult res = vkBeginCommandBuffer(frameData.mainCmd, &begin); assert(res == VK_SUCCESS); + + if (useThread) { + std::unique_lock lock(frameData.push_mutex); + frameData.readyForFence = true; + frameData.push_condVar.notify_all(); + } +} + +void VulkanRenderManager::FlushSync() { + // TODO: Reset curRenderStep_? + int curFrame = vulkan_->GetCurFrame(); + FrameData &frameData = frameData_[curFrame]; + if (!useThread) { + frameData.steps = std::move(steps_); + frameData.type = VKRRunType::SYNC; + Run(curFrame); + } else { + std::unique_lock lock(frameData.pull_mutex); + VLOG("PUSH: Frame[%d].readyForRun = true (sync)", curFrame); + frameData.steps = std::move(steps_); + frameData.readyForRun = true; + assert(frameData.readyForFence == false); + frameData.type = VKRRunType::SYNC; + frameData.pull_condVar.notify_all(); + } + + if (useThread) { + std::unique_lock lock(frameData.push_mutex); + // Wait for the flush to be hit, since we're syncing. + while (!frameData.readyForFence) { + VLOG("PUSH: Waiting for frame[%d].readyForFence = 1 (sync)", curFrame); + frameData.push_condVar.wait(lock); + } + frameData.readyForFence = false; + } } diff --git a/ext/native/thin3d/VulkanRenderManager.h b/ext/native/thin3d/VulkanRenderManager.h index 53a90a1bcc..50fb2f56d8 100644 --- a/ext/native/thin3d/VulkanRenderManager.h +++ b/ext/native/thin3d/VulkanRenderManager.h @@ -70,6 +70,11 @@ private: VulkanContext *vulkan_; }; +enum class VKRRunType { + END, + SYNC, +}; + class VulkanRenderManager { public: VulkanRenderManager(VulkanContext *vulkan); @@ -206,6 +211,7 @@ private: // Bad for performance but sometimes necessary for synchronous CPU readbacks (screenshots and whatnot). void FlushSync(); + void EndSyncFrame(int frame); // Permanent objects VkSemaphore acquireSemaphore_; @@ -220,8 +226,9 @@ private: std::condition_variable pull_condVar; bool readyForFence = true; - bool readyForRun = false; + VKRRunType type = VKRRunType::END; + VkFence fence; // These are on different threads so need separate pools. VkCommandPool cmdPoolInit; From dc200a4fbcb276f2f4033339abeeb5a8f7cc2438 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 4 Nov 2017 22:29:12 -0700 Subject: [PATCH 05/11] Vulkan: Fix a comment. --- ext/native/thin3d/VulkanRenderManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index e701d12f12..038e85939d 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -659,7 +659,7 @@ void VulkanRenderManager::Submit(int frame, bool triggerFence) { assert(res == VK_SUCCESS); } - // TODO: If !triggerFence, we're actually not using the thread right now. + // When !triggerFence, we notify after syncing with Vulkan. if (useThread && triggerFence) { VLOG("PULL: Frame %d.readyForFence = true", frame); std::unique_lock lock(frameData.push_mutex); From ab9aee9a07a853006eb35c6a3071e5f53e867e3c Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 07:07:14 -0800 Subject: [PATCH 06/11] Vulkan: Account inside frame from push side. We don't want a dependency on the thread state, of course. --- ext/native/thin3d/VulkanRenderManager.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index 038e85939d..ea38d21eaf 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -573,6 +573,8 @@ void VulkanRenderManager::Finish() { frameData.pull_condVar.notify_all(); } vulkan_->EndFrame(); + + insideFrame_ = false; } void VulkanRenderManager::Wipe() { @@ -671,7 +673,6 @@ void VulkanRenderManager::Submit(int frame, bool triggerFence) { void VulkanRenderManager::EndSubmitFrame(int frame) { FrameData &frameData = frameData_[frame]; frameData.hasBegun = false; - insideFrame_ = false; Submit(frame, true); From 2ad9eb047edfe73ffc5e640373b47cc4b440c169 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 08:13:18 -0800 Subject: [PATCH 07/11] Vulkan: Refcount framebuffer deletes. Fixes crash in GoW when using a thread. --- GPU/Common/FramebufferCommon.cpp | 2 +- ext/native/thin3d/VulkanQueueRunner.cpp | 13 +++++++++++++ ext/native/thin3d/VulkanRenderManager.cpp | 19 +++++++++++++++++++ ext/native/thin3d/VulkanRenderManager.h | 14 +++++++++++--- ext/native/thin3d/thin3d_vulkan.cpp | 3 ++- 5 files changed, 46 insertions(+), 5 deletions(-) diff --git a/GPU/Common/FramebufferCommon.cpp b/GPU/Common/FramebufferCommon.cpp index f97b2117a3..371a43e371 100644 --- a/GPU/Common/FramebufferCommon.cpp +++ b/GPU/Common/FramebufferCommon.cpp @@ -1072,7 +1072,7 @@ void FramebufferManagerCommon::DecimateFBOs() { currentRenderVfb_ = 0; for (auto iter : fbosToDelete_) { - delete iter; + iter->Release(); } fbosToDelete_.clear(); diff --git a/ext/native/thin3d/VulkanQueueRunner.cpp b/ext/native/thin3d/VulkanQueueRunner.cpp index e1969f69fa..10948f82b2 100644 --- a/ext/native/thin3d/VulkanQueueRunner.cpp +++ b/ext/native/thin3d/VulkanQueueRunner.cpp @@ -326,6 +326,7 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c vkCmdPipelineBarrier(cmd, srcStage, dstStage, 0, 0, nullptr, 0, nullptr, 1, &barrier); iter.fb->color.layout = barrier.newLayout; + iter.fb->Release(); } } @@ -573,6 +574,10 @@ void VulkanQueueRunner::PerformBindFramebufferAsRenderTarget(const VKRStep &step rp_begin.clearValueCount = numClearVals; rp_begin.pClearValues = numClearVals ? clearVal : nullptr; vkCmdBeginRenderPass(cmd, &rp_begin, VK_SUBPASS_CONTENTS_INLINE); + + if (step.render.framebuffer) { + step.render.framebuffer->Release(); + } } void VulkanQueueRunner::PerformCopy(const VKRStep &step, VkCommandBuffer cmd) { @@ -646,6 +651,9 @@ void VulkanQueueRunner::PerformCopy(const VKRStep &step, VkCommandBuffer cmd) { } vkCmdCopyImage(cmd, src->depth.image, src->depth.layout, dst->depth.image, dst->depth.layout, 1, ©); } + + src->Release(); + dst->Release(); } void VulkanQueueRunner::PerformBlit(const VKRStep &step, VkCommandBuffer cmd) { @@ -729,6 +737,9 @@ void VulkanQueueRunner::PerformBlit(const VKRStep &step, VkCommandBuffer cmd) { } vkCmdBlitImage(cmd, src->depth.image, src->depth.layout, dst->depth.image, dst->depth.layout, 1, &blit, step.blit.filter); } + + src->Release(); + dst->Release(); } void VulkanQueueRunner::SetupTransitionToTransferSrc(VKRImage &img, VkImageMemoryBarrier &barrier, VkPipelineStageFlags &stage, VkImageAspectFlags aspect) { @@ -831,6 +842,8 @@ void VulkanQueueRunner::PerformReadback(const VKRStep &step, VkCommandBuffer cmd vkCmdCopyImageToBuffer(cmd, srcImage->image, srcImage->layout, readbackBuffer_, 1, ®ion); // NOTE: Can't read the buffer using the CPU here - need to sync first. + + step.readback.src->Release(); } void VulkanQueueRunner::CopyReadbackBuffer(int width, int height, Draw::DataFormat destFormat, int pixelStride, uint8_t *pixels) { diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index ea38d21eaf..c2c89a2100 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -96,6 +96,16 @@ void CreateImage(VulkanContext *vulkan, VkCommandBuffer cmd, VKRImage &img, int img.format = format; } +bool VKRFramebuffer::Release() { + if (--refcount_ == 0) { + delete this; + return true; + } else if (refcount_ >= 10000 || refcount_ < 0) { + ELOG("Refcount (%d) invalid for object %p - corrupt?", refcount_.load(), this); + } + return false; +} + VulkanRenderManager::VulkanRenderManager(VulkanContext *vulkan) : vulkan_(vulkan), queueRunner_(vulkan) { VkSemaphoreCreateInfo semaphoreCreateInfo = { VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO }; semaphoreCreateInfo.flags = 0; @@ -351,6 +361,9 @@ void VulkanRenderManager::BindFramebufferAsRenderTarget(VKRFramebuffer *fb, VKRR step->render.numDraws = 0; step->render.finalColorLayout = !fb ? VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL : VK_IMAGE_LAYOUT_UNDEFINED; steps_.push_back(step); + if (fb) { + fb->AddRef(); + } curRenderStep_ = step; curWidth_ = fb ? fb->width : vulkan_->GetBackbufferWidth(); @@ -364,6 +377,7 @@ void VulkanRenderManager::CopyFramebufferToMemorySync(VKRFramebuffer *src, int a step->readback.srcRect.offset = { x, y }; step->readback.srcRect.extent = { (uint32_t)w, (uint32_t)h }; steps_.push_back(step); + src->AddRef(); curRenderStep_ = nullptr; @@ -505,6 +519,8 @@ void VulkanRenderManager::CopyFramebuffer(VKRFramebuffer *src, VkRect2D srcRect, step->copy.srcRect = srcRect; step->copy.dst = dst; step->copy.dstPos = dstPos; + src->AddRef(); + dst->AddRef(); std::unique_lock lock(mutex_); steps_.push_back(step); @@ -530,6 +546,8 @@ void VulkanRenderManager::BlitFramebuffer(VKRFramebuffer *src, VkRect2D srcRect, step->blit.dst = dst; step->blit.dstRect = dstRect; step->blit.filter = filter; + src->AddRef(); + dst->AddRef(); std::unique_lock lock(mutex_); steps_.push_back(step); @@ -553,6 +571,7 @@ VkImageView VulkanRenderManager::BindFramebufferAsTexture(VKRFramebuffer *fb, in } curRenderStep_->preTransitions.push_back({ fb, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL }); + fb->AddRef(); return fb->color.imageView; } diff --git a/ext/native/thin3d/VulkanRenderManager.h b/ext/native/thin3d/VulkanRenderManager.h index 50fb2f56d8..9b9ae5f679 100644 --- a/ext/native/thin3d/VulkanRenderManager.h +++ b/ext/native/thin3d/VulkanRenderManager.h @@ -4,10 +4,11 @@ // Only draws and binds are handled here, resource creation and allocations are handled as normal - // that's the nice thing with Vulkan. -#include -#include -#include +#include #include +#include +#include +#include #include "Common/Vulkan/VulkanContext.h" #include "math/dataconv.h" @@ -28,6 +29,7 @@ void CreateImage(VulkanContext *vulkan, VkCommandBuffer cmd, VKRImage &img, int class VKRFramebuffer { public: VKRFramebuffer(VulkanContext *vk, VkCommandBuffer initCmd, VkRenderPass renderPass, int _width, int _height) : vulkan_(vk) { + refcount_ = 1; width = _width; height = _height; @@ -58,6 +60,11 @@ public: vulkan_->Delete().QueueDeleteFramebuffer(framebuf); } + void AddRef() { + refcount_++; + } + bool Release(); + int numShadows = 1; // TODO: Support this. VkFramebuffer framebuf = VK_NULL_HANDLE; @@ -68,6 +75,7 @@ public: private: VulkanContext *vulkan_; + std::atomic refcount_; }; enum class VKRRunType { diff --git a/ext/native/thin3d/thin3d_vulkan.cpp b/ext/native/thin3d/thin3d_vulkan.cpp index 4e6eff5ddd..70a7cdd65b 100644 --- a/ext/native/thin3d/thin3d_vulkan.cpp +++ b/ext/native/thin3d/thin3d_vulkan.cpp @@ -1248,9 +1248,10 @@ uint32_t VKContext::GetDataFormatSupport(DataFormat fmt) const { // use this frame's init command buffer. class VKFramebuffer : public Framebuffer { public: + // Inherits ownership so no AddRef. VKFramebuffer(VKRFramebuffer *fb) : buf_(fb) {} ~VKFramebuffer() { - delete buf_; + buf_->Release(); } VKRFramebuffer *GetFB() const { return buf_; } private: From efbba31608fb41715e74b3d37f880ced77fb7d0b Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 08:13:43 -0800 Subject: [PATCH 08/11] GPU: Restore initial readback on output. Needed when the last rendered FB needs to be downloaded. --- GPU/Common/FramebufferCommon.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GPU/Common/FramebufferCommon.cpp b/GPU/Common/FramebufferCommon.cpp index 371a43e371..3173705c59 100644 --- a/GPU/Common/FramebufferCommon.cpp +++ b/GPU/Common/FramebufferCommon.cpp @@ -837,7 +837,7 @@ void FramebufferManagerCommon::SetViewport2D(int x, int y, int w, int h) { } void FramebufferManagerCommon::CopyDisplayToOutput() { - // DownloadFramebufferOnSwitch(currentRenderVfb_); + DownloadFramebufferOnSwitch(currentRenderVfb_); currentRenderVfb_ = 0; From ebe9dcafde21a81aea29ea6895bde623eb768880 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 08:39:02 -0800 Subject: [PATCH 09/11] Vulkan: Wait for queue idle in DestroyBackbuffers. Otherwise it's only done after destroying Draw, so no need to mutex. --- Windows/GPU/WindowsVulkanContext.cpp | 1 - android/jni/app-android.cpp | 1 - ext/native/thin3d/VulkanRenderManager.cpp | 2 ++ 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Windows/GPU/WindowsVulkanContext.cpp b/Windows/GPU/WindowsVulkanContext.cpp index f8263b565e..4d8e8f152f 100644 --- a/Windows/GPU/WindowsVulkanContext.cpp +++ b/Windows/GPU/WindowsVulkanContext.cpp @@ -228,7 +228,6 @@ void WindowsVulkanContext::SwapBuffers() { } void WindowsVulkanContext::Resize() { - g_Vulkan->WaitUntilQueueIdle(); draw_->HandleEvent(Draw::Event::LOST_BACKBUFFER, g_Vulkan->GetBackbufferWidth(), g_Vulkan->GetBackbufferHeight()); g_Vulkan->DestroyObjects(); diff --git a/android/jni/app-android.cpp b/android/jni/app-android.cpp index e0e772d1c1..19a133502f 100644 --- a/android/jni/app-android.cpp +++ b/android/jni/app-android.cpp @@ -341,7 +341,6 @@ void AndroidVulkanContext::SwapBuffers() { } void AndroidVulkanContext::Resize() { - g_Vulkan->WaitUntilQueueIdle(); draw_->HandleEvent(Draw::Event::LOST_BACKBUFFER, g_Vulkan->GetBackbufferWidth(), g_Vulkan->GetBackbufferHeight()); g_Vulkan->DestroyObjects(); diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index c2c89a2100..0b6c7e015b 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -218,6 +218,8 @@ void VulkanRenderManager::DestroyBackbuffers() { } thread_.join(); } + vulkan_->WaitUntilQueueIdle(); + VkDevice device = vulkan_->GetDevice(); for (uint32_t i = 0; i < swapchainImageCount_; i++) { vulkan_->Delete().QueueDeleteImageView(swapchainImages_[i].view); From 56d34402ffe3f00af6ceb5786dd5addd4bd8bba4 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 08:40:11 -0800 Subject: [PATCH 10/11] Vulkan: Resignal unexecuted fences on thread stop. When resizing or similar, we may end up with frames we never ran. This also happens on startup. We need them signaled at start so we can wait on them, or we may deadlock. --- ext/native/thin3d/VulkanRenderManager.cpp | 38 ++++++++++++++--------- ext/native/thin3d/VulkanRenderManager.h | 2 ++ 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index 0b6c7e015b..53db35ffc6 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -189,12 +189,6 @@ void VulkanRenderManager::CreateBackbuffers() { // Start the thread. if (useThread) { - for (int i = 0; i < vulkan_->GetInflightFrames(); i++) { - // Reset all the frameData. Might be dirty from previous thread stop. - frameData_[i].readyForRun = false; - frameData_[i].readyForFence = true; - } - run_ = true; // Won't necessarily be 0. threadInitFrame_ = vulkan_->GetCurFrame(); @@ -202,22 +196,37 @@ void VulkanRenderManager::CreateBackbuffers() { } } -void VulkanRenderManager::DestroyBackbuffers() { - if (useThread) { +void VulkanRenderManager::StopThread(bool shutdown) { + if (useThread && run_) { run_ = false; // Stop the thread. for (int i = 0; i < vulkan_->GetInflightFrames(); i++) { + auto &frameData = frameData_[i]; { - std::unique_lock lock(frameData_[i].push_mutex); - frameData_[i].push_condVar.notify_all(); + std::unique_lock lock(frameData.push_mutex); + frameData.push_condVar.notify_all(); } { - std::unique_lock lock(frameData_[i].pull_mutex); - frameData_[i].pull_condVar.notify_all(); + std::unique_lock lock(frameData.pull_mutex); + frameData.pull_condVar.notify_all(); } } thread_.join(); + + // Resignal fences for next time around - must be done after join. + for (int i = 0; i < vulkan_->GetInflightFrames(); i++) { + auto &frameData = frameData_[i]; + frameData.readyForRun = false; + if (!shutdown && !frameData.readyForFence) { + vkDestroyFence(vulkan_->GetDevice(), frameData.fence, nullptr); + frameData.fence = vulkan_->CreateFence(true); + } + } } +} + +void VulkanRenderManager::DestroyBackbuffers() { + StopThread(false); vulkan_->WaitUntilQueueIdle(); VkDevice device = vulkan_->GetDevice(); @@ -237,9 +246,10 @@ void VulkanRenderManager::DestroyBackbuffers() { } VulkanRenderManager::~VulkanRenderManager() { - run_ = false; - VkDevice device = vulkan_->GetDevice(); + StopThread(true); vulkan_->WaitUntilQueueIdle(); + + VkDevice device = vulkan_->GetDevice(); vkDestroySemaphore(device, acquireSemaphore_, nullptr); vkDestroySemaphore(device, renderingCompleteSemaphore_, nullptr); for (int i = 0; i < vulkan_->GetInflightFrames(); i++) { diff --git a/ext/native/thin3d/VulkanRenderManager.h b/ext/native/thin3d/VulkanRenderManager.h index 9b9ae5f679..f567163fa5 100644 --- a/ext/native/thin3d/VulkanRenderManager.h +++ b/ext/native/thin3d/VulkanRenderManager.h @@ -221,6 +221,8 @@ private: void FlushSync(); void EndSyncFrame(int frame); + void StopThread(bool shutdown); + // Permanent objects VkSemaphore acquireSemaphore_; VkSemaphore renderingCompleteSemaphore_; From ead4c5f9a8036939877f8897b95ab33a96c6b716 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 08:48:18 -0800 Subject: [PATCH 11/11] Vulkan: Make sure backbuffer is bound for UI. Fixes #10048. --- UI/EmuScreen.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/UI/EmuScreen.cpp b/UI/EmuScreen.cpp index 39a1c3fb87..8ce39c35a5 100644 --- a/UI/EmuScreen.cpp +++ b/UI/EmuScreen.cpp @@ -1038,6 +1038,9 @@ void EmuScreen::render() { if (!osm.IsEmpty() || g_Config.bShowDebugStats || g_Config.iShowFPSCounter || g_Config.bShowTouchControls || g_Config.bShowDeveloperMenu || g_Config.bShowAudioDebug || saveStatePreview_->GetVisibility() != UI::V_GONE || g_Config.bShowFrameProfiler) { DrawContext *thin3d = screenManager()->getDrawContext(); + // It's possible we never ended up outputted anything - make sure we have the backbuffer. + thin3d->BindFramebufferAsRenderTarget(nullptr, { RPAction::KEEP, RPAction::KEEP }); + // This sets up some important states but not the viewport. screenManager()->getUIContext()->Begin();