Don't add duplicate points when clipping

cc/base/math_util.cc

Index: cc/base/math_util.cc
diff --git a/cc/base/math_util.cc b/cc/base/math_util.cc
index d28c195971f6cd14eb753b6d9eddd51d1dd082fb..b34eb5ef03f13c148bc3862b3aa83fbb44fd7330 100644
--- a/cc/base/math_util.cc
+++ b/cc/base/math_util.cc
@@ -155,9 +155,34 @@ static inline void ExpandBoundsToIncludePoint(float* xmin,
   *ymax = std::max(p.y(), *ymax);
 }
 
+static inline bool isNearlyTheSame(const float f, const float g) {
+  static const float epsilon_scale = 0.00001f;

[flackr, 2017/01/11 18:23:57]

nit: Can you add a short comment as to why / how we vary what near means with
respect to epsilon_scale?

[Peter Mayo, 2017/01/11 22:23:31]

Done.

+  return std::abs(f - g) <
+         epsilon_scale *
+             std::max(std::max(std::abs(f), std::abs(g)), epsilon_scale);
+}
+
+static inline bool isNearlyTheSame(const gfx::PointF& lhs,
+                                   const gfx::PointF& rhs) {
+  return isNearlyTheSame(lhs.x(), rhs.x()) && isNearlyTheSame(lhs.y(), rhs.y());
+}
+
+static inline bool isNearlyTheSame(const gfx::Point3F& lhs,
+                                   const gfx::Point3F& rhs) {
+  return isNearlyTheSame(lhs.x(), rhs.x()) &&
+         isNearlyTheSame(lhs.y(), rhs.y()) && isNearlyTheSame(lhs.z(), rhs.z());
+}
+
 static inline void AddVertexToClippedQuad(const gfx::PointF& new_vertex,
                                           gfx::PointF clipped_quad[8],
                                           int* num_vertices_in_clipped_quad) {
+  if (*num_vertices_in_clipped_quad > 0) {
+    if (isNearlyTheSame(clipped_quad[*num_vertices_in_clipped_quad - 1],
+                        new_vertex))
+      return;
+    if (isNearlyTheSame(clipped_quad[0], new_vertex))

[flackr, 2017/01/11 18:23:57]

Instead of comparing every added vertex with vertex 0 could we add a check in
MapClippedQuad and MapClippedQuad3d after all of the vertices have been added
that the last is equal to the first and if so reduce the number of vertices to
clip the last one?

[Peter Mayo, 2017/01/11 22:23:31]

This expands the scope in which the "isNearlyTheSame" is used, but cuts down on
execution, so yes I think this is a net good idea.

+      return;
+  }
   clipped_quad[*num_vertices_in_clipped_quad] = new_vertex;
   (*num_vertices_in_clipped_quad)++;
 }
@@ -165,6 +190,13 @@ static inline void AddVertexToClippedQuad(const gfx::PointF& new_vertex,
 static inline void AddVertexToClippedQuad3d(const gfx::Point3F& new_vertex,
                                             gfx::Point3F clipped_quad[8],
                                             int* num_vertices_in_clipped_quad) {
+  if (*num_vertices_in_clipped_quad > 0) {
+    if (isNearlyTheSame(clipped_quad[*num_vertices_in_clipped_quad - 1],
+                        new_vertex))
+      return;
+    if (isNearlyTheSame(clipped_quad[0], new_vertex))
+      return;
+  }
   clipped_quad[*num_vertices_in_clipped_quad] = new_vertex;
   (*num_vertices_in_clipped_quad)++;
 }
@@ -935,4 +967,18 @@ ScopedSubnormalFloatDisabler::~ScopedSubnormalFloatDisabler() {
 #endif
 }
 
+bool IsNearlyTheSameForUnitTesting(const float left, const float right) {
+  return isNearlyTheSame(left, right);
+}
+
+bool IsNearlyTheSameForUnitTesting(const gfx::PointF& left,
+                                   const gfx::PointF& right) {
+  return isNearlyTheSame(left, right);
+}
+
+bool IsNearlyTheSameForUnitTesting(const gfx::Point3F& left,
+                                   const gfx::Point3F& right) {
+  return isNearlyTheSame(left, right);
+}
+
 }  // namespace cc