Reduce the requirement for saving previousLocation

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

Index: third_party/WebKit/Source/core/paint/PaintInvalidator.cpp
diff --git a/third_party/WebKit/Source/core/paint/PaintInvalidator.cpp b/third_party/WebKit/Source/core/paint/PaintInvalidator.cpp
index 0548d0fafb2748aa0c73f1f1a6a4143f93290d5b..979772e9d5ab8ae544dcd2d645b7adc62dfceb7f 100644
--- a/third_party/WebKit/Source/core/paint/PaintInvalidator.cpp
+++ b/third_party/WebKit/Source/core/paint/PaintInvalidator.cpp
@@ -52,6 +52,9 @@ static LayoutRect mapLocalRectToPaintInvalidationBacking(
     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
@@ -97,23 +100,27 @@ static LayoutRect mapLocalRectToPaintInvalidationBacking(
   } 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);
@@ -159,11 +166,6 @@ LayoutRect PaintInvalidator::computeVisualRectInBacking(
 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());
@@ -346,14 +348,27 @@ void PaintInvalidator::updateContext(const LayoutObject& object,
   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);
 }