Tweaks to ToggleButton details.

ui/views/controls/button/toggle_button.cc

 // Copyright 2016 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/views/controls/button/toggle_button.h"
 
 #include "third_party/skia/include/core/SkDrawLooper.h"
 #include "third_party/skia/include/core/SkPaint.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/views/animation/ink_drop_ripple.h"
 #include "ui/views/border.h"
 
 namespace views {
 
 namespace {
 
 // Constants are measured in dip.
 const int kTrackHeight = 12;
 const int kTrackWidth = 28;
 // Margins from edge of track to edge of view.
-const int kTrackVerticalMargin = 5;
+const int kTrackVerticalMargin = 4;
 const int kTrackHorizontalMargin = 6;
-// Margin from edge of thumb to closest edge of view. Note that the thumb
-// margins must be sufficiently large to allow space for the shadow.
+// Margin from edge of thumb to closest edge of view.
 const int kThumbHorizontalMargin = 4;
 // Margin from top/bottom edge of thumb to top/bottom edge of view.
-const int kThumbVerticalMargin = 3;
-
-// TODO(estade): get the base color (black) from the theme?
-const SkColor kTrackOffColor =
-    SkColorSetA(SK_ColorBLACK, gfx::kDisabledControlAlpha);
+const int kThumbVerticalMargin = 2;
 
 }  // namespace
 
-// Class representing the thumb. When the thumb is clicked it is separated into
-// its own layer and the ink drop layer is made a child of the thumb layer
-// allowing the two to animate in sync.
-class ToggleButton::ThumbView : public views::View {
+// Class representing the thumb (the circle that slides horizontally).
+class ToggleButton::ThumbView : public InkDropHostView {
  public:
-  ThumbView() : color_ratio_(0.) {}
-  ~ThumbView() override {}
-
-  void AddInkDropLayer(ui::Layer* ink_drop_layer) {
+  ThumbView() : color_ratio_(0.) {
     SetPaintToLayer(true);

[varkha, 2016/10/21 00:35:18]

The old implementation was more lines but it was only using extra layers when
the toggle is interacted with (and flattened otherwise). The new impl is always
using an extra layer - something we were trying to minimize before with the
ripple. Something probably only noticeable when there are many toggle buttons at
the same time but still worth consideration.

[Evan Stade, 2016/10/21 17:19:15]

It's not clear to me that creating and destroying this layer on every ink drop
is a performance improvement, but the real reason I added it is because the
shadow extends slightly beyond the bounds of the view.  That said, it's not
truly necessary if we leave the vertical margin constants alone, I mainly
changed those to match the overall height Sebastien spec'd.

[varkha, 2016/10/21 17:55:36]

+bruthig@ and sadrul@ to see if they have stronger opinions here. I think this
is what we are doing with the rest of the ripple-enabled buttons because we
tried to keep texture memory down as a concern.

[bruthig, 2016/10/21 18:36:40]

Even though the ThumbView is always painting to a Layer the ink drop layers will
be cleaned up when not active.  Considering that the ShelfButtons paint to their
own layer I don't have any concerns with having 1 extra layer per toggle. 
Having said that, if it's easy enough to not require a layer then why not
eliminate the layer...

In summary, no strong opinion ;)

FTR I'd wait for sadrul@ to give an opinion too.

[sadrul, 2016/10/24 14:46:12]

Always having the layer seem unrelated to the bugs being fixed. Can you move the
layer-change to a separate CL so we can discuss that separately, and not block
the bug fix?

[Evan Stade, 2016/10/27 17:12:14]

Done.

     layer()->SetFillsBoundsOpaquely(false);
-    layer()->Add(ink_drop_layer);
   }
-
-  void RemoveInkDropLayer(ui::Layer* ink_drop_layer) {
-    layer()->Remove(ink_drop_layer);
-    SetPaintToLayer(false);
-  }
+  ~ThumbView() override {}
 
   void Update(const gfx::Rect& bounds, double color_ratio) {
     SetBoundsRect(bounds);
     color_ratio_ = color_ratio;
     SchedulePaint();
   }
 
+  static gfx::Insets GetShadowInsets() {
+    gfx::Insets blur(kShadowBlur);
+    gfx::Insets offset(-kShadowOffsetY, -kShadowOffsetX, kShadowOffsetY,
+                       kShadowOffsetX);
+    return blur + offset;
+  }
+
  private:
+  static const int kShadowOffsetX = 0;
+  static const int kShadowOffsetY = 1;
+  static const int kShadowBlur = 2;
+
   // views::View:
   const char* GetClassName() const override {
     return "ToggleButton::ThumbView";
   }
 
   void OnPaint(gfx::Canvas* canvas) override {
     std::vector<gfx::ShadowValue> shadows;
-    shadows.emplace_back(gfx::Vector2d(0, 1), 4.f,
-                         SkColorSetA(SK_ColorBLACK, 0x99));
+    shadows.emplace_back(
+        gfx::Vector2d(kShadowOffsetX, kShadowOffsetY), 2 * kShadowBlur,
+        SkColorSetA(GetNativeTheme()->GetSystemColor(
+                        ui::NativeTheme::kColorId_LabelEnabledColor),
+                    0x99));
     SkPaint thumb_paint;
     thumb_paint.setLooper(gfx::CreateShadowDrawLooperCorrectBlur(shadows));
-    thumb_paint.setStyle(SkPaint::kFill_Style);
     thumb_paint.setAntiAlias(true);
     const SkColor thumb_on_color = GetNativeTheme()->GetSystemColor(
         ui::NativeTheme::kColorId_ProminentButtonColor);
-    // TODO(estade): get this color from the theme?
-    const SkColor thumb_off_color = SK_ColorWHITE;
+    const SkColor thumb_off_color = GetNativeTheme()->GetSystemColor(
+        ui::NativeTheme::kColorId_DialogBackground);
     const SkAlpha blend = static_cast<SkAlpha>(SK_AlphaOPAQUE * color_ratio_);
     thumb_paint.setColor(
         color_utils::AlphaBlend(thumb_on_color, thumb_off_color, blend));
-    gfx::Rect thumb_bounds = GetLocalBounds();
-    thumb_bounds.Inset(gfx::Insets(kThumbVerticalMargin));
-    canvas->DrawCircle(gfx::RectF(thumb_bounds).CenterPoint(),
-                       thumb_bounds.height() / 2.f, thumb_paint);
+
+    gfx::RectF thumb_bounds(GetLocalBounds());
+    thumb_bounds.Inset(GetShadowInsets());
+    thumb_bounds.Inset(gfx::InsetsF(0.5f));
+    gfx::RectF bounds_in_px = thumb_bounds;
+    // We want the circle to have an integer pixel diameter and to be aligned
+    // with pixel boundaries, so we scale dip bounds to pixel bounds, round, and
+    // unscale back to dip bounds.
+    bounds_in_px.Scale(canvas->image_scale());
+    gfx::RectF bounds_in_dip(gfx::ToEnclosingRect(bounds_in_px));
+    bounds_in_dip.Scale(1.f / canvas->image_scale());
+    canvas->DrawCircle(bounds_in_dip.CenterPoint(),
+                       bounds_in_dip.height() / 2.f, thumb_paint);
   }
 
   // Color ratio between 0 and 1 that controls the thumb color.
   double color_ratio_;
 
   DISALLOW_COPY_AND_ASSIGN(ThumbView);
 };
 
 // static
 const char ToggleButton::kViewClassName[] = "ToggleButton";
 
 ToggleButton::ToggleButton(ButtonListener* listener)
     : CustomButton(listener),
       is_on_(false),
       slide_animation_(this),
-      thumb_view_(new ToggleButton::ThumbView()) {
+      thumb_view_(new ThumbView()) {
   slide_animation_.SetSlideDuration(80 /* ms */);
   slide_animation_.SetTweenType(gfx::Tween::LINEAR);
   SetBorder(Border::CreateEmptyBorder(
       gfx::Insets(kTrackVerticalMargin, kTrackHorizontalMargin)));
-  AddChildView(thumb_view_.get());
+  AddChildView(thumb_view_);
   SetInkDropMode(InkDropMode::ON);
   set_has_ink_drop_action_on_click(true);
 }
 
 ToggleButton::~ToggleButton() {
   // Destroying ink drop early allows ink drop layer to be properly removed,
   SetInkDropMode(InkDropMode::OFF);
 }
 
 void ToggleButton::SetIsOn(bool is_on, bool animate) {
@@ skipping 11 matching lines @@
 
 gfx::Rect ToggleButton::GetThumbBounds() const {
   gfx::Rect thumb_bounds = GetLocalBounds();
   thumb_bounds.Inset(gfx::Insets(kThumbVerticalMargin, kThumbHorizontalMargin));
   thumb_bounds.set_x(thumb_bounds.x() +
                      slide_animation_.GetCurrentValue() *
                          (thumb_bounds.width() - thumb_bounds.height()));
   // The thumb is a circle, so the width should match the height.
   thumb_bounds.set_width(thumb_bounds.height());
   thumb_bounds.set_x(GetMirroredXForRect(thumb_bounds));
+  thumb_bounds.Inset(-ThumbView::GetShadowInsets());

[varkha, 2016/10/21 00:35:18]

Thank you!

   return thumb_bounds;
 }
 
 void ToggleButton::UpdateThumb() {
-  gfx::Rect thumb_bounds = GetThumbBounds();
-  thumb_bounds.Inset(gfx::Insets(-kThumbVerticalMargin));
-  thumb_view_->Update(thumb_bounds, slide_animation_.GetCurrentValue());
+  thumb_view_->Update(GetThumbBounds(), slide_animation_.GetCurrentValue());
 }
 
 gfx::Size ToggleButton::GetPreferredSize() const {
   gfx::Rect rect(0, 0, kTrackWidth, kTrackHeight);
   if (border())
     rect.Inset(-border()->GetInsets());
   return rect.size();
 }
 
 const char* ToggleButton::GetClassName() const {
   return kViewClassName;
 }
 
 void ToggleButton::OnPaint(gfx::Canvas* canvas) {
   // Paint the toggle track.
   gfx::RectF track_rect(GetContentsBounds());
   SkPaint track_paint;
   track_paint.setAntiAlias(true);
-  const SkColor track_on_color =
-      SkColorSetA(GetNativeTheme()->GetSystemColor(
-                      ui::NativeTheme::kColorId_ProminentButtonColor),
-                  0xFF / 2);
   const double color_ratio = slide_animation_.GetCurrentValue();
   track_paint.setColor(color_utils::AlphaBlend(
-      track_on_color, kTrackOffColor,
+      GetTrackColor(true), GetTrackColor(false),
       static_cast<SkAlpha>(SK_AlphaOPAQUE * color_ratio)));
   canvas->DrawRoundRect(track_rect, track_rect.height() / 2, track_paint);
 }
 
 void ToggleButton::NotifyClick(const ui::Event& event) {
   SetIsOn(!is_on(), true);
   CustomButton::NotifyClick(event);
 }
 
 void ToggleButton::OnBoundsChanged(const gfx::Rect& previous_bounds) {
   UpdateThumb();
 }
 
 void ToggleButton::OnNativeThemeChanged(const ui::NativeTheme* theme) {
   SchedulePaint();
 }
 
 void ToggleButton::AddInkDropLayer(ui::Layer* ink_drop_layer) {
   thumb_view_->AddInkDropLayer(ink_drop_layer);
   UpdateThumb();
   SchedulePaint();

[bruthig, 2016/10/21 18:36:40]

varkha@, IF the ThumbView is always painting to a layer are these
SchedulePaint() calls still necessary on lines 193 and 198?

 }
 
 void ToggleButton::RemoveInkDropLayer(ui::Layer* ink_drop_layer) {
   thumb_view_->RemoveInkDropLayer(ink_drop_layer);
   SchedulePaint();
 }
 
 std::unique_ptr<InkDropRipple> ToggleButton::CreateInkDropRipple() const {
   const int radius = (kTrackHeight + kTrackVerticalMargin * 2) / 2;
   return CreateDefaultInkDropRipple(gfx::Point(radius, radius));
 }
 
 SkColor ToggleButton::GetInkDropBaseColor() const {
-  return is_on()
-             ? GetNativeTheme()->GetSystemColor(
-                   ui::NativeTheme::kColorId_ProminentButtonColor)
-             : kTrackOffColor;
+  return GetTrackColor(is_on());
 }
 
 bool ToggleButton::ShouldShowInkDropHighlight() const {
   return false;
 }
 
 void ToggleButton::AnimationProgressed(const gfx::Animation* animation) {
   if (animation == &slide_animation_) {
     // TODO(varkha, estade): The thumb is using its own view. Investigate if
     // repainting in every animation step to update colors could be avoided.
     UpdateThumb();
     SchedulePaint();
     return;
   }
   CustomButton::AnimationProgressed(animation);
 }
 
+SkColor ToggleButton::GetTrackColor(bool is_on) const {
+  const SkAlpha kOffTrackAlpha = 0x29;
+  const SkAlpha kOnTrackAlpha = kOffTrackAlpha * 2;
+  ui::NativeTheme::ColorId color_id =
+      is_on ? ui::NativeTheme::kColorId_ProminentButtonColor
+            : ui::NativeTheme::kColorId_LabelEnabledColor;
+  return SkColorSetA(GetNativeTheme()->GetSystemColor(color_id),
+                     is_on ? kOnTrackAlpha : kOffTrackAlpha);
+}
+
 }  // namespace views