[SPInvalidation] Micro-optimize PaintLayerClipper::calculateRects

third_party/WebKit/Source/core/paint/PaintLayerClipper.cpp

Index: third_party/WebKit/Source/core/paint/PaintLayerClipper.cpp
diff --git a/third_party/WebKit/Source/core/paint/PaintLayerClipper.cpp b/third_party/WebKit/Source/core/paint/PaintLayerClipper.cpp
index 6aa997cb1f083039c7f65042cb7cfb7409366aa0..90c0a35dc0525451b968f6a51301cfee96d5e48b 100644
--- a/third_party/WebKit/Source/core/paint/PaintLayerClipper.cpp
+++ b/third_party/WebKit/Source/core/paint/PaintLayerClipper.cpp
@@ -266,6 +266,13 @@ void PaintLayerClipper::CalculateRectsWithGeometryMapper(
     ClipRect& background_rect,
     ClipRect& foreground_rect,
     const LayoutPoint* offset_from_root) const {
+  LayoutPoint offset;
+  if (offset_from_root)
+    offset = *offset_from_root;
+  else
+    layer_.ConvertToLayerCoords(context.root_layer, offset);
+  layer_bounds = LayoutRect(offset, LayoutSize(layer_.size()));
+
   // TODO(chrishtr): fix the underlying bug that causes this situation.
   if (!layer_.GetLayoutObject().PaintProperties() &&
       !layer_.GetLayoutObject().LocalBorderBoxProperties()) {
@@ -276,18 +283,23 @@ void PaintLayerClipper::CalculateRectsWithGeometryMapper(
     background_rect.Move(context.sub_pixel_accumulation);
     background_rect.Intersect(paint_dirty_rect);
 
-    CalculateForegroundClipRectWithGeometryMapper(context, foreground_rect);
-    foreground_rect.Move(context.sub_pixel_accumulation);
-    foreground_rect.Intersect(paint_dirty_rect);
+    foreground_rect = background_rect;
+    LayoutBoxModelObject& layout_object = layer_.GetLayoutObject();
+    if (ShouldClipOverflow(context)) {
+      LayoutRect overflow_and_clip_rect =
+        ToLayoutBox(layout_object)
+            .OverflowClipRect(offset, context.overlay_scrollbar_clip_behavior);

[trchen, 2017/04/14 23:35:35]

Should it be (offset + context.sub_pixel_accumulation)?

[chrishtr, 2017/04/14 23:51:56]

Do you mean I should adjust OverflowClipRect() by an additional
sub_pixel_accumulation? It seems not if one is trying to copy 
non-GM behavior in CalculateRects(), which does not do that I
believe. Maybe that behavior is incorrect?

[trchen, 2017/04/15 00:09:51]

Hmmmmm. That looks very weird indeed.

The non-GM version seemed to me that every time we inherit the parent layer's
foreground rect, we shift it by context.sub_pixel_accumulation. (Feels wrong,
and doesn't match this CL's version either.)

If I understand it correctly, context.sub_pixel_accumulation is the difference
between root layer origin and backing origin (i.e. paint offset for root layer).
I feel it is cleaner to just change #269 to:
LayoutPoint offset = context.sub_pixel_accumulation;

Then the question becomes whether *offset_from_root already included
context.sub_pixel_accumulation. :/

+      foreground_rect.Intersect(overflow_and_clip_rect);
+      if (layout_object.StyleRef().HasBorderRadius())
+        foreground_rect.SetHasRadius(true);
+    }
+    
+    if (layout_object.HasClip()) {

[trchen, 2017/04/14 23:35:35]

nit: When constructed from bottom-up, apply CSS clip before overflow clip.

[chrishtr, 2017/04/14 23:51:56]

Done, except now it technically disagrees with non-GM. :)

+      LayoutRect new_pos_clip = ToLayoutBox(layout_object).ClipRect(offset);

[trchen, 2017/04/14 23:35:35]

Ditto. (offset + context.sub_pixel_accumulation)?

[chrishtr, 2017/04/14 23:51:56]

Same answer as above.

+      foreground_rect.Intersect(new_pos_clip);
+    }
   }
 
-  LayoutPoint offset;
-  if (offset_from_root)
-    offset = *offset_from_root;
-  else
-    layer_.ConvertToLayerCoords(context.root_layer, offset);
-  layer_bounds = LayoutRect(offset, LayoutSize(layer_.size()));
-
 #ifdef CHECK_CLIP_RECTS
   ClipRect testBackgroundRect, testForegroundRect;
   LayoutRect testLayerBounds;