From 62484f01edf2968544168022ca5bbc1a7bf45b80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 4 Feb 2023 12:05:50 +0100 Subject: [PATCH] Make ReadbackFramebufferSync able to use the stretch ability of ReadbackDepthbufferSync --- GPU/Common/FramebufferManagerCommon.cpp | 34 +++++++++++++++---------- GPU/Common/FramebufferManagerCommon.h | 4 +++ GPU/Common/TextureCacheCommon.cpp | 4 +-- GPU/GPU.cpp | 4 +++ GPU/GPU.h | 2 ++ 5 files changed, 33 insertions(+), 15 deletions(-) diff --git a/GPU/Common/FramebufferManagerCommon.cpp b/GPU/Common/FramebufferManagerCommon.cpp index 0f7c5fe2cf..fc6b9bf86c 100644 --- a/GPU/Common/FramebufferManagerCommon.cpp +++ b/GPU/Common/FramebufferManagerCommon.cpp @@ -2072,7 +2072,8 @@ VirtualFramebuffer *FramebufferManagerCommon::CreateRAMFramebuffer(uint32_t fbAd return vfb; } -// 1:1 pixel sides buffers, we resize buffers to these before we read them back. +// 1:1 pixel size buffers, we resize buffers to these before we read them back. +// TODO: We shouldn't keep whole VirtualFramebuffer structs for these - the fbo and last_frame_render is enough. VirtualFramebuffer *FramebufferManagerCommon::FindDownloadTempBuffer(VirtualFramebuffer *vfb, RasterChannel channel) { // 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). @@ -2080,10 +2081,14 @@ VirtualFramebuffer *FramebufferManagerCommon::FindDownloadTempBuffer(VirtualFram // We maintain a separate vector of framebuffer objects for blitting. for (VirtualFramebuffer *v : bvfbs_) { - if (v->fb_address == vfb->fb_address && v->fb_format == vfb->fb_format) { + if (v->Address(channel) == vfb->Address(channel) && v->Format(channel) == vfb->Format(channel)) { if (v->bufferWidth == vfb->bufferWidth && v->bufferHeight == vfb->bufferHeight) { nvfb = v; - v->fb_stride = vfb->fb_stride; + if (channel == RASTER_COLOR) { + v->fb_stride = vfb->fb_stride; + } else { + v->z_stride = vfb->z_stride; + } v->width = vfb->width; v->height = vfb->height; break; @@ -2095,10 +2100,10 @@ VirtualFramebuffer *FramebufferManagerCommon::FindDownloadTempBuffer(VirtualFram if (!nvfb) { nvfb = new VirtualFramebuffer{}; nvfb->fbo = nullptr; - nvfb->fb_address = vfb->fb_address; - nvfb->fb_stride = vfb->fb_stride; - nvfb->z_address = vfb->z_address; - nvfb->z_stride = vfb->z_stride; + nvfb->fb_address = channel == RASTER_COLOR ? vfb->fb_address : 0; + nvfb->fb_stride = channel == RASTER_COLOR ? vfb->fb_stride : 0; + nvfb->z_address = channel == RASTER_DEPTH ? vfb->z_address : 0; + nvfb->z_stride = channel == RASTER_DEPTH ? vfb->z_stride : 0; nvfb->width = vfb->width; nvfb->height = vfb->height; nvfb->renderWidth = vfb->bufferWidth; @@ -2111,10 +2116,10 @@ VirtualFramebuffer *FramebufferManagerCommon::FindDownloadTempBuffer(VirtualFram nvfb->drawnHeight = vfb->drawnHeight; char name[64]; - snprintf(name, sizeof(name), "download_temp"); - // TODO: We don't have a way to create a depth-only framebuffer yet. - // Also, at least on Vulkan we always create both depth and color, need to rework how we handle renderpasses. - nvfb->fbo = draw_->CreateFramebuffer({ nvfb->bufferWidth, nvfb->bufferHeight, 1, 1, 0, channel == RASTER_DEPTH ? true : false, name }); + snprintf(name, sizeof(name), "download_temp_%08x_%s", vfb->Address(channel), RasterChannelToString(channel)); + + // We always create a color-only framebuffer here - readbacks of depth convert to color while translating the values. + nvfb->fbo = draw_->CreateFramebuffer({ nvfb->bufferWidth, nvfb->bufferHeight, 1, 1, 0, false, name }); if (!nvfb->fbo) { ERROR_LOG(FRAMEBUF, "Error creating FBO! %d x %d", nvfb->renderWidth, nvfb->renderHeight); delete nvfb; @@ -2732,7 +2737,8 @@ void FramebufferManagerCommon::ReadbackFramebufferSync(VirtualFramebuffer *vfb, return; } - if (vfb->renderWidth == vfb->width && vfb->renderHeight == vfb->height) { + // Note that ReadbackDepthBufferSync can stretch on its own while converting data format, so we don't need to downscale in that case. + if (vfb->renderScaleFactor == 1 || channel == RASTER_DEPTH) { // No need to stretch-blit } else { VirtualFramebuffer *nvfb = FindDownloadTempBuffer(vfb, channel); @@ -2767,7 +2773,9 @@ void FramebufferManagerCommon::ReadbackFramebufferSync(VirtualFramebuffer *vfb, if (channel == RASTER_DEPTH) { _assert_msg_(vfb && vfb->z_address != 0 && vfb->z_stride != 0, "Depth buffer invalid"); - ReadbackDepthbufferSync(vfb->fbo, x, y, w, h, (uint16_t *)destPtr, stride, w, h); + ReadbackDepthbufferSync(vfb->fbo, + x * vfb->renderScaleFactor, y * vfb->renderScaleFactor, + w * vfb->renderScaleFactor, h * vfb->renderScaleFactor, (uint16_t *)destPtr, stride, w, h); } else { draw_->CopyFramebufferToMemorySync(vfb->fbo, channel == RASTER_COLOR ? Draw::FB_COLOR_BIT : Draw::FB_DEPTH_BIT, x, y, w, h, destFormat, destPtr, stride, "ReadbackFramebufferSync"); } diff --git a/GPU/Common/FramebufferManagerCommon.h b/GPU/Common/FramebufferManagerCommon.h index 2046300edf..da63f69de0 100644 --- a/GPU/Common/FramebufferManagerCommon.h +++ b/GPU/Common/FramebufferManagerCommon.h @@ -154,6 +154,10 @@ struct VirtualFramebuffer { inline int BufferWidthInBytes() const { return bufferWidth * BufferFormatBytesPerPixel(fb_format); } inline int FbStrideInBytes() const { return fb_stride * BufferFormatBytesPerPixel(fb_format); } inline int ZStrideInBytes() const { return z_stride * 2; } + + inline int Stride(RasterChannel channel) const { return channel == RASTER_COLOR ? fb_stride : z_stride; } + inline u32 Address(RasterChannel channel) const { return channel == RASTER_COLOR ? fb_address : z_address; } + inline int Format(RasterChannel channel) const { return channel == RASTER_COLOR ? fb_format : GE_FORMAT_DEPTH16; } }; struct FramebufferHeuristicParams { diff --git a/GPU/Common/TextureCacheCommon.cpp b/GPU/Common/TextureCacheCommon.cpp index dcbd5e4c3f..0d276ed39a 100644 --- a/GPU/Common/TextureCacheCommon.cpp +++ b/GPU/Common/TextureCacheCommon.cpp @@ -1032,7 +1032,7 @@ bool TextureCacheCommon::MatchFramebuffer( // 3rd Birthday (and a bunch of other games) render to a 16 bit clut texture. if (matchingClutFormat) { if (!noOffset) { - WARN_LOG_ONCE(subareaClut, G3D, "Matching framebuffer (%s) using %s with offset at %08x +%dx%d", channel == RASTER_DEPTH ? "DEPTH" : "COLOR", GeTextureFormatToString(entry.format), fb_address, matchInfo->xOffset, matchInfo->yOffset); + WARN_LOG_ONCE(subareaClut, G3D, "Matching framebuffer (%s) using %s with offset at %08x +%dx%d", RasterChannelToString(channel), GeTextureFormatToString(entry.format), fb_address, matchInfo->xOffset, matchInfo->yOffset); } return true; } else if (IsClutFormat((GETextureFormat)(entry.format)) || IsDXTFormat((GETextureFormat)(entry.format))) { @@ -2596,7 +2596,7 @@ void TextureCacheCommon::ClearNextFrame() { std::string AttachCandidate::ToString() const { return StringFromFormat("[%s seq:%d rel:%d C:%08x/%d(%s) Z:%08x/%d X:%d Y:%d reint: %s]", - this->channel == RASTER_COLOR ? "COLOR" : "DEPTH", + RasterChannelToString(this->channel), this->channel == RASTER_COLOR ? this->fb->colorBindSeq : this->fb->depthBindSeq, this->relevancy, this->fb->fb_address, this->fb->fb_stride, GeBufferFormatToString(this->fb->fb_format), diff --git a/GPU/GPU.cpp b/GPU/GPU.cpp index abfe5a5a0b..2a7d477204 100644 --- a/GPU/GPU.cpp +++ b/GPU/GPU.cpp @@ -136,3 +136,7 @@ void GPU_Shutdown() { delete gpu; gpu = nullptr; } + +const char *RasterChannelToString(RasterChannel channel) { + return channel == RASTER_COLOR ? "COLOR" : "DEPTH"; +} diff --git a/GPU/GPU.h b/GPU/GPU.h index d9bbed5cee..4411733728 100644 --- a/GPU/GPU.h +++ b/GPU/GPU.h @@ -147,3 +147,5 @@ bool GPU_Init(GraphicsContext *ctx, Draw::DrawContext *draw); bool GPU_IsReady(); bool GPU_IsStarted(); void GPU_Shutdown(); + +const char *RasterChannelToString(RasterChannel channel);