commit a0c6731b121abe248565e31e2ecd556d65a44336
author Mike Klein <mtklein@google.com> Fri Oct 16 09:49:07 2020 -0500
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Fri Oct 16 15:50:32 2020 +0000
tree fa92eb6c7ec01be195258bc85e9e62ba98f95f86
parent 7db7139f40321161d3db4f2c4c7b478855d1e237

update SkTPin()

 - Reword the comment to explain how it differs from std::clamp().
 - Document and test an alternative form using std::{min,max}.

Change-Id: I3a97b98a15303478a5a7ff8d0536829f6d5f1586
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/327696
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>

include/core/SkTypes.h

diff --git a/include/core/SkTypes.h b/include/core/SkTypes.h
index 9f7c1d7..34938b9 100644
--- a/include/core/SkTypes.h
+++ b/include/core/SkTypes.h
@@ -581,13 +581,21 @@
     return value;
 }
 
-/** @return value pinned (clamped) between min and max, inclusively.
+/** @return x pinned (clamped) between lo and hi, inclusively.
 
-    NOTE: Unlike std::clamp, SkTPin has well-defined behavior if 'value' is a
-          floating point NaN. In that case, 'max' is returned.
+    Unlike std::clamp(), SkTPin() always returns a value between lo and hi.
+    If x is NaN, SkTPin() returns hi but std::clamp() returns NaN.
 */
-template <typename T> static constexpr const T& SkTPin(const T& value, const T& min, const T& max) {
-    return value < min ? min : (value < max ? value : max);
+template <typename T>
+static constexpr const T& SkTPin(const T& x, const T& lo, const T& hi) {
+#if 0
+    return std::max(std::min(hi, x), lo);
+#else
+    // To avoid the need to #include <algorithm>, we use this equivalent logic:
+    return x < lo ? lo
+        :  x < hi ? x
+        : /*else*/  hi;
+#endif
 }
 
 ////////////////////////////////////////////////////////////////////////////////

tests/MathTest.cpp

diff --git a/tests/MathTest.cpp b/tests/MathTest.cpp
index 8f68b92..64aff81 100644
--- a/tests/MathTest.cpp
+++ b/tests/MathTest.cpp
@@ -16,6 +16,7 @@
 #include "src/core/SkMathPriv.h"
 #include "tests/Test.h"
 
+#include <algorithm>
 #include <cinttypes>
 
 static void test_clz(skiatest::Reporter* reporter) {
@@ -685,6 +686,13 @@
         // Ensure that SkTPin bounds even non-finite values (including NaN)
         SkScalar p = SkTPin<SkScalar>(r.fFloat, 0, 100);
         REPORTER_ASSERT(reporter, p >= 0 && p <= 100);
+
+        // SkTPin has a logical equivalent in terms of std::min and std::max that we don't use,
+        // I think only because we'd like to avoid pulling <algorithm> into SkTypes.h.
+        auto equiv = [](float x, float lo, float hi) {
+            return std::max(std::min(hi, x), lo);
+        };
+        REPORTER_ASSERT(reporter, p == equiv(r.fFloat, 0, 100));
     }
 }