From 2ad9eb047edfe73ffc5e640373b47cc4b440c169 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 08:13:18 -0800 Subject: [PATCH] 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: