Bug 689546 - Simplify attribute updates to svg elements. r=jwatt
authorRobert Longson <longsonr@gmail.com>
Fri, 30 Sep 2011 09:25:01 +0100
changeset 77908 831df43787ef647af37fa028fc7fa9a14dbe0578
parent 77907 8232007db7e5107a110f344b7587cf72568d1ab3
child 77909 d74000e4f76a2cfe0b7995ad5a7db2b1adc6dc0c
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewersjwatt
bugs689546
milestone10.0a1
Bug 689546 - Simplify attribute updates to svg elements. r=jwatt
content/svg/content/src/nsSVGSVGElement.cpp
content/svg/content/src/nsSVGSVGElement.h
layout/svg/base/src/nsSVGForeignObjectFrame.cpp
layout/svg/base/src/nsSVGInnerSVGFrame.cpp
layout/svg/base/src/nsSVGOuterSVGFrame.cpp
--- a/content/svg/content/src/nsSVGSVGElement.cpp
+++ b/content/svg/content/src/nsSVGSVGElement.cpp
@@ -1195,76 +1195,36 @@ nsSVGSVGElement::PrependLocalTransformTo
     zoomPanTM.Scale(mCurrentScale, mCurrentScale);
     return GetViewBoxTransform() * zoomPanTM * aMatrix;
   }
 
   // outer-<svg>, but inline in some other content:
   return GetViewBoxTransform() * aMatrix;
 }
 
-void
-nsSVGSVGElement::DidChangeLength(PRUint8 aAttrEnum, bool aDoSetAttr)
-{
-  nsSVGSVGElementBase::DidChangeLength(aAttrEnum, aDoSetAttr);
-
-  InvalidateTransformNotifyFrame();
-}
-
 nsSVGElement::LengthAttributesInfo
 nsSVGSVGElement::GetLengthInfo()
 {
   return LengthAttributesInfo(mLengthAttributes, sLengthInfo,
                               NS_ARRAY_LENGTH(sLengthInfo));
 }
 
 nsSVGElement::EnumAttributesInfo
 nsSVGSVGElement::GetEnumInfo()
 {
   return EnumAttributesInfo(mEnumAttributes, sEnumInfo,
                             NS_ARRAY_LENGTH(sEnumInfo));
 }
 
-void
-nsSVGSVGElement::DidChangeViewBox(bool aDoSetAttr)
-{
-  nsSVGSVGElementBase::DidChangeViewBox(aDoSetAttr);
-
-  InvalidateTransformNotifyFrame();
-}
-
-void
-nsSVGSVGElement::DidAnimateViewBox()
-{
-  nsSVGSVGElementBase::DidAnimateViewBox();
-  
-  InvalidateTransformNotifyFrame();
-}
-
 nsSVGViewBox *
 nsSVGSVGElement::GetViewBox()
 {
   return &mViewBox;
 }
 
-void
-nsSVGSVGElement::DidChangePreserveAspectRatio(bool aDoSetAttr)
-{
-  nsSVGSVGElementBase::DidChangePreserveAspectRatio(aDoSetAttr);
-
-  InvalidateTransformNotifyFrame();
-}
-
-void
-nsSVGSVGElement::DidAnimatePreserveAspectRatio()
-{
-  nsSVGSVGElementBase::DidAnimatePreserveAspectRatio();
-
-  InvalidateTransformNotifyFrame();
-}
-
 SVGAnimatedPreserveAspectRatio *
 nsSVGSVGElement::GetPreserveAspectRatio()
 {
   return &mPreserveAspectRatio;
 }
 
 bool
 nsSVGSVGElement::ShouldSynthesizeViewBox() const
--- a/content/svg/content/src/nsSVGSVGElement.h
+++ b/content/svg/content/src/nsSVGSVGElement.h
@@ -188,22 +188,16 @@ public:
   // nsIContent interface
   NS_IMETHOD_(bool) IsAttributeMapped(const nsIAtom* aAttribute) const;
 #ifdef MOZ_SMIL
   virtual nsresult PreHandleEvent(nsEventChainPreVisitor& aVisitor);
 #endif // MOZ_SMIL
 
   // nsSVGElement specializations:
   virtual gfxMatrix PrependLocalTransformTo(const gfxMatrix &aMatrix) const;
-  virtual void DidChangeLength(PRUint8 aAttrEnum, bool aDoSetAttr);
-  virtual void DidChangeViewBox(bool aDoSetAttr);
-  virtual void DidChangePreserveAspectRatio(bool aDoSetAttr);
-
-  virtual void DidAnimateViewBox();
-  virtual void DidAnimatePreserveAspectRatio();
   
   // nsSVGSVGElement methods:
   float GetLength(PRUint8 mCtxType);
 
   // public helpers:
   gfxMatrix GetViewBoxTransform() const;
   bool      HasValidViewbox() const { return mViewBox.IsValid(); }
 
--- a/layout/svg/base/src/nsSVGForeignObjectFrame.cpp
+++ b/layout/svg/base/src/nsSVGForeignObjectFrame.cpp
@@ -118,16 +118,18 @@ nsSVGForeignObjectFrame::AttributeChange
   if (aNameSpaceID == kNameSpaceID_None) {
     if (aAttribute == nsGkAtoms::width ||
         aAttribute == nsGkAtoms::height) {
       UpdateGraphic(); // update mRect before requesting reflow
       // XXXjwatt: why mark intrinsic widths dirty? can't we just use eResize?
       RequestReflow(nsIPresShell::eStyleChange);
     } else if (aAttribute == nsGkAtoms::x ||
                aAttribute == nsGkAtoms::y ||
+               aAttribute == nsGkAtoms::viewBox ||
+               aAttribute == nsGkAtoms::preserveAspectRatio ||
                aAttribute == nsGkAtoms::transform) {
       // make sure our cached transform matrix gets (lazily) updated
       mCanvasTM = nsnull;
       UpdateGraphic();
     }
   }
 
   return NS_OK;
--- a/layout/svg/base/src/nsSVGInnerSVGFrame.cpp
+++ b/layout/svg/base/src/nsSVGInnerSVGFrame.cpp
@@ -120,17 +120,17 @@ nsSVGInnerSVGFrame::NotifySVGChanged(PRU
 
     // Coordinate context changes affect mCanvasTM if we have a
     // percentage 'x' or 'y', or if we have a percentage 'width' or 'height' AND
     // a 'viewBox'.
 
     if (!(aFlags & TRANSFORM_CHANGED) &&
         (svg->mLengthAttributes[nsSVGSVGElement::X].IsPercentage() ||
          svg->mLengthAttributes[nsSVGSVGElement::Y].IsPercentage() ||
-         (mContent->HasAttr(kNameSpaceID_None, nsGkAtoms::viewBox) &&
+         (svg->mViewBox.IsValid() &&
           (svg->mLengthAttributes[nsSVGSVGElement::WIDTH].IsPercentage() ||
            svg->mLengthAttributes[nsSVGSVGElement::HEIGHT].IsPercentage())))) {
     
       aFlags |= TRANSFORM_CHANGED;
     }
 
     // XXX We could clear the COORD_CONTEXT_CHANGED flag in some circumstances
     // if we have a non-percentage 'width' AND 'height, or if we have a 'viewBox'
@@ -158,27 +158,39 @@ nsSVGInnerSVGFrame::NotifySVGChanged(PRU
 
 NS_IMETHODIMP
 nsSVGInnerSVGFrame::AttributeChanged(PRInt32  aNameSpaceID,
                                      nsIAtom* aAttribute,
                                      PRInt32  aModType)
 {
   if (aNameSpaceID == kNameSpaceID_None) {
     if (aAttribute == nsGkAtoms::width ||
-        aAttribute == nsGkAtoms::height ||
-        aAttribute == nsGkAtoms::preserveAspectRatio ||
-        aAttribute == nsGkAtoms::viewBox) {
-      nsSVGUtils::UpdateGraphic(this);
+        aAttribute == nsGkAtoms::height) {
+
+      if (static_cast<nsSVGSVGElement*>(mContent)->mViewBox.IsValid()) {
+
+        // make sure our cached transform matrix gets (lazily) updated
+        mCanvasTM = nsnull;
+
+        nsSVGUtils::NotifyChildrenOfSVGChange(this, TRANSFORM_CHANGED);
+      } else {
+        nsSVGUtils::NotifyChildrenOfSVGChange(this, COORD_CONTEXT_CHANGED);
+      }
+
     } else if (aAttribute == nsGkAtoms::transform ||
+               aAttribute == nsGkAtoms::preserveAspectRatio ||
+               aAttribute == nsGkAtoms::viewBox ||
                aAttribute == nsGkAtoms::x ||
                aAttribute == nsGkAtoms::y) {
       // make sure our cached transform matrix gets (lazily) updated
       mCanvasTM = nsnull;
 
-      nsSVGUtils::NotifyChildrenOfSVGChange(this, TRANSFORM_CHANGED);
+      nsSVGUtils::NotifyChildrenOfSVGChange(
+          this, aAttribute == nsGkAtoms::viewBox ?
+                  TRANSFORM_CHANGED | COORD_CONTEXT_CHANGED : TRANSFORM_CHANGED);
     }
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP_(nsIFrame*)
 nsSVGInnerSVGFrame::GetFrameForPoint(const nsPoint &aPoint)
@@ -224,42 +236,18 @@ nsSVGInnerSVGFrame::UnsuspendRedraw()
     return NS_ERROR_FAILURE;
   }
   return outerSVGFrame->UnsuspendRedraw();
 }
 
 NS_IMETHODIMP
 nsSVGInnerSVGFrame::NotifyViewportChange()
 {
-  PRUint32 flags = COORD_CONTEXT_CHANGED;
-
-#if 1
-  // XXX nsSVGSVGElement::InvalidateTransformNotifyFrame calls us for changes
-  // to 'x' and 'y'. Until this is fixed, add TRANSFORM_CHANGED to flags
-  // unconditionally.
-
-  flags |= TRANSFORM_CHANGED;
-
-  // make sure canvas transform matrix gets (lazily) recalculated:
-  mCanvasTM = nsnull;
-#else
-  // viewport changes only affect our transform if we have a viewBox attribute
-  if (mContent->HasAttr(kNameSpaceID_None, nsGkAtoms::viewBox)) {
-    // make sure canvas transform matrix gets (lazily) recalculated:
-    mCanvasTM = nsnull;
-
-    flags |= TRANSFORM_CHANGED;
-  }
-#endif
-  
-  // inform children
-  SuspendRedraw();
-  nsSVGUtils::NotifyChildrenOfSVGChange(this, flags);
-  UnsuspendRedraw();
-  return NS_OK;
+  NS_ERROR("Inner SVG frames should not get Viewport changes.");
+  return NS_ERROR_FAILURE;
 }
 
 //----------------------------------------------------------------------
 // nsSVGContainerFrame methods:
 
 gfxMatrix
 nsSVGInnerSVGFrame::GetCanvasTM()
 {
--- a/layout/svg/base/src/nsSVGOuterSVGFrame.cpp
+++ b/layout/svg/base/src/nsSVGOuterSVGFrame.cpp
@@ -525,32 +525,46 @@ DependsOnIntrinsicSize(const nsIFrame* a
 }
 
 NS_IMETHODIMP
 nsSVGOuterSVGFrame::AttributeChanged(PRInt32  aNameSpaceID,
                                      nsIAtom* aAttribute,
                                      PRInt32  aModType)
 {
   if (aNameSpaceID == kNameSpaceID_None &&
-      !(GetStateBits() & NS_FRAME_FIRST_REFLOW) &&
-      (aAttribute == nsGkAtoms::width || aAttribute == nsGkAtoms::height)) {
-    nsIFrame* embeddingFrame;
-    if (IsRootOfReplacedElementSubDoc(&embeddingFrame) && embeddingFrame) {
-      if (DependsOnIntrinsicSize(embeddingFrame)) {
-        // Tell embeddingFrame's presShell it needs to be reflowed (which takes
-        // care of reflowing us too).
-        embeddingFrame->PresContext()->PresShell()->
-          FrameNeedsReflow(embeddingFrame, nsIPresShell::eStyleChange, NS_FRAME_IS_DIRTY);
-      }
-      // else our width and height is overridden - don't reflow anything
-    } else {
-      // We are not embedded by reference, so our 'width' and 'height'
-      // attributes are not overridden - we need to reflow.
-      PresContext()->PresShell()->
-        FrameNeedsReflow(this, nsIPresShell::eStyleChange, NS_FRAME_IS_DIRTY);
+      !(GetStateBits() & NS_FRAME_FIRST_REFLOW)) {
+    if (aAttribute == nsGkAtoms::viewBox ||
+        aAttribute == nsGkAtoms::preserveAspectRatio ||
+        aAttribute == nsGkAtoms::transform) {
+
+      // make sure our cached transform matrix gets (lazily) updated
+      mCanvasTM = nsnull;
+
+      nsSVGUtils::NotifyChildrenOfSVGChange(
+          this, aAttribute == nsGkAtoms::viewBox ?
+                  TRANSFORM_CHANGED | COORD_CONTEXT_CHANGED : TRANSFORM_CHANGED);
+
+    } else if (aAttribute == nsGkAtoms::width ||
+               aAttribute == nsGkAtoms::height) {
+
+        nsIFrame* embeddingFrame;
+        if (IsRootOfReplacedElementSubDoc(&embeddingFrame) && embeddingFrame) {
+          if (DependsOnIntrinsicSize(embeddingFrame)) {
+            // Tell embeddingFrame's presShell it needs to be reflowed (which takes
+            // care of reflowing us too).
+            embeddingFrame->PresContext()->PresShell()->
+              FrameNeedsReflow(embeddingFrame, nsIPresShell::eStyleChange, NS_FRAME_IS_DIRTY);
+          }
+          // else our width and height is overridden - don't reflow anything
+        } else {
+          // We are not embedded by reference, so our 'width' and 'height'
+          // attributes are not overridden - we need to reflow.
+          PresContext()->PresShell()->
+            FrameNeedsReflow(this, nsIPresShell::eStyleChange, NS_FRAME_IS_DIRTY);
+        }
     }
   }
 
   return NS_OK;
 }
 
 //----------------------------------------------------------------------
 // painting