Bug 1350493 - Speed up and clean up SVGTransformableElement::PrependLocalTransformsTo and its overrides. r=longsonr

dom/svg/SVGContentUtils.cpp

@@ -862,38 +862,3 @@ SVGContentUtils::ShapeTypeHasNoCorners(const nsIContent* aContent) {
   return aContent && aContent->IsAnyOfSVGElements(nsGkAtoms::circle,
                                                   nsGkAtoms::ellipse);
 }
-
-gfxMatrix
-SVGContentUtils::PrependLocalTransformsTo(
-  const gfxMatrix &aMatrix,
-  SVGTransformTypes aWhich,
-  const gfx::Matrix* aAnimateMotionTransform,
-  const nsSVGAnimatedTransformList* aTransforms)
-{
-  gfxMatrix result(aMatrix);
-
-  if (aWhich == eChildToUserSpace) {
-    // We don't have anything to prepend.
-    // eChildToUserSpace is not the common case, which is why we return
-    // 'result' to benefit from NRVO rather than returning aMatrix before
-    // creating 'result'.
-    return result;
-  }
-
-  MOZ_ASSERT(aWhich == eAllTransforms || aWhich == eUserSpaceToParent,
-             "Unknown TransformTypes");
-
-  // animateMotion's resulting transform is supposed to apply *on top of*
-  // any transformations from the |transform| attribute. So since we're
-  // PRE-multiplying, we need to apply the animateMotion transform *first*.
-  if (aAnimateMotionTransform) {
-    result.PreMultiply(ThebesMatrix(*aAnimateMotionTransform));
-  }
-
-  if (aTransforms) {
-    result.PreMultiply(
-      aTransforms->GetAnimValue().GetConsolidationMatrix());
-  }
-
-  return result;
-}

dom/svg/SVGContentUtils.h

@@ -384,17 +384,6 @@ public:
    *  to have no corners: circle or ellipse
    */
   static bool ShapeTypeHasNoCorners(const nsIContent* aContent);
-
-  /**
-   *  Prepends an element's local transforms to the transform matrix.
-   *  This is a helper for nsSVGElement::PrependLocalTransformsTo.
-   *  Any callers probably really want to call that method instead of this one.
-   */
-  static gfxMatrix PrependLocalTransformsTo(
-    const gfxMatrix &aMatrix,
-    SVGTransformTypes aWhich,
-    const Matrix* aAnimateMotionTransform,
-    const mozilla::nsSVGAnimatedTransformList* aTransforms);
 };
 
 #endif

dom/svg/SVGSVGElement.cpp

@@ -954,39 +954,52 @@ SVGSVGElement::GetLength(uint8_t aCtxType)
 // nsSVGElement methods
 
 /* virtual */ gfxMatrix
-SVGSVGElement::PrependLocalTransformsTo(
-  const gfxMatrix &aMatrix, SVGTransformTypes aWhich) const
+SVGSVGElement::PrependLocalTransformsTo(const gfxMatrix& aMatrix,
+                                        SVGTransformTypes aWhich) const
 {
   // 'transform' attribute (or an override from a fragment identifier):
-  gfxMatrix fromUserSpace =
-    SVGContentUtils::PrependLocalTransformsTo(
-      aMatrix, aWhich, mAnimateMotionTransform,
-      mSVGView && mSVGView->mTransforms ? mSVGView->mTransforms : mTransforms);
+  gfxMatrix userToParent;
 
-  if (aWhich == eUserSpaceToParent) {
-    return fromUserSpace;
+  if (aWhich == eUserSpaceToParent || aWhich == eAllTransforms) {
+    userToParent = GetUserToParentTransform(mAnimateMotionTransform,
+                                            mSVGView && mSVGView->mTransforms
+                                              ? mSVGView->mTransforms
+                                              : mTransforms);
+    if (aWhich == eUserSpaceToParent) {
+      return userToParent * aMatrix;
+    }
   }
 
+  gfxMatrix childToUser;
+
   if (IsInner()) {
     float x, y;
     const_cast<SVGSVGElement*>(this)->GetAnimatedLengthValues(&x, &y, nullptr);
-    if (aWhich == eAllTransforms) {
-      // the common case
-      return ThebesMatrix(GetViewBoxTransform()) * gfxMatrix::Translation(x, y) * fromUserSpace;
-    }
-    MOZ_ASSERT(aWhich == eChildToUserSpace, "Unknown TransformTypes");
-    return ThebesMatrix(GetViewBoxTransform()) * gfxMatrix::Translation(x, y) * aMatrix;
+    childToUser = ThebesMatrix(GetViewBoxTransform().PostTranslate(x, y));
+  } else if (IsRoot()) {
+    childToUser = ThebesMatrix(GetViewBoxTransform()
+                                 .PostScale(mCurrentScale, mCurrentScale)
+                                 .PostTranslate(mCurrentTranslate.GetX(),
+                                                mCurrentTranslate.GetY()));
+  } else {
+    // outer-<svg>, but inline in some other content:
+    childToUser = ThebesMatrix(GetViewBoxTransform());
   }
 
-  if (IsRoot()) {
-    gfxMatrix zoomPanTM;
-    zoomPanTM.Translate(gfxPoint(mCurrentTranslate.GetX(), mCurrentTranslate.GetY()));
-    zoomPanTM.Scale(mCurrentScale, mCurrentScale);
-    return ThebesMatrix(GetViewBoxTransform()) * zoomPanTM * fromUserSpace;
+  if (aWhich == eAllTransforms) {
+    return childToUser * userToParent * aMatrix;
   }
 
-  // outer-<svg>, but inline in some other content:
-  return ThebesMatrix(GetViewBoxTransform()) * fromUserSpace;
+  MOZ_ASSERT(aWhich == eChildToUserSpace, "Unknown TransformTypes");
+
+  // The following may look broken because pre-multiplying our eChildToUserSpace
+  // transform with another matrix without including our eUserSpaceToParent
+  // transform between the two wouldn't make sense.  We don't expect that to
+  // ever happen though.  We get here either when the identity matrix has been
+  // passed because our caller just wants our eChildToUserSpace transform, or
+  // when our eUserSpaceToParent transform has already been multiplied into the
+  // matrix that our caller passes (such as when we're called from PaintSVG).
+  return childToUser * aMatrix;
 }
 
 nsSVGAnimatedTransformList*

dom/svg/SVGTransformableElement.cpp

@@ -101,12 +101,15 @@ SVGTransformableElement::IsEventAttributeName(nsIAtom* aName)
 // nsSVGElement overrides
 
 gfxMatrix
-SVGTransformableElement::PrependLocalTransformsTo(
-  const gfxMatrix &aMatrix,
-  SVGTransformTypes aWhich) const
+SVGTransformableElement::PrependLocalTransformsTo(const gfxMatrix& aMatrix,
+                                                  SVGTransformTypes aWhich) const
 {
-  return SVGContentUtils::PrependLocalTransformsTo(
-    aMatrix, aWhich, mAnimateMotionTransform, mTransforms);
+  if (aWhich == eChildToUserSpace) {
+    // We don't have any eUserSpaceToParent transforms. (Sub-classes that do
+    // must override this function and handle that themselves.)
+    return aMatrix;
+  }
+  return GetUserToParentTransform(mAnimateMotionTransform, mTransforms) * aMatrix;
 }
 
 const gfx::Matrix*
@@ -255,6 +258,24 @@ SVGTransformableElement::GetTransformToElement(SVGGraphicsElement& aElement,
   return mat.forget();
 }
 
+/* static */ gfxMatrix
+SVGTransformableElement::GetUserToParentTransform(
+                           const gfx::Matrix* aAnimateMotionTransform,
+                           const nsSVGAnimatedTransformList* aTransforms)
+{
+  gfxMatrix result;
+
+  if (aAnimateMotionTransform) {
+    result.PreMultiply(ThebesMatrix(*aAnimateMotionTransform));
+  }
+
+  if (aTransforms) {
+    result.PreMultiply(aTransforms->GetAnimValue().GetConsolidationMatrix());
+  }
+
+  return result;
+}
+
 } // namespace dom
 } // namespace mozilla
 

dom/svg/SVGTransformableElement.h

@@ -69,6 +69,17 @@ public:
   virtual bool IsTransformable() override { return true; }
 
 protected:
+  /**
+   * Helper for overrides of PrependLocalTransformsTo.  If both arguments are
+   * provided they are multiplied in the order in which the arguments appear,
+   * and the result is returned.  If neither argument is provided, the identity
+   * matrix is returned.  If only one argument is provided its transform is
+   * returned.
+   */
+  static gfxMatrix GetUserToParentTransform(
+                     const gfx::Matrix* aAnimateMotionTransform,
+                     const nsSVGAnimatedTransformList* aTransforms);
+
   nsAutoPtr<nsSVGAnimatedTransformList> mTransforms;
 
   // XXX maybe move this to property table, to save space on un-animated elems?

dom/svg/SVGUseElement.cpp

@@ -455,20 +455,36 @@ SVGUseElement::PrependLocalTransformsTo(
   const gfxMatrix &aMatrix, SVGTransformTypes aWhich) const
 {
   // 'transform' attribute:
-  gfxMatrix fromUserSpace =
-    SVGUseElementBase::PrependLocalTransformsTo(aMatrix, aWhich);
-  if (aWhich == eUserSpaceToParent) {
-    return fromUserSpace;
+  gfxMatrix userToParent;
+
+  if (aWhich == eUserSpaceToParent || aWhich == eAllTransforms) {
+    userToParent = GetUserToParentTransform(mAnimateMotionTransform,
+                                            mTransforms);
+    if (aWhich == eUserSpaceToParent) {
+      return userToParent * aMatrix;
+    }
   }
+
   // our 'x' and 'y' attributes:
   float x, y;
   const_cast<SVGUseElement*>(this)->GetAnimatedLengthValues(&x, &y, nullptr);
-  gfxMatrix toUserSpace = gfxMatrix::Translation(x, y);
-  if (aWhich == eChildToUserSpace) {
-    return toUserSpace * aMatrix;
+
+  gfxMatrix childToUser = gfxMatrix::Translation(x, y);
+
+  if (aWhich == eAllTransforms) {
+    return childToUser * userToParent * aMatrix;
   }
-  MOZ_ASSERT(aWhich == eAllTransforms, "Unknown TransformTypes");
-  return toUserSpace * fromUserSpace;
+
+  MOZ_ASSERT(aWhich == eChildToUserSpace, "Unknown TransformTypes");
+
+  // The following may look broken because pre-multiplying our eChildToUserSpace
+  // transform with another matrix without including our eUserSpaceToParent
+  // transform between the two wouldn't make sense.  We don't expect that to
+  // ever happen though.  We get here either when the identity matrix has been
+  // passed because our caller just wants our eChildToUserSpace transform, or
+  // when our eUserSpaceToParent transform has already been multiplied into the
+  // matrix that our caller passes (such as when we're called from PaintSVG).
+  return childToUser * aMatrix;
 }
 
 /* virtual */ bool