Palette tool laser prototype.

ash/common/system/chromeos/palette/tools/laser_pointer_points.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_points.h"
+
+#include <limits.h>

[jdufault, 2016/08/17 21:41:07]

<limits>

[sammiequon, 2016/08/18 00:52:07]

Done.

+
+namespace ash {
+
+LaserPointerPoints::LaserPointerPoints(base::TimeDelta life_duration)
+    : life_duration_(life_duration) {}
+
+LaserPointerPoints::~LaserPointerPoints() {}
+
+void LaserPointerPoints::AddPoint(const gfx::Point& point) {
+  // Creates and adds the new point. Updates the collection and bounding box

[jdufault, 2016/08/17 21:41:08]

Drop comment, it is already on the declaration.

[sammiequon, 2016/08/18 00:52:08]

Done.

+  // accordingly.
+  base::Time pointTime = base::Time::Now();

[jdufault, 2016/08/17 21:41:08]

point_time

[jdufault, 2016/08/17 21:41:08]

inline this variable since it is only used once.

[sammiequon, 2016/08/18 00:52:07]

Done.

[sammiequon, 2016/08/18 00:52:07]

Done.

+  LaserPoint newPoint;

[jdufault, 2016/08/17 21:41:07]

new_point

[sammiequon, 2016/08/18 00:52:08]

Done.

+  newPoint.location = point;
+  newPoint.creation_time = pointTime;
+
+  if (points_.empty())
+    bounds_.SetRect(point.x(), point.y(), 0, 0);
+  else
+    CalculateBounds(newPoint.location);
+
+  points_.push_back(newPoint);
+  ClearOldPoints();
+}
+
+void LaserPointerPoints::Clear() {
+  points_.clear();
+}
+
+gfx::Rect LaserPointerPoints::GetBoundingBox() const {
+  return bounds_;
+}
+
+LaserPointerPoints::LaserPoint LaserPointerPoints::GetOldest() const {
+  if (!points_.empty())

[jdufault, 2016/08/17 21:41:08]

Invert all of these conditions since the pattern is usually

  if (failure)
    handle

ie,

  if (points_.empty())
    return LaserPoint();

But thinking about it more, instead of having GetOldest return a default value,
I think it's better to just DCHECK on IsEmpty since the default point doesn't
have much meaning.

  GetOldest() {
    DCHECK(!IsEmpty());
    return points_.front();
  }

[sammiequon, 2016/08/18 00:52:08]

Done.

+    return points_.front();
+  return LaserPoint();
+}
+
+LaserPointerPoints::LaserPoint LaserPointerPoints::GetMostRecent() const {

[jdufault, 2016/08/17 21:41:08]

See GetOldest comment.

[sammiequon, 2016/08/18 00:52:08]

Done.

+  if (!points_.empty())
+    return points_.back();
+  return LaserPoint();
+}
+
+bool LaserPointerPoints::IsEmpty() const {
+  return points_.empty();
+}
+
+int LaserPointerPoints::GetNumberOfPoints() const {
+  return points_.size();
+}
+
+std::deque<LaserPointerPoints::LaserPoint>::const_iterator
+LaserPointerPoints::PointsStart() const {
+  return points_.cbegin();
+}
+
+std::deque<LaserPointerPoints::LaserPoint>::const_iterator
+LaserPointerPoints::PointsEnd() const {
+  return points_.cend();
+}
+
+void LaserPointerPoints::CalculateBounds(const gfx::Point& point) {
+  // This will do nothing if the point is within the current bounds.
+  gfx::Point min_point(bounds_.origin());
+  gfx::Point max_point(bounds_.bottom_right());
+  min_point = gfx::Point(std::min(min_point.x(), point.x()),
+                         std::min(min_point.y(), point.y()));
+  max_point = gfx::Point(std::max(max_point.x(), point.x()),
+                         std::max(max_point.y(), point.y()));
+  bounds_ = gfx::BoundingRect(min_point, max_point);
+}
+
+void LaserPointerPoints::ClearOldPoints() {
+  if (!points_.empty()) {
+    LaserPoint newest = points_.back();
+    bool needs_recalculate_bounds = false;
+    auto it = points_.begin();
+    for (; it != points_.end(); ++it) {
+      if (newest.creation_time - it->creation_time > life_duration_) {
+        if (!needs_recalculate_bounds && !bounds_.Contains(it->location))
+          needs_recalculate_bounds = true;
+      } else {
+        // Since the points are sorted by time we can end the loop early once
+        // the condition fails.
+        break;
+      }
+    }
+    points_.erase(points_.begin(), it);

[jdufault, 2016/08/17 21:41:08]

Nice, this works well.

[sammiequon, 2016/08/18 00:52:07]

:).

+
+    if (needs_recalculate_bounds)
+      RecalculateBounds();
+  }
+}
+
+void LaserPointerPoints::RecalculateBounds() {
+  int j = 0;

[jdufault, 2016/08/17 21:41:08]

Because the point length is only 40 or so, I don't think having this fancy
recalculate bounds logic is worth the effort to understand it.

If we ever need a new bounds just recompute it entirely. I'd actually remove the
cached bounds_ variable for the time being and if it's obviously slow, we can
add it back.

[sammiequon, 2016/08/18 00:52:07]

Removed. Doesn't seem to cause too much lag.

+  for (auto it = points_.begin(); it != points_.end(); ++it, ++j) {
+    if (j == 0)
+      bounds_.SetRect(it->location.x(), it->location.y(), 0, 0);
+    else
+      CalculateBounds(it->location);
+  }
+}
+}  // namespace ash