Bug 421584. SVG that is filtered does not repaint correctly when filtered elements change/move. Patch by Robert Longson longsonr@gmail.com, r=jwatt@jwatt.org, sr=roc@ocallahan.org, a=blocking1.9

layout/svg/base/src/nsSVGContainerFrame.cpp

@@ -149,21 +149,18 @@ NS_IMETHODIMP
 nsSVGDisplayContainerFrame::RemoveFrame(nsIAtom* aListName,
                                         nsIFrame* aOldFrame)
 {
-  nsRect dirtyRect;
-  
-  nsISVGChildFrame* SVGFrame = nsnull;
-  CallQueryInterface(aOldFrame, &SVGFrame);
-
-  if (SVGFrame && !(GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD)) {
-    dirtyRect = nsSVGUtils::FindFilterInvalidation(aOldFrame);
+  if (!(GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD)) {
+    nsSVGOuterSVGFrame* outerSVGFrame = nsSVGUtils::GetOuterSVGFrame(this);
+    NS_ASSERTION(outerSVGFrame, "no outer svg frame");
+    if (outerSVGFrame)
+      outerSVGFrame->InvalidateCoveredRegion(aOldFrame);
   }
 
   nsresult rv = nsSVGContainerFrame::RemoveFrame(aListName, aOldFrame);
 
-  nsSVGOuterSVGFrame* outerSVGFrame = nsSVGUtils::GetOuterSVGFrame(this);
-  NS_ASSERTION(outerSVGFrame, "no outer svg frame");
-  if (outerSVGFrame)
-    outerSVGFrame->InvalidateRect(dirtyRect);
+  if (!GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD) {
+    nsSVGUtils::NotifyAncestorsOfFilterRegionChange(this);
+  }
 
   return rv;
 }

layout/svg/base/src/nsSVGForeignObjectFrame.cpp

@@ -386,12 +386,13 @@ nsSVGForeignObjectFrame::NotifySVGChanged(PRUint32 aFlags)
   PRBool reflow = PR_FALSE;
 
   if (aFlags & TRANSFORM_CHANGED) {
-    // Perhaps unexpectedly, we reflow if our CTM changes. This is because
+    // 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
     // and, as a result, CTM changes may require text to break at different
-    // points.
-    // XXX roc says we shouldn't do this. See bug 381285 comment 20.
-    reflow = PR_TRUE;
+    // points. The problem would be how to keep performance acceptable when
+    // e.g. the transform of an ancestor is animated.
+    // We also seem to get some sort of infinite loop post bug 421584 if we
+    // reflow.
     mCanvasTM = nsnull;
 
   } else if (aFlags & COORD_CONTEXT_CHANGED) {
@@ -572,19 +573,15 @@ void nsSVGForeignObjectFrame::UpdateGraphic()
     // Invalidate the area we used to cover
     // XXXjwatt: if we fix the following XXX, try to subtract the new region from the old here.
     // hmm, if x then y is changed, our second call could invalidate an "old" area we never actually painted to.
-    outerSVGFrame->InvalidateRect(mRect);
+    outerSVGFrame->InvalidateCoveredRegion(this);
 
     UpdateCoveredRegion();
 
     // Invalidate the area we now cover
     // XXXjwatt: when we're called due to an event that also requires reflow,
     // we want to let reflow trigger this rasterization so it doesn't happen twice.
-    nsRect filterRect = nsSVGUtils::FindFilterInvalidation(this);
-    if (!filterRect.IsEmpty()) {
-      outerSVGFrame->InvalidateRect(filterRect);
-    } else {
-      outerSVGFrame->InvalidateRect(mRect);
-    }
+    outerSVGFrame->InvalidateCoveredRegion(this);
+    nsSVGUtils::NotifyAncestorsOfFilterRegionChange(this);
   }
 
   // Clear any layout dirty region since we invalidated our whole area.
@@ -708,6 +705,11 @@ nsSVGForeignObjectFrame::FlushDirtyRegion()
   r.ScaleRoundOut(1.0f / PresContext()->AppUnitsPerDevPixel());
   float x = r.x, y = r.y, w = r.width, h = r.height;
   r = GetTransformedRegion(x, y, w, h, tm);
+
+  // XXX invalidate the entire covered region
+  // See bug 418063
+  r.UnionRect(r, mRect);
+
   outerSVGFrame->InvalidateRect(r);
 
   mDirtyRegion.SetEmpty();

layout/svg/base/src/nsSVGOuterSVGFrame.cpp

@@ -634,13 +634,29 @@ nsSVGOuterSVGFrame::GetType() const
 //----------------------------------------------------------------------
 // nsSVGOuterSVGFrame methods:
 
-nsresult
+void
+nsSVGOuterSVGFrame::InvalidateCoveredRegion(nsIFrame *aFrame)
+{
+  nsISVGChildFrame *svgFrame = nsnull;
+  CallQueryInterface(aFrame, &svgFrame);
+  if (!svgFrame)
+    return;
+
+  nsRect rect = nsSVGUtils::FindFilterInvalidation(aFrame);
+  if (rect.IsEmpty()) {
+    rect = svgFrame->GetCoveredRegion();
+  }
+
+  InvalidateRect(rect);
+}
+
+void
 nsSVGOuterSVGFrame::InvalidateRect(nsRect aRect)
 {
-  aRect.ScaleRoundOut(PresContext()->AppUnitsPerDevPixel());
-  Invalidate(aRect);
-
-  return NS_OK;
+  if (!aRect.IsEmpty()) {
+    aRect.ScaleRoundOut(PresContext()->AppUnitsPerDevPixel());
+    Invalidate(aRect);
+  }
 }
 
 PRBool

layout/svg/base/src/nsSVGOuterSVGFrame.h

@@ -130,8 +130,9 @@ public:
 
   // nsSVGOuterSVGFrame methods:
 
+  void InvalidateCoveredRegion(nsIFrame *aFrame);
   /* Invalidate takes a nsRect in screen pixel coordinates */
-  nsresult InvalidateRect(nsRect aRect);
+  void InvalidateRect(nsRect aRect);
   PRBool IsRedrawSuspended();
 
   // nsISVGSVGFrame interface:

layout/svg/base/src/nsSVGPathGeometryFrame.cpp

@@ -261,7 +261,7 @@ nsSVGPathGeometryFrame::DidSetStyleContext()
   nsSVGOuterSVGFrame *outerSVGFrame = nsSVGUtils::GetOuterSVGFrame(this);
   if (outerSVGFrame) {
     // invalidate here while we still have the filter information
-    outerSVGFrame->InvalidateRect(nsSVGUtils::FindFilterInvalidation(this));
+    outerSVGFrame->InvalidateCoveredRegion(this);
   }
 
   RemovePathProperties();
@@ -757,13 +757,14 @@ nsSVGPathGeometryFrame::UpdateGraphic(PRBool suppressInvalidation)
     if (suppressInvalidation)
       return NS_OK;
 
-    outerSVGFrame->InvalidateRect(nsSVGUtils::FindFilterInvalidation(this));
+    outerSVGFrame->InvalidateCoveredRegion(this);
 
     UpdateMarkerProperty();
     UpdateCoveredRegion();
     nsSVGUtils::UpdateFilterRegion(this);
 
-    outerSVGFrame->InvalidateRect(nsSVGUtils::FindFilterInvalidation(this));
+    outerSVGFrame->InvalidateCoveredRegion(this);
+    nsSVGUtils::NotifyAncestorsOfFilterRegionChange(this);
   }
 
   return NS_OK;

layout/svg/base/src/nsSVGUtils.cpp

@@ -239,9 +239,9 @@ nsSVGFilterProperty::DoUpdate()
 {
   nsSVGOuterSVGFrame *outerSVGFrame = nsSVGUtils::GetOuterSVGFrame(mFrame);
   if (outerSVGFrame) {
-    outerSVGFrame->InvalidateRect(mFilterRect);
+    outerSVGFrame->InvalidateCoveredRegion(mFrame);
     UpdateRect();
-    outerSVGFrame->InvalidateRect(mFilterRect);
+    outerSVGFrame->InvalidateCoveredRegion(mFrame);
   }
 }
 
@@ -253,7 +253,7 @@ nsSVGFilterProperty::ParentChainChanged(nsIContent *aContent)
 
   nsSVGOuterSVGFrame *outerSVGFrame = nsSVGUtils::GetOuterSVGFrame(mFrame);
   if (outerSVGFrame)
-    outerSVGFrame->InvalidateRect(mFilterRect);
+    outerSVGFrame->InvalidateCoveredRegion(mFrame);
 
   mFrame->DeleteProperty(nsGkAtoms::filter);
 }
@@ -302,7 +302,7 @@ nsSVGClipPathProperty::DoUpdate()
 
   nsSVGOuterSVGFrame *outerSVGFrame = nsSVGUtils::GetOuterSVGFrame(mFrame);
   if (outerSVGFrame)
-    outerSVGFrame->InvalidateRect(svgChildFrame->GetCoveredRegion());
+    outerSVGFrame->InvalidateCoveredRegion(mFrame);
 }
 
 void
@@ -359,7 +359,7 @@ nsSVGMaskProperty::DoUpdate()
 
   nsSVGOuterSVGFrame *outerSVGFrame = nsSVGUtils::GetOuterSVGFrame(mFrame);
   if (outerSVGFrame)
-    outerSVGFrame->InvalidateRect(svgChildFrame->GetCoveredRegion());
+    outerSVGFrame->InvalidateCoveredRegion(mFrame);
 }
 
 void
@@ -857,12 +857,7 @@ nsSVGUtils::GetBBox(nsFrameList *aFrames, nsIDOMSVGRect **_retval)
 nsRect
 nsSVGUtils::FindFilterInvalidation(nsIFrame *aFrame)
 {
-  nsISVGChildFrame *svgFrame = nsnull;
-  CallQueryInterface(aFrame, &svgFrame);
-  if (!svgFrame)
-    return nsRect();
-
-  nsRect rect = svgFrame->GetCoveredRegion();
+  nsRect rect;
 
   while (aFrame) {
     if (aFrame->GetStateBits() & NS_STATE_IS_OUTER_SVG)
@@ -893,6 +888,25 @@ nsSVGUtils::UpdateFilterRegion(nsIFrame *aFrame)
   }
 }
 
+void 
+nsSVGUtils::NotifyAncestorsOfFilterRegionChange(nsIFrame *aFrame)
+{
+  aFrame = aFrame->GetParent();
+
+  while (aFrame) {
+    if (aFrame->GetStateBits() & NS_STATE_IS_OUTER_SVG)
+      return;
+
+    if (aFrame->GetStateBits() & NS_STATE_SVG_FILTERED) {
+      nsSVGFilterProperty *property;
+      property = static_cast<nsSVGFilterProperty *>
+                            (aFrame->GetProperty(nsGkAtoms::filter));
+      property->Invalidate();
+    }
+    aFrame = aFrame->GetParent();
+  }
+}
+
 float
 nsSVGUtils::ObjectSpace(nsIDOMSVGRect *aRect, nsSVGLength2 *aLength)
 {

layout/svg/base/src/nsSVGUtils.h

@@ -270,6 +270,11 @@ public:
    */
   static void UpdateFilterRegion(nsIFrame *aFrame);
 
+  /*
+   * Update the filter invalidation region for ancestor frames, if relevant.
+   */
+  static void NotifyAncestorsOfFilterRegionChange(nsIFrame *aFrame);
+
   /* enum for specifying coordinate direction for ObjectSpace/UserSpace */
   enum ctxDirection { X, Y, XY };