Merge pull request #12941 from hrydgard/vulkan-more-framebuffer-stuff

Vulkan: Deal with the reformat clear better

GPU/Common/FramebufferCommon.cpp

@@ -894,13 +894,6 @@ void FramebufferManagerCommon::CopyDisplayToOutput(bool reallyDirty) {
 		if (Memory::IsValidAddress(fbaddr)) {
 			// The game is displaying something directly from RAM. In GTA, it's decoded video.
 			if (!vfb) {
-				if (useBufferedRendering_) {
-					// Bind and clear the backbuffer. This should be the first time during the frame that it's bound.
-					draw_->BindFramebufferAsRenderTarget(nullptr, { Draw::RPAction::CLEAR, Draw::RPAction::CLEAR, Draw::RPAction::CLEAR }, "CopyDisplayToOutput_Backbuffer");
-				}
-				// Just a pointer to plain memory to draw. We should create a framebuffer, then draw to it.
-				SetViewport2D(0, 0, pixelWidth_, pixelHeight_);
-				draw_->SetScissorRect(0, 0, pixelWidth_, pixelHeight_);
 				DrawFramebufferToOutput(Memory::GetPointer(fbaddr), displayFormat_, displayStride_);
 				gstate_c.Dirty(DIRTY_BLEND_STATE);
 				return;

GPU/Vulkan/FramebufferVulkan.cpp

@@ -265,8 +265,14 @@ void FramebufferManagerVulkan::ReformatFramebufferFrom(VirtualFramebuffer *vfb,
 	// to exactly reproduce in 4444 and 8888 formats.
 
 	if (old == GE_FORMAT_565) {
-		// Previously started a render pass here to clear, but better to just, well, explicitly clear.
-		draw_->Clear(Draw::FBChannel::FB_COLOR_BIT | Draw::FBChannel::FB_STENCIL_BIT, 0, 0.0f, 0);
+		// We have to bind here instead of clear, since it can be that no framebuffer is bound.
+		// The backend can sometimes directly optimize it to a clear.
+		draw_->BindFramebufferAsRenderTarget(vfb->fbo, { Draw::RPAction::CLEAR, Draw::RPAction::KEEP, Draw::RPAction::CLEAR }, "ReformatFramebuffer"); 
+		// draw_->Clear(Draw::FBChannel::FB_COLOR_BIT | Draw::FBChannel::FB_STENCIL_BIT, 0, 0.0f, 0);
+
+		// Need to dirty anything that has command buffer dynamic state, in case we started a new pass above.
+		// Should find a way to feed that information back, maybe... Or simply correct the issue in the rendermanager.
+		gstate_c.Dirty(DIRTY_DEPTHSTENCIL_STATE | DIRTY_VIEWPORTSCISSOR_STATE | DIRTY_BLEND_STATE);
 	}
 }
 

ext/native/thin3d/VulkanRenderManager.cpp

@@ -456,11 +456,25 @@ VkCommandBuffer VulkanRenderManager::GetInitCmd() {
 
 void VulkanRenderManager::BindFramebufferAsRenderTarget(VKRFramebuffer *fb, VKRRenderPassAction color, VKRRenderPassAction depth, VKRRenderPassAction stencil, uint32_t clearColor, float clearDepth, uint8_t clearStencil, const char *tag) {
 	assert(insideFrame_);
-	// Eliminate dupes.
+	// Eliminate dupes, instantly convert to a clear if possible.
 	if (!steps_.empty() && steps_.back()->stepType == VKRStepType::RENDER && steps_.back()->render.framebuffer == fb) {
-		if (color != VKRRenderPassAction::CLEAR && depth != VKRRenderPassAction::CLEAR && stencil != VKRRenderPassAction::CLEAR) {
-			// We don't move to a new step, this bind was unnecessary and we can safely skip it.
-			// Not sure how much this is still happening but probably worth checking for nevertheless.
+		u32 clearMask = 0;
+		if (color == VKRRenderPassAction::CLEAR) {
+			clearMask |= VK_IMAGE_ASPECT_COLOR_BIT;
+		}
+		if (depth == VKRRenderPassAction::CLEAR) {
+			clearMask |= VK_IMAGE_ASPECT_DEPTH_BIT;
+		}
+		if (stencil == VKRRenderPassAction::CLEAR) {
+			clearMask |= VK_IMAGE_ASPECT_STENCIL_BIT;
+		}
+
+		// If we need a clear and the previous step has commands already, it's best to just add a clear and keep going.
+		// If there's no clear needed, let's also do that.
+		//
+		// However, if we do need a clear and there are no commands in the previous pass,
+		// we want the queuerunner to have the opportunity to merge, so we'll go ahead and make a new renderpass.
+		if (clearMask == 0 || !steps_.back()->commands.empty()) {
 			curRenderStep_ = steps_.back();
 			curStepHasViewport_ = false;
 			curStepHasScissor_ = false;
@@ -473,6 +487,14 @@ void VulkanRenderManager::BindFramebufferAsRenderTarget(VKRFramebuffer *fb, VKRR
 					break;
 				}
 			}
+			if (clearMask != 0) {
+				VkRenderData data{ VKRRenderCommand::CLEAR };
+				data.clear.clearColor = clearColor;
+				data.clear.clearZ = clearDepth;
+				data.clear.clearStencil = clearStencil;
+				data.clear.clearMask = clearMask;
+				curRenderStep_->commands.push_back(data);
+			}
 			return;
 		}
 	}