Fix clip and offset issues of slimmingPaintInvalidation for SPv1

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

 // Copyright 2016 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/PaintInvalidator.h"
 
 #include "core/editing/FrameSelection.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/Settings.h"
 #include "core/layout/LayoutBlockFlow.h"
 #include "core/layout/LayoutObject.h"
 #include "core/layout/LayoutTable.h"
 #include "core/layout/LayoutView.h"
 #include "core/layout/svg/SVGLayoutSupport.h"
 #include "core/paint/ObjectPaintProperties.h"
 #include "core/paint/PaintLayer.h"
 #include "core/paint/PaintLayerScrollableArea.h"
 #include "core/paint/PaintPropertyTreeBuilder.h"
 
 namespace blink {
 
+static void slowMapToVisualRectInAncestorSpace(const LayoutObject& object, const LayoutBoxModelObject& ancestor, LayoutRect& rect)
+{
+    if (object.isLayoutView())
+        toLayoutView(object).mapToVisualRectInAncestorSpace(&ancestor, rect, InputIsInFrameCoordinates, DefaultVisualRectFlags);
+    else
+        object.mapToVisualRectInAncestorSpace(&ancestor, rect);
+}
+
 // TODO(wangxianzhu): Combine this into PaintInvalidator::mapLocalRectToPaintInvalidationBacking() when removing PaintInvalidationState.
 static LayoutRect mapLocalRectToPaintInvalidationBacking(GeometryMapper& geometryMapper, const LayoutObject& object, const FloatRect& localRect, const PaintInvalidatorContext& context)
 {
     // TODO(wkorman): The flip below is required because visual rects are
     // currently in "physical coordinates with flipped block-flow direction"
     // (see LayoutBoxModelObject.h) but we need them to be in physical
     // coordinates.
     FloatRect rect = localRect;
     if (object.isBox())
         toLayoutBox(object).flipForWritingMode(rect);
 
     LayoutRect result;
-    if (object == context.paintInvalidationContainer) {
+    if (context.forcedSubtreeInvalidationFlags & PaintInvalidatorContext::ForcedSubtreeSlowPathRect) {
+        result = LayoutRect(rect);
+        slowMapToVisualRectInAncestorSpace(object, *context.paintInvalidationContainer, result);
+    } else if (object == context.paintInvalidationContainer) {
         result = LayoutRect(rect);
     } else {
-        rect.moveBy(FloatPoint(context.treeBuilderContext.current.paintOffset));
-
-        bool success = false;
         PropertyTreeState currentTreeState(context.treeBuilderContext.current.transform, context.treeBuilderContext.current.clip, context.treeBuilderContext.currentEffect);
         PropertyTreeState containerTreeState;
-        context.paintInvalidationContainer->objectPaintProperties()->getContentsProperties(containerTreeState);
+        const ObjectPaintProperties* containerPaintProperties = context.paintInvalidationContainer->objectPaintProperties();
+        containerPaintProperties->getContentsProperties(containerTreeState);
+
+        rect.moveBy(FloatPoint(context.treeBuilderContext.current.paintOffset));
+        bool success = false;
         result = LayoutRect(geometryMapper.mapToVisualRectInDestinationSpace(rect, currentTreeState, containerTreeState, success));
         DCHECK(success);
+        result.moveBy(-containerPaintProperties->localBorderBoxProperties()->paintOffset);

[chrishtr, 2016/09/13 23:30:03]

This was a straight-up bug in slimmingPaintInvalidation? Why are you computing
the diff of the paint
offsets?

[Xianzhu, 2016/09/13 23:47:15]

Yes, this was a bug, but didn't notice it because there are too many failures on
spv2. 

Now with it running on spv1 I discovered this from failures of a much smaller
number.

After Line 57 we get a rect in the space of container's PaintTreeStatus. The
rect still includes container's paint offset of the container which should be
subtracted from the rect to get a rect in container's space.

[chrishtr, 2016/09/13 23:54:25]

Ah yes, makes sense.

     }
 
     if (context.paintInvalidationContainer->layer()->groupedMapping())
         PaintLayer::mapRectInPaintInvalidationContainerToBacking(*context.paintInvalidationContainer, result);
     return result;
 }
 
 void PaintInvalidatorContext::mapLocalRectToPaintInvalidationBacking(const LayoutObject& object, LayoutRect& rect) const
 {
     GeometryMapper geometryMapper;
@@ skipping 98 matching lines @@
             context.forcedSubtreeInvalidationFlags = 0;
         }
     }
 
     DCHECK(context.paintInvalidationContainer == object.containerForPaintInvalidation());
     DCHECK(context.paintingLayer == object.paintingLayer());
 
     if (object.mayNeedPaintInvalidationSubtree())
         context.forcedSubtreeInvalidationFlags |= PaintInvalidatorContext::ForcedSubtreeInvalidationChecking;
 
+    // TODO(crbug.com/637313): This is temporary before we support filters in paint property tree.
+    if (object.hasFilterInducingProperty())
+        context.forcedSubtreeInvalidationFlags |= PaintInvalidatorContext::ForcedSubtreeSlowPathRect;
+
     context.oldBounds = object.previousPaintInvalidationRect();
     context.oldLocation = object.previousPositionFromPaintInvalidationBacking();
     context.newBounds = computePaintInvalidationRectInBacking(object, context);
     context.newLocation = computeLocationFromPaintInvalidationBacking(object, context);
 
     IntSize adjustment = object.scrollAdjustmentForPaintInvalidation(*context.paintInvalidationContainer);
     context.newLocation.move(adjustment);
     context.newBounds.move(adjustment);
 
     object.getMutableForPainting().setPreviousPaintInvalidationRect(context.newBounds);
@@ skipping 14 matching lines @@
     if (frameView.frame().selection().isCaretBoundsDirty())
         frameView.frame().selection().invalidateCaretRect();
 
     // Temporary callback for crbug.com/487345,402044
     // TODO(ojan): Make this more general to be used by PositionObserver
     // and rAF throttling.
     IntRect visibleRect = frameView.rootFrameToContents(frameView.computeVisibleArea());
     layoutView->sendMediaPositionChangeNotifications(visibleRect);
 }
 
+static bool hasPercentageTransform(const ComputedStyle& style)
+{
+    if (TransformOperation* translate = style.translate()) {
+        if (translate->dependsOnBoxSize())
+            return true;
+    }
+    return style.transform().dependsOnBoxSize()
+        || (style.transformOriginX() != Length(50, Percent) && style.transformOriginX().isPercentOrCalc())

[chrishtr, 2016/09/13 23:30:03]

There's no canonical way to detect non-default transform origin?

[Xianzhu, 2016/09/13 23:47:15]

We only need to check non-default percentage transform origin here.

This code is copied from LayoutBoxModelObject.cpp and is a workaround of
crbug.com/533277 (see line 265 below) and will be removed when we implement
paint offset change detection.

+        || (style.transformOriginY() != Length(50, Percent) && style.transformOriginY().isPercentOrCalc());
+}
+
 void PaintInvalidator::invalidatePaintIfNeeded(const LayoutObject& object, PaintInvalidatorContext& context)
 {
     object.getMutableForPainting().ensureIsReadyForPaintInvalidation();
 
     if (!context.forcedSubtreeInvalidationFlags && !object.shouldCheckForPaintInvalidationRegardlessOfPaintInvalidationState())
         return;
 
     updatePaintingLayer(object, context);
 
     if (object.document().printing())
         return; // Don't invalidate paints if we're printing.
 
     TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("blink.invalidation"), "PaintInvalidator::invalidatePaintIfNeeded()", "object", object.debugName().ascii());
 
     updateContext(object, context);
 
     if (!object.shouldCheckForPaintInvalidationRegardlessOfPaintInvalidationState() && context.forcedSubtreeInvalidationFlags == PaintInvalidatorContext::ForcedSubtreeInvalidationRectUpdate) {
         // We are done updating the paint invalidation rect. No other paint invalidation work to do for this object.
         return;
     }
 
-    switch (object.invalidatePaintIfNeeded(context)) {
+    PaintInvalidationReason reason = object.invalidatePaintIfNeeded(context);
+    switch (reason) {
     case PaintInvalidationDelayedFull:
         m_pendingDelayedPaintInvalidations.append(&object);
         break;
     case PaintInvalidationSubtree:
         context.forcedSubtreeInvalidationFlags |= (PaintInvalidatorContext::ForcedSubtreeFullInvalidation | PaintInvalidatorContext::ForcedSubtreeFullInvalidationForStackedContents);
         break;
     case PaintInvalidationSVGResourceChange:
         context.forcedSubtreeInvalidationFlags |= PaintInvalidatorContext::ForcedSubtreeInvalidationChecking;
         break;
     default:
         break;
     }
 
     if (context.oldLocation != context.newLocation)
         context.forcedSubtreeInvalidationFlags |= PaintInvalidatorContext::ForcedSubtreeInvalidationChecking;
 
+    // TODO(crbug.com/533277): This is a workaround for the bug. Remove when we detect paint offset change.
+    if (reason != PaintInvalidationNone && hasPercentageTransform(object.styleRef()))
+        context.forcedSubtreeInvalidationFlags |= PaintInvalidatorContext::ForcedSubtreeInvalidationChecking;
+
+    // TODO(crbug.com/490725): This is a workaround for the bug, to force descendant to update paint invalidation
+    // rects on clipping change.
+    if (!RuntimeEnabledFeatures::slimmingPaintV2Enabled()
+        && context.oldBounds != context.newBounds
+        // Note that isLayoutView() below becomes unnecessary after the launch of root layer scrolling.
+        && (object.hasOverflowClip() || object.isLayoutView())
+        && !toLayoutBox(object).usesCompositedScrolling())
+        context.forcedSubtreeInvalidationFlags |= PaintInvalidatorContext::ForcedSubtreeInvalidationRectUpdate;
+
     object.getMutableForPainting().clearPaintInvalidationFlags();
 }
 
 void PaintInvalidator::processPendingDelayedPaintInvalidations()
 {
     for (auto target : m_pendingDelayedPaintInvalidations)
         target->getMutableForPainting().setShouldDoFullPaintInvalidation(PaintInvalidationDelayedFull);
 }
 
 } // namespace blink