From 32a7e7345e395ca211cde2681a5052acc3288256 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 16 May 2020 16:55:21 -0700 Subject: [PATCH 1/6] GPU: Centralize framebuffer download. And try not to use a potentially-null nvfb. Primarily this can happen on Direct3D backends. --- GPU/Common/FramebufferCommon.cpp | 46 ++++++++++++++++++++++----- GPU/Common/FramebufferCommon.h | 2 +- GPU/D3D11/FramebufferManagerD3D11.cpp | 13 -------- GPU/D3D11/FramebufferManagerD3D11.h | 1 - GPU/Directx9/FramebufferDX9.cpp | 13 -------- GPU/Directx9/FramebufferDX9.h | 1 - GPU/GLES/FramebufferManagerGLES.cpp | 28 ---------------- GPU/GLES/FramebufferManagerGLES.h | 1 - GPU/Vulkan/FramebufferVulkan.cpp | 13 -------- GPU/Vulkan/FramebufferVulkan.h | 1 - GPU/Vulkan/GPU_Vulkan.cpp | 1 + 11 files changed, 40 insertions(+), 80 deletions(-) diff --git a/GPU/Common/FramebufferCommon.cpp b/GPU/Common/FramebufferCommon.cpp index e3701b6d48..298f54d8ba 100644 --- a/GPU/Common/FramebufferCommon.cpp +++ b/GPU/Common/FramebufferCommon.cpp @@ -1351,11 +1351,10 @@ VirtualFramebuffer *FramebufferManagerCommon::CreateRAMFramebuffer(uint32_t fbAd VirtualFramebuffer *FramebufferManagerCommon::FindDownloadTempBuffer(VirtualFramebuffer *vfb) { // For now we'll keep these on the same struct as the ones that can get displayed // (and blatantly copy work already done above while at it). - VirtualFramebuffer *nvfb = 0; + VirtualFramebuffer *nvfb = nullptr; // We maintain a separate vector of framebuffer objects for blitting. - for (size_t i = 0; i < bvfbs_.size(); ++i) { - VirtualFramebuffer *v = bvfbs_[i]; + for (VirtualFramebuffer *v : bvfbs_) { if (v->fb_address == vfb->fb_address && v->format == vfb->format) { if (v->bufferWidth == vfb->bufferWidth && v->bufferHeight == vfb->bufferHeight) { nvfb = v; @@ -1405,6 +1404,34 @@ VirtualFramebuffer *FramebufferManagerCommon::FindDownloadTempBuffer(VirtualFram return nvfb; } +bool FramebufferManagerCommon::CreateDownloadTempBuffer(VirtualFramebuffer *nvfb) { + // When updating VRAM, it need to be exact format. + if (!gstate_c.Supports(GPU_PREFER_CPU_DOWNLOAD)) { + switch (nvfb->format) { + case GE_FORMAT_4444: + nvfb->colorDepth = Draw::FBO_4444; + break; + case GE_FORMAT_5551: + nvfb->colorDepth = Draw::FBO_5551; + break; + case GE_FORMAT_565: + nvfb->colorDepth = Draw::FBO_565; + break; + case GE_FORMAT_8888: + default: + nvfb->colorDepth = Draw::FBO_8888; + break; + } + } + + nvfb->fbo = draw_->CreateFramebuffer({ nvfb->bufferWidth, nvfb->bufferHeight, 1, 1, false, (Draw::FBColorDepth)nvfb->colorDepth }); + if (!nvfb->fbo) { + ERROR_LOG(FRAMEBUF, "Error creating GL FBO! %i x %i", nvfb->renderWidth, nvfb->renderHeight); + return false; + } + return true; +} + void FramebufferManagerCommon::ApplyClearToMemory(int x1, int y1, int x2, int y2, u32 clearColor) { if (currentRenderVfb_) { if ((currentRenderVfb_->usageFlags & FB_USAGE_DOWNLOAD_CLEAR) != 0) { @@ -1969,8 +1996,10 @@ void FramebufferManagerCommon::ReadFramebufferToMemory(VirtualFramebuffer *vfb, PackFramebufferSync_(vfb, x, y, w, h); } else { VirtualFramebuffer *nvfb = FindDownloadTempBuffer(vfb); - BlitFramebuffer(nvfb, x, y, vfb, x, y, w, h, 0); - PackFramebufferSync_(nvfb, x, y, w, h); + if (nvfb) { + BlitFramebuffer(nvfb, x, y, vfb, x, y, w, h, 0); + PackFramebufferSync_(nvfb, x, y, w, h); + } } textureCache_->ForgetLastTexture(); @@ -2014,9 +2043,10 @@ void FramebufferManagerCommon::DownloadFramebufferForClut(u32 fb_address, u32 lo // We'll pseudo-blit framebuffers here to get a resized version of vfb. VirtualFramebuffer *nvfb = FindDownloadTempBuffer(vfb); - BlitFramebuffer(nvfb, x, y, vfb, x, y, w, h, 0); - - PackFramebufferSync_(nvfb, x, y, w, h); + if (nvfb) { + BlitFramebuffer(nvfb, x, y, vfb, x, y, w, h, 0); + PackFramebufferSync_(nvfb, x, y, w, h); + } textureCache_->ForgetLastTexture(); RebindFramebuffer(); diff --git a/GPU/Common/FramebufferCommon.h b/GPU/Common/FramebufferCommon.h index f95a12670f..bf92ae189d 100644 --- a/GPU/Common/FramebufferCommon.h +++ b/GPU/Common/FramebufferCommon.h @@ -339,7 +339,7 @@ protected: void DownloadFramebufferOnSwitch(VirtualFramebuffer *vfb); void FindTransferFramebuffers(VirtualFramebuffer *&dstBuffer, VirtualFramebuffer *&srcBuffer, u32 dstBasePtr, int dstStride, int &dstX, int &dstY, u32 srcBasePtr, int srcStride, int &srcX, int &srcY, int &srcWidth, int &srcHeight, int &dstWidth, int &dstHeight, int bpp); VirtualFramebuffer *FindDownloadTempBuffer(VirtualFramebuffer *vfb); - virtual bool CreateDownloadTempBuffer(VirtualFramebuffer *nvfb) = 0; + virtual bool CreateDownloadTempBuffer(VirtualFramebuffer *nvfb); virtual void UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) = 0; VirtualFramebuffer *CreateRAMFramebuffer(uint32_t fbAddress, int width, int height, int stride, GEBufferFormat format); diff --git a/GPU/D3D11/FramebufferManagerD3D11.cpp b/GPU/D3D11/FramebufferManagerD3D11.cpp index 6bca9211b9..70e2b19bfc 100644 --- a/GPU/D3D11/FramebufferManagerD3D11.cpp +++ b/GPU/D3D11/FramebufferManagerD3D11.cpp @@ -366,19 +366,6 @@ void FramebufferManagerD3D11::BindFramebufferAsColorTexture(int stage, VirtualFr } } -bool FramebufferManagerD3D11::CreateDownloadTempBuffer(VirtualFramebuffer *nvfb) { - nvfb->colorDepth = Draw::FBO_8888; - - nvfb->fbo = draw_->CreateFramebuffer({ nvfb->bufferWidth, nvfb->bufferHeight, 1, 1, true, (Draw::FBColorDepth)nvfb->colorDepth }); - if (!(nvfb->fbo)) { - ERROR_LOG(FRAMEBUF, "Error creating FBO! %i x %i", nvfb->renderWidth, nvfb->renderHeight); - return false; - } - - draw_->BindFramebufferAsRenderTarget(nvfb->fbo, { Draw::RPAction::CLEAR, Draw::RPAction::CLEAR, Draw::RPAction::CLEAR }); - return true; -} - void FramebufferManagerD3D11::UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) { // Nothing to do here. } diff --git a/GPU/D3D11/FramebufferManagerD3D11.h b/GPU/D3D11/FramebufferManagerD3D11.h index aa8641bb14..73b917f326 100644 --- a/GPU/D3D11/FramebufferManagerD3D11.h +++ b/GPU/D3D11/FramebufferManagerD3D11.h @@ -60,7 +60,6 @@ protected: // Used by ReadFramebufferToMemory and later framebuffer block copies void BlitFramebuffer(VirtualFramebuffer *dst, int dstX, int dstY, VirtualFramebuffer *src, int srcX, int srcY, int w, int h, int bpp) override; - bool CreateDownloadTempBuffer(VirtualFramebuffer *nvfb) override; void UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) override; private: diff --git a/GPU/Directx9/FramebufferDX9.cpp b/GPU/Directx9/FramebufferDX9.cpp index 877b196e64..0300bbf45a 100644 --- a/GPU/Directx9/FramebufferDX9.cpp +++ b/GPU/Directx9/FramebufferDX9.cpp @@ -379,19 +379,6 @@ static const D3DVERTEXELEMENT9 g_FramebufferVertexElements[] = { } } - bool FramebufferManagerDX9::CreateDownloadTempBuffer(VirtualFramebuffer *nvfb) { - nvfb->colorDepth = Draw::FBO_8888; - - nvfb->fbo = draw_->CreateFramebuffer({ nvfb->bufferWidth, nvfb->bufferHeight, 1, 1, true, (Draw::FBColorDepth)nvfb->colorDepth }); - if (!(nvfb->fbo)) { - ERROR_LOG(FRAMEBUF, "Error creating FBO! %i x %i", nvfb->renderWidth, nvfb->renderHeight); - return false; - } - - draw_->BindFramebufferAsRenderTarget(nvfb->fbo, { Draw::RPAction::CLEAR, Draw::RPAction::CLEAR, Draw::RPAction::CLEAR }); - return true; - } - void FramebufferManagerDX9::UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) { // Nothing to do here. } diff --git a/GPU/Directx9/FramebufferDX9.h b/GPU/Directx9/FramebufferDX9.h index 3de91750c1..bab50d4dfc 100644 --- a/GPU/Directx9/FramebufferDX9.h +++ b/GPU/Directx9/FramebufferDX9.h @@ -71,7 +71,6 @@ protected: // Used by ReadFramebufferToMemory and later framebuffer block copies void BlitFramebuffer(VirtualFramebuffer *dst, int dstX, int dstY, VirtualFramebuffer *src, int srcX, int srcY, int w, int h, int bpp) override; - bool CreateDownloadTempBuffer(VirtualFramebuffer *nvfb) override; void UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) override; private: diff --git a/GPU/GLES/FramebufferManagerGLES.cpp b/GPU/GLES/FramebufferManagerGLES.cpp index 0800a8b917..fe597a4d49 100644 --- a/GPU/GLES/FramebufferManagerGLES.cpp +++ b/GPU/GLES/FramebufferManagerGLES.cpp @@ -313,34 +313,6 @@ void FramebufferManagerGLES::BindFramebufferAsColorTexture(int stage, VirtualFra } } -bool FramebufferManagerGLES::CreateDownloadTempBuffer(VirtualFramebuffer *nvfb) { - // When updating VRAM, it need to be exact format. - if (!gstate_c.Supports(GPU_PREFER_CPU_DOWNLOAD)) { - switch (nvfb->format) { - case GE_FORMAT_4444: - nvfb->colorDepth = Draw::FBO_4444; - break; - case GE_FORMAT_5551: - nvfb->colorDepth = Draw::FBO_5551; - break; - case GE_FORMAT_565: - nvfb->colorDepth = Draw::FBO_565; - break; - case GE_FORMAT_8888: - default: - nvfb->colorDepth = Draw::FBO_8888; - break; - } - } - - nvfb->fbo = draw_->CreateFramebuffer({ nvfb->bufferWidth, nvfb->bufferHeight, 1, 1, false, (Draw::FBColorDepth)nvfb->colorDepth }); - if (!nvfb->fbo) { - ERROR_LOG(FRAMEBUF, "Error creating GL FBO! %i x %i", nvfb->renderWidth, nvfb->renderHeight); - return false; - } - return true; -} - void FramebufferManagerGLES::UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) { _assert_msg_(G3D, nvfb->fbo, "Expecting a valid nvfb in UpdateDownloadTempBuffer"); diff --git a/GPU/GLES/FramebufferManagerGLES.h b/GPU/GLES/FramebufferManagerGLES.h index 33d364fe7b..25f541fd81 100644 --- a/GPU/GLES/FramebufferManagerGLES.h +++ b/GPU/GLES/FramebufferManagerGLES.h @@ -64,7 +64,6 @@ protected: // Used by ReadFramebufferToMemory and later framebuffer block copies void BlitFramebuffer(VirtualFramebuffer *dst, int dstX, int dstY, VirtualFramebuffer *src, int srcX, int srcY, int w, int h, int bpp) override; - bool CreateDownloadTempBuffer(VirtualFramebuffer *nvfb) override; void UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) override; private: diff --git a/GPU/Vulkan/FramebufferVulkan.cpp b/GPU/Vulkan/FramebufferVulkan.cpp index 5bb733ddb0..dfef7d47b7 100644 --- a/GPU/Vulkan/FramebufferVulkan.cpp +++ b/GPU/Vulkan/FramebufferVulkan.cpp @@ -325,19 +325,6 @@ VkImageView FramebufferManagerVulkan::BindFramebufferAsColorTexture(int stage, V } } -bool FramebufferManagerVulkan::CreateDownloadTempBuffer(VirtualFramebuffer *nvfb) { - nvfb->colorDepth = Draw::FBO_8888; - - nvfb->fbo = draw_->CreateFramebuffer({ nvfb->bufferWidth, nvfb->bufferHeight, 1, 1, true, (Draw::FBColorDepth)nvfb->colorDepth }); - if (!(nvfb->fbo)) { - ERROR_LOG(FRAMEBUF, "Error creating FBO! %i x %i", nvfb->renderWidth, nvfb->renderHeight); - return false; - } - - draw_->BindFramebufferAsRenderTarget(nvfb->fbo, { Draw::RPAction::CLEAR, Draw::RPAction::CLEAR, Draw::RPAction::CLEAR }); - return true; -} - void FramebufferManagerVulkan::UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) { // Nothing to do here. } diff --git a/GPU/Vulkan/FramebufferVulkan.h b/GPU/Vulkan/FramebufferVulkan.h index d17d836216..bbd799e1d0 100644 --- a/GPU/Vulkan/FramebufferVulkan.h +++ b/GPU/Vulkan/FramebufferVulkan.h @@ -71,7 +71,6 @@ protected: // Used by ReadFramebufferToMemory and later framebuffer block copies void BlitFramebuffer(VirtualFramebuffer *dst, int dstX, int dstY, VirtualFramebuffer *src, int srcX, int srcY, int w, int h, int bpp) override; - bool CreateDownloadTempBuffer(VirtualFramebuffer *nvfb) override; void UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) override; private: diff --git a/GPU/Vulkan/GPU_Vulkan.cpp b/GPU/Vulkan/GPU_Vulkan.cpp index 6cd2493f5a..43de4d1078 100644 --- a/GPU/Vulkan/GPU_Vulkan.cpp +++ b/GPU/Vulkan/GPU_Vulkan.cpp @@ -231,6 +231,7 @@ void GPU_Vulkan::CheckGPUFeatures() { features |= GPU_SUPPORTS_INSTANCE_RENDERING; features |= GPU_SUPPORTS_VERTEX_TEXTURE_FETCH; features |= GPU_SUPPORTS_TEXTURE_FLOAT; + features |= GPU_PREFER_CPU_DOWNLOAD; if (vulkan_->GetDeviceFeatures().enabled.wideLines) { features |= GPU_SUPPORTS_WIDE_LINES; From a36239473dd4dbed126e3d0ec1af83a5451feaf0 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 16 May 2020 16:56:31 -0700 Subject: [PATCH 2/6] GLES: Add dependency tracking for render passes. Let's try to invalidate when it's possible. We move the invalidate to the end of the render when detected. --- CMakeLists.txt | 1 + GPU/GLES/FramebufferManagerGLES.cpp | 2 +- ext/native/native.vcxproj | 1 + ext/native/native.vcxproj.filters | 3 ++ ext/native/thin3d/GLQueueRunner.cpp | 8 ++-- ext/native/thin3d/GLQueueRunner.h | 6 +++ ext/native/thin3d/GLRenderManager.cpp | 19 ++++++++ ext/native/thin3d/VulkanQueueRunner.h | 65 +------------------------- ext/native/util/tiny_set.h | 67 +++++++++++++++++++++++++++ 9 files changed, 104 insertions(+), 68 deletions(-) create mode 100644 ext/native/util/tiny_set.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 4d96d72320..a40d4334ec 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1070,6 +1070,7 @@ add_library(native STATIC ext/native/util/text/wrap_text.h ext/native/util/text/wrap_text.cpp ext/native/util/const_map.h + ext/native/util/tiny_set.h ext/native/ext/jpge/jpgd.cpp ext/native/ext/jpge/jpgd.h ext/native/ext/jpge/jpge.cpp diff --git a/GPU/GLES/FramebufferManagerGLES.cpp b/GPU/GLES/FramebufferManagerGLES.cpp index fe597a4d49..266b4fde93 100644 --- a/GPU/GLES/FramebufferManagerGLES.cpp +++ b/GPU/GLES/FramebufferManagerGLES.cpp @@ -317,7 +317,7 @@ void FramebufferManagerGLES::UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) _assert_msg_(G3D, nvfb->fbo, "Expecting a valid nvfb in UpdateDownloadTempBuffer"); // Discard the previous contents of this buffer where possible. - if (gl_extensions.GLES3 && glInvalidateFramebuffer != nullptr) { + if (gl_extensions.GLES3) { draw_->BindFramebufferAsRenderTarget(nvfb->fbo, { Draw::RPAction::DONT_CARE, Draw::RPAction::DONT_CARE, Draw::RPAction::DONT_CARE }); } else if (gl_extensions.IsGLES) { draw_->BindFramebufferAsRenderTarget(nvfb->fbo, { Draw::RPAction::CLEAR, Draw::RPAction::CLEAR, Draw::RPAction::CLEAR }); diff --git a/ext/native/native.vcxproj b/ext/native/native.vcxproj index 80b0f68e59..60f361b959 100644 --- a/ext/native/native.vcxproj +++ b/ext/native/native.vcxproj @@ -489,6 +489,7 @@ + diff --git a/ext/native/native.vcxproj.filters b/ext/native/native.vcxproj.filters index 82e64872aa..9c6af13184 100644 --- a/ext/native/native.vcxproj.filters +++ b/ext/native/native.vcxproj.filters @@ -350,6 +350,9 @@ ui + + util + diff --git a/ext/native/thin3d/GLQueueRunner.cpp b/ext/native/thin3d/GLQueueRunner.cpp index 20c5bf7800..78289b0fcb 100644 --- a/ext/native/thin3d/GLQueueRunner.cpp +++ b/ext/native/thin3d/GLQueueRunner.cpp @@ -827,13 +827,15 @@ void GLQueueRunner::PerformRenderPass(const GLRStep &step) { { GLenum attachments[3]; int count = 0; + bool hasDepth = step.render.framebuffer ? step.render.framebuffer->z_stencil_ : false; if (c.clear.clearMask & GL_COLOR_BUFFER_BIT) attachments[count++] = GL_COLOR_ATTACHMENT0; - if (c.clear.clearMask & GL_DEPTH_BUFFER_BIT) + if (hasDepth && (c.clear.clearMask & GL_DEPTH_BUFFER_BIT)) attachments[count++] = GL_DEPTH_ATTACHMENT; - if (c.clear.clearMask & GL_STENCIL_BUFFER_BIT) + if (hasDepth && (c.clear.clearMask & GL_STENCIL_BUFFER_BIT)) attachments[count++] = GL_STENCIL_BUFFER_BIT; - glInvalidateFramebuffer(GL_FRAMEBUFFER, count, attachments); + if (glInvalidateFramebuffer != nullptr && count != 0) + glInvalidateFramebuffer(GL_FRAMEBUFFER, count, attachments); CHECK_GL_ERROR_IF_DEBUG(); break; } diff --git a/ext/native/thin3d/GLQueueRunner.h b/ext/native/thin3d/GLQueueRunner.h index 41bceedb20..813f8c020e 100644 --- a/ext/native/thin3d/GLQueueRunner.h +++ b/ext/native/thin3d/GLQueueRunner.h @@ -6,6 +6,7 @@ #include "gfx/gl_common.h" #include "thin3d/DataFormat.h" +#include "util/tiny_set.h" struct GLRViewport { float x, y, w, h, minZ, maxZ; @@ -295,9 +296,14 @@ struct GLRStep { GLRStep(GLRStepType _type) : stepType(_type) {} GLRStepType stepType; std::vector commands; + TinySet dependencies; union { struct { GLRFramebuffer *framebuffer; + GLRRenderPassAction color; + GLRRenderPassAction depth; + GLRRenderPassAction stencil; + // Note: not accurate. int numDraws; } render; struct { diff --git a/ext/native/thin3d/GLRenderManager.cpp b/ext/native/thin3d/GLRenderManager.cpp index 670021cf93..e1d8393162 100644 --- a/ext/native/thin3d/GLRenderManager.cpp +++ b/ext/native/thin3d/GLRenderManager.cpp @@ -279,6 +279,9 @@ void GLRenderManager::BindFramebufferAsRenderTarget(GLRFramebuffer *fb, GLRRende GLRStep *step = new GLRStep{ GLRStepType::RENDER }; // This is what queues up new passes, and can end previous ones. step->render.framebuffer = fb; + step->render.color = color; + step->render.depth = depth; + step->render.stencil = stencil; step->render.numDraws = 0; steps_.push_back(step); @@ -308,6 +311,12 @@ void GLRenderManager::BindFramebufferAsRenderTarget(GLRFramebuffer *fb, GLRRende } curRenderStep_ = step; + if (fb) { + if (color == GLRRenderPassAction::KEEP || depth == GLRRenderPassAction::KEEP || stencil == GLRRenderPassAction::KEEP) { + step->dependencies.insert(fb); + } + } + // Every step clears this state. gstate_c.Dirty(DIRTY_BLEND_STATE | DIRTY_DEPTHSTENCIL_STATE | DIRTY_RASTER_STATE); } @@ -319,6 +328,7 @@ void GLRenderManager::BindFramebufferAsTexture(GLRFramebuffer *fb, int binding, data.bind_fb_texture.framebuffer = fb; data.bind_fb_texture.aspect = aspectBit; curRenderStep_->commands.push_back(data); + curRenderStep_->dependencies.insert(fb); } void GLRenderManager::CopyFramebuffer(GLRFramebuffer *src, GLRect2D srcRect, GLRFramebuffer *dst, GLOffset2D dstPos, int aspectMask) { @@ -328,6 +338,10 @@ void GLRenderManager::CopyFramebuffer(GLRFramebuffer *src, GLRect2D srcRect, GLR step->copy.src = src; step->copy.dst = dst; step->copy.aspectMask = aspectMask; + step->dependencies.insert(src); + bool fillsDst = dst && srcRect.x == 0 && srcRect.y == 0 && srcRect.w == dst->width && srcRect.h == dst->height; + if (dstPos.x != 0 || dstPos.y != 0 || !fillsDst) + step->dependencies.insert(dst); steps_.push_back(step); // Every step clears this state. @@ -342,6 +356,10 @@ void GLRenderManager::BlitFramebuffer(GLRFramebuffer *src, GLRect2D srcRect, GLR step->blit.dst = dst; step->blit.aspectMask = aspectMask; step->blit.filter = filter; + step->dependencies.insert(src); + bool fillsDst = dst && dstRect.x == 0 && dstRect.y == 0 && dstRect.w == dst->width && dstRect.h == dst->height; + if (!fillsDst) + step->dependencies.insert(dst); steps_.push_back(step); // Every step clears this state. @@ -356,6 +374,7 @@ bool GLRenderManager::CopyFramebufferToMemorySync(GLRFramebuffer *src, int aspec step->readback.srcRect = { x, y, w, h }; step->readback.aspectMask = aspectBits; step->readback.dstFormat = destFormat; + step->dependencies.insert(src); steps_.push_back(step); // Every step clears this state. diff --git a/ext/native/thin3d/VulkanQueueRunner.h b/ext/native/thin3d/VulkanQueueRunner.h index 129c441b86..1c9e46a5cf 100644 --- a/ext/native/thin3d/VulkanQueueRunner.h +++ b/ext/native/thin3d/VulkanQueueRunner.h @@ -6,6 +6,7 @@ #include "Common/Vulkan/VulkanContext.h" #include "math/dataconv.h" #include "thin3d/DataFormat.h" +#include "util/tiny_set.h" class VKRFramebuffer; struct VKRImage; @@ -17,70 +18,6 @@ enum { QUEUE_HACK_RENDERPASS_MERGE = 8, }; -// Insert-only small-set implementation. Performs no allocation unless MaxFastSize is exceeded. -template -struct TinySet { - ~TinySet() { delete slowLookup_; } - inline void insert(T t) { - // Fast linear scan. - for (int i = 0; i < fastCount; i++) { - if (fastLookup_[i] == t) - return; // We already have it. - } - // Fast insertion - if (fastCount < MaxFastSize) { - fastLookup_[fastCount++] = t; - return; - } - // Fall back to slow path. - insertSlow(t); - } - bool contains(T t) const { - for (int i = 0; i < fastCount; i++) { - if (fastLookup_[i] == t) - return true; - } - if (slowLookup_) { - for (auto x : *slowLookup_) { - if (x == t) - return true; - } - } - return false; - } - bool contains(const TinySet &otherSet) { - // Awkward, kind of ruins the fun. - for (int i = 0; i < fastCount; i++) { - if (otherSet.contains(fastLookup_[i])) - return true; - } - if (slowLookup_) { - for (auto x : *slowLookup_) { - if (otherSet.contains(x)) - return true; - } - } - return false; - } - -private: - void insertSlow(T t) { - if (!slowLookup_) { - slowLookup_ = new std::vector(); - } else { - for (size_t i = 0; i < slowLookup_->size(); i++) { - if ((*slowLookup_)[i] == t) - return; - } - } - slowLookup_->push_back(t); - } - T fastLookup_[MaxFastSize]; - int fastCount = 0; - int slowCount = 0; - std::vector *slowLookup_ = nullptr; -}; - enum class VKRRenderCommand : uint8_t { REMOVED, BIND_PIPELINE, diff --git a/ext/native/util/tiny_set.h b/ext/native/util/tiny_set.h new file mode 100644 index 0000000000..f82bf35633 --- /dev/null +++ b/ext/native/util/tiny_set.h @@ -0,0 +1,67 @@ +#pragma once + +#include + +// Insert-only small-set implementation. Performs no allocation unless MaxFastSize is exceeded. +template +struct TinySet { + ~TinySet() { delete slowLookup_; } + inline void insert(T t) { + // Fast linear scan. + for (int i = 0; i < fastCount; i++) { + if (fastLookup_[i] == t) + return; // We already have it. + } + // Fast insertion + if (fastCount < MaxFastSize) { + fastLookup_[fastCount++] = t; + return; + } + // Fall back to slow path. + insertSlow(t); + } + bool contains(T t) const { + for (int i = 0; i < fastCount; i++) { + if (fastLookup_[i] == t) + return true; + } + if (slowLookup_) { + for (auto x : *slowLookup_) { + if (x == t) + return true; + } + } + return false; + } + bool contains(const TinySet &otherSet) { + // Awkward, kind of ruins the fun. + for (int i = 0; i < fastCount; i++) { + if (otherSet.contains(fastLookup_[i])) + return true; + } + if (slowLookup_) { + for (auto x : *slowLookup_) { + if (otherSet.contains(x)) + return true; + } + } + return false; + } + +private: + void insertSlow(T t) { + if (!slowLookup_) { + slowLookup_ = new std::vector(); + } else { + for (size_t i = 0; i < slowLookup_->size(); i++) { + if ((*slowLookup_)[i] == t) + return; + } + } + slowLookup_->push_back(t); + } + T fastLookup_[MaxFastSize]; + int fastCount = 0; + int slowCount = 0; + std::vector *slowLookup_ = nullptr; +}; From 47bc11a6828ccd031e70698b01d312b07363ea04 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 16 May 2020 18:30:59 -0700 Subject: [PATCH 3/6] Vulkan: Generalize dependency tracking. Might as well track on all steps. We also can know if the dst was fully written to here. --- ext/native/thin3d/VulkanQueueRunner.cpp | 17 +++++------------ ext/native/thin3d/VulkanRenderManager.cpp | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/ext/native/thin3d/VulkanQueueRunner.cpp b/ext/native/thin3d/VulkanQueueRunner.cpp index 28aa244776..c896b06b57 100644 --- a/ext/native/thin3d/VulkanQueueRunner.cpp +++ b/ext/native/thin3d/VulkanQueueRunner.cpp @@ -767,11 +767,13 @@ void VulkanQueueRunner::ApplyRenderPassMerge(std::vector &steps) { TinySet touchedFramebuffers; // must be the same fast-size as the dependencies TinySet for annoying reasons. for (int j = i + 1; j < (int)steps.size(); j++) { // If any other passes are reading from this framebuffer as-is, we cancel the scan. + if (steps[j]->dependencies.contains(fb)) { + // Reading from itself means a KEEP, which is okay. + if (steps[j]->stepType != VKRStepType::RENDER || steps[j]->render.framebuffer != fb) + break; + } switch (steps[j]->stepType) { case VKRStepType::RENDER: - if (steps[j]->dependencies.contains(fb)) { - goto done_fb; - } // Prevent Unknown's example case from https://github.com/hrydgard/ppsspp/pull/12242 if (steps[j]->dependencies.contains(touchedFramebuffers)) { goto done_fb; @@ -793,23 +795,14 @@ void VulkanQueueRunner::ApplyRenderPassMerge(std::vector &steps) { } break; case VKRStepType::COPY: - if (steps[j]->copy.src == fb || steps[j]->copy.dst == fb) { - goto done_fb; - } touchedFramebuffers.insert(steps[j]->copy.dst); break; case VKRStepType::BLIT: - if (steps[j]->blit.src == fb || steps[j]->blit.dst == fb) { - goto done_fb; - } touchedFramebuffers.insert(steps[j]->blit.dst); break; case VKRStepType::READBACK: // Not sure this has much effect, when executed READBACK is always the last step // since we stall the GPU and wait immediately after. - if (steps[j]->readback.src == fb) { - goto done_fb; - } break; default: break; diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index 7a639b7d4e..fb39fc3cd0 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -490,6 +490,13 @@ void VulkanRenderManager::BindFramebufferAsRenderTarget(VKRFramebuffer *fb, VKRR step->render.finalColorLayout = !fb ? VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL : VK_IMAGE_LAYOUT_UNDEFINED; steps_.push_back(step); + if (fb) { + // If there's a KEEP, we naturally read from the framebuffer. + if (color == VKRRenderPassAction::KEEP || depth == VKRRenderPassAction::KEEP || stencil == VKRRenderPassAction::KEEP) { + step->dependencies.insert(fb); + } + } + curRenderStep_ = step; if (fb) { curWidth_ = fb->width; @@ -513,6 +520,7 @@ bool VulkanRenderManager::CopyFramebufferToMemorySync(VKRFramebuffer *src, int a step->readback.src = src; step->readback.srcRect.offset = { x, y }; step->readback.srcRect.extent = { (uint32_t)w, (uint32_t)h }; + step->dependencies.insert(src); steps_.push_back(step); curRenderStep_ = nullptr; @@ -821,6 +829,10 @@ void VulkanRenderManager::CopyFramebuffer(VKRFramebuffer *src, VkRect2D srcRect, step->copy.srcRect = srcRect; step->copy.dst = dst; step->copy.dstPos = dstPos; + step->dependencies.insert(src); + bool fillsDst = dst && srcRect.offset.x == 0 && srcRect.offset.y == 0 && srcRect.extent.width == dst->width && srcRect.extent.height == dst->height; + if (dstPos.x != 0 || dstPos.y != 0 || !fillsDst) + step->dependencies.insert(dst); std::unique_lock lock(mutex_); steps_.push_back(step); @@ -859,6 +871,10 @@ void VulkanRenderManager::BlitFramebuffer(VKRFramebuffer *src, VkRect2D srcRect, step->blit.dst = dst; step->blit.dstRect = dstRect; step->blit.filter = filter; + step->dependencies.insert(src); + bool fillsDst = dst && dstRect.offset.x == 0 && dstRect.offset.y == 0 && dstRect.extent.width == dst->width && dstRect.extent.height == dst->height; + if (!fillsDst) + step->dependencies.insert(dst); std::unique_lock lock(mutex_); steps_.push_back(step); From caf9d39fec4a39919415a4ec59795cbac3c28a01 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 16 May 2020 18:51:23 -0700 Subject: [PATCH 4/6] GLES: Fix invalidation of backbuffer. And of the stencil buffer. --- ext/native/thin3d/GLQueueRunner.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ext/native/thin3d/GLQueueRunner.cpp b/ext/native/thin3d/GLQueueRunner.cpp index 78289b0fcb..5a297aa974 100644 --- a/ext/native/thin3d/GLQueueRunner.cpp +++ b/ext/native/thin3d/GLQueueRunner.cpp @@ -827,13 +827,14 @@ void GLQueueRunner::PerformRenderPass(const GLRStep &step) { { GLenum attachments[3]; int count = 0; - bool hasDepth = step.render.framebuffer ? step.render.framebuffer->z_stencil_ : false; + bool isFBO = step.render.framebuffer != nullptr; + bool hasDepth = isFBO ? step.render.framebuffer->z_stencil_ : false; if (c.clear.clearMask & GL_COLOR_BUFFER_BIT) - attachments[count++] = GL_COLOR_ATTACHMENT0; + attachments[count++] = isFBO ? GL_COLOR_ATTACHMENT0 : GL_COLOR; if (hasDepth && (c.clear.clearMask & GL_DEPTH_BUFFER_BIT)) - attachments[count++] = GL_DEPTH_ATTACHMENT; + attachments[count++] = isFBO ? GL_DEPTH_ATTACHMENT : GL_DEPTH; if (hasDepth && (c.clear.clearMask & GL_STENCIL_BUFFER_BIT)) - attachments[count++] = GL_STENCIL_BUFFER_BIT; + attachments[count++] = isFBO ? GL_STENCIL_ATTACHMENT : GL_STENCIL; if (glInvalidateFramebuffer != nullptr && count != 0) glInvalidateFramebuffer(GL_FRAMEBUFFER, count, attachments); CHECK_GL_ERROR_IF_DEBUG(); From 112a58cb1a041f62fdadd3014ac74ad262184694 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 17 May 2020 14:53:37 -0700 Subject: [PATCH 5/6] Vulkan: Make merge pass a bit easier to read. --- ext/native/thin3d/VulkanQueueRunner.cpp | 28 ++++++++++++++----------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/ext/native/thin3d/VulkanQueueRunner.cpp b/ext/native/thin3d/VulkanQueueRunner.cpp index c896b06b57..12eb1a4ca2 100644 --- a/ext/native/thin3d/VulkanQueueRunner.cpp +++ b/ext/native/thin3d/VulkanQueueRunner.cpp @@ -1,4 +1,4 @@ -#include +#include #include "base/timeutil.h" #include "DataFormat.h" @@ -752,13 +752,25 @@ std::string VulkanQueueRunner::StepToString(const VKRStep &step) const { void VulkanQueueRunner::ApplyRenderPassMerge(std::vector &steps) { // First let's count how many times each framebuffer is rendered to. // If it's more than one, let's do our best to merge them. This can help God of War quite a bit. - std::map counts; + std::unordered_map counts; for (int i = 0; i < (int)steps.size(); i++) { if (steps[i]->stepType == VKRStepType::RENDER) { counts[steps[i]->render.framebuffer]++; } } + auto mergeRenderSteps = [](VKRStep *dst, VKRStep *src) { + // OK. Now, if it's a render, slurp up all the commands and kill the step. + // Also slurp up any pretransitions. + dst->preTransitions.insert(dst->preTransitions.end(), src->preTransitions.begin(), src->preTransitions.end()); + dst->commands.insert(dst->commands.end(), src->commands.begin(), src->commands.end()); + src->stepType = VKRStepType::RENDER_SKIP; + }; + auto renderHasNoClear = [](const VKRStep *step) { + const auto &r = step->render; + return r.color != VKRRenderPassAction::CLEAR && r.depth != VKRRenderPassAction::CLEAR && r.stencil != VKRRenderPassAction::CLEAR; + }; + // Now, let's go through the steps. If we find one that is rendered to more than once, // we'll scan forward and slurp up any rendering that can be merged across. for (int i = 0; i < (int)steps.size(); i++) { @@ -778,16 +790,8 @@ void VulkanQueueRunner::ApplyRenderPassMerge(std::vector &steps) { if (steps[j]->dependencies.contains(touchedFramebuffers)) { goto done_fb; } - if (steps[j]->render.framebuffer == fb && - steps[j]->render.color != VKRRenderPassAction::CLEAR && - steps[j]->render.depth != VKRRenderPassAction::CLEAR && - steps[j]->render.stencil != VKRRenderPassAction::CLEAR) { - // ok. Now, if it's a render, slurp up all the commands - // and kill the step. - // Also slurp up any pretransitions. - steps[i]->preTransitions.insert(steps[i]->preTransitions.end(), steps[j]->preTransitions.begin(), steps[j]->preTransitions.end()); - steps[i]->commands.insert(steps[i]->commands.end(), steps[j]->commands.begin(), steps[j]->commands.end()); - steps[j]->stepType = VKRStepType::RENDER_SKIP; + if (steps[j]->render.framebuffer == fb && renderHasNoClear(steps[j])) { + mergeRenderSteps(steps[i], steps[j]); } // Remember the framebuffer this wrote to. We can't merge with later passes that depend on these. if (steps[j]->render.framebuffer != fb) { From 059e2e5e16fbabb253dc7d6a5a1ddc451e5a2731 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 17 May 2020 15:02:51 -0700 Subject: [PATCH 6/6] Vulkan: Stop merge at clear/touched read only. Before we would merge on a future render after a clear, which can't be good. But we can safely ignore a draw to another FB that reads from a touched FB. --- ext/native/thin3d/VulkanQueueRunner.cpp | 39 +++++++++++++++++-------- ext/native/util/tiny_set.h | 6 +++- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/ext/native/thin3d/VulkanQueueRunner.cpp b/ext/native/thin3d/VulkanQueueRunner.cpp index 12eb1a4ca2..a74a679e6e 100644 --- a/ext/native/thin3d/VulkanQueueRunner.cpp +++ b/ext/native/thin3d/VulkanQueueRunner.cpp @@ -764,11 +764,13 @@ void VulkanQueueRunner::ApplyRenderPassMerge(std::vector &steps) { // Also slurp up any pretransitions. dst->preTransitions.insert(dst->preTransitions.end(), src->preTransitions.begin(), src->preTransitions.end()); dst->commands.insert(dst->commands.end(), src->commands.begin(), src->commands.end()); + // So we don't consider it for other things, maybe doesn't matter. + src->dependencies.clear(); src->stepType = VKRStepType::RENDER_SKIP; }; - auto renderHasNoClear = [](const VKRStep *step) { + auto renderHasClear = [](const VKRStep *step) { const auto &r = step->render; - return r.color != VKRRenderPassAction::CLEAR && r.depth != VKRRenderPassAction::CLEAR && r.stencil != VKRRenderPassAction::CLEAR; + return r.color == VKRRenderPassAction::CLEAR || r.depth == VKRRenderPassAction::CLEAR || r.stencil == VKRRenderPassAction::CLEAR; }; // Now, let's go through the steps. If we find one that is rendered to more than once, @@ -786,30 +788,43 @@ void VulkanQueueRunner::ApplyRenderPassMerge(std::vector &steps) { } switch (steps[j]->stepType) { case VKRStepType::RENDER: - // Prevent Unknown's example case from https://github.com/hrydgard/ppsspp/pull/12242 - if (steps[j]->dependencies.contains(touchedFramebuffers)) { - goto done_fb; - } - if (steps[j]->render.framebuffer == fb && renderHasNoClear(steps[j])) { - mergeRenderSteps(steps[i], steps[j]); - } - // Remember the framebuffer this wrote to. We can't merge with later passes that depend on these. - if (steps[j]->render.framebuffer != fb) { + if (steps[j]->render.framebuffer == fb) { + // Prevent Unknown's example case from https://github.com/hrydgard/ppsspp/pull/12242 + if (renderHasClear(steps[j]) || steps[j]->dependencies.contains(touchedFramebuffers)) { + goto done_fb; + } else { + // Safe to merge, great. + mergeRenderSteps(steps[i], steps[j]); + } + } else { + // Remember the framebuffer this wrote to. We can't merge with later passes that depend on these. touchedFramebuffers.insert(steps[j]->render.framebuffer); } break; case VKRStepType::COPY: + if (steps[j]->copy.dst == fb) { + // Without framebuffer "renaming", we can't merge past a clobbered fb. + goto done_fb; + } touchedFramebuffers.insert(steps[j]->copy.dst); break; case VKRStepType::BLIT: + if (steps[j]->blit.dst == fb) { + // Without framebuffer "renaming", we can't merge past a clobbered fb. + goto done_fb; + } touchedFramebuffers.insert(steps[j]->blit.dst); break; case VKRStepType::READBACK: // Not sure this has much effect, when executed READBACK is always the last step // since we stall the GPU and wait immediately after. break; - default: + case VKRStepType::RENDER_SKIP: + case VKRStepType::READBACK_IMAGE: break; + default: + // We added a new step? Might be unsafe. + goto done_fb; } } done_fb: diff --git a/ext/native/util/tiny_set.h b/ext/native/util/tiny_set.h index f82bf35633..8a7cf89fa6 100644 --- a/ext/native/util/tiny_set.h +++ b/ext/native/util/tiny_set.h @@ -47,6 +47,11 @@ struct TinySet { } return false; } + void clear() { + delete slowLookup_; + slowLookup_ = nullptr; + fastCount = 0; + } private: void insertSlow(T t) { @@ -62,6 +67,5 @@ private: } T fastLookup_[MaxFastSize]; int fastCount = 0; - int slowCount = 0; std::vector *slowLookup_ = nullptr; };