Improve painting of cros tray separators.

ash/common/system/tray/tray_background_view.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 "ash/common/system/tray/tray_background_view.h"
 
 #include <algorithm>
 
 #include "ash/common/ash_constants.h"
-#include "ash/common/material_design/material_design_controller.h"
 #include "ash/common/shelf/shelf_constants.h"
 #include "ash/common/shelf/wm_shelf.h"
 #include "ash/common/shelf/wm_shelf_util.h"
 #include "ash/common/system/tray/system_tray.h"
 #include "ash/common/system/tray/tray_constants.h"
 #include "ash/common/system/tray/tray_event_filter.h"
 #include "ash/common/wm_lookup.h"
 #include "ash/common/wm_shell.h"
 #include "ash/common/wm_window.h"
 #include "ash/public/cpp/shell_window_ids.h"
@@ skipping 539 matching lines @@
   if (action_performed)
     return;
   ActionableView::HandlePerformActionResult(action_performed, event);
 }
 
 void TrayBackgroundView::OnPaintFocus(gfx::Canvas* canvas) {
   // The tray itself expands to the right and bottom edge of the screen to make
   // sure clicking on the edges brings up the popup. However, the focus border
   // should be only around the container.
   gfx::RectF paint_bounds;
-  if (MaterialDesignController::IsShelfMaterial()) {
-    paint_bounds = gfx::RectF(GetBackgroundBounds());
-    paint_bounds.Inset(gfx::Insets(-kFocusBorderThickness));
-  } else {
-    paint_bounds = gfx::RectF(GetContentsBounds());
-    paint_bounds.Inset(gfx::Insets(1));
-  }
+  paint_bounds = gfx::RectF(GetBackgroundBounds());
+  paint_bounds.Inset(gfx::Insets(-kFocusBorderThickness));
   canvas->DrawSolidFocusRect(paint_bounds, kFocusBorderColor,
                              kFocusBorderThickness);
 }
 
 void TrayBackgroundView::OnPaint(gfx::Canvas* canvas) {
   ActionableView::OnPaint(canvas);
-  if (!MaterialDesignController::IsShelfMaterial() ||
-      shelf()->GetBackgroundType() ==
+  if (shelf()->GetBackgroundType() ==
           ShelfBackgroundType::SHELF_BACKGROUND_DEFAULT ||
       !separator_visible_) {
     return;
   }
   // In the given |canvas|, for a horizontal shelf draw a separator line to the
   // right or left of the TrayBackgroundView when the system is LTR or RTL
   // aligned, respectively. For a vertical shelf draw the separator line
   // underneath the items instead.
-  const bool horizontal_shelf = IsHorizontalAlignment(shelf_alignment_);
   const gfx::Rect local_bounds = GetLocalBounds();
-  const int height = kTrayItemSize;
-  const int x =
-      horizontal_shelf
-          ? (base::i18n::IsRTL() ? 0 : (local_bounds.width() - kSeparatorWidth))
-          : (local_bounds.height() - kSeparatorWidth);
-  const int y = (GetShelfConstant(SHELF_SIZE) - kTrayItemSize) / 2;
-  gfx::ScopedCanvas scoped_canvas(canvas);
-  const float scale = canvas->UndoDeviceScaleFactor();
-  cc::PaintFlags flags;
-  flags.setColor(kTraySeparatorColor);
-  flags.setAntiAlias(true);
+  const SkColor color = SkColorSetA(SK_ColorWHITE, 0x4D);

[tdanderson, 2017/02/28 17:18:19]

Why are you changing the alpha from 0x99 to 0x4D?

[Evan Stade, 2017/02/28 18:15:26]

see my comment that I sent out with the review for explanation.

[tdanderson, 2017/02/28 18:41:34]

Acknowledged.

 
-  const gfx::Rect bounds = horizontal_shelf
-                               ? gfx::Rect(x, y, kSeparatorWidth, height)
-                               : gfx::Rect(y, x, height, kSeparatorWidth);
-  gfx::RectF rect(gfx::ScaleRect(gfx::RectF(bounds), scale));
-  gfx::PointF line_start =
-      horizontal_shelf
-          ? (base::i18n::IsRTL() ? rect.origin() : rect.top_right())
-          : rect.bottom_left();
-  gfx::PointF line_end =
-      (horizontal_shelf && base::i18n::IsRTL() ? rect.bottom_left()
-                                               : rect.bottom_right());
-
-  canvas->DrawLine(line_start, line_end, flags);
+  if (IsHorizontalAlignment(shelf_alignment_)) {
+    const gfx::PointF point(
+        base::i18n::IsRTL() ? 0 : (local_bounds.width() - kSeparatorWidth),
+        (GetShelfConstant(SHELF_SIZE) - kTrayItemSize) / 2);

[tdanderson, 2017/02/28 17:18:19]

looks like a case of a missing )

+    const gfx::Vector2dF vector(0, kTrayItemSize);
+    canvas->Draw1pxLine(point, point + vector, color);
+  } else {
+    const gfx::PointF point((GetShelfConstant(SHELF_SIZE) - kTrayItemSize) / 2,

[tdanderson, 2017/02/28 17:18:19]

I think your x and y should be reversed here

[Evan Stade, 2017/02/28 18:15:26]

I don't think so. This is for vertical shelf.

[tdanderson, 2017/02/28 18:41:34]

My mistake, I was just looking at the current code's definition of x and y,
neglecting to notice that these are swapped by line 207 for a vertical shelf.

+                            local_bounds.height() - kSeparatorWidth);
+    const gfx::Vector2dF vector(kTrayItemSize, 0);
+    canvas->Draw1pxLine(point, point + vector, color);
+  }
 }
 
 gfx::Insets TrayBackgroundView::GetBackgroundInsets() const {
   gfx::Insets insets = GetMirroredBackgroundInsets(shelf_alignment_);
 
   // |insets| are relative to contents bounds. Change them to be relative to
   // local bounds.
   gfx::Insets local_contents_insets =
       GetLocalBounds().InsetsFrom(GetContentsBounds());
   MirrorInsetsIfNecessary(&local_contents_insets);
   insets += local_contents_insets;
 
   return insets;
 }
 
 gfx::Rect TrayBackgroundView::GetBackgroundBounds() const {
   gfx::Insets insets = GetBackgroundInsets();
   gfx::Rect bounds = GetLocalBounds();
   bounds.Inset(insets);
   return bounds;
 }
 
 }  // namespace ash