color_utils cleanup:

ui/gfx/color_utils.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/gfx/color_utils.h"
 
 #include <stdint.h>
 
 #include <algorithm>
 #include <cmath>
@@ skipping 24 matching lines @@
     --hue;
 
   double result = temp1;
   if (hue * 6.0 < 1.0)
     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);
+  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
 
 
 // ----------------------------------------------------------------------------
 
-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) {
   double r = static_cast<double>(SkColorGetR(c)) / 255.0;
   double g = static_cast<double>(SkColorGetG(c)) / 255.0;
   double b = static_cast<double>(SkColorGetB(c)) / 255.0;
   double vmax = std::max(std::max(r, g), b);
   double vmin = std::min(std::min(r, g), b);
   double delta = vmax - vmin;
   hsl->l = (vmax + vmin) / 2;
@@ skipping 129 matching lines @@
 
 void BuildLumaHistogram(const SkBitmap& bitmap, int histogram[256]) {
   DCHECK_EQ(kN32_SkColorType, bitmap.colorType());
 
   SkAutoLockPixels bitmap_lock(bitmap);
 
   int pixel_width = bitmap.width();
   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))];
   }
 }
 
 double CalculateBoringScore(const SkBitmap& bitmap) {
   if (bitmap.isNull() || bitmap.empty())
     return 1.0;
   int histogram[256] = {0};
   BuildLumaHistogram(bitmap, histogram);
 
   int color_count = *std::max_element(histogram, histogram + 256);
@@ skipping 24 matching lines @@
   double b = (SkColorGetB(foreground) * f_weight +
               SkColorGetB(background) * b_weight) / 255.0;
 
   return SkColorSetARGB(static_cast<int>(std::round(normalizer)),
                         static_cast<int>(std::round(r)),
                         static_cast<int>(std::round(g)),
                         static_cast<int>(std::round(b)));
 }
 
 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) {
   return SkColorSetARGB(SkColorGetA(color), 255 - SkColorGetR(color),
                         255 - SkColorGetG(color), 255 - SkColorGetB(color));
 }
 
 SkColor GetSysSkColor(int which) {
 #if defined(OS_WIN)
   return skia::COLORREFToSkColor(GetSysColor(which));
@@ skipping 16 matching lines @@
   if (IsDark(text_color)) {
     // For black text, this comes out to kChromeIconGrey.
     return color_utils::AlphaBlend(SK_ColorWHITE, text_color,
                                    SkColorGetR(gfx::kChromeIconGrey));
   }
   // The dimming is less dramatic when darkening a light color.
   return color_utils::AlphaBlend(SK_ColorBLACK, text_color, 0x33);
 }
 
 }  // namespace color_utils