Bug 919900 - Correct bounding box calculations for empty text elements with svg.text.css-frames.enabled. r=heycam
authorRobert Longson <longsonr@gmail.com>
Thu, 26 Sep 2013 13:42:15 +0100
changeset 148788 e97818c39f6ffe0661e9c3d82f0df924b67c21ab
parent 148787 37266243cead6482bc01196d5e6df2ac072f6188
child 148797 4c62a8db672a4c7056aa8a683b71a5a8dc4f070a
push id34335
push userlongsonr@gmail.com
push dateThu, 26 Sep 2013 12:42:28 +0000
treeherdermozilla-inbound@e97818c39f6f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs919900
milestone27.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 919900 - Correct bounding box calculations for empty text elements with svg.text.css-frames.enabled. r=heycam
content/svg/content/test/bbox-helper.svg
content/svg/content/test/bounds-helper.svg
content/svg/content/test/test_bounds.html
layout/svg/nsSVGTextFrame2.cpp
--- a/content/svg/content/test/bbox-helper.svg
+++ b/content/svg/content/test/bbox-helper.svg
@@ -16,14 +16,16 @@
   </g>
   <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/>
+    <!-- nor should an empty text element -->
+    <text x="185" y="25"/>
     <circle cx="100" cy="100" r="5"/>
     <g/>
     <circle cx="100" cy="100" r="5"/>
     <g/>
   </g>
 </svg>
--- a/content/svg/content/test/bounds-helper.svg
+++ b/content/svg/content/test/bounds-helper.svg
@@ -22,10 +22,15 @@ text { font: 20px monospace; }
     <rect id="rect3a" x="25" y="80" width="50" height="50" fill="none" stroke-width="4" stroke="blue"/>
     <rect id="rect3b" vector-effect="non-scaling-stroke" x="100" y="100" width="25" height="25" fill="orange" stroke-width="4" stroke="yellow"/>
     <image id="i" x="10" y="10" width="100" height="100"/>
   </g>
   <g transform="scale(2) rotate(45 175 75)">
     <rect id="rect4" x="150" y="50" width="50" height="50" fill="yellow"/>
     <rect id="rect4a" x="150" y="50" width="50" height="50" fill="none" stroke-width="4" stroke="blue"/>
   </g>
+  <text id="text4" x="185" y="25"/>
+  <g id="g2">
+    <rect x="100" y="100" width="50" height="50" fill="pink"/>
+    <text x="200" y="200"/>
+  </g>
 </g>
 </svg>
--- a/content/svg/content/test/test_bounds.html
+++ b/content/svg/content/test/test_bounds.html
@@ -136,16 +136,28 @@ function runTest()
 
   var i = doc.getElementById("i");
   var iBounds = i.getBoundingClientRect();
   is(iBounds.left, 20, "i.getBoundingClientRect().left");
   is(iBounds.top, 20, "i.getBoundingClientRect().top");
   is(iBounds.width, 200, "i.getBoundingClientRect().width");
   is(iBounds.height, 200, "i.getBoundingClientRect().height");
 
+  var text4Bounds = doc.getElementById("text4").getBoundingClientRect();
+  is(text4Bounds.left, 0, "text4.getBoundingClientRect().left");
+  is(text4Bounds.top, 0, "text4.getBoundingClientRect().top");
+  is(text4Bounds.width, 0, "text4.getBoundingClientRect().width");
+  is(text4Bounds.height, 0, "text4.getBoundingClientRect().height");
+
+  var gBounds = doc.getElementById("g2").getBoundingClientRect();
+  is(gBounds.left, 100, "g2.getBoundingClientRect().left");
+  is(gBounds.top, 100, "g2.getBoundingClientRect().top");
+  is(gBounds.width, 50, "g2.getBoundingClientRect().width");
+  is(gBounds.height, 50, "g2.getBoundingClientRect().height");
+
   SimpleTest.finish();
 }
 
 window.addEventListener("load", runTest, false);
 </script>
 </pre>
 </body>
 </html>
--- a/layout/svg/nsSVGTextFrame2.cpp
+++ b/layout/svg/nsSVGTextFrame2.cpp
@@ -640,17 +640,17 @@ struct TextRenderedRun
    * Returns a rectangle that bounds the fill and/or stroke of the rendered run
    * in run user space.
    *
    * @param aContext The context to use for unit conversions.
    * @param aFlags A combination of the flags above (eIncludeFill and
    *   eIncludeStroke) indicating what parts of the text to include in
    *   the rectangle.
    */
-  gfxRect GetRunUserSpaceRect(nsPresContext* aContext, uint32_t aFlags) const;
+  SVGBBox GetRunUserSpaceRect(nsPresContext* aContext, uint32_t aFlags) const;
 
   /**
    * Returns a rectangle that covers the fill and/or stroke of the rendered run
    * in "frame user space".
    *
    * Frame user space is a coordinate space of the same scale as the <text>
    * element's user space, but with its rotation set to the rotation of
    * the glyphs within this rendered run and its origin set to the position
@@ -677,30 +677,30 @@ struct TextRenderedRun
    * returned by this method will be (12, 0, 12, 14), since we are
    * advance("a") horizontally in to the text frame.
    *
    * @param aContext The context to use for unit conversions.
    * @param aFlags A combination of the flags above (eIncludeFill and
    *   eIncludeStroke) indicating what parts of the text to include in
    *   the rectangle.
    */
-  gfxRect GetFrameUserSpaceRect(nsPresContext* aContext, uint32_t aFlags) const;
+  SVGBBox GetFrameUserSpaceRect(nsPresContext* aContext, uint32_t aFlags) const;
 
   /**
    * Returns a rectangle that covers the fill and/or stroke of the rendered run
    * in the <text> element's user space.
    *
    * @param aContext The context to use for unit conversions.
    * @param aFlags A combination of the flags above indicating what parts of the
    *   text to include in the rectangle.
    * @param aAdditionalTransform An additional transform to apply to the
    *   frame user space rectangle before its bounds are transformed into
    *   user space.
    */
-  gfxRect GetUserSpaceRect(nsPresContext* aContext, uint32_t aFlags,
+  SVGBBox GetUserSpaceRect(nsPresContext* aContext, uint32_t aFlags,
                            const gfxMatrix* aAdditionalTransform = nullptr) const;
 
   /**
    * Gets the app unit amounts to clip from the left and right edges of
    * the nsTextFrame in order to paint just this rendered run.
    *
    * Note that if clip edge amounts land in the middle of a glyph, the
    * glyph won't be painted at all.  The clip edges are thus more of
@@ -860,21 +860,21 @@ TextRenderedRun::GetTransformFromRunUser
   GetClipEdges(left, right);
 
   // Translate by the horizontal distance into the text frame this
   // rendered run is.
   return m.Translate(gfxPoint(gfxFloat(left) / aContext->AppUnitsPerCSSPixel(),
                               0));
 }
 
-gfxRect
+SVGBBox
 TextRenderedRun::GetRunUserSpaceRect(nsPresContext* aContext,
                                      uint32_t aFlags) const
 {
-  gfxRect r;
+  SVGBBox r;
   if (!mFrame) {
     return r;
   }
 
   // Determine the amount of overflow above and below the frame's mRect.
   //
   // We need to call GetVisualOverflowRectRelativeToSelf because this includes
   // overflowing decorations, which the MeasureText call below does not.  We
@@ -925,38 +925,44 @@ TextRenderedRun::GetRunUserSpaceRect(nsP
   // Include the fill if requested.
   if (aFlags & eIncludeFill) {
     r = fill;
   }
 
   // Include the stroke if requested.
   if ((aFlags & eIncludeStroke) &&
       nsSVGUtils::GetStrokeWidth(mFrame) > 0) {
-    r = r.Union(nsSVGUtils::PathExtentsToMaxStrokeExtents(fill, mFrame,
-                                                          gfxMatrix()));
+    r.UnionEdges(nsSVGUtils::PathExtentsToMaxStrokeExtents(fill, mFrame,
+                                                           gfxMatrix()));
   }
 
   return r;
 }
 
-gfxRect
+SVGBBox
 TextRenderedRun::GetFrameUserSpaceRect(nsPresContext* aContext,
                                        uint32_t aFlags) const
 {
-  gfxRect r = GetRunUserSpaceRect(aContext, aFlags);
+  SVGBBox r = GetRunUserSpaceRect(aContext, aFlags);
+  if (r.IsEmpty()) {
+    return r;
+  }
   gfxMatrix m = GetTransformFromRunUserSpaceToFrameUserSpace(aContext);
   return m.TransformBounds(r);
 }
 
-gfxRect
+SVGBBox
 TextRenderedRun::GetUserSpaceRect(nsPresContext* aContext,
                                   uint32_t aFlags,
                                   const gfxMatrix* aAdditionalTransform) const
 {
-  gfxRect r = GetRunUserSpaceRect(aContext, aFlags);
+  SVGBBox r = GetRunUserSpaceRect(aContext, aFlags);
+  if (r.IsEmpty()) {
+    return r;
+  }
   gfxMatrix m = GetTransformFromRunUserSpaceToUserSpace(aContext);
   if (aAdditionalTransform) {
     m.Multiply(*aAdditionalTransform);
   }
   return m.TransformBounds(r);
 }
 
 void
@@ -3302,26 +3308,27 @@ nsSVGTextFrame2::FindCloserFrameForSelec
   nsPresContext* presContext = PresContext();
 
   // Find the frame that has the closest rendered run rect to aPoint.
   TextRenderedRunIterator it(this);
   for (TextRenderedRun run = it.Current(); run.mFrame; run = it.Next()) {
     uint32_t flags = TextRenderedRun::eIncludeFill |
                      TextRenderedRun::eIncludeStroke |
                      TextRenderedRun::eNoHorizontalOverflow;
-    gfxRect userRect = run.GetUserSpaceRect(presContext, flags);
-
-    nsRect rect = nsSVGUtils::ToCanvasBounds(userRect,
-                                             GetCanvasTM(FOR_HIT_TESTING),
-                                             presContext);
-
-    if (nsLayoutUtils::PointIsCloserToRect(aPoint, rect,
-                                           aCurrentBestFrame->mXDistance,
-                                           aCurrentBestFrame->mYDistance)) {
-      aCurrentBestFrame->mFrame = run.mFrame;
+    SVGBBox userRect = run.GetUserSpaceRect(presContext, flags);
+    if (!userRect.IsEmpty()) {
+      nsRect rect = nsSVGUtils::ToCanvasBounds(userRect,
+                                               GetCanvasTM(FOR_HIT_TESTING),
+                                               presContext);
+
+      if (nsLayoutUtils::PointIsCloserToRect(aPoint, rect,
+                                             aCurrentBestFrame->mXDistance,
+                                             aCurrentBestFrame->mYDistance)) {
+        aCurrentBestFrame->mFrame = run.mFrame;
+      }
     }
   }
 }
 
 //----------------------------------------------------------------------
 // nsISVGChildFrame methods
 
 void
@@ -3635,43 +3642,47 @@ nsSVGTextFrame2::ReflowSVG()
     return;
   }
 
   MaybeReflowAnonymousBlockChild();
   UpdateGlyphPositioning();
 
   nsPresContext* presContext = PresContext();
 
-  gfxRect r;
+  SVGBBox r;
   TextRenderedRunIterator it(this, TextRenderedRunIterator::eAllFrames);
   for (TextRenderedRun run = it.Current(); run.mFrame; run = it.Next()) {
     uint32_t runFlags = 0;
     uint16_t hitTestFlags = nsSVGUtils::GetGeometryHitTestFlags(run.mFrame);
 
     if ((hitTestFlags & SVG_HIT_TEST_FILL) ||
         run.mFrame->StyleSVG()->mFill.mType != eStyleSVGPaintType_None) {
       runFlags |= TextRenderedRun::eIncludeFill;
     }
     if ((hitTestFlags & SVG_HIT_TEST_STROKE) ||
         nsSVGUtils::HasStroke(run.mFrame)) {
       runFlags |= TextRenderedRun::eIncludeStroke;
     }
 
     if (runFlags) {
-      r = r.Union(run.GetUserSpaceRect(presContext, runFlags));
+      r.UnionEdges(run.GetUserSpaceRect(presContext, runFlags));
     }
   }
-  mRect =
-    nsLayoutUtils::RoundGfxRectToAppRect(r, presContext->AppUnitsPerCSSPixel());
-
-  // Due to rounding issues when we have a transform applied, we sometimes
-  // don't include an additional row of pixels.  For now, just inflate our
-  // covered region.
-  mRect.Inflate(presContext->AppUnitsPerDevPixel());
-
+
+  if (r.IsEmpty()) {
+    mRect.SetEmpty();
+  } else {
+    mRect =
+      nsLayoutUtils::RoundGfxRectToAppRect(r, presContext->AppUnitsPerCSSPixel());
+
+    // Due to rounding issues when we have a transform applied, we sometimes
+    // don't include an additional row of pixels.  For now, just inflate our
+    // covered region.
+    mRect.Inflate(presContext->AppUnitsPerDevPixel());
+  }
 
   if (mState & NS_FRAME_FIRST_REFLOW) {
     // Make sure we have our filter property (if any) before calling
     // FinishAndStoreOverflow (subsequent filter changes are handled off
     // nsChangeHint_UpdateEffects):
     nsSVGEffects::UpdateEffects(this);
   }
 
@@ -3714,25 +3725,25 @@ TextRenderedRunFlagsForBBoxContribution(
 SVGBBox
 nsSVGTextFrame2::GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                      uint32_t aFlags)
 {
   NS_ASSERTION(GetFirstPrincipalChild(), "must have a child frame");
 
   UpdateGlyphPositioning();
 
-  gfxRect bbox;
+  SVGBBox bbox;
   nsPresContext* presContext = PresContext();
 
   TextRenderedRunIterator it(this);
   for (TextRenderedRun run = it.Current(); run.mFrame; run = it.Next()) {
     uint32_t flags = TextRenderedRunFlagsForBBoxContribution(run, aFlags);
-    gfxRect bboxForRun =
+    SVGBBox bboxForRun =
       run.GetUserSpaceRect(presContext, flags, &aToBBoxUserspace);
-    bbox = bbox.Union(bboxForRun);
+    bbox.UnionEdges(bboxForRun);
   }
 
   return bbox;
 }
 
 //----------------------------------------------------------------------
 // nsSVGContainerFrame methods
 
@@ -5299,17 +5310,20 @@ nsSVGTextFrame2::TransformFrameRectToTex
     m.PreMultiply(run.GetTransformFromRunUserSpaceToUserSpace(presContext).Invert());
     m.PreMultiply(run.GetTransformFromRunUserSpaceToFrameUserSpace(presContext));
     gfxRect incomingRectInFrameUserSpace =
       m.TransformBounds(incomingRectInUserSpace);
 
     // Intersect it with this run's rectangle.
     uint32_t flags = TextRenderedRun::eIncludeFill |
                      TextRenderedRun::eIncludeStroke;
-    gfxRect runRectInFrameUserSpace = run.GetFrameUserSpaceRect(presContext, flags);
+    SVGBBox runRectInFrameUserSpace = run.GetFrameUserSpaceRect(presContext, flags);
+    if (runRectInFrameUserSpace.IsEmpty()) {
+      continue;
+    }
     gfxRect runIntersectionInFrameUserSpace =
       incomingRectInFrameUserSpace.Intersect(runRectInFrameUserSpace);
 
     if (!runIntersectionInFrameUserSpace.IsEmpty()) {
       // Take the font size scale into account.
       runIntersectionInFrameUserSpace.x *= mFontSizeScaleFactor;
       runIntersectionInFrameUserSpace.y *= mFontSizeScaleFactor;
       runIntersectionInFrameUserSpace.width *= mFontSizeScaleFactor;