From 50696c5b1fbb405170d8d090e5e2b83ee8605c04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 11 Oct 2026 14:50:19 +0200 Subject: [PATCH] Remove the "game uses sequential copies" readback heuristic After more than three partial readbacks of one buffer in a frame, ReadFramebufferToMemory read the whole buffer instead for the rest of the session, writing rows the game never asked for. Everybody's Golf 2 reads back four portraits' color and depth per frame, which turned it on. The whole 480x273 depth buffer then got written to VRAM, and its last row is where the game keeps its texture heap. A texture header was overwritten: the portrait showed the scene behind it, and the next time the portraits were built the game crashed writing through a null pointer. It was added for speed in Grand Knights History (c42d04829f), which copies many slivers of a framebuffer, so that game may now do more readbacks. Co-Authored-By: Claude Opus 5.5 (1M context) --- GPU/Common/FramebufferManagerCommon.cpp | 25 +------------------------ GPU/Common/FramebufferManagerCommon.h | 4 +--- 2 files changed, 2 insertions(+), 27 deletions(-) diff --git a/GPU/Common/FramebufferManagerCommon.cpp b/GPU/Common/FramebufferManagerCommon.cpp index 6764b6effc..1c36468cb4 100644 --- a/GPU/Common/FramebufferManagerCommon.cpp +++ b/GPU/Common/FramebufferManagerCommon.cpp @@ -3445,34 +3445,11 @@ void FramebufferManagerCommon::ReadFramebufferToMemory(VirtualFramebuffer *vfb, if (x + w >= vfb->bufferWidth) { w = vfb->bufferWidth - x; } - if (gameUsesSequentialCopies_) { - // Ignore the x/y/etc., read the entire thing. See below. - x = 0; - y = 0; - w = vfb->width; - h = vfb->height; - vfb->memoryUpdated = true; - vfb->usageFlags |= FB_USAGE_DOWNLOAD; - } else if (x == 0 && y == 0 && w == vfb->width && h == vfb->height) { + if (x == 0 && y == 0 && w == vfb->width && h == vfb->height) { // Mark it as fully downloaded until next render to it. if (channel == RASTER_COLOR) vfb->memoryUpdated = true; vfb->usageFlags |= FB_USAGE_DOWNLOAD; - } else { - // Let's try to set the flag eventually, if the game copies a lot. - // Some games (like Grand Knights History) copy subranges very frequently. - const static int FREQUENT_SEQUENTIAL_COPIES = 3; - static int frameLastCopy = 0; - static u32 bufferLastCopy = 0; - static int copiesThisFrame = 0; - if (frameLastCopy != gpuStats.totals.numFlips || bufferLastCopy != vfb->fb_address) { - frameLastCopy = gpuStats.totals.numFlips; - bufferLastCopy = vfb->fb_address; - copiesThisFrame = 0; - } - if (++copiesThisFrame > FREQUENT_SEQUENTIAL_COPIES) { - gameUsesSequentialCopies_ = true; - } } // This handles any required stretching internally. diff --git a/GPU/Common/FramebufferManagerCommon.h b/GPU/Common/FramebufferManagerCommon.h index 3f09413277..33ea28f292 100644 --- a/GPU/Common/FramebufferManagerCommon.h +++ b/GPU/Common/FramebufferManagerCommon.h @@ -127,7 +127,7 @@ struct VirtualFramebuffer { // Tracking for downloads-to-CLUT. u16 clutUpdatedBytes; - // Means that the whole image has already been read back to memory - used when combining small readbacks (gameUsesSequentialCopies_). + // Means that the whole image has already been read back to memory. bool memoryUpdated; // TODO: Fold into usageFlags? @@ -630,8 +630,6 @@ protected: std::vector drawPixelsCache_; - bool gameUsesSequentialCopies_ = false; - // Sampled in BeginFrame/UpdateSize for safety. float renderWidth_ = 0.0f; float renderHeight_ = 0.0f;