Don't add duplicate points when clipping

Neighbours can clip to the same infinity. Using two of the same
vertex in a polygon can cause the computed normal to be (0,0,0),
which causes badness and failures.

BUG=626095
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2551263002
Cr-Commit-Position: refs/heads/master@{#443296}

cc/base/math_util.cc

@@ -155,9 +155,36 @@ static inline void ExpandBoundsToIncludePoint(float* xmin,
   *ymax = std::max(p.y(), *ymax);
 }
 
+static inline bool IsNearlyTheSame(float f, float g) {
+  // The idea behind this is to use this fraction of the larger of the
+  // two numbers as the limit of the difference.  This breaks down near
+  // zero, so we reuse this as the minimum absolute size we will use
+  // for the base of the scale too.
+  static const float epsilon_scale = 0.00001f;
+  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 &&
+      IsNearlyTheSame(clipped_quad[*num_vertices_in_clipped_quad - 1],
+                      new_vertex))
+    return;
+
   clipped_quad[*num_vertices_in_clipped_quad] = new_vertex;
   (*num_vertices_in_clipped_quad)++;
 }
@@ -165,6 +192,11 @@ 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 &&
+      IsNearlyTheSame(clipped_quad[*num_vertices_in_clipped_quad - 1],
+                      new_vertex))
+    return;
+
   clipped_quad[*num_vertices_in_clipped_quad] = new_vertex;
   (*num_vertices_in_clipped_quad)++;
 }
@@ -345,6 +377,11 @@ void MathUtil::MapClippedQuad(const gfx::Transform& transform,
                            clipped_quad, num_vertices_in_clipped_quad);
   }
 
+  if (*num_vertices_in_clipped_quad > 2 &&
+      IsNearlyTheSame(clipped_quad[0],
+                      clipped_quad[*num_vertices_in_clipped_quad - 1]))
+    *num_vertices_in_clipped_quad -= 1;
+
   DCHECK_LE(*num_vertices_in_clipped_quad, 8);
 }
 
@@ -406,6 +443,11 @@ bool MathUtil::MapClippedQuad3d(const gfx::Transform& transform,
                              clipped_quad, num_vertices_in_clipped_quad);
   }
 
+  if (*num_vertices_in_clipped_quad > 2 &&
+      IsNearlyTheSame(clipped_quad[0],
+                      clipped_quad[*num_vertices_in_clipped_quad - 1]))
+    *num_vertices_in_clipped_quad -= 1;
+
   DCHECK_LE(*num_vertices_in_clipped_quad, 8);
   return (*num_vertices_in_clipped_quad >= 4);
 }
@@ -935,4 +977,18 @@ ScopedSubnormalFloatDisabler::~ScopedSubnormalFloatDisabler() {
 #endif
 }
 
+bool MathUtil::IsNearlyTheSameForTesting(float left, float right) {
+  return IsNearlyTheSame(left, right);
+}
+
+bool MathUtil::IsNearlyTheSameForTesting(const gfx::PointF& left,
+                                         const gfx::PointF& right) {
+  return IsNearlyTheSame(left, right);
+}
+
+bool MathUtil::IsNearlyTheSameForTesting(const gfx::Point3F& left,
+                                         const gfx::Point3F& right) {
+  return IsNearlyTheSame(left, right);
+}
+
 }  // namespace cc

cc/base/math_util.h

@@ -310,6 +310,12 @@ class CC_EXPORT MathUtil {
   // Returns vector that y axis (0,1,0) transforms to under given transform.
   static gfx::Vector3dF GetYAxis(const gfx::Transform& transform);
 
+  static bool IsNearlyTheSameForTesting(float left, float right);
+  static bool IsNearlyTheSameForTesting(const gfx::PointF& l,
+                                        const gfx::PointF& r);
+  static bool IsNearlyTheSameForTesting(const gfx::Point3F& l,
+                                        const gfx::Point3F& r);
+
  private:
   template <typename T>
   static T RoundUpInternal(T n, T mul) {

cc/base/math_util_unittest.cc

@@ -11,6 +11,7 @@
 #include "cc/test/geometry_test_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/gfx/geometry/quad_f.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/rect_f.h"
 #include "ui/gfx/transform.h"
@@ -455,5 +456,176 @@ TEST(MathUtilTest, RoundDownUnderflow) {
   EXPECT_TRUE(MathUtil::VerifyRoundDown<int16_t>(-123, 50));
 }
 
+#define EXPECT_SIMILAR_VALUE(x, y) \
+  EXPECT_TRUE(MathUtil::IsNearlyTheSameForTesting(x, y))
+#define EXPECT_DISSIMILAR_VALUE(x, y) \
+  EXPECT_FALSE(MathUtil::IsNearlyTheSameForTesting(x, y))
+
+// Arbitrary point that shouldn't be different from zero.
+static const float zeroish = 1.0e-11f;
+
+TEST(MathUtilTest, Approximate) {
+  // Same should be similar.
+  EXPECT_SIMILAR_VALUE(1.0f, 1.0f);
+
+  // Zero should not cause similarity issues.
+  EXPECT_SIMILAR_VALUE(0.0f, 0.0f);
+
+  // Chosen sensitivity makes hardware sense, whether small or large.
+  EXPECT_SIMILAR_VALUE(0.0f, std::nextafter(0.0f, 1.0f));
+  EXPECT_SIMILAR_VALUE(1000000.0f, std::nextafter(1000000.0f, 0.0f));
+
+  // Make sure that neither the side you approach, nor the order of
+  // parameters matter at the borderline case.
+  EXPECT_SIMILAR_VALUE(std::nextafter(0.0f, 1.0f), 0.0f);
+  EXPECT_SIMILAR_VALUE(std::nextafter(1000000.0f, 0.0f), 1000000.0f);
+  EXPECT_SIMILAR_VALUE(0.0f, std::nextafter(0.0f, -1.0f));
+  EXPECT_SIMILAR_VALUE(1000000.0f, std::nextafter(1000000.0f, 1e9f));
+  EXPECT_SIMILAR_VALUE(std::nextafter(0.0f, -1.0f), 0.0f);
+  EXPECT_SIMILAR_VALUE(std::nextafter(1000000.0f, 1e9f), 1000000.0f);
+
+  // Double check our arbitrary constant.  Mostly this is for the
+  // following Point tests.
+  EXPECT_SIMILAR_VALUE(0.0f, zeroish);
+
+  // Arbitrary point that is different from one for Approximate tests.
+  EXPECT_SIMILAR_VALUE(1.0f, 1.000001f);
+
+  // Arbitrary (large) difference close to 1.
+  EXPECT_SIMILAR_VALUE(10000000.0f, 10000001.0f);
+
+  // Make sure one side being zero doesn't hide real differences.
+  EXPECT_DISSIMILAR_VALUE(0.0f, 1.0f);
+  EXPECT_DISSIMILAR_VALUE(1.0f, 0.0f);
+
+  // Make sure visible differences don't disappear.
+  EXPECT_DISSIMILAR_VALUE(1.0f, 2.0f);
+  EXPECT_DISSIMILAR_VALUE(10000.0f, 10001.0f);
+}
+
+#define EXPECT_SIMILAR_POINT_F(x, y) \
+  EXPECT_TRUE(MathUtil::IsNearlyTheSameForTesting(gfx::PointF x, gfx::PointF y))
+#define EXPECT_DISSIMILAR_POINT_F(x, y) \
+  EXPECT_FALSE(                         \
+      MathUtil::IsNearlyTheSameForTesting(gfx::PointF x, gfx::PointF y))
+
+TEST(MathUtilTest, ApproximatePointF) {
+  // Same is similar.
+  EXPECT_SIMILAR_POINT_F((0.0f, 0.0f), (0.0f, 0.0f));
+
+  // Not over sensitive on each axis.
+  EXPECT_SIMILAR_POINT_F((zeroish, 0.0f), (0.0f, 0.0f));
+  EXPECT_SIMILAR_POINT_F((0.0f, zeroish), (0.0f, 0.0f));
+  EXPECT_SIMILAR_POINT_F((0.0f, 0.0f), (zeroish, 0.0f));
+  EXPECT_SIMILAR_POINT_F((0.0f, 0.0f), (0.0f, zeroish));
+
+  // Still sensitive to any axis.
+  EXPECT_DISSIMILAR_POINT_F((1.0f, 0.0f), (0.0f, 0.0f));
+  EXPECT_DISSIMILAR_POINT_F((0.0f, 1.0f), (0.0f, 0.0f));
+  EXPECT_DISSIMILAR_POINT_F((0.0f, 0.0f), (1.0f, 0.0f));
+  EXPECT_DISSIMILAR_POINT_F((0.0f, 0.0f), (0.0f, 1.0f));
+
+  // Not crossed over, sensitive on each side of each axis.
+  EXPECT_SIMILAR_POINT_F((0.0f, 1.0f), (0.0f, 1.0f));
+  EXPECT_SIMILAR_POINT_F((1.0f, 2.0f), (1.0f, 2.0f));
+  EXPECT_DISSIMILAR_POINT_F((3.0f, 2.0f), (1.0f, 2.0f));
+  EXPECT_DISSIMILAR_POINT_F((1.0f, 3.0f), (1.0f, 1.0f));
+  EXPECT_DISSIMILAR_POINT_F((1.0f, 2.0f), (3.0f, 2.0f));
+  EXPECT_DISSIMILAR_POINT_F((1.0f, 2.0f), (1.0f, 3.0f));
+}
+
+#define EXPECT_SIMILAR_POINT_3F(x, y) \
+  EXPECT_TRUE(                        \
+      MathUtil::IsNearlyTheSameForTesting(gfx::Point3F x, gfx::Point3F y))
+#define EXPECT_DISSIMILAR_POINT_3F(x, y) \
+  EXPECT_FALSE(                          \
+      MathUtil::IsNearlyTheSameForTesting(gfx::Point3F x, gfx::Point3F y))
+
+TEST(MathUtilTest, ApproximatePoint3F) {
+  // Same same.
+  EXPECT_SIMILAR_POINT_3F((0.0f, 0.0f, 0.0f), (0.0f, 0.0f, 0.0f));
+  EXPECT_SIMILAR_POINT_3F((zeroish, 0.0f, 0.0f), (0.0f, 0.0f, 0.0f));
+  EXPECT_SIMILAR_POINT_3F((0.0f, zeroish, 0.0f), (0.0f, 0.0f, 0.0f));
+  EXPECT_SIMILAR_POINT_3F((0.0f, 0.0f, zeroish), (0.0f, 0.0f, 0.0f));
+  EXPECT_SIMILAR_POINT_3F((0.0f, 0.0f, 0.0f), (zeroish, 0.0f, 0.0f));
+  EXPECT_SIMILAR_POINT_3F((0.0f, 0.0f, 0.0f), (0.0f, zeroish, 0.0f));
+  EXPECT_SIMILAR_POINT_3F((0.0f, 0.0f, 0.0f), (0.0f, 0.0f, zeroish));
+
+  // Not crossed over, sensitive on each side of each axis.
+  EXPECT_SIMILAR_POINT_3F((1.0f, 2.0f, 3.0f), (1.0f, 2.0f, 3.0f));
+  EXPECT_DISSIMILAR_POINT_3F((4.0f, 2.0f, 3.0f), (1.0f, 2.0f, 3.0f));
+  EXPECT_DISSIMILAR_POINT_3F((1.0f, 4.0f, 3.0f), (1.0f, 1.0f, 3.0f));
+  EXPECT_DISSIMILAR_POINT_3F((1.0f, 2.0f, 4.0f), (1.0f, 2.0f, 1.0f));
+  EXPECT_DISSIMILAR_POINT_3F((1.0f, 2.0f, 3.0f), (4.0f, 2.0f, 3.0f));
+  EXPECT_DISSIMILAR_POINT_3F((1.0f, 2.0f, 3.0f), (1.0f, 4.0f, 3.0f));
+  EXPECT_DISSIMILAR_POINT_3F((1.0f, 2.0f, 3.0f), (1.0f, 2.0f, 4.0f));
+}
+
+// This takes a quad for which two points, (at x = -99) are behind and below
+// the eyepoint and checks to make sure we build a triangle.  We used to build
+// a degenerate quad.
+TEST(MathUtilTest, MapClippedQuadDuplicateTriangle) {
+  gfx::Transform transform;
+  transform.MakeIdentity();
+  transform.ApplyPerspectiveDepth(50.0);
+  transform.RotateAboutYAxis(89.0);
+  // We are amost looking along the X-Y plane from (-50, almost 0)
+
+  gfx::QuadF src_quad(gfx::PointF(0.0f, 100.0f), gfx::PointF(0.0f, -100.0f),
+                      gfx::PointF(-99.0f, -300.0f),
+                      gfx::PointF(-99.0f, -100.0f));
+
+  gfx::PointF clipped_quad[8];
+  int num_vertices_in_clipped_quad;
+
+  MathUtil::MapClippedQuad(transform, src_quad, clipped_quad,
+                           &num_vertices_in_clipped_quad);
+
+  EXPECT_EQ(num_vertices_in_clipped_quad, 3);
+}
+
+// This takes a quad for which two points, (at x = -99) are behind and below
+// the eyepoint and checks to make sure we build a triangle.  We used to build
+// a degenerate quad.  The quirk here is that the two shared points are first
+// and last, not sequential.
+TEST(MathUtilTest, MapClippedQuadDuplicateTriangleWrapped) {
+  gfx::Transform transform;
+  transform.MakeIdentity();
+  transform.ApplyPerspectiveDepth(50.0);
+  transform.RotateAboutYAxis(89.0);
+
+  gfx::QuadF src_quad(gfx::PointF(-99.0f, -100.0f), gfx::PointF(0.0f, 100.0f),
+                      gfx::PointF(0.0f, -100.0f), gfx::PointF(-99.0f, -300.0f));
+
+  gfx::PointF clipped_quad[8];
+  int num_vertices_in_clipped_quad;
+
+  MathUtil::MapClippedQuad(transform, src_quad, clipped_quad,
+                           &num_vertices_in_clipped_quad);
+
+  EXPECT_EQ(num_vertices_in_clipped_quad, 3);
+}
+
+// Here we map and clip a quad with only one point that disappears to infinity
+// behind us.  We don't want two vertices at infinity crossing in and out
+// of w < 0 space.
+TEST(MathUtilTest, MapClippedQuadDuplicateQuad) {
+  gfx::Transform transform;
+  transform.MakeIdentity();
+  transform.ApplyPerspectiveDepth(50.0);
+  transform.RotateAboutYAxis(89.0);
+
+  gfx::QuadF src_quad(gfx::PointF(0.0f, 100.0f), gfx::PointF(400.0f, 0.0f),
+                      gfx::PointF(0.0f, -100.0f), gfx::PointF(-99.0f, -300.0f));
+
+  gfx::PointF clipped_quad[8];
+  int num_vertices_in_clipped_quad;
+
+  MathUtil::MapClippedQuad(transform, src_quad, clipped_quad,
+                           &num_vertices_in_clipped_quad);
+
+  EXPECT_EQ(num_vertices_in_clipped_quad, 4);
+}
+
 }  // namespace
 }  // namespace cc