From b8695a57da067bc1776c8310bb7cd68f5706d320 Mon Sep 17 00:00:00 2001 From: Scott Mansell Date: Tue, 15 Jul 2014 00:42:33 +1200 Subject: [PATCH] Fix incorrect clamping in SWRenderer. A previous PR changed a whole lot of min/maxes to std::min/std::max but made a mistake here and used a templated min which cast it's arguments to unsigned instead of casting return value. This resulted in glitchy artifacts in bright areas (See issue 7439) I rewrote the code to use a proper clamping function so it's cleaner to read. --- Source/Core/Common/MathUtil.h | 8 ++++++++ .../VideoBackends/Software/SWRenderer.cpp | 12 +++++------ Source/UnitTests/Common/MathUtilTest.cpp | 20 ++++++------------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/Source/Core/Common/MathUtil.h b/Source/Core/Common/MathUtil.h index cec15f391e..db35cbedcf 100644 --- a/Source/Core/Common/MathUtil.h +++ b/Source/Core/Common/MathUtil.h @@ -20,6 +20,14 @@ inline void Clamp(T* val, const T& min, const T& max) *val = max; } +template +inline T Clamp(const T val, const T& min, const T& max) +{ + T ret = val; + Clamp(&ret, min, max); + return ret; +} + // The most significant bit of the fraction is an is-quiet bit on all architectures we care about. static const u64 DOUBLE_SIGN = 0x8000000000000000ULL, diff --git a/Source/Core/VideoBackends/Software/SWRenderer.cpp b/Source/Core/VideoBackends/Software/SWRenderer.cpp index 40e89dd3d1..875f9bd7ee 100644 --- a/Source/Core/VideoBackends/Software/SWRenderer.cpp +++ b/Source/Core/VideoBackends/Software/SWRenderer.cpp @@ -182,14 +182,14 @@ void SWRenderer::UpdateColorTexture(EfbInterface::yuv422_packed *xfb, u32 fbWidt // We do the inverse BT.601 conversion for YCbCr to RGB // http://www.equasys.de/colorconversion.html#YCbCr-RGBColorFormatConversion - TexturePointer[offset++] = std::min(255.0f, std::max(0.0f, 1.164f * Y1 + 1.596f * V)); - TexturePointer[offset++] = std::min(255.0f, std::max(0.0f, 1.164f * Y1 - 0.392f * U - 0.813f * V)); - TexturePointer[offset++] = std::min(255.0f, std::max(0.0f, 1.164f * Y1 + 2.017f * U )); + TexturePointer[offset++] = MathUtil::Clamp(int(1.164f * Y1 + 1.596f * V), 0, 255); + TexturePointer[offset++] = MathUtil::Clamp(int(1.164f * Y1 - 0.392f * U - 0.813f * V), 0, 255); + TexturePointer[offset++] = MathUtil::Clamp(int(1.164f * Y1 + 2.017f * U ), 0, 255); TexturePointer[offset++] = 255; - TexturePointer[offset++] = std::min(255.0f, std::max(0.0f, 1.164f * Y2 + 1.596f * V)); - TexturePointer[offset++] = std::min(255.0f, std::max(0.0f, 1.164f * Y2 - 0.392f * U - 0.813f * V)); - TexturePointer[offset++] = std::min(255.0f, std::max(0.0f, 1.164f * Y2 + 2.017f * U )); + TexturePointer[offset++] = MathUtil::Clamp(int(1.164f * Y2 + 1.596f * V), 0, 255); + TexturePointer[offset++] = MathUtil::Clamp(int(1.164f * Y2 - 0.392f * U - 0.813f * V), 0, 255); + TexturePointer[offset++] = MathUtil::Clamp(int(1.164f * Y2 + 2.017f * U ), 0, 255); TexturePointer[offset++] = 255; } xfb += fbWidth; diff --git a/Source/UnitTests/Common/MathUtilTest.cpp b/Source/UnitTests/Common/MathUtilTest.cpp index d2056df3f9..3df22847be 100644 --- a/Source/UnitTests/Common/MathUtilTest.cpp +++ b/Source/UnitTests/Common/MathUtilTest.cpp @@ -8,24 +8,16 @@ #include "Common/MathUtil.h" -template -T ClampAndReturn(const T& val, const T& min, const T& max) -{ - T ret = val; - MathUtil::Clamp(&ret, min, max); - return ret; -} - TEST(MathUtil, Clamp) { - EXPECT_EQ(1, ClampAndReturn(1, 0, 2)); - EXPECT_EQ(1.0, ClampAndReturn(1.0, 0.0, 2.0)); + EXPECT_EQ(1, MathUtil::Clamp(1, 0, 2)); + EXPECT_EQ(1.0, MathUtil::Clamp(1.0, 0.0, 2.0)); - EXPECT_EQ(2, ClampAndReturn(4, 0, 2)); - EXPECT_EQ(2.0, ClampAndReturn(4.0, 0.0, 2.0)); + EXPECT_EQ(2, MathUtil::Clamp(4, 0, 2)); + EXPECT_EQ(2.0, MathUtil::Clamp(4.0, 0.0, 2.0)); - EXPECT_EQ(0, ClampAndReturn(-1, 0, 2)); - EXPECT_EQ(0.0, ClampAndReturn(-1.0, 0.0, 2.0)); + EXPECT_EQ(0, MathUtil::Clamp(-1, 0, 2)); + EXPECT_EQ(0.0, MathUtil::Clamp(-1.0, 0.0, 2.0)); } TEST(MathUtil, IsINF)