From 0d92c4210137b5c1a1d7548dae68f25f9f3fc1f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 1 Aug 2022 23:33:23 +0200 Subject: [PATCH 1/3] Vulkan: Add the ability to inject depth copies/blits before the current render pass, if any. --- Common/GPU/Vulkan/VulkanRenderManager.cpp | 45 +++++++++++++++++++---- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanRenderManager.cpp b/Common/GPU/Vulkan/VulkanRenderManager.cpp index b2df509a81fc..17ce28fb08c7 100644 --- a/Common/GPU/Vulkan/VulkanRenderManager.cpp +++ b/Common/GPU/Vulkan/VulkanRenderManager.cpp @@ -1054,7 +1054,17 @@ void VulkanRenderManager::CopyFramebuffer(VKRFramebuffer *src, VkRect2D srcRect, _dbg_assert_msg_(dstPos.x + srcRect.extent.width <= (uint32_t)dst->width, "dstPos + extent x > width"); _dbg_assert_msg_(dstPos.y + srcRect.extent.height <= (uint32_t)dst->height, "dstPos + extent y > height"); - for (int i = (int)steps_.size() - 1; i >= 0; i--) { + bool injectBeforeCurrent = false; + int lastStepToCheck = (int)steps_.size() - 1; + if (aspectMask == VK_IMAGE_ASPECT_DEPTH_BIT && curRenderStep_) { + // We allow injection of these while in the render pass, so we don't have to split the render pass. + injectBeforeCurrent = true; + lastStepToCheck--; + } else { + EndCurRenderStep(); + } + + for (int i = lastStepToCheck; i >= 0; i--) { if (steps_[i]->stepType == VKRStepType::RENDER && steps_[i]->render.framebuffer == src) { if (aspectMask & VK_IMAGE_ASPECT_COLOR_BIT) { if (steps_[i]->render.finalColorLayout == VK_IMAGE_LAYOUT_UNDEFINED) { @@ -1070,7 +1080,8 @@ void VulkanRenderManager::CopyFramebuffer(VKRFramebuffer *src, VkRect2D srcRect, break; } } - for (int i = (int)steps_.size() - 1; i >= 0; i--) { + + for (int i = lastStepToCheck; i >= 0; i--) { if (steps_[i]->stepType == VKRStepType::RENDER && steps_[i]->render.framebuffer == dst) { if (aspectMask & VK_IMAGE_ASPECT_COLOR_BIT) { if (steps_[i]->render.finalColorLayout == VK_IMAGE_LAYOUT_UNDEFINED) { @@ -1102,7 +1113,12 @@ void VulkanRenderManager::CopyFramebuffer(VKRFramebuffer *src, VkRect2D srcRect, step->dependencies.insert(dst); std::unique_lock lock(mutex_); - steps_.push_back(step); + + if (injectBeforeCurrent) { + steps_.insert(steps_.begin() + steps_.size() - 1, step); + } else { + steps_.push_back(step); + } } void VulkanRenderManager::BlitFramebuffer(VKRFramebuffer *src, VkRect2D srcRect, VKRFramebuffer *dst, VkRect2D dstRect, VkImageAspectFlags aspectMask, VkFilter filter, const char *tag) { @@ -1122,17 +1138,25 @@ void VulkanRenderManager::BlitFramebuffer(VKRFramebuffer *src, VkRect2D srcRect, _dbg_assert_msg_(dstRect.extent.width > 0, "blit dstwidth == 0"); _dbg_assert_msg_(dstRect.extent.height > 0, "blit dstheight == 0"); - // TODO: Seem to be missing final layouts here like in Copy... + bool injectBeforeCurrent = false; - for (int i = (int)steps_.size() - 1; i >= 0; i--) { + int lastStepToCheck = (int)steps_.size() - 1; + + if (aspectMask == VK_IMAGE_ASPECT_DEPTH_BIT && curRenderStep_) { + // We allow injection of these while in the render pass, so we don't have to split the render pass. + injectBeforeCurrent = true; + lastStepToCheck--; + } else { + EndCurRenderStep(); + } + + for (int i = lastStepToCheck; i >= 0; i--) { if (steps_[i]->stepType == VKRStepType::RENDER && steps_[i]->render.framebuffer == src) { steps_[i]->render.numReads++; break; } } - EndCurRenderStep(); - VKRStep *step = new VKRStep{ VKRStepType::BLIT }; step->blit.aspectMask = aspectMask; @@ -1148,7 +1172,12 @@ void VulkanRenderManager::BlitFramebuffer(VKRFramebuffer *src, VkRect2D srcRect, step->dependencies.insert(dst); std::unique_lock lock(mutex_); - steps_.push_back(step); + + if (injectBeforeCurrent) { + steps_.insert(steps_.begin() + steps_.size() - 1, step); + } else { + steps_.push_back(step); + } } VkImageView VulkanRenderManager::BindFramebufferAsTexture(VKRFramebuffer *fb, int binding, VkImageAspectFlags aspectBit, int attachment) { From ec84046a926fedbf06d8fd26c019839f88a94b2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 1 Aug 2022 23:55:58 +0200 Subject: [PATCH 2/3] Defer depth copies until depth buffer is actually used. --- GPU/Common/FramebufferManagerCommon.cpp | 45 ++++++++++++------------- GPU/Common/FramebufferManagerCommon.h | 2 ++ GPU/GPUCommon.cpp | 8 +++++ GPU/GPUState.h | 3 ++ 4 files changed, 35 insertions(+), 23 deletions(-) diff --git a/GPU/Common/FramebufferManagerCommon.cpp b/GPU/Common/FramebufferManagerCommon.cpp index 0da6dc00cadc..8398463ba02f 100644 --- a/GPU/Common/FramebufferManagerCommon.cpp +++ b/GPU/Common/FramebufferManagerCommon.cpp @@ -425,6 +425,8 @@ VirtualFramebuffer *FramebufferManagerCommon::DoSetRenderFrameBuffer(const Frame // None found? Create one. if (!vfb) { + gstate_c.usingDepth = false; // reset depth buffer tracking + vfb = new VirtualFramebuffer{}; vfb->fbo = nullptr; vfb->fb_address = params.fb_address; @@ -451,18 +453,6 @@ VirtualFramebuffer *FramebufferManagerCommon::DoSetRenderFrameBuffer(const Frame ResizeFramebufFBO(vfb, drawing_width, drawing_height, true); NotifyRenderFramebufferCreated(vfb); - // Looks up by z_address, so if one is found here and not have last pointers equal to this one, - // there is another one. - TrackedDepthBuffer *prevDepth = GetOrCreateTrackedDepthBuffer(vfb); - - // We might already want to copy depth, in case this is a temp buffer. See #7810. - if (prevDepth->vfb != vfb) { - if (!params.isClearingDepth && prevDepth->vfb) { - BlitFramebufferDepth(prevDepth->vfb, vfb); - } - prevDepth->vfb = vfb; - } - SetColorUpdated(vfb, skipDrawReason); INFO_LOG(FRAMEBUF, "Creating FBO for %08x (z: %08x) : %d x %d x %s", vfb->fb_address, vfb->z_address, vfb->width, vfb->height, GeBufferFormatToString(vfb->format)); @@ -520,6 +510,7 @@ VirtualFramebuffer *FramebufferManagerCommon::DoSetRenderFrameBuffer(const Frame VirtualFramebuffer *prev = currentRenderVfb_; currentRenderVfb_ = vfb; NotifyRenderFramebufferSwitched(prev, vfb, params.isClearingDepth); + gstate_c.usingDepth = false; // reset depth buffer tracking } else { vfb->last_frame_render = gpuStats.numFlips; frameLastFramebufUsed_ = gpuStats.numFlips; @@ -537,6 +528,25 @@ VirtualFramebuffer *FramebufferManagerCommon::DoSetRenderFrameBuffer(const Frame return vfb; } +// Called on the first use of depth in a render pass. +void FramebufferManagerCommon::SetDepthFrameBuffer() { + if (!currentRenderVfb_) { + return; + } + + // Looks up by z_address, so if one is found here and not have last pointers equal to this one, + // there is another one. + TrackedDepthBuffer *prevDepth = GetOrCreateTrackedDepthBuffer(currentRenderVfb_); + + // We might already want to copy depth, in case this is a temp buffer. See #7810. + if (prevDepth->vfb != currentRenderVfb_) { + if (!gstate_c.clearingDepth && prevDepth->vfb) { + BlitFramebufferDepth(prevDepth->vfb, currentRenderVfb_); + } + prevDepth->vfb = currentRenderVfb_; + } +} + void FramebufferManagerCommon::DestroyFramebuf(VirtualFramebuffer *v) { // Notify the texture cache of both the color and depth buffers. textureCache_->NotifyFramebuffer(v, NOTIFY_FB_DESTROYED); @@ -857,17 +867,6 @@ void FramebufferManagerCommon::NotifyRenderFramebufferSwitched(VirtualFramebuffe textureCache_->ForgetLastTexture(); shaderManager_->DirtyLastShader(); - // Copy depth between the framebuffers, if the z_address is the same (checked inside.) - TrackedDepthBuffer *prevDepth = GetOrCreateTrackedDepthBuffer(vfb); - - // We might already want to copy depth, in case this is a temp buffer. See #7810. - if (prevDepth->vfb != vfb) { - if (!isClearingDepth && prevDepth->vfb) { - BlitFramebufferDepth(prevDepth->vfb, vfb); - } - prevDepth->vfb = vfb; - } - if (vfb->drawnFormat != vfb->format) { ReinterpretFramebuffer(vfb, vfb->drawnFormat, vfb->format); } diff --git a/GPU/Common/FramebufferManagerCommon.h b/GPU/Common/FramebufferManagerCommon.h index ca995fda06ec..35886c1b36b1 100644 --- a/GPU/Common/FramebufferManagerCommon.h +++ b/GPU/Common/FramebufferManagerCommon.h @@ -248,6 +248,8 @@ class FramebufferManagerCommon { return vfb; } } + void SetDepthFrameBuffer(); + void RebindFramebuffer(const char *tag); std::vector GetFramebufferList(); diff --git a/GPU/GPUCommon.cpp b/GPU/GPUCommon.cpp index 2996348b54a7..f9f2637a1ecc 100644 --- a/GPU/GPUCommon.cpp +++ b/GPU/GPUCommon.cpp @@ -1685,6 +1685,14 @@ void GPUCommon::Execute_Prim(u32 op, u32 diff) { return; } + bool isClearingDepth = gstate.isModeClear() && gstate.isClearModeDepthMask();; + + if (!gstate_c.usingDepth && (gstate.isDepthTestEnabled() || isClearingDepth)) { + gstate_c.usingDepth = true; + gstate_c.clearingDepth = isClearingDepth; + framebufferManager_->SetDepthFrameBuffer(); + } + const void *verts = Memory::GetPointerUnchecked(gstate_c.vertexAddr); const void *inds = nullptr; u32 vertexType = gstate.vertType; diff --git a/GPU/GPUState.h b/GPU/GPUState.h index 8d6d04039bd9..3f30be89f748 100644 --- a/GPU/GPUState.h +++ b/GPU/GPUState.h @@ -572,6 +572,9 @@ struct GPUStateCache { uint64_t dirty; + bool usingDepth; // For deferred depth copies. + bool clearingDepth; + bool textureFullAlpha; bool vertexFullAlpha; From 46979cd689d2127a0e8031913400127070cf7be3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Tue, 2 Aug 2022 00:31:20 +0200 Subject: [PATCH 3/3] Unsuccessful attempt at fixing Colin McRae --- GPU/Common/FramebufferManagerCommon.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/GPU/Common/FramebufferManagerCommon.cpp b/GPU/Common/FramebufferManagerCommon.cpp index 8398463ba02f..b7a3682a957e 100644 --- a/GPU/Common/FramebufferManagerCommon.cpp +++ b/GPU/Common/FramebufferManagerCommon.cpp @@ -425,8 +425,6 @@ VirtualFramebuffer *FramebufferManagerCommon::DoSetRenderFrameBuffer(const Frame // None found? Create one. if (!vfb) { - gstate_c.usingDepth = false; // reset depth buffer tracking - vfb = new VirtualFramebuffer{}; vfb->fbo = nullptr; vfb->fb_address = params.fb_address; @@ -470,6 +468,12 @@ VirtualFramebuffer *FramebufferManagerCommon::DoSetRenderFrameBuffer(const Frame // TODO: Is it worth trying to upload the depth buffer (only if it wasn't copied above..?) } + // We don't try to optimize depth buffer tracking on the frame a framebuffer is created. + // See #7810 + gstate_c.usingDepth = true; + gstate_c.clearingDepth = false; + SetDepthFrameBuffer(); + // Let's check for depth buffer overlap. Might be interesting. bool sharingReported = false; for (size_t i = 0, end = vfbs_.size(); i < end; ++i) {