Bug 777476 - text bounding boxes should ignore whitespace-only nodes when whitespace compression suppresses them. r=jwatt a=lsblakk
authorRobert Longson <longsonr@gmail.com>
Sun, 12 Aug 2012 10:45:09 +0100
changeset 100480 3ee4b5b63f6811f5fd7a189cdcb31feb1dae42eb
parent 100479 50f5c2689179782da8321ba49bb5c9fda6ae5b56
child 100481 19ad43a4a27b4deb009eecf1e6a1e33c199107ec
push id1262
push userlongsonr@gmail.com
push dateSun, 12 Aug 2012 09:45:32 +0000
treeherdermozilla-beta@3ee4b5b63f68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt, lsblakk
bugs777476
milestone15.0
Bug 777476 - text bounding boxes should ignore whitespace-only nodes when whitespace compression suppresses them. r=jwatt a=lsblakk
content/svg/content/test/bbox-helper.svg
content/svg/content/test/test_bbox.xhtml
layout/svg/base/src/nsSVGGlyphFrame.cpp
--- a/content/svg/content/test/bbox-helper.svg
+++ b/content/svg/content/test/bbox-helper.svg
@@ -1,16 +1,19 @@
 <?xml version="1.0"?>
 <svg xmlns="http://www.w3.org/2000/svg">
   <g transform="scale(0.5)">
     <foreignObject id="fO" x="10" y="10" width="100" height="100"/>
   </g>
   <text id="b" x="20" y="20">b</text>
   <text id="a" x="20" y="30">a</text>
   <text id="y" x="20" y="40">y</text>
+  <text id="tspan">
+    <tspan x="20" y="20">b</tspan>
+  </text>
   <g id="v">
     <circle cx="100" cy="50" r="5"/>
     <path d="M 100,100 L 100,200"/>
   </g>
   <g id="h">
     <circle cx="200" cy="50" r="5"/>
     <path d="M 200,100 L 300,100"/>
   </g>
--- a/content/svg/content/test/test_bbox.xhtml
+++ b/content/svg/content/test/test_bbox.xhtml
@@ -28,25 +28,34 @@ function run()
   }
   function checkBBox(id, x, y, width, height) {
     var bbox = getBBox(id);
     is(bbox.x, x, id + ".getBBox().x");
     is(bbox.y, y, id + ".getBBox().y");
     is(bbox.width, width, id + ".getBBox().width");
     is(bbox.height, height, id + ".getBBox().height");
   }
-  function checkBBoxHeight(id1, id2) {
+  function compareBBox(id1, id2) {
+    var bbox1 = getBBox(id1);
+    var bbox2 = getBBox(id2);
+    is(bbox1.x, bbox2.x, id1 + ".getBBox().x");
+    is(bbox1.y, bbox2.y, id1 + ".getBBox().y");
+    is(bbox1.width, bbox2.width, id1 + ".getBBox().width");
+    is(bbox1.height, bbox2.height, id1 + ".getBBox().height");
+  }
+  function compareBBoxHeight(id1, id2) {
     var bbox1 = getBBox(id1);
     var bbox2 = getBBox(id2);
     is(bbox1.height, bbox2.height, id1 + ".getBBox().height");
   }
 
   checkBBox("fO", 10, 10, 100, 100);
-  checkBBoxHeight("a", "b");
-  checkBBoxHeight("a", "y");
+  compareBBoxHeight("a", "b");
+  compareBBoxHeight("a", "y");
+  compareBBox("b", "tspan");
   checkBBox("v", 95, 45, 10, 155);
   checkBBox("h", 195, 45, 105, 55);
   checkBBox("e", 95, 95, 10, 10);
   
   SimpleTest.finish();
 }
 
 window.addEventListener("load", run, false);
--- a/layout/svg/base/src/nsSVGGlyphFrame.cpp
+++ b/layout/svg/base/src/nsSVGGlyphFrame.cpp
@@ -71,21 +71,22 @@ public:
    * cluster.
    * @param aForceGlobalTransform passed on to EnsureTextRun (see below)
    */
   CharacterIterator(nsSVGGlyphFrame *aSource, bool aForceGlobalTransform);
   /**
    * This matrix will be applied to aContext in the SetupFor methods below,
    * before any glyph translation/rotation.
    */
-  void SetInitialMatrix(gfxContext *aContext) {
+  bool SetInitialMatrix(gfxContext *aContext) {
     mInitialMatrix = aContext->CurrentMatrix();
     if (mInitialMatrix.IsSingular()) {
       mInError = true;
     }
+    return !mInError;
   }
   /**
    * Try to set up aContext so we can draw the whole textrun at once.
    * This applies any global transform requested by SetInitialMatrix,
    * then applies the positioning of the text. Returns false if drawing
    * the whole textrun at once is impossible due to individual positioning
    * and/or rotation of glyphs.
    */
@@ -317,17 +318,19 @@ nsSVGGlyphFrame::PaintSVG(nsRenderingCon
   }
 
   if (renderMode != SVGAutoRenderState::NORMAL) {
 
     gfxMatrix matrix = gfx->CurrentMatrix();
     SetupGlobalTransform(gfx);
 
     CharacterIterator iter(this, true);
-    iter.SetInitialMatrix(gfx);
+    if (!iter.SetInitialMatrix(gfx)) {
+      return NS_OK;
+    }
 
     if (GetClipRule() == NS_STYLE_FILL_RULE_EVENODD)
       gfx->SetFillRule(gfxContext::FILL_RULE_EVEN_ODD);
     else
       gfx->SetFillRule(gfxContext::FILL_RULE_WINDING);
 
     if (renderMode == SVGAutoRenderState::CLIP_MASK) {
       gfx->SetColor(gfxRGBA(1.0f, 1.0f, 1.0f, 1.0f));
@@ -341,17 +344,20 @@ nsSVGGlyphFrame::PaintSVG(nsRenderingCon
   }
 
   // We are adding patterns or gradients to the context. Save
   // it so we don't leak them into the next object we draw
   gfx->Save();
   SetupGlobalTransform(gfx);
 
   CharacterIterator iter(this, true);
-  iter.SetInitialMatrix(gfx);
+  if (!iter.SetInitialMatrix(gfx)) {
+    gfx->Restore();
+    return NS_OK;
+  }
 
   nsRefPtr<gfxPattern> strokePattern;
   DrawMode drawMode = SetupCairoState(gfx, getter_AddRefs(strokePattern));
 
   if (drawMode) {
     DrawCharacters(&iter, gfx, drawMode, strokePattern);
   }
   
@@ -366,17 +372,19 @@ nsSVGGlyphFrame::GetFrameForPoint(const 
   PRUint16 hitTestFlags = GetHitTestFlags();
   if (!hitTestFlags) {
     return nsnull;
   }
 
   nsRefPtr<gfxContext> context = MakeTmpCtx();
   SetupGlobalTransform(context);
   CharacterIterator iter(this, true);
-  iter.SetInitialMatrix(context);
+  if (!iter.SetInitialMatrix(context)) {
+    return nsnull;
+  }
 
   // The SVG 1.1 spec says that text is hit tested against the character cells
   // of the text, not the fill and stroke. See the section starting "For text
   // elements..." here:
   //
   //   http://www.w3.org/TR/SVG11/interact.html#PointerEventsProperty
   //
   // Currently we just test the character cells if GetHitTestFlags says we're
@@ -438,17 +446,19 @@ nsSVGGlyphFrame::UpdateBounds()
   bool hasStroke = HasStroke();
   if (hasStroke) {
     SetupCairoStrokeGeometry(tmpCtx);
   } else if (GetStyleSVG()->mFill.mType == eStyleSVGPaintType_None) {
     return;
   }
 
   CharacterIterator iter(this, true);
-  iter.SetInitialMatrix(tmpCtx);
+  if (!iter.SetInitialMatrix(tmpCtx)) {
+    return;
+  }
   AddBoundingBoxesToPath(&iter, tmpCtx);
   tmpCtx->IdentityMatrix();
 
   // 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
@@ -561,49 +571,50 @@ nsSVGGlyphFrame::DrawCharacters(Characte
                    aIter->ClusterLength(), nsnull, nsnull, aStrokePattern);
   }
 }
 
 SVGBBox
 nsSVGGlyphFrame::GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                      PRUint32 aFlags)
 {
+  SVGBBox bbox;
+
   if (mOverrideCanvasTM) {
     *mOverrideCanvasTM = aToBBoxUserspace;
   } else {
     mOverrideCanvasTM = new gfxMatrix(aToBBoxUserspace);
   }
 
   nsRefPtr<gfxContext> tmpCtx = MakeTmpCtx();
   SetupGlobalTransform(tmpCtx);
   CharacterIterator iter(this, true);
-  iter.SetInitialMatrix(tmpCtx);
+  if (!iter.SetInitialMatrix(tmpCtx)) {
+    return bbox;
+  }
   AddBoundingBoxesToPath(&iter, tmpCtx);
   tmpCtx->IdentityMatrix();
 
   mOverrideCanvasTM = nsnull;
 
-  gfxRect bbox;
-
   gfxRect pathExtents = tmpCtx->GetUserPathExtent();
 
   // Account for fill:
   if ((aFlags & nsSVGUtils::eBBoxIncludeFill) != 0 &&
       ((aFlags & nsSVGUtils::eBBoxIgnoreFillIfNone) == 0 ||
        GetStyleSVG()->mFill.mType != eStyleSVGPaintType_None)) {
     bbox = pathExtents;
   }
 
   // Account for stroke:
   if ((aFlags & nsSVGUtils::eBBoxIncludeStroke) != 0 &&
       ((aFlags & nsSVGUtils::eBBoxIgnoreStrokeIfNone) == 0 || HasStroke())) {
-    bbox =
-      bbox.Union(nsSVGUtils::PathExtentsToMaxStrokeExtents(pathExtents,
-                                                           this,
-                                                           aToBBoxUserspace));
+    bbox.UnionEdges(nsSVGUtils::PathExtentsToMaxStrokeExtents(pathExtents,
+                                                              this,
+                                                              aToBBoxUserspace));
   }
 
   return bbox;
 }
 
 //----------------------------------------------------------------------
 // nsSVGGeometryFrame methods: