Fix rounding error when convering HSL values to RGB.

Previously we were truncating the values when we should have
been rounding.

Also loosen some checks for comparing doubles.

BUG=72256

Review URL: http://codereview.chromium.org/6575016

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@75965 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 55 insertions, 20 deletions

diff --git a/ui/gfx/color_utils.cc b/ui/gfx/color_utils.cc
index 50ebe92..8177c80 100644
--- a/ui/gfx/color_utils.cc
+++ b/ui/gfx/color_utils.cc
@@ -25,20 +25,22 @@ namespace color_utils {
 
 namespace {
 
-double calcHue(double temp1, double temp2, double hue) {
+int calcHue(double temp1, double temp2, double hue) {
   if (hue < 0.0)
     ++hue;
   else if (hue > 1.0)
     --hue;
 
+  double result = temp1;
   if (hue * 6.0 < 1.0)
-    return temp1 + (temp2 - temp1) * hue * 6.0;
-  if (hue * 2.0 < 1.0)
-    return temp2;
-  if (hue * 3.0 < 2.0)
-    return temp1 + (temp2 - temp1) * (2.0 / 3.0 - hue) * 6.0;
-
-  return temp1;
+    result = temp1 + (temp2 - temp1) * hue * 6.0;
+  else if (hue * 2.0 < 1.0)
+    result = temp2;
+  else if (hue * 3.0 < 2.0)
+    result = temp1 + (temp2 - temp1) * (2.0 / 3.0 - hue) * 6.0;
+
+  // Scale the result from 0 - 255 and round off the value.
+  return static_cast<int>(result * 255 + .5);
 }
 
 int GetLumaForColor(SkColor* color) {
@@ -140,9 +142,9 @@ SkColor HSLToSkColor(const HSL& hsl, SkAlpha alpha) {
       (lightness + saturation - (lightness * saturation));
   double temp1 = 2.0 * lightness - temp2;
   return SkColorSetARGB(alpha,
-      static_cast<int>(calcHue(temp1, temp2, hue + 1.0 / 3.0) * 255),
-      static_cast<int>(calcHue(temp1, temp2, hue) * 255),
-      static_cast<int>(calcHue(temp1, temp2, hue - 1.0 / 3.0) * 255));
+      calcHue(temp1, temp2, hue + 1.0 / 3.0),
+      calcHue(temp1, temp2, hue),
+      calcHue(temp1, temp2, hue - 1.0 / 3.0));
 }
 
 SkColor HSLShift(SkColor color, const HSL& shift) {
diff --git a/ui/gfx/color_utils_unittest.cc b/ui/gfx/color_utils_unittest.cc
index 83a6ce63..59eaeba 100644
--- a/ui/gfx/color_utils_unittest.cc
+++ b/ui/gfx/color_utils_unittest.cc
@@ -12,16 +12,16 @@
 TEST(ColorUtils, SkColorToHSLRed) {
   color_utils::HSL hsl = { 0, 0, 0 };
   color_utils::SkColorToHSL(SK_ColorRED, &hsl);
-  EXPECT_EQ(hsl.h, 0);
-  EXPECT_EQ(hsl.s, 1);
-  EXPECT_EQ(hsl.l, 0.5);
+  EXPECT_DOUBLE_EQ(hsl.h, 0);
+  EXPECT_DOUBLE_EQ(hsl.s, 1);
+  EXPECT_DOUBLE_EQ(hsl.l, 0.5);
 }
 
 TEST(ColorUtils, SkColorToHSLGrey) {
   color_utils::HSL hsl = { 0, 0, 0 };
   color_utils::SkColorToHSL(SkColorSetARGB(255, 128, 128, 128), &hsl);
-  EXPECT_EQ(hsl.h, 0);
-  EXPECT_EQ(hsl.s, 0);
+  EXPECT_DOUBLE_EQ(hsl.h, 0);
+  EXPECT_DOUBLE_EQ(hsl.s, 0);
   EXPECT_EQ(static_cast<int>(hsl.l * 100),
             static_cast<int>(0.5 * 100));  // Accurate to two decimal places.
 }
@@ -36,16 +36,49 @@ TEST(ColorUtils, HSLToSkColorWithAlpha) {
   EXPECT_EQ(SkColorGetB(red), SkColorGetB(result));
 }
 
+
+TEST(ColorUtils, RGBtoHSLRoundTrip) {
+  // Just spot check values near the edges.
+  for (int r = 0; r < 10; ++r) {
+    for (int g = 0; g < 10; ++g) {
+      for (int b = 0; b < 10; ++b) {
+        SkColor rgb = SkColorSetARGB(255, r, g, b);
+        color_utils::HSL hsl = { 0, 0, 0 };
+        color_utils::SkColorToHSL(rgb, &hsl);
+        SkColor out = color_utils::HSLToSkColor(hsl, 255);
+        EXPECT_EQ(SkColorGetR(out), SkColorGetR(rgb));
+        EXPECT_EQ(SkColorGetG(out), SkColorGetG(rgb));
+        EXPECT_EQ(SkColorGetB(out), SkColorGetB(rgb));
+      }
+    }
+  }
+  for (int r = 240; r < 256; ++r) {
+    for (int g = 240; g < 256; ++g) {
+      for (int b = 240; b < 256; ++b) {
+        SkColor rgb = SkColorSetARGB(255, r, g, b);
+        color_utils::HSL hsl = { 0, 0, 0 };
+        color_utils::SkColorToHSL(rgb, &hsl);
+        SkColor out = color_utils::HSLToSkColor(hsl, 255);
+        EXPECT_EQ(SkColorGetR(out), SkColorGetR(rgb));
+        EXPECT_EQ(SkColorGetG(out), SkColorGetG(rgb));
+        EXPECT_EQ(SkColorGetB(out), SkColorGetB(rgb));
+      }
+    }
+  }
+}
+
 TEST(ColorUtils, ColorToHSLRegisterSpill) {
   // In a opt build on Linux, this was causing a register spill on my laptop
   // (Pentium M) when converting from SkColor to HSL.
   SkColor input = SkColorSetARGB(255, 206, 154, 89);
   color_utils::HSL hsl = { -1, -1, -1 };
   SkColor result = color_utils::HSLShift(input, hsl);
-  EXPECT_EQ(255U, SkColorGetA(result));
-  EXPECT_EQ(206U, SkColorGetR(result));
-  EXPECT_EQ(153U, SkColorGetG(result));
-  EXPECT_EQ(88U, SkColorGetB(result));
+  // |result| should be the same as |input| since we passed in a value meaning
+  // no color shift.
+  EXPECT_EQ(SkColorGetA(input), SkColorGetA(result));
+  EXPECT_EQ(SkColorGetR(input), SkColorGetR(result));
+  EXPECT_EQ(SkColorGetG(input), SkColorGetG(result));
+  EXPECT_EQ(SkColorGetB(input), SkColorGetB(result));
 }
 
 TEST(ColorUtils, AlphaBlend) {