Bug 614732 - For SVG leaf frames, use mRect to store the frame's user space bounds, and stop using it to store its covered region (compute that on demand). r=roc.

layout/reftests/backgrounds/vector/empty/reftest.list

@@ -3,15 +3,16 @@
 == wide--contain--height.html ref-wide-empty.html
 == wide--contain--width.html ref-wide-empty.html
 
-# Either OS X 32-bit or 10.5, judging from imprecise Tinderbox results, renders
-# these tests as empty boxes, not filled boxes.  We don't really care about this
+# We don't really care about the failures for this
 # extreme edge case (the test exists more to test for safety against division by
 # zero), so there is no bug has been filed to fix it, although a patch would
 # probably be accepted.
-random-if(cocoaWidget) == tall--cover--height.html ref-tall-lime.html
-random-if(cocoaWidget) == tall--cover--width.html ref-tall-lime.html
-random-if(cocoaWidget) == wide--cover--height.html ref-wide-lime.html
-random-if(cocoaWidget) == wide--cover--width.html ref-wide-lime.html
+# They're still marked as failing though, rather than 'load', since
+# we want to know if they start working when we upgrade to Azure.
+fails == tall--cover--height.html ref-tall-lime.html
+fails == tall--cover--width.html ref-tall-lime.html
+fails == wide--cover--height.html ref-wide-lime.html
+fails == wide--cover--width.html ref-wide-lime.html
 
 == zero-height-ratio-contain.html ref-tall-empty.html
 == zero-height-ratio-cover.html ref-tall-empty.html

layout/svg/base/src/nsSVGForeignObjectFrame.cpp

@@ -229,10 +229,29 @@ nsSVGForeignObjectFrame::PaintSVG(nsSVGRenderState *aContext,
     return NS_ERROR_FAILURE;
   }
 
+  nsRect kidDirtyRect = kid->GetVisualOverflowRect();
+
   /* Check if we need to draw anything. */
   if (aDirtyRect) {
-    PRInt32 appUnitsPerDevPx = PresContext()->AppUnitsPerDevPixel();
-    if (!mRect.ToOutsidePixels(appUnitsPerDevPx).Intersects(*aDirtyRect))
+    // Transform the dirty rect into app units in our userspace.
+    gfxMatrix invmatrix = matrix;
+    invmatrix.Invert();
+    NS_ASSERTION(!invmatrix.IsSingular(),
+                 "inverse of non-singular matrix should be non-singular");
+
+    gfxRect transDirtyRect = gfxRect(aDirtyRect->x, aDirtyRect->y,
+                                     aDirtyRect->width, aDirtyRect->height);
+    transDirtyRect = invmatrix.TransformBounds(transDirtyRect);
+
+    kidDirtyRect.IntersectRect(kidDirtyRect,
+      nsLayoutUtils::RoundGfxRectToAppRect(transDirtyRect,
+                       PresContext()->AppUnitsPerCSSPixel()));
+
+    // XXX after bug 614732 is fixed, we will compare mRect with aDirtyRect,
+    // not with kidDirtyRect. I.e.
+    // PRInt32 appUnitsPerDevPx = PresContext()->AppUnitsPerDevPixel();
+    // mRect.ToOutsidePixels(appUnitsPerDevPx).Intersects(*aDirtyRect)
+    if (kidDirtyRect.IsEmpty())
       return NS_OK;
   }
 
@@ -258,17 +277,6 @@ nsSVGForeignObjectFrame::PaintSVG(nsSVGRenderState *aContext,
   NS_ASSERTION(!invmatrix.IsSingular(),
                "inverse of non-singular matrix should be non-singular");
 
-  nsRect kidDirtyRect = kid->GetVisualOverflowRect();
-  if (aDirtyRect) {
-    gfxRect transDirtyRect = gfxRect(aDirtyRect->x, aDirtyRect->y,
-                                     aDirtyRect->width, aDirtyRect->height);
-    transDirtyRect = invmatrix.TransformBounds(transDirtyRect);
-
-    kidDirtyRect.IntersectRect(kidDirtyRect,
-      nsLayoutUtils::RoundGfxRectToAppRect(transDirtyRect,
-                       PresContext()->AppUnitsPerCSSPixel()));
-  }
-
   PRUint32 flags = nsLayoutUtils::PAINT_IN_TRANSFORM;
   if (aContext->IsPaintingToWindow()) {
     flags |= nsLayoutUtils::PAINT_TO_WINDOW;
@@ -340,7 +348,7 @@ nsSVGForeignObjectFrame::GetFrameForPoint(const nsPoint &aPoint)
 NS_IMETHODIMP_(nsRect)
 nsSVGForeignObjectFrame::GetCoveredRegion()
 {
-  return mRect;
+  return nsSVGUtils::TransformFrameRectToOuterSVG(mRect, GetCanvasTM(), PresContext());
 }
 
 NS_IMETHODIMP
@@ -358,8 +366,10 @@ nsSVGForeignObjectFrame::UpdateCoveredRegion()
   if (h < 0.0f) h = 0.0f;
 
   // GetCanvasTM includes the x,y translation
-  mRect = ToCanvasBounds(gfxRect(0.0, 0.0, w, h), GetCanvasTM(), PresContext());
-  
+  mRect = nsLayoutUtils::RoundGfxRectToAppRect(
+                           gfxRect(0.0, 0.0, w, h),
+                           PresContext()->AppUnitsPerDevPixel());
+
   return NS_OK;
 }
 
@@ -641,19 +651,18 @@ nsSVGForeignObjectFrame::InvalidateDirtyRect(nsSVGOuterSVGFrame* aOuter,
   if (aRect.IsEmpty())
     return;
 
-  // The areas dirtied by children are in app units, relative to this frame.
-  // We need to convert the rect to userspace to use IntersectRect.
-
-  gfxRect r(aRect.x, aRect.y, aRect.width, aRect.height);
-  r.Scale(1.0 / nsPresContext::AppUnitsPerCSSPixel());
-
-  nsRect rect = ToCanvasBounds(r, GetCanvasTM(), PresContext());
-
   // Don't invalidate areas outside our bounds:
-  rect.IntersectRect(rect, mRect);
+  nsRect rect = aRect.Intersect(mRect);
   if (rect.IsEmpty())
     return;
 
+  // The areas dirtied by children are in app units, relative to this frame.
+  // We need to convert the rect from app units in our userspace to app units
+  // relative to our nsSVGOuterSVGFrame's content rect.
+
+  gfxRect r(aRect.x, aRect.y, aRect.width, aRect.height);
+  r.Scale(1.0 / nsPresContext::AppUnitsPerCSSPixel());
+  rect = ToCanvasBounds(r, GetCanvasTM(), PresContext());
   rect = nsSVGUtils::FindFilterInvalidation(this, rect);
   aOuter->InvalidateWithFlags(rect, aFlags);
 }

layout/svg/base/src/nsSVGGlyphFrame.cpp

@@ -440,7 +440,7 @@ nsSVGGlyphFrame::GetFrameForPoint(const nsPoint &aPoint)
 NS_IMETHODIMP_(nsRect)
 nsSVGGlyphFrame::GetCoveredRegion()
 {
-  return mRect;
+  return nsSVGUtils::TransformFrameRectToOuterSVG(mRect, GetCanvasTM(), PresContext());
 }
 
 NS_IMETHODIMP
@@ -448,13 +448,9 @@ nsSVGGlyphFrame::UpdateCoveredRegion()
 {
   mRect.SetEmpty();
 
-  gfxMatrix matrix = GetCanvasTM();
-  if (matrix.IsSingular()) {
-    return NS_ERROR_FAILURE;
-  }
-
+  // XXX here we have tmpCtx use its default identity matrix, but does this
+  // function call anything that will call GetCanvasTM and break things?
   nsRefPtr<gfxContext> tmpCtx = MakeTmpCtx();
-  tmpCtx->Multiply(matrix);
 
   bool hasStroke = HasStroke();
   if (hasStroke) {
@@ -487,7 +483,8 @@ nsSVGGlyphFrame::UpdateCoveredRegion()
   // calls to record and then reset the stroke width.
   gfxRect extent = tmpCtx->GetUserPathExtent();
   if (hasStroke) {
-    extent = nsSVGUtils::PathExtentsToMaxStrokeExtents(extent, this, matrix);
+    extent =
+      nsSVGUtils::PathExtentsToMaxStrokeExtents(extent, this, gfxMatrix());
   }
 
   if (!extent.IsEmpty()) {

layout/svg/base/src/nsSVGImageFrame.cpp

@@ -365,7 +365,9 @@ nsSVGImageFrame::PaintSVG(nsSVGRenderState *aContext,
     if (aDirtyRect) {
       dirtyRect = aDirtyRect->ToAppUnits(appUnitsPerDevPx);
       // Adjust dirtyRect to match our local coordinate system.
-      dirtyRect.MoveBy(-mRect.TopLeft());
+      nsRect rootRect =
+        nsSVGUtils::TransformFrameRectToOuterSVG(mRect, GetCanvasTM(), PresContext());
+      dirtyRect.MoveBy(-rootRect.TopLeft());
     }
 
     // XXXbholley - I don't think huge images in SVGs are common enough to
@@ -483,8 +485,8 @@ nsSVGImageFrame::UpdateCoveredRegion()
 
   gfxContext context(gfxPlatform::GetPlatform()->ScreenReferenceSurface());
 
-  GeneratePath(&context);
-  context.IdentityMatrix();
+  gfxMatrix identity;
+  GeneratePath(&context, &identity);
 
   gfxRect extent = context.GetUserPathExtent();
 

layout/svg/base/src/nsSVGPathGeometryFrame.cpp

@@ -159,8 +159,16 @@ nsSVGPathGeometryFrame::GetFrameForPoint(const nsPoint &aPoint)
     fillRule = GetClipRule();
   } else {
     hitTestFlags = GetHitTestFlags();
+    // XXX once bug 614732 is fixed, aPoint won't need any conversion in order
+    // to compare it with mRect.
+    gfxMatrix canvasTM = GetCanvasTM();
+    if (canvasTM.IsSingular()) {
+      return nsnull;
+    }
+    nsPoint point =
+      nsSVGUtils::TransformOuterSVGPointToChildFrame(aPoint, canvasTM, PresContext());
     if (!hitTestFlags || ((hitTestFlags & SVG_HIT_TEST_CHECK_MRECT) &&
-                          !mRect.Contains(aPoint)))
+                          !mRect.Contains(point)))
       return nsnull;
     fillRule = GetStyleSVG()->mFillRule;
   }
@@ -196,13 +204,13 @@ nsSVGPathGeometryFrame::GetFrameForPoint(const nsPoint &aPoint)
 NS_IMETHODIMP_(nsRect)
 nsSVGPathGeometryFrame::GetCoveredRegion()
 {
-  return mRect;
+  return nsSVGUtils::TransformFrameRectToOuterSVG(mRect, GetCanvasTM(), PresContext());
 }
 
 NS_IMETHODIMP
 nsSVGPathGeometryFrame::UpdateCoveredRegion()
 {
-  gfxRect extent = GetBBoxContribution(GetCanvasTM(),
+  gfxRect extent = GetBBoxContribution(gfxMatrix(),
     nsSVGUtils::eBBoxIncludeFill | nsSVGUtils::eBBoxIgnoreFillIfNone |
     nsSVGUtils::eBBoxIncludeStroke | nsSVGUtils::eBBoxIgnoreStrokeIfNone |
     nsSVGUtils::eBBoxIncludeMarkers);

layout/svg/base/src/nsSVGUtils.cpp

@@ -1010,8 +1010,10 @@ nsSVGUtils::PaintFrameWithEffects(nsSVGRenderState *aContext,
       if (!aDirtyRect->Intersects(filterFrame->GetFilterBBox(aFrame, nsnull)))
         return;
     } else {
+      nsRect leafBounds = nsSVGUtils::TransformFrameRectToOuterSVG(
+        aFrame->GetRect(), GetCanvasTM(aFrame), aFrame->PresContext());
       nsRect rect = aDirtyRect->ToAppUnits(aFrame->PresContext()->AppUnitsPerDevPixel());
-      if (!rect.Intersects(aFrame->GetRect()))
+      if (!rect.Intersects(leafBounds))
         return;
     }
   }
@@ -1178,6 +1180,35 @@ nsSVGUtils::GetCoveredRegion(const nsFrameList &aFrames)
   return rect;
 }
 
+nsPoint
+nsSVGUtils::TransformOuterSVGPointToChildFrame(nsPoint aPoint,
+                                               const gfxMatrix& aFrameToCanvasTM,
+                                               nsPresContext* aPresContext)
+{
+  gfxMatrix devToUser = aFrameToCanvasTM;
+  devToUser.Invert();
+  NS_ABORT_IF_FALSE(!devToUser.IsSingular(), "should not get here");
+  gfxPoint devPt = gfxPoint(aPoint.x, aPoint.y) /
+    aPresContext->AppUnitsPerDevPixel();
+  gfxPoint userPt = devToUser.Transform(devPt).Round();
+  gfxPoint appPt = userPt * aPresContext->AppUnitsPerCSSPixel();
+  userPt.x = clamped(appPt.x, gfxFloat(nscoord_MIN), gfxFloat(nscoord_MAX));
+  userPt.y = clamped(appPt.y, gfxFloat(nscoord_MIN), gfxFloat(nscoord_MAX));
+  // now guaranteed to be safe:
+  return nsPoint(nscoord(userPt.x), nscoord(userPt.y));
+}
+
+nsRect
+nsSVGUtils::TransformFrameRectToOuterSVG(const nsRect& aRect,
+                                         const gfxMatrix& aMatrix,
+                                         nsPresContext* aPresContext)
+{
+  gfxRect r(aRect.x, aRect.y, aRect.width, aRect.height);
+  r.Scale(1.0 / nsPresContext::AppUnitsPerCSSPixel());
+  return nsLayoutUtils::RoundGfxRectToAppRect(
+    aMatrix.TransformBounds(r), aPresContext->AppUnitsPerDevPixel());
+}
+
 gfxIntSize
 nsSVGUtils::ConvertToSurfaceSize(const gfxSize& aSize,
                                  bool *aResultOverflows)

layout/svg/base/src/nsSVGUtils.h

@@ -447,6 +447,20 @@ public:
   static nsRect
   GetCoveredRegion(const nsFrameList &aFrames);
 
+  // Converts aPoint from an app unit point in outer-<svg> content rect space
+  // to an app unit point in a frame's SVG userspace. 
+  // This is a temporary helper we should no longer need after bug 614732 is
+  // fixed.
+  static nsPoint
+  TransformOuterSVGPointToChildFrame(nsPoint aPoint,
+                                     const gfxMatrix& aFrameToCanvasTM,
+                                     nsPresContext* aPresContext);
+
+  static nsRect
+  TransformFrameRectToOuterSVG(const nsRect& aRect,
+                               const gfxMatrix& aMatrix,
+                               nsPresContext* aPresContext);
+
   /*
    * Convert a surface size to an integer for use by thebes
    * possibly making it smaller in the process so the surface does not