Move all fast-path paint invalidation mapping into PaintInvalidationState

third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp

Index: third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp
diff --git a/third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp b/third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp
index 5d6930866a88cb43f9dbfe1682c7d8d66f02195e..2aaafe66aef0f89593d1d002b733723e6b5392fe 100644
--- a/third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp
+++ b/third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp
@@ -59,12 +59,37 @@ struct SearchCandidate {
     float candidateDistance;
 };
 
-static inline LayoutRect adjustedEnclosingIntRect(const FloatRect& rect,
-    const AffineTransform& rootTransform, float strokeWidthForHairlinePadding)
+FloatRect SVGLayoutSupport::localOverflowRectForPaintInvalidation(const LayoutObject& object)
 {
-    FloatRect adjustedRect = rect;
+    // This doesn't apply to LayoutSVGRoot. Use LayoutSVGRoot::localOverflowRectForPaintInvalidation() instead.
+    ASSERT(!object.isSVGRoot());
 
-    if (strokeWidthForHairlinePadding) {
+    // Return early for any cases where we don't actually paint
+    if (object.styleRef().visibility() != VISIBLE && !object.enclosingLayer()->hasVisibleContent())
+        return FloatRect();
+
+    FloatRect paintInvalidationRect = object.paintInvalidationRectInLocalSVGCoordinates();
+    if (int outlineOutset = object.styleRef().outlineOutsetExtent())
+        paintInvalidationRect.inflate(outlineOutset);
+    return paintInvalidationRect;
+}
+
+LayoutRect SVGLayoutSupport::clippedOverflowRectForPaintInvalidation(const LayoutObject& object, const LayoutBoxModelObject& paintInvalidationContainer)
+{
+    LayoutRect rect;
+    const LayoutSVGRoot& svgRoot = mapRectToSVGRootForPaintInvalidation(object, localOverflowRectForPaintInvalidation(object), rect);
+    svgRoot.mapToVisibleRectInAncestorSpace(&paintInvalidationContainer, rect);
+    return rect;
+}
+
+LayoutRect SVGLayoutSupport::transformPaintInvalidationRect(const LayoutObject& object, const AffineTransform& rootTransform, const FloatRect& localRect)
+{
+    // TODO(wangxianzhu): I don't understand the following statement. Just moved from its original place:
+    // These are quirks carried forward from the old paint invalidation infrastructure.

[pdr., 2016/03/25 01:08:01]

Lets just remove the comment since it's not helping. I'm not sure what it means
either.

[Xianzhu, 2016/03/25 16:35:55]

Done.

+
+    FloatRect adjustedRect = rootTransform.mapRect(localRect);
+
+    if (float strokeWidthForHairlinePadding = object.isSVGShape() && object.styleRef().svgStyle().hasStroke()? toLayoutSVGShape(object).strokeWidth() : 0) {

[pdr., 2016/03/25 01:08:01]

Nit: missing space before ?

This is a confusing conditional though. Maybe refactor as:
if (object.isSVGShape() && object.styleRef().svgStyle().hasVisibleStroke()) {
  float strokeWidthForHairlinePadding =
object.styleRef().svgStyle().strokeWidth();
  ...
}

[Xianzhu, 2016/03/25 16:35:55]

Done.

         // For hairline strokes (stroke-width < 1 in device space), Skia rasterizes up to 0.4(9) off
         // the stroke center. That means enclosingIntRect is not enough - we must also pad to 0.5.
         // This is still fragile as it misses out on CC/DSF CTM components.
@@ -83,39 +108,6 @@ static inline LayoutRect adjustedEnclosingIntRect(const FloatRect& rect,
     return LayoutRect(enclosingIntRect(adjustedRect));
 }
 
-LayoutRect SVGLayoutSupport::clippedOverflowRectForPaintInvalidation(const LayoutObject& object,
-    const LayoutBoxModelObject* paintInvalidationContainer,
-    const PaintInvalidationState* paintInvalidationState, float strokeWidthForHairlinePadding)
-{
-    // Return early for any cases where we don't actually paint
-    if (object.styleRef().visibility() != VISIBLE && !object.enclosingLayer()->hasVisibleContent())
-        return LayoutRect();
-
-    FloatRect paintInvalidationRect = object.paintInvalidationRectInLocalSVGCoordinates();
-    if (int outlineOutset = object.styleRef().outlineOutsetExtent())
-        paintInvalidationRect.inflate(outlineOutset);
-
-    if (paintInvalidationState && paintInvalidationState->canMapToAncestor(paintInvalidationContainer)) {
-        // Compute accumulated SVG transform and apply to local paint rect.
-        AffineTransform transform = paintInvalidationState->svgTransform() * object.localToSVGParentTransform();
-
-        // FIXME: These are quirks carried forward from the old paint invalidation infrastructure.
-        LayoutRect rect = adjustedEnclosingIntRect(transform.mapRect(paintInvalidationRect),
-            transform, strokeWidthForHairlinePadding);
-        // Offset by SVG root paint offset and apply clipping as needed.
-        rect.move(paintInvalidationState->paintOffset());
-        if (paintInvalidationState->isClipped())
-            rect.intersect(paintInvalidationState->clipRect());
-        return rect;
-    }
-
-    LayoutRect rect;
-    const LayoutSVGRoot& svgRoot = mapRectToSVGRootForPaintInvalidation(object,
-        paintInvalidationRect, rect, strokeWidthForHairlinePadding);
-    svgRoot.mapToVisibleRectInAncestorSpace(paintInvalidationContainer, rect, nullptr);
-    return rect;
-}
-
 static const LayoutSVGRoot& computeTransformToSVGRoot(const LayoutObject& object, AffineTransform& rootBorderBoxTransform)
 {
     ASSERT(object.isSVG() && !object.isSVGRoot());
@@ -130,27 +122,18 @@ static const LayoutSVGRoot& computeTransformToSVGRoot(const LayoutObject& object
 }
 
 const LayoutSVGRoot& SVGLayoutSupport::mapRectToSVGRootForPaintInvalidation(const LayoutObject& object,

[pdr., 2016/03/25 01:08:01]

Both callers of mapRectToSVGRootForPaintInvalidation also call
mapToVisibleRectInAncestorSpace. Can this logic be folded together into
something like:
LayoutRect SVGLayoutSupport::mapToVisibleRectInAncestorSpace(..., ancestor)
{
    AffineTransform rootBorderBoxTransform;
    const LayoutSVGRoot& svgRoot = computeTransformToSVGRoot(object,
rootBorderBoxTransform);
    LayoutRect rect = transformPaintInvalidationRect(object,
rootBorderBoxTransform, localPaintInvalidationRect);
    return svgRoot.mapToVisibleRectInAncestorSpace(ancestor, rect);
}

[Xianzhu, 2016/03/25 16:35:55]

Good idea. Done.

-    const FloatRect& localPaintInvalidationRect, LayoutRect& rect, float strokeWidthForHairlinePadding)
+    const FloatRect& localPaintInvalidationRect, LayoutRect& rect)
 {
     AffineTransform rootBorderBoxTransform;
     const LayoutSVGRoot& svgRoot = computeTransformToSVGRoot(object, rootBorderBoxTransform);
-
-    rect = adjustedEnclosingIntRect(rootBorderBoxTransform.mapRect(localPaintInvalidationRect),
-        rootBorderBoxTransform, strokeWidthForHairlinePadding);
+    rect = transformPaintInvalidationRect(object, rootBorderBoxTransform, localPaintInvalidationRect);
     return svgRoot;
 }
 
-void SVGLayoutSupport::mapLocalToAncestor(const LayoutObject* object, const LayoutBoxModelObject* ancestor, TransformState& transformState, bool* wasFixed, const PaintInvalidationState* paintInvalidationState)
+void SVGLayoutSupport::mapLocalToAncestor(const LayoutObject* object, const LayoutBoxModelObject* ancestor, TransformState& transformState, bool* wasFixed)
 {
     transformState.applyTransform(object->localToSVGParentTransform());
 
-    if (paintInvalidationState && paintInvalidationState->canMapToAncestor(ancestor)) {
-        // |svgTransform| contains localToBorderBoxTransform mentioned below.
-        transformState.applyTransform(paintInvalidationState->svgTransform());
-        transformState.move(paintInvalidationState->paintOffset());
-        return;
-    }
-
     LayoutObject* parent = object->parent();
 
     // At the SVG/HTML boundary (aka LayoutSVGRoot), we apply the localToBorderBoxTransform
@@ -160,7 +143,7 @@ void SVGLayoutSupport::mapLocalToAncestor(const LayoutObject* object, const Layo
         transformState.applyTransform(toLayoutSVGRoot(parent)->localToBorderBoxTransform());
 
     MapCoordinatesFlags mode = UseTransforms;
-    parent->mapLocalToAncestor(ancestor, transformState, mode, wasFixed, paintInvalidationState);
+    parent->mapLocalToAncestor(ancestor, transformState, mode, wasFixed);
 }
 
 void SVGLayoutSupport::mapAncestorToLocal(const LayoutObject& object, const LayoutBoxModelObject* ancestor, TransformState& transformState)