Palette tool laser prototype.

ash/common/system/chromeos/palette/tools/laser_pointer_view.cc

+// Copyright 2016 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/chromeos/palette/tools/laser_pointer_view.h"
+
+#include <memory>
+#include "ash/common/shell_window_ids.h"

[jdufault, 2016/08/16 19:33:58]

newline between <memory> and "ash/..."

[sammiequon, 2016/08/16 23:18:55]

Done.

+#include "ash/common/wm_root_window_controller.h"
+#include "ash/common/wm_shell.h"
+#include "ash/common/wm_window.h"
+#include "third_party/skia/include/core/SkColor.h"
+#include "third_party/skia/include/core/SkPaint.h"
+#include "third_party/skia/include/core/SkRect.h"
+#include "ui/compositor/layer.h"
+#include "ui/gfx/canvas.h"
+#include "ui/gfx/transform.h"
+#include "ui/wm/core/coordinate_conversion.h"
+
+namespace ash {
+namespace {
+
+const double kMouselaserUiStartRadius = 4;

[jdufault, 2016/08/16 19:33:57]

What about

kPointInitialRadius = 4
kPointFinalRadius = 0.25

[jdufault, 2016/08/16 19:33:58]

Drop Mouse* prefix on all of these.

[sammiequon, 2016/08/16 23:18:55]

Done.

[sammiequon, 2016/08/16 23:18:55]

Done.

+const double kMouselaserUiEndRadius = 0.25;
+const int kMouselaserUiStartOpacityValue = 200;

[jdufault, 2016/08/16 19:33:58]

kPointInitialOpacity
kPointFinalOpacity

[sammiequon, 2016/08/16 23:18:56]

Done.

+const int kMouselaserUiEndOpacityValue = 0;
+const int kMouselaserBlueValue = 0;

[jdufault, 2016/08/16 19:33:58]

kPointColor

[sammiequon, 2016/08/16 23:18:55]

Done.

+const int kMouselaserGreenValue = 0;
+const int kMouselaserRedValue = 255;

[jdufault, 2016/08/16 19:33:57]

Declare the RGB values together in an SkColor instance, ie,

https://cs.chromium.org/chromium/src/ash/common/system/tray/tray_constants.cc...

[sammiequon, 2016/08/16 23:18:55]

Done.

+
+views::Widget* CreateLaserPointerWidget() {
+  views::Widget* widget = new views::Widget;
+  views::Widget::InitParams params;
+  params.type = views::Widget::InitParams::TYPE_WINDOW_FRAMELESS;
+  params.name = "LaserOverlay";
+  params.accept_events = false;
+  params.activatable = views::Widget::InitParams::ACTIVATABLE_NO;
+  params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  params.opacity = views::Widget::InitParams::TRANSLUCENT_WINDOW;
+  WmShell::Get()
+      ->GetRootWindowForNewWindows()
+      ->GetRootWindowController()
+      ->ConfigureWidgetInitParamsForContainer(
+          widget, kShellWindowId_OverlayContainer, &params);
+
+  widget->Init(params);
+  return widget;
+}
+
+float DistanceBetweenPoints(const gfx::Point& point1,
+                            const gfx::Point& point2) {
+  gfx::Vector2d vector = point1 - point2;

[jdufault, 2016/08/16 19:33:57]

return (point1 - point2).Length()

[sammiequon, 2016/08/16 23:18:55]

Done.

+  return vector.Length();
+}
+
+void PaintLaser(gfx::Canvas* canvas,
+                const LaserPointerPoints& points,
+                const gfx::Rect& widget_bounds) {
+  if (points.IsEmpty())
+    return;
+
+  SkPaint paint;
+  paint.setStyle(SkPaint::kStroke_Style);
+  paint.setAntiAlias(true);
+  paint.setStrokeJoin(SkPaint::kBevel_Join);
+  SkColor paintColor;
+
+  base::Time oldest = points.GetOldest().creation_time;
+  base::Time most_recent = points.GetMostRecent().creation_time;
+  gfx::Point last_point, current_point;
+  int point_count = 0;
+  int num_points = points.GetNumberOfPoints();
+  for (auto it = points.PointsStart(); it != points.PointsEnd();
+       it++, point_count++) {
+    double normalized;

[jdufault, 2016/08/16 19:33:57]

double normalized = 1.0;
if (oldest != most_recent) {
  // ...
}

[jdufault, 2016/08/16 19:33:57]

What do you think of the name 'progress' or 'relative_age' instead of
'normalized'?

[jdufault, 2016/08/16 19:33:58]

Add a comment along the lines of

// Normalized is a value between [0,1] where 0 means the point is about to be
removed and 1 means that the point was just added.

[sammiequon, 2016/08/16 23:18:55]

Done.

[sammiequon, 2016/08/16 23:18:56]

Done.

[sammiequon, 2016/08/16 23:18:56]

Done.

+    // Only one point, the regular normalization will return undefined for
+    // this case.
+    if (oldest == most_recent)
+      normalized = 1.0;
+    else {  // Normalize the distance.
+      normalized = (it->creation_time - oldest).InMillisecondsF() /
+                   (most_recent - oldest).InMillisecondsF();
+      normalized = 1.0 - normalized;
+    }
+
+    // Set the radius and opacity based on the distance.
+    double radius =
+        kMouselaserUiStartRadius -

[jdufault, 2016/08/16 19:33:57]

Add a lerp function and replace all of the instances of this with it.

double radius = LinearIntepolate(initial, final, progress)

[sammiequon, 2016/08/16 23:18:55]

Done.

+        normalized * (kMouselaserUiStartRadius - kMouselaserUiEndRadius);
+    int opacity = kMouselaserUiStartOpacityValue -
+                  int{normalized * (kMouselaserUiStartOpacityValue -
+                                    kMouselaserUiEndOpacityValue)};
+    gfx::Vector2d center = it->location - widget_bounds.origin();
+    current_point = gfx::Point(center.x(), center.y());
+    paintColor = SkColorSetARGB(opacity, kMouselaserRedValue,

[jdufault, 2016/08/16 19:33:58]

paint_color

[sammiequon, 2016/08/16 23:18:55]

Done.

+                                kMouselaserGreenValue, kMouselaserBlueValue);
+    paint.setColor(paintColor);
+    paint.setStrokeWidth(radius * 2);
+
+    // If we draw points that are within a stroke width of each other, the
+    // result will be very jagged.
+    float distance_threshold = float{radius * 2};

[jdufault, 2016/08/16 19:33:57]

Can we use a double to avoid the float cast?

[sammiequon, 2016/08/16 23:18:55]

I think since the points are floats and the time delta is in double a cast will
be required eventually. I think here would minimize the amount of casts.

+    if (point_count == 0) {
+      last_point = current_point;
+    } else if (DistanceBetweenPoints(last_point, current_point) >
+               distance_threshold) {
+      canvas->DrawLine(last_point, current_point, paint);
+      last_point = current_point;
+    } else if (point_count == num_points - 1) {
+      canvas->DrawLine(last_point, current_point, paint);
+    }
+  }
+  paintColor =
+      SkColorSetARGB(kMouselaserUiStartOpacityValue, kMouselaserRedValue,
+                     kMouselaserGreenValue, kMouselaserBlueValue);
+  paint.setColor(paintColor);
+  paint.setStyle(SkPaint::kFill_Style);
+  canvas->DrawCircle(current_point, kMouselaserUiStartRadius, paint);
+}
+}  // namespace

[jdufault, 2016/08/16 19:33:58]

newline above

[sammiequon, 2016/08/16 23:18:55]

Done.

+
+////////////////////////////////////////////////////////////////////////////////
+
+// LaserPointerView
+LaserPointerView::LaserPointerView(base::TimeDelta life_duration)
+    : views::View(), laser_points_(life_duration) {
+  widget_.reset(CreateLaserPointerWidget());
+  widget_->Show();
+  widget_->SetContentsView(this);
+  set_owned_by_client();
+}
+
+LaserPointerView::~LaserPointerView() {}
+
+void LaserPointerView::Stop() {
+  // Clear the points so that there is nothing to be rendered.
+  laser_points_.Clear();
+  SchedulePaint();
+}
+
+void LaserPointerView::AddNewPoint(const gfx::Point& new_point) {
+  laser_points_.AddPoint(new_point);
+  gfx::Rect old_bounding_box, new_bounding_box;
+  old_bounding_box = laser_points_.GetBoundingBox();
+  new_bounding_box.SetRect(

[jdufault, 2016/08/16 19:33:58]

Can you use only one bounding_box variable, all of the parameters will be
evaluated before the function call so it should be safe.

[sammiequon, 2016/08/16 23:18:55]

Done.

+      old_bounding_box.x() - kMouselaserUiStartRadius,
+      old_bounding_box.y() - kMouselaserUiStartRadius,
+      old_bounding_box.width() + 2 * kMouselaserUiStartRadius,

[jdufault, 2016/08/16 19:33:57]

old_bounding_box.width() + (kMouselaserUiStartRadius * 2)

[sammiequon, 2016/08/16 23:18:55]

Done.

+      old_bounding_box.height() + 2 * kMouselaserUiStartRadius);
+  widget_->SetBounds(new_bounding_box);
+  SchedulePaint();
+}
+
+void LaserPointerView::OnPaint(gfx::Canvas* canvas) {
+  canvas->Save();
+  PaintLaser(canvas, laser_points_, widget_->GetWindowBoundsInScreen());

[jdufault, 2016/08/16 19:33:58]

Inline this method.

[sammiequon, 2016/08/16 23:18:55]

Done.

+  canvas->Restore();
+}
+}  // namespace ash