Bug 1159053 - Cache SVG getBBox and objectBoundingBox calculations for better performance. r=heycam
authorJonathan Watt <jwatt@jwatt.org>
Mon, 27 Apr 2015 11:15:36 +0100
changeset 273262 605d1ea1ff14b2f37a873ba0693e1891a8785a89
parent 273261 4eb6bc8c98c864f4e3011b72aafcadb21647aa4e
child 273263 fa682ace73660c86a62b3928dbc2f7eb878b781d
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1159053
milestone40.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 1159053 - Cache SVG getBBox and objectBoundingBox calculations for better performance. r=heycam
dom/svg/test/mochitest.ini
dom/svg/test/test_bbox-changes.xhtml
layout/svg/nsSVGEffects.cpp
layout/svg/nsSVGUtils.cpp
layout/svg/nsSVGUtils.h
--- a/dom/svg/test/mochitest.ini
+++ b/dom/svg/test/mochitest.ini
@@ -29,16 +29,17 @@ support-files =
 
 [test_a_href_01.xhtml]
 [test_a_href_02.xhtml]
 [test_animLengthObjectIdentity.xhtml]
 [test_animLengthReadonly.xhtml]
 [test_animLengthUnits.xhtml]
 [test_bbox-with-invalid-viewBox.xhtml]
 [test_bbox.xhtml]
+[test_bbox-changes.xhtml]
 [test_bounds.html]
 [test_bug872812.html]
 [test_getBBox-method.html]
 [test_dataTypes.html]
 [test_dataTypesModEvents.html]
 [test_fragments.html]
 [test_getCTM.html]
 [test_getElementById.xhtml]
new file mode 100644
--- /dev/null
+++ b/dom/svg/test/test_bbox-changes.xhtml
@@ -0,0 +1,78 @@
+<!DOCTYPE html>
+<html xmlns="http://www.w3.org/1999/xhtml">
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1159053
+-->
+<head>
+  <title>Test that the results of getBBox update for changes</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+
+<p id="display">
+  <svg xmlns="http://www.w3.org/2000/svg">
+    <rect id="rect1" x="10" y="10" width="10" height="10"/>
+    <rect id="rect2" x="30" y="10" width="10" height="10"/>
+    <g id="g">
+      <circle id="circle1" cx="60" cy="20" r="5"/>
+      <circle id="circle2" cx="120" cy="20" r="5"/>
+    </g>
+  </svg>
+</p>
+
+<div id="content" style="display: none"></div>
+
+<pre id="test">
+<script class="testbody" type="application/javascript">//<![CDATA[
+
+SimpleTest.waitForExplicitFinish();
+
+function init_and_run() {
+  SpecialPowers.pushPrefEnv({"set": [["svg.new-getBBox.enabled", true]]}, run);
+}
+
+function checkBBox(id, options, x, y, width, height, msg) {
+  var bbox = document.getElementById(id).getBBox(options);
+  is(bbox.x, x, id + ".getBBox().x" + msg);
+  is(bbox.y, y, id + ".getBBox().y" + msg);
+  is(bbox.width, width, id + ".getBBox().width" + msg);
+  is(bbox.height, height, id + ".getBBox().height" + msg);
+}
+
+function run()
+{
+  // First call getBBox on 'rect1' with stroke included:
+  $("rect1").setAttribute("stroke", "black");
+  $("rect1").setAttribute("stroke-width", "10");
+  checkBBox("rect1", { fill:true, stroke:true }, 5, 5, 20, 20, " with stroke");
+
+  // Now remove the stroke from 'rect1' and check again:
+  $("rect1").removeAttribute("stroke");
+  $("rect1").removeAttribute("stroke-width");
+  checkBBox("rect1", { fill:true }, 10, 10, 10, 10, " after stroke removed");
+
+  // First call getBBox on 'rect2' without a stroke included:
+  checkBBox("rect2", { fill:true }, 30, 10, 10, 10, " with stroke");
+
+  // Now add a stroke to 'rect2' and check again:
+  $("rect2").setAttribute("stroke", "black");
+  $("rect2").setAttribute("stroke-width", "10");
+  checkBBox("rect2", { fill:true, stroke:true }, 25, 5, 20, 20, " with stroke");
+
+  // Check the initial result for getBBox on the group:
+  checkBBox("g", { fill:true }, 55, 15, 70, 10, " before child moves");
+
+  // Now move one of the circle children and check again:
+  $("circle2").setAttribute("cx", "110");
+  checkBBox("g", { fill:true }, 55, 15, 60, 10, " after child moves");
+
+  SimpleTest.finish();
+}
+
+window.addEventListener("load", init_and_run, false);
+
+//]]></script>
+</pre>
+</body>
+</html>
--- a/layout/svg/nsSVGEffects.cpp
+++ b/layout/svg/nsSVGEffects.cpp
@@ -773,16 +773,19 @@ nsSVGEffects::RemoveAllRenderingObserver
 void
 nsSVGEffects::InvalidateRenderingObservers(nsIFrame *aFrame)
 {
   NS_ASSERTION(!aFrame->GetPrevContinuation(), "aFrame must be first continuation");
 
   if (!aFrame->GetContent()->IsElement())
     return;
 
+  // If the rendering has changed, the bounds may well have changed too:
+  aFrame->Properties().Delete(nsSVGUtils::ObjectBoundingBoxProperty());
+
   nsSVGRenderingObserverList *observerList =
     GetObserverList(aFrame->GetContent()->AsElement());
   if (observerList) {
     observerList->InvalidateAll();
     return;
   }
 
   // Check ancestor SVG containers. The root frame cannot be of type
@@ -797,16 +800,22 @@ nsSVGEffects::InvalidateRenderingObserve
       }
     }
   }
 }
 
 void
 nsSVGEffects::InvalidateDirectRenderingObservers(Element *aElement, uint32_t aFlags /* = 0 */)
 {
+  nsIFrame* frame = aElement->GetPrimaryFrame();
+  if (frame) {
+    // If the rendering has changed, the bounds may well have changed too:
+    frame->Properties().Delete(nsSVGUtils::ObjectBoundingBoxProperty());
+  }
+
   if (aElement->HasRenderingObservers()) {
     nsSVGRenderingObserverList *observerList = GetObserverList(aElement);
     if (observerList) {
       if (aFlags & INVALIDATE_REFLOW) {
         observerList->InvalidateAllForReflow();
       } else {
         observerList->InvalidateAll();
       }
--- a/layout/svg/nsSVGUtils.cpp
+++ b/layout/svg/nsSVGUtils.cpp
@@ -898,16 +898,27 @@ nsSVGUtils::GetBBox(nsIFrame *aFrame, ui
       }
       svg = do_QueryFrame(ancestor);
     }
     nsIContent* content = aFrame->GetContent();
     if (content->IsSVGElement() &&
         !static_cast<const nsSVGElement*>(content)->HasValidDimensions()) {
       return bbox;
     }
+
+    FrameProperties props = aFrame->Properties();
+
+    if (aFlags == eBBoxIncludeFillGeometry) {
+      gfxRect* prop =
+        static_cast<gfxRect*>(props.Get(ObjectBoundingBoxProperty()));
+      if (prop) {
+        return *prop;
+      }
+    }
+
     gfxMatrix matrix;
     if (aFrame->GetType() == nsGkAtoms::svgForeignObjectFrame ||
         aFrame->GetType() == nsGkAtoms::svgUseFrame) {
       // 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.
       // NOTE: When changing this to apply to other frame types, make sure to
       // also update nsSVGUtils::FrameSpaceInCSSPxToUserSpaceOffset.
@@ -966,16 +977,23 @@ nsSVGUtils::GetBBox(nsIFrame *aFrame, ui
             bbox = bbox.Intersect(clipRect);
           }
         }
       }
       if (bbox.IsEmpty()) {
         bbox = gfxRect(0, 0, 0, 0);
       }
     }
+
+    if (aFlags == eBBoxIncludeFillGeometry) {
+      // Obtaining the bbox for objectBoundingBox calculations is common so we
+      // cache the result for future calls, since calculation can be expensive:
+      props.Set(ObjectBoundingBoxProperty(), new gfxRect(bbox));
+    }
+
     return bbox;
   }
   return nsSVGIntegrationUtils::GetSVGBBoxForNonSVGFrame(aFrame);
 }
 
 gfxPoint
 nsSVGUtils::FrameSpaceInCSSPxToUserSpaceOffset(nsIFrame *aFrame)
 {
--- a/layout/svg/nsSVGUtils.h
+++ b/layout/svg/nsSVGUtils.h
@@ -16,16 +16,17 @@
 #include "gfxPoint.h"
 #include "gfxRect.h"
 #include "mozilla/gfx/Rect.h"
 #include "nsAlgorithm.h"
 #include "nsChangeHint.h"
 #include "nsColor.h"
 #include "nsCOMPtr.h"
 #include "nsID.h"
+#include "nsIFrame.h"
 #include "nsISupportsBase.h"
 #include "nsMathUtils.h"
 #include "nsStyleStruct.h"
 #include "mozilla/Constants.h"
 #include <algorithm>
 
 class gfxContext;
 class nsFrameList;
@@ -174,22 +175,30 @@ NS_DEFINE_STATIC_IID_ACCESSOR(nsISVGFilt
  * General functions used by all of SVG layout and possibly content code.
  * If a method is used by content and depends only on other content methods
  * it should go in SVGContentUtils instead.
  */
 class nsSVGUtils
 {
 public:
   typedef mozilla::dom::Element Element;
+  typedef mozilla::FramePropertyDescriptor FramePropertyDescriptor;
   typedef mozilla::gfx::AntialiasMode AntialiasMode;
   typedef mozilla::gfx::FillRule FillRule;
   typedef mozilla::gfx::GeneralPattern GeneralPattern;
 
   static void Init();
 
+  static void DestroyObjectBoundingBoxProperty(void* aPropertyValue) {
+    delete static_cast<gfxRect*>(aPropertyValue);
+  }
+
+  NS_DECLARE_FRAME_PROPERTY(ObjectBoundingBoxProperty,
+                            DestroyObjectBoundingBoxProperty);
+
   /**
    * Gets the nearest nsSVGInnerSVGFrame or nsSVGOuterSVGFrame frame. aFrame
    * must be an SVG frame. If aFrame is of type nsGkAtoms::svgOuterSVGFrame,
    * returns nullptr.
    */
   static nsSVGDisplayContainerFrame* GetNearestSVGViewport(nsIFrame *aFrame);
 
   /**
@@ -389,16 +398,18 @@ public:
     eBBoxIncludeMarkers        = 1 << 4,
     eBBoxIncludeClipped        = 1 << 5
   };
   /**
    * Get the SVG bbox (the SVG spec's simplified idea of bounds) of aFrame in
    * aFrame's userspace.
    */
   static gfxRect GetBBox(nsIFrame *aFrame,
+                         // If the default arg changes, update the handling for
+                         // ObjectBoundingBoxProperty() in the implementation.
                          uint32_t aFlags = eBBoxIncludeFillGeometry);
 
   /*
    * "User space" is the space that the frame's BBox (as calculated by
    * nsSVGUtils::GetBBox) is in. "Frame space" is the space that has its origin
    * at the top left of the union of the frame's border-box rects over all
    * continuations.
    * This function returns the offset one needs to add to something in frame