Account for filter bounds for backgroundRect during PaintLayer painting

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 ac670bf730d2374730a42870f98319e2bd4f9368..a4077c17eb1322c80edaf613490fae4bb94b2081 100644
--- a/third_party/WebKit/Source/core/paint/PaintLayerClipper.cpp
+++ b/third_party/WebKit/Source/core/paint/PaintLayerClipper.cpp
@@ -201,8 +201,7 @@ LayoutRect PaintLayerClipper::localClipRect(
   ClipRectsContext context(&clippingRootLayer, PaintingClipRects);
   if (m_geometryMapper) {
     ClipRect clipRect;
-    calculateClipRectWithGeometryMapper(context, false, clipRect);
-    applyOverflowClipToBackgroundRectWithGeometryMapper(context, clipRect);
+    calculateBackgroundClipRectWithGeometryMapper(context, clipRect);
     LayoutRect premappedRect = clipRect.rect();
 
     // The rect now needs to be transformed to the local space of this
@@ -257,29 +256,6 @@ LayoutRect PaintLayerClipper::localClipRect(
   } while (false);
 #endif
 
-void PaintLayerClipper::mapLocalToRootWithGeometryMapper(
-    const ClipRectsContext& context,
-    LayoutRect& rectToMap) const {
-  DCHECK(m_geometryMapper);
-
-  const auto* layerTransform = m_layer.layoutObject()
-                                   .paintProperties()
-                                   ->localBorderBoxProperties()
-                                   ->transform();
-  const auto* rootTransform = context.rootLayer->layoutObject()
-                                  .paintProperties()
-                                  ->localBorderBoxProperties()
-                                  ->transform();
-
-  FloatRect floatRect(rectToMap);
-  floatRect.moveBy(FloatPoint(m_layer.layoutObject().paintOffset()));
-  m_geometryMapper->sourceToDestinationRect(layerTransform, rootTransform,
-                                            floatRect);
-  rectToMap = LayoutRect(floatRect);
-  rectToMap.moveBy(-context.rootLayer->layoutObject().paintOffset());
-  rectToMap.move(context.subPixelAccumulation);
-}
-
 void PaintLayerClipper::calculateRectsWithGeometryMapper(
     const ClipRectsContext& context,
     const LayoutRect& paintDirtyRect,
@@ -293,15 +269,11 @@ void PaintLayerClipper::calculateRectsWithGeometryMapper(
     backgroundRect = ClipRect(LayoutRect(LayoutRect::infiniteIntRect()));
     foregroundRect = ClipRect(LayoutRect(LayoutRect::infiniteIntRect()));
   } else {
-    calculateClipRectWithGeometryMapper(context, false, backgroundRect);
-
+    calculateBackgroundClipRectWithGeometryMapper(context, backgroundRect);
     backgroundRect.move(context.subPixelAccumulation);
     backgroundRect.intersect(paintDirtyRect);
 
-    applyOverflowClipToBackgroundRectWithGeometryMapper(context,
-                                                        backgroundRect);
-
-    calculateClipRectWithGeometryMapper(context, true, foregroundRect);
+    calculateForegroundClipRectWithGeometryMapper(context, foregroundRect);
     foregroundRect.move(context.subPixelAccumulation);
     foregroundRect.intersect(paintDirtyRect);
   }
@@ -370,15 +342,7 @@ void PaintLayerClipper::calculateRects(
     // FIXME: Does not do the right thing with columns yet, since we don't yet
     // factor in the individual column boxes as overflow.
 
-    // The LayoutView is special since its overflow clipping rect may be larger
-    // than its box rect (crbug.com/492871).
-    LayoutRect layerBoundsWithVisualOverflow =
-        layoutObject.isLayoutView()
-            ? toLayoutView(layoutObject).viewRect()
-            : toLayoutBox(layoutObject).visualOverflowRect();
-    // PaintLayer are in physical coordinates, so the overflow has to be
-    // flipped.
-    toLayoutBox(layoutObject).flipForWritingMode(layerBoundsWithVisualOverflow);
+    LayoutRect layerBoundsWithVisualOverflow = localVisualRect();
     layerBoundsWithVisualOverflow.moveBy(offset);
     backgroundRect.intersect(layerBoundsWithVisualOverflow);
   }
@@ -443,9 +407,53 @@ static ClipRect backgroundClipRectForPosition(const ClipRects& parentRects,
   return parentRects.overflowClipRect();
 }
 
-void PaintLayerClipper::calculateClipRectWithGeometryMapper(
+void PaintLayerClipper::calculateBackgroundClipRectWithGeometryMapper(

[trchen, 2017/03/30 18:46:40]

This look a lot similar to calculateForegroundClipRectWithGeometryMapper()
below.
Is it possible to extract some common logic to helper functions? Like functions
to map PaintLayer --> PaintPropertyTreeState for different stage, e.g.
pre-overflow-clip, pre-effect, post-effect states.

Doesn't have to be in this CL though. We can land then polish.

[chrishtr, 2017/03/30 23:50:14]

I went ahead and factored out some common code, since it was easy enough.

+    const ClipRectsContext& context,
+    ClipRect& output) const {
+  DCHECK(m_geometryMapper);
+  const auto* properties = m_layer.layoutObject().paintProperties();
+  DCHECK(properties && properties->localBorderBoxProperties());
+
+  PropertyTreeState propertyTreeState = *properties->localBorderBoxProperties();
+  const auto* ancestorProperties =
+      context.rootLayer->layoutObject().paintProperties();
+  DCHECK(ancestorProperties && ancestorProperties->localBorderBoxProperties());
+  PropertyTreeState destinationPropertyTreeState =
+      *ancestorProperties->localBorderBoxProperties();
+  // CSS clip of the root is always applied.
+  if (ancestorProperties->cssClip()) {
+    DCHECK(destinationPropertyTreeState.clip() ==
+           ancestorProperties->cssClip());
+    destinationPropertyTreeState.setClip(
+        ancestorProperties->cssClip()->parent());
+  }
+
+  if (&m_layer != context.rootLayer) {
+    // Set the clip of |destinationPropertyTreeState| to be inside the
+    // ancestor's overflow clip, so that that clip is not applied.
+    if (context.respectOverflowClip == IgnoreOverflowClip &&
+        ancestorProperties->overflowClip())
+      destinationPropertyTreeState.setClip(ancestorProperties->overflowClip());
+  }
+
+  if (shouldClipOverflow(context)) {
+    FloatRect clipRect(localVisualRect());

[trchen, 2017/03/30 18:46:40]

This part still puzzles me. It seems to me the function is supposed to return a
clip rect that is suitable for clipping the stacking context's background phase.
Why should the visual overflow (which is supposed to be clipped by this rect)
take part in computation of this rect? A comment would help the reader to
understand.

[chrishtr, 2017/03/30 23:50:14]

The background phase also paints the visual overflow, including in this CL's
example, software filters. The backgroundRect is clipped to the bounds
of all of these effects, as far as I can tell as a performance optimization.

+    clipRect.moveBy(FloatPoint(m_layer.layoutObject().paintOffset()));
+    m_geometryMapper->sourceToDestinationVisualRect(
+        propertyTreeState, destinationPropertyTreeState, clipRect);
+    output.setRect(FloatClipRect(clipRect));
+  } else {
+    const FloatClipRect& clippedRectInRootLayerSpace =
+        m_geometryMapper->sourceToDestinationClipRect(
+            propertyTreeState, destinationPropertyTreeState);
+    output.setRect(clippedRectInRootLayerSpace);
+  }
+
+  output.moveBy(-context.rootLayer->layoutObject().paintOffset());
+}
+
+void PaintLayerClipper::calculateForegroundClipRectWithGeometryMapper(
     const ClipRectsContext& context,
-    bool isForeground,
     ClipRect& output) const {
   DCHECK(m_geometryMapper);
   const auto* properties = m_layer.layoutObject().paintProperties();
@@ -468,7 +476,7 @@ void PaintLayerClipper::calculateClipRectWithGeometryMapper(
   if (&m_layer == context.rootLayer) {

[trchen, 2017/03/30 18:46:40]

Not related to this CL. Just taking note to remind myself this juggling looks
puzzling and I can make it cleaner.

[chrishtr, 2017/03/30 23:50:14]

ack

     // Set the overflow clip for |propertyTreeState| so that it differs from
     // |destinationPropertyTreeState| in its clip.
-    if (isForeground && context.respectOverflowClip == RespectOverflowClip &&
+    if (context.respectOverflowClip == RespectOverflowClip &&
         properties->overflowClip())
       propertyTreeState.setClip(properties->overflowClip());
   } else {
@@ -480,7 +488,7 @@ void PaintLayerClipper::calculateClipRectWithGeometryMapper(
 
     // Set the overflow clip for |propertyTreeState| so that it differs from
     // destinationPropertyTreeState| in its clip.
-    if (isForeground && properties->overflowClip())
+    if (properties->overflowClip())
       propertyTreeState.setClip(properties->overflowClip());
   }
 
@@ -492,12 +500,10 @@ void PaintLayerClipper::calculateClipRectWithGeometryMapper(
   output.moveBy(-context.rootLayer->layoutObject().paintOffset());
 }
 
-void PaintLayerClipper::applyOverflowClipToBackgroundRectWithGeometryMapper(
-    const ClipRectsContext& context,
-    ClipRect& clip) const {
+LayoutRect PaintLayerClipper::localVisualRect() const {
   const LayoutObject& layoutObject = m_layer.layoutObject();
-  if (!shouldClipOverflow(context))
-    return;
+  // The LayoutView is special since its overflow clipping rect may be larger
+  // than its box rect (crbug.com/492871).
   LayoutRect layerBoundsWithVisualOverflow =
       layoutObject.isLayoutView()
           ? toLayoutView(layoutObject).viewRect()
@@ -507,8 +513,11 @@ void PaintLayerClipper::applyOverflowClipToBackgroundRectWithGeometryMapper(
           // PaintLayer are in physical coordinates, so the overflow has to be
           // flipped.
           layerBoundsWithVisualOverflow);
-  mapLocalToRootWithGeometryMapper(context, layerBoundsWithVisualOverflow);
-  clip.intersect(layerBoundsWithVisualOverflow);
+  if (m_layer.paintsWithFilters()) {
+    layerBoundsWithVisualOverflow =
+        m_layer.mapLayoutRectForFilter(layerBoundsWithVisualOverflow);
+  }
+  return layerBoundsWithVisualOverflow;
 }
 
 void PaintLayerClipper::calculateBackgroundClipRect(
@@ -521,7 +530,7 @@ void PaintLayerClipper::calculateBackgroundClipRect(
       return;
     }
 
-    calculateClipRectWithGeometryMapper(context, false, output);
+    calculateBackgroundClipRectWithGeometryMapper(context, output);
 #ifdef CHECK_CLIP_RECTS
     ClipRect testBackgroundClipRect =
         PaintLayerClipper(m_layer, nullptr).backgroundClipRect(context);