Merge pull request #17536 from hrydgard/vulkan-image-barrier-fixes

Vulkan image barrier fix, image leak fix from #17534

Common/GPU/D3D11/thin3d_d3d11.cpp

@@ -131,14 +131,16 @@ class D3D11DrawContext : public DrawContext {
 		stencilDirty_ = true;
 	}
 
-	void EndFrame() override;
 
 	void Draw(int vertexCount, int offset) override;
 	void DrawIndexed(int vertexCount, int offset) override;
 	void DrawUP(const void *vdata, int vertexCount) override;
 	void Clear(int mask, uint32_t colorval, float depthVal, int stencilVal) override;
 
 	void BeginFrame() override;
+	void EndFrame() override;
+
+	int GetFrameCount() override { return frameCount_; }
 
 	std::string GetInfoString(InfoField info) const override {
 		switch (info) {
@@ -221,6 +223,7 @@ class D3D11DrawContext : public DrawContext {
 	int nextIndexBufferOffset_ = 0;
 
 	InvalidationCallback invalidationCallback_;
+	int frameCount_ = 0;
 
 	// Dynamic state
 	float blendFactor_[4]{};
@@ -423,6 +426,7 @@ void D3D11DrawContext::HandleEvent(Event ev, int width, int height, void *param1
 
 void D3D11DrawContext::EndFrame() {
 	curPipeline_ = nullptr;
+	frameCount_++;
 }
 
 void D3D11DrawContext::SetViewport(const Viewport &viewport) {

Common/GPU/D3D9/thin3d_d3d9.cpp

@@ -580,6 +580,7 @@ class D3D9Context : public DrawContext {
 	}
 
 	void EndFrame() override;
+	int GetFrameCount() override { return frameCount_; }
 
 	void UpdateDynamicUniformBuffer(const void *ub, size_t size) override;
 
@@ -640,6 +641,7 @@ class D3D9Context : public DrawContext {
 	D3DCAPS9 d3dCaps_;
 	char shadeLangVersion_[64]{};
 	DeviceCaps caps_{};
+	int frameCount_ = 0;
 
 	// Bound state
 	AutoRef<D3D9Pipeline> curPipeline_;
@@ -964,6 +966,7 @@ void D3D9Context::BindNativeTexture(int index, void *nativeTexture) {
 
 void D3D9Context::EndFrame() {
 	curPipeline_ = nullptr;
+	frameCount_++;
 }
 
 static void SemanticToD3D9UsageAndIndex(int semantic, BYTE *usage, BYTE *index) {

Common/GPU/OpenGL/thin3d_gl.cpp

@@ -370,6 +370,10 @@ class OpenGLContext : public DrawContext {
 	void BeginFrame() override;
 	void EndFrame() override;
 
+	int GetFrameCount() override {
+		return frameCount_;
+	}
+
 	void UpdateBuffer(Buffer *buffer, const uint8_t *data, size_t offset, size_t size, UpdateBufferFlags flags) override;
 	void UpdateTextureLevels(Texture *texture, const uint8_t **data, TextureCallback initDataCallback, int numLevels) override;
 
@@ -492,6 +496,7 @@ class OpenGLContext : public DrawContext {
 	void ApplySamplers();
 
 	GLRenderManager renderManager_;
+	int frameCount_ = 0;
 
 	DeviceCaps caps_{};
 
@@ -795,6 +800,7 @@ void OpenGLContext::EndFrame() {
 	renderManager_.Finish();
 
 	Invalidate(InvalidationFlags::CACHED_RENDER_STATE);
+	frameCount_++;
 }
 
 void OpenGLContext::Invalidate(InvalidationFlags flags) {

Common/GPU/Vulkan/VulkanDebug.cpp

@@ -53,8 +53,14 @@ VKAPI_ATTR VkBool32 VKAPI_CALL VulkanDebugUtilsCallback(
 		return false;
 	case 1303270965:
 		// Benign perf warning, image blit using GENERAL layout.
-		// UNASSIGNED
-		return false;
+		// TODO: Oops, turns out we filtered out a bit too much here!
+		// We really need that performance flag check to sort out the stuff that matters.
+		// Will enable it soon, but it'll take some fixing.
+		//
+		if (messageType & VK_DEBUG_UTILS_MESSAGE_TYPE_PERFORMANCE_BIT_EXT)
+			return false;
+		break;
+
 	case 606910136:
 	case -392708513:
 	case -384083808:

Common/GPU/Vulkan/VulkanImage.cpp

@@ -258,6 +258,22 @@ void VulkanTexture::EndCreate(VkCommandBuffer cmd, bool vertexTexture, VkPipelin
 		prevStage == VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT ? VK_ACCESS_SHADER_WRITE_BIT : VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_SHADER_READ_BIT);
 }
 
+void VulkanTexture::PrepareForTransferDst(VkCommandBuffer cmd, int levels) {
+	TransitionImageLayout2(cmd, image_, 0, levels, 1,
+		VK_IMAGE_ASPECT_COLOR_BIT,
+		VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
+		VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT,
+		VK_ACCESS_SHADER_READ_BIT, VK_ACCESS_TRANSFER_WRITE_BIT);
+}
+
+void VulkanTexture::RestoreAfterTransferDst(VkCommandBuffer cmd, int levels) {
+	TransitionImageLayout2(cmd, image_, 0, levels, 1,
+		VK_IMAGE_ASPECT_COLOR_BIT,
+		VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL,
+		VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT,
+		VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_SHADER_READ_BIT);
+}
+
 VkImageView VulkanTexture::CreateViewForMip(int mip) {
 	VkImageViewCreateInfo view_info = { VK_STRUCTURE_TYPE_IMAGE_VIEW_CREATE_INFO };
 	view_info.image = image_;

Common/GPU/Vulkan/VulkanImage.h

@@ -37,6 +37,10 @@ class VulkanTexture {
 	void GenerateMips(VkCommandBuffer cmd, int firstMipToGenerate, bool fromCompute);
 	void EndCreate(VkCommandBuffer cmd, bool vertexTexture, VkPipelineStageFlags prevStage, VkImageLayout layout = VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL);
 
+	// For updating levels after creation. Careful with the timelines!
+	void PrepareForTransferDst(VkCommandBuffer cmd, int levels);
+	void RestoreAfterTransferDst(VkCommandBuffer cmd, int levels);
+
 	// When loading mips from compute shaders, you need to pass VK_IMAGE_LAYOUT_GENERAL to the above function.
 	// In addition, ignore UploadMip and GenerateMip, and instead use GetViewForMip. Make sure to delete the returned views when used.
 	VkImageView CreateViewForMip(int mip);

Common/GPU/Vulkan/thin3d_vulkan.cpp

@@ -484,6 +484,10 @@ class VKContext : public DrawContext {
 	void EndFrame() override;
 	void WipeQueue() override;
 
+	int GetFrameCount() override {
+		return frameCount_;
+	}
+
 	void FlushState() override {}
 
 	void ResetStats() override {
@@ -528,6 +532,7 @@ class VKContext : public DrawContext {
 	VulkanTexture *GetNullTexture();
 	VulkanContext *vulkan_ = nullptr;
 
+	int frameCount_ = 0;
 	VulkanRenderManager renderManager_;
 
 	VulkanTexture *nullTexture_ = nullptr;
@@ -799,8 +804,9 @@ bool VKTexture::Create(VkCommandBuffer cmd, VulkanPushPool *pushBuffer, const Te
 void VKTexture::Update(VkCommandBuffer cmd, VulkanPushPool *pushBuffer, const uint8_t * const *data, TextureCallback initDataCallback, int numLevels) {
 	// Before we can use UpdateInternal, we need to transition the image to the same state as after CreateDirect,
 	// making it ready for writing.
-
+	vkTex_->PrepareForTransferDst(cmd, numLevels);
 	UpdateInternal(cmd, pushBuffer, data, initDataCallback, numLevels);
+	vkTex_->RestoreAfterTransferDst(cmd, numLevels);
 }
 
 void VKTexture::UpdateInternal(VkCommandBuffer cmd, VulkanPushPool *pushBuffer, const uint8_t * const *data, TextureCallback initDataCallback, int numLevels) {
@@ -1104,6 +1110,8 @@ void VKContext::EndFrame() {
 
 	// Unbind stuff, to avoid accidentally relying on it across frames (and provide some protection against forgotten unbinds of deleted things).
 	Invalidate(InvalidationFlags::CACHED_RENDER_STATE);
+
+	frameCount_++;
 }
 
 void VKContext::Invalidate(InvalidationFlags flags) {

Common/GPU/thin3d.h

@@ -841,6 +841,10 @@ class DrawContext {
 	// Not very elegant, but more elegant than the old passId hack.
 	virtual void SetInvalidationCallback(InvalidationCallback callback) = 0;
 
+	// Total amount of frames rendered. Unaffected by game pause, so more robust than gpuStats.numFlips
+	virtual int GetFrameCount() = 0;
+	virtual int GetFramesInFlight() { return 3; }
+
 protected:
 	ShaderModule *vsPresets_[VS_MAX_PRESET];
 	ShaderModule *fsPresets_[FS_MAX_PRESET];

GPU/Common/FramebufferManagerCommon.cpp

@@ -1402,16 +1402,19 @@ Draw::Texture *FramebufferManagerCommon::MakePixelTexture(const u8 *srcPixels, G
 
 	Draw::DataFormat texFormat = srcPixelFormat == GE_FORMAT_DEPTH16 ? depthFormat : preferredPixelsFormat_;
 
+	int frameNumber = draw_->GetFrameCount();
+
 	// Look for a matching texture we can re-use.
 	for (auto &iter : drawPixelsCache_) {
-		if (iter.frameNumber > gpuStats.numFlips - 3 || iter.tex->Width() != width || iter.tex->Height() != height || iter.tex->Format() != texFormat) {
+		if (iter.frameNumber >= frameNumber - 3 || iter.tex->Width() != width || iter.tex->Height() != height || iter.tex->Format() != texFormat) {
 			continue;
 		}
 
 		// OK, current one seems good, let's use it (and mark it used).
 		gpuStats.numDrawPixels++;
 		draw_->UpdateTextureLevels(iter.tex, &srcPixels, generateTexture, 1);
-		iter.frameNumber = gpuStats.numFlips;
+		// NOTE: numFlips is no good - this is called every frame when paused sometimes!
+		iter.frameNumber = frameNumber;
 		return iter.tex;
 	}
 
@@ -1440,9 +1443,9 @@ Draw::Texture *FramebufferManagerCommon::MakePixelTexture(const u8 *srcPixels, G
 	gpuStats.numDrawPixels++;
 	gpuStats.numTexturesDecoded++;  // Separate stat for this later?
 
-	INFO_LOG(G3D, "Creating drawPixelsCache texture: %dx%d", tex->Width(), tex->Height());
+	// INFO_LOG(G3D, "Creating drawPixelsCache texture: %dx%d", tex->Width(), tex->Height());
 
-	DrawPixelsEntry entry{ tex, gpuStats.numFlips };
+	DrawPixelsEntry entry{ tex, frameNumber };
 	drawPixelsCache_.push_back(entry);
 	return tex;
 }
@@ -1695,9 +1698,9 @@ void FramebufferManagerCommon::DecimateFBOs() {
 	// And DrawPixels cached textures.
 
 	for (auto it = drawPixelsCache_.begin(); it != drawPixelsCache_.end(); ) {
-		int age = gpuStats.numFlips - it->frameNumber;
+		int age = draw_->GetFrameCount() - it->frameNumber;
 		if (age > 10) {
-			INFO_LOG(G3D, "Releasing drawPixelsCache texture: %dx%d", it->tex->Width(), it->tex->Height());
+			// INFO_LOG(G3D, "Releasing drawPixelsCache texture: %dx%d", it->tex->Width(), it->tex->Height());
 			it->tex->Release();
 			it->tex = nullptr;
 			it = drawPixelsCache_.erase(it);