Implement incremental paint property tree rebuilding

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"
 #include "core/layout/LayoutPart.h"
 #include "core/layout/LayoutView.h"
 
 namespace blink {
 
 struct PrePaintTreeWalkContext {
   PrePaintTreeWalkContext() : paintInvalidatorContext(treeBuilderContext) {}
   PrePaintTreeWalkContext(const PrePaintTreeWalkContext& parentContext)
       : treeBuilderContext(parentContext.treeBuilderContext),
         paintInvalidatorContext(treeBuilderContext,
                                 parentContext.paintInvalidatorContext) {}
 
+  bool needToUpdatePaintPropertySubtree = false;
   PaintPropertyTreeBuilderContext treeBuilderContext;
   PaintInvalidatorContext paintInvalidatorContext;
 };
 
 void PrePaintTreeWalk::walk(FrameView& rootFrame) {
   DCHECK(rootFrame.frame().document()->lifecycle().state() ==
          DocumentLifecycle::InPrePaint);
 
   PrePaintTreeWalkContext initialContext;
   initialContext.treeBuilderContext =
       m_propertyTreeBuilder.setupInitialContext();
   walk(rootFrame, initialContext);
   m_paintInvalidator.processPendingDelayedPaintInvalidations();
 }
 
 void PrePaintTreeWalk::walk(FrameView& frameView,
                             const PrePaintTreeWalkContext& context) {
   if (frameView.shouldThrottleRendering())
     return;
 
   PrePaintTreeWalkContext localContext(context);
-  m_propertyTreeBuilder.buildTreeNodes(frameView,
-                                       localContext.treeBuilderContext);
+
+  // Check whether we need to update the paint property trees.
+  if (!localContext.needToUpdatePaintPropertySubtree) {
+    if (context.paintInvalidatorContext.forcedSubtreeInvalidationFlags) {
+      // forcedSubtreeInvalidationFlags will be true if locations have changed
+      // which will affect paint properties (e.g., PaintOffset).
+      localContext.needToUpdatePaintPropertySubtree = true;
+    } else if (frameView.needsPaintPropertyUpdate()) {
+      // Paint properties can depend on their ancestor properties so ensure
+      // the entire subtree is rebuilt on any changes.
+      // TODO(pdr): Add additional granularity to the needs update approach
+      // such as the ability to do local updates that don't change the subtree.
+      localContext.needToUpdatePaintPropertySubtree = true;
+    }
+  }
+  if (localContext.needToUpdatePaintPropertySubtree)
+    frameView.setNeedsPaintPropertyUpdate();
+
+  m_propertyTreeBuilder.updateProperties(frameView,
+                                         localContext.treeBuilderContext);
 
   if (RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled()) {
     m_paintInvalidator.invalidatePaintIfNeeded(
         frameView, localContext.paintInvalidatorContext);
   }
 
   if (LayoutView* layoutView = frameView.layoutView())
     walk(*layoutView, localContext);
 
 #if DCHECK_IS_ON()
   if (RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled())
     frameView.layoutView()->assertSubtreeClearedPaintInvalidationFlags();
 #endif
+
+  frameView.clearNeedsPaintPropertyUpdate();
 }
 
 void PrePaintTreeWalk::walk(const LayoutObject& object,
                             const PrePaintTreeWalkContext& context) {
   PrePaintTreeWalkContext localContext(context);
 
+  // Check whether we need to update the paint property trees.
+  if (!localContext.needToUpdatePaintPropertySubtree) {
+    if (context.paintInvalidatorContext.forcedSubtreeInvalidationFlags) {
+      // forcedSubtreeInvalidationFlags will be true if locations have changed
+      // which will affect paint properties (e.g., PaintOffset).
+      localContext.needToUpdatePaintPropertySubtree = true;
+    } else if (object.needsPaintPropertyUpdate()) {
+      // Paint properties can depend on their ancestor properties so ensure
+      // the entire subtree is rebuilt on any changes.
+      // TODO(pdr): Add additional granularity to the needs update approach
+      // such as the ability to do local updates that don't change the subtree.
+      localContext.needToUpdatePaintPropertySubtree = true;
+    } else if (
+        object
+            .shouldCheckForPaintInvalidationRegardlessOfPaintInvalidationState()) {
+      // TODO(pdr): Better document the exact reason we need to check this
+      // paint invalidation state.
+      localContext.needToUpdatePaintPropertySubtree = true;

[Xianzhu, 2016/10/26 19:21:51]

I think we don't need to set needToUpdatePaintPropertySubtree but just
object.setNeedsPaintPropertyUpdate().

About your question in https://codereview.chromium.org/2404213004/#msg13, I
think we can use mayNeedpaintInvalidation() || shouldDoFullPaintInvalidation()
here to exclude the cases that the object needs checking paint invalidation just
because of having selection and/or childShouldCheckForPaintInvalidation().

[pdr., 2016/10/26 23:10:19]

Initially I'd like to rebuild entire subtrees anytime
object.needsPaintPropertyUpdate is set. This way we don't yet have to worry
about descendant properties that point to stale properties on this object. I'd
then like to improve this in the future. Does that sound reasonable to you, or
do you think this should be thought out more before landing parts of this patch?

I have two ideas for improving setNeedsPaintPropertyUpdate:
* We could introduce a local-only (or even transform-only, clip-only, etc) bit
such as setNeedsLocalPaintPropertyUpdate that does not rebuild the entire
subtree. This would be for the case when a transform changes, for example.
* Another option is to only rebuild a property subtree if nodes change. If just
the values of property nodes change (as opposed to new ones added or deleted),
all child nodes will still be valid and we do not need to rebuild the subtree at
all.

Does that sound like a good idea? Thoughts welcome.
Updated! Can you take another look at this conditional and see if my comments
can be improved?

[Xianzhu, 2016/10/26 23:19:40]

SGTM.

(I overlooked the previous needToUpdatePaintPropertySubtree = true and
misunderstood the above code when writing the previous comment.)

+    }
+  }
+  if (localContext.needToUpdatePaintPropertySubtree)
+    object.getMutableForPainting().setNeedsPaintPropertyUpdate();
+
+  // TODO(pdr): Ensure multi column works with incremental property tree
+  // construction.
   if (object.isLayoutMultiColumnSpannerPlaceholder()) {
     // Walk multi-column spanner as if it replaces the placeholder.
     // Set the flag so that the tree builder can specially handle out-of-flow
     // positioned descendants if their containers are between the multi-column
     // container and the spanner. See PaintPropertyTreeBuilder for details.
     localContext.treeBuilderContext.isUnderMultiColumnSpanner = true;
     object.getMutableForPainting().clearPaintInvalidationFlags();
     walk(*toLayoutMultiColumnSpannerPlaceholder(object)
               .layoutObjectInFlowThread(),
          localContext);
     return;
   }
 
-  m_propertyTreeBuilder.buildTreeNodesForSelf(object,
-                                              localContext.treeBuilderContext);
+  m_propertyTreeBuilder.updatePropertiesForSelf(
+      object, localContext.treeBuilderContext);
+
   if (RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled())
     m_paintInvalidator.invalidatePaintIfNeeded(
         object, localContext.paintInvalidatorContext);
-  m_propertyTreeBuilder.buildTreeNodesForChildren(
+
+  m_propertyTreeBuilder.updatePropertiesForChildren(
       object, localContext.treeBuilderContext);
 
   for (const LayoutObject* child = object.slowFirstChild(); child;
        child = child->nextSibling()) {
     // Column spanners are walked through their placeholders. See above.
     if (child->isColumnSpanAll())
       continue;
     walk(*child, localContext);
   }
 
   if (object.isLayoutPart()) {
     const LayoutPart& layoutPart = toLayoutPart(object);
     Widget* widget = layoutPart.widget();
     if (widget && widget->isFrameView()) {
       localContext.treeBuilderContext.current.paintOffset +=
           layoutPart.replacedContentRect().location() -
           widget->frameRect().location();
       localContext.treeBuilderContext.current.paintOffset =
           roundedIntPoint(localContext.treeBuilderContext.current.paintOffset);
       walk(*toFrameView(widget), localContext);
     }
     // TODO(pdr): Investigate RemoteFrameView (crbug.com/579281).
   }
+
+  object.getMutableForPainting().clearNeedsPaintPropertyUpdate();
 }
 
 }  // namespace blink