Adjust MD shelf app list icon for better centering.

ash/common/shelf/app_list_button.cc

Index: ash/common/shelf/app_list_button.cc
diff --git a/ash/common/shelf/app_list_button.cc b/ash/common/shelf/app_list_button.cc
index 714c37c58e80757d52de718d2259159717c07847..1727ecc37c8a0c32c28e4f6dd94f4218e2d7df14 100644
--- a/ash/common/shelf/app_list_button.cc
+++ b/ash/common/shelf/app_list_button.cc
@@ -14,7 +14,6 @@
 #include "ash/common/shelf/wm_shelf_util.h"
 #include "ash/common/wm_shell.h"
 #include "ash/public/cpp/shelf_types.h"
-#include "ash/resources/vector_icons/vector_icons.h"
 #include "base/command_line.h"
 #include "grit/ash_resources.h"
 #include "grit/ash_strings.h"
@@ -23,7 +22,6 @@
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas.h"
-#include "ui/gfx/paint_vector_icon.h"
 #include "ui/views/animation/ink_drop_impl.h"
 #include "ui/views/animation/square_ink_drop_ripple.h"
 #include "ui/views/painter.h"
@@ -141,55 +139,47 @@ void AppListButton::OnGestureEvent(ui::GestureEvent* event) {
 void AppListButton::OnPaint(gfx::Canvas* canvas) {
   // Call the base class first to paint any background/borders.
   View::OnPaint(canvas);
-  ResourceBundle& rb = ResourceBundle::GetSharedInstance();
-
-  const gfx::ImageSkia& foreground_image =
-      MaterialDesignController::IsShelfMaterial()
-          ? CreateVectorIcon(kShelfAppListIcon, kShelfIconColor)
-          : *rb.GetImageNamed(IDR_ASH_SHELF_ICON_APPLIST).ToImageSkia();
 
   if (ash::MaterialDesignController::IsShelfMaterial()) {
-    PaintBackgroundMD(canvas);
-    PaintForegroundMD(canvas, foreground_image);
+    PaintMd(canvas);
   } else {
+    ResourceBundle& rb = ResourceBundle::GetSharedInstance();
+    const gfx::ImageSkia& foreground_image =
+        *rb.GetImageNamed(IDR_ASH_SHELF_ICON_APPLIST).ToImageSkia();
     PaintAppListButton(canvas, foreground_image);
   }
 
   views::Painter::PaintFocusPainter(this, canvas, focus_painter());
 }
 
-void AppListButton::PaintBackgroundMD(gfx::Canvas* canvas) {
+void AppListButton::PaintMd(gfx::Canvas* canvas) {
   // Paint the circular background of AppList button.

[tdanderson, 2016/11/16 18:19:36]

nit: consider DCHECK(IsMd())

[Evan Stade, 2016/11/16 18:29:53]

I like to use that as documentation but in this case the function name serves
that purpose.

[tdanderson, 2016/11/16 20:20:18]

Acknowledged.

-  gfx::Point circle_center = GetContentsBounds().CenterPoint();
+  gfx::PointF circle_center = gfx::RectF(GetContentsBounds()).CenterPoint();
   if (!IsHorizontalAlignment(wm_shelf_->GetAlignment()))
-    circle_center = gfx::Point(circle_center.y(), circle_center.x());
-
-  SkPaint background_paint;
-  background_paint.setColor(SkColorSetA(kShelfBaseColor, background_alpha_));
-  background_paint.setFlags(SkPaint::kAntiAlias_Flag);
-  background_paint.setStyle(SkPaint::kFill_Style);
-
-  canvas->DrawCircle(circle_center, kAppListButtonRadius, background_paint);
-}
-
-void AppListButton::PaintForegroundMD(gfx::Canvas* canvas,
-                                      const gfx::ImageSkia& foreground_image) {
-  gfx::Rect foreground_bounds(foreground_image.size());
-  gfx::Rect contents_bounds = GetContentsBounds();
-
-  if (IsHorizontalAlignment(wm_shelf_->GetAlignment())) {
-    foreground_bounds.set_x(
-        (contents_bounds.width() - foreground_bounds.width()) / 2);
-    foreground_bounds.set_y(
-        (contents_bounds.height() - foreground_bounds.height()) / 2);
-  } else {
-    foreground_bounds.set_x(
-        (contents_bounds.height() - foreground_bounds.height()) / 2);
-    foreground_bounds.set_y(
-        (contents_bounds.width() - foreground_bounds.width()) / 2);
-  }
-  canvas->DrawImageInt(foreground_image, foreground_bounds.x(),
-                       foreground_bounds.y());
+    circle_center = gfx::PointF(circle_center.y(), circle_center.x());
+
+  SkPaint bg_paint;
+  bg_paint.setColor(SkColorSetA(kShelfBaseColor, background_alpha_));
+  bg_paint.setFlags(SkPaint::kAntiAlias_Flag);
+  bg_paint.setStyle(SkPaint::kFill_Style);
+  canvas->DrawCircle(circle_center, kAppListButtonRadius, bg_paint);
+
+  // Paint a white ring as the foreground. The ceil/dsf math assures that the
+  // ring draws sharply and is centered at all scale factors.
+  const float kRingOuterRadiusDp = 6.5f;
+  const float kRingThicknessDp = 1.5f;
+  const float dsf = canvas->UndoDeviceScaleFactor();

[tdanderson, 2016/11/16 18:19:36]

Do you not need a corresponding call to Restore()? UndoDeviceScaleFactor() looks
to mutate the canvas.

[Evan Stade, 2016/11/16 18:29:53]

we have done that a lot in the past, but Sadrul pointed out it was unnecessary
because View::Paint creates a new canvas for each view (i.e. this canvas won't
be used again after this function finishes).

[tdanderson, 2016/11/16 20:20:18]

Acknowledged.

+  circle_center.Scale(dsf);
+
+  SkPaint fg_paint;
+  fg_paint.setFlags(SkPaint::kAntiAlias_Flag);
+  fg_paint.setStyle(SkPaint::kStroke_Style);
+  fg_paint.setColor(kShelfIconColor);
+  const float thickness = std::ceil(kRingThicknessDp * dsf);
+  const float radius = std::ceil(kRingOuterRadiusDp * dsf) - thickness / 2;
+  fg_paint.setStrokeWidth(thickness);
+  // Make sure the center of the circle lands on pixel centers.
+  canvas->DrawCircle(circle_center, radius, fg_paint);
 }
 
 void AppListButton::PaintAppListButton(gfx::Canvas* canvas,