Bug 1165185 - Make the nsSVGOuterSVGAnonChildFrame a reference frame by always returning true from IsSVGTransformed. draft
authorMarkus Stange <mstange@themasta.com>
Fri, 20 Apr 2018 23:30:04 -0400
changeset 786081 dcdb79a355b94b04ded49030875ab6f12e937332
parent 786080 117ff7a62e906772ffb64adc52dd873938f2e367
child 789316 9fbb74fca70738ada450413547ec36e8295ac34a
push id107392
push userbmo:mstange@themasta.com
push dateSat, 21 Apr 2018 03:30:38 +0000
bugs1165185
milestone61.0a1
Bug 1165185 - Make the nsSVGOuterSVGAnonChildFrame a reference frame by always returning true from IsSVGTransformed. This causes transforms of the <svg> contents to be unaffected by scrolling / offset changes of the <svg> element. MozReview-Commit-ID: JtDmwppqBis
layout/svg/nsSVGOuterSVGFrame.cpp
--- a/layout/svg/nsSVGOuterSVGFrame.cpp
+++ b/layout/svg/nsSVGOuterSVGFrame.cpp
@@ -996,53 +996,63 @@ nsSVGOuterSVGAnonChildFrame::Init(nsICon
                                   nsContainerFrame* aParent,
                                   nsIFrame*         aPrevInFlow)
 {
   MOZ_ASSERT(aParent->IsSVGOuterSVGFrame(), "Unexpected parent");
   nsSVGDisplayContainerFrame::Init(aContent, aParent, aPrevInFlow);
 }
 #endif
 
-bool
-nsSVGOuterSVGAnonChildFrame::IsSVGTransformed(Matrix* aOwnTransform,
-                                              Matrix* aFromParentTransform) const
+static Matrix
+ComputeOuterSVGAnonChildFrameTransform(const nsSVGOuterSVGAnonChildFrame* aFrame)
 {
   // Our elements 'transform' attribute is applied to our nsSVGOuterSVGFrame
   // parent, and the element's children-only transforms are applied to us, the
   // anonymous child frame. Since we are the child frame, we apply the
   // children-only transforms as if they are our own transform.
-
-  SVGSVGElement* content = static_cast<SVGSVGElement*>(GetContent());
+  SVGSVGElement* content = static_cast<SVGSVGElement*>(aFrame->GetContent());
 
   if (!content->HasChildrenOnlyTransform()) {
-    return false;
+    return Matrix();
   }
 
   // Outer-<svg> doesn't use x/y, so we can pass eChildToUserSpace here.
   gfxMatrix ownMatrix =
     content->PrependLocalTransformsTo(gfxMatrix(), eChildToUserSpace);
 
-  if (ownMatrix.IsIdentity()) {
-    return false;
+  if (ownMatrix.HasNonTranslation()) {
+    // viewBox, currentScale and currentTranslate should only produce a
+    // rectilinear transform.
+    MOZ_ASSERT(ownMatrix.IsRectilinear(),
+                "Non-rectilinear transform will break the following logic");
+
+    // The nsDisplayTransform code will apply this transform to our frame,
+    // including to our frame position.  We don't want our frame position to
+    // be scaled though, so we need to correct for that in the transform.
+    // XXX Yeah, this is a bit hacky.
+    CSSPoint pos = CSSPixel::FromAppUnits(aFrame->GetPosition());
+    CSSPoint scaledPos = CSSPoint(ownMatrix._11 * pos.x, ownMatrix._22 * pos.y);
+    CSSPoint deltaPos = scaledPos - pos;
+    ownMatrix *= gfxMatrix::Translation(-deltaPos.x, -deltaPos.y);
   }
 
-  if (aOwnTransform) {
-    if (ownMatrix.HasNonTranslation()) {
-      // viewBox, currentScale and currentTranslate should only produce a
-      // rectilinear transform.
-      MOZ_ASSERT(ownMatrix.IsRectilinear(),
-                 "Non-rectilinear transform will break the following logic");
+  return gfx::ToMatrix(ownMatrix);
+}
 
-      // The nsDisplayTransform code will apply this transform to our frame,
-      // including to our frame position.  We don't want our frame position to
-      // be scaled though, so we need to correct for that in the transform.
-      // XXX Yeah, this is a bit hacky.
-      CSSPoint pos = CSSPixel::FromAppUnits(GetPosition());
-      CSSPoint scaledPos = CSSPoint(ownMatrix._11 * pos.x, ownMatrix._22 * pos.y);
-      CSSPoint deltaPos = scaledPos - pos;
-      ownMatrix *= gfxMatrix::Translation(-deltaPos.x, -deltaPos.y);
-    }
-
-    *aOwnTransform = gfx::ToMatrix(ownMatrix);
+// We want this frame to be a reference frame. An easy way to achieve that is
+// to always return true from this method, even for identity transforms.
+// This frame being a reference frame ensures that the offset between this
+// <svg> element and the parent reference frame is completely absorbed the
+// nsDisplayTransform that's created for this frame, and that this offset does
+// not affect our descendants' transforms. Consequently, if the <svg> element
+// moves, e.g. during scrolling, the transform matrices of our contents are
+// unaffected, and our contents are less likely to get accidentally invalidated
+// due to floating point inaccuracies in the matrix comparison.
+bool
+nsSVGOuterSVGAnonChildFrame::IsSVGTransformed(Matrix* aOwnTransform,
+                                              Matrix* aFromParentTransform) const
+{
+  if (aOwnTransform) {
+    *aOwnTransform = ComputeOuterSVGAnonChildFrameTransform(this);
   }
 
   return true;
 }