Reduce the requirement for saving previousLocation

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"
@@ skipping 34 matching lines @@
 // PaintInvalidator::mapLocalRectToBacking() when removing
 // PaintInvalidationState.
 // This function is templatized to avoid FloatRect<->LayoutRect conversions
 // which affect performance.
 template <typename Rect, typename Point>
 static LayoutRect mapLocalRectToPaintInvalidationBacking(
     const LayoutObject& object,
     const Rect& localRect,
     const PaintInvalidatorContext& context,
     GeometryMapper& geometryMapper) {
+  if (localRect.isEmpty())
+    return LayoutRect();
+
   bool isSVGChild = object.isSVGChild();
 
   // 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.
   Rect rect = localRect;
   // Writing-mode flipping doesn't apply to non-root SVG.
   if (!isSVGChild) {
     if (object.isBox()) {
@@ skipping 25 matching lines @@
   LayoutRect result;
   if (context.forcedSubtreeInvalidationFlags &
       PaintInvalidatorContext::ForcedSubtreeSlowPathRect) {
     result = slowMapToVisualRectInAncestorSpace(
         object, *context.paintInvalidationContainer, rect);
   } else if (object == context.paintInvalidationContainer) {
     result = LayoutRect(rect);
   } else {
     // For non-root SVG, the input rect is in local SVG coordinates in which
     // paint offset doesn't apply.
-    if (!isSVGChild) {
+    if (!isSVGChild)
       rect.moveBy(Point(object.paintOffset()));
-      // Use enclosingIntRect to ensure the final visual rect will cover the
-      // rect in source coordinates no matter if the painting will use pixel
-      // snapping.
-      rect = Rect(enclosingIntRect(rect));
-    }
 
     const auto* containerContentsProperties =
         context.paintInvalidationContainer->paintProperties()
             ->contentsProperties();
+
     if (context.treeBuilderContext.current.transform ==
             containerContentsProperties->transform() &&
         context.treeBuilderContext.current.clip ==
             containerContentsProperties->clip()) {
       result = LayoutRect(rect);
     } else {
+      // Use enclosingIntRect to ensure the final visual rect will cover the
+      // rect in source coordinates no matter if the painting will use pixel
+      // snapping, when transforms are applied.

[wkorman, 2017/02/21 22:54:19]

Are transforms the only potential cause of subpixel variance?

[Xianzhu, 2017/02/21 23:15:59]

Yes.

We need to apply enclosingIntRect before earlier because 
 transform(enclosingIntRect(rect))) may differ from
enclosingIntRect(transform(rect)).

In other cases, we can always keep visualRect subpixel-aware and apply
enclosingIntRect() in the last step (in
CompositedLayerMapping::setContentsNeedDisplayInRect()).

Updated comment to mention the other cases.

+      if (!isSVGChild &&
+          context.treeBuilderContext.current.transform !=
+              containerContentsProperties->transform())
+        rect = Rect(enclosingIntRect(rect));
+
       PropertyTreeState currentTreeState(
           context.treeBuilderContext.current.transform,
           context.treeBuilderContext.current.clip, nullptr);
       result = LayoutRect(
           geometryMapper
               .sourceToDestinationVisualRect(FloatRect(rect), currentTreeState,
                                              *containerContentsProperties)
               .rect());
     }
 
@@ skipping 25 matching lines @@
     return mapLocalRectToPaintInvalidationBacking<FloatRect, FloatPoint>(
         object, localRect, context, m_geometryMapper);
   }
   return mapLocalRectToPaintInvalidationBacking<LayoutRect, LayoutPoint>(
       object, object.localVisualRect(), context, m_geometryMapper);
 }
 
 LayoutPoint PaintInvalidator::computeLocationInBacking(
     const LayoutObject& object,
     const PaintInvalidatorContext& context) {
-  // Use visual rect location for LayoutTexts because it suffices to check
-  // visual rect change for layout caused invalidation.
-  if (object.isText())
-    return context.newVisualRect.location();
-
   LayoutPoint point;
   if (object != context.paintInvalidationContainer) {
     point.moveBy(object.paintOffset());
 
     const auto* containerTransform =
         context.paintInvalidationContainer->paintProperties()
             ->contentsProperties()
             ->transform();
     if (context.treeBuilderContext.current.transform != containerTransform) {
       point = LayoutPoint(m_geometryMapper
@@ skipping 162 matching lines @@
   // geometry effects, after skia optimizes filter's mapRect operation.
   // TODO(crbug.com/648274): This is a workaround for multi-column contents.
   if (object.hasFilterInducingProperty() || object.isLayoutFlowThread()) {
     context.forcedSubtreeInvalidationFlags |=
         PaintInvalidatorContext::ForcedSubtreeSlowPathRect;
   }
 
   ObjectPaintInvalidator objectPaintInvalidator(object);
   context.oldVisualRect = object.previousVisualRect();
   context.oldLocation = objectPaintInvalidator.previousLocationInBacking();
-  context.newVisualRect = computeVisualRectInBacking(object, context);
-  context.newLocation = computeLocationInBacking(object, context);
 
   IntSize adjustment = object.scrollAdjustmentForPaintInvalidation(
       *context.paintInvalidationContainer);
-  context.newLocation.move(adjustment);
+  context.newVisualRect = computeVisualRectInBacking(object, context);
   context.newVisualRect.move(adjustment);
 
+  if (object.isText()) {
+    // Use visual rect location for LayoutTexts because it suffices to check
+    // visual rect change for layout caused invalidation.

[wkorman, 2017/02/21 22:54:19]

insert 'whether a' before 'visual'

[Xianzhu, 2017/02/21 23:15:59]

Done.

+    context.newLocation = context.newVisualRect.location();
+  } else {
+    context.newLocation = computeLocationInBacking(object, context);
+    context.newLocation.move(adjustment);

[wkorman, 2017/02/21 22:54:19]

We don't need this for the text case?

[Xianzhu, 2017/02/21 23:15:59]

No, because context.newVisualRect.location() has already adjustment applied.

+
+    // Location of empty visual rect doesn't affect paint invalidation. Set it
+    // to newLocation to avoid saving the previous location separately in
+    // ObjectPaintInvalidator.
+    if (context.newVisualRect.isEmpty())
+      context.newVisualRect.setLocation(context.newLocation);
+  }
+
   object.getMutableForPainting().setPreviousVisualRect(context.newVisualRect);
   objectPaintInvalidator.setPreviousLocationInBacking(context.newLocation);
 }
 
 void PaintInvalidator::invalidatePaintIfNeeded(
     FrameView& frameView,
     PaintInvalidatorContext& context) {
   LayoutView* layoutView = frameView.layoutView();
   CHECK(layoutView);
 
@@ skipping 80 matching lines @@
         PaintInvalidatorContext::ForcedSubtreeInvalidationRectUpdate;
 }
 
 void PaintInvalidator::processPendingDelayedPaintInvalidations() {
   for (auto target : m_pendingDelayedPaintInvalidations)
     target->getMutableForPainting().setShouldDoFullPaintInvalidation(
         PaintInvalidationDelayedFull);
 }
 
 }  // namespace blink