From b267f54259c7e1b9d9bd8a5874af559b76925184 Mon Sep 17 00:00:00 2001 From: Techjar Date: Thu, 10 Jun 2021 08:44:56 -0400 Subject: [PATCH] VideoCommon: Only include centered pixels in bounding box At higher resolutions, our bounding box dimensions end up being slightly larger than original hardware in some cases. This is not necessarily wrong, it's just an artifact of rendering at a higher resolution, due to bringing out detail that wouldn't have appeared on original hardware. It causes a texel to fall partially on what would have been a single pixel at native resolution, resulting in the coordinates getting bumped up to the next valid value. In many cases, these slightly larger bounding boxes are perfectly fine, as games don't hard-code expected dimensions. It is problematic in Paper Mario TTYD though, for a somewhat complicated reason. Paper Mario TTYD frequently uses EFB copies to pre-render a bunch of animation frames for a character sprite (especially in Chapter 2), so that it can then render 100 or more of them without bringing the GameCube to its knees. Based on my observation, the game seems to set aside a region of memory to store these EFB copies. This region is obviously fairly small, as the GameCube only has 24MB of RAM. There are 2 rooms in Chapter 2 where you fight a horde of as many as 100 Jabbies, which are also rendered using EFB copies, so in this room the game ends up making 130(!) EFB copies just for Puni and Jabbi sprites. This seems to nearly fill the region of memory it set aside for them. Unfortunately, our slightly larger bounding boxes at higher resolutions results in overflowing this memory, causing very strange behavior. Some EFB copies partially overlap game state, resulting in reading it as a garbage RGB5A3 texture that constantly changes. Others apparently somehow trigger a corner case in our persistent buffer mapping, causing them to partially overwrite earlier EFB copies. What this change does is only include the screen coordinates that align with the equivalent native resolution pixel centers, which generally results in the bounding boxes being more in line with original hardware. It isn't perfect, but it's enough to fix Paper Mario TTYD's Jabbi rooms by avoiding the buffer overflow. Notably, it is more accurate at odd resolutions than at even resolutions. Native resolution is completely unaffected by this change, as should be the case. This change may also have a small positive impact on shader performance at higher resolutions, as there will be less atomic operations performed. --- Source/Core/VideoCommon/PixelShaderGen.cpp | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/Source/Core/VideoCommon/PixelShaderGen.cpp b/Source/Core/VideoCommon/PixelShaderGen.cpp index 37e856ae42..32c80be7bb 100644 --- a/Source/Core/VideoCommon/PixelShaderGen.cpp +++ b/Source/Core/VideoCommon/PixelShaderGen.cpp @@ -495,9 +495,18 @@ void UpdateBoundingBoxBuffer(int2 min_pos, int2 max_pos) {{ }} void UpdateBoundingBox(float2 rawpos) {{ + // We only want to include coordinates for pixels aligned with the native resolution pixel centers. + // This makes bounding box sizes more accurate (though not perfect) at higher resolutions, + // avoiding EFB copy buffer overflow in affected games. + // + // For a more detailed explanation, see https://dolp.in/pr9801 + int2 int_efb_scale = iround(1 / {efb_scale}.xy); + if (any(int2(rawpos) % int_efb_scale != int_efb_scale >> 1)) // divide by two + return; + // The rightmost shaded pixel is not included in the right bounding box register, // such that width = right - left + 1. This has been verified on hardware. - int2 pos = int2(rawpos * cefbscale); + int2 pos = int2(rawpos * {efb_scale}.xy); #ifdef API_OPENGL // We need to invert the Y coordinate due to OpenGL's lower-left origin @@ -506,8 +515,8 @@ void UpdateBoundingBox(float2 rawpos) {{ // The GC/Wii GPU rasterizes in 2x2 pixel groups, so bounding box values will be rounded to the // extents of these groups, rather than the exact pixel. - int2 pos_tl = pos & ~1; - int2 pos_br = pos | 1; + int2 pos_tl = pos & ~1; // round down to even + int2 pos_br = pos | 1; // round up to odd #ifdef SUPPORTS_SUBGROUP_REDUCTION if (CAN_USE_SUBGROUP_REDUCTION) {{ @@ -526,7 +535,7 @@ void UpdateBoundingBox(float2 rawpos) {{ }} )", - fmt::arg("efb_height", EFB_HEIGHT)); + fmt::arg("efb_height", EFB_HEIGHT), fmt::arg("efb_scale", I_EFBSCALE)); } }