From 1ac8e46d55643f663e439d2cb5d05a40fc68d011 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Lehni?= Date: Sun, 17 Jan 2016 23:57:56 +0100 Subject: [PATCH] Various fixes on handling of #strokeScaling and #strokeBounds calculations. --- src/item/Item.js | 47 ++++++++++++++++++++-------------------- src/item/PlacedSymbol.js | 5 +++-- src/item/Shape.js | 6 ++--- src/path/Path.js | 37 ++++++++++++++++--------------- 4 files changed, 50 insertions(+), 45 deletions(-) diff --git a/src/item/Item.js b/src/item/Item.js index a085fef6cb..f0d8501f99 100644 --- a/src/item/Item.js +++ b/src/item/Item.js @@ -781,12 +781,12 @@ var Item = Base.extend(Emitter, /** @lends Item# */{ // 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. + // 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(.*)$/), - internalGetter = match ? 'get' + match[1] : null; + internal = match ? 'get' + match[1] : null; this[getter] = function(_matrix) { // TODO: If we're getting stroke based bounds (strokeBounds, // roughBounds, internalRoughBounds), and the object does not have @@ -798,11 +798,10 @@ var Item = Base.extend(Emitter, /** @lends Item# */{ // Allow subclasses to override _boundsGetter if they use the // same calculations for multiple type of bounds. // The default is getter: - name = !internalGetter && (typeof boundsGetter === 'string' + name = !internal && (typeof boundsGetter === 'string' ? boundsGetter : boundsGetter && boundsGetter[getter]) || getter, - bounds = this._getCachedBounds(name, _matrix, this, - internalGetter); + bounds = this._getCachedBounds(name, _matrix, this, internal); // If we're returning 'bounds', create a LinkedRectangle that uses // the setBounds() setter to update the Item whenever the bounds are // changed: @@ -823,7 +822,7 @@ var Item = Base.extend(Emitter, /** @lends Item# */{ * Subclasses override it to define calculations for the various required * bounding types. */ - _getBounds: function(getter, matrix, cacheItem) { + _getBounds: function(getter, matrix, cacheItem, internal) { // 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 PlacedSymbol and Raster. @@ -844,7 +843,8 @@ var Item = Base.extend(Emitter, /** @lends Item# */{ var child = children[i]; if (child._visible && !child.isEmpty()) { var rect = child._getCachedBounds(getter, - matrix && matrix.appended(child._matrix), cacheItem); + matrix && matrix.appended(child._matrix), + cacheItem, internal); x1 = Math.min(rect.x, x1); y1 = Math.min(rect.y, y1); x2 = Math.max(rect.x + rect.width, x2); @@ -881,13 +881,13 @@ var Item = Base.extend(Emitter, /** @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, internalGetter) { + _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(); - // Do not transform by the internal matrix if there is a internalGetter. - var _matrix = internalGetter ? null : this._matrix._orNullIfIdentity(), + // Do not transform by the internal matrix if there is a internal getter + var _matrix = internal ? null : this._matrix._orNullIfIdentity(), cache = (!matrix || matrix.equals(_matrix)) && getter; // NOTE: This needs to happen before returning cached values, since even // then, _boundsCache needs to be kept up-to-date. @@ -899,8 +899,8 @@ var Item = Base.extend(Emitter, /** @lends Item# */{ // 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 || _matrix, cacheItem); + var bounds = this._getBounds(internal || getter, matrix || _matrix, + cacheItem, internal); // If we can cache the result, update the _bounds cache structure // before returning if (cache) { @@ -908,7 +908,7 @@ var Item = Base.extend(Emitter, /** @lends Item# */{ this._bounds = {}; var cached = this._bounds[cache] = bounds.clone(); // Mark as internal, so Item#transform() won't transform it! - cached._internal = !!internalGetter; + cached._internal = !!internal; } return bounds; }, @@ -918,9 +918,10 @@ var Item = Base.extend(Emitter, /** @lends Item# */{ * when calculating bounds: the item's matrix if {@link #strokeScaling} is * `true`, otherwise the shiftless, inverted view matrix. */ - _getStrokeMatrix: function(matrix) { + _getStrokeMatrix: function(matrix, internal) { return this.getStrokeScaling() ? matrix - : this._parent.getViewMatrix().invert()._shiftless(); + : (internal ? this : this._parent).getViewMatrix() + .invert()._shiftless(); }, statics: { @@ -1746,11 +1747,9 @@ var Item = Base.extend(Emitter, /** @lends Item# */{ // Calculate the transformed padding as 2D size that describes the // transformed tolerance circle / ellipse. Make sure it's never 0 // since we're using it for division. + tolerance = Math.max(options.tolerance, /*#=*/Numerical.TOLERANCE), tolerancePadding = options._tolerancePadding = new Size( - Path._getStrokePadding(1, strokeMatrix) - ).multiply( - Math.max(options.tolerance, /*#=*/Numerical.TOLERANCE) - ); + Path._getStrokePadding(tolerance, strokeMatrix)); // Transform point to local coordinates. point = matrix._inverseTransform(point); // If the matrix is non-reversible, point will now be `null`: @@ -1794,21 +1793,23 @@ var Item = Base.extend(Emitter, /** @lends Item# */{ } } + options._viewMatrix = viewMatrix; + options._strokeMatrix = strokeMatrix; var children = !res && this._children; if (children) { var opts = this._getChildHitTestOptions(options); - opts._viewMatrix = viewMatrix; // Loop backwards, so items that get drawn last are tested first for (var i = children.length - 1; i >= 0 && !res; i--) res = children[i]._hitTest(point, opts); - // Restore viewMatrix for next child, as opts === options sometimes. - opts._viewMatrix = parentViewMatrix; } if (!res && checkSelf) - res = this._hitTestSelf(point, options, strokeMatrix); + res = this._hitTestSelf(point, options); // Transform the point back to the outer coordinate system. if (res && res.point) res.point = matrix.transform(res.point); + // Restore viewMatrix for next child, so appended matrix chains are + // calculated correctly. + options._viewMatrix = parentViewMatrix; return res; }, diff --git a/src/item/PlacedSymbol.js b/src/item/PlacedSymbol.js index 942c136fba..e56cdd6466 100644 --- a/src/item/PlacedSymbol.js +++ b/src/item/PlacedSymbol.js @@ -108,11 +108,12 @@ var PlacedSymbol = Item.extend(/** @lends PlacedSymbol# */{ }, - _getBounds: function(getter, matrix, cacheItem) { + _getBounds: function(getter, matrix, cacheItem, internal) { var definition = this.symbol._definition; // Redirect the call to the symbol definition to calculate the bounds return definition._getCachedBounds(getter, - matrix && matrix.appended(definition._matrix), cacheItem); + matrix && matrix.appended(definition._matrix), + cacheItem, internal); }, _hitTestSelf: function(point, options) { diff --git a/src/item/Shape.js b/src/item/Shape.js index 6e2a5ea6f4..4fe8eb7b92 100644 --- a/src/item/Shape.js +++ b/src/item/Shape.js @@ -275,18 +275,18 @@ var Shape = Item.extend(/** @lends Shape# */{ return !(this.hasFill() && this.hasStroke()); }, - _getBounds: function(getter, matrix) { + _getBounds: function(getter, matrix, cacheItem, internal) { var rect = new Rectangle(this._size).setCenter(0, 0), style = this._style, strokeWidth = style.hasStroke() && - /^getStrokeBounds$|^get.*RoughBounds$/.test(getter) && + /^get(?:Stroke|Rough)Bounds$/.test(getter) && 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))) : rect; + strokeWidth, this._getStrokeMatrix(matrix, internal))) : rect; } }, new function() { // Scope for _contains() and _hitTestSelf() code. diff --git a/src/path/Path.js b/src/path/Path.js index 4beeddbe6a..9de6d5cfb1 100644 --- a/src/path/Path.js +++ b/src/path/Path.js @@ -1518,7 +1518,7 @@ var Path = PathItem.extend(/** @lends Path# */{ toPath: '#clone', - _hitTestSelf: function(point, options, strokeMatrix) { + _hitTestSelf: function(point, options) { var that = this, style = this.getStyle(), segments = this._segments, @@ -1533,23 +1533,23 @@ var Path = PathItem.extend(/** @lends Path# */{ hitStroke = options.stroke && style.hasStroke(), hitFill = options.fill && style.hasFill(), hitCurves = options.curves, - radius = hitStroke + strokeRadius = hitStroke ? style.getStrokeWidth() / 2 // Set radius to 0 when we're hit-testing fills with // tolerance, to handle tolerance through stroke hit-test // functionality. Also use 0 when hit-testing curves. : hitFill && options.tolerance > 0 || hitCurves ? 0 : null; - if (radius !== null) { - if (radius > 0) { + if (strokeRadius !== null) { + if (strokeRadius > 0) { join = style.getStrokeJoin(); cap = style.getStrokeCap(); - miterLimit = radius * style.getMiterLimit(); + miterLimit = strokeRadius * style.getMiterLimit(); // Add the stroke radius to tolerance padding, taking // #strokeScaling into account through _getStrokeMatrix(). strokePadding = tolerancePadding.add( - Path._getStrokePadding(radius, - !style.getStrokeScaling() && strokeMatrix)); + Path._getStrokePadding(strokeRadius, + !style.getStrokeScaling() && options._strokeMatrix)); } else { join = cap = 'round'; } @@ -1606,11 +1606,12 @@ var Path = PathItem.extend(/** @lends Path# */{ if (join !== 'round' && (segment._handleIn.isZero() || segment._handleOut.isZero())) // _addBevelJoin() handles both 'bevel' and 'miter'! - Path._addBevelJoin(segment, join, radius, miterLimit, - addToArea, true); + Path._addBevelJoin(segment, join, strokeRadius, + miterLimit, addToArea, true); } else if (cap !== 'round') { // It's a cap - Path._addSquareCap(segment, cap, radius, addToArea, true); + Path._addSquareCap(segment, cap, strokeRadius, addToArea, + true); } // See if the above produced an area to check for if (!area.isEmpty()) { @@ -1639,7 +1640,7 @@ var Path = PathItem.extend(/** @lends Path# */{ return res; } // If we're querying for stroke, perform that before fill - if (radius !== null) { + if (strokeRadius !== null) { loc = this.getNearestLocation(point); // Note that paths need at least two segments to have an actual // stroke. But we still check for segments with the radius fallback @@ -2712,8 +2713,9 @@ 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(getter, matrix, cacheItem, internal) { + return Path[getter](this._segments, this._closed, this, matrix, + internal); }, // Mess with indentation in order to get more line-space below: @@ -2764,14 +2766,14 @@ statics: { * * @private */ - getStrokeBounds: function(segments, closed, path, matrix) { + getStrokeBounds: function(segments, closed, path, matrix, internal) { var style = path._style; if (!style.hasStroke()) return Path.getBounds(segments, closed, path, matrix); var length = segments.length - (closed ? 0 : 1), strokeWidth = style.getStrokeWidth(), strokeRadius = strokeWidth / 2, - strokeMatrix = path._getStrokeMatrix(matrix), + strokeMatrix = path._getStrokeMatrix(matrix, internal), 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. @@ -2971,14 +2973,15 @@ statics: { * * @private */ - getRoughBounds: function(segments, closed, path, matrix) { + getRoughBounds: function(segments, closed, path, matrix, internal) { // Delegate to handleBounds, but pass on radius values for stroke and // joins. Handle miter joins specially, by passing the largest radius // possible. var style = path._style, strokeRadius = style.hasStroke() ? style.getStrokeWidth() / 2 : 0, joinRadius = strokeRadius, - strokeMatrix = strokeRadius && path._getStrokeMatrix(matrix); + strokeMatrix = strokeRadius && + path._getStrokeMatrix(matrix, internal); if (strokeRadius > 0) { if (style.getStrokeJoin() === 'miter') joinRadius = strokeRadius * style.getMiterLimit();