Bug 687830 - Simplify marker implementation by calculating viewbox on paint rather than attempting to cache it. r=roc
authorRobert Longson <longsonr@gmail.com>
Wed, 21 Sep 2011 08:53:09 +0100
changeset 78561 a3d4a447c8fc8512b169b9d5679f6ab198d5fab6
parent 78560 069e927983a90bf6d6b04e06f23b5aad05b087a4
child 78562 4ea7e20806667913ab2fc941b4d0605c53c263c2
child 78576 c0070ea57a4eea55d3ab2b05bd8d3daed6c2fa7a
push id78
push userclegnitto@mozilla.com
push dateFri, 16 Dec 2011 17:32:24 +0000
treeherdermozilla-release@79d24e644fdd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs687830
milestone9.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 687830 - Simplify marker implementation by calculating viewbox on paint rather than attempting to cache it. r=roc
content/svg/content/src/nsSVGMarkerElement.cpp
content/svg/content/src/nsSVGMarkerElement.h
layout/svg/base/src/nsSVGMarkerFrame.cpp
--- a/content/svg/content/src/nsSVGMarkerElement.cpp
+++ b/content/svg/content/src/nsSVGMarkerElement.cpp
@@ -283,74 +283,32 @@ nsSVGMarkerElement::ParseAttribute(PRInt
                                                 aValue, aResult);
 }
 
 nsresult
 nsSVGMarkerElement::UnsetAttr(PRInt32 aNamespaceID, nsIAtom* aName,
                               PRBool aNotify)
 {
   if (aNamespaceID == kNameSpaceID_None) {
-    if (aName == nsGkAtoms::viewBox && mCoordCtx) {
-      mViewBox.SetBaseValue(0, 0, mLengthAttributes[MARKERWIDTH].GetAnimValue(mCoordCtx),
-                            mLengthAttributes[MARKERHEIGHT].GetAnimValue(mCoordCtx),
-                            this, PR_FALSE);
-      return nsGenericElement::UnsetAttr(aNamespaceID, aName, aNotify);
-    } else if (aName == nsGkAtoms::orient) {
+    if (aName == nsGkAtoms::orient) {
       mOrientType.SetBaseValue(SVG_MARKER_ORIENT_ANGLE);
     }
   }
 
   return nsSVGMarkerElementBase::UnsetAttr(aNamespaceID, aName, aNotify);
 }
 
 //----------------------------------------------------------------------
 // nsSVGElement methods
 
-void
-nsSVGMarkerElement::DidChangeLength(PRUint8 aAttrEnum, PRBool aDoSetAttr)
-{
-  nsSVGMarkerElementBase::DidChangeLength(aAttrEnum, aDoSetAttr);
-
-  mViewBoxToViewportTransform = nsnull;
-
-  if (mCoordCtx && !HasAttr(kNameSpaceID_None, nsGkAtoms::viewBox) &&
-      (aAttrEnum == MARKERWIDTH || aAttrEnum == MARKERHEIGHT)) {
-    mViewBox.SetBaseValue(0, 0, mLengthAttributes[MARKERWIDTH].GetAnimValue(mCoordCtx),
-                          mLengthAttributes[MARKERHEIGHT].GetAnimValue(mCoordCtx),
-                          this, PR_FALSE);
-  }
-}
-
-void
-nsSVGMarkerElement::DidChangeViewBox(PRBool aDoSetAttr)
-{
-  nsSVGMarkerElementBase::DidChangeViewBox(aDoSetAttr);
-
-  mViewBoxToViewportTransform = nsnull;
-}
-
-void
-nsSVGMarkerElement::DidChangePreserveAspectRatio(PRBool aDoSetAttr)
-{
-  nsSVGMarkerElementBase::DidChangePreserveAspectRatio(aDoSetAttr);
-
-  mViewBoxToViewportTransform = nsnull;
-}
-
 void 
 nsSVGMarkerElement::SetParentCoordCtxProvider(nsSVGSVGElement *aContext)
 {
   mCoordCtx = aContext;
   mViewBoxToViewportTransform = nsnull;
-
-  if (mCoordCtx && !HasAttr(kNameSpaceID_None, nsGkAtoms::viewBox)) {
-    mViewBox.SetBaseValue(0, 0, mLengthAttributes[MARKERWIDTH].GetAnimValue(mCoordCtx),
-                          mLengthAttributes[MARKERHEIGHT].GetAnimValue(mCoordCtx),
-                          this, PR_FALSE);
-  }
 }
 
 nsSVGElement::LengthAttributesInfo
 nsSVGMarkerElement::GetLengthInfo()
 {
   return LengthAttributesInfo(mLengthAttributes, sLengthInfo,
                               NS_ARRAY_LENGTH(sLengthInfo));
 }
@@ -383,55 +341,64 @@ nsSVGMarkerElement::GetPreserveAspectRat
 
 //----------------------------------------------------------------------
 // public helpers
 
 gfxMatrix
 nsSVGMarkerElement::GetMarkerTransform(float aStrokeWidth,
                                        float aX, float aY, float aAutoAngle)
 {
-  float scale = 1.0;
-  if (mEnumAttributes[MARKERUNITS].GetAnimValue() ==
-      SVG_MARKERUNITS_STROKEWIDTH)
-    scale = aStrokeWidth;
+  gfxFloat scale = mEnumAttributes[MARKERUNITS].GetAnimValue() ==
+                     SVG_MARKERUNITS_STROKEWIDTH ? aStrokeWidth : 1.0;
 
-  float angle = mOrientType.GetAnimValue() == SVG_MARKER_ORIENT_AUTO ?
-                aAutoAngle :
-                mAngleAttributes[ORIENT].GetAnimValue() * M_PI / 180.0;
+  gfxFloat angle = mOrientType.GetAnimValue() == SVG_MARKER_ORIENT_AUTO ?
+                    aAutoAngle :
+                    mAngleAttributes[ORIENT].GetAnimValue() * M_PI / 180.0;
 
   return gfxMatrix(cos(angle) * scale,   sin(angle) * scale,
                    -sin(angle) * scale,  cos(angle) * scale,
                    aX,                    aY);
 }
 
+nsSVGViewBoxRect
+nsSVGMarkerElement::GetViewBoxRect()
+{
+  if (mViewBox.IsValid()) {
+    return mViewBox.GetAnimValue();
+  }
+  return nsSVGViewBoxRect(
+           0, 0,
+           mLengthAttributes[MARKERWIDTH].GetAnimValue(mCoordCtx),
+           mLengthAttributes[MARKERHEIGHT].GetAnimValue(mCoordCtx));
+}
+
 gfxMatrix
 nsSVGMarkerElement::GetViewBoxTransform()
 {
   if (!mViewBoxToViewportTransform) {
     float viewportWidth =
       mLengthAttributes[MARKERWIDTH].GetAnimValue(mCoordCtx);
     float viewportHeight = 
       mLengthAttributes[MARKERHEIGHT].GetAnimValue(mCoordCtx);
    
-    const nsSVGViewBoxRect& viewbox = mViewBox.GetAnimValue(); 
+    nsSVGViewBoxRect viewbox = GetViewBoxRect();
 
-    if (viewbox.width <= 0.0f || viewbox.height <= 0.0f) {
-      return gfxMatrix(0.0, 0.0, 0.0, 0.0, 0.0, 0.0); // invalid - don't paint element
-    }
-
-    float refX = mLengthAttributes[REFX].GetAnimValue(mCoordCtx);
-    float refY = mLengthAttributes[REFY].GetAnimValue(mCoordCtx);
+    NS_ABORT_IF_FALSE(viewbox.width > 0.0f && viewbox.height > 0.0f,
+                      "Rendering should be disabled");
 
     gfxMatrix viewBoxTM =
       nsSVGUtils::GetViewBoxTransform(this,
                                       viewportWidth, viewportHeight,
                                       viewbox.x, viewbox.y,
                                       viewbox.width, viewbox.height,
                                       mPreserveAspectRatio);
 
+    float refX = mLengthAttributes[REFX].GetAnimValue(mCoordCtx);
+    float refY = mLengthAttributes[REFY].GetAnimValue(mCoordCtx);
+
     gfxPoint ref = viewBoxTM.Transform(gfxPoint(refX, refY));
 
     gfxMatrix TM = viewBoxTM * gfxMatrix().Translate(gfxPoint(-ref.x, -ref.y));
 
     mViewBoxToViewportTransform = NS_NewSVGMatrix(TM);
   }
 
   return nsSVGUtils::ConvertSVGMatrixToThebes(mViewBoxToViewportTransform);
--- a/content/svg/content/src/nsSVGMarkerElement.h
+++ b/content/svg/content/src/nsSVGMarkerElement.h
@@ -127,24 +127,20 @@ public:
   // nsIContent interface
   NS_IMETHOD_(PRBool) IsAttributeMapped(const nsIAtom* name) const;
 
   virtual PRBool GetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
                          nsAString& aResult) const;
   virtual nsresult UnsetAttr(PRInt32 aNameSpaceID, nsIAtom* aAttribute,
                              PRBool aNotify);
 
-  // nsSVGElement specializations:
-  virtual void DidChangeLength(PRUint8 aAttrEnum, PRBool aDoSetAttr);
-  virtual void DidChangeViewBox(PRBool aDoSetAttr);
-  virtual void DidChangePreserveAspectRatio(PRBool aDoSetAttr);
-
   // public helpers
   gfxMatrix GetMarkerTransform(float aStrokeWidth,
                                float aX, float aY, float aAutoAngle);
+  nsSVGViewBoxRect GetViewBoxRect();
   gfxMatrix GetViewBoxTransform();
 
   virtual nsresult Clone(nsINodeInfo *aNodeInfo, nsINode **aResult) const;
 
   nsSVGOrientType* GetOrientType() { return &mOrientType; }
 
   virtual nsXPCClassInfo* GetClassInfo();
 protected:
--- a/layout/svg/base/src/nsSVGMarkerFrame.cpp
+++ b/layout/svg/base/src/nsSVGMarkerFrame.cpp
@@ -127,50 +127,39 @@ nsSVGMarkerFrame::PaintMark(nsSVGRenderS
                             nsSVGMark *aMark, float aStrokeWidth)
 {
   // If the flag is set when we get here, it means this marker frame
   // has already been used painting the current mark, and the document
   // has a marker reference loop.
   if (mInUse)
     return NS_OK;
 
+  AutoMarkerReferencer markerRef(this, aMarkedFrame);
+
   nsSVGMarkerElement *marker = static_cast<nsSVGMarkerElement*>(mContent);
 
-  nsCOMPtr<nsIDOMSVGAnimatedRect> arect;
-  nsresult rv = marker->GetViewBox(getter_AddRefs(arect));
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  nsCOMPtr<nsIDOMSVGRect> rect;
-  rv = arect->GetAnimVal(getter_AddRefs(rect));
-  NS_ENSURE_SUCCESS(rv, rv);
+  const nsSVGViewBoxRect viewBox = marker->GetViewBoxRect();
 
-  float x, y, width, height;
-  rect->GetX(&x);
-  rect->GetY(&y);
-  rect->GetWidth(&width);
-  rect->GetHeight(&height);
-
-  if (width <= 0.0f || height <= 0.0f) {
+  if (viewBox.width <= 0.0f || viewBox.height <= 0.0f) {
     // We must disable rendering if the viewBox width or height are zero.
     return NS_OK;
   }
 
-  AutoMarkerReferencer markerRef(this, aMarkedFrame);
-
   mStrokeWidth = aStrokeWidth;
   mX = aMark->x;
   mY = aMark->y;
   mAutoAngle = aMark->angle;
 
   gfxContext *gfx = aContext->GetGfxContext();
 
   if (GetStyleDisplay()->IsScrollableOverflow()) {
     gfx->Save();
     gfxRect clipRect =
-      nsSVGUtils::GetClipRectForFrame(this, x, y, width, height);
+      nsSVGUtils::GetClipRectForFrame(this, viewBox.x, viewBox.y,
+                                      viewBox.width, viewBox.height);
     nsSVGUtils::SetClipRect(gfx, GetCanvasTM(), clipRect);
   }
 
   for (nsIFrame* kid = mFrames.FirstChild(); kid;
        kid = kid->GetNextSibling()) {
     nsISVGChildFrame* SVGFrame = do_QueryFrame(kid);
     if (SVGFrame) {
       // The CTM of each frame referencing us may be different.