diff --git a/CHANGELOG.md b/CHANGELOG.md index aa0c1dfc..8af11c09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -169,6 +169,10 @@ contribute to the code. - Allow `Item#position` to be selected via `Item#position.selected` (#980). ### Fixed +- Fix calculations of `Item#strokeBounds` for all possible combinations of + `Item#strokeScaling` and `Item#applyMatrix` for `Path`, `Shape` and + `SymbolItem`, along with correct handling of such strokes in Item#hitTest() + (#697, #856, #1014). - Make new code-base unified for Node.js/browser work with module bundlers like Webpack (#986). - Improve hit-testing and `#contains()` checks on path with horizontal lines @@ -184,9 +188,6 @@ contribute to the code. - Do not rasterize items if the resulting raster will be empty (#828). - Fix SVG serialization in JSDOM `7.0.0` and newer (#821). - Correctly handle gradients in SVG import on Firefox (#666). -- Fix `Shape#strokeBounds` when `#strokeScaling` is false (#856). -- Correctly handle `#strokeScaling` when calculating `Path` and `Shape` bounds - (#697). - Consistently interpret curves as straight or not-straight (#838). - Switch blendMode to 'lighter' in Candy Crash example for better performance (#453). diff --git a/src/item/Item.js b/src/item/Item.js index d790f64a..1a3b0f70 100644 --- a/src/item/Item.js +++ b/src/item/Item.js @@ -932,11 +932,14 @@ new function() { // Injection scope for various item event handlers /** * Returns to correct matrix to use to transform stroke related geometries * when calculating bounds: the item's matrix if {@link #strokeScaling} is - * `true`, otherwise the shiftless, inverted view matrix. + * `true`, otherwise the parent's inverted view matrix. The returned matrix + * is always shiftless, meaning its translation vector is reset to zero. */ _getStrokeMatrix: function(matrix, options) { - return this.getStrokeScaling() ? matrix : (options && options.internal - ? this : this._parent).getViewMatrix().invert()._shiftless(); + var mx = this.getStrokeScaling() ? matrix : (options && options.internal + ? this : this._parent || this._parentSymbol._item) + .getViewMatrix().invert(); + return mx && mx._shiftless(); }, statics: /** @lends Item */{ @@ -1870,7 +1873,9 @@ new function() { // Injection scope for hit-test functions shared with project // If this is the first one in the recursion, factor in the // zoom of the view and the globalMatrix of the item. : this.getGlobalMatrix().prepend(this.getView()._matrix), - strokeMatrix = viewMatrix.inverted(), + strokeMatrix = this.getStrokeScaling() + ? null + : viewMatrix.inverted()._shiftless(), // 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. diff --git a/src/path/Path.js b/src/path/Path.js index ebb29112..a412c4bb 100644 --- a/src/path/Path.js +++ b/src/path/Path.js @@ -1584,8 +1584,7 @@ var Path = PathItem.extend(/** @lends Path# */{ // Add the stroke radius to tolerance padding, taking // #strokeScaling into account through _getStrokeMatrix(). strokePadding = strokePadding.add( - Path._getStrokePadding(strokeRadius, - !style.getStrokeScaling() && strokeMatrix)); + Path._getStrokePadding(strokeRadius, strokeMatrix)); } else { join = cap = 'round'; } @@ -1643,11 +1642,11 @@ var Path = PathItem.extend(/** @lends Path# */{ || segment._handleOut.isZero())) // _addBevelJoin() handles both 'bevel' and 'miter'! Path._addBevelJoin(segment, join, strokeRadius, - miterLimit, addToArea, true); + miterLimit, null, strokeMatrix, addToArea, true); } else if (cap !== 'round') { // It's a cap - Path._addSquareCap(segment, cap, strokeRadius, addToArea, - true); + Path._addSquareCap(segment, cap, strokeRadius, null, + strokeMatrix, addToArea, true); } // See if the above produced an area to check for if (!area.isEmpty()) { @@ -2599,16 +2598,14 @@ statics: { joinBounds = new Rectangle(new Size(strokePadding)); // helper function that is passed to _addBevelJoin() and _addSquareCap() - // to handle the point transformations. Use strokeMatrix here! - function add(point) { - bounds = bounds.include(strokeMatrix - ? strokeMatrix._transformPoint(point, point) : point); + // to handle the point transformations. + function addPoint(point) { + bounds = bounds.include(point); } function addRound(segment) { - var point = segment._point; - bounds = bounds.unite(joinBounds.setCenter(matrix - ? matrix._transformPoint(point) : point)); + bounds = bounds.unite( + joinBounds.setCenter(segment._point.transform(matrix))); } function addJoin(segment, join) { @@ -2620,7 +2617,8 @@ statics: { && handleIn.isCollinear(handleOut)) { addRound(segment); } else { - Path._addBevelJoin(segment, join, strokeRadius, miterLimit, add); + Path._addBevelJoin(segment, join, strokeRadius, miterLimit, + matrix, strokeMatrix, addPoint); } } @@ -2628,7 +2626,8 @@ statics: { if (cap === 'round') { addRound(segment); } else { - Path._addSquareCap(segment, cap, strokeRadius, add); + Path._addSquareCap(segment, cap, strokeRadius, matrix, + strokeMatrix, addPoint); } } @@ -2658,9 +2657,8 @@ statics: { // and calculate the bounding box of the resulting rotated elipse: // Get rotated hor and ver vectors, and determine rotation angle // and elipse values from them: - var mx = matrix._shiftless(), - hor = mx.transform(new Point(radius, 0)), - ver = mx.transform(new Point(0, radius)), + var hor = new Point(radius, 0).transform(matrix), + ver = new Point(0, radius).transform(matrix), phi = hor.getAngleInRadians(), a = hor.getLength(), b = ver.getLength(); @@ -2689,7 +2687,8 @@ statics: { Math.abs(b * Math.sin(ty) * cos + a * Math.cos(ty) * sin)]; }, - _addBevelJoin: function(segment, join, radius, miterLimit, addPoint, area) { + _addBevelJoin: function(segment, join, radius, miterLimit, matrix, + strokeMatrix, addPoint, isArea) { // Handles both 'bevel' and 'miter' joins, as they share a lot of code. var curve2 = segment.getCurve(), curve1 = curve2.getPrevious(), @@ -2699,40 +2698,53 @@ statics: { step = normal1.getDirectedAngle(normal2) < 0 ? -radius : radius; normal1.setLength(step); normal2.setLength(step); - if (area) { + // use different matrices to transform segment points and stroke vectors + // to support Style#strokeScaling. + if (matrix) + matrix._transformPoint(point, point); + if (strokeMatrix) { + strokeMatrix._transformPoint(normal1, normal1); + strokeMatrix._transformPoint(normal2, normal2); + } + if (isArea) { addPoint(point); addPoint(point.add(normal1)); } if (join === 'miter') { // Intersect the two lines - var corner = new Line( - point.add(normal1), + var corner = new Line(point.add(normal1), new Point(-normal1.y, normal1.x), true - ).intersect(new Line( - point.add(normal2), + ).intersect(new Line(point.add(normal2), new Point(-normal2.y, normal2.x), true ), true); // See if we actually get a bevel point and if its distance is below // the miterLimit. If not, make a normal bevel. if (corner && point.getDistance(corner) <= miterLimit) { addPoint(corner); - if (!area) + if (!isArea) return; } } // Produce a normal bevel - if (!area) + if (!isArea) addPoint(point.add(normal1)); addPoint(point.add(normal2)); }, - _addSquareCap: function(segment, cap, radius, addPoint, area) { + _addSquareCap: function(segment, cap, radius, matrix, strokeMatrix, + addPoint, isArea) { // Handles both 'square' and 'butt' caps, as they share a lot of code. // Calculate the corner points of butt and square caps var point = segment._point, loc = segment.getLocation(), normal = loc.getNormal().multiply(radius); // normal is normalized - if (area) { + // use different matrices to transform segment points and stroke vectors + // to support Style#strokeScaling. + if (matrix) + matrix._transformPoint(point, point); + if (strokeMatrix) + strokeMatrix._transformPoint(normal, normal); + if (isArea) { addPoint(point.subtract(normal)); addPoint(point.add(normal)); } @@ -2741,9 +2753,10 @@ statics: { // Checking loc.getTime() for 0 is to see whether this is the first // or the last segment of the open path, in order to determine in which // direction to move the point. - if (cap === 'square') + if (cap === 'square') { point = point.add(normal.rotate( loc.getTime() === 0 ? -90 : 90)); + } addPoint(point.add(normal)); addPoint(point.subtract(normal)); }, diff --git a/test/tests/Item_Bounds.js b/test/tests/Item_Bounds.js index 41ec527a..ce72412d 100644 --- a/test/tests/Item_Bounds.js +++ b/test/tests/Item_Bounds.js @@ -77,7 +77,7 @@ test('path.bounds when contained in a transformed group', function() { equals(path.bounds, new Rectangle(110, 110, 50, 50), 'path.bounds after group translation'); }); -test('shape.strokeBounds when scaled with strokeScaling set to false', function(){ +test('shape.strokeBounds when scaled without strokeScaling', function(){ var shape = new Shape.Rectangle({ point: [5, 5], size: [20, 20], @@ -88,6 +88,8 @@ test('shape.strokeBounds when scaled with strokeScaling set to false', function( equals(shape.strokeBounds, new Rectangle(0, 0, 30, 30), 'shape.strokeBounds before scaling'); shape.scale(2, 2, [5, 5]); equals(shape.strokeBounds, new Rectangle(0, 0, 50, 50), 'shape.strokeBounds after scaling'); + shape.strokeScaling = true; + equals(shape.strokeBounds, new Rectangle(-5, -5, 60, 60), 'shape.strokeBounds after enabling strokeScaling'); }); test('text.bounds', function() { @@ -101,8 +103,6 @@ test('text.bounds', function() { equals(text.bounds, new Rectangle(50, 87.4, 76.25, 16.8), 'text.bounds', { tolerance: 1.0 }); }); -QUnit.module('Path Bounds'); - test('path.bounds', function() { var path = new Path([ new Segment(new Point(121, 334), new Point(-19, 38), new Point(30.7666015625, -61.53369140625)), @@ -685,3 +685,53 @@ test('compoundPath.strokeBounds', function() { equals(function() { return path.bounds; }, bounds); equals(function() { return path.strokeBounds; }, strokeBounds); }); + +test('path.strokeBounds with applyMatrix disabled', function() { + var path = new Path.Rectangle({ + applyMatrix: true, + point: [10, 10], + size: [20, 20], + strokeScaling: true, + strokeColor: 'red', + strokeWidth: 10 + }); + equals(path.strokeBounds, new Rectangle(5, 5, 30, 30), 'path.strokeBounds, applyMatrix enabled'); + path.applyMatrix = false; + equals(path.strokeBounds, new Rectangle(5, 5, 30, 30), 'path.strokeBounds, applyMatrix disabled'); + path.scale([4, 2], [0, 0]); + var expected = new Rectangle(20, 10, 120, 60); + equals(path.strokeBounds, expected, 'path.strokeBounds after scaling, applyMatrix disabled'); + function testHitResult() { + // Hit-testing needs to handle applyMatrix disabled with stroke scaling, + // even when hit-testing on "distorted" stroke joins: + var hitResult = path.hitTest(expected.topLeft); + equals(function() { return hitResult && hitResult.type == 'stroke'; }, true); + equals(function() { return hitResult && hitResult.item == path; }, true); + // Test a little bit outside the bounds, and the stroke hit-test on the + // join should return null: + var hitResult = path.hitTest(expected.topLeft.subtract(1e-3)); + equals(function() { return hitResult == null; }, true); + } + testHitResult(); + path.applyMatrix = true; + expected = new Rectangle(35, 15, 90, 50); + equals(path.strokeBounds, expected, 'path.strokeBounds after scaling, applyMatrix enabled'); + testHitResult(); +}); + +test('symbolItem.bounds with strokeScaling disabled', function() { + var path = new Path.Rectangle({ + size: [20, 20], + strokeWidth: 10, + strokeColor: 'red', + strokeScaling: false + }); + var symbol = new SymbolDefinition(path); + var placed = symbol.place([100, 100]); + equals(placed.bounds, new Rectangle(85, 85, 30, 30), 'placed.bounds'); + placed.scale(4, 2); + equals(placed.bounds, new Rectangle(55, 75, 90, 50), 'placed.bounds after scaling'); + path.strokeScaling = true; + equals(placed.bounds, new Rectangle(40, 70, 120, 60), 'placed.bounds after scaling, strokeScaling enabled'); +}); + diff --git a/test/tests/Path.js b/test/tests/Path.js index 58855920..6d4cb5c1 100644 --- a/test/tests/Path.js +++ b/test/tests/Path.js @@ -339,7 +339,8 @@ test('Path#interpolate', function() { equals(path, halfway); }); -QUnit.module('Path Curves'); +//////////////////////////////////////////////////////////////////////////////// +// Path Curves test('path.curves synchronisation', function() { var path = new Path(); @@ -515,7 +516,8 @@ test('Splitting a path with one curve in the middle result in two paths of the s }, true); }); -QUnit.module('Path Drawing Commands'); +//////////////////////////////////////////////////////////////////////////////// +// Path Drawing Commands test('path.lineTo(point);', function() { var path = new Path();