Bug 736031 - getBBox returns incorrect results with empty containers. r=jwatt
authorRobert Longson <longsonr@gmail.com>
Mon, 16 Apr 2012 09:23:48 +0100
changeset 91747 01fd11649cc8f93cc9d4df770884019938794361
parent 91746 2f8fbd24bde1a0ce3110573e35438083110f9f4d
child 91748 5972e57175a560315683a098c0e57563027db456
push id22472
push usereakhgari@mozilla.com
push dateMon, 16 Apr 2012 15:03:21 +0000
treeherdermozilla-central@0066df252596 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt
bugs736031
milestone14.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 736031 - getBBox returns incorrect results with empty containers. r=jwatt
content/svg/content/test/bbox-helper.svg
layout/svg/base/src/nsISVGChildFrame.h
layout/svg/base/src/nsSVGContainerFrame.cpp
layout/svg/base/src/nsSVGContainerFrame.h
layout/svg/base/src/nsSVGForeignObjectFrame.cpp
layout/svg/base/src/nsSVGForeignObjectFrame.h
layout/svg/base/src/nsSVGGlyphFrame.cpp
layout/svg/base/src/nsSVGGlyphFrame.h
layout/svg/base/src/nsSVGMarkerFrame.cpp
layout/svg/base/src/nsSVGMarkerFrame.h
layout/svg/base/src/nsSVGPathGeometryFrame.cpp
layout/svg/base/src/nsSVGPathGeometryFrame.h
layout/svg/base/src/nsSVGSwitchFrame.cpp
layout/svg/base/src/nsSVGTextFrame.cpp
layout/svg/base/src/nsSVGTextFrame.h
layout/svg/base/src/nsSVGUtils.cpp
layout/svg/base/src/nsSVGUtils.h
--- a/content/svg/content/test/bbox-helper.svg
+++ b/content/svg/content/test/bbox-helper.svg
@@ -13,10 +13,13 @@
   <g id="h">
     <circle cx="200" cy="50" r="5"/>
     <path d="M 200,100 L 300,100"/>
   </g>
   <g id="e">
     <!-- empty container should not affect parent's bbox -->
     <g/>
     <circle cx="100" cy="100" r="5"/>
+    <g/>
+    <circle cx="100" cy="100" r="5"/>
+    <g/>
   </g>
 </svg>
--- a/layout/svg/base/src/nsISVGChildFrame.h
+++ b/layout/svg/base/src/nsISVGChildFrame.h
@@ -43,16 +43,17 @@
 #include "nsQueryFrame.h"
 #include "nsRect.h"
 
 class nsIFrame;
 class nsRenderingContext;
 
 struct gfxMatrix;
 struct nsPoint;
+class SVGBBox;
 
 namespace mozilla {
 class SVGAnimatedLengthList;
 class SVGAnimatedNumberList;
 class SVGLengthList;
 class SVGNumberList;
 class SVGUserUnitList;
 }
@@ -129,17 +130,17 @@ public:
    * @param aToBBoxUserspace The transform from the userspace established by
    *   this element to the userspace established by the ancestor on which
    *   getBBox was called. This will be the identity matrix if we are the
    *   element on which getBBox was called.
    *
    * @param aFlags Flags indicating whether, stroke, for example, should be
    *   included in the bbox calculation.
    */
-  virtual gfxRect GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
+  virtual SVGBBox GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                       PRUint32 aFlags) = 0;
 
   // Are we a container frame?
   NS_IMETHOD_(bool) IsDisplayContainer()=0;
 
   // Does this frame have an current covered region in mRect (aka GetRect())?
   NS_IMETHOD_(bool) HasValidCoveredRect()=0;
 };
--- a/layout/svg/base/src/nsSVGContainerFrame.cpp
+++ b/layout/svg/base/src/nsSVGContainerFrame.cpp
@@ -246,42 +246,34 @@ nsSVGDisplayContainerFrame::NotifySVGCha
                     "Must be NS_STATE_SVG_NONDISPLAY_CHILD!");
 
   NS_ABORT_IF_FALSE(aFlags & (TRANSFORM_CHANGED | COORD_CONTEXT_CHANGED),
                     "Invalidation logic may need adjusting");
 
   nsSVGUtils::NotifyChildrenOfSVGChange(this, aFlags);
 }
 
-gfxRect
+SVGBBox
 nsSVGDisplayContainerFrame::GetBBoxContribution(
   const gfxMatrix &aToBBoxUserspace,
   PRUint32 aFlags)
 {
-  gfxRect bboxUnion;
-  bool firstChild = true;
+  SVGBBox bboxUnion;
 
   nsIFrame* kid = mFrames.FirstChild();
   while (kid) {
     nsISVGChildFrame* svgKid = do_QueryFrame(kid);
     if (svgKid) {
       gfxMatrix transform = aToBBoxUserspace;
       nsIContent *content = kid->GetContent();
       if (content->IsSVG()) {
         transform = static_cast<nsSVGElement*>(content)->
                       PrependLocalTransformsTo(aToBBoxUserspace);
       }
       // We need to include zero width/height vertical/horizontal lines, so we have
-      // to use UnionEdges, but we must special case the first bbox so that we don't
-      // include the initial gfxRect(0,0,0,0).
-      gfxRect childBBox = svgKid->GetBBoxContribution(transform, aFlags);
-      if (firstChild && (childBBox.Width() > 0 || childBBox.Height() > 0)) {
-        bboxUnion = childBBox;
-        firstChild = false;
-        continue;
-      }
-      bboxUnion = bboxUnion.UnionEdges(childBBox);
+      // to use UnionEdges.
+      bboxUnion.UnionEdges(svgKid->GetBBoxContribution(transform, aFlags));
     }
     kid = kid->GetNextSibling();
   }
 
   return bboxUnion;
 }
--- a/layout/svg/base/src/nsSVGContainerFrame.h
+++ b/layout/svg/base/src/nsSVGContainerFrame.h
@@ -123,15 +123,15 @@ public:
 
   // nsISVGChildFrame interface:
   NS_IMETHOD PaintSVG(nsRenderingContext* aContext,
                       const nsIntRect *aDirtyRect);
   NS_IMETHOD_(nsIFrame*) GetFrameForPoint(const nsPoint &aPoint);
   NS_IMETHOD_(nsRect) GetCoveredRegion();
   virtual void UpdateBounds();
   virtual void NotifySVGChanged(PRUint32 aFlags);
-  virtual gfxRect GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
+  virtual SVGBBox GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                       PRUint32 aFlags);
   NS_IMETHOD_(bool) IsDisplayContainer() { return true; }
   NS_IMETHOD_(bool) HasValidCoveredRect() { return false; }
 };
 
 #endif
--- a/layout/svg/base/src/nsSVGForeignObjectFrame.cpp
+++ b/layout/svg/base/src/nsSVGForeignObjectFrame.cpp
@@ -485,32 +485,32 @@ nsSVGForeignObjectFrame::NotifySVGChange
 
   if (needNewCanvasTM) {
     // Do this after calling InvalidateAndScheduleBoundsUpdate in case we
     // change the code and it needs to use it.
     mCanvasTM = nsnull;
   }
 }
 
-gfxRect
+SVGBBox
 nsSVGForeignObjectFrame::GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                              PRUint32 aFlags)
 {
   nsSVGForeignObjectElement *content =
     static_cast<nsSVGForeignObjectElement*>(mContent);
 
   float x, y, w, h;
   content->GetAnimatedLengthValues(&x, &y, &w, &h, nsnull);
 
   if (w < 0.0f) w = 0.0f;
   if (h < 0.0f) h = 0.0f;
 
   if (aToBBoxUserspace.IsSingular()) {
     // XXX ReportToConsole
-    return gfxRect(0.0, 0.0, 0.0, 0.0);
+    return SVGBBox();
   }
   return aToBBoxUserspace.TransformBounds(gfxRect(0.0, 0.0, w, h));
 }
 
 //----------------------------------------------------------------------
 
 gfxMatrix
 nsSVGForeignObjectFrame::GetCanvasTM()
--- a/layout/svg/base/src/nsSVGForeignObjectFrame.h
+++ b/layout/svg/base/src/nsSVGForeignObjectFrame.h
@@ -121,17 +121,17 @@ public:
 
   // nsISVGChildFrame interface:
   NS_IMETHOD PaintSVG(nsRenderingContext *aContext,
                       const nsIntRect *aDirtyRect);
   NS_IMETHOD_(nsIFrame*) GetFrameForPoint(const nsPoint &aPoint);
   NS_IMETHOD_(nsRect) GetCoveredRegion();
   virtual void UpdateBounds();
   virtual void NotifySVGChanged(PRUint32 aFlags);
-  virtual gfxRect GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
+  virtual SVGBBox GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                       PRUint32 aFlags);
   NS_IMETHOD_(bool) IsDisplayContainer() { return true; }
   NS_IMETHOD_(bool) HasValidCoveredRect() {
     return !(GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD);
   }
 
   gfxMatrix GetCanvasTM();
 
--- a/layout/svg/base/src/nsSVGGlyphFrame.cpp
+++ b/layout/svg/base/src/nsSVGGlyphFrame.cpp
@@ -585,17 +585,17 @@ nsSVGGlyphFrame::DrawCharacters(Characte
   PRUint32 i;
   while ((i = aIter->NextCluster()) != aIter->InvalidCluster()) {
     aIter->SetupForDrawing(aContext);
     mTextRun->Draw(aContext, gfxPoint(0, 0), aDrawMode, i,
                    aIter->ClusterLength(), nsnull, nsnull, aStrokePattern);
   }
 }
 
-gfxRect
+SVGBBox
 nsSVGGlyphFrame::GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                      PRUint32 aFlags)
 {
   if (mOverrideCanvasTM) {
     *mOverrideCanvasTM = aToBBoxUserspace;
   } else {
     mOverrideCanvasTM = new gfxMatrix(aToBBoxUserspace);
   }
--- a/layout/svg/base/src/nsSVGGlyphFrame.h
+++ b/layout/svg/base/src/nsSVGGlyphFrame.h
@@ -173,17 +173,17 @@ public:
   }
 #endif
 
   // nsISVGChildFrame interface:
   // These four always use the global transform, even if NS_STATE_NONDISPLAY_CHILD
   NS_IMETHOD PaintSVG(nsRenderingContext *aContext,
                       const nsIntRect *aDirtyRect);
   NS_IMETHOD_(nsIFrame*) GetFrameForPoint(const nsPoint &aPoint);
-  virtual gfxRect GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
+  virtual SVGBBox GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                       PRUint32 aFlags);
 
   NS_IMETHOD_(nsRect) GetCoveredRegion();
   virtual void UpdateBounds();
   virtual void NotifySVGChanged(PRUint32 aFlags);
   NS_IMETHOD_(bool) IsDisplayContainer() { return false; }
   NS_IMETHOD_(bool) HasValidCoveredRect() {
     return !(GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD);
--- a/layout/svg/base/src/nsSVGMarkerFrame.cpp
+++ b/layout/svg/base/src/nsSVGMarkerFrame.cpp
@@ -172,72 +172,64 @@ nsSVGMarkerFrame::PaintMark(nsRenderingC
   }
 
   if (GetStyleDisplay()->IsScrollableOverflow())
     gfx->Restore();
 
   return NS_OK;
 }
 
-gfxRect
+SVGBBox
 nsSVGMarkerFrame::GetMarkBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                           PRUint32 aFlags,
                                           nsSVGPathGeometryFrame *aMarkedFrame,
                                           const nsSVGMark *aMark,
                                           float aStrokeWidth)
 {
+  SVGBBox bbox;
+
   // If the flag is set when we get here, it means this marker frame
   // has already been used in calculating the current mark bbox, and
   // the document has a marker reference loop.
   if (mInUse)
-    return gfxRect();
+    return bbox;
 
   AutoMarkerReferencer markerRef(this, aMarkedFrame);
 
   nsSVGMarkerElement *content = static_cast<nsSVGMarkerElement*>(mContent);
 
   const nsSVGViewBoxRect viewBox = content->GetViewBoxRect();
 
   if (viewBox.width <= 0.0f || viewBox.height <= 0.0f) {
-    return gfxRect();
+    return bbox;
   }
 
   mStrokeWidth = aStrokeWidth;
   mX = aMark->x;
   mY = aMark->y;
   mAutoAngle = aMark->angle;
 
   gfxMatrix markerTM =
     content->GetMarkerTransform(mStrokeWidth, mX, mY, mAutoAngle);
   gfxMatrix viewBoxTM = content->GetViewBoxTransform();
 
   gfxMatrix tm = viewBoxTM * markerTM * aToBBoxUserspace;
 
-  gfxRect bbox;
-  bool firstChild = true;
-
   for (nsIFrame* kid = mFrames.FirstChild();
        kid;
        kid = kid->GetNextSibling()) {
     nsISVGChildFrame* child = do_QueryFrame(kid);
     if (child) {
       // When we're being called to obtain the invalidation area, we need to
       // pass down all the flags so that stroke is included. However, once DOM
       // getBBox() accepts flags, maybe we should strip some of those here?
 
       // We need to include zero width/height vertical/horizontal lines, so we have
-      // to use UnionEdges, but we must special case the first bbox so that we don't
-      // include the initial gfxRect(0,0,0,0).
-      gfxRect childBBox = child->GetBBoxContribution(tm, aFlags);
-      if (firstChild && (childBBox.Width() > 0 || childBBox.Height() > 0)) {
-        bbox = childBBox;
-        firstChild = false;
-        continue;
-      }
-      bbox = bbox.UnionEdges(childBBox);
+      // to use UnionEdges.
+      bbox.UnionEdges(child->GetBBoxContribution(tm, aFlags));
     }
   }
 
   return bbox;
 }
 
 void
 nsSVGMarkerFrame::SetParentCoordCtxProvider(nsSVGSVGElement *aContext)
--- a/layout/svg/base/src/nsSVGMarkerFrame.h
+++ b/layout/svg/base/src/nsSVGMarkerFrame.h
@@ -100,17 +100,17 @@ public:
 #endif
 
   // nsSVGMarkerFrame methods:
   nsresult PaintMark(nsRenderingContext *aContext,
                      nsSVGPathGeometryFrame *aMarkedFrame,
                      nsSVGMark *aMark,
                      float aStrokeWidth);
 
-  gfxRect GetMarkBBoxContribution(const gfxMatrix &aToBBoxUserspace,
+  SVGBBox GetMarkBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                   PRUint32 aFlags,
                                   nsSVGPathGeometryFrame *aMarkedFrame,
                                   const nsSVGMark *aMark,
                                   float aStrokeWidth);
 
 private:
   // stuff needed for callback
   nsSVGPathGeometryFrame *mMarkedFrame;
--- a/layout/svg/base/src/nsSVGPathGeometryFrame.cpp
+++ b/layout/svg/base/src/nsSVGPathGeometryFrame.cpp
@@ -256,33 +256,33 @@ nsSVGPathGeometryFrame::NotifySVGChanged
   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);
   }
 }
 
-gfxRect
+SVGBBox
 nsSVGPathGeometryFrame::GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                             PRUint32 aFlags)
 {
+  SVGBBox bbox;
+
   if (aToBBoxUserspace.IsSingular()) {
     // XXX ReportToConsole
-    return gfxRect(0.0, 0.0, 0.0, 0.0);
+    return bbox;
   }
 
   nsRefPtr<gfxContext> context =
     new gfxContext(gfxPlatform::GetPlatform()->ScreenReferenceSurface());
 
   GeneratePath(context, &aToBBoxUserspace);
   context->IdentityMatrix();
 
-  gfxRect bbox;
-
   // Be careful when replacing the following logic to get the fill and stroke
   // extents independently (instead of computing the stroke extents from the
   // path extents). You may think that you can just use the stroke extents if
   // there is both a fill and a stroke. In reality it's necessary to calculate
   // both the fill and stroke extents, and take the union of the two. There are
   // two reasons for this:
   //
   // # Due to stroke dashing, in certain cases the fill extents could actually
@@ -311,20 +311,19 @@ nsSVGPathGeometryFrame::GetBBoxContribut
       // pathExtents before it can be used with PathExtentsToMaxStrokeExtents
       // though, because if pathExtents is empty, its position will not have
       // been set. Happily we can use context->GetUserStrokeExtent() to find
       // the center point of the extents even though it gets the extents wrong.
       SetupCairoStrokeGeometry(context);
       pathExtents.MoveTo(context->GetUserStrokeExtent().Center());
       pathExtents.SizeTo(0, 0);
     }
-    bbox =
-      bbox.Union(nsSVGUtils::PathExtentsToMaxStrokeExtents(pathExtents,
-                                                           this,
-                                                           aToBBoxUserspace));
+    bbox.UnionEdges(nsSVGUtils::PathExtentsToMaxStrokeExtents(pathExtents,
+                                                              this,
+                                                              aToBBoxUserspace));
   }
 
   // Account for markers:
   if ((aFlags & nsSVGUtils::eBBoxIncludeMarkers) != 0 &&
       static_cast<nsSVGPathGeometryElement*>(mContent)->IsMarkable()) {
 
     float strokeWidth = GetStrokeWidth();
     MarkerProperties properties = GetMarkerProperties(this);
@@ -332,38 +331,38 @@ nsSVGPathGeometryFrame::GetBBoxContribut
     if (properties.MarkersExist()) {
       nsTArray<nsSVGMark> marks;
       static_cast<nsSVGPathGeometryElement*>(mContent)->GetMarkPoints(&marks);
       PRUint32 num = marks.Length();
 
       if (num) {
         nsSVGMarkerFrame *frame = properties.GetMarkerStartFrame();
         if (frame) {
-          gfxRect mbbox =
+          SVGBBox mbbox =
             frame->GetMarkBBoxContribution(aToBBoxUserspace, aFlags, this,
                                            &marks[0], strokeWidth);
-          bbox.UnionRect(bbox, mbbox);
+          bbox.UnionEdges(mbbox);
         }
 
         frame = properties.GetMarkerMidFrame();
         if (frame) {
           for (PRUint32 i = 1; i < num - 1; i++) {
-            gfxRect mbbox =
+            SVGBBox mbbox =
               frame->GetMarkBBoxContribution(aToBBoxUserspace, aFlags, this,
                                              &marks[i], strokeWidth);
-            bbox.UnionRect(bbox, mbbox);
+            bbox.UnionEdges(mbbox);
           }
         }
 
         frame = properties.GetMarkerEndFrame();
         if (frame) {
-          gfxRect mbbox =
+          SVGBBox mbbox =
             frame->GetMarkBBoxContribution(aToBBoxUserspace, aFlags, this,
                                            &marks[num-1], strokeWidth);
-          bbox.UnionRect(bbox, mbbox);
+          bbox.UnionEdges(mbbox);
         }
       }
     }
   }
 
   return bbox;
 }
 
--- a/layout/svg/base/src/nsSVGPathGeometryFrame.h
+++ b/layout/svg/base/src/nsSVGPathGeometryFrame.h
@@ -102,17 +102,17 @@ public:
 protected:
   // nsISVGChildFrame interface:
   NS_IMETHOD PaintSVG(nsRenderingContext *aContext,
                       const nsIntRect *aDirtyRect);
   NS_IMETHOD_(nsIFrame*) GetFrameForPoint(const nsPoint &aPoint);
   NS_IMETHOD_(nsRect) GetCoveredRegion();
   virtual void UpdateBounds();
   virtual void NotifySVGChanged(PRUint32 aFlags);
-  virtual gfxRect GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
+  virtual SVGBBox GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                       PRUint32 aFlags);
   NS_IMETHOD_(bool) IsDisplayContainer() { return false; }
   NS_IMETHOD_(bool) HasValidCoveredRect() {
     return !(GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD);
   }
 
 protected:
   void GeneratePath(gfxContext *aContext,
--- a/layout/svg/base/src/nsSVGSwitchFrame.cpp
+++ b/layout/svg/base/src/nsSVGSwitchFrame.cpp
@@ -77,17 +77,17 @@ public:
     return MakeFrameName(NS_LITERAL_STRING("SVGSwitch"), aResult);
   }
 #endif
 
   // nsISVGChildFrame interface:
   NS_IMETHOD PaintSVG(nsRenderingContext* aContext, const nsIntRect *aDirtyRect);
   NS_IMETHODIMP_(nsIFrame*) GetFrameForPoint(const nsPoint &aPoint);
   NS_IMETHODIMP_(nsRect) GetCoveredRegion();
-  virtual gfxRect GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
+  virtual SVGBBox GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                       PRUint32 aFlags);
 
 private:
   nsIFrame *GetActiveChildFrame();
 };
 
 //----------------------------------------------------------------------
 // Implementation
@@ -159,32 +159,34 @@ nsSVGSwitchFrame::GetCoveredRegion()
     nsISVGChildFrame* child = do_QueryFrame(kid);
     if (child) {
       rect = child->GetCoveredRegion();
     }
   }
   return rect;
 }
 
-gfxRect
+SVGBBox
 nsSVGSwitchFrame::GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                       PRUint32 aFlags)
 {
   nsIFrame* kid = GetActiveChildFrame();
-  nsISVGChildFrame* svgKid = do_QueryFrame(kid);
-  if (svgKid) {
-    nsIContent *content = kid->GetContent();
-    gfxMatrix transform = aToBBoxUserspace;
-    if (content->IsSVG()) {
-      transform = static_cast<nsSVGElement*>(content)->
-                    PrependLocalTransformsTo(aToBBoxUserspace);
+  if (kid) {
+    nsISVGChildFrame* svgKid = do_QueryFrame(kid);
+    if (svgKid) {
+      nsIContent *content = kid->GetContent();
+      gfxMatrix transform = aToBBoxUserspace;
+      if (content->IsSVG()) {
+        transform = static_cast<nsSVGElement*>(content)->
+                      PrependLocalTransformsTo(aToBBoxUserspace);
+      }
+      return svgKid->GetBBoxContribution(transform, aFlags);
     }
-    return svgKid->GetBBoxContribution(transform, aFlags);
   }
-  return gfxRect(0.0, 0.0, 0.0, 0.0);
+  return SVGBBox();
 }
 
 nsIFrame *
 nsSVGSwitchFrame::GetActiveChildFrame()
 {
   nsIContent *activeChild =
     static_cast<nsSVGSwitchElement*>(mContent)->GetActiveChild();
 
--- a/layout/svg/base/src/nsSVGTextFrame.cpp
+++ b/layout/svg/base/src/nsSVGTextFrame.cpp
@@ -256,17 +256,17 @@ nsSVGTextFrame::UpdateBounds()
   // With glyph positions updated, our descendants can invalidate their new
   // areas correctly:
   nsSVGTextFrameBase::UpdateBounds();
 
   // XXXsvgreflow once we store bounds on containers, call
   // nsSVGUtils::InvalidateBounds(this) if not first reflow.
 }
 
-gfxRect
+SVGBBox
 nsSVGTextFrame::GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                     PRUint32 aFlags)
 {
   UpdateGlyphPositioning(true);
 
   return nsSVGTextFrameBase::GetBBoxContribution(aToBBoxUserspace, aFlags);
 }
 
--- a/layout/svg/base/src/nsSVGTextFrame.h
+++ b/layout/svg/base/src/nsSVGTextFrame.h
@@ -87,17 +87,17 @@ public:
   // nsISVGChildFrame interface:
   virtual void NotifySVGChanged(PRUint32 aFlags);
   // Override these four to ensure that UpdateGlyphPositioning is called
   // to bring glyph positions up to date
   NS_IMETHOD PaintSVG(nsRenderingContext* aContext,
                       const nsIntRect *aDirtyRect);
   NS_IMETHOD_(nsIFrame*) GetFrameForPoint(const nsPoint & aPoint);
   virtual void UpdateBounds();
-  virtual gfxRect GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
+  virtual SVGBBox GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                       PRUint32 aFlags);
   
   // nsSVGContainerFrame methods:
   virtual gfxMatrix GetCanvasTM();
   
   // nsSVGTextContainerFrame methods:
   virtual PRUint32 GetNumberOfChars();
   virtual float GetComputedTextLength();
--- a/layout/svg/base/src/nsSVGUtils.cpp
+++ b/layout/svg/base/src/nsSVGUtils.cpp
@@ -1533,22 +1533,19 @@ nsSVGUtils::GetBBox(nsIFrame *aFrame, PR
       // The spec says getBBox "Returns the tight bounding box in *current user
       // space*". So we should really be doing this for all elements, but that
       // needs investigation to check that we won't break too much content.
       NS_ABORT_IF_FALSE(content->IsSVG(), "bad cast");
       nsSVGElement *element = static_cast<nsSVGElement*>(content);
       matrix = element->PrependLocalTransformsTo(matrix,
                           nsSVGElement::eChildToUserSpace);
     }
-    bbox = svg->GetBBoxContribution(matrix, aFlags);
-  } else {
-    bbox = nsSVGIntegrationUtils::GetSVGBBoxForNonSVGFrame(aFrame);
+    return svg->GetBBoxContribution(matrix, aFlags);
   }
-  NS_ASSERTION(bbox.Width() >= 0.0 && bbox.Height() >= 0.0, "Invalid bbox!");
-  return bbox;
+  return nsSVGIntegrationUtils::GetSVGBBoxForNonSVGFrame(aFrame);
 }
 
 gfxRect
 nsSVGUtils::GetRelativeRect(PRUint16 aUnits, const nsSVGLength2 *aXYWH,
                             const gfxRect &aBBox, nsIFrame *aFrame)
 {
   float x, y, width, height;
   if (aUnits == nsIDOMSVGUnitTypes::SVG_UNIT_TYPE_OBJECTBOUNDINGBOX) {
--- a/layout/svg/base/src/nsSVGUtils.h
+++ b/layout/svg/base/src/nsSVGUtils.h
@@ -138,16 +138,54 @@ IsSVGWhitespace(PRUnichar aChar)
 }
 
 /*
  * Checks the smil enabled preference.  Declared as a function to match
  * NS_SVGEnabled().
  */
 bool NS_SMILEnabled();
 
+/**
+ * Sometimes we need to distinguish between an empty box and a box
+ * that contains an element that has no size e.g. a point at the origin.
+ */
+class SVGBBox {
+public:
+  SVGBBox() 
+    : mIsEmpty(true) {}
+
+  SVGBBox(const gfxRect& aRect) 
+    : mBBox(aRect), mIsEmpty(false) {}
+
+  SVGBBox& operator=(const gfxRect& aRect) {
+    mBBox = aRect;
+    mIsEmpty = false;
+    return *this;
+  }
+
+  operator const gfxRect& () const {
+    return mBBox;
+  }
+
+  bool IsEmpty() const {
+    return mIsEmpty;
+  }
+
+  void UnionEdges(const SVGBBox& aSVGBBox) {
+    if (aSVGBBox.mIsEmpty) {
+      return;
+    }
+    mBBox = mIsEmpty ? aSVGBBox.mBBox : mBBox.UnionEdges(aSVGBBox.mBBox);
+    mIsEmpty = false;
+  }
+
+private:
+  gfxRect mBBox;
+  bool    mIsEmpty;
+};
 
 // GRRR WINDOWS HATE HATE HATE
 #undef CLIP_MASK
 
 class NS_STACK_CLASS SVGAutoRenderState
 {
 public:
   enum RenderMode { NORMAL, CLIP, CLIP_MASK };