Reduce copying of local data structures in GeometryMapper and PaintLayerClipper.

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

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/paint/PrePaintTreeWalk.h"
 
 #include "core/dom/DocumentLifecycle.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/LocalFrame.h"
 #include "core/layout/LayoutMultiColumnSpannerPlaceholder.h"
@@ skipping 96 matching lines @@
 
 // Returns whether |a| is an ancestor of or equal to |b|.
 static bool isAncestorOfOrEqualTo(const ClipPaintPropertyNode* a,
                                   const ClipPaintPropertyNode* b) {
   while (b && b != a) {
     b = b->parent();
   }
   return b == a;
 }
 
-FloatClipRect PrePaintTreeWalk::clipRectForContext(
+void PrePaintTreeWalk::computeClipRectForContext(
     const PaintPropertyTreeBuilderContext::ContainingBlockContext& context,
     const EffectPaintPropertyNode* effect,
     const PropertyTreeState& ancestorState,
     const LayoutPoint& ancestorPaintOffset,
-    bool& hasClip) {
+    bool& hasClip,
+    FloatClipRect& clipRect) {
   // Only return a non-infinite clip if clips differ, or the "ancestor" state is
   // actually an ancestor clip. This ensures no accuracy issues due to
   // transforms applied to infinite rects.
   if (isAncestorOfOrEqualTo(context.clip, ancestorState.clip()))
-    return FloatClipRect();
+    clipRect = FloatClipRect();
 
   hasClip = true;
-
   PropertyTreeState localState(context.transform, context.clip, effect);
 
-  FloatClipRect rect(
-      m_geometryMapper.sourceToDestinationClipRect(localState, ancestorState));
-
-  rect.moveBy(-FloatPoint(ancestorPaintOffset));
-  return rect;
+  clipRect =
+      m_geometryMapper.sourceToDestinationClipRect(localState, ancestorState);
+  clipRect.moveBy(-FloatPoint(ancestorPaintOffset));
 }
 
 void PrePaintTreeWalk::invalidatePaintLayerOptimizationsIfNeeded(
     const LayoutObject& object,
     PrePaintTreeWalkContext& context) {
   if (!object.hasLayer())

[Xianzhu, 2017/03/10 00:06:49]

(Can be follow-up) We can tighten this condition by checking some fast subset of
conditions in shouldCreateSubsequence() in PaintLayerPainter.cpp, to increase
chance of early returns.

     return;
 
   PaintLayer& paintLayer = *toLayoutBoxModelObject(object).layer();
   if (object.styleRef().hasTransform() ||
       &object == context.paintInvalidatorContext.paintInvalidationContainer) {
     context.ancestorTransformedOrRootPaintLayer = &paintLayer;
   }
 

[Xianzhu, 2017/03/10 00:06:49]

(Can be follow-up) We can also check for some fast subset of conditions in
shouldCreateSubsequence() in PaintLayerPainter.cpp to early return if we won't
create subsequnece for the layer.

   const ObjectPaintProperties& ancestorPaintProperties =
       *context.ancestorTransformedOrRootPaintLayer->layoutObject()
            .paintProperties();
   PropertyTreeState ancestorState =
       *ancestorPaintProperties.localBorderBoxProperties();

[Xianzhu, 2017/03/10 00:06:49]

(Can be follow-up) If using the condition I proposed below line 178, the above
statement can be moved into the condition block. We can use
   const auto* ancestorClip =
ancestorPaintProperties.localBorderBoxProperties()->clip()
instead of ancestorState before the condition.

   const EffectPaintPropertyNode* effect =
       context.treeBuilderContext.currentEffect;
 
 #ifdef CHECK_CLIP_RECTS
   ShouldRespectOverflowClipType respectOverflowClip = RespectOverflowClip;
 #endif
   if (context.ancestorTransformedOrRootPaintLayer->compositingState() ==
           PaintsIntoOwnBacking &&
       ancestorPaintProperties.overflowClip()) {
     ancestorState.setClip(ancestorPaintProperties.overflowClip());
 #ifdef CHECK_CLIP_RECTS
     respectOverflowClip = IgnoreOverflowClip;
 #endif
   }
 
 #ifdef CHECK_CLIP_RECTS
   ClipRects& oldClipRects =
       paintLayer.clipper(PaintLayer::DoNotUseGeometryMapper)
           .paintingClipRects(&ancestorTransformedOrRootPaintLayer,
                              respectOverflowClip, LayoutSize());
 #endif
 
   bool hasClip = false;
   RefPtr<ClipRects> clipRects = ClipRects::create();

[Xianzhu, 2017/03/10 00:06:49]

(Can be follow-up) We can also skip line 178-210 if there won't be any clip,
like:
  RefPtr<ClipRects> clipRects;
  if (ancestorClip != context.treeBuilderContext.current.clip &&
      ancestorClip != context.treeBuilderContext.fixedPosition.clip &&
      ancestorClip != context.treeBuilderContext.absolutePositon.clip) {
    ...
  }
  ClipRects* previousClipRects = ...
  if ((!clipRects && !previousClipRects) ||
      (clipRects && previousClipRects && *clipRects == *previousClipRects))
    return;

  clear flags;
  ...  

[chrishtr, 2017/03/10 01:45:29]

Do you mean if ancestorState.clip() is above all of those? Yes. I wonder if this
would
improve things though. It would avoid some allocations though. Will try in a
followup.

   const LayoutPoint& ancestorPaintOffset =
       context.ancestorTransformedOrRootPaintLayer->layoutObject().paintOffset();
-  clipRects->setOverflowClipRect(
-      clipRectForContext(context.treeBuilderContext.current, effect,
-                         ancestorState, ancestorPaintOffset, hasClip));
+
+  FloatClipRect clipRect;
+  computeClipRectForContext(context.treeBuilderContext.current, effect,
+                            ancestorState, ancestorPaintOffset, hasClip,
+                            clipRect);
+  clipRects->setOverflowClipRect(clipRect);
 #ifdef CHECK_CLIP_RECTS
   CHECK(!hasClip ||
         clipRects->overflowClipRect() == oldClipRects.overflowClipRect())
       << "rect= " << clipRects->overflowClipRect().toString();
 #endif
 
-  clipRects->setFixedClipRect(
-      clipRectForContext(context.treeBuilderContext.fixedPosition, effect,
-                         ancestorState, ancestorPaintOffset, hasClip));
+  computeClipRectForContext(context.treeBuilderContext.fixedPosition, effect,
+                            ancestorState, ancestorPaintOffset, hasClip,
+                            clipRect);
+  clipRects->setFixedClipRect(clipRect);
 #ifdef CHECK_CLIP_RECTS
   CHECK(hasClip || clipRects->fixedClipRect() == oldClipRects.fixedClipRect())
       << " fixed=" << clipRects->fixedClipRect().toString();
 #endif
 
-  clipRects->setPosClipRect(
-      clipRectForContext(context.treeBuilderContext.absolutePosition, effect,
-                         ancestorState, ancestorPaintOffset, hasClip));
+  computeClipRectForContext(context.treeBuilderContext.absolutePosition, effect,
+                            ancestorState, ancestorPaintOffset, hasClip,
+                            clipRect);
+  clipRects->setPosClipRect(clipRect);
 #ifdef CHECK_CLIP_RECTS
   CHECK(!hasClip || clipRects->posClipRect() == oldClipRects.posClipRect())
       << " abs=" << clipRects->posClipRect().toString();
 #endif
 
   ClipRects* previousClipRects = paintLayer.previousPaintingClipRects();
 
   if (!previousClipRects || *clipRects != *previousClipRects) {
     paintLayer.setNeedsRepaint();
     paintLayer.setPreviousPaintPhaseDescendantOutlinesEmpty(false);
@@ skipping 64 matching lines @@
           roundedIntPoint(context->treeBuilderContext.current.paintOffset);
       walk(*toFrameView(frameViewBase), *context);
     }
     // TODO(pdr): Investigate RemoteFrameView (crbug.com/579281).
   }
 
   object.getMutableForPainting().clearPaintFlags();
 }
 
 }  // namespace blink