Add support for caching and invalidating SVG marker transforms

This patch adds a cache of the SVG marker transform, similar to the pattern
used in LayoutSVGViewportContainer. This should be a small perf win because
we no longer re-calculate the transform when painting each marker instance.
With this new cache logic, invalidation logic has been added which includes
a fix for paint property under-invalidation (marker-viewBox-changes.svg).

BUG=645667,600618

Review-Url: https://codereview.chromium.org/2565943002
Cr-Commit-Position: refs/heads/master@{#437993}

third_party/WebKit/LayoutTests/TestExpectations

@@ -101,7 +101,6 @@ crbug.com/646176 virtual/spinvalidation/paint/invalidation/ [ Pass ]
 crbug.com/645667 virtual/spinvalidation/paint/invalidation/outline-clip-change.html [ Crash Failure ]
 crbug.com/645667 virtual/spinvalidation/paint/invalidation/filter-repaint-accelerated-on-accelerated-filter.html [ Crash ]
 crbug.com/645667 virtual/spinvalidation/paint/invalidation/position-change-keeping-geometry.html [ Crash ]
-crbug.com/645667 virtual/spinvalidation/paint/invalidation/svg/marker-viewBox-changes.svg [ Crash Failure ]
 crbug.com/645667 virtual/spinvalidation/paint/invalidation/compositing/should-not-repaint-composited-descendants.html [ Crash ]
 crbug.com/645667 virtual/spinvalidation/paint/invalidation/compositing/resize-repaint.html [ Crash ]
 crbug.com/645667 virtual/spinvalidation/paint/invalidation/compositing/shrink-layer.html [ Crash ]

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

@@ -27,7 +27,7 @@
 namespace blink {
 
 LayoutSVGResourceMarker::LayoutSVGResourceMarker(SVGMarkerElement* node)
-    : LayoutSVGResourceContainer(node) {}
+    : LayoutSVGResourceContainer(node), m_needsTransformUpdate(true) {}
 
 LayoutSVGResourceMarker::~LayoutSVGResourceMarker() {}
 
@@ -72,10 +72,6 @@ FloatRect LayoutSVGResourceMarker::markerBoundaries(
   return markerTransformation.mapRect(coordinates);
 }
 
-AffineTransform LayoutSVGResourceMarker::localToSVGParentTransform() const {
-  return viewportTransform();
-}
-
 FloatPoint LayoutSVGResourceMarker::referencePoint() const {
   SVGMarkerElement* marker = toSVGMarkerElement(element());
   ASSERT(marker);
@@ -119,7 +115,7 @@ AffineTransform LayoutSVGResourceMarker::markerTransformation(
   // The reference point (refX, refY) is in the coordinate space of the marker's
   // contents so we include the value in each marker's transform.
   FloatPoint mappedReferencePoint =
-      viewportTransform().mapPoint(referencePoint());
+      localToSVGParentTransform().mapPoint(referencePoint());
   transform.translate(-mappedReferencePoint.x(), -mappedReferencePoint.y());
   return transform;
 }
@@ -133,17 +129,19 @@ bool LayoutSVGResourceMarker::shouldPaint() const {
          !marker->viewBox()->currentValue()->value().isEmpty();
 }
 
-AffineTransform LayoutSVGResourceMarker::viewportTransform() const {
-  SVGMarkerElement* marker = toSVGMarkerElement(element());
-  ASSERT(marker);
-
-  return marker->viewBoxToViewTransform(m_viewportSize.width(),
-                                        m_viewportSize.height());
+void LayoutSVGResourceMarker::setNeedsTransformUpdate() {
+  setMayNeedPaintInvalidationSubtree();
+  if (RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled()) {
+    // The transform paint property relies on the SVG transform being up-to-date
+    // (see: PaintPropertyTreeBuilder::updateTransformForNonRootSVG).
+    setNeedsPaintPropertyUpdate();
+  }
+  m_needsTransformUpdate = true;
 }
 
-void LayoutSVGResourceMarker::calcViewport() {
-  if (!selfNeedsLayout())
-    return;
+SVGTransformChange LayoutSVGResourceMarker::calculateLocalTransform() {
+  if (!m_needsTransformUpdate)
+    return SVGTransformChange::None;
 
   SVGMarkerElement* marker = toSVGMarkerElement(element());
   ASSERT(marker);
@@ -152,14 +150,13 @@ void LayoutSVGResourceMarker::calcViewport() {
   float width = marker->markerWidth()->currentValue()->value(lengthContext);
   float height = marker->markerHeight()->currentValue()->value(lengthContext);
   m_viewportSize = FloatSize(width, height);
-}
 
-SVGTransformChange LayoutSVGResourceMarker::calculateLocalTransform() {
-  // TODO(fs): Temporarily, needing a layout implies that the local transform
-  // has changed. This should be updated to be more precise and factor in the
-  // actual (relevant) changes to the computed user-space transform.
-  return selfNeedsLayout() ? SVGTransformChange::Full
-                           : SVGTransformChange::None;
+  SVGTransformChangeDetector changeDetector(m_localToParentTransform);
+  m_localToParentTransform = marker->viewBoxToViewTransform(
+      m_viewportSize.width(), m_viewportSize.height());
+
+  m_needsTransformUpdate = false;
+  return changeDetector.computeChange(m_localToParentTransform);
 }
 
 }  // namespace blink

third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceMarker.h

@@ -42,11 +42,19 @@ class LayoutSVGResourceMarker final : public LayoutSVGResourceContainer {
   // Calculates marker boundaries, mapped to the target element's coordinate
   // space.
   FloatRect markerBoundaries(const AffineTransform& markerTransformation) const;
-
-  AffineTransform localToSVGParentTransform() const override;
   AffineTransform markerTransformation(const FloatPoint& origin,
                                        float angle,
                                        float strokeWidth) const;
+
+  AffineTransform localToSVGParentTransform() const final {
+    return m_localToParentTransform;
+  }
+  void setNeedsTransformUpdate() final;
+
+  // The viewport origin is (0,0) and not the reference point because each
+  // marker instance includes the reference in markerTransformation().
+  FloatRect viewport() const { return FloatRect(FloatPoint(), m_viewportSize); }
+
   bool shouldPaint() const;
 
   FloatPoint referencePoint() const;
@@ -54,21 +62,16 @@ class LayoutSVGResourceMarker final : public LayoutSVGResourceContainer {
   SVGMarkerUnitsType markerUnits() const;
   SVGMarkerOrientType orientType() const;
 
-  // The viewport origin is (0,0) and not the reference point because each
-  // marker instance includes the reference in markerTransformation().
-  FloatRect viewport() const { return FloatRect(FloatPoint(), m_viewportSize); }
-
   static const LayoutSVGResourceType s_resourceType = MarkerResourceType;
   LayoutSVGResourceType resourceType() const override { return s_resourceType; }
 
  private:
   void layout() override;
-  void calcViewport() override;
-  SVGTransformChange calculateLocalTransform() override;
-
-  AffineTransform viewportTransform() const;
+  SVGTransformChange calculateLocalTransform() final;
 
+  AffineTransform m_localToParentTransform;
   FloatSize m_viewportSize;
+  bool m_needsTransformUpdate;
 };
 
 DEFINE_LAYOUT_SVG_RESOURCE_TYPE_CASTS(LayoutSVGResourceMarker,

third_party/WebKit/Source/core/svg/SVGMarkerElement.cpp

@@ -99,22 +99,26 @@ AffineTransform SVGMarkerElement::viewBoxToViewTransform(
 }
 
 void SVGMarkerElement::svgAttributeChanged(const QualifiedName& attrName) {
-  bool isLengthAttr = attrName == SVGNames::refXAttr ||
-                      attrName == SVGNames::refYAttr ||
-                      attrName == SVGNames::markerWidthAttr ||
-                      attrName == SVGNames::markerHeightAttr;
-
-  if (isLengthAttr)
+  bool viewboxAttributeChanged = SVGFitToViewBox::isKnownAttribute(attrName);
+  bool lengthAttributeChanged = attrName == SVGNames::refXAttr ||
+                                attrName == SVGNames::refYAttr ||
+                                attrName == SVGNames::markerWidthAttr ||
+                                attrName == SVGNames::markerHeightAttr;
+  if (lengthAttributeChanged)
     updateRelativeLengthsInformation();
 
-  if (isLengthAttr || attrName == SVGNames::markerUnitsAttr ||
-      attrName == SVGNames::orientAttr ||
-      SVGFitToViewBox::isKnownAttribute(attrName)) {
+  if (viewboxAttributeChanged || lengthAttributeChanged ||
+      attrName == SVGNames::markerUnitsAttr ||
+      attrName == SVGNames::orientAttr) {
     SVGElement::InvalidationGuard invalidationGuard(this);
-    LayoutSVGResourceContainer* layoutObject =
-        toLayoutSVGResourceContainer(this->layoutObject());
-    if (layoutObject)
-      layoutObject->invalidateCacheAndMarkForLayout();
+    auto* resourceContainer = toLayoutSVGResourceContainer(layoutObject());
+    if (resourceContainer) {
+      // The marker transform depends on both viewbox attributes, and the marker
+      // size attributes (width, height).
+      if (viewboxAttributeChanged || lengthAttributeChanged)
+        resourceContainer->setNeedsTransformUpdate();
+      resourceContainer->invalidateCacheAndMarkForLayout();
+    }
 
     return;
   }