Merge pull request #13912 from jordan-woyak/simplify-saturating-cast

MathUtil: Simplify SaturatingCast implementation and fix edge case.

2 files changed, 16 insertions, 24 deletions

diff --git a/Source/Core/Common/MathUtil.h b/Source/Core/Common/MathUtil.h
index 1494a894d9..47440f2239 100644
--- a/Source/Core/Common/MathUtil.h
+++ b/Source/Core/Common/MathUtil.h
@@ -6,8 +6,10 @@
 #include <algorithm>
 #include <bit>
 #include <cmath>
+#include <concepts>
 #include <limits>
 #include <type_traits>
+#include <utility>
 
 #include "Common/CommonTypes.h"
 
@@ -31,41 +33,24 @@ constexpr auto Lerp(const T& x, const T& y, const F& a) -> decltype(x + (y - x)
 
 // Casts the specified value to a Dest. The value will be clamped to fit in the destination type.
 // Warning: The result of SaturatingCast(NaN) is undefined.
-template <typename Dest, typename T>
+template <std::integral Dest, typename T>
 constexpr Dest SaturatingCast(T value)
 {
-  static_assert(std::is_integral<Dest>());
-
-  [[maybe_unused]] constexpr Dest lo = std::numeric_limits<Dest>::lowest();
+  constexpr Dest lo = std::numeric_limits<Dest>::min();
   constexpr Dest hi = std::numeric_limits<Dest>::max();
 
-  // T being a signed integer and Dest unsigned is a problematic case because the value will
-  // be converted into an unsigned integer, and u32(...) < 0 is always false.
-  if constexpr (std::is_integral<T>() && std::is_signed<T>() && std::is_unsigned<Dest>())
+  if constexpr (std::is_integral_v<T>)
   {
-    static_assert(lo == 0);
-    if (value < 0)
+    if (std::cmp_less(value, lo))
       return lo;
-    // Now that we got rid of negative values, we can safely cast value to an unsigned T
-    // since unsigned T can represent any positive value signed T could represent.
-    // The compiler will then promote the LHS or the RHS if necessary.
-    if (std::make_unsigned_t<T>(value) > hi)
-      return hi;
-  }
-  else if constexpr (std::is_integral<T>() && std::is_unsigned<T>() && std::is_signed<Dest>())
-  {
-    // value and hi will never be negative, and hi is representable as an unsigned Dest.
-    if (value > std::make_unsigned_t<Dest>(hi))
+    if (std::cmp_greater(value, hi))
       return hi;
   }
   else
   {
-    // Do not use std::clamp or a similar function here to avoid overflow.
-    // For example, if Dest = s64 and T = int, we want integer promotion to convert value to a s64
-    // instead of changing lo or hi into an int.
-    if (value < lo)
+    if (value < static_cast<T>(lo))
       return lo;
-    if (value > hi)
+    if (value >= static_cast<T>(hi))
       return hi;
   }
   return static_cast<Dest>(value);
diff --git a/Source/UnitTests/Common/MathUtilTest.cpp b/Source/UnitTests/Common/MathUtilTest.cpp
index 966f3a4445..c60c19e6a1 100644
--- a/Source/UnitTests/Common/MathUtilTest.cpp
+++ b/Source/UnitTests/Common/MathUtilTest.cpp
@@ -66,6 +66,13 @@ TEST(MathUtil, SaturatingCast)
   // 16777217 = 2^24 + 1 is the first integer that cannot be represented correctly with a f32.
   EXPECT_EQ(16777216, MathUtil::SaturatingCast<s32>(float(16777216)));
   EXPECT_EQ(16777216, MathUtil::SaturatingCast<s32>(float(16777217)));
+
+  // Note that values in the range [2147483584, 2147483776] have an equivalent float representation.
+  EXPECT_EQ(std::numeric_limits<s32>::max(), MathUtil::SaturatingCast<s32>(2147483648.f));
+  EXPECT_EQ(std::numeric_limits<s32>::min(), MathUtil::SaturatingCast<s32>(-2147483649.f));
+
+  // Cast from a signed integer type to a smaller signed integer type
+  EXPECT_EQ(-128, (MathUtil::SaturatingCast<s8, int>(-129)));
 }
 
 TEST(MathUtil, RectangleEquality)