Type conversion fixes, ui/gfx/ edition.

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 <math.h>
 #if defined(OS_WIN)
 #include <windows.h>
 #endif
 
 #include <algorithm>
 
 #include "base/basictypes.h"
 #include "base/logging.h"
+#include "base/numerics/safe_conversions.h"
 #include "build/build_config.h"
 #if defined(OS_WIN)
 #include "skia/ext/skia_utils_win.h"
 #endif
 #include "third_party/skia/include/core/SkBitmap.h"
 
 namespace color_utils {
 
 
 // Helper functions -----------------------------------------------------------
@@ skipping 44 matching lines @@
       (foreground_luminance / background_luminance) :
       (background_luminance / foreground_luminance);
 }
 
 }  // namespace
 
 
 // ----------------------------------------------------------------------------
 
 unsigned char GetLuminanceForColor(SkColor color) {
-  int luma = static_cast<int>((0.3 * SkColorGetR(color)) +
-                              (0.59 * SkColorGetG(color)) +
-                              (0.11 * SkColorGetB(color)));
-  return std::max(std::min(luma, 255), 0);
+  return base::saturated_cast<unsigned char>(

[danakj, 2014/10/18 18:40:06]

nice change here

+      (0.3 * SkColorGetR(color)) +
+      (0.59 * SkColorGetG(color)) +
+      (0.11 * SkColorGetB(color)));
 }
 
 double RelativeLuminance(SkColor color) {
   return (0.2126 * ConvertSRGB(SkColorGetR(color))) +
          (0.7152 * ConvertSRGB(SkColorGetG(color))) +
          (0.0722 * ConvertSRGB(SkColorGetB(color)));
 }
 
 void SkColorToHSL(SkColor c, HSL* hsl) {
   double r = static_cast<double>(SkColorGetR(c)) / 255.0;
@@ skipping 35 matching lines @@
   // If there's no color, we don't care about hue and can do everything based on
   // brightness.
   if (!saturation) {
     uint8 light;
 
     if (lightness < 0)
       light = 0;
     else if (lightness >= 1.0)
       light = 255;
     else
-      light = SkDoubleToFixed(lightness) >> 8;
+      light = static_cast<uint8>(SkDoubleToFixed(lightness) >> 8);
 
     return SkColorSetARGB(alpha, light, light, light);
   }
 
   double temp2 = (lightness < 0.5) ?
       (lightness * (1.0 + saturation)) :
       (lightness + saturation - (lightness * saturation));
   double temp1 = 2.0 * lightness - temp2;
   return SkColorSetARGB(alpha,
       calcHue(temp1, temp2, hue + 1.0 / 3.0),
@@ skipping 41 matching lines @@
                          : hsl.h >= lower_bound.h && hsl.h <= upper_bound.h;
   return (upper_bound.h < 0 || lower_bound.h < 0 || matches_hue) &&
          (upper_bound.s < 0 || lower_bound.s < 0 ||
           (hsl.s >= lower_bound.s && hsl.s <= upper_bound.s)) &&
          (upper_bound.l < 0 || lower_bound.l < 0 ||
           (hsl.l >= lower_bound.l && hsl.l <= upper_bound.l));
 }
 
 SkColor HSLShift(SkColor color, const HSL& shift) {
   HSL hsl;
-  int alpha = SkColorGetA(color);
+  SkAlpha alpha = SkColorGetA(color);

[danakj, 2014/10/18 18:40:06]

why not uint8?

[Peter Kasting, 2014/10/20 23:38:56]

SkAlpha is typedefed to uint8, so they should be the same; using an SkAlpha to
hold an alpha value felt semantically clearer to me.  I'll change it if there's
some strong reason to.

[danakj, 2014/10/23 15:35:44]

uint8 is a familiar type and we all know what it is. SkAlpha is familiar to less
folks so I like using it less if it's the same thing.

[Peter Kasting, 2014/10/23 17:29:45]

Sorry, I forgot the main rationale here when I replied before, or I would have
mentioned it then.  This value is passed to HSLToSkColor() below, which takes an
SkAlpha, not a uint8.  I think we should ensure those types match.

(Also, if someone is writing Skia code, and dealing with color alpha values, and
yet is confused by SkAlpha, I feel like something is wrong...)

[danakj, 2014/10/23 17:43:22]

I think we rely on SkColorGet[ARGB]() to return a byte in many places in the
code base so it's not something we could just change. 


HSLToSkColor eventually ends up in SkColorSetARGBInline() which takes a uint8_t
for the alpha value, so it's just converting the SkAlpha back. I think
HSLToSkColor should really use a uint8_t also, that'd be simpler to
read/understand.

But I don't feel strongly really it's my preference here so go with what you
like, I don't mean to block the rest of the changes over this.

   SkColorToHSL(color, &hsl);
 
   // Replace the hue with the tint's hue.
   if (shift.h >= 0)
     hsl.h = shift.h;
 
   // Change the saturation.
   if (shift.s >= 0) {
     if (shift.s <= 0.5)
       hsl.s *= shift.s * 2.0;
@@ skipping 95 matching lines @@
 SkColor GetSysSkColor(int which) {
 #if defined(OS_WIN)
   return skia::COLORREFToSkColor(GetSysColor(which));
 #else
   NOTIMPLEMENTED();
   return SK_ColorLTGRAY;
 #endif
 }
 
 }  // namespace color_utils