Vulkan: Noticed a framebuffer refcount leak, and changed my mind rega…

…rding those :) Let's do it this way instead.

ext/native/thin3d/VulkanQueueRunner.cpp

@@ -348,7 +348,6 @@ 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();
 	}
 
 	// Don't execute empty renderpasses.
@@ -601,10 +600,6 @@ 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) {
@@ -678,9 +673,6 @@ void VulkanQueueRunner::PerformCopy(const VKRStep &step, VkCommandBuffer cmd) {
 		}
 		vkCmdCopyImage(cmd, src->depth.image, src->depth.layout, dst->depth.image, dst->depth.layout, 1, &copy);
 	}
-
-	src->Release();
-	dst->Release();
 }
 
 void VulkanQueueRunner::PerformBlit(const VKRStep &step, VkCommandBuffer cmd) {
@@ -764,9 +756,6 @@ 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) {
@@ -867,8 +856,6 @@ void VulkanQueueRunner::PerformReadback(const VKRStep &step, VkCommandBuffer cmd
 	vkCmdCopyImageToBuffer(cmd, srcImage->image, srcImage->layout, readbackBuffer_, 1, &region);
 
 	// NOTE: Can't read the buffer using the CPU here - need to sync first.
-
-	step.readback.src->Release();
 }
 
 void VulkanQueueRunner::PerformReadbackImage(const VKRStep &step, VkCommandBuffer cmd) {

ext/native/thin3d/VulkanRenderManager.cpp

@@ -96,16 +96,6 @@ 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;
@@ -406,9 +396,6 @@ 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();
@@ -422,7 +409,6 @@ 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;
 
@@ -599,8 +585,6 @@ 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<std::mutex> lock(mutex_);
 	steps_.push_back(step);
@@ -632,8 +616,6 @@ 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<std::mutex> lock(mutex_);
 	steps_.push_back(step);
@@ -657,7 +639,6 @@ VkImageView VulkanRenderManager::BindFramebufferAsTexture(VKRFramebuffer *fb, in
 	}
 
 	curRenderStep_->preTransitions.push_back({ fb, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL });
-	fb->AddRef();
 	return fb->color.imageView;
 }
 
@@ -684,29 +665,6 @@ void VulkanRenderManager::Finish() {
 
 void VulkanRenderManager::Wipe() {
 	for (auto step : steps_) {
-		// Need to release held framebuffers.
-		switch (step->stepType) {
-		case VKRStepType::RENDER:
-			for (const auto &iter : step->preTransitions) {
-				iter.fb->Release();
-			}
-			break;
-		case VKRStepType::COPY:
-			step->copy.src->Release();
-			step->copy.dst->Release();
-			break;
-		case VKRStepType::BLIT:
-			step->blit.src->Release();
-			step->blit.dst->Release();
-			break;
-		case VKRStepType::READBACK:
-			step->readback.src->Release();
-			break;
-		case VKRStepType::READBACK_IMAGE:
-			break;
-		default:
-			assert(false);
-		}
 		delete step;
 	}
 	steps_.clear();

ext/native/thin3d/VulkanRenderManager.h

@@ -29,7 +29,6 @@ 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;
 
@@ -50,6 +49,7 @@ class VKRFramebuffer {
 
 		vkCreateFramebuffer(vulkan_->GetDevice(), &fbci, nullptr, &framebuf);
 	}
+
 	~VKRFramebuffer() {
 		vulkan_->Delete().QueueDeleteImage(color.image);
 		vulkan_->Delete().QueueDeleteImage(depth.image);
@@ -60,11 +60,6 @@ class VKRFramebuffer {
 		vulkan_->Delete().QueueDeleteFramebuffer(framebuf);
 	}
 
-	void AddRef() {
-		refcount_++;
-	}
-	bool Release();
-
 	int numShadows = 1;  // TODO: Support this.
 
 	VkFramebuffer framebuf = VK_NULL_HANDLE;
@@ -73,9 +68,7 @@ class VKRFramebuffer {
 	int width = 0;
 	int height = 0;
 
-private:
 	VulkanContext *vulkan_;
-	std::atomic<int> refcount_;
 };
 
 enum class VKRRunType {

ext/native/thin3d/thin3d_vulkan.cpp

@@ -1251,10 +1251,14 @@ class VKFramebuffer : public Framebuffer {
 	// Inherits ownership so no AddRef.
 	VKFramebuffer(VKRFramebuffer *fb) : buf_(fb) {}
 	~VKFramebuffer() {
-		buf_->Release();
+		buf_->vulkan_->Delete().QueueCallback([](void *fb) {
+			VKRFramebuffer *vfb = static_cast<VKRFramebuffer *>(fb);
+			delete vfb;
+		}, buf_);
 	}
 	VKRFramebuffer *GetFB() const { return buf_; }
 private:
+	VulkanContext *vulkan_;  // Unfortunate to have to keep this in each VKFramebuffer.
 	VKRFramebuffer *buf_;
 };