Make SkColorToHSV and SkHSVToColor "perfect" inverses

For all possible opaque SkColors, make converting to HSV and back return
the original SkColor.

In SkHSVToColor, store values as normalized floats (instead of
converting to byte values) as long as possible.

Add a test that cycles through all opaque SkColors and verifies correct
conversion.

BUG=b/33737498

Change-Id: I7ff61a999a271565a9ffe82ae3c9676fc49d67e3
Reviewed-on: https://skia-review.googlesource.com/6720
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>

3 files changed, 51 insertions, 18 deletions

diff --git a/gn/tests.gni b/gn/tests.gni
index 65b79ab8f6..caafae456a 100644
--- a/gn/tests.gni
+++ b/gn/tests.gni
@@ -98,6 +98,7 @@ tests_sources = [
   "$_tests/GrTextureStripAtlasTest.cpp",
   "$_tests/GrTRecorderTest.cpp",
   "$_tests/HashTest.cpp",
+  "$_tests/HSVRoundTripTest.cpp",
   "$_tests/image-bitmap.cpp",
   "$_tests/ICCTest.cpp",
   "$_tests/ImageCacheTest.cpp",
diff --git a/src/core/SkColor.cpp b/src/core/SkColor.cpp
index 6dacc063e0..bee114b1e8 100644
--- a/src/core/SkColor.cpp
+++ b/src/core/SkColor.cpp
@@ -73,30 +73,32 @@ void SkRGBToHSV(U8CPU r, U8CPU g, U8CPU b, SkScalar hsv[3]) {
 SkColor SkHSVToColor(U8CPU a, const SkScalar hsv[3]) {
     SkASSERT(hsv);
 
-    U8CPU s = SkUnitScalarClampToByte(hsv[1]);
-    U8CPU v = SkUnitScalarClampToByte(hsv[2]);
+    SkScalar s = SkScalarPin(hsv[1], 0, 1);
+    SkScalar v = SkScalarPin(hsv[2], 0, 1);
 
-    if (0 == s) { // shade of gray
-        return SkColorSetARGB(a, v, v, v);
+    U8CPU v_byte = SkScalarRoundToInt(v * 255);
+
+    if (SkScalarNearlyZero(s)) { // shade of gray
+        return SkColorSetARGB(a, v_byte, v_byte, v_byte);
     }
-    SkFixed hx = (hsv[0] < 0 || hsv[0] >= SkIntToScalar(360)) ? 0 : SkScalarToFixed(hsv[0]/60);
-    SkFixed f = hx & 0xFFFF;
+    SkScalar hx = (hsv[0] < 0 || hsv[0] >= SkIntToScalar(360)) ? 0 : hsv[0]/60;
+    SkScalar w = SkScalarFloorToScalar(hx);
+    SkScalar f = hx - w;
 
-    unsigned v_scale = SkAlpha255To256(v);
-    unsigned p = SkAlphaMul(255 - s, v_scale);
-    unsigned q = SkAlphaMul(255 - (s * f >> 16), v_scale);
-    unsigned t = SkAlphaMul(255 - (s * (SK_Fixed1 - f) >> 16), v_scale);
+    unsigned p = SkScalarRoundToInt((SK_Scalar1 - s) * v * 255);
+    unsigned q = SkScalarRoundToInt((SK_Scalar1 - (s * f)) * v * 255);
+    unsigned t = SkScalarRoundToInt((SK_Scalar1 - (s * (SK_Scalar1 - f))) * v * 255);
 
     unsigned r, g, b;
 
-    SkASSERT((unsigned)(hx >> 16) < 6);
-    switch (hx >> 16) {
-        case 0: r = v; g = t; b = p; break;
-        case 1: r = q; g = v; b = p; break;
-        case 2: r = p; g = v; b = t; break;
-        case 3: r = p; g = q; b = v; break;
-        case 4: r = t;  g = p; b = v; break;
-        default: r = v; g = p; b = q; break;
+    SkASSERT((unsigned)(w) < 6);
+    switch ((unsigned)(w)) {
+        case 0: r = v_byte;  g = t;      b = p; break;
+        case 1: r = q;       g = v_byte; b = p; break;
+        case 2: r = p;       g = v_byte; b = t; break;
+        case 3: r = p;       g = q;      b = v_byte; break;
+        case 4: r = t;       g = p;      b = v_byte; break;
+        default: r = v_byte; g = p;      b = q; break;
     }
     return SkColorSetARGB(a, r, g, b);
 }
diff --git a/tests/HSVRoundTripTest.cpp b/tests/HSVRoundTripTest.cpp
new file mode 100644
index 0000000000..4d25895a9a
--- /dev/null
+++ b/tests/HSVRoundTripTest.cpp
@@ -0,0 +1,30 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "Test.h"
+
+#include "SkColor.h"
+
+DEF_TEST(ColorToHSVRoundTrip, reporter) {
+    SkScalar hsv[3];
+    for (U8CPU r = 0; r <= 255; r++) {
+        for (U8CPU g = 0; g <= 255; g++) {
+            for (U8CPU b = 0; b <= 255; b++) {
+                SkColor color = SkColorSetARGBInline(0xFF, r, g, b);
+                SkColorToHSV(color, hsv);
+                SkColor result = SkHSVToColor(0xFF, hsv);
+                if (result != color) {
+                    ERRORF(reporter, "HSV roundtrip mismatch!\n"
+                                     "\toriginal: %X\n"
+                                     "\tHSV: %f, %f, %f\n"
+                                     "\tresult: %X\n",
+                           color, hsv[0], hsv[1], hsv[2], result);
+                }
+            }
+        }
+    }
+}