Bug 919900 - Correct bounding box calculations for empty text elements with svg.text.css-frames.enabled. r=heycam, a=akeybl
authorRobert Longson <longsonr@gmail.com>
Sun, 29 Sep 2013 12:43:03 +0100
changeset 155560 f75dbfe92bcc1a2ddcf37f5e236795bf2f5e96d1
parent 155559 d9f5ddccb15ebfbe2ff6fb3225ea07c4c406db95
child 155561 f7271bceffa15e053bdd8803a3182013f68390aa
push id4328
push userryanvm@gmail.com
push dateMon, 30 Sep 2013 13:20:21 +0000
treeherdermozilla-aurora@e0353f4ef662 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam, akeybl
bugs919900
milestone26.0a2
Bug 919900 - Correct bounding box calculations for empty text elements with svg.text.css-frames.enabled. r=heycam, a=akeybl
content/svg/content/test/bbox-helper.svg
content/svg/content/test/bounds-helper.svg
content/svg/content/test/test_bounds.html
layout/reftests/svg/reftest.list
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/reftests/svg/reftest.list
+++ b/layout/reftests/svg/reftest.list
@@ -101,17 +101,17 @@ skip-if(B2G) == dynamic-pattern-contents
 == dynamic-rect-05.svg pass.svg
 == dynamic-reflow-01.svg dynamic-reflow-01-ref.svg
 == dynamic-small-object-scaled-up-01.svg pass.svg
 == dynamic-small-object-scaled-up-02.svg pass.svg
 == dynamic-switch-01.svg pass.svg
 == dynamic-text-01.svg dynamic-text-01-ref.svg
 fuzzy-if(d2d&&layersGPUAccelerated,2,12739) == dynamic-text-02.svg dynamic-text-02-ref.svg # bug 776038 for Win7
 fuzzy-if(d2d&&layersGPUAccelerated,2,10539) == dynamic-text-03.svg dynamic-text-03-ref.svg # bug 776038 for Win7
-random-if(/^Windows\x20NT\x205\.1/.test(http.oscpu)) == dynamic-text-04.svg dynamic-text-04-ref.svg # bug 421587 for WinXP
+random-if(/^Windows\x20NT\x205\.1/.test(http.oscpu)) fuzzy-if(/^Windows\x20NT\x206\.1/.test(http.oscpu),47,89) == dynamic-text-04.svg dynamic-text-04-ref.svg # bug 421587 for WinXP, bug 776038 for Win7
 skip-if(B2G) == dynamic-text-05.svg pass.svg
 skip-if(B2G) == dynamic-text-06.svg pass.svg
 == dynamic-text-07.svg dynamic-text-07-ref.svg
 == dynamic-text-08.svg dynamic-text-08-ref.svg
 skip-if(B2G) == dynamic-textPath-01.svg dynamic-textPath-01-ref.svg
 == dynamic-textPath-02.svg dynamic-textPath-02-ref.svg
 == dynamic-textPath-03.svg dynamic-textPath-03-ref.svg
 == dynamic-use-01.svg pass.svg
--- 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
@@ -3630,43 +3637,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);
   }
 
@@ -3709,25 +3720,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
 
@@ -5294,17 +5305,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;