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 77250 a3d4a447c8fc8512b169b9d5679f6ab198d5fab6
parent 77249 069e927983a90bf6d6b04e06f23b5aad05b087a4
child 77251 4ea7e20806667913ab2fc941b4d0605c53c263c2
child 77265 c0070ea57a4eea55d3ab2b05bd8d3daed6c2fa7a
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewersroc
bugs687830
milestone9.0a1
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.