From 34d1661c48203fe369cd1de38b5167e4a16ba69a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 28 Sep 2022 13:40:57 +0200 Subject: [PATCH 1/5] Quiet the Vulkan miniprofiler (for texture uploads etc) a bit --- Common/GPU/Vulkan/VulkanProfiler.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanProfiler.cpp b/Common/GPU/Vulkan/VulkanProfiler.cpp index 36305484da97..fccea843b82d 100644 --- a/Common/GPU/Vulkan/VulkanProfiler.cpp +++ b/Common/GPU/Vulkan/VulkanProfiler.cpp @@ -34,13 +34,13 @@ void VulkanProfiler::BeginFrame(VulkanContext *vulkan, VkCommandBuffer firstComm static const char * const indent[4] = { "", " ", " ", " " }; if (!scopes_.empty()) { - NOTICE_LOG(G3D, "Profiling events this frame:"); + INFO_LOG(G3D, "Profiling events this frame:"); } // Log it all out. for (auto &scope : scopes_) { if (scope.endQueryId == -1) { - NOTICE_LOG(G3D, "Unclosed scope: %s", scope.name.c_str()); + WARN_LOG(G3D, "Unclosed scope: %s", scope.name.c_str()); continue; } uint64_t startTime = results[scope.startQueryId]; @@ -50,7 +50,7 @@ void VulkanProfiler::BeginFrame(VulkanContext *vulkan, VkCommandBuffer firstComm double milliseconds = (double)delta * timestampConversionFactor; - NOTICE_LOG(G3D, "%s%s (%0.3f ms)", indent[scope.level & 3], scope.name.c_str(), milliseconds); + INFO_LOG(G3D, "%s%s (%0.3f ms)", indent[scope.level & 3], scope.name.c_str(), milliseconds); } scopes_.clear(); From de51d067f210e2ce9a3d8dd222791762e622da60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 28 Sep 2022 13:41:41 +0200 Subject: [PATCH 2/5] If a framebuffer starts using a different depth buffer than before, re-point. Fixes depth artifacts in Silent Hill: Origins. See issue #16126 --- GPU/Common/FramebufferManagerCommon.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/GPU/Common/FramebufferManagerCommon.cpp b/GPU/Common/FramebufferManagerCommon.cpp index b4db88965a56..807472703dbe 100644 --- a/GPU/Common/FramebufferManagerCommon.cpp +++ b/GPU/Common/FramebufferManagerCommon.cpp @@ -551,6 +551,15 @@ void FramebufferManagerCommon::SetDepthFrameBuffer(bool isClearingDepth) { bool newlyUsingDepth = (currentRenderVfb_->usageFlags & FB_USAGE_RENDER_DEPTH) == 0; currentRenderVfb_->usageFlags |= FB_USAGE_RENDER_DEPTH; + uint32_t boundDepthBuffer = gstate.getDepthBufAddress() & 0x3FFFFFFF; + if (currentRenderVfb_->z_address != boundDepthBuffer) { + WARN_LOG(G3D, "Framebuffer at %08x/%d has switched associated depth buffer from %08x to %08x, updating.", + currentRenderVfb_->fb_address, currentRenderVfb_->fb_stride, currentRenderVfb_->z_address, boundDepthBuffer); + // Technically, here we should copy away the depth buffer to another framebuffer that uses that z_address, or maybe + // even write it back to RAM. However, this is rare. Silent Hill is one example, see #16126. + currentRenderVfb_->z_address = boundDepthBuffer; + } + // If this first draw call is anything other than a clear, "resolve" the depth buffer, // by copying from any overlapping buffers with fresher content. if (!isClearingDepth && useBufferedRendering_) { @@ -1541,9 +1550,9 @@ void FramebufferManagerCommon::ResizeFramebufFBO(VirtualFramebuffer *vfb, int w, bool creating = old.bufferWidth == 0; if (creating) { - WARN_LOG(FRAMEBUF, "Creating %s FBO at %08x/%d %dx%d (force=%d)", GeBufferFormatToString(vfb->fb_format), vfb->fb_address, vfb->fb_stride, vfb->bufferWidth, vfb->bufferHeight, (int)force); + WARN_LOG(FRAMEBUF, "Creating %s FBO at %08x/%08x stride=%d %dx%d (force=%d)", GeBufferFormatToString(vfb->fb_format), vfb->fb_address, vfb->z_address, vfb->fb_stride, vfb->bufferWidth, vfb->bufferHeight, (int)force); } else { - WARN_LOG(FRAMEBUF, "Resizing %s FBO at %08x/%d from %dx%d to %dx%d (force=%d, skipCopy=%d)", GeBufferFormatToString(vfb->fb_format), vfb->fb_address, vfb->fb_stride, old.bufferWidth, old.bufferHeight, vfb->bufferWidth, vfb->bufferHeight, (int)force, (int)skipCopy); + WARN_LOG(FRAMEBUF, "Resizing %s FBO at %08x/%08x stride=%d from %dx%d to %dx%d (force=%d, skipCopy=%d)", GeBufferFormatToString(vfb->fb_format), vfb->fb_address, vfb->z_address, vfb->fb_stride, old.bufferWidth, old.bufferHeight, vfb->bufferWidth, vfb->bufferHeight, (int)force, (int)skipCopy); } // During hardware rendering, we always render at full color depth even if the game wouldn't on real hardware. From 3adad176cc8b3be05a14d3f6347eb104031b953f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 28 Sep 2022 13:56:03 +0200 Subject: [PATCH 3/5] Add Silent Hill: Origins to compatibility setting BlockTransferAllowCreateFB. Eliminates a readback with (so far) no perceptible bad effects. --- assets/compat.ini | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/assets/compat.ini b/assets/compat.ini index a86f7562b3d1..2e2f4efe245d 100644 --- a/assets/compat.ini +++ b/assets/compat.ini @@ -709,6 +709,14 @@ ULUS10428 = true ULES01375 = true ULUS10429 = true +# Silent Hill: Origins +# Avoids readback. +ULES00869 = true +ULUS10285 = true +ULKS46161 = true +ULJM05281 = true +NPJH50051 = true + [IntraVRAMBlockTransferAllowCreateFB] # Final Fantasy - Type 0 ULJM05900 = true From bd759790b025df70fcd869f1043ec0d25a77fa52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 28 Sep 2022 14:09:40 +0200 Subject: [PATCH 4/5] Update the Vulkan debug names when reassigning depth buffers. --- Common/GPU/Vulkan/VulkanRenderManager.cpp | 22 ++++++++++++++++++++-- Common/GPU/Vulkan/VulkanRenderManager.h | 2 ++ Common/GPU/Vulkan/thin3d_vulkan.cpp | 3 +++ Common/GPU/thin3d.h | 1 + GPU/Common/FramebufferManagerCommon.cpp | 14 +++++++++----- 5 files changed, 35 insertions(+), 7 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanRenderManager.cpp b/Common/GPU/Vulkan/VulkanRenderManager.cpp index 49abbde2d037..b0d83273016b 100644 --- a/Common/GPU/Vulkan/VulkanRenderManager.cpp +++ b/Common/GPU/Vulkan/VulkanRenderManager.cpp @@ -153,16 +153,34 @@ VKRFramebuffer::VKRFramebuffer(VulkanContext *vk, VkCommandBuffer initCmd, VKRRe _dbg_assert_(tag); CreateImage(vulkan_, initCmd, color, width, height, VK_FORMAT_R8G8B8A8_UNORM, VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL, true, tag); - vulkan_->SetDebugName(color.image, VK_OBJECT_TYPE_IMAGE, StringFromFormat("fb_color_%s", tag).c_str()); if (createDepthStencilBuffer) { CreateImage(vulkan_, initCmd, depth, width, height, vulkan_->GetDeviceInfo().preferredDepthStencilFormat, VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL, false, tag); - vulkan_->SetDebugName(depth.image, VK_OBJECT_TYPE_IMAGE, StringFromFormat("fb_depth_%s", tag).c_str()); } + UpdateTag(tag); + // We create the actual framebuffer objects on demand, because some combinations might not make sense. // Framebuffer objects are just pointers to a set of images, so no biggie. } +void VKRFramebuffer::UpdateTag(const char *newTag) { + char name[128]; + snprintf(name, sizeof(name), "fb_color_%s", tag_.c_str()); + vulkan_->SetDebugName(color.image, VK_OBJECT_TYPE_IMAGE, name); + vulkan_->SetDebugName(color.image, VK_OBJECT_TYPE_IMAGE_VIEW, name); + if (depth.image) { + snprintf(name, sizeof(name), "fb_depth_%s", tag_.c_str()); + vulkan_->SetDebugName(depth.image, VK_OBJECT_TYPE_IMAGE, name); + vulkan_->SetDebugName(depth.image, VK_OBJECT_TYPE_IMAGE_VIEW, name); + } + for (int rpType = 0; rpType < RP_TYPE_COUNT; rpType++) { + if (framebuf[rpType]) { + snprintf(name, sizeof(name), "fb_%s", tag_.c_str()); + vulkan_->SetDebugName(framebuf[(int)rpType], VK_OBJECT_TYPE_FRAMEBUFFER, name); + } + } +} + VkFramebuffer VKRFramebuffer::Get(VKRRenderPass *compatibleRenderPass, RenderPassType rpType) { if (framebuf[(int)rpType]) { return framebuf[(int)rpType]; diff --git a/Common/GPU/Vulkan/VulkanRenderManager.h b/Common/GPU/Vulkan/VulkanRenderManager.h index 5059324116a1..66692d99a7d4 100644 --- a/Common/GPU/Vulkan/VulkanRenderManager.h +++ b/Common/GPU/Vulkan/VulkanRenderManager.h @@ -57,6 +57,8 @@ class VKRFramebuffer { return tag_.c_str(); } + void UpdateTag(const char *newTag); + // TODO: Hide. VulkanContext *vulkan_; private: diff --git a/Common/GPU/Vulkan/thin3d_vulkan.cpp b/Common/GPU/Vulkan/thin3d_vulkan.cpp index ceba56f78b29..111cea1d2b97 100644 --- a/Common/GPU/Vulkan/thin3d_vulkan.cpp +++ b/Common/GPU/Vulkan/thin3d_vulkan.cpp @@ -1490,6 +1490,9 @@ class VKFramebuffer : public Framebuffer { buf_ = nullptr; } VKRFramebuffer *GetFB() const { return buf_; } + void UpdateTag(const char *newTag) override { + buf_->UpdateTag(newTag); + } private: VKRFramebuffer *buf_; }; diff --git a/Common/GPU/thin3d.h b/Common/GPU/thin3d.h index 0fba6ee6f262..ee0d50602ae3 100644 --- a/Common/GPU/thin3d.h +++ b/Common/GPU/thin3d.h @@ -411,6 +411,7 @@ class Framebuffer : public RefCountedObject { public: int Width() { return width_; } int Height() { return height_; } + virtual void UpdateTag(const char *tag) {} protected: int width_ = -1, height_ = -1; }; diff --git a/GPU/Common/FramebufferManagerCommon.cpp b/GPU/Common/FramebufferManagerCommon.cpp index 807472703dbe..f97f2e91c08e 100644 --- a/GPU/Common/FramebufferManagerCommon.cpp +++ b/GPU/Common/FramebufferManagerCommon.cpp @@ -49,6 +49,10 @@ #include "GPU/GPUInterface.h" #include "GPU/GPUState.h" +static size_t FormatFramebufferName(VirtualFramebuffer *vfb, char *tag, size_t len) { + return snprintf(tag, len, "FB_%08x_%08x_%dx%d_%s", vfb->fb_address, vfb->z_address, vfb->bufferWidth, vfb->bufferHeight, GeBufferFormatToString(vfb->fb_format)); +} + FramebufferManagerCommon::FramebufferManagerCommon(Draw::DrawContext *draw) : draw_(draw), draw2D_(draw_) { presentation_ = new PresentationCommon(draw); @@ -553,11 +557,15 @@ void FramebufferManagerCommon::SetDepthFrameBuffer(bool isClearingDepth) { uint32_t boundDepthBuffer = gstate.getDepthBufAddress() & 0x3FFFFFFF; if (currentRenderVfb_->z_address != boundDepthBuffer) { - WARN_LOG(G3D, "Framebuffer at %08x/%d has switched associated depth buffer from %08x to %08x, updating.", + WARN_LOG_N_TIMES(z_reassign, 5, G3D, "Framebuffer at %08x/%d has switched associated depth buffer from %08x to %08x, updating.", currentRenderVfb_->fb_address, currentRenderVfb_->fb_stride, currentRenderVfb_->z_address, boundDepthBuffer); + // Technically, here we should copy away the depth buffer to another framebuffer that uses that z_address, or maybe // even write it back to RAM. However, this is rare. Silent Hill is one example, see #16126. currentRenderVfb_->z_address = boundDepthBuffer; + char tag[128]; + FormatFramebufferName(currentRenderVfb_, tag, sizeof(tag)); + currentRenderVfb_->fbo->UpdateTag(tag); } // If this first draw call is anything other than a clear, "resolve" the depth buffer, @@ -1495,10 +1503,6 @@ void FramebufferManagerCommon::DecimateFBOs() { } } -static size_t FormatFramebufferName(VirtualFramebuffer *vfb, char *tag, size_t len) { - return snprintf(tag, len, "FB_%08x_%08x_%dx%d_%s", vfb->fb_address, vfb->z_address, vfb->bufferWidth, vfb->bufferHeight, GeBufferFormatToString(vfb->fb_format)); -} - // Requires width/height to be set already. void FramebufferManagerCommon::ResizeFramebufFBO(VirtualFramebuffer *vfb, int w, int h, bool force, bool skipCopy) { _dbg_assert_(w > 0); From 8534b8d7ca678c6ad41f5f835cbe1de601174010 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 28 Sep 2022 16:44:40 +0200 Subject: [PATCH 5/5] Typo fix --- Common/GPU/Vulkan/VulkanRenderManager.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanRenderManager.cpp b/Common/GPU/Vulkan/VulkanRenderManager.cpp index b0d83273016b..47d4d1896e66 100644 --- a/Common/GPU/Vulkan/VulkanRenderManager.cpp +++ b/Common/GPU/Vulkan/VulkanRenderManager.cpp @@ -167,11 +167,11 @@ void VKRFramebuffer::UpdateTag(const char *newTag) { char name[128]; snprintf(name, sizeof(name), "fb_color_%s", tag_.c_str()); vulkan_->SetDebugName(color.image, VK_OBJECT_TYPE_IMAGE, name); - vulkan_->SetDebugName(color.image, VK_OBJECT_TYPE_IMAGE_VIEW, name); + vulkan_->SetDebugName(color.imageView, VK_OBJECT_TYPE_IMAGE_VIEW, name); if (depth.image) { snprintf(name, sizeof(name), "fb_depth_%s", tag_.c_str()); vulkan_->SetDebugName(depth.image, VK_OBJECT_TYPE_IMAGE, name); - vulkan_->SetDebugName(depth.image, VK_OBJECT_TYPE_IMAGE_VIEW, name); + vulkan_->SetDebugName(depth.imageView, VK_OBJECT_TYPE_IMAGE_VIEW, name); } for (int rpType = 0; rpType < RP_TYPE_COUNT; rpType++) { if (framebuf[rpType]) {