Draw nicer arrows when the scrollbar buttons are not square.

ui/native_theme/native_theme_base.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/native_theme/native_theme_base.h"
 
 #include <limits>
 #include <memory>
 
 #include "base/command_line.h"
@@ skipping 350 matching lines @@
   SkPaint paint;
   paint.setColor(color);
 
   SkPath path = PathForArrow(rect, direction);
 
   gc->drawPath(path, paint);
 }
 
 SkPath NativeThemeBase::PathForArrow(const gfx::Rect& rect,
                                      Part direction) const {
-  gfx::Rect bounding_rect(rect);
-  const int padding_width = ceil(rect.width() / 4.f);
-  const int padding_height = ceil(rect.height() / 4.f);
-  bounding_rect.Inset(padding_width, padding_height);
-  const gfx::Point center = bounding_rect.CenterPoint();
-
+  gfx::RectF bounding_rect = BoundingRectForArrow(rect);
+  const gfx::PointF center = bounding_rect.CenterPoint();
   SkPath path;
   SkMatrix transform;
   transform.setIdentity();
   if (direction == kScrollbarUpArrow || direction == kScrollbarDownArrow) {
     int arrow_altitude = bounding_rect.height() / 2 + 1;
     path.moveTo(bounding_rect.x(), bounding_rect.bottom());
     path.rLineTo(bounding_rect.width(), 0);
     path.rLineTo(-bounding_rect.width() / 2.0f, -arrow_altitude);
     path.close();
     path.offset(0, -arrow_altitude / 2 + 1);
     if (direction == kScrollbarDownArrow) {
-      path.offset(0, -1);

[Peter Kasting, 2016/06/10 01:03:23]

What were these offsets for?

[Bret, 2016/06/11 00:11:43]

Before this change the bounding box was an integer Rect. When it got the center
point (line 375 above) it would be rounded, so down and left arrows were
slightly off center. But since I changed the bounding box to a RectF, that
problem went away.

       transform.setScale(1, -1, center.x(), center.y());
     }
   } else {
     int arrow_altitude = bounding_rect.width() / 2 + 1;
     path.moveTo(bounding_rect.x(), bounding_rect.y());
     path.rLineTo(0, bounding_rect.height());
     path.rLineTo(arrow_altitude, -bounding_rect.height() / 2.0f);
     path.close();
     path.offset(arrow_altitude / 2, 0);
     if (direction == kScrollbarLeftArrow) {
-      path.offset(-1, 0);
       transform.setScale(-1, 1, center.x(), center.y());
     }
   }
   path.transform(transform);
 
   return path;
 }
 
+gfx::RectF NativeThemeBase::BoundingRectForArrow(const gfx::Rect& rect) const {

[Peter Kasting, 2016/06/10 01:03:23]

It seems like this whole function just does something like the following (except
hopefully this is a little less inaccurate rounding-wise due to not ceil()ing
multiple times):

  std::pair<int> sides = std::minmax(rect.width(), rect.height());
  float side_length =
      std::min(static_cast<float>(sides.first), sides.second * 0.75f);
  gfx::RectF bounding_rect(rect.x() + (rect.width() - side_length) / 2,
                           rect.y() + (rect.height() - side_length) / 2,
                           side_length, side_length);
  return gfx::ToEnclosedRect(bounding_rect);

[Bret, 2016/06/11 00:11:43]

I managed to synthesize your suggestion with what I want and I think it ended up
fairly nice. The fact that we do the extra ceils is important to maintain the
same look when the buttons are square.

+  gfx::RectF bounding_rect(rect);
+  int padding_width = ceil(rect.width() / 4.f);
+  int padding_height = ceil(rect.height() / 4.f);
+  bounding_rect.Inset(padding_width, padding_height);
+
+  // Ensure bounding_rect is a square.
+  if (bounding_rect.width() > bounding_rect.height()) {
+    float square_adjust =
+        (bounding_rect.width() - bounding_rect.height()) / 2.f;
+    bounding_rect.Inset(0, -square_adjust);
+    if (bounding_rect.height() > rect.height()) {
+      float clamp_adjust = (bounding_rect.height() - rect.height()) / 2.f;
+      bounding_rect.Inset(clamp_adjust, clamp_adjust);
+    }
+  } else if (bounding_rect.height() > bounding_rect.width()) {
+    float square_adjust =
+        (bounding_rect.height() - bounding_rect.width()) / 2.f;
+    bounding_rect.Inset(-square_adjust, 0);
+    if (bounding_rect.width() > rect.width()) {
+      float clamp_adjust = (bounding_rect.width() - rect.width()) / 2.f;
+      bounding_rect.Inset(clamp_adjust, clamp_adjust);
+    }
+  }
+
+  // Snap bounding_rect to whole pixels.
+  bounding_rect.set_x(ceil(bounding_rect.x()));
+  bounding_rect.set_y(ceil(bounding_rect.y()));
+  // Width and height are always aligned to whole pixels.
+
+  return bounding_rect;
+}
+
 void NativeThemeBase::PaintScrollbarTrack(SkCanvas* canvas,
     Part part,
     State state,
     const ScrollbarTrackExtraParams& extra_params,
     const gfx::Rect& rect) const {
   SkPaint paint;
   SkIRect skrect;
 
   skrect.set(rect.x(), rect.y(), rect.right(), rect.bottom());
   SkScalar track_hsv[3];
@@ skipping 590 matching lines @@
   SkScalar min_diff = Clamp((hsv1[1] + hsv2[1]) * 1.2f, 0.28f, 0.5f);
   SkScalar diff = Clamp(fabs(hsv1[2] - hsv2[2]) / 2, min_diff, 0.5f);
 
   if (hsv1[2] + hsv2[2] > 1.0)
     diff = -diff;
 
   return SaturateAndBrighten(hsv2, -0.2f, diff);
 }
 
 }  // namespace ui