Clamp projected coordinates to large but finite values

Bug: 224618
Change-Id: If7edeef0cc8b7970599388150c6d955171f555ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2050665Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Cr-Commit-Position: refs/heads/master@{#741118}

cc/base/math_util.cc

@@ -72,17 +72,14 @@ static void homogenousLimitAtZero(SkScalar a1,
                                   float* limit) {
   // This is the tolerance for detecting an eyepoint-aligned edge.
   static const float kStationaryPointEplison = 0.00001f;
-  // This needs to be big enough to not be the limit of clipping, but not so
-  // big that using it as a size destroys the offset in a rect.
-  static const float kInfiniteCoordinate = 1000000.0f;
 
   if (std::abs(a1 * w2 / w1 / a2 - 1.0f) > kStationaryPointEplison) {
     // We are going to explode towards an infity, but we choose the one that
     // corresponds to the one on the positive side of w.
     if (((1.0f - t) * a1 + t * a2) > 0) {
-      *limit = kInfiniteCoordinate;
+      *limit = HomogeneousCoordinate::kInfiniteCoordinate;
     } else {
-      *limit = -kInfiniteCoordinate;
+      *limit = -HomogeneousCoordinate::kInfiniteCoordinate;
     }
   } else {
     *limit = a1 / w1;  // (== a2 / w2) && == (1.0f - t) * a1 / w1 + t * a2 / w2

cc/base/math_util.h

@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "base/logging.h"
+#include "base/numerics/ranges.h"
 #include "build/build_config.h"
 #include "cc/base/base_export.h"
 #include "ui/gfx/geometry/box_f.h"
@@ -41,6 +42,14 @@ class Vector3dF;
 namespace cc {
 
 struct HomogeneousCoordinate {
+  // This needs to be big enough that it does not incorrectly clip the projected
+  // coordinate. For local to device projection, this must be bigger than the
+  // expected size of a display. For inverse projection, this is hopefully
+  // larger than the required local layer size of page content. If it is made
+  // too big then bounding box calculations based on projected coordinates can
+  // lose precision and lead to incorrect page rendering.
+  static constexpr float kInfiniteCoordinate = 1000000.0f;
+
   HomogeneousCoordinate(SkScalar x, SkScalar y, SkScalar z, SkScalar w) {
     vec[0] = x;
     vec[1] = y;
@@ -58,7 +67,12 @@ struct HomogeneousCoordinate {
     // never be called when w == 0, and we do not yet need to handle that case.
     DCHECK(w());
     SkScalar inv_w = SK_Scalar1 / w();
-    return gfx::PointF(x() * inv_w, y() * inv_w);
+    // However, w may be close to 0 and we lose precision on our geometry
+    // calculations if we allow scaling to extremely large values.
+    return gfx::PointF(base::ClampToRange(x() * inv_w, -kInfiniteCoordinate,
+                                          (float)kInfiniteCoordinate),
+                       base::ClampToRange(y() * inv_w, -kInfiniteCoordinate,
+                                          (float)kInfiniteCoordinate));
   }
 
   gfx::Point3F CartesianPoint3d() const {
@@ -69,7 +83,14 @@ struct HomogeneousCoordinate {
     // never be called when w == 0, and we do not yet need to handle that case.
     DCHECK(w());
     SkScalar inv_w = SK_Scalar1 / w();
-    return gfx::Point3F(x() * inv_w, y() * inv_w, z() * inv_w);
+    // However, w may be close to 0 and we lose precision on our geometry
+    // calculations if we allow scaling to extremely large values.
+    return gfx::Point3F(base::ClampToRange(x() * inv_w, -kInfiniteCoordinate,
+                                           (float)kInfiniteCoordinate),
+                        base::ClampToRange(y() * inv_w, -kInfiniteCoordinate,
+                                           (float)kInfiniteCoordinate),
+                        base::ClampToRange(z() * inv_w, -kInfiniteCoordinate,
+                                           (float)kInfiniteCoordinate));
   }
 
   SkScalar x() const { return vec[0]; }

cc/base/math_util_unittest.cc

@@ -148,6 +148,42 @@ TEST(MathUtilTest, EnclosingClippedRectUsesCorrectInitialBounds) {
       gfx::RectF(gfx::PointF(-100, -100), gfx::SizeF(90, 90)), result, 0.15f);
 }
 
+TEST(MathUtilTest, EnclosingClippedRectHandlesSmallPositiveW) {
+  // When all homogeneous coordinates have w > 0, no clipping against the w = 0
+  // plane is performed and the projected points are sent to gfx::QuadF's
+  // bounding box function. w can be made arbitrarily close to 0 on the positive
+  // side and cause precision problems later on unless it's handled properly.
+
+  // Coordinates inspired by a real test page. One edge maps to approximately
+  // negative infinity, and the other is at x~109.
+  HomogeneousCoordinate h1(-154.0f, -109.0f, 0.0f, 6e-8f);
+  HomogeneousCoordinate h2(152.0f, 44.0f, 0.0f, 1.4f);
+  HomogeneousCoordinate h3(152.0f, 261.0f, 0.0f, 1.4f);
+  HomogeneousCoordinate h4(-154.0f, 108.0f, 0.0f, 6e-8f);
+
+  // Confirm original behavior is problematic if we just divide by w.
+  gfx::QuadF naiveQuad = {{h1.x() / h1.w(), h1.y() / h1.w()},
+                          {h2.x() / h2.w(), h2.y() / h2.w()},
+                          {h3.x() / h3.w(), h3.y() / h3.w()},
+                          {h4.x() / h4.w(), h4.y() / h4.w()}};
+  // The calculated min and max coordinates differ by ~2^31, well outside a
+  // floats ability to represent onscreen pixel coordinates and in this case,
+  // the projected bounds fail to represent that one edge is still on screen.
+  gfx::RectF naiveBounds = naiveQuad.BoundingBox();
+  EXPECT_TRUE(naiveBounds.right() <= 0.0f);
+
+  // The bounds of the enclosing clipped rect should be neg. infinity to ~109
+  // for x, and neg. infinity to pos. infinity for y.
+  gfx::RectF goodBounds = MathUtil::ComputeEnclosingClippedRect(h1, h2, h3, h4);
+  EXPECT_FALSE(goodBounds.IsEmpty());
+  EXPECT_FLOAT_EQ(-HomogeneousCoordinate::kInfiniteCoordinate, goodBounds.y());
+  EXPECT_FLOAT_EQ(HomogeneousCoordinate::kInfiniteCoordinate,
+                  goodBounds.bottom());
+  EXPECT_FLOAT_EQ(-HomogeneousCoordinate::kInfiniteCoordinate, goodBounds.x());
+  // 0.01f was empirically determined.
+  EXPECT_NEAR(152.0f / 1.4f, goodBounds.right(), 0.01f);
+}
+
 TEST(MathUtilTest, EnclosingRectOfVerticesUsesCorrectInitialBounds) {
   gfx::PointF vertices[3];
   int num_vertices = 3;