color_utils cleanup:

ui/gfx/color_utils.cc

Index: ui/gfx/color_utils.cc
diff --git a/ui/gfx/color_utils.cc b/ui/gfx/color_utils.cc
index 2e3c96f86dc674824e2e8d998be1aae58831e951..1366697b80617bd12f175e6a522fe4befe558323 100644
--- a/ui/gfx/color_utils.cc
+++ b/ui/gfx/color_utils.cc
@@ -42,35 +42,21 @@ int calcHue(double temp1, double temp2, double hue) {
   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);
+  return static_cast<int>(std::round(result * 255));
 }
 
-// Next two functions' formulas from:
-// http://www.w3.org/TR/WCAG20/#relativeluminancedef
-// http://www.w3.org/TR/WCAG20/#contrast-ratiodef
-
-double ConvertSRGB(double eight_bit_component) {
+// Assumes sRGB.
+double Linearize(double eight_bit_component) {
   const double component = eight_bit_component / 255.0;
   return (component <= 0.03928) ?
       (component / 12.92) : pow((component + 0.055) / 1.055, 2.4);
 }
 
-SkColor LumaInvertColor(SkColor color) {
+SkColor LightnessInvertColor(SkColor color) {
   HSL hsl;
   SkColorToHSL(color, &hsl);
   hsl.l = 1.0 - hsl.l;
-  return HSLToSkColor(hsl, 255);
-}
-
-double ContrastRatio(double foreground_luminance, double background_luminance) {
-  DCHECK_GE(foreground_luminance, 0.0);
-  DCHECK_GE(background_luminance, 0.0);
-  foreground_luminance += 0.05;
-  background_luminance += 0.05;
-  return (foreground_luminance > background_luminance) ?
-      (foreground_luminance / background_luminance) :
-      (background_luminance / foreground_luminance);
+  return HSLToSkColor(hsl, SkColorGetA(color));

[danakj, 2016/03/04 01:54:49]

This is a behaviour change, can this be a separate CL? That'll make bugtracking
easier if it shows up.

[Peter Kasting, 2016/03/04 02:10:52]

It's a behavior change, but only for GetReadableColor(), which to my knowledge
isn't currently called with alpha != 255 (and if somehow it is, this should be
more correct).

[danakj, 2016/03/04 19:05:55]

Ah, ok then.

 }
 
 }  // namespace
@@ -78,21 +64,29 @@ double ContrastRatio(double foreground_luminance, double background_luminance) {
 
 // ----------------------------------------------------------------------------
 
-double GetContrastRatio(SkColor color_a, SkColor color_b) {
+double ContrastRatio(SkColor color_a, SkColor color_b) {
   return ContrastRatio(RelativeLuminance(color_a), RelativeLuminance(color_b));
 }
 
-unsigned char GetLuminanceForColor(SkColor color) {
-  return base::saturated_cast<unsigned char>(
-      (0.3 * SkColorGetR(color)) +
-      (0.59 * SkColorGetG(color)) +
-      (0.11 * SkColorGetB(color)));
+double ContrastRatio(double luminance_a, double luminance_b) {
+  DCHECK_GE(luminance_a, 0.0);
+  DCHECK_GE(luminance_b, 0.0);
+  luminance_a += 0.05;
+  luminance_b += 0.05;
+  return (luminance_a > luminance_b) ? (luminance_a / luminance_b)
+                                     : (luminance_b / luminance_a);
 }
 
 double RelativeLuminance(SkColor color) {
-  return (0.2126 * ConvertSRGB(SkColorGetR(color))) +
-         (0.7152 * ConvertSRGB(SkColorGetG(color))) +
-         (0.0722 * ConvertSRGB(SkColorGetB(color)));
+  return (0.2126 * Linearize(SkColorGetR(color))) +
+         (0.7152 * Linearize(SkColorGetG(color))) +
+         (0.0722 * Linearize(SkColorGetB(color)));
+}
+
+uint8_t Luma(SkColor color) {
+  return static_cast<uint8_t>(std::round((0.299 * SkColorGetR(color)) +

[danakj, 2016/03/04 01:54:49]

These constants are diff too, similar request for separate CL

[Peter Kasting, 2016/03/04 02:10:52]

It's true, this is a behavior change.  That said, it's extremely minor; it's
hard to imagine people noticing any significant effect of this.

[danakj, 2016/03/04 19:05:55]

Yeh, it's just good practice IMO. Like maybe they require rebaselines, or
something. You never know what weird effect something can have and burying it in
a big CL of refactoring is usually not a good plan.

[Peter Kasting, 2016/03/04 20:28:57]

I agree in general principle.

In this specific case, all of the places calling this are doing the same check:
"is this light, or dark?" (and then usually drawing black or white atop it). 
The fallout from this is that we might toggle our behavior for colors that
happen to fall almost exactly on the midpoint (because their computed luma value
may change by at most 1).  In such cases, picking either light or dark is about
equally good, so I feel comfortable enough letting this change be part of this
CL.

+                                         (0.587 * SkColorGetG(color)) +
+                                         (0.114 * SkColorGetB(color))));
 }
 
 void SkColorToHSL(SkColor c, HSL* hsl) {
@@ -242,7 +236,7 @@ void BuildLumaHistogram(const SkBitmap& bitmap, int histogram[256]) {
   int pixel_height = bitmap.height();
   for (int y = 0; y < pixel_height; ++y) {
     for (int x = 0; x < pixel_width; ++x)
-      ++histogram[GetLuminanceForColor(bitmap.getColor(x, y))];
+      ++histogram[Luma(bitmap.getColor(x, y))];
   }
 }
 
@@ -287,20 +281,26 @@ SkColor AlphaBlend(SkColor foreground, SkColor background, SkAlpha alpha) {
 }
 
 bool IsDark(SkColor color) {
-  return GetLuminanceForColor(color) < 128;
+  return Luma(color) < 128;
 }
 
-SkColor BlendTowardOppositeLuminance(SkColor color, SkAlpha alpha) {
+SkColor BlendTowardOppositeLuma(SkColor color, SkAlpha alpha) {
   return AlphaBlend(IsDark(color) ? SK_ColorWHITE : SK_ColorBLACK, color,
                     alpha);
 }
 
 SkColor GetReadableColor(SkColor foreground, SkColor background) {
-  const SkColor foreground2 = LumaInvertColor(foreground);
+  return PickContrastingColor(foreground, LightnessInvertColor(foreground),
+                              background);
+}
+
+SkColor PickContrastingColor(SkColor foreground1,
+                             SkColor foreground2,
+                             SkColor background) {
   const double background_luminance = RelativeLuminance(background);
-  return (ContrastRatio(RelativeLuminance(foreground), background_luminance) >=
+  return (ContrastRatio(RelativeLuminance(foreground1), background_luminance) >=
           ContrastRatio(RelativeLuminance(foreground2), background_luminance)) ?
-      foreground : foreground2;
+      foreground1 : foreground2;
 }
 
 SkColor InvertColor(SkColor color) {