diff --git a/src/item/Item.js b/src/item/Item.js index c56e3b50..58c1d726 100644 --- a/src/item/Item.js +++ b/src/item/Item.js @@ -801,21 +801,29 @@ var Item = Base.extend(Callback, /** @lends Item# */{ delete this._position; } }, Base.each(['getBounds', 'getStrokeBounds', 'getHandleBounds', - 'getRoughBounds', 'getInternalBounds'], + 'getRoughBounds', 'getInternalBounds', 'getInternalRoughBounds'], 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. - var internal = key === 'getInternalBounds'; + // Treat internalBounds and internalRoughBounds untransformed, as + // required by the code that uses these methods internally, but make + // sure they can be cached like all the others as well. + // Pass on the getter that these version actually use, untransformed, + // as internalGetter. + // NOTE: These need to be versions of other methods, as otherwise the + // cache gets messed up. + var match = key.match(/^getInternal(.*)$/), + internalGetter = match ? 'get' + match[1] : null; 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 key: - bounds = this._getCachedBounds(!internal + bounds = this._getCachedBounds(!internalGetter && (typeof getter === 'string' ? getter : getter && getter[key]) - || key, arguments[0], null, internal); + || key, arguments[0], null, internalGetter); // If we're returning 'bounds', create a LinkedRectangle that uses // the setBounds() setter to update the Item whenever the bounds are // changed: @@ -885,12 +893,13 @@ 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, internal) { + _getCachedBounds: function(getter, matrix, cacheItem, internalGetter) { // 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 = internal ? null : this._matrix.orNullIfIdentity(), + // Do not transform by the internal matrix if there is a internalGetter. + var _matrix = internalGetter ? 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, @@ -907,13 +916,12 @@ var Item = Base.extend(Callback, /** @lends Item# */{ // Set-up the parent's boundsCache structure if it does not // exist yet and add the cacheItem to it. var id = cacheItem._id, - ref = cacheParent._boundsCache - = cacheParent._boundsCache || { - // Use both a hashtable for ids and an array for the list, - // so we can keep track of items that were added already - ids: {}, - list: [] - }; + ref = cacheParent._boundsCache = cacheParent._boundsCache || { + // Use both a hashtable for ids and an array for the list, + // so we can keep track of items that were added already + ids: {}, + list: [] + }; if (!ref.ids[id]) { ref.list.push(cacheItem); ref.ids[id] = cacheItem; @@ -931,8 +939,11 @@ 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 === 'getInternalBounds' - ? 'getBounds' : getter, matrix, cache ? this : cacheItem); + // getInternalBounds is getBounds untransformed. Do not replace earlier, + // so we can cache both separately, since they're not in the same + // transformation space! + var bounds = this._getBounds(internalGetter || getter, matrix, + cache ? this : cacheItem); // If we can cache the result, update the _bounds cache structure // before returning if (cache) { @@ -940,7 +951,7 @@ var Item = Base.extend(Callback, /** @lends Item# */{ this._bounds = {}; var cached = this._bounds[cache] = bounds.clone(); // Mark as internal, so Item#transform() won't transform it! - cached._internal = internal; + cached._internal = !!internalGetter; } return bounds; }, diff --git a/src/path/Path.js b/src/path/Path.js index e655f82a..fbefd638 100644 --- a/src/path/Path.js +++ b/src/path/Path.js @@ -1713,11 +1713,7 @@ var Path = PathItem.extend(/** @lends Path# */{ // If the path is not closed, we should not bail out in case it has a // fill color! 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)) + || !this.getInternalRoughBounds()._containsPoint(point)) return 0; // Use the crossing number algorithm, by counting the crossings of the // beam in right y-direction with the shape, and see if it's an odd