cros: Change look of partial magnifier.

ash/magnifier/partial_magnification_controller.cc

Index: ash/magnifier/partial_magnification_controller.cc
diff --git a/ash/magnifier/partial_magnification_controller.cc b/ash/magnifier/partial_magnification_controller.cc
index e005b44eed382918eb6c650dcb9ace95e0b0227f..b2f54d6478cb667720385f99d911ab48dec98d34 100644
--- a/ash/magnifier/partial_magnification_controller.cc
+++ b/ash/magnifier/partial_magnification_controller.cc
@@ -5,6 +5,7 @@
 #include "ash/magnifier/partial_magnification_controller.h"
 
 #include "ash/shell.h"
+#include "third_party/skia/include/core/SkDrawLooper.h"

[sammiequon, 2016/09/22 00:51:46]

Is this allowed?

[jdufault, 2016/09/22 19:59:12]

Yes, third_party/skia is explicitly added in ash/DEPS.

[sammiequon, 2016/09/23 01:17:37]

Oh right, forgot to check there. Thanks!

 #include "ui/aura/window_event_dispatcher.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/compositor/layer.h"
@@ -23,16 +24,22 @@ namespace {
 
 // Ratio of magnifier scale.
 const float kMagnificationScale = 2.f;
-// Radius of the magnifying glass in DIP.
-const int kMagnifierRadius = 200;
+// Radius of the magnifying glass in DIP. This includes the thickness of the

[jdufault, 2016/09/22 19:59:12]

I think we should specify the radius of the magnified content without the
border, the size of the border, and the size of the shadow separately without
combining them together.

Doing this will make it easy to tweak and understand the values. I think it will
also simplify some of the computations below since it will make the data
dependencies explicit.

[sammiequon, 2016/09/23 01:17:37]

Makes sense, not sure why I didn't feel the need to do this the first time.
Done.

+// magnifying glass shadow and border.
+const int kMagnifierRadius = 222;
 // Size of the border around the magnifying glass in DIP.
 const int kBorderSize = 10;
 // Thickness of the outline around magnifying glass border in DIP.
-const int kBorderOutlineThickness = 2;
+const int kBorderOutlineThickness = 1;
 // The color of the border and its outlines. The border has an outline on both
 // sides, producing a black/white/black ring.
-const SkColor kBorderColor = SK_ColorWHITE;
-const SkColor kBorderOutlineColor = SK_ColorBLACK;
+const SkColor kBorderColor = SkColorSetARGB(204, 255, 255, 255);
+const SkColor kBorderOutlineColor = SkColorSetARGB(51, 0, 0, 0);

[jdufault, 2016/09/22 19:59:12]

Can we use SK_ColorTRANSPARENT for all of the colors with an alpha of 0?

[jdufault, 2016/09/22 22:24:44]

Sorry, ignore this comment. I was reading the colors as RGBA.

+// The colors of the two shadow around the magnifiying glass.
+const SkColor kShadowOneColor = SkColorSetARGB(26, 0, 0, 0);
+const SkColor kShadowTwoColor = SkColorSetARGB(61, 0, 0, 0);
+// Thickness of the shadow around the magnifying glass in DIP.
+const int kShadowThickness = 24;
 // Inset on the zoom filter.
 const int kZoomInset = 0;
 // Vertical offset between the center of the magnifier and the tip of the
@@ -80,10 +87,10 @@ bool ShouldSkipEventFiltering(const gfx::Point& point) {
 // can show a circular magnifier.
 class PartialMagnificationController::ContentMask : public ui::LayerDelegate {
  public:
-  // If |stroke| is true, the circle will be a stroke. This is useful if we wish
-  // to clip a border.
-  ContentMask(bool stroke, gfx::Size mask_bounds)
-      : layer_(ui::LAYER_TEXTURED), stroke_(stroke) {
+  // If |is_border| is true, the circle will be a stroke. This is useful if we
+  // wish to clip a border.
+  ContentMask(bool is_border, gfx::Size mask_bounds)
+      : layer_(ui::LAYER_TEXTURED), is_border_(is_border) {
     layer_.set_delegate(this);
     layer_.SetFillsBoundsOpaquely(false);
     layer_.SetBounds(gfx::Rect(mask_bounds));
@@ -101,12 +108,16 @@ class PartialMagnificationController::ContentMask : public ui::LayerDelegate {
     SkPaint paint;
     paint.setAlpha(255);
     paint.setAntiAlias(true);
-    paint.setStrokeWidth(kBorderSize);
-    paint.setStyle(stroke_ ? SkPaint::kStroke_Style : SkPaint::kFill_Style);
+    paint.setStrokeWidth(kBorderSize + kShadowThickness);
+    paint.setStyle(is_border_ ? SkPaint::kStroke_Style : SkPaint::kFill_Style);
 
+    // If we want to clip the border, the radius of the circle is a little
+    // bigger.
     gfx::Rect rect(layer()->bounds().size());
-    recorder.canvas()->DrawCircle(rect.CenterPoint(),
-                                  rect.width() / 2 - kBorderSize / 2, paint);
+    int clipping_radius =
+        rect.width() / 2 - (kShadowThickness + kBorderSize) / 2 -
+        (is_border_ ? 0 : (kShadowThickness + kBorderSize) / 2);
+    recorder.canvas()->DrawCircle(rect.CenterPoint(), clipping_radius, paint);
   }
 
   void OnDelegatedFrameDamage(const gfx::Rect& damage_rect_in_dip) override {}
@@ -120,46 +131,68 @@ class PartialMagnificationController::ContentMask : public ui::LayerDelegate {
   }
 
   ui::Layer layer_;
-  bool stroke_;
-
+  bool is_border_;
   DISALLOW_COPY_AND_ASSIGN(ContentMask);
 };
 
 // The border renderer draws the border as well as outline on both the outer and
-// inner radius to increase visibility.
+// inner radius to increase visibility. The border renderer also handles drawing
+// the shadow.
 class PartialMagnificationController::BorderRenderer
     : public ui::LayerDelegate {
  public:
-  explicit BorderRenderer(const gfx::Rect& magnifier_bounds)
-      : magnifier_bounds_(magnifier_bounds) {}
+  explicit BorderRenderer(const gfx::Rect& window_bounds)
+      : magnifier_window_bounds_(window_bounds) {
+    gfx::ShadowValue shadow1(gfx::Vector2d(0, -kShadowThickness),
+                             kShadowThickness, kShadowOneColor);
+    gfx::ShadowValue shadow2(gfx::Vector2d(0, 0), kShadowThickness,
+                             kShadowTwoColor);
+    magnifier_shadows_.push_back(shadow1);

[jdufault, 2016/09/22 19:59:12]

Drop the temporaries and push_back the values directly.

[sammiequon, 2016/09/23 01:17:37]

Done.

+    magnifier_shadows_.push_back(shadow2);
+  }
 
   ~BorderRenderer() override {}
 
  private:
   // ui::LayerDelegate:
   void OnPaintLayer(const ui::PaintContext& context) override {
-    ui::PaintRecorder recorder(context, magnifier_bounds_.size());
+    ui::PaintRecorder recorder(context, magnifier_window_bounds_.size());
+
+    // Draw the shadow.
+    SkPaint shadow_paint;
+    shadow_paint.setAntiAlias(true);
+    shadow_paint.setColor(SK_ColorTRANSPARENT);
+    shadow_paint.setLooper(gfx::CreateShadowDrawLooper(magnifier_shadows_));
+    gfx::Rect shadow_bounds(magnifier_window_bounds_.size());
+    shadow_bounds.Inset(-gfx::ShadowValue::GetMargin(magnifier_shadows_));
+    recorder.canvas()->DrawCircle(
+        shadow_bounds.CenterPoint(),
+        shadow_bounds.width() / 2 - kShadowThickness / 2, shadow_paint);
 
-    SkPaint paint;
-    paint.setAntiAlias(true);
-    paint.setStyle(SkPaint::kStroke_Style);
+    SkPaint border_paint;
+    border_paint.setAntiAlias(true);
+    border_paint.setStyle(SkPaint::kStroke_Style);
 
-    const int magnifier_radius = magnifier_bounds_.width() / 2;
+    // The radius of the magnifier and its border.
+    const int magnifier_radius =
+        magnifier_window_bounds_.width() / 2 - kShadowThickness;
     // Draw the inner border.
-    paint.setStrokeWidth(kBorderSize);
-    paint.setColor(kBorderColor);
-    recorder.canvas()->DrawCircle(magnifier_bounds_.CenterPoint(),
-                                  magnifier_radius - kBorderSize / 2, paint);
+    border_paint.setStrokeWidth(kBorderSize);
+    border_paint.setColor(kBorderColor);
+    recorder.canvas()->DrawCircle(magnifier_window_bounds_.CenterPoint(),
+                                  magnifier_radius - kBorderSize / 2,
+                                  border_paint);
 
     // Draw border outer outline and then draw the border inner outline.
-    paint.setStrokeWidth(kBorderOutlineThickness);
-    paint.setColor(kBorderOutlineColor);
+    border_paint.setStrokeWidth(kBorderOutlineThickness);
+    border_paint.setColor(kBorderOutlineColor);
     recorder.canvas()->DrawCircle(
-        magnifier_bounds_.CenterPoint(),
-        magnifier_radius - kBorderOutlineThickness / 2, paint);
+        magnifier_window_bounds_.CenterPoint(),
+        magnifier_radius - kBorderOutlineThickness / 2, border_paint);
     recorder.canvas()->DrawCircle(
-        magnifier_bounds_.CenterPoint(),
-        magnifier_radius - kBorderSize + kBorderOutlineThickness / 2, paint);
+        magnifier_window_bounds_.CenterPoint(),
+        magnifier_radius - kBorderSize + kBorderOutlineThickness / 2,
+        border_paint);
   }
 
   void OnDelegatedFrameDamage(const gfx::Rect& damage_rect_in_dip) override {}
@@ -170,7 +203,8 @@ class PartialMagnificationController::BorderRenderer
     return base::Closure();
   }
 
-  gfx::Rect magnifier_bounds_;
+  gfx::Rect magnifier_window_bounds_;
+  std::vector<gfx::ShadowValue> magnifier_shadows_;
 
   DISALLOW_COPY_AND_ASSIGN(BorderRenderer);
 };
@@ -245,7 +279,7 @@ void PartialMagnificationController::OnLocatedEvent(
   if (!is_enabled_)
     return;
 
-  if (pointer_details.pointer_type != ui::EventPointerType::POINTER_TYPE_PEN)
+  if (pointer_details.pointer_type == ui::EventPointerType::POINTER_TYPE_PEN)

[sammiequon, 2016/09/22 00:51:46]

Missed this again.

     return;
 
   // Compute the event location in screen space.
@@ -322,6 +356,7 @@ void PartialMagnificationController::CreateMagnifierWindow(
   border_layer_->SetBounds(gfx::Rect(GetWindowSize()));
   border_renderer_.reset(new BorderRenderer(gfx::Rect(GetWindowSize())));
   border_layer_->set_delegate(border_renderer_.get());
+  border_layer_->SetFillsBoundsOpaquely(false);
   root_layer->Add(border_layer_.get());
 
   border_mask_.reset(new ContentMask(true, GetWindowSize()));