Adjust overlay scrollbar appearance.

ui/native_theme/native_theme_aura.cc

Index: ui/native_theme/native_theme_aura.cc
diff --git a/ui/native_theme/native_theme_aura.cc b/ui/native_theme/native_theme_aura.cc
index 5c2a462e90f1b83947ca481a9020801aa78cc118..d2cfc1fb55c8a45627deec0c0bf59c7cbbe5226c 100644
--- a/ui/native_theme/native_theme_aura.cc
+++ b/ui/native_theme/native_theme_aura.cc
@@ -33,7 +33,6 @@ namespace {
 
 // Constants for painting overlay scrollbars. Other properties needed outside
 // this painting code are defined in overlay_scrollbar_constants_aura.h.
-constexpr int kOverlayScrollbarStrokeWidth = 1;
 constexpr int kOverlayScrollbarMinimumLength = 32;
 
 // 2 pixel border with 1 pixel center patch. The border is 2 pixels despite the
@@ -73,7 +72,7 @@ NativeThemeAura::NativeThemeAura(bool use_overlay_scrollbars)
 
   if (use_overlay_scrollbars_) {
     scrollbar_width_ =
-        kOverlayScrollbarThumbWidthPressed + kOverlayScrollbarStrokeWidth * 2;
+        kOverlayScrollbarThumbWidthPressed + kOverlayScrollbarStrokeWidth;
   }
 
   // Images and alphas declarations assume the following order.
@@ -199,14 +198,6 @@ void NativeThemeAura::PaintScrollbarThumb(
   SkColor thumb_color;
 
   if (use_overlay_scrollbars_) {
-    // Constants used for painting overlay scrollbar thumb.
-    constexpr SkAlpha kOverlayScrollbarFillAlphaNormal = 0x80;
-    constexpr SkAlpha kOverlayScrollbarFillAlphaHovered = 0xB3;
-    constexpr SkAlpha kOverlayScrollbarFillAlphaPressed = 0xB3;
-    constexpr SkAlpha kOverlayScrollbarStrokeAlphaNormal = 0x4D;
-    constexpr SkAlpha kOverlayScrollbarStrokeAlphaHovered = 0x80;
-    constexpr SkAlpha kOverlayScrollbarStrokeAlphaPressed = 0x80;
-
     // Indexed by ScrollbarOverlayColorTheme.
     constexpr SkColor kOverlayScrollbarThumbColor[] = {SK_ColorBLACK,
                                                        SK_ColorWHITE};
@@ -222,16 +213,16 @@ void NativeThemeAura::PaintScrollbarThumb(
         stroke_alpha = SK_AlphaTRANSPARENT;
         break;
       case NativeTheme::kHovered:
-        thumb_alpha = kOverlayScrollbarFillAlphaHovered;
-        stroke_alpha = kOverlayScrollbarStrokeAlphaHovered;
+        thumb_alpha = SK_AlphaOPAQUE * kOverlayScrollbarThumbHoverAlpha;
+        stroke_alpha = SK_AlphaOPAQUE * kOverlayScrollbarStrokeHoverAlpha;
         break;
       case NativeTheme::kNormal:
-        thumb_alpha = kOverlayScrollbarFillAlphaNormal;
-        stroke_alpha = kOverlayScrollbarStrokeAlphaNormal;
+        thumb_alpha = SK_AlphaOPAQUE * kOverlayScrollbarThumbNormalAlpha;
+        stroke_alpha = SK_AlphaOPAQUE * kOverlayScrollbarStrokeNormalAlpha;
         break;
       case NativeTheme::kPressed:
-        thumb_alpha = kOverlayScrollbarFillAlphaPressed;
-        stroke_alpha = kOverlayScrollbarStrokeAlphaPressed;
+        thumb_alpha = SK_AlphaOPAQUE * kOverlayScrollbarThumbHoverAlpha;
+        stroke_alpha = SK_AlphaOPAQUE * kOverlayScrollbarStrokeHoverAlpha;
         break;
       case NativeTheme::kNumStates:
         NOTREACHED();
@@ -247,21 +238,21 @@ void NativeThemeAura::PaintScrollbarThumb(
     flags.setStrokeWidth(kStrokeWidth);
 
     gfx::RectF stroke_rect(thumb_rect);
-    constexpr float kHalfStrokeWidth = kStrokeWidth / 2.f;
-    stroke_rect.Inset(kHalfStrokeWidth, kHalfStrokeWidth);
+    gfx::InsetsF stroke_insets(kStrokeWidth / 2.f);
+    // The edge to which the scrollbar is attached shouldn't have a border.
+    gfx::Insets edge_adjust_insets;
+    if (part == NativeTheme::kScrollbarHorizontalThumb)
+      edge_adjust_insets = gfx::Insets(0, 0, -kStrokeWidth, 0);
+    else
+      edge_adjust_insets = gfx::Insets(0, 0, 0, -kStrokeWidth);
+    stroke_rect.Inset(stroke_insets + edge_adjust_insets);

[bokan, 2017/04/18 13:17:56]

My understanding of Skia isn't so good, why didn't the old code work? (I can see
it didn't, but I don't understand why)

Does this new code mean that we're drawing the stroke for the omitted edge
outside the thumb rect, since the stroke_rect edge runs through the center of
the pixels just outside the rect? Do we have to do that? Shouldn't the fill
below paint over the right-most column (in the case of a vertical scrollbar) of
pixels in the canvas? Or is there some kind of blending going on?

[Evan Stade, 2017/04/18 14:13:30]

It worked fine for the fill portion, this is just applying the same for the
stroke.
Yes
Correct, the fill is not fully opaque.

     canvas->drawRect(gfx::RectFToSkRect(stroke_rect), flags);
 
     // Inset the all the edges edges so we fill-in the stroke below.
-    // The edge to which the scrollbar is attached shouldn't have a border.
     // For left vertical scrollbar, we will horizontally flip the canvas in
     // ScrollbarThemeOverlay::paintThumb.
-    gfx::Insets insets(kStrokeWidth);
-    if (part == NativeTheme::kScrollbarHorizontalThumb)
-      insets -= gfx::Insets(0, 0, kStrokeWidth, 0);
-    else
-      insets -= gfx::Insets(0, 0, 0, kStrokeWidth);
-
-    thumb_rect.Inset(insets);
+    gfx::Insets fill_insets(kStrokeWidth);
+    thumb_rect.Inset(fill_insets + edge_adjust_insets);
   } else {
     switch (state) {
       case NativeTheme::kDisabled: