From e238d23194bb699b8cb9db96fcb4a5979d7a210e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Lehni?= Date: Mon, 9 Dec 2013 10:53:19 +0100 Subject: [PATCH] Implement caching of internal, untransformed bounds. --- src/item/Item.js | 41 +++++++++++++++++++++++------------------ src/path/Path.js | 2 ++ src/project/Project.js | 5 +---- test/tests/TextItem.js | 4 ++-- 4 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/item/Item.js b/src/item/Item.js index fc0088d3..9b30f73b 100644 --- a/src/item/Item.js +++ b/src/item/Item.js @@ -798,22 +798,26 @@ var Item = Base.extend(Callback, /** @lends Item# */{ // No need for _changed() since the only thing this affects is _position delete this._position; } -}, Base.each(['getBounds', 'getStrokeBounds', 'getHandleBounds', 'getRoughBounds'], - function(name) { +}, Base.each(['getBounds', 'getStrokeBounds', 'getHandleBounds', + 'getRoughBounds', 'getInternalBounds'], + function(key) { // Produce getters for bounds properties. These handle caching, matrices // and redirect the call to the private _getBounds, which can be // overridden by subclasses, see below. - this[name] = function(/* matrix */) { + var internal = key === 'getInternalBounds'; + this[key] = function(/* matrix */) { var getter = this._boundsGetter, // Allow subclasses to override _boundsGetter if they use // the same calculations for multiple type of bounds. - // The default is name: - bounds = this._getCachedBounds(typeof getter == 'string' - ? getter : getter && getter[name] || name, arguments[0]); + // The default is key: + bounds = this._getCachedBounds(!internal + && (typeof getter === 'string' + ? getter : getter && getter[key]) + || key, arguments[0], null, internal); // If we're returning 'bounds', create a LinkedRectangle that uses // the setBounds() setter to update the Item whenever the bounds are // changed: - return name === 'getBounds' + return key === 'getBounds' ? new LinkedRectangle(bounds.x, bounds.y, bounds.width, bounds.height, this, 'setBounds') : bounds; @@ -879,12 +883,12 @@ var Item = Base.extend(Callback, /** @lends Item# */{ * Private method that deals with the calling of _getBounds, recursive * matrix concatenation and handles all the complicated caching mechanisms. */ - _getCachedBounds: function(getter, matrix, cacheItem) { + _getCachedBounds: function(getter, matrix, cacheItem, internal) { // See if we can cache these bounds. We only cache the bounds // transformed with the internally stored _matrix, (the default if no // matrix is passed). matrix = matrix && matrix.orNullIfIdentity(); - var _matrix = this._matrix.orNullIfIdentity(), + var _matrix = internal ? null : this._matrix.orNullIfIdentity(), cache = (!matrix || matrix.equals(_matrix)) && getter; // Set up a boundsCache structure that keeps track of items that keep // cached bounds that depend on this item. We store this in our parent, @@ -925,13 +929,16 @@ var Item = Base.extend(Callback, /** @lends Item# */{ : matrix; // If we're caching bounds on this item, pass it on as cacheItem, so the // children can setup the _boundsCache structures for it. - var bounds = this._getBounds(getter, matrix, cache ? this : cacheItem); + var bounds = this._getBounds(getter === 'getInternalBounds' + ? 'getBounds' : getter, matrix, cache ? this : cacheItem); // If we can cache the result, update the _bounds cache structure // before returning if (cache) { if (!this._bounds) this._bounds = {}; - this._bounds[cache] = bounds.clone(); + var cached = this._bounds[cache] = bounds.clone(); + // Mark as internal, so Item#transform() won't transform it! + cached._internal = internal; } return bounds; }, @@ -1542,11 +1549,7 @@ var Item = Base.extend(Callback, /** @lends Item# */{ } // We only implement it here for items with rectangular content, // for anything else we need to override #contains() - // TODO: There currently is no caching for the results of direct calls - // to this._getBounds('getBounds') (without the application of the - // internal matrix). Performance improvements could be achieved if - // these were cached too. See #_getCachedBounds(). - return point.isInside(this._getBounds('getBounds')); + return point.isInside(this.getInternalBounds()); }, /** @@ -1622,7 +1625,7 @@ var Item = Base.extend(Callback, /** @lends Item# */{ if (checkSelf && (options.center || options.bounds) && this._parent) { // Don't get the transformed bounds, check against transformed // points instead - var bounds = this._getBounds('getBounds'); + var bounds = this.getInternalBounds(); if (options.center) res = checkBounds('center', 'Center'); if (!res && options.bounds) { @@ -2677,7 +2680,9 @@ var Item = Base.extend(Callback, /** @lends Item# */{ // in _bounds and transform each. for (var key in bounds) { var rect = bounds[key]; - matrix._transformBounds(rect, rect); + // See _getCachedBounds for an explanation of this: + if (!rect._internal) + matrix._transformBounds(rect, rect); } // If we have cached bounds, update _position again as its // center. We need to take into account _boundsGetter here too, in diff --git a/src/path/Path.js b/src/path/Path.js index bead0c3b..5a3c48cb 100644 --- a/src/path/Path.js +++ b/src/path/Path.js @@ -1716,6 +1716,8 @@ var Path = PathItem.extend(/** @lends Path# */{ if (!closed && !this.hasFill() // We need to call the internal _getBounds, to get non- // transformed bounds. + // TODO: Implement caching for internal rough bounds and switch + // hit-testing code to using this too. || !this._getBounds('getRoughBounds')._containsPoint(point)) return 0; // Use the crossing number algorithm, by counting the crossings of the diff --git a/src/project/Project.js b/src/project/Project.js index b455d433..b533b98c 100644 --- a/src/project/Project.js +++ b/src/project/Project.js @@ -462,11 +462,8 @@ var Project = PaperScopeItem.extend(/** @lends Project# */{ if (item._drawSelected) item._drawSelected(ctx, mx); if (item._boundsSelected) { - // We need to call the internal _getBounds, to get non- - // transformed bounds. - // TODO: Implement caching for these too? var coords = mx._transformCorners( - item._getBounds('getBounds')); + item.getInternalBounds()); // Now draw a rectangle that connects the transformed // bounds corners, and draw the corners. ctx.beginPath(); diff --git a/test/tests/TextItem.js b/test/tests/TextItem.js index cd6c700e..94daeb4a 100644 --- a/test/tests/TextItem.js +++ b/test/tests/TextItem.js @@ -20,8 +20,8 @@ test('PointText', function() { content: 'Hello World!' }); equals(text.fillColor, { red: 0, green: 0, blue: 0 }, 'text.fillColor should be black by default'); - equals(text.point, { x: 100, y: 100 }); - equals(text.bounds, { x: 100, y: 87.4, width: 55, height: 16.8 }); + comparePoints(text.point, { x: 100, y: 100 }); + compareRectangles(text.bounds, { x: 100, y: 87.4, width: 77, height: 16.8 }); equals(function() { return text.hitTest(text.bounds.center) != null; }, true);