Avoid unnecessary FloatRect<->LayoutRect conversions in PaintInvalidator

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 1aeca106f4747a34c6b96f6cab09c9375ebdb3b4..b2b5ae54581901ecf055f65d87bae34a16077f88 100644
--- a/third_party/WebKit/Source/core/paint/PaintInvalidator.cpp
+++ b/third_party/WebKit/Source/core/paint/PaintInvalidator.cpp
@@ -20,14 +20,15 @@
 
 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;
   }
 
@@ -43,10 +44,11 @@ static LayoutRect slowMapToVisualRectInAncestorSpace(
 // 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();
 
@@ -54,7 +56,7 @@ static LayoutRect mapLocalRectToPaintInvalidationBacking(
   // 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()) {
@@ -76,7 +78,7 @@ static LayoutRect mapLocalRectToPaintInvalidationBacking(
     // 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.
@@ -94,11 +96,11 @@ static LayoutRect mapLocalRectToPaintInvalidationBacking(
     // 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 =
@@ -114,7 +116,7 @@ static LayoutRect mapLocalRectToPaintInvalidationBacking(
           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.
@@ -133,31 +135,23 @@ 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);
-}
-
-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);
+  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);
 }
 
-LayoutPoint PaintInvalidator::computeLocationInBacking(
+inline LayoutPoint PaintInvalidator::computeLocationInBacking(
     const LayoutObject& object,
     const PaintInvalidatorContext& context) {
   // Use visual rect location for LayoutTexts because it suffices to check
@@ -196,8 +190,9 @@ LayoutPoint PaintInvalidator::computeLocationInBacking(
   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();
@@ -267,8 +262,8 @@ class ScopedUndoFrameViewContentClipAndScroll {
 
 }  // namespace
 
-void PaintInvalidator::updateContext(const LayoutObject& object,
-                                     PaintInvalidatorContext& context) {
+inline void PaintInvalidator::updateContext(const LayoutObject& object,
+                                            PaintInvalidatorContext& context) {
   Optional<ScopedUndoFrameViewContentClipAndScroll>
       undoFrameViewContentClipAndScroll;