[PE] Ignore floating point error for clip rects

A clip rect mapped through multiple transform spaces can be
(1.99999, 2.00001, 10.999998, 20.000002). Previously we use
ToEnclosingRect to convert it to an integral rect and will result
a rect that has 1 pixel bigger in some directions, causing
incorrect clipping result.

Now add gfx::ToEnclosingIgnoringError to convert a near integral
gfx::RectF to the nearest integral gfx::Rect.

Bug: 879173
Cq-Include-Trybots: luci.chromium.try:linux-blink-gen-property-trees;luci.chromium.try:linux_layout_tests_slimming_paint_v2;luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Id7ab68c04f8f1efed1dfe1810b82dd24d1a41ea5
Reviewed-on: https://chromium-review.googlesource.com/1252143Reviewed-by: Ian Vollick <vollick@chromium.org>
Reviewed-by: Ali Juma <ajuma@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595947}

cc/trees/draw_property_utils.cc

@@ -29,6 +29,11 @@ namespace draw_property_utils {
 
 namespace {
 
+static gfx::Rect ToEnclosingClipRect(const gfx::RectF& clip_rect) {
+  constexpr float kClipError = 0.00001f;
+  return gfx::ToEnclosingRectIgnoringError(clip_rect, kClipError);
+}
+
 static bool IsRootLayer(const Layer* layer) {
   return !layer->parent();
 }
@@ -129,10 +134,11 @@ static ConditionalClip ComputeCurrentClip(const ClipNode* clip_node,
                                           const PropertyTrees* property_trees,
                                           int target_transform_id,
                                           int target_effect_id) {
-  if (clip_node->transform_id != target_transform_id)
+  if (clip_node->transform_id != target_transform_id) {
     return ComputeLocalRectInTargetSpace(clip_node->clip, property_trees,
                                          clip_node->transform_id,
                                          target_transform_id, target_effect_id);
+  }
 
   const EffectTree& effect_tree = property_trees->effect_tree;
   gfx::RectF current_clip = clip_node->clip;
@@ -183,7 +189,7 @@ static bool ApplyClipNodeToAccumulatedClip(const PropertyTrees* property_trees,
       // Do the expansion.
       gfx::RectF expanded_clip_in_expanding_space =
           gfx::RectF(clip_node->clip_expander->MapRectReverse(
-              gfx::ToEnclosingRect(accumulated_clip_rect_in_expanding_space),
+              ToEnclosingClipRect(accumulated_clip_rect_in_expanding_space),
               property_trees));
 
       // Put the expanded clip back into the original target space.
@@ -557,13 +563,13 @@ static void SetSurfaceClipRect(const ClipNode* parent_clip_node,
   bool include_expanding_clips = false;
   if (render_surface->EffectTreeIndex() == EffectTree::kContentsRootNodeId) {
     render_surface->SetClipRect(
-        gfx::ToEnclosingRect(clip_tree.Node(effect_node->clip_id)->clip));
+        ToEnclosingClipRect(clip_tree.Node(effect_node->clip_id)->clip));
   } else {
     ConditionalClip accumulated_clip_rect =
         ComputeAccumulatedClip(property_trees, include_expanding_clips,
                                effect_node->clip_id, target_node->id);
     render_surface->SetClipRect(
-        gfx::ToEnclosingRect(accumulated_clip_rect.clip_rect));
+        ToEnclosingClipRect(accumulated_clip_rect.clip_rect));
   }
 }
 
@@ -641,7 +647,7 @@ static gfx::Rect LayerVisibleRect(PropertyTrees* property_trees,
   gfx::RectF clip_in_layer_space = accumulated_clip_in_layer_space.clip_rect;
   clip_in_layer_space.Offset(-layer->offset_to_transform_parent());
 
-  gfx::Rect visible_rect = gfx::ToEnclosingRect(clip_in_layer_space);
+  gfx::Rect visible_rect = ToEnclosingClipRect(clip_in_layer_space);
   visible_rect.Intersect(layer_content_rect);
   return visible_rect;
 }
@@ -916,7 +922,7 @@ void ComputeDrawPropertiesOfVisibleLayers(const LayerImplList* layer_list,
     // is_clipped should be set before visible rect computation as it is used
     // there.
     layer->draw_properties().is_clipped = clip.is_clipped;
-    layer->draw_properties().clip_rect = gfx::ToEnclosingRect(clip.clip_rect);
+    layer->draw_properties().clip_rect = ToEnclosingClipRect(clip.clip_rect);
     layer->draw_properties().visible_layer_rect =
         LayerVisibleRect(property_trees, layer);
   }
@@ -949,8 +955,7 @@ void ComputeMaskDrawProperties(LayerImpl* mask_layer,
   // is_clipped should be set before visible rect computation as it is used
   // there.
   mask_layer->draw_properties().is_clipped = clip.is_clipped;
-  mask_layer->draw_properties().clip_rect =
-      gfx::ToEnclosingRect(clip.clip_rect);
+  mask_layer->draw_properties().clip_rect = ToEnclosingClipRect(clip.clip_rect);
   // Calculate actual visible layer rect for mask layers, since we could have
   // tiled mask layers and the tile manager would need this info for rastering.
   mask_layer->draw_properties().visible_layer_rect =

third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-gen-property-trees

@@ -201,10 +201,6 @@ Bug(none) virtual/prefer_compositing_to_lcd_text/compositing/overflow/tiled-mask
 Bug(none) virtual/prefer_compositing_to_lcd_text/compositing/overflow/scaled-overflow.html [ Failure ]
 Bug(none) compositing/overflow/scaled-overflow.html [ Failure ]
 
-# Incorrect clipping.
-crbug.com/879173 compositing/overflow/nested-render-surfaces-with-rotation.html [ Failure ]
-crbug.com/879173 virtual/prefer_compositing_to_lcd_text/compositing/overflow/nested-render-surfaces-with-rotation.html [ Failure ]
-
 # Incorrect scrollbar invalidation.
 crbug.com/887000 virtual/prefer_compositing_to_lcd_text/scrollbars/hidden-scrollbars-invisible.html [ Failure ]
 

ui/gfx/geometry/rect_conversions.cc

@@ -12,6 +12,20 @@
 
 namespace gfx {
 
+namespace {
+
+int ToFlooredIntIgnoringError(float f, float error) {
+  int rounded = ToRoundedInt(f);
+  return std::abs(rounded - f) < error ? rounded : ToFlooredInt(f);
+}
+
+int ToCeiledIntIgnoringError(float f, float error) {
+  int rounded = ToRoundedInt(f);
+  return std::abs(rounded - f) < error ? rounded : ToCeiledInt(f);
+}
+
+}  // anonymous namespace
+
 Rect ToEnclosingRect(const RectF& r) {
   int left = ToFlooredInt(r.x());
   int right = r.width() ? ToCeiledInt(r.right()) : left;
@@ -23,6 +37,17 @@ Rect ToEnclosingRect(const RectF& r) {
   return result;
 }
 
+Rect ToEnclosingRectIgnoringError(const RectF& r, float error) {
+  int left = ToFlooredIntIgnoringError(r.x(), error);
+  int right = r.width() ? ToCeiledIntIgnoringError(r.right(), error) : left;
+  int top = ToFlooredIntIgnoringError(r.y(), error);
+  int bottom = r.height() ? ToCeiledIntIgnoringError(r.bottom(), error) : top;
+
+  Rect result;
+  result.SetByBounds(left, top, right, bottom);
+  return result;
+}
+
 Rect ToEnclosedRect(const RectF& rect) {
   Rect result;
   result.SetByBounds(ToCeiledInt(rect.x()), ToCeiledInt(rect.y()),

ui/gfx/geometry/rect_conversions.h

@@ -13,6 +13,13 @@ namespace gfx {
 // Returns the smallest Rect that encloses the given RectF.
 GFX_EXPORT Rect ToEnclosingRect(const RectF& rect);
 
+// Similar to ToEnclosingRect(), but for each edge, if the distance between the
+// edge and the nearest integer grid is smaller than |error|, the edge is
+// snapped to the integer grid. Unlike ToNearestRect() which only accepts
+// integer rect with or without floating point error, this function also accepts
+// non-integer rect.
+GFX_EXPORT Rect ToEnclosingRectIgnoringError(const RectF& rect, float error);
+
 // Returns the largest Rect that is enclosed by the given RectF.
 GFX_EXPORT Rect ToEnclosedRect(const RectF& rect);
 

ui/gfx/geometry/rect_unittest.cc

@@ -6,7 +6,7 @@
 
 #include <stddef.h>
 
-#include "base/macros.h"
+#include "base/stl_util.h"
 #include "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/geometry/rect.h"
@@ -40,7 +40,7 @@ TEST(RectTest, Contains) {
     {0, 0, -10, -10, 0, 0, false},
   #endif
   };
-  for (size_t i = 0; i < arraysize(contains_cases); ++i) {
+  for (size_t i = 0; i < base::size(contains_cases); ++i) {
     const ContainsCase& value = contains_cases[i];
     Rect rect(value.rect_x, value.rect_y, value.rect_width, value.rect_height);
     EXPECT_EQ(value.contained, rect.Contains(value.point_x, value.point_y));
@@ -70,7 +70,7 @@ TEST(RectTest, Intersects) {
     { 10, 10, 10, 10, 20, 15, 10, 10, false },
     { 10, 10, 10, 10, 21, 15, 10, 10, false }
   };
-  for (size_t i = 0; i < arraysize(tests); ++i) {
+  for (size_t i = 0; i < base::size(tests); ++i) {
     Rect 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);
     EXPECT_EQ(tests[i].intersects, r1.Intersects(r2));
@@ -112,7 +112,7 @@ TEST(RectTest, Intersect) {
       0, 0, 2, 2,
       0, 0, 0, 0 }
   };
-  for (size_t i = 0; i < arraysize(tests); ++i) {
+  for (size_t i = 0; i < base::size(tests); ++i) {
     Rect 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 r3(tests[i].x3, tests[i].y3, tests[i].w3, tests[i].h3);
@@ -161,7 +161,7 @@ TEST(RectTest, Union) {
       2, 2, 2, 2,
       2, 2, 2, 2 }
   };
-  for (size_t i = 0; i < arraysize(tests); ++i) {
+  for (size_t i = 0; i < base::size(tests); ++i) {
     Rect 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 r3(tests[i].x3, tests[i].y3, tests[i].w3, tests[i].h3);
@@ -213,7 +213,7 @@ TEST(RectTest, AdjustToFit) {
       0, 0, 3, 3,
       2, 2, 1, 1 }
   };
-  for (size_t i = 0; i < arraysize(tests); ++i) {
+  for (size_t i = 0; i < base::size(tests); ++i) {
     Rect 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 r3(tests[i].x3, tests[i].y3, tests[i].w3, tests[i].h3);
@@ -457,7 +457,7 @@ TEST(RectTest, ScaleRect) {
       std::numeric_limits<float>::max() }
   };
 
-  for (size_t i = 0; i < arraysize(tests); ++i) {
+  for (size_t i = 0; i < base::size(tests); ++i) {
     RectF r1(tests[i].x1, tests[i].y1, tests[i].w1, tests[i].h1);
     RectF r2(tests[i].x2, tests[i].y2, tests[i].w2, tests[i].h2);
 
@@ -496,9 +496,12 @@ TEST(RectTest, ToEnclosedRect) {
       {{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}},
-      {{max_int_f, max_int_f, max_int_f, max_int_f}, {max_int, max_int, 0, 0}}};
+      {{max_int_f, max_int_f, max_int_f, max_int_f}, {max_int, max_int, 0, 0}},
+      {{1.9999f, 2.0002f, 5.9998f, 6.0001f}, {2, 3, 5, 5}},
+      {{1.9999f, 2.0001f, 6.0002f, 5.9998f}, {2, 3, 6, 4}},
+      {{1.9998f, 2.0002f, 6.0001f, 5.9999f}, {2, 3, 5, 5}}};
 
-  for (size_t i = 0; i < arraysize(tests); ++i) {
+  for (size_t i = 0; i < base::size(tests); ++i) {
     RectF source(tests[i].in.x, tests[i].in.y, tests[i].in.width,
                  tests[i].in.height);
     Rect enclosed = ToEnclosedRect(source);
@@ -556,9 +559,12 @@ TEST(RectTest, ToEnclosingRect) {
       {{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}},
       {{max_int_f, max_int_f, max_int_f, max_int_f}, {max_int, max_int, 0, 0}},
-      {{-0.5f, -0.5f, 22777712.f, 1.f}, {-1, -1, 22777713, 2}}};
+      {{-0.5f, -0.5f, 22777712.f, 1.f}, {-1, -1, 22777713, 2}},
+      {{1.9999f, 2.0002f, 5.9998f, 6.0001f}, {1, 2, 7, 7}},
+      {{1.9999f, 2.0001f, 6.0002f, 5.9998f}, {1, 2, 8, 6}},
+      {{1.9998f, 2.0002f, 6.0001f, 5.9999f}, {1, 2, 7, 7}}};
 
-  for (size_t i = 0; i < arraysize(tests); ++i) {
+  for (size_t i = 0; i < base::size(tests); ++i) {
     RectF source(tests[i].in.x, tests[i].in.y, tests[i].in.width,
                  tests[i].in.height);
 
@@ -585,6 +591,53 @@ TEST(RectTest, ToEnclosingRect) {
   }
 }
 
+TEST(RectTest, ToEnclosingRectIgnoringError) {
+  static const int max_int = std::numeric_limits<int>::max();
+  static const float max_float = std::numeric_limits<float>::max();
+  static const float epsilon_float = std::numeric_limits<float>::epsilon();
+  static const float max_int_f = static_cast<float>(max_int);
+  static const float error = 0.001f;
+  static const struct Test {
+    struct {
+      float x;
+      float y;
+      float width;
+      float height;
+    } in;
+    struct {
+      int x;
+      int y;
+      int width;
+      int height;
+    } expected;
+  } tests[] = {
+      {{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}},
+      {{max_int_f, max_int_f, max_int_f, max_int_f}, {max_int, max_int, 0, 0}},
+      {{-0.5f, -0.5f, 22777712.f, 1.f}, {-1, -1, 22777713, 2}},
+      {{1.9999f, 2.0002f, 5.9998f, 6.0001f}, {2, 2, 6, 6}},
+      {{1.9999f, 2.0001f, 6.0002f, 5.9998f}, {2, 2, 6, 6}},
+      {{1.9998f, 2.0002f, 6.0001f, 5.9999f}, {2, 2, 6, 6}}};
+
+  for (size_t i = 0; i < base::size(tests); ++i) {
+    RectF source(tests[i].in.x, tests[i].in.y, tests[i].in.width,
+                 tests[i].in.height);
+
+    Rect enclosing = ToEnclosingRectIgnoringError(source, error);
+    EXPECT_EQ(tests[i].expected.x, enclosing.x());
+    EXPECT_EQ(tests[i].expected.y, enclosing.y());
+    EXPECT_EQ(tests[i].expected.width, enclosing.width());
+    EXPECT_EQ(tests[i].expected.height, enclosing.height());
+  }
+}
+
 TEST(RectTest, ToNearestRect) {
   Rect rect;
   EXPECT_EQ(rect, ToNearestRect(RectF(rect)));
@@ -617,7 +670,7 @@ TEST(RectTest, ToFlooredRect) {
       20000, 20000, 0, 0 },
   };
 
-  for (size_t i = 0; i < arraysize(tests); ++i) {
+  for (size_t i = 0; i < base::size(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);
 
@@ -666,7 +719,7 @@ TEST(RectTest, ScaleToEnclosedRect) {
     }
   };
 
-  for (size_t i = 0; i < arraysize(tests); ++i) {
+  for (size_t i = 0; i < base::size(tests); ++i) {
     Rect result = ScaleToEnclosedRect(tests[i].input_rect,
                                       tests[i].input_scale);
     EXPECT_EQ(tests[i].expected_rect, result);
@@ -710,7 +763,7 @@ TEST(RectTest, ScaleToEnclosingRect) {
     }
   };
 
-  for (size_t i = 0; i < arraysize(tests); ++i) {
+  for (size_t i = 0; i < base::size(tests); ++i) {
     Rect result =
         ScaleToEnclosingRect(tests[i].input_rect, tests[i].input_scale);
     EXPECT_EQ(tests[i].expected_rect, result);
@@ -760,7 +813,7 @@ TEST(RectTest, BoundingRect) {
     { Point(-4, 6), Point(6, -4), Rect(-4, -4, 10, 10) },
   };
 
-  for (size_t i = 0; i < arraysize(int_tests); ++i) {
+  for (size_t i = 0; i < base::size(int_tests); ++i) {
     Rect actual = BoundingRect(int_tests[i].a, int_tests[i].b);
     EXPECT_EQ(int_tests[i].expected, actual);
   }
@@ -797,7 +850,7 @@ TEST(RectTest, BoundingRect) {
       RectF(-4.2f, -4.2f, 11.0f, 11.0f) }
   };
 
-  for (size_t i = 0; i < arraysize(float_tests); ++i) {
+  for (size_t i = 0; i < base::size(float_tests); ++i) {
     RectF actual = BoundingRect(float_tests[i].a, float_tests[i].b);
     EXPECT_RECTF_EQ(float_tests[i].expected, actual);
   }