Bug 684479 - Bounding boxes for strokes are unnecessarily big for many shapes. r=roc
authorRobert Longson <longsonr@gmail.com>
Mon, 05 Sep 2011 18:53:34 +0100
changeset 77871 620b73b3d768044892f08c52b8cfb407b928d228
parent 77870 b38795f5d13ba43408eaaecdae7a80d8ddff6b24
child 77872 f2ece80d840ebbe0f37ed84b5260264897c85725
push id78
push userclegnitto@mozilla.com
push dateFri, 16 Dec 2011 17:32:24 +0000
treeherdermozilla-release@79d24e644fdd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs684479
milestone9.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 684479 - Bounding boxes for strokes are unnecessarily big for many shapes. r=roc
content/svg/content/test/Makefile.in
content/svg/content/test/bounds-helper.svg
content/svg/content/test/test_bounds.html
layout/svg/base/src/nsSVGUtils.cpp
layout/svg/base/src/nsSVGUtils.h
--- a/content/svg/content/test/Makefile.in
+++ b/content/svg/content/test/Makefile.in
@@ -55,16 +55,17 @@ include $(topsrcdir)/config/rules.mk
 		a_href_helper_01.svg \
 		a_href_helper_02_03.svg \
 		a_href_helper_04.svg \
 		test_animLengthObjectIdentity.xhtml \
 		test_animLengthReadonly.xhtml \
 		test_animLengthUnits.xhtml \
 		test_bbox.xhtml \
 		test_bbox-with-invalid-viewBox.xhtml \
+		test_bounds.html \
 		bbox-helper.svg \
 		bounds-helper.svg \
 		test_dataTypes.html \
 		dataTypes-helper.svg \
 		getCTM-helper.svg \
 		test_getCTM.html \
 		test_getSubStringLength.xhtml \
 		getSubStringLength-helper.svg \
--- a/content/svg/content/test/bounds-helper.svg
+++ b/content/svg/content/test/bounds-helper.svg
@@ -2,29 +2,28 @@
 <svg xmlns="http://www.w3.org/2000/svg" width="750"
      xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1">
   <style type="text/css">
 text { font: 20px monospace; }
   </style>
 
 <g id="g">
   <text id="text1" x="25" y="25">abc</text>
+  <text id="text1a" x="85" y="25" stroke="black" stroke-width="4">abc</text>
   <rect id="rect1" x="50" y="50" width="50" height="50" fill="green"/>
-  <rect id="rect1a" x="50" y="50" width="50" height="50" fill="none" stroke-width="2" stroke="yellow"/>
+  <rect id="rect1a" x="50" y="50" width="50" height="50" fill="none" stroke-width="4" stroke="yellow"/>
   <text id="text2" x="125" y="25">abc</text>
+  <text id="text2a" x="185" y="25" stroke="black" stroke-width="10">abc</text>
   <g transform="rotate(45 175 75)">
     <rect id="rect2" x="150" y="50" width="50" height="50" fill="yellow"/>
-    <rect id="rect2a" x="150" y="50" width="50" height="50" fill="none" stroke-width="2" stroke="blue"/>
+    <rect id="rect2a" x="150" y="50" width="50" height="50" fill="none" stroke-width="4" stroke="blue"/>
     <text id="text3" x="150" y="50" text-anchor="middle">abc</text>
   </g>
   <g transform="scale(2)">
     <rect id="rect3" x="25" y="80" width="50" height="50" fill="green"/>
-    <rect id="rect3a" x="25" y="80" width="50" height="50" fill="none" stroke-width="2" stroke="blue"/>
+    <rect id="rect3a" x="25" y="80" width="50" height="50" fill="none" stroke-width="4" stroke="blue"/>
   </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="2" stroke="blue"/>
-    <text id="text4" x="125" y="125">abc</text>
+    <rect id="rect4a" x="150" y="50" width="50" height="50" fill="none" stroke-width="4" stroke="blue"/>
   </g>
-  <text id="text1a" x="85" y="25" stroke="black" stroke-width="1">M</text>
-  <text id="text2a" x="185" y="25" stroke="black" stroke-width="10">M</text>
 </g>
 </svg>
--- a/content/svg/content/test/test_bounds.html
+++ b/content/svg/content/test/test_bounds.html
@@ -14,111 +14,130 @@ https://bugzilla.mozilla.org/show_bug.cg
 <div id="content" style="display: none"></div>
 
 <iframe id="svg" src="bounds-helper.svg"></iframe>
 
 <pre id="test">
 <script class="testbody" type="application/javascript">
 SimpleTest.waitForExplicitFinish();
 
+function Rect(left, top, width, height)
+{
+  this.left = left;
+  this.top = top;
+  this.width = width;
+  this.height = height;
+}
+
+Rect.prototype.roundOut = function()
+{
+  this.width = Math.ceil(this.left + this.width) - Math.floor(this.left);
+  this.height = Math.ceil(this.top + this.height) - Math.floor(this.top);
+  this.left = Math.floor(this.left);
+  this.top = Math.floor(this.top);
+}
+
+var delta = 1;
+
+function isApproximately(a, b, message)
+{
+  ok(delta >= Math.abs(a - b), message + " - got " + a + ", expected " + b + " ± " + delta);
+}
+
 function runTest()
 {
-  function isRounded(a, b, message) {
-    is (Math.round(a), Math.round(b), message);
-  }
+  var doc = $("svg").contentWindow.document;
 
-  var doc = $("svg").contentWindow.document;
-  
   var text1 = doc.getElementById("text1");
-  
-  var len = text1.getComputedTextLength();
-  
+
   var text1Bounds = text1.getBoundingClientRect();
   var text2Bounds = doc.getElementById("text2").getBoundingClientRect();
   var text3Bounds = doc.getElementById("text3").getBoundingClientRect();
-  var text4Bounds = doc.getElementById("text4").getBoundingClientRect();
 
   var sin45 = Math.sin(Math.PI / 4);
 
-  isRounded(text1Bounds.left, 25, "text1.getBoundingClientRect().left");
-  isRounded(text1Bounds.width, len, "text1.getBoundingClientRect().width");
+  isApproximately(text1Bounds.left, 24, "text1.getBoundingClientRect().left");
 
-  isRounded(text2Bounds.left, text1Bounds.left + 100, "text2.getBoundingClientRect().left");
-  isRounded(text2Bounds.top, text1Bounds.top, "text2.getBoundingClientRect().top");
-  isRounded(text2Bounds.width, text1Bounds.width, "text2.getBoundingClientRect().width");
-  isRounded(text2Bounds.height, text1Bounds.height, "text2.getBoundingClientRect().height");
+  is(text2Bounds.left, text1Bounds.left + 100, "text2.getBoundingClientRect().left");
+  is(text2Bounds.top, text1Bounds.top, "text2.getBoundingClientRect().top");
+  is(text2Bounds.width, text1Bounds.width, "text2.getBoundingClientRect().width");
+  is(text2Bounds.height, text1Bounds.height, "text2.getBoundingClientRect().height");
 
-  isRounded(text3Bounds.width, (text1Bounds.width + text1Bounds.height) * sin45 + .5, "text3.getBoundingClientRect().width");
-  isRounded(text3Bounds.height, (text1Bounds.height  + text1Bounds.width) * sin45 + .5, "text3.getBoundingClientRect().height");
-
-  isRounded(text4Bounds.width, 2 * (text1Bounds.width + text1Bounds.height) * sin45, "text4.getBoundingClientRect().width");
-  isRounded(text4Bounds.height, 2 * ((text1Bounds.height  + text1Bounds.width) * sin45 - .5), "text4.getBoundingClientRect().height");
+  var r = (text1Bounds.width + text1Bounds.height) * sin45;
+  isApproximately(text3Bounds.width, Math.ceil(r), "text3.getBoundingClientRect().width");
+  isApproximately(text3Bounds.height, Math.ceil(r), "text3.getBoundingClientRect().height");
 
   var rect1Bounds = doc.getElementById("rect1").getBoundingClientRect();
   var rect2Bounds = doc.getElementById("rect2").getBoundingClientRect();
   var rect3Bounds = doc.getElementById("rect3").getBoundingClientRect();
   var rect4Bounds = doc.getElementById("rect4").getBoundingClientRect();
-  
-  isRounded(rect1Bounds.left, 50, "rect1.getBoundingClientRect().left");
-  isRounded(rect1Bounds.top, 50, "rect1.getBoundingClientRect().top");
-  isRounded(rect1Bounds.width, 50, "rect1.getBoundingClientRect().width");
-  isRounded(rect1Bounds.height, 50, "rect1.getBoundingClientRect().height");
+
+  is(rect1Bounds.left, 50, "rect1.getBoundingClientRect().left");
+  is(rect1Bounds.top, 50, "rect1.getBoundingClientRect().top");
+  is(rect1Bounds.width, 50, "rect1.getBoundingClientRect().width");
+  is(rect1Bounds.height, 50, "rect1.getBoundingClientRect().height");
 
-  isRounded(rect2Bounds.left, 175 - 50 * sin45 - .5, "rect2.getBoundingClientRect().left");
-  isRounded(rect2Bounds.top, 75 - 50 * sin45 - .5, "rect2.getBoundingClientRect().top");
-  isRounded(rect2Bounds.width, (50 * sin45 + .5) * 2, "rect2.getBoundingClientRect().width");
-  isRounded(rect2Bounds.height, (50 * sin45 + .5) * 2, "rect2.getBoundingClientRect().height");
+  rect = new Rect(175 - 50 * sin45, 75 - 50 * sin45, 50 * sin45 * 2, 50 * sin45 * 2);
+  rect.roundOut();
+  is(rect2Bounds.left, rect.left, "rect2.getBoundingClientRect().left");
+  is(rect2Bounds.top, rect.top, "rect2.getBoundingClientRect().top");
+  is(rect2Bounds.width, rect.width, "rect2.getBoundingClientRect().width");
+  is(rect2Bounds.height, rect.height, "rect2.getBoundingClientRect().height");
 
-  isRounded(rect3Bounds.left, 50, "rect3.getBoundingClientRect().left");
-  isRounded(rect3Bounds.top, 160, "rect3.getBoundingClientRect().top");
-  isRounded(rect3Bounds.width, 100, "rect3.getBoundingClientRect().width");
-  isRounded(rect3Bounds.height, 100, "rect3.getBoundingClientRect().height");
+  is(rect3Bounds.left, 50, "rect3.getBoundingClientRect().left");
+  is(rect3Bounds.top, 160, "rect3.getBoundingClientRect().top");
+  is(rect3Bounds.width, 100, "rect3.getBoundingClientRect().width");
+  is(rect3Bounds.height, 100, "rect3.getBoundingClientRect().height");
 
-  isRounded(rect4Bounds.left, 350 - 100 * sin45 - .5, "rect4.getBoundingClientRect().left");
-  isRounded(rect4Bounds.top, 150 - 100 * sin45 - .5, "rect4.getBoundingClientRect().top");
-  isRounded(rect4Bounds.width, (100 * sin45 + .5) * 2, "rect4.getBoundingClientRect().width");
-  isRounded(rect4Bounds.height, (100 * sin45 + .5) * 2, "rect4.getBoundingClientRect().height");
+  rect = new Rect(350 - 100 * sin45, 150 - 100 * sin45, 100 * sin45 * 2, 100 * sin45 * 2);
+  rect.roundOut();
+  is(rect4Bounds.left, rect.left, "rect4.getBoundingClientRect().left");
+  is(rect4Bounds.top, rect.top, "rect4.getBoundingClientRect().top");
+  is(rect4Bounds.width, rect.width, "rect4.getBoundingClientRect().width");
+  is(rect4Bounds.height, rect.height, "rect4.getBoundingClientRect().height");
 
   var rect1aBounds = doc.getElementById("rect1a").getBoundingClientRect();
   var rect2aBounds = doc.getElementById("rect2a").getBoundingClientRect();
   var rect3aBounds = doc.getElementById("rect3a").getBoundingClientRect();
   var rect4aBounds = doc.getElementById("rect4a").getBoundingClientRect();
-  
-  isRounded(rect1aBounds.left, 49, "rect1a.getBoundingClientRect().left");
-  isRounded(rect1aBounds.top, 49, "rect1a.getBoundingClientRect().top");
-  isRounded(rect1aBounds.width, 52, "rect1a.getBoundingClientRect().width");
-  isRounded(rect1aBounds.height, 52, "rect1a.getBoundingClientRect().height");
+
+  is(rect1aBounds.left, 48, "rect1a.getBoundingClientRect().left");
+  is(rect1aBounds.top, 48, "rect1a.getBoundingClientRect().top");
+  is(rect1aBounds.width, 54, "rect1a.getBoundingClientRect().width");
+  is(rect1aBounds.height, 54, "rect1a.getBoundingClientRect().height");
 
-  isRounded(rect2aBounds.left, 175 - 52 * sin45 - .5, "rect2a.getBoundingClientRect().left");
-  isRounded(rect2aBounds.top, 75 - 52 * sin45 - .5, "rect2a.getBoundingClientRect().top");
-  isRounded(rect2aBounds.width, 52 * sin45 * 2, "rect2a.getBoundingClientRect().width");
-  isRounded(rect2aBounds.height, 52 * sin45 * 2, "rect2a.getBoundingClientRect().height");
+  rect = new Rect(175 - 54 * sin45, 75 - 54 * sin45, 54 * sin45 * 2, 54 * sin45 * 2);
+  rect.roundOut();
+  is(rect2aBounds.left, rect.left, "rect2a.getBoundingClientRect().left");
+  is(rect2aBounds.top, rect.top, "rect2a.getBoundingClientRect().top");
+  is(rect2aBounds.width, rect.width, "rect2a.getBoundingClientRect().width");
+  is(rect2aBounds.height, rect.height, "rect2a.getBoundingClientRect().height");
 
-  isRounded(rect3aBounds.left, 48, "rect3a.getBoundingClientRect().left");
-  isRounded(rect3aBounds.top, 158, "rect3a.getBoundingClientRect().top");
-  isRounded(rect3aBounds.width, 104, "rect3a.getBoundingClientRect().width");
-  isRounded(rect3aBounds.height, 104, "rect3a.getBoundingClientRect().height");
+  is(rect3aBounds.left, 46, "rect3a.getBoundingClientRect().left");
+  is(rect3aBounds.top, 156, "rect3a.getBoundingClientRect().top");
+  is(rect3aBounds.width, 108, "rect3a.getBoundingClientRect().width");
+  is(rect3aBounds.height, 108, "rect3a.getBoundingClientRect().height");
 
-  isRounded(rect4aBounds.left, 350 - 104 * sin45 - .5, "rect4a.getBoundingClientRect().left");
-  isRounded(rect4aBounds.top, 150 - 104 * sin45 - .5, "rect4a.getBoundingClientRect().top");
-  isRounded(rect4aBounds.width, (104 * sin45 + .5) * 2, "rect4a.getBoundingClientRect().width");
-  isRounded(rect4aBounds.height, (104 * sin45 + .5) * 2, "rect4a.getBoundingClientRect().height");
+  rect = new Rect(350 - 108 * sin45, 150 - 108 * sin45, 108 * sin45 * 2, 108 * sin45 * 2);
+  rect.roundOut();
+  is(rect4aBounds.left, rect.left, "rect4a.getBoundingClientRect().left");
+  is(rect4aBounds.top, rect.top, "rect4a.getBoundingClientRect().top");
+  is(rect4aBounds.width, rect.width, "rect4a.getBoundingClientRect().width");
+  is(rect4aBounds.height, rect.height, "rect4a.getBoundingClientRect().height");
 
   var text1a = doc.getElementById("text1a");
-  
+
   var text1aBounds = text1a.getBoundingClientRect();
   var text2aBounds = doc.getElementById("text2a").getBoundingClientRect();
 
-  var len = text1a.getComputedTextLength();
+  isApproximately(text1aBounds.left, 82, "text1a.getBoundingClientRect().left");
+  is(text1aBounds.width, text1Bounds.width + 4, "text1a.getBoundingClientRect().width");
 
-  isRounded(text1aBounds.left, 85 - 1, "text1a.getBoundingClientRect().left");
-  isRounded(text1aBounds.width, len + 1, "text1a.getBoundingClientRect().width");
-  
-  isRounded(text2aBounds.left, text1aBounds.left + 100 - 4, "text2a.getBoundingClientRect().left");
-  isRounded(text2aBounds.width, text1aBounds.width + 9, "text2a.getBoundingClientRect().width");
+  is(text2aBounds.left, text1aBounds.left + 100 - 3, "text2a.getBoundingClientRect().left");
+  is(text2aBounds.width, text1aBounds.width + 6, "text2a.getBoundingClientRect().width");
 
   SimpleTest.finish();
 }
 
 window.addEventListener("load", runTest, false);
 </script>
 </pre>
 </body>
--- a/layout/svg/base/src/nsSVGUtils.cpp
+++ b/layout/svg/base/src/nsSVGUtils.cpp
@@ -64,32 +64,32 @@
 #include "nsSVGClipPathFrame.h"
 #include "nsSVGMaskFrame.h"
 #include "nsSVGContainerFrame.h"
 #include "nsSVGTextContainerFrame.h"
 #include "nsSVGLength2.h"
 #include "nsGenericElement.h"
 #include "nsSVGGraphicElement.h"
 #include "nsAttrValue.h"
-#include "nsSVGGeometryFrame.h"
 #include "nsIScriptError.h"
 #include "gfxContext.h"
 #include "gfxMatrix.h"
 #include "gfxRect.h"
 #include "gfxImageSurface.h"
 #include "gfxPlatform.h"
 #include "nsSVGForeignObjectFrame.h"
 #include "nsIDOMSVGUnitTypes.h"
 #include "nsSVGEffects.h"
 #include "nsMathUtils.h"
 #include "nsSVGIntegrationUtils.h"
 #include "nsSVGFilterPaintCallback.h"
 #include "nsSVGGeometryFrame.h"
 #include "nsComputedDOMStyle.h"
 #include "nsSVGPathGeometryFrame.h"
+#include "nsSVGPathGeometryElement.h"
 #include "prdtoa.h"
 #include "mozilla/dom/Element.h"
 #include "gfxUtils.h"
 #include "mozilla/Preferences.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
 
@@ -1426,47 +1426,67 @@ nsSVGUtils::WritePPM(const char *fname, 
       fwrite(data + y * stride + 4 * x + GFX_ARGB32_OFFSET_G, 1, 1, f);
       fwrite(data + y * stride + 4 * x + GFX_ARGB32_OFFSET_B, 1, 1, f);
     }
   }
   fclose(f);
 }
 #endif
 
-/*static*/ gfxRect
-nsSVGUtils::PathExtentsToMaxStrokeExtents(const gfxRect& aPathExtents,
-                                          nsSVGGeometryFrame* aFrame)
+// The logic here comes from _cairo_stroke_style_max_distance_from_path
+static gfxRect
+PathExtentsToMaxStrokeExtents(const gfxRect& aPathExtents,
+                              nsSVGGeometryFrame* aFrame,
+                              double styleExpansionFactor)
 {
-  // The logic here comes from _cairo_stroke_style_max_distance_from_path
-
-  double style_expansion = 0.5;
-
-  const nsStyleSVG* style = aFrame->GetStyleSVG();
-
-  if (style->mStrokeLinecap == NS_STYLE_STROKE_LINECAP_SQUARE) {
-    style_expansion = M_SQRT1_2;
-  }
-
-  if (style->mStrokeLinejoin == NS_STYLE_STROKE_LINEJOIN_MITER &&
-      style_expansion < style->mStrokeMiterlimit) {
-    style_expansion = style->mStrokeMiterlimit;
-  }
-
-  style_expansion *= aFrame->GetStrokeWidth();
+  double style_expansion =
+    styleExpansionFactor * aFrame->GetStrokeWidth();
 
   gfxMatrix ctm = aFrame->GetCanvasTM();
 
   double dx = style_expansion * (fabs(ctm.xx) + fabs(ctm.xy));
   double dy = style_expansion * (fabs(ctm.yy) + fabs(ctm.yx));
 
   gfxRect strokeExtents = aPathExtents;
   strokeExtents.Inflate(dx, dy);
   return strokeExtents;
 }
 
+/*static*/ gfxRect
+nsSVGUtils::PathExtentsToMaxStrokeExtents(const gfxRect& aPathExtents,
+                                          nsSVGGeometryFrame* aFrame)
+{
+  return ::PathExtentsToMaxStrokeExtents(aPathExtents, aFrame, 0.5);
+}
+
+/*static*/ gfxRect
+nsSVGUtils::PathExtentsToMaxStrokeExtents(const gfxRect& aPathExtents,
+                                          nsSVGPathGeometryFrame* aFrame)
+{
+  double styleExpansionFactor = 0.5;
+
+  if (static_cast<nsSVGPathGeometryElement*>(aFrame->GetContent())->IsMarkable()) {
+    const nsStyleSVG* style = aFrame->GetStyleSVG();
+
+    if (style->mStrokeLinecap == NS_STYLE_STROKE_LINECAP_SQUARE) {
+      styleExpansionFactor = M_SQRT1_2;
+    }
+
+    if (style->mStrokeLinejoin == NS_STYLE_STROKE_LINEJOIN_MITER &&
+        styleExpansionFactor < style->mStrokeMiterlimit &&
+        aFrame->GetContent()->Tag() != nsGkAtoms::line) {
+      styleExpansionFactor = style->mStrokeMiterlimit;
+    }
+  }
+
+  return ::PathExtentsToMaxStrokeExtents(aPathExtents,
+                                         aFrame,
+                                         styleExpansionFactor);
+}
+
 // ----------------------------------------------------------------------
 
 nsSVGRenderState::nsSVGRenderState(nsRenderingContext *aContext) :
   mRenderMode(NORMAL), mRenderingContext(aContext), mPaintingToWindow(PR_FALSE)
 {
   mGfxContext = aContext->ThebesContext();
 }
 
--- a/layout/svg/base/src/nsSVGUtils.h
+++ b/layout/svg/base/src/nsSVGUtils.h
@@ -71,16 +71,17 @@ class gfxContext;
 class gfxASurface;
 class gfxPattern;
 class gfxImageSurface;
 struct gfxSize;
 struct nsStyleFont;
 class nsSVGEnum;
 class nsISVGChildFrame;
 class nsSVGGeometryFrame;
+class nsSVGPathGeometryFrame;
 class nsSVGDisplayContainerFrame;
 
 namespace mozilla {
 class SVGAnimatedPreserveAspectRatio;
 class SVGPreserveAspectRatio;
 namespace dom {
 class Element;
 } // namespace dom
@@ -559,16 +560,18 @@ public:
    * without doing the calculations for the actual tight extents. We exploit
    * the fact that cairo does have an API for getting the tight device space
    * fill/path extents.
    *
    * This should die once bug 478152 is fixed.
    */
   static gfxRect PathExtentsToMaxStrokeExtents(const gfxRect& aPathExtents,
                                                nsSVGGeometryFrame* aFrame);
+  static gfxRect PathExtentsToMaxStrokeExtents(const gfxRect& aPathExtents,
+                                               nsSVGPathGeometryFrame* aFrame);
 
   /**
    * Convert a floating-point value to a 32-bit integer value, clamping to
    * the range of valid integers.
    */
   static PRInt32 ClampToInt(double aVal)
   {
     return NS_lround(NS_MAX(double(PR_INT32_MIN),