From 7128befb39185c051502fbdf0e71fabc9ddbb30c Mon Sep 17 00:00:00 2001 From: Scott Mansell Date: Wed, 17 Nov 2021 05:40:45 +1300 Subject: [PATCH] Fix copy filter clamping regression in Spyro This fixes horizontal lines in the bloom effect of Spyro: A Hero's Tail, which is a regression caused by PR #10204 Screenshot of regression: https://user-images.githubusercontent.com/138484/142030503-90fcd8d5-63d3-4820-874a-72e9be0c4768.png Fixed: https://user-images.githubusercontent.com/138484/142031598-b85ff55c-1302-4e4d-bcb2-57848974056b.png Spyro uses an 640x80 pixel sub-buffer within the EFB to calculate it's bloom effects, which it places below the main 640x448 buffer. EFB layout: https://user-images.githubusercontent.com/138484/142030573-e933b6ae-c37e-4be6-86d4-0bc779b92535.png Note: Colors are wrong because the main color buffer uses RGBA6, while the bloom is calculated in RGB8 This allows it to do bloom without backing up part of the EFB to main memory, as most games do. But, since some of the sub-buffers used in the bloom effect are taller than 80 pixels, they need to be sliced up into smaller sub, sub buffers which get combined later when copied to main memory. At one point, a 320x224 buffer is broken up into 320x80, 320x64 and 320x80 slices. These are copied out with the copy filter set to a vertical blur. Because there was an off-by-one errror in the clamping coordinates, the bottom line of the color buffer would be blurred into the top of each slice. Final combined EFB copy: https://user-images.githubusercontent.com/138484/142031360-2c076839-7c96-4b3b-a093-d899d0a2c7ae.png Fixed version: https://user-images.githubusercontent.com/138484/142031370-72e41a35-3b3e-4662-a483-79203e357ecc.png Before #10204 the copy filter wasn't enabled for efb copies, and most other games don't do this type of slicing. FIFO CI shows that a few other games are effected, it's always just a minor difference to the top line where there was previously a slight hint of garbage. --- Source/Core/VideoCommon/TextureCacheBase.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Source/Core/VideoCommon/TextureCacheBase.cpp b/Source/Core/VideoCommon/TextureCacheBase.cpp index 0f66284180..5153ed8dba 100644 --- a/Source/Core/VideoCommon/TextureCacheBase.cpp +++ b/Source/Core/VideoCommon/TextureCacheBase.cpp @@ -2685,7 +2685,8 @@ void TextureCacheBase::CopyEFBToCacheEntry(TCacheEntry* entry, bool is_depth_cop uniforms.filter_coefficients[2] = filter_coefficients.lower; uniforms.gamma_rcp = 1.0f / gamma; uniforms.clamp_top = clamp_top ? framebuffer_rect.top * rcp_efb_height : 0.0f; - uniforms.clamp_bottom = clamp_bottom ? framebuffer_rect.bottom * rcp_efb_height : 1.0f; + int bottom_coord = framebuffer_rect.bottom - 1; + uniforms.clamp_bottom = clamp_bottom ? bottom_coord * rcp_efb_height : 1.0f; uniforms.pixel_height = g_ActiveConfig.bCopyEFBScaled ? rcp_efb_height : 1.0f / EFB_HEIGHT; uniforms.padding = 0; g_vertex_manager->UploadUtilityUniforms(&uniforms, sizeof(uniforms)); @@ -2750,7 +2751,8 @@ void TextureCacheBase::CopyEFB(AbstractStagingTexture* dst, const EFBCopyParams& encoder_params.y_scale = y_scale; encoder_params.gamma_rcp = 1.0f / gamma; encoder_params.clamp_top = clamp_top ? framebuffer_rect.top * rcp_efb_height : 0.0f; - encoder_params.clamp_bottom = clamp_bottom ? framebuffer_rect.bottom * rcp_efb_height : 1.0f; + int bottom_coord = framebuffer_rect.bottom - 1; + encoder_params.clamp_bottom = clamp_bottom ? bottom_coord * rcp_efb_height : 1.0f; encoder_params.filter_coefficients[0] = filter_coefficients.upper; encoder_params.filter_coefficients[1] = filter_coefficients.middle; encoder_params.filter_coefficients[2] = filter_coefficients.lower;