Account for filter bounds for backgroundRect during PaintLayer painting

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

 /*
  * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012 Apple Inc. All rights
  * reserved.
  *
  * Portions are Copyright (C) 1998 Netscape Communications Corporation.
  *
  * Other contributors:
  *   Robert O'Callahan <roc+@cs.cmu.edu>
  *   David Baron <dbaron@fas.harvard.edu>
  *   Christian Biesinger <cbiesinger@web.de>
@@ skipping 184 matching lines @@
       layers.push(layer);
   }
 }
 
 LayoutRect PaintLayerClipper::localClipRect(
     const PaintLayer& clippingRootLayer) const {
   ClipRectsContext context(&clippingRootLayer, PaintingClipRects);
   if (m_geometryMapper) {
     ClipRect clipRect;
     calculateClipRectWithGeometryMapper(context, false, clipRect);
-    applyOverflowClipToBackgroundRectWithGeometryMapper(context, clipRect);
+    applyVisualOverflowClipToBackgroundRectWithGeometryMapper(context,
+                                                              clipRect);

[trchen, 2017/03/29 23:50:08]

Why is this line needed? The only caller of PLC::localClipRect() is
PaintLayer::boundingBoxForCompositingInternal(), and the caller intersect the
rect with physicalBoundingBox() immediately.

[chrishtr, 2017/03/30 04:35:26]

There is not really any point, but if I remove this line then the
GeometryMapper mode of this class will disagree with non-GeometryMapper,
which delegates to calculateRects. I don't think it does any harm.

     LayoutRect premappedRect = clipRect.rect();
 
     // The rect now needs to be transformed to the local space of this
     // PaintLayer.
     premappedRect.moveBy(context.rootLayer->layoutObject().paintOffset());
 
     const auto* clipRootLayerTransform = clippingRootLayer.layoutObject()
                                              .paintProperties()
                                              ->localBorderBoxProperties()
                                              ->transform();
@@ skipping 75 matching lines @@
   // TODO(chrishtr): fix the underlying bug that causes this situation.
   if (!properties) {
     backgroundRect = ClipRect(LayoutRect(LayoutRect::infiniteIntRect()));
     foregroundRect = ClipRect(LayoutRect(LayoutRect::infiniteIntRect()));
   } else {
     calculateClipRectWithGeometryMapper(context, false, backgroundRect);
 
     backgroundRect.move(context.subPixelAccumulation);
     backgroundRect.intersect(paintDirtyRect);
 
-    applyOverflowClipToBackgroundRectWithGeometryMapper(context,
-                                                        backgroundRect);
+    applyVisualOverflowClipToBackgroundRectWithGeometryMapper(context,
+                                                              backgroundRect);
 
     calculateClipRectWithGeometryMapper(context, true, foregroundRect);
     foregroundRect.move(context.subPixelAccumulation);
     foregroundRect.intersect(paintDirtyRect);
   }
   LayoutPoint offset;
   if (offsetFromRoot)
     offset = *offsetFromRoot;
   else
     m_layer.convertToLayerCoords(context.rootLayer, offset);
@@ skipping 59 matching lines @@
 
     // 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);
+    if (m_layer.paintsWithFilters()) {
+      layerBoundsWithVisualOverflow =
+          m_layer.mapLayoutRectForFilter(layerBoundsWithVisualOverflow);
+    }
     layerBoundsWithVisualOverflow.moveBy(offset);
     backgroundRect.intersect(layerBoundsWithVisualOverflow);
   }
 
   // CSS clip (different than clipping due to overflow) can clip to any box,
   // even if it falls outside of the border box.
   if (layoutObject.hasClip()) {
     // Clip applies to *us* as well, so go ahead and update the damageRect.
     LayoutRect newPosClip = toLayoutBox(layoutObject).clipRect(offset);
     backgroundRect.intersect(newPosClip);
@@ skipping 93 matching lines @@
   }
 
   const FloatClipRect& clippedRectInRootLayerSpace =
       m_geometryMapper->sourceToDestinationClipRect(
           propertyTreeState, destinationPropertyTreeState);
   output.setRect(clippedRectInRootLayerSpace);
 
   output.moveBy(-context.rootLayer->layoutObject().paintOffset());
 }
 
-void PaintLayerClipper::applyOverflowClipToBackgroundRectWithGeometryMapper(
-    const ClipRectsContext& context,
-    ClipRect& clip) const {
+void PaintLayerClipper::
+    applyVisualOverflowClipToBackgroundRectWithGeometryMapper(
+        const ClipRectsContext& context,
+        ClipRect& clip) const {

[trchen, 2017/03/29 23:50:08]

The name doesn't seem to match the implementation... (In fact the implementation
looks weird.)

It seems to me that clip would be a in-out parameter, and the expected input
would be the accumulated clip that should apply to all phases of the current
layer. Then if the current object does not create its own overflow clip, returns
the accumulated clip untouched. Otherwise, return its propagating visual
overflow clipped by the accumulated clip.

[chrishtr, 2017/03/30 04:35:26]

Method is now deleted.

   const LayoutObject& layoutObject = m_layer.layoutObject();
   if (!shouldClipOverflow(context))
     return;
   LayoutRect layerBoundsWithVisualOverflow =
       layoutObject.isLayoutView()
           ? toLayoutView(layoutObject).viewRect()
           : toLayoutBox(layoutObject).visualOverflowRect();
   toLayoutBox(layoutObject)
       .flipForWritingMode(
           // PaintLayer are in physical coordinates, so the overflow has to be
           // flipped.
           layerBoundsWithVisualOverflow);
+  if (m_layer.paintsWithFilters()) {
+    layerBoundsWithVisualOverflow =
+        m_layer.mapLayoutRectForFilter(layerBoundsWithVisualOverflow);
+  }
+

[trchen, 2017/03/29 23:50:08]

Is it possible to make it a geometry mapper helper? This line together with line
521 basically reads
geometryMapper->mapVisualRectToAncestor(rootLayerState, layerState,
layerBoundsWithVisualOverflow)
to me.

[chrishtr, 2017/03/30 04:35:26]

That's a great observation. In fact it led me to realize that this whole
method is not necessary, since I can call sourceToDestinationVisualRect
in cases when shouldClipOverflow() is true. This will be a significant
time savings in paint.

   mapLocalToRootWithGeometryMapper(context, layerBoundsWithVisualOverflow);
   clip.intersect(layerBoundsWithVisualOverflow);
 }
 
 void PaintLayerClipper::calculateBackgroundClipRect(
     const ClipRectsContext& context,
     ClipRect& output) const {
   if (m_geometryMapper) {
     // TODO(chrishtr): fix the underlying bug that causes this situation.
     if (!m_layer.layoutObject().paintProperties()) {
@@ skipping 76 matching lines @@
   DCHECK(!m_geometryMapper);
   ClipRectsContext context(rootLayer, PaintingClipRects,
                            IgnorePlatformOverlayScrollbarSize,
                            subpixelAccumulation);
   if (respectOverflowClip == IgnoreOverflowClip)
     context.setIgnoreOverflowClip();
   return getClipRects(context);
 }
 
 }  // namespace blink