Remove updatePaintingInfoForFragments() and per-fragment shouldPaintContent flag.

Source/core/paint/LayerPainter.cpp

Index: Source/core/paint/LayerPainter.cpp
diff --git a/Source/core/paint/LayerPainter.cpp b/Source/core/paint/LayerPainter.cpp
index 6ee9a82b608de4f7c194ff0ef9ad7429b358e675..f108d37a6acd3d228334c1deec2e08b08f635259 100644
--- a/Source/core/paint/LayerPainter.cpp
+++ b/Source/core/paint/LayerPainter.cpp
@@ -257,12 +257,15 @@ void LayerPainter::paintLayerContents(GraphicsContext* context, const LayerPaint
 
     LayerFragments layerFragments;
     if (shouldPaintContent || shouldPaintOutline || isPaintingOverlayScrollbars) {
-        // Collect the fragments. This will compute the clip rectangles and paint offsets for each layer fragment, as well as whether or not the content of each
-        // fragment should paint.
-        m_renderLayer.collectFragments(layerFragments, localPaintingInfo.rootLayer, localPaintingInfo.paintDirtyRect,
-            (paintFlags & PaintLayerUncachedClipRects) ? UncachedClipRects : PaintingClipRects, IgnoreOverlayScrollbarSize,
-            shouldRespectOverflowClip(paintFlags, m_renderLayer.renderer()), &offsetFromRoot, localPaintingInfo.subPixelAccumulation);
-        updatePaintingInfoForFragments(layerFragments, localPaintingInfo, paintFlags, shouldPaintContent, &offsetFromRoot);
+        // Collect the fragments. This will compute the clip rectangles and paint offsets for each layer fragment.
+        ClipRectsCacheSlot cacheSlot = (paintFlags & PaintLayerUncachedClipRects) ? UncachedClipRects : PaintingClipRects;
+        ShouldRespectOverflowClip respectOverflowClip = shouldRespectOverflowClip(paintFlags, m_renderLayer.renderer());
+        RenderLayer::DirtyRectIntersectionResult intersectionResult;
+        m_renderLayer.collectFragments(layerFragments, &intersectionResult, localPaintingInfo.rootLayer, localPaintingInfo.paintDirtyRect, cacheSlot, IgnoreOverlayScrollbarSize, respectOverflowClip, &offsetFromRoot, localPaintingInfo.subPixelAccumulation);
+        if (intersectionResult == RenderLayer::NotIntersectingDirtyRect && shouldPaintContent) {
+            if (&m_renderLayer != localPaintingInfo.rootLayer || !(paintFlags & PaintLayerPaintingOverflowContents))
+                shouldPaintContent = false;
+        }
     }
 
     bool selectionOnly  = localPaintingInfo.paintBehavior & PaintBehaviorSelectionOnly;
@@ -421,20 +424,6 @@ void LayerPainter::applyRoundedRectClips(const LayerPaintingInfo& localPaintingI
     }
 }
 
-void LayerPainter::updatePaintingInfoForFragments(LayerFragments& fragments, const LayerPaintingInfo& localPaintingInfo, PaintLayerFlags localPaintFlags,
-    bool shouldPaintContent, const LayoutPoint* offsetFromRoot)
-{
-    ASSERT(offsetFromRoot);
-    for (size_t i = 0; i < fragments.size(); ++i) {

[chrishtr, 2014/10/28 18:47:42]

How about just a method:

bool LayerPainter::atLeastOneFragmentIsVisible(LayerFragments& fragments) {
  for (fragment: fragments) {
      if (&m_renderLayer == localPaintingInfo.rootLayer && !(localPaintFlags &
PaintLayerPaintingOverflowContents))
          return true;

      if (m_renderLayer.intersectsDamageRect(fragment.layerBounds,
fragment.backgroundRect.rect(), localPaintingInfo.rootLayer, &newOffsetFromRoot)
          return true;
  }
  return false;
}

Then plumb this bool into the various methods and replace
fragment.shouldPaintContent with the overall bool.

[mstensho (USE GERRIT), 2014/10/28 20:08:21]

You mean that you don't want the extra code (and parameter) that I added to
collectFragments()? The reason why I wanted to get rid of
updatePaintingInfoForFragments() was that fragment.layerBounds doesn't contain
overflow. It doesn't contain overflow because
RenderLayerClipper::calculateRects() (called from collectFragments()) sets
layerBounds to the border box, and doesn't care about overflow. While that does
look somewhat weird, since it's called "layerBounds" and all, it seems to be the
right thing to do. There are a lot of instances where we do
layerBounds.location() to find the location of the layer (e.g.
hitTestContentsForFragments() in RenderLayer and paintBackgroundForFragments()
and a bunch of other methods in LayerPainter), so including overflow there would
be wrong.

So we either need to add another member to LayerFragment, which expresses
overflow, or refrain from intersecting "layerBounds" with the damage rect.

[chrishtr, 2014/10/28 20:18:25]

Is this method hard to implement?


I was suggesting a way to reduce the size of this patch and simplify the code...
With the proposed method I think you can reduce this patch to calling
atLeastOneFragmentIsVisible instead of calling updatePaintingInfoForFragments,
plus plumbing the return value into the places were we need to consult it

I had forgotten about the overflow complication. However: it appears we can
modify the proposed LayerPainter::atLeastOneFragmentIsVisible to handle this, as
follows:

bool LayerPainter::atLeastOneFragmentIsVisible(LayerFragments& fragments) {
   // FIXME: implement correctly for columns
   if (there are columns)
     return true;

   for (fragment: fragments) {
       if (&m_renderLayer == localPaintingInfo.rootLayer && !(localPaintFlags &
PaintLayerPaintingOverflowContents))
           return true;
 
       if (m_renderLayer.intersectsDamageRect(fragment.layerBounds,
 fragment.backgroundRect.rect(), localPaintingInfo.rootLayer,
&newOffsetFromRoot)
           return true;
   }
   return false;
 }

[mstensho (USE GERRIT), 2014/10/28 21:33:20]

Done. It's kind of meaningless to loop through the fragments for non-multicol
only, since there'll always only be one fragment then. But this is a messy
neighborhood, and there's really no clean solution that doesn't involve a lot of
refactoring.

-        LayerFragment& fragment = fragments.at(i);
-        fragment.shouldPaintContent = shouldPaintContent;
-        if (&m_renderLayer != localPaintingInfo.rootLayer || !(localPaintFlags & PaintLayerPaintingOverflowContents)) {
-            LayoutPoint newOffsetFromRoot = *offsetFromRoot + fragment.paginationOffset;
-            fragment.shouldPaintContent &= m_renderLayer.intersectsDamageRect(fragment.layerBounds, fragment.backgroundRect.rect(), localPaintingInfo.rootLayer, &newOffsetFromRoot);
-        }
-    }
-}
-
 void LayerPainter::paintLayerByApplyingTransform(GraphicsContext* context, const LayerPaintingInfo& paintingInfo, PaintLayerFlags paintFlags, const LayoutPoint& translationOffset)
 {
     // This involves subtracting out the position of the layer in our current coordinate space, but preserving
@@ -672,8 +661,6 @@ void LayerPainter::paintBackgroundForFragments(const LayerFragments& layerFragme
 {
     for (size_t i = 0; i < layerFragments.size(); ++i) {
         const LayerFragment& fragment = layerFragments.at(i);
-        if (!fragment.shouldPaintContent)
-            continue;
 
         // Begin transparency layers lazily now that we know we have to paint something.
         if (haveTransparency || m_renderLayer.paintsWithBlendMode())
@@ -701,7 +688,7 @@ void LayerPainter::paintForegroundForFragments(const LayerFragments& layerFragme
     if (haveTransparency || m_renderLayer.paintsWithBlendMode()) {
         for (size_t i = 0; i < layerFragments.size(); ++i) {
             const LayerFragment& fragment = layerFragments.at(i);
-            if (fragment.shouldPaintContent && !fragment.foregroundRect.isEmpty()) {
+            if (!fragment.foregroundRect.isEmpty()) {
                 beginTransparencyLayers(context, localPaintingInfo.rootLayer, transparencyPaintDirtyRect, localPaintingInfo.subPixelAccumulation, localPaintingInfo.paintBehavior);
                 break;
             }
@@ -709,7 +696,7 @@ void LayerPainter::paintForegroundForFragments(const LayerFragments& layerFragme
     }
 
     // Optimize clipping for the single fragment case.
-    bool shouldClip = localPaintingInfo.clipToDirtyRect && layerFragments.size() == 1 && layerFragments[0].shouldPaintContent && !layerFragments[0].foregroundRect.isEmpty();
+    bool shouldClip = localPaintingInfo.clipToDirtyRect && layerFragments.size() == 1 && !layerFragments[0].foregroundRect.isEmpty();
 
     OwnPtr<ClipRecorder> clipRecorder;
     if (shouldClip && needsToClip(localPaintingInfo, layerFragments[0].foregroundRect)) {
@@ -736,7 +723,7 @@ void LayerPainter::paintForegroundForFragmentsWithPhase(PaintPhase phase, const
 
     for (size_t i = 0; i < layerFragments.size(); ++i) {
         const LayerFragment& fragment = layerFragments.at(i);
-        if (!fragment.shouldPaintContent || fragment.foregroundRect.isEmpty())
+        if (fragment.foregroundRect.isEmpty())
             continue;
 
         OwnPtr<ClipRecorder> clipRecorder;
@@ -796,9 +783,6 @@ void LayerPainter::paintMaskForFragments(const LayerFragments& layerFragments, G
 {
     for (size_t i = 0; i < layerFragments.size(); ++i) {
         const LayerFragment& fragment = layerFragments.at(i);
-        if (!fragment.shouldPaintContent)
-            continue;
-
         OwnPtr<ClipRecorder> clipRecorder;
         if (localPaintingInfo.clipToDirtyRect && needsToClip(localPaintingInfo, fragment.backgroundRect)) {
             clipRecorder = adoptPtr(new ClipRecorder(&m_renderLayer, context, ClipDisplayItem::LayerFragmentMask, fragment.backgroundRect));
@@ -817,9 +801,6 @@ void LayerPainter::paintChildClippingMaskForFragments(const LayerFragments& laye
 {
     for (size_t i = 0; i < layerFragments.size(); ++i) {
         const LayerFragment& fragment = layerFragments.at(i);
-        if (!fragment.shouldPaintContent)
-            continue;
-
         OwnPtr<ClipRecorder> clipRecorder;
         if (localPaintingInfo.clipToDirtyRect && needsToClip(localPaintingInfo, fragment.foregroundRect)) {
             clipRecorder = adoptPtr(new ClipRecorder(&m_renderLayer, context, ClipDisplayItem::LayerFragmentClippingMask, fragment.foregroundRect));
@@ -848,7 +829,7 @@ void LayerPainter::paintTransformedLayerIntoFragments(GraphicsContext* context,
     LayerFragments enclosingPaginationFragments;
     LayoutPoint offsetOfPaginationLayerFromRoot;
     LayoutRect transformedExtent = RenderLayer::transparencyClipBox(&m_renderLayer, m_renderLayer.enclosingPaginationLayer(), RenderLayer::PaintingTransparencyClipBox, RenderLayer::RootOfTransparencyClipBox, paintingInfo.subPixelAccumulation, paintingInfo.paintBehavior);
-    m_renderLayer.enclosingPaginationLayer()->collectFragments(enclosingPaginationFragments, paintingInfo.rootLayer, paintingInfo.paintDirtyRect,
+    m_renderLayer.enclosingPaginationLayer()->collectFragments(enclosingPaginationFragments, 0, paintingInfo.rootLayer, paintingInfo.paintDirtyRect,
         (paintFlags & PaintLayerUncachedClipRects) ? UncachedClipRects : PaintingClipRects, IgnoreOverlayScrollbarSize,
         shouldRespectOverflowClip(paintFlags, m_renderLayer.renderer()), &offsetOfPaginationLayerFromRoot, paintingInfo.subPixelAccumulation, &transformedExtent);