Handle large rects better.

ui/gfx/geometry/rect_unittest.cc

Index: ui/gfx/geometry/rect_unittest.cc
diff --git a/ui/gfx/geometry/rect_unittest.cc b/ui/gfx/geometry/rect_unittest.cc
index 63b2d52c6bdef6b0478f4622c81f4a046fd22475..7a96466cdfadb2d1e257a54ef2f2752165cfa5ba 100644
--- a/ui/gfx/geometry/rect_unittest.cc
+++ b/ui/gfx/geometry/rect_unittest.cc
@@ -425,6 +425,14 @@ TEST(RectTest, SharesEdgeWith) {
 #define EXPECT_FLOAT_AND_NAN_EQ(a, b) \
   { if (a == a || b == b) { EXPECT_FLOAT_EQ(a, b); } }
 
+static const int dont_care = -451329;
+#define MAYBE_EXPECT_INT_EQ(a, b) \

[danakj, 2017/03/28 20:40:35]

This is pretty magical, instead of this, can you just break these cases out of
the for loops and don't check the things that don't matter (maybe with a comment
why)?

[Peter Mayo, 2017/03/29 18:49:26]

Done. Increased the structure of the tests while I was at it.

+  {                               \
+    if (a != dont_care) {         \
+      EXPECT_EQ(a, b);            \
+    }                             \
+  }
+
 TEST(RectTest, ScaleRect) {
   static const struct Test {
     int x1;  // source
@@ -482,38 +490,28 @@ TEST(RectTest, ToEnclosedRect) {
     int y2;
     int w2;
     int h2;
-  } tests[] = {{0.0f, 0.0f, 0.0f, 0.0f, 0, 0, 0, 0},
-               {-1.5f, -1.5f, 3.0f, 3.0f, -1, -1, 2, 2},
-               {-1.5f, -1.5f, 3.5f, 3.5f, -1, -1, 3, 3},
-               {max_float, max_float, 2.0f, 2.0f, max_int, max_int, 0, 0},
-               {0.0f, 0.0f, max_float, max_float, 0, 0, max_int, max_int},
-               {20000.5f, 20000.5f, 0.5f, 0.5f, 20001, 20001, 0, 0},
-               {static_cast<float>(min_int),
-                static_cast<float>(min_int),
-                max_int * 2.f,
-                max_int * 2.f,
-                min_int,
-                min_int,
-                max_int,
-                max_int},
-               {static_cast<float>(max_int),
-                static_cast<float>(max_int),
-                static_cast<float>(max_int),
-                static_cast<float>(max_int),
-                max_int,
-                max_int,
-                0,
-                0}};
+  } tests[] = {
+      {0.0f, 0.0f, 0.0f, 0.0f, 0, 0, 0, 0},
+      {-1.5f, -1.5f, 3.0f, 3.0f, -1, -1, 2, 2},
+      {-1.5f, -1.5f, 3.5f, 3.5f, -1, -1, 3, 3},
+      {max_float, max_float, 2.0f, 2.0f, max_int, max_int, 0, 0},
+      {0.0f, 0.0f, max_float, max_float, 0, 0, max_int, max_int},
+      {20000.5f, 20000.5f, 0.5f, 0.5f, 20001, 20001, 0, 0},
+      {static_cast<float>(min_int), static_cast<float>(min_int), max_int * 2.f,
+       max_int * 2.f, dont_care, dont_care, max_int, max_int},
+      {static_cast<float>(max_int), static_cast<float>(max_int),
+       static_cast<float>(max_int), static_cast<float>(max_int), max_int,
+       max_int, 0, 0}};
 
   for (size_t i = 0; i < arraysize(tests); ++i) {
     RectF r1(tests[i].x1, tests[i].y1, tests[i].w1, tests[i].h1);
     Rect r2(tests[i].x2, tests[i].y2, tests[i].w2, tests[i].h2);
 
     Rect enclosed = ToEnclosedRect(r1);
-    EXPECT_FLOAT_AND_NAN_EQ(r2.x(), enclosed.x());
-    EXPECT_FLOAT_AND_NAN_EQ(r2.y(), enclosed.y());
-    EXPECT_FLOAT_AND_NAN_EQ(r2.width(), enclosed.width());
-    EXPECT_FLOAT_AND_NAN_EQ(r2.height(), enclosed.height());
+    MAYBE_EXPECT_INT_EQ(r2.x(), enclosed.x());
+    MAYBE_EXPECT_INT_EQ(r2.y(), enclosed.y());
+    MAYBE_EXPECT_INT_EQ(r2.width(), enclosed.width());
+    MAYBE_EXPECT_INT_EQ(r2.height(), enclosed.height());
   }
 }
 
@@ -535,13 +533,14 @@ TEST(RectTest, ToEnclosingRect) {
       {0.0f, 0.0f, 0.0f, 0.0f, 0, 0, 0, 0},
       {5.5f, 5.5f, 0.0f, 0.0f, 5, 5, 0, 0},
       {3.5f, 2.5f, epsilon_float, -0.0f, 3, 2, 0, 0},
+      {3.5f, 2.5f, 0.f, 0.001f, 3, 2, 0, 1},
       {-1.5f, -1.5f, 3.0f, 3.0f, -2, -2, 4, 4},
       {-1.5f, -1.5f, 3.5f, 3.5f, -2, -2, 4, 4},
       {max_float, max_float, 2.0f, 2.0f, max_int, max_int, 0, 0},
       {0.0f, 0.0f, max_float, max_float, 0, 0, max_int, max_int},
       {20000.5f, 20000.5f, 0.5f, 0.5f, 20000, 20000, 1, 1},
       {static_cast<float>(min_int), static_cast<float>(min_int), max_int * 2.f,
-       max_int * 2.f, min_int, min_int, max_int, max_int},
+       max_int * 2.f, dont_care, dont_care, max_int, max_int},
       {static_cast<float>(max_int), static_cast<float>(max_int),
        static_cast<float>(max_int), static_cast<float>(max_int), max_int,
        max_int, 0, 0},
@@ -552,10 +551,10 @@ TEST(RectTest, ToEnclosingRect) {
     Rect r2(tests[i].x2, tests[i].y2, tests[i].w2, tests[i].h2);
 
     Rect enclosed = ToEnclosingRect(r1);
-    EXPECT_EQ(r2.x(), enclosed.x());
-    EXPECT_EQ(r2.y(), enclosed.y());
-    EXPECT_EQ(r2.width(), enclosed.width());
-    EXPECT_EQ(r2.height(), enclosed.height());
+    MAYBE_EXPECT_INT_EQ(r2.x(), enclosed.x());
+    MAYBE_EXPECT_INT_EQ(r2.y(), enclosed.y());
+    MAYBE_EXPECT_INT_EQ(r2.width(), enclosed.width());
+    MAYBE_EXPECT_INT_EQ(r2.height(), enclosed.height());
   }
 }
 
@@ -1048,13 +1047,14 @@ TEST(RectTest, IntegerOverflow) {
   }
 
   // Unioning a left=minint rect with a right=maxint rect.
-  // Width is always clamped before adjusting position, so this
-  // should taking the min left/top and then finding the max width.
+  // We can't represent both ends of the spectrum in the same rect.
+  // Make sure we keep the most useful area.
   {
+    int part_limit = min_limit / 3;
     Rect left_minint(min_limit, min_limit, 1, 1);
     Rect right_maxint(limit - 1, limit - 1, limit, limit);
-    Rect expected(min_limit, min_limit, limit, limit);
-    EXPECT_EQ(UnionRects(left_minint, right_maxint), expected);
+    Rect expected(part_limit, part_limit, 2 * part_limit, 2 * part_limit);
+    EXPECT_TRUE(UnionRects(left_minint, right_maxint).Contains(expected));

[danakj, 2017/03/28 20:40:35]

Can you (also?) verify the width and height exactly, and the x and y are within
some range?

[Peter Mayo, 2017/03/29 18:49:26]

Done.

   }
 }