Avoid unnecessary FloatRect<->LayoutRect conversions in PaintInvalidator

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 {
 
+template <typename Rect>
 static LayoutRect slowMapToVisualRectInAncestorSpace(
     const LayoutObject& object,
     const LayoutBoxModelObject& ancestor,
-    const FloatRect& rect) {
+    const Rect& rect) {
   if (object.isSVGChild()) {
     LayoutRect result;
-    SVGLayoutSupport::mapToVisualRectInAncestorSpace(object, &ancestor, rect,
-                                                     result);
+    SVGLayoutSupport::mapToVisualRectInAncestorSpace(object, &ancestor,
+                                                     FloatRect(rect), result);
     return result;
   }
 
   LayoutRect result(rect);
   if (object.isLayoutView())
     toLayoutView(object).mapToVisualRectInAncestorSpace(
         &ancestor, result, InputIsInFrameCoordinates, DefaultVisualRectFlags);
   else
     object.mapToVisualRectInAncestorSpace(&ancestor, result);
   return result;
 }
 
 // TODO(wangxianzhu): Combine this into
 // PaintInvalidator::mapLocalRectToBacking() when removing
 // PaintInvalidationState.
+template <typename Rect, typename Point>

[pdr., 2017/01/18 17:57:00]

Can you add the following comment here:
// This function is templatized to avoid FloatRect<->LayoutRect conversions
which affect performance.

[Xianzhu, 2017/01/18 18:21:06]

Done.

 static LayoutRect mapLocalRectToPaintInvalidationBacking(
     GeometryMapper& geometryMapper,
     const LayoutObject& object,
-    const FloatRect& localRect,
+    const Rect& localRect,
     const PaintInvalidatorContext& context) {
   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.
-  FloatRect rect = localRect;
+  Rect rect = localRect;
   // Writing-mode flipping doesn't apply to non-root SVG.
   if (!isSVGChild) {
     if (object.isBox()) {
       toLayoutBox(object).flipForWritingMode(rect);
     } else if (!(context.forcedSubtreeInvalidationFlags &
                  PaintInvalidatorContext::ForcedSubtreeSlowPathRect)) {
       // For SPv2 and the GeometryMapper path, we also need to convert the rect
       // for non-boxes into physical coordinates before applying paint offset.
       // (Otherwise we'll call mapToVisualrectInAncestorSpace() which requires
       // physical coordinates for boxes, but "physical coordinates with flipped
       // block-flow direction" for non-boxes for which we don't need to flip.)
       // TODO(wangxianzhu): Avoid containingBlock().
       object.containingBlock()->flipForWritingMode(rect);
     }
   }
 
   if (RuntimeEnabledFeatures::slimmingPaintV2Enabled()) {
     // In SPv2, visual rects are in the space of their local transform node.
     // For SVG, the input rect is in local SVG coordinates in which paint
     // offset doesn't apply.
     if (!isSVGChild)
-      rect.moveBy(FloatPoint(object.paintOffset()));
+      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.
     return LayoutRect(enclosingIntRect(rect));
   }
 
   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) {
-      rect.moveBy(FloatPoint(object.paintOffset()));
+      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 = enclosingIntRect(rect);
+      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 {
       PropertyTreeState currentTreeState(
           context.treeBuilderContext.current.transform,
           context.treeBuilderContext.current.clip, nullptr, nullptr);
       result = LayoutRect(geometryMapper.sourceToDestinationVisualRect(
-          rect, currentTreeState, *containerContentsProperties));
+          FloatRect(rect), currentTreeState, *containerContentsProperties));
     }
 
     // Convert the result to the container's contents space.
     result.moveBy(-context.paintInvalidationContainer->paintOffset());
   }
 
   object.adjustVisualRectForRasterEffects(result);
 
   PaintLayer::mapRectInPaintInvalidationContainerToBacking(
       *context.paintInvalidationContainer, result);
 
   return result;
 }
 
 void PaintInvalidatorContext::mapLocalRectToPaintInvalidationBacking(
     const LayoutObject& object,
     LayoutRect& rect) const {
   GeometryMapper geometryMapper;
-  rect = blink::mapLocalRectToPaintInvalidationBacking(geometryMapper, object,
-                                                       FloatRect(rect), *this);
+  rect = blink::mapLocalRectToPaintInvalidationBacking<LayoutRect, LayoutPoint>(
+      geometryMapper, object, rect, *this);
 }
 
-LayoutRect PaintInvalidator::mapLocalRectToPaintInvalidationBacking(
+inline LayoutRect PaintInvalidator::computeVisualRectInBacking(

[pdr., 2017/01/18 17:57:00]

Could you split the inline changes into a separate patch? I believe they help
but would like to be able to separate out the effect on the perf bots.

[Xianzhu, 2017/01/18 18:21:06]

Done.

     const LayoutObject& object,
-    const FloatRect& localRect,
     const PaintInvalidatorContext& context) {
-  return blink::mapLocalRectToPaintInvalidationBacking(m_geometryMapper, object,
-                                                       localRect, context);
+  if (object.isSVGChild()) {
+    FloatRect localRect = SVGLayoutSupport::localVisualRect(object);
+    return mapLocalRectToPaintInvalidationBacking<FloatRect, FloatPoint>(
+        m_geometryMapper, object, localRect, context);
+  }
+  return mapLocalRectToPaintInvalidationBacking<LayoutRect, LayoutPoint>(
+      m_geometryMapper, object, object.localVisualRect(), context);
 }
 
-LayoutRect PaintInvalidator::computeVisualRectInBacking(
-    const LayoutObject& object,
-    const PaintInvalidatorContext& context) {
-  FloatRect localRect;
-  if (object.isSVGChild())
-    localRect = SVGLayoutSupport::localVisualRect(object);
-  else
-    localRect = FloatRect(object.localVisualRect());
-
-  return mapLocalRectToPaintInvalidationBacking(object, localRect, context);
-}
-
-LayoutPoint PaintInvalidator::computeLocationInBacking(
+inline 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());
@@ skipping 18 matching lines @@
   if (context.paintInvalidationContainer->layer()->groupedMapping()) {
     FloatPoint floatPoint(point);
     PaintLayer::mapPointInPaintInvalidationContainerToBacking(
         *context.paintInvalidationContainer, floatPoint);
     point = LayoutPoint(floatPoint);
   }
 
   return point;
 }
 
-void PaintInvalidator::updatePaintingLayer(const LayoutObject& object,
-                                           PaintInvalidatorContext& context) {
+inline void PaintInvalidator::updatePaintingLayer(
+    const LayoutObject& object,
+    PaintInvalidatorContext& context) {
   if (object.hasLayer() &&
       toLayoutBoxModelObject(object).hasSelfPaintingLayer()) {
     context.paintingLayer = toLayoutBoxModelObject(object).layer();
   } else if (object.isColumnSpanAll() ||
              (object.isFloating() && !object.parent()->isLayoutBlock())) {
     // See LayoutObject::paintingLayer() for the special-cases of floating under
     // inline and multicolumn.
     context.paintingLayer = object.paintingLayer();
   }
 
@@ skipping 49 matching lines @@
     m_treeBuilderContext.current = m_savedContext;
   }
 
  private:
   PaintPropertyTreeBuilderContext& m_treeBuilderContext;
   PaintPropertyTreeBuilderContext::ContainingBlockContext m_savedContext;
 };
 
 }  // namespace
 
-void PaintInvalidator::updateContext(const LayoutObject& object,
-                                     PaintInvalidatorContext& context) {
+inline void PaintInvalidator::updateContext(const LayoutObject& object,
+                                            PaintInvalidatorContext& context) {
   Optional<ScopedUndoFrameViewContentClipAndScroll>
       undoFrameViewContentClipAndScroll;
 
   if (object.isPaintInvalidationContainer()) {
     context.paintInvalidationContainer = toLayoutBoxModelObject(&object);
     if (object.styleRef().isStackingContext())
       context.paintInvalidationContainerForStackedContents =
           toLayoutBoxModelObject(&object);
   } else if (object.isLayoutView()) {
     // paintInvalidationContainerForStackedContents is only for stacked
@@ skipping 170 matching lines @@
         PaintInvalidatorContext::ForcedSubtreeInvalidationRectUpdate;
 }
 
 void PaintInvalidator::processPendingDelayedPaintInvalidations() {
   for (auto target : m_pendingDelayedPaintInvalidations)
     target->getMutableForPainting().setShouldDoFullPaintInvalidation(
         PaintInvalidationDelayedFull);
 }
 
 }  // namespace blink