Magnifier border is now more visible on light backgrounds.

ash/magnifier/partial_magnification_controller.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/magnifier/partial_magnification_controller.h"
 
 #include "ash/shell.h"
 #include "ui/aura/window_event_dispatcher.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/compositor/layer.h"
 #include "ui/compositor/paint_recorder.h"
 #include "ui/events/event.h"
 #include "ui/events/event_constants.h"
 #include "ui/views/widget/widget.h"
 #include "ui/wm/core/coordinate_conversion.h"
 
 namespace ash {
 namespace {
 
 // Ratio of magnifier scale.
 const float kMagnificationScale = 2.f;
 // Radius of the magnifying glass in DIP.
 const int kMagnifierRadius = 200;
 // Size of the border around the magnifying glass in DIP.
 const int kBorderSize = 10;
+// Thickness of the outline around magnifiying glass border.

[jdufault, 2016/08/26 22:56:46]

specify units (DIP)

[sammiequon, 2016/08/29 17:28:25]

Done.

+const int kBorderOutlineThickness = 2;
+const SkColor kBorderColor = SK_ColorWHITE;
+const SkColor kBorderOutlineColor = SK_ColorBLACK;
 // Inset on the zoom filter.
 const int kZoomInset = 0;
 // Vertical offset between the center of the magnifier and the tip of the
 // pointer. TODO(jdufault): The vertical offset should only apply to the window
 // location, not the magnified contents. See crbug.com/637617.
 const int kVerticalOffset = 0;
 
 // Name of the magnifier window.
 const char kPartialMagniferWindowName[] = "PartialMagnifierWindow";
 
@@ skipping 31 matching lines @@
     layer_.set_delegate(this);
     layer_.SetFillsBoundsOpaquely(false);
     layer_.SetBounds(gfx::Rect(mask_bounds));
   }
 
   ~ContentMask() override { layer_.set_delegate(nullptr); }
 
   ui::Layer* layer() { return &layer_; }
 
  private:
-  // Overridden from LayerDelegate.
+  // ui::LayerDelegate.
   void OnPaintLayer(const ui::PaintContext& context) override {
     ui::PaintRecorder recorder(context, layer()->size());
 
     SkPaint paint;
     paint.setAlpha(255);
     paint.setAntiAlias(true);
     paint.setStrokeWidth(kBorderSize);
     paint.setStyle(stroke_ ? SkPaint::kStroke_Style : SkPaint::kFill_Style);
 
     gfx::Rect rect(layer()->bounds().size());
@@ skipping 10 matching lines @@
   base::Closure PrepareForLayerBoundsChange() override {
     return base::Closure();
   }
 
   ui::Layer layer_;
   bool stroke_;
 
   DISALLOW_COPY_AND_ASSIGN(ContentMask);
 };
 
+// The border render draws the border as well as outline on both the outer and
+// inner radius to increase visibility.
+class PartialMagnificationController::BorderRenderer
+    : public ui::LayerDelegate {

[jdufault, 2016/08/26 22:56:46]

Is this from git cl format? Can you run it just in case?

[sammiequon, 2016/08/29 17:28:26]

Yes, ran it again. It is 81 long if one line.

+ public:
+  BorderRenderer(const gfx::Rect& bounds) : bounds_(bounds) {}
+
+  ~BorderRenderer() override {}
+
+ private:
+  // ui::LayerDelegate.
+  void OnPaintLayer(const ui::PaintContext& context) override {
+    ui::PaintRecorder recorder(context, bounds_.size());
+
+    // Draw the border.

[jdufault, 2016/08/26 22:56:46]

What about adding inner to make it more clear that this is only part of the
border?

  // Draw the inner border.

[sammiequon, 2016/08/29 17:28:26]

Done.

+    SkPaint paint;
+    paint.setAntiAlias(true);
+    paint.setStrokeWidth(kBorderSize);
+    paint.setStyle(SkPaint::kStroke_Style);

[jdufault, 2016/08/26 22:56:46]

Move all of the common paint config options away from the comment so it is clear
that these settings are global / shared among every paint operation.


  SkPaint paint;
  paint.setAntiAlias(true);
  paint.setStylue(SkPaint::kStroke_Style);

  // Draw the inner border.
  ...

[sammiequon, 2016/08/29 17:28:25]

Done.

+    paint.setColor(kBorderColor);
+    recorder.canvas()->DrawCircle(bounds_.CenterPoint(),
+                                  bounds_.width() / 2 - kBorderSize / 2, paint);
+
+    paint.setStrokeWidth(kBorderOutlineThickness);
+    paint.setColor(kBorderOutlineColor);
+    // Draw border outer outline.

[jdufault, 2016/08/26 22:56:46]

What about moving these comments above setStrokeWidth and collapsing them to

  // Draw the border outer outline and then draw the border inner outline.
  paint.setStrokeWidth(...)
  paint.setColor(...)
  ...DrawCircle(...)
  ...DrawCircle(...)

[sammiequon, 2016/08/29 17:28:25]

Done.

+    recorder.canvas()->DrawCircle(
+        bounds_.CenterPoint(),
+        bounds_.width() / 2 - kBorderOutlineThickness / 2, paint);
+    // Draw border inner outline.
+    recorder.canvas()->DrawCircle(
+        bounds_.CenterPoint(),
+        bounds_.width() / 2 - kBorderSize + kBorderOutlineThickness / 2, paint);
+  }
+
+  void OnDelegatedFrameDamage(const gfx::Rect& damage_rect_in_dip) override {}
+
+  void OnDeviceScaleFactorChanged(float device_scale_factor) override {}
+
+  base::Closure PrepareForLayerBoundsChange() override {
+    return base::Closure();
+  }
+
+  gfx::Rect bounds_;
+
+  DISALLOW_COPY_AND_ASSIGN(BorderRenderer);
+};
+
 PartialMagnificationController::PartialMagnificationController() {
   Shell::GetInstance()->AddPreTargetHandler(this);
 }
 
 PartialMagnificationController::~PartialMagnificationController() {
   CloseMagnifierWindow();
 
   Shell::GetInstance()->RemovePreTargetHandler(this);
 }
 
@@ skipping 118 matching lines @@
   aura::Window* window = host_widget_->GetNativeView();
   window->SetName(kPartialMagniferWindowName);
 
   ui::Layer* root_layer = host_widget_->GetNativeView()->layer();
 
   zoom_layer_.reset(new ui::Layer(ui::LayerType::LAYER_SOLID_COLOR));
   zoom_layer_->SetBounds(gfx::Rect(GetWindowSize()));
   zoom_layer_->SetBackgroundZoom(kMagnificationScale, kZoomInset);
   root_layer->Add(zoom_layer_.get());
 
-  border_layer_.reset(new ui::Layer(ui::LayerType::LAYER_SOLID_COLOR));
+  border_layer_.reset(new ui::Layer(ui::LayerType::LAYER_TEXTURED));
   border_layer_->SetBounds(gfx::Rect(GetWindowSize()));
-  border_layer_->SetColor(SK_ColorWHITE);
+  border_layer_->set_delegate(new BorderRenderer(gfx::Rect(GetWindowSize())));
   root_layer->Add(border_layer_.get());
 
   border_mask_.reset(new ContentMask(true, GetWindowSize()));
   border_layer_->SetMaskLayer(border_mask_->layer());
 
   zoom_mask_.reset(new ContentMask(false, GetWindowSize()));
   zoom_layer_->SetMaskLayer(zoom_mask_->layer());
 
   host_widget_->AddObserver(this);
 }
 
 void PartialMagnificationController::CloseMagnifierWindow() {
   if (host_widget_) {
     RemoveZoomWidgetObservers();
     host_widget_->Close();
     host_widget_ = nullptr;
   }
 }
 
 void PartialMagnificationController::RemoveZoomWidgetObservers() {
   DCHECK(host_widget_);
   host_widget_->RemoveObserver(this);
   aura::Window* root_window = host_widget_->GetNativeView()->GetRootWindow();
   DCHECK(root_window);
   root_window->RemoveObserver(this);
 }
 
 }  // namespace ash