Don't add duplicate points when clipping

cc/base/math_util.cc

 // Copyright 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 "cc/base/math_util.h"
 
 #include <algorithm>
 #include <cmath>
 #include <limits>
 #ifdef __SSE__
@@ skipping 137 matching lines @@
                                               float* xmax,
                                               float* ymin,
                                               float* ymax,
                                               const gfx::PointF& p) {
   *xmin = std::min(p.x(), *xmin);
   *xmax = std::max(p.x(), *xmax);
   *ymin = std::min(p.y(), *ymin);
   *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)++;
 }
 
 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)++;
 }
 
 gfx::Rect MathUtil::MapEnclosingClippedRect(const gfx::Transform& transform,
                                             const gfx::Rect& src_rect) {
   if (transform.IsIdentityOrIntegerTranslation()) {
     gfx::Vector2d offset(static_cast<int>(transform.matrix().getFloat(0, 3)),
                          static_cast<int>(transform.matrix().getFloat(1, 3)));
     return src_rect + offset;
@@ skipping 102 matching lines @@
   HomogeneousCoordinate hc0(result[0], result[1], result[2], result[3]);
   HomogeneousCoordinate hc1(result[4], result[5], result[6], result[7]);
   DCHECK(!hc0.ShouldBeClipped());
   DCHECK(!hc1.ShouldBeClipped());
 
   gfx::PointF top_left(hc0.CartesianPoint2d());
   gfx::PointF bottom_right(hc1.CartesianPoint2d());
   return gfx::ToEnclosedRect(gfx::BoundingRect(top_left, bottom_right));
 }
 
 void MathUtil::MapClippedQuad(const gfx::Transform& transform,

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

It's interesting that there seem to be no calls to the 2d version of
MapClippedQuad. Seems like we should maybe remove it.

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

A fine idea for another CL.  I have a few more for this file.

                               const gfx::QuadF& src_quad,
                               gfx::PointF clipped_quad[8],
                               int* num_vertices_in_clipped_quad) {
   HomogeneousCoordinate h1 =
       MapHomogeneousPoint(transform, gfx::Point3F(src_quad.p1()));
   HomogeneousCoordinate h2 =
       MapHomogeneousPoint(transform, gfx::Point3F(src_quad.p2()));
   HomogeneousCoordinate h3 =
       MapHomogeneousPoint(transform, gfx::Point3F(src_quad.p3()));
   HomogeneousCoordinate h4 =
@@ skipping 99 matching lines @@
     AddVertexToClippedQuad3d(
         h4.CartesianPoint3d(), clipped_quad, num_vertices_in_clipped_quad);
   }
 
   if (h4.ShouldBeClipped() ^ h1.ShouldBeClipped()) {
     AddVertexToClippedQuad3d(ComputeClippedCartesianPoint3dForEdge(h4, h1),
                              clipped_quad, num_vertices_in_clipped_quad);
   }
 
   DCHECK_LE(*num_vertices_in_clipped_quad, 8);
   return (*num_vertices_in_clipped_quad >= 4);

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

It seems like noone uses this return value and we exclude polygons with fewer
than 3 points in direct_renderer.cc. Can we change this to void?

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

A fine idea for another CL.  I have a few more for this file.

 }
 
 gfx::RectF MathUtil::ComputeEnclosingRectOfVertices(
     const gfx::PointF vertices[],
     int num_vertices) {
   if (num_vertices < 2)
     return gfx::RectF();
 
   float xmin = std::numeric_limits<float>::max();
   float xmax = -std::numeric_limits<float>::max();
@@ skipping 507 matching lines @@
   _mm_setcsr(orig_state_ | 0x8040);
 #endif
 }
 
 ScopedSubnormalFloatDisabler::~ScopedSubnormalFloatDisabler() {
 #ifdef __SSE__
   _mm_setcsr(orig_state_);
 #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