Bug 767647 - Stop invalidating once for every SVG descendant of a changed SVG container, and stop invalidating the descendants' rendering observers. r=longsonr.

content/svg/content/src/nsSVGUseElement.cpp

@@ -369,6 +369,17 @@ nsSVGUseElement::DestroyAnonymousContent()
   nsContentUtils::DestroyAnonymousContent(&mClone);
 }
 
+bool
+nsSVGUseElement::OurWidthAndHeightAreUsed() const
+{
+  if (mClone) {
+    nsCOMPtr<nsIDOMSVGSVGElement>    svg    = do_QueryInterface(mClone);
+    nsCOMPtr<nsIDOMSVGSymbolElement> symbol = do_QueryInterface(mClone);
+    return svg || symbol;
+  }
+  return false;
+}
+
 //----------------------------------------------------------------------
 // implementation helpers
 
@@ -377,6 +388,7 @@ nsSVGUseElement::SyncWidthOrHeight(nsIAtom* aName)
 {
   NS_ASSERTION(aName == nsGkAtoms::width || aName == nsGkAtoms::height,
                "The clue is in the function name");
+  NS_ASSERTION(OurWidthAndHeightAreUsed(), "Don't call this");
 
   if (!mClone) {
     return;

content/svg/content/src/nsSVGUseElement.h

@@ -102,6 +102,12 @@ protected:
   virtual LengthAttributesInfo GetLengthInfo();
   virtual StringAttributesInfo GetStringInfo();
 
+  /**
+   * Returns true if our width and height should be used, or false if they
+   * should be ignored. As per the spec, this depends on the type of the
+   * element that we're referencing.
+   */
+  bool OurWidthAndHeightAreUsed() const;
   void SyncWidthOrHeight(nsIAtom *aName);
   void LookupHref();
   void TriggerReclone();

layout/svg/base/src/nsISVGChildFrame.h

@@ -80,6 +80,15 @@ public:
     COORD_CONTEXT_CHANGED = 0x04,
     FULL_ZOOM_CHANGED     = 0x08
   };
+  /**
+   * This is called on a frame when there has been a change to one of its
+   * ancestors that might affect the frame too. SVGChangedFlags are passed
+   * to indicate what changed.
+   *
+   * Implementations do not need to invalidate, since the caller will 
+   * invalidate the entire area of the ancestor that changed. However, they
+   * may need to update their bounds.
+   */
   virtual void NotifySVGChanged(PRUint32 aFlags)=0;
 
   /**

layout/svg/base/src/nsSVGAFrame.cpp

@@ -106,7 +106,7 @@ nsSVGAFrame::AttributeChanged(PRInt32         aNameSpaceID,
 {
   if (aNameSpaceID == kNameSpaceID_None &&
       aAttribute == nsGkAtoms::transform) {
-
+    nsSVGUtils::InvalidateAndScheduleBoundsUpdate(this);
     NotifySVGChanged(TRANSFORM_CHANGED);
   }
 

layout/svg/base/src/nsSVGContainerFrame.cpp

@@ -278,8 +278,13 @@ nsSVGDisplayContainerFrame::UpdateBounds()
   mState &= ~(NS_FRAME_FIRST_REFLOW | NS_FRAME_IS_DIRTY |
               NS_FRAME_HAS_DIRTY_CHILDREN);
 
-  // XXXSDL Make Invalidate() call nsSVGUtils::InvalidateBounds(this)
-  // so that we invalidate under FinishAndStoreOverflow().
+  if (!(GetParent()->GetStateBits() & NS_FRAME_FIRST_REFLOW)) {
+    // We only invalidate if our outer-<svg> has already had its
+    // initial reflow (since if it hasn't, its entire area will be
+    // invalidated when it gets that initial reflow):
+    // XXXSDL Let FinishAndStoreOverflow do this.
+    nsSVGUtils::InvalidateBounds(this, true);
+  }
 }  
 
 void

layout/svg/base/src/nsSVGForeignObjectFrame.cpp

@@ -425,7 +425,9 @@ nsSVGForeignObjectFrame::NotifySVGChanged(PRUint32 aFlags)
   }
 
   if (aFlags & TRANSFORM_CHANGED) {
-    needNewBounds = true; // needed if it was _our_ transform that changed
+    if (mCanvasTM && mCanvasTM->IsSingular()) {
+      needNewBounds = true; // old bounds are bogus
+    }
     needNewCanvasTM = true;
     // In an ideal world we would reflow when our CTM changes. This is because
     // glyph metrics do not necessarily scale uniformly with change in scale
@@ -436,9 +438,13 @@ nsSVGForeignObjectFrame::NotifySVGChanged(PRUint32 aFlags)
     // reflow.
   }
 
-  if (needNewBounds &&
-      !(aFlags & DO_NOT_NOTIFY_RENDERING_OBSERVERS)) {
-    nsSVGUtils::InvalidateAndScheduleBoundsUpdate(this);
+  if (needNewBounds) {
+    // Ancestor changes can't affect how we render from the perspective of
+    // any rendering observers that we may have, so we don't need to
+    // invalidate them. We also don't need to invalidate ourself, since our
+    // changed ancestor will have invalidated its entire area, which includes
+    // our area.
+    nsSVGUtils::ScheduleBoundsUpdate(this);
   }
 
   // If we're called while the PresShell is handling reflow events then we

layout/svg/base/src/nsSVGGFrame.cpp

@@ -88,7 +88,7 @@ nsSVGGFrame::AttributeChanged(PRInt32         aNameSpaceID,
 {
   if (aNameSpaceID == kNameSpaceID_None &&
       aAttribute == nsGkAtoms::transform) {
-
+    nsSVGUtils::InvalidateAndScheduleBoundsUpdate(this);
     NotifySVGChanged(TRANSFORM_CHANGED);
   }
   

layout/svg/base/src/nsSVGGlyphFrame.cpp

@@ -506,11 +506,14 @@ nsSVGGlyphFrame::NotifySVGChanged(PRUint32 aFlags)
   NS_ABORT_IF_FALSE(aFlags & (TRANSFORM_CHANGED | COORD_CONTEXT_CHANGED),
                     "Invalidation logic may need adjusting");
 
-  // XXXjwatt: seems to me that this could change the glyph metrics,
-  // in which case we should call NotifyGlyphMetricsChange instead.
-  if (!(aFlags & DO_NOT_NOTIFY_RENDERING_OBSERVERS)) {
-    nsSVGUtils::InvalidateAndScheduleBoundsUpdate(this);
-  }
+  // Ancestor changes can't affect how we render from the perspective of
+  // any rendering observers that we may have, so we don't need to
+  // invalidate them. We also don't need to invalidate ourself, since our
+  // changed ancestor will have invalidated its entire area, which includes
+  // our area.
+  // XXXjwatt: seems to me that our ancestor's change could change our glyph
+  // metrics, in which case we should call NotifyGlyphMetricsChange instead.
+  nsSVGUtils::ScheduleBoundsUpdate(this);
 
   if (aFlags & TRANSFORM_CHANGED) {
     ClearTextRun();

layout/svg/base/src/nsSVGInnerSVGFrame.cpp

@@ -104,27 +104,40 @@ nsSVGInnerSVGFrame::NotifySVGChanged(PRUint32 aFlags)
   NS_ABORT_IF_FALSE(aFlags & (TRANSFORM_CHANGED | COORD_CONTEXT_CHANGED),
                     "Invalidation logic may need adjusting");
 
+  bool updateBounds = false;
+
   if (aFlags & COORD_CONTEXT_CHANGED) {
 
     nsSVGSVGElement *svg = static_cast<nsSVGSVGElement*>(mContent);
 
+    bool xOrYIsPercentage =
+      svg->mLengthAttributes[nsSVGSVGElement::X].IsPercentage() ||
+      svg->mLengthAttributes[nsSVGSVGElement::Y].IsPercentage();
+    bool widthOrHeightIsPercentage =
+      svg->mLengthAttributes[nsSVGSVGElement::WIDTH].IsPercentage() ||
+      svg->mLengthAttributes[nsSVGSVGElement::HEIGHT].IsPercentage();
+
+    if (xOrYIsPercentage || widthOrHeightIsPercentage) {
+      // Ancestor changes can't affect how we render from the perspective of
+      // any rendering observers that we may have, so we don't need to
+      // invalidate them. We also don't need to invalidate ourself, since our
+      // changed ancestor will have invalidated its entire area, which includes
+      // our area.
+      // For perf reasons we call this before calling NotifySVGChanged() below.
+      nsSVGUtils::ScheduleBoundsUpdate(this);
+    }
+
     // 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() ||
-         (svg->HasViewBox() &&
-          (svg->mLengthAttributes[nsSVGSVGElement::WIDTH].IsPercentage() ||
-           svg->mLengthAttributes[nsSVGSVGElement::HEIGHT].IsPercentage())))) {
-    
+        (xOrYIsPercentage ||
+         (widthOrHeightIsPercentage && svg->HasViewBox()))) {
       aFlags |= TRANSFORM_CHANGED;
     }
 
-    if (svg->HasViewBox() ||
-        (!svg->mLengthAttributes[nsSVGSVGElement::WIDTH].IsPercentage() &&
-         !svg->mLengthAttributes[nsSVGSVGElement::HEIGHT].IsPercentage())) {
+    if (svg->HasViewBox() || !widthOrHeightIsPercentage) {
       // Remove COORD_CONTEXT_CHANGED, since we establish the coordinate
       // context for our descendants and this notification won't change its
       // dimensions:
@@ -155,6 +168,7 @@ nsSVGInnerSVGFrame::AttributeChanged(PRInt32  aNameSpaceID,
 
     if (aAttribute == nsGkAtoms::width ||
         aAttribute == nsGkAtoms::height) {
+      nsSVGUtils::InvalidateAndScheduleBoundsUpdate(this);
 
       if (content->HasViewBoxOrSyntheticViewBox()) {
         // make sure our cached transform matrix gets (lazily) updated
@@ -178,6 +192,8 @@ nsSVGInnerSVGFrame::AttributeChanged(PRInt32  aNameSpaceID,
       // make sure our cached transform matrix gets (lazily) updated
       mCanvasTM = nsnull;
 
+      nsSVGUtils::InvalidateAndScheduleBoundsUpdate(this);
+
       nsSVGUtils::NotifyChildrenOfSVGChange(
           this, aAttribute == nsGkAtoms::viewBox ?
                   TRANSFORM_CHANGED | COORD_CONTEXT_CHANGED : TRANSFORM_CHANGED);

layout/svg/base/src/nsSVGPathGeometryFrame.cpp

@@ -265,9 +265,12 @@ nsSVGPathGeometryFrame::NotifySVGChanged(PRUint32 aFlags)
   NS_ABORT_IF_FALSE(aFlags & (TRANSFORM_CHANGED | COORD_CONTEXT_CHANGED),
                     "Invalidation logic may need adjusting");
 
-  if (!(aFlags & DO_NOT_NOTIFY_RENDERING_OBSERVERS)) {
-    nsSVGUtils::InvalidateAndScheduleBoundsUpdate(this);
-  }
+  // Ancestor changes can't affect how we render from the perspective of
+  // any rendering observers that we may have, so we don't need to
+  // invalidate them. We also don't need to invalidate ourself, since our
+  // changed ancestor will have invalidated its entire area, which includes
+  // our area.
+  nsSVGUtils::ScheduleBoundsUpdate(this);
 }
 
 SVGBBox

layout/svg/base/src/nsSVGSwitchFrame.cpp

@@ -191,8 +191,13 @@ nsSVGSwitchFrame::UpdateBounds()
   mState &= ~(NS_FRAME_FIRST_REFLOW | NS_FRAME_IS_DIRTY |
               NS_FRAME_HAS_DIRTY_CHILDREN);
 
-  // XXXSDL Make Invalidate() call nsSVGUtils::InvalidateBounds(this)
-  // so that we invalidate under FinishAndStoreOverflow().
+  if (!(GetParent()->GetStateBits() & NS_FRAME_FIRST_REFLOW)) {
+    // We only invalidate if our outer-<svg> has already had its
+    // initial reflow (since if it hasn't, its entire area will be
+    // invalidated when it gets that initial reflow):
+    // XXXSDL Let FinishAndStoreOverflow do this.
+    nsSVGUtils::InvalidateBounds(this, true);
+  }
 }
 
 SVGBBox

layout/svg/base/src/nsSVGTSpanFrame.cpp

@@ -79,6 +79,7 @@ nsSVGTSpanFrame::AttributeChanged(PRInt32         aNameSpaceID,
        aAttribute == nsGkAtoms::dx ||
        aAttribute == nsGkAtoms::dy ||
        aAttribute == nsGkAtoms::rotate)) {
+    nsSVGUtils::InvalidateAndScheduleBoundsUpdate(this);
     NotifyGlyphMetricsChange();
   }
 

layout/svg/base/src/nsSVGTextFrame.cpp

@@ -55,14 +55,14 @@ nsSVGTextFrame::AttributeChanged(PRInt32         aNameSpaceID,
     return NS_OK;
 
   if (aAttribute == nsGkAtoms::transform) {
-
+    nsSVGUtils::InvalidateAndScheduleBoundsUpdate(this);
     NotifySVGChanged(TRANSFORM_CHANGED);
-   
   } else if (aAttribute == nsGkAtoms::x ||
              aAttribute == nsGkAtoms::y ||
              aAttribute == nsGkAtoms::dx ||
              aAttribute == nsGkAtoms::dy ||
              aAttribute == nsGkAtoms::rotate) {
+    nsSVGUtils::InvalidateAndScheduleBoundsUpdate(this);
     NotifyGlyphMetricsChange();
   }
 
@@ -169,6 +169,16 @@ nsSVGTextFrame::NotifySVGChanged(PRUint32 aFlags)
     mCanvasTM = nsnull;
   }
 
+  if (updateGlyphMetrics) {
+    // Ancestor changes can't affect how we render from the perspective of
+    // any rendering observers that we may have, so we don't need to
+    // invalidate them. We also don't need to invalidate ourself, since our
+    // changed ancestor will have invalidated its entire area, which includes
+    // our area.
+    // For perf reasons we call this before calling NotifySVGChanged() below.
+    nsSVGUtils::ScheduleBoundsUpdate(this);
+  }
+
   nsSVGTextFrameBase::NotifySVGChanged(aFlags);
 
   if (updateGlyphMetrics) {
@@ -224,8 +234,13 @@ nsSVGTextFrame::UpdateBounds()
   // areas correctly:
   nsSVGTextFrameBase::UpdateBounds();
 
-  // XXXSDL once we store bounds on containers, call
-  // nsSVGUtils::InvalidateBounds(this) if not first reflow.
+  if (!(GetParent()->GetStateBits() & NS_FRAME_FIRST_REFLOW)) {
+    // We only invalidate if our outer-<svg> has already had its
+    // initial reflow (since if it hasn't, its entire area will be
+    // invalidated when it gets that initial reflow):
+    // XXXSDL Let FinishAndStoreOverflow do this.
+    nsSVGUtils::InvalidateBounds(this, true);
+  }
 }
 
 SVGBBox
@@ -260,9 +275,32 @@ nsSVGTextFrame::GetCanvasTM()
 //----------------------------------------------------------------------
 //
 
+static void
+MarkDirtyBitsOnDescendants(nsIFrame *aFrame)
+{
+  if (aFrame->GetStateBits() & (NS_FRAME_IS_DIRTY | NS_FRAME_FIRST_REFLOW)) {
+    // Nothing to do if we're already dirty, or if the outer-<svg>
+    // hasn't yet had its initial reflow.
+    return;
+  }
+  nsIFrame* kid = aFrame->GetFirstPrincipalChild();
+  while (kid) {
+    nsISVGChildFrame* svgkid = do_QueryFrame(kid);
+    if (svgkid && !(kid->GetStateBits() & NS_FRAME_IS_DIRTY)) {
+      MarkDirtyBitsOnDescendants(kid);
+      kid->AddStateBits(NS_FRAME_IS_DIRTY);
+    }
+    kid = kid->GetNextSibling();
+  }
+}
+
 void
 nsSVGTextFrame::NotifyGlyphMetricsChange()
 {
+  // NotifySVGChanged isn't appropriate here, so we just mark our descendants
+  // as fully dirty to get UpdateBounds() called on them:
+  MarkDirtyBitsOnDescendants(this);
+
   nsSVGUtils::InvalidateAndScheduleBoundsUpdate(this);
 
   mPositioningDirty = true;

layout/svg/base/src/nsSVGTextPathFrame.cpp

@@ -158,9 +158,11 @@ nsSVGTextPathFrame::AttributeChanged(PRInt32         aNameSpaceID,
 {
   if (aNameSpaceID == kNameSpaceID_None &&
       aAttribute == nsGkAtoms::startOffset) {
+    nsSVGUtils::InvalidateAndScheduleBoundsUpdate(this);
     NotifyGlyphMetricsChange();
   } else if (aNameSpaceID == kNameSpaceID_XLink &&
              aAttribute == nsGkAtoms::href) {
+    nsSVGUtils::InvalidateAndScheduleBoundsUpdate(this);
     // Blow away our reference, if any
     Properties().Delete(nsSVGEffects::HrefProperty());
     NotifyGlyphMetricsChange();

layout/svg/base/src/nsSVGUseFrame.cpp

@@ -116,31 +116,37 @@ nsSVGUseFrame::AttributeChanged(PRInt32         aNameSpaceID,
                                 nsIAtom*        aAttribute,
                                 PRInt32         aModType)
 {
+  nsSVGUseElement *useElement = static_cast<nsSVGUseElement*>(mContent);
+
   if (aNameSpaceID == kNameSpaceID_None) {
     if (aAttribute == nsGkAtoms::x ||
         aAttribute == nsGkAtoms::y) {
       // make sure our cached transform matrix gets (lazily) updated
       mCanvasTM = nsnull;
-    
+      nsSVGUtils::InvalidateAndScheduleBoundsUpdate(this);
       nsSVGUtils::NotifyChildrenOfSVGChange(this, TRANSFORM_CHANGED);
     } else if (aAttribute == nsGkAtoms::width ||
                aAttribute == nsGkAtoms::height) {
-      static_cast<nsSVGUseElement*>(mContent)->SyncWidthOrHeight(aAttribute);
-
-      if (mHasValidDimensions != 
-          static_cast<nsSVGUseElement*>(mContent)->HasValidDimensions()) {
-
+      bool invalidate = false;
+      if (mHasValidDimensions != useElement->HasValidDimensions()) {
         mHasValidDimensions = !mHasValidDimensions;
+        invalidate = true;
+      }
+      if (useElement->OurWidthAndHeightAreUsed()) {
+        invalidate = true;
+        useElement->SyncWidthOrHeight(aAttribute);
+      }
+      if (invalidate) {
         nsSVGUtils::InvalidateAndScheduleBoundsUpdate(this);
       }
     }
   } else if (aNameSpaceID == kNameSpaceID_XLink &&
              aAttribute == nsGkAtoms::href) {
     // we're changing our nature, clear out the clone information
-    nsSVGUseElement *use = static_cast<nsSVGUseElement*>(mContent);
-    use->mOriginal = nsnull;
-    use->UnlinkSource();
-    use->TriggerReclone();
+    nsSVGUtils::InvalidateAndScheduleBoundsUpdate(this);
+    useElement->mOriginal = nsnull;
+    useElement->UnlinkSource();
+    useElement->TriggerReclone();
   }
 
   return nsSVGUseFrameBase::AttributeChanged(aNameSpaceID,
@@ -191,6 +197,13 @@ nsSVGUseFrame::NotifySVGChanged(PRUint32 aFlags)
     if (use->mLengthAttributes[nsSVGUseElement::X].IsPercentage() ||
         use->mLengthAttributes[nsSVGUseElement::Y].IsPercentage()) {
       aFlags |= TRANSFORM_CHANGED;
+      // Ancestor changes can't affect how we render from the perspective of
+      // any rendering observers that we may have, so we don't need to
+      // invalidate them. We also don't need to invalidate ourself, since our
+      // changed ancestor will have invalidated its entire area, which includes
+      // our area.
+      // For perf reasons we call this before calling NotifySVGChanged() below.
+      nsSVGUtils::ScheduleBoundsUpdate(this);
     }
   }
 

layout/svg/base/src/nsSVGUtils.cpp

@@ -726,25 +726,6 @@ nsSVGUtils::InvalidateBounds(nsIFrame *aFrame, bool aDuringUpdate)
   }
 }
 
-static void
-MarkDirtyBitsOnDescendants(nsIFrame *aFrame)
-{
-  NS_ABORT_IF_FALSE(aFrame->IsFrameOfType(nsIFrame::eSVG),
-                    "Passed bad frame!");
-
-  nsIFrame* kid = aFrame->GetFirstPrincipalChild();
-  while (kid) {
-    nsISVGChildFrame* svgkid = do_QueryFrame(kid);
-    if (svgkid &&
-        !(kid->GetStateBits() &
-          (NS_STATE_SVG_NONDISPLAY_CHILD | NS_FRAME_IS_DIRTY))) {
-      MarkDirtyBitsOnDescendants(kid);
-      kid->AddStateBits(NS_FRAME_IS_DIRTY);
-    }
-    kid = kid->GetNextSibling();
-  }
-}
-
 void
 nsSVGUtils::ScheduleBoundsUpdate(nsIFrame *aFrame)
 {
@@ -773,10 +754,6 @@ nsSVGUtils::ScheduleBoundsUpdate(nsIFrame *aFrame)
     return;
   }
 
-  // XXXSDL once we store bounds on containers, we will not need to
-  // mark our descendants dirty.
-  MarkDirtyBitsOnDescendants(aFrame);
-
   nsSVGOuterSVGFrame *outerSVGFrame = nsnull;
 
   // We must not add dirty bits to the nsSVGOuterSVGFrame or else

layout/svg/base/src/nsSVGUtils.h

@@ -477,8 +477,15 @@ public:
    */
   static gfxMatrix GetCanvasTM(nsIFrame* aFrame);
 
-  /*
-   * Tells child frames that something that might affect them has changed
+  /**
+   * Notify the descendants of aFrame of a change to one of their ancestors
+   * that might affect them.
+   *
+   * If the changed ancestor renders and needs to be invalidated, it should
+   * call nsSVGUtils::InvalidateAndScheduleBoundsUpdate or
+   * nsSVGUtils::InvalidateBounds _before_ calling this method. That makes it
+   * cheaper when descendants schedule their own bounds update because the code
+   * that walks up the parent chain marking dirty bits can stop earlier.
    */
   static void
   NotifyChildrenOfSVGChange(nsIFrame *aFrame, PRUint32 aFlags);