diff --git a/src/core/Base.js b/src/core/Base.js index dbd7b10b..7952ba51 100644 --- a/src/core/Base.js +++ b/src/core/Base.js @@ -92,7 +92,7 @@ Base.inject(/** @lends Base# */{ this[key] = value; } } - return true; + return props; } }, diff --git a/src/item/Group.js b/src/item/Group.js index 9281bd34..29df2e14 100644 --- a/src/item/Group.js +++ b/src/item/Group.js @@ -167,21 +167,13 @@ var Group = Item.extend(/** @lends Group# */{ child.setClipMask(clipped); }, - _getBounds: function _getBounds(getter, matrix, cacheItem, internal) { - var clipItem = this._getClipItem(), - // We need to fall-back to bounds getter that do not take stroke - // into account - clipBoundsGetter = { - getStrokeBounds: 'getBounds', - getRoughBounds: 'getHandleBounds', - getInternalRoughBounds: 'getInternalBounds' - }; + _getBounds: function _getBounds(matrix, options) { + var clipItem = this._getClipItem(); return clipItem - ? clipItem._getCachedBounds(clipBoundsGetter[getter] || getter, - matrix && matrix.appended(clipItem._matrix), - cacheItem, internal) - : _getBounds.base.call(this, getter, matrix, cacheItem, - internal); + ? clipItem._getCachedBounds( + matrix && matrix.appended(clipItem._matrix), + Base.set({}, options, { stroke: false })) + : _getBounds.base.call(this, matrix, options); }, _hitTestChildren: function _hitTestChildren(point, options) { diff --git a/src/item/Item.js b/src/item/Item.js index 1c92b8b6..e6909ff3 100644 --- a/src/item/Item.js +++ b/src/item/Item.js @@ -780,55 +780,14 @@ new function() { // // Scope to inject various item event handlers }, _pivot: null, -}, Base.each(['bounds', 'strokeBounds', 'handleBounds', 'roughBounds', - 'internalBounds', 'internalHandleBounds', 'internalRoughBounds'], - 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. - // 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 `internal`. - // NOTE: These need to be versions of other methods, as otherwise the - // cache gets messed up. - var getter = 'get' + Base.capitalize(key), - match = key.match(/^internal(.*)$/), - internal = match ? 'get' + match[1] : null, - // Determine if the stroke is involved in the calculation of the - // bounds: strokeBounds, roughBounds,internalRoughBounds - stroke = /^stroke|rough/i.test(key); - this[getter] = function(_matrix) { - // TODO: If we're getting stroke based bounds (strokeBounds, - // roughBounds, internalRoughBounds), and the object does not have - // a stroke, fall back to the bounds getter without the stroke: - // strokeBounds -> bounds - // roughBounds -> handleBounds - // internalRoughBounds -> internalHandleBounds - var boundsGetter = this._boundsGetter, - // Allow subclasses to override _boundsGetter if they use the - // same calculations for multiple type of bounds. - // The default is getter: - name = !internal && (typeof boundsGetter === 'string' - ? boundsGetter : boundsGetter && boundsGetter[getter]) - || getter, - // We can only cache the bounds if the path uses stroke-scaling, - // or if no stroke is involved in the calculation of the bounds. - // When strokeScaling is false, the bounds are affected by the - // zoom level of the view, hence we can't cache. - canCache = !stroke || this.getStrokeScaling(), - // If we're caching bounds, pass on this item as cacheItem, so - // the children can setup _boundsCache structures for it. - bounds = this._getCachedBounds(name, _matrix, canCache && this, - internal); - // If we're returning 'bounds', create a LinkedRectangle that uses - // the setBounds() setter to update the Item whenever the bounds are - // changed: - return key === 'bounds' - ? new LinkedRectangle(bounds.x, bounds.y, bounds.width, - bounds.height, this, 'setBounds') - : bounds; +}, Base.each({ // Produce getters for bounds properties: + getStrokeBounds: { stroke: true }, + getHandleBounds: { handle: true }, + getInternalBounds: { internal: true } + }, + function(options, key) { + this[key] = function(matrix) { + return this.getBounds(matrix, options); }; }, /** @lends Item# */{ @@ -836,13 +795,41 @@ new function() { // // Scope to inject various item event handlers // See _matrix parameter above. beans: true, + getBounds: function(matrix, options) { + var hasMatrix = options || matrix instanceof Matrix, + opts = Base.set({}, hasMatrix ? options : matrix, + this._boundsOptions); + // We can only cache the bounds if the path uses stroke-scaling, or if + // no stroke is involved in the calculation of the bounds. + // When strokeScaling is false, the bounds are affected by the zoom + // level of the view, hence we can't cache. + // TODO: Look more into handling of stroke-scaling, e.g. on groups with + // some children that have strokeScaling, as well as SymbolItem with + // SymbolDefinition that have strokeScaling! + // TODO: Once that is resolved, we should be able to turn off + // opts.stroke if a resolved item definition does not have a stroke, + // allowing the code to share caches between #strokeBounds and #bounds. + if (!opts.stroke || this.getStrokeScaling()) + opts.cacheItem = this; + // If we're caching bounds, pass on this item as cacheItem, so + // the children can setup _boundsCache structures for it. + var bounds = this._getCachedBounds(hasMatrix && matrix, opts); + // If we're returning '#bounds', create a LinkedRectangle that uses + // the setBounds() setter to update the Item whenever the bounds are + // changed: + return arguments.length === 0 + ? new LinkedRectangle(bounds.x, bounds.y, bounds.width, + bounds.height, this, 'setBounds') + : bounds; + }, + /** * Protected method used in all the bounds getters. It loops through all the * children, gets their bounds and finds the bounds around all of them. * Subclasses override it to define calculations for the various required * bounding types. */ - _getBounds: function(getter, matrix, cacheItem, internal) { + _getBounds: function(matrix, options) { // NOTE: We cannot cache these results here, since we do not get // _changed() notifications here for changing geometry in children. // But cacheName is used in sub-classes such as SymbolItem and Raster. @@ -854,7 +841,7 @@ new function() { // // Scope to inject various item event handlers // Call _updateBoundsCache() even when the group only holds empty / // invisible items), so future changes in these items will cause right // handling of _boundsCache. - Item._updateBoundsCache(this, cacheItem); + Item._updateBoundsCache(this, options.cacheItem); var x1 = Infinity, x2 = -x1, y1 = x1, @@ -862,9 +849,8 @@ new function() { // // Scope to inject various item event handlers for (var i = 0, l = children.length; i < l; i++) { var child = children[i]; if (child._visible && !child.isEmpty()) { - var rect = child._getCachedBounds(getter, - matrix && matrix.appended(child._matrix), - cacheItem, internal); + var rect = child._getCachedBounds( + matrix && matrix.appended(child._matrix), options); x1 = Math.min(rect.x, x1); y1 = Math.min(rect.y, y1); x2 = Math.max(rect.x + rect.width, x2); @@ -901,29 +887,38 @@ new function() { // // Scope to inject various item event handlers * 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(matrix, options) { // 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(); - // Do not transform by the internal matrix if there is a internal getter - var _matrix = internal ? null : this._matrix._orNullIfIdentity(), - cache = cacheItem && (!matrix || matrix.equals(_matrix)) && getter; + // Do not transform by the internal matrix for internal, untransformed + // bounds. + var internal = options.internal, + cacheItem = options.cacheItem, + _matrix = internal ? null : this._matrix._orNullIfIdentity(), + // Create a key for caching, reflecting all bounds options. + cacheKey = cacheItem && (!matrix || matrix.equals(_matrix)) && [ + options.stroke ? 1 : 0, + options.handle ? 1 : 0, + internal ? 1 : 0 + ].join(''); // NOTE: This needs to happen before returning cached values, since even // then, _boundsCache needs to be kept up-to-date. Item._updateBoundsCache(this._parent || this._parentSymbol, cacheItem); - if (cache && this._bounds && this._bounds[cache]) - return this._bounds[cache].clone(); - var bounds = this._getBounds(internal || getter, matrix || _matrix, - cacheItem, internal); + if (cacheKey && this._bounds && cacheKey in this._bounds) + return this._bounds[cacheKey].rect.clone(); + var bounds = this._getBounds(matrix || _matrix, options); // If we can cache the result, update the _bounds cache structure // before returning - if (cache) { + if (cacheKey) { if (!this._bounds) this._bounds = {}; - var cached = this._bounds[cache] = bounds.clone(); - // Mark as internal, so Item#transform() won't transform it! - cached._internal = !!internal; + var cached = this._bounds[cacheKey] = { + rect: bounds.clone(), + // Mark as internal, so Item#transform() won't transform it + internal: options.internal + }; } return bounds; }, @@ -933,10 +928,9 @@ new function() { // // Scope to inject various item event handlers * when calculating bounds: the item's matrix if {@link #strokeScaling} is * `true`, otherwise the shiftless, inverted view matrix. */ - _getStrokeMatrix: function(matrix, internal) { - return this.getStrokeScaling() ? matrix - : (internal ? this : this._parent).getViewMatrix() - .invert()._shiftless(); + _getStrokeMatrix: function(matrix, options) { + return this.getStrokeScaling() ? matrix : (options && options.internal + ? this : this._parent).getViewMatrix().invert()._shiftless(); }, statics: { @@ -951,7 +945,7 @@ new function() { // // Scope to inject various item event handlers * times the same structure. */ _updateBoundsCache: function(parent, item) { - if (parent) { + if (parent && item) { // Set-up the parent's boundsCache structure if it does not // exist yet and add the item to it. var id = item._id, @@ -1780,8 +1774,9 @@ new function() { // // Scope to inject various item event handlers // Transform point to local coordinates. point = matrix._inverseTransform(point); // If the matrix is non-reversible, point will now be `null`: - if (!point || !this._children && !this.getInternalRoughBounds() - .expand(tolerancePadding.multiply(2))._containsPoint(point)) + if (!point || !this._children && + !this.getBounds({ internal: true, stroke: true, handle: true }) + .expand(tolerancePadding.multiply(2))._containsPoint(point)) return null; // Filter for type, guides and selected items if that's required. var checkSelf = !(options.guides && !this._guide @@ -2036,7 +2031,7 @@ new function() { // // Scope to inject various item event handlers if (obj) { // Create a copy of the match object that doesn't contain // these special properties: - match = Base.set({}, match, { + match = new Base()._set(match, { recursive: true, inside: true, overlapping: true }); } @@ -3248,11 +3243,13 @@ new function() { // // Scope to inject various item event handlers // Transform the old bound by looping through all the cached bounds // in _bounds and transform each. for (var key in bounds) { - var rect = bounds[key]; + var cache = bounds[key]; // If these are internal bounds, only transform them if this // item applied its matrix. - if (applyMatrix || !rect._internal) + if (applyMatrix || !cache.internal) { + var rect = cache.rect; 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 @@ -4147,7 +4144,8 @@ new function() { // // Scope to inject various item event handlers this._drawSelected(ctx, mx, selectedItems); if (this._boundsSelected) { var half = size / 2, - coords = mx._transformCorners(this.getInternalBounds()); + coords = mx._transformCorners( + this.getInternalBounds()); // Now draw a rectangle that connects the transformed // bounds corners, and draw the corners. ctx.beginPath(); diff --git a/src/item/Raster.js b/src/item/Raster.js index 55893ec2..bac67059 100644 --- a/src/item/Raster.js +++ b/src/item/Raster.js @@ -23,7 +23,7 @@ var Raster = Item.extend(/** @lends Raster# */{ _canApplyMatrix: false, // Raster doesn't make the distinction between the different bounds, // so use the same name for all of them - _boundsGetter: 'getBounds', + _boundsOptions: { stroke: false, handle: false }, _boundsSelected: true, _serializeFields: { crossOrigin: null, // NOTE: Needs to be set before source to work! @@ -690,7 +690,7 @@ var Raster = Item.extend(/** @lends Raster# */{ * @type Function */ - _getBounds: function(getter, matrix) { + _getBounds: function(matrix, options) { var rect = new Rectangle(this._size).setCenter(0, 0); return matrix ? matrix._transformBounds(rect) : rect; }, diff --git a/src/item/Shape.js b/src/item/Shape.js index e0c824ac..7eb772fd 100644 --- a/src/item/Shape.js +++ b/src/item/Shape.js @@ -275,18 +275,19 @@ var Shape = Item.extend(/** @lends Shape# */{ return !(this.hasFill() && this.hasStroke()); }, - _getBounds: function(getter, matrix, cacheItem, internal) { + _getBounds: function(matrix, options) { var rect = new Rectangle(this._size).setCenter(0, 0), style = this._style, - strokeWidth = style.hasStroke() && - /^get(?:Stroke|Rough)Bounds$/.test(getter) && - style.getStrokeWidth(); + strokeWidth = options.stroke && style.hasStroke() + && style.getStrokeWidth(); // If we're getting the strokeBounds, include the stroke width before // or after transforming the rect, based on strokeScaling. if (matrix) rect = matrix._transformBounds(rect); - return strokeWidth ? rect.expand(Path._getStrokePadding( - strokeWidth, this._getStrokeMatrix(matrix, internal))) : rect; + return strokeWidth + ? rect.expand(Path._getStrokePadding(strokeWidth, + this._getStrokeMatrix(matrix, options))) + : rect; } }, new function() { // Scope for _contains() and _hitTestSelf() code. diff --git a/src/item/SymbolItem.js b/src/item/SymbolItem.js index c861562b..cd7f6df2 100644 --- a/src/item/SymbolItem.js +++ b/src/item/SymbolItem.js @@ -23,7 +23,7 @@ var SymbolItem = Item.extend(/** @lends SymbolItem# */{ _applyMatrix: false, _canApplyMatrix: false, // SymbolItem uses strokeBounds for bounds - _boundsGetter: { getBounds: 'getStrokeBounds' }, + _boundsOptions: { stroke: true }, _boundsSelected: true, _serializeFields: { symbol: null @@ -114,12 +114,11 @@ var SymbolItem = Item.extend(/** @lends SymbolItem# */{ }, - _getBounds: function(getter, matrix, cacheItem, internal) { + _getBounds: function(matrix, options) { var item = this._definition._item; // Redirect the call to the definition item to calculate the bounds. - return item._getCachedBounds(getter, - matrix && matrix.appended(item._matrix), - cacheItem, internal); + return item._getCachedBounds(matrix && matrix.appended(item._matrix), + options); }, _hitTestSelf: function(point, options) { diff --git a/src/path/Path.js b/src/path/Path.js index 580841e3..258e1239 100644 --- a/src/path/Path.js +++ b/src/path/Path.js @@ -2512,8 +2512,15 @@ new function() { // PostScript-style drawing commands // Curve. But not all of them use all these parameters, and some define // additional ones after. - _getBounds: function(getter, matrix) { - return Path[getter](this._segments, this._closed, this, matrix); + _getBounds: function(matrix, options) { + var method = options.handle + ? options.stroke + ? 'getRoughBounds' + : 'getHandleBounds' + : options.stroke + ? 'getStrokeBounds' + : 'getBounds'; + return Path[method](this._segments, this._closed, this, matrix, options); }, // Mess with indentation in order to get more line-space below: @@ -2523,7 +2530,7 @@ statics: { * * @private */ - getBounds: function(segments, closed, path, matrix, strokePadding) { + getBounds: function(segments, closed, path, matrix, options, strokePadding) { var first = segments[0]; // If there are no segments, return "empty" rectangle, just like groups, // since #bounds is assumed to never return null. @@ -2564,18 +2571,18 @@ statics: { * * @private */ - getStrokeBounds: function(segments, closed, path, matrix, internal) { + getStrokeBounds: function(segments, closed, path, matrix, options) { var style = path._style; if (!style.hasStroke()) - return Path.getBounds(segments, closed, path, matrix); + return Path.getBounds(segments, closed, path, matrix, options); var length = segments.length - (closed ? 0 : 1), strokeWidth = style.getStrokeWidth(), strokeRadius = strokeWidth / 2, - strokeMatrix = path._getStrokeMatrix(matrix, internal), + strokeMatrix = path._getStrokeMatrix(matrix, options), strokePadding = Path._getStrokePadding(strokeWidth, strokeMatrix), // Start with normal path bounds with added stroke padding. Then we // only need to look at each segment and handle join / cap / miter. - bounds = Path.getBounds(segments, closed, path, matrix, + bounds = Path.getBounds(segments, closed, path, matrix, options, strokePadding), join = style.getStrokeJoin(), cap = style.getStrokeCap(), @@ -2736,8 +2743,8 @@ statics: { * * @private */ - getHandleBounds: function(segments, closed, path, matrix, strokePadding, - joinPadding) { + getHandleBounds: function(segments, closed, path, matrix, options, + strokePadding, joinPadding) { var coords = new Array(6), x1 = Infinity, x2 = -x1, @@ -2772,7 +2779,7 @@ statics: { * * @private */ - getRoughBounds: function(segments, closed, path, matrix, internal) { + getRoughBounds: function(segments, closed, path, matrix, options) { // Delegate to handleBounds, but pass on radius values for stroke and // joins. Handle miter joins specially, by passing the largest radius // possible. @@ -2780,14 +2787,14 @@ statics: { strokeRadius = style.hasStroke() ? style.getStrokeWidth() / 2 : 0, joinRadius = strokeRadius, strokeMatrix = strokeRadius && - path._getStrokeMatrix(matrix, internal); + path._getStrokeMatrix(matrix, options); if (strokeRadius > 0) { if (style.getStrokeJoin() === 'miter') joinRadius = strokeRadius * style.getMiterLimit(); if (style.getStrokeCap() === 'square') joinRadius = Math.max(joinRadius, strokeRadius * Math.sqrt(2)); } - return Path.getHandleBounds(segments, closed, path, matrix, + return Path.getHandleBounds(segments, closed, path, matrix, options, Path._getStrokePadding(strokeRadius, strokeMatrix), Path._getStrokePadding(joinRadius, strokeMatrix)); } diff --git a/src/path/PathItem.js b/src/path/PathItem.js index e4ace31d..509e6278 100644 --- a/src/path/PathItem.js +++ b/src/path/PathItem.js @@ -176,8 +176,9 @@ var PathItem = Item.extend(/** @lends PathItem# */{ // Check the transformed point against the untransformed (internal) // handle bounds, which is the fastest rough bounding box to calculate // for a quick check before calculating the actual winding. - var winding = point.isInside(this.getInternalHandleBounds()) - && this._getWinding(point); + var winding = point.isInside( + this.getBounds({ internal: true, handle: true })) + && this._getWinding(point); return !!(this.getFillRule() === 'evenodd' ? winding & 1 : winding); /*#*/ } // !__options.nativeContains && __options.booleanOperations }, diff --git a/src/text/PointText.js b/src/text/PointText.js index f9a2a901..9a788c7a 100644 --- a/src/text/PointText.js +++ b/src/text/PointText.js @@ -102,7 +102,7 @@ var PointText = TextItem.extend(/** @lends PointText# */{ } }, - _getBounds: function(getter, matrix) { + _getBounds: function(matrix, options) { var style = this._style, lines = this._lines, numLines = lines.length, diff --git a/src/text/TextItem.js b/src/text/TextItem.js index 4d5644d2..c9fc505f 100644 --- a/src/text/TextItem.js +++ b/src/text/TextItem.js @@ -31,7 +31,7 @@ var TextItem = Item.extend(/** @lends TextItem# */{ }, // TextItem doesn't make the distinction between the different bounds, // so use the same name for all of them - _boundsGetter: 'getBounds', + _boundsOptions: { stroke: false, handle: false }, initialize: function TextItem(arg) { this._content = ''; diff --git a/test/helpers.js b/test/helpers.js index 4b837637..9de84e89 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -11,7 +11,7 @@ */ var isNode = typeof global === 'object', - isPhantom = !!window.callPhantom, + isPhantom = typeof window === 'object' && !!window.callPhantom, root; if (isNode) { diff --git a/test/tests/Item_Bounds.js b/test/tests/Item_Bounds.js index 7094e978..5a46017b 100644 --- a/test/tests/Item_Bounds.js +++ b/test/tests/Item_Bounds.js @@ -585,25 +585,69 @@ test('path.strokeBounds without strokeScaling and zoomed view', function() { center: [0, 0], radius: 100, strokeColor: 'black', - strokeWidth: 15, + strokeWidth: 20, + strokeScaling: false, + applyMatrix: false + }); + + view.zoom = 2; + + equals(path.strokeBounds, new Rectangle(-105, -105, 210, 210), + 'path.strokeBounds with zoomed view'); + + view.zoom = 1; + + equals(path.strokeBounds, new Rectangle(-110, -110, 220, 220), + 'path.strokeBounds without zoomed view'); + + path.scale(0.5, 1); + + view.zoom = 2; + + // Internal stroke bounds need to apply stroke deformation with + // strokeScaling: + equals(path.getBounds({ internal: true, stroke: true }), + new Rectangle(-110, -105, 220, 210), + 'path.getBounds({ internal: true, stroke: true })' + + ' with path.applyMatrix = false, path.scale(0.5, 1);'); + + path.applyMatrix = true; + + equals(path.getBounds({ internal: true, stroke: true }), + new Rectangle(-55, -105, 110, 210), + 'path.getBounds({ internal: true, stroke: true })' + + ' with path.applyMatrix = true, path.scale(0.5, 1);'); +}); + +test('shape.strokeBounds without strokeScaling and zoomed view', function() { + var shape = new Shape.Circle({ + center: [0, 0], + radius: 100, + strokeColor: 'black', + strokeWidth: 20, strokeScaling: false }); view.zoom = 2; - new Path.Rectangle({ - rectangle: path.strokeBounds, - strokeColor: 'red', - strokeScaling: false - }); - - equals(path.strokeBounds, new Rectangle(-103.75, -103.75, 207.5, 207.5), - 'path.strokeBounds with zoomed view'); + equals(shape.strokeBounds, new Rectangle(-105, -105, 210, 210), + 'shape.strokeBounds with zoomed view'); view.zoom = 1; - equals(path.strokeBounds, new Rectangle(-107.5, -107.5, 215, 215), - 'path.strokeBounds without zoomed view'); + equals(shape.strokeBounds, new Rectangle(-110, -110, 220, 220), + 'shape.strokeBounds without zoomed view'); + + shape.scale(0.5, 1); + + view.zoom = 2; + + // Internal stroke bounds need to apply stroke deformation with + // strokeScaling: + equals(shape.getBounds({ internal: true, stroke: true }), + new Rectangle(-110, -105, 220, 210), + 'shape.getBounds({ internal: true, stroke: true })' + + ' with shape.scale(0.5, 1);'); }); test('path.internalBounds', function() {