Fix Item#strokeBounds with #strokeScaling and #applyMatrix

Closes #1014

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).

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
+        var mx = this.getStrokeScaling() ? matrix : (options && options.internal
-                ? this : this._parent).getViewMatrix().invert()._shiftless();
+                ? 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.

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,
+                    Path._getStrokePadding(strokeRadius, strokeMatrix));
-                        !style.getStrokeScaling() && 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,
+                    Path._addSquareCap(segment, cap, strokeRadius, null,
-                          true);
+                            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!
+        // to handle the point transformations.
-        function add(point) {
+        function addPoint(point) {
-            bounds = bounds.include(strokeMatrix
+            bounds = bounds.include(point);
-                ? strokeMatrix._transformPoint(point, point) : point);
         }
 
         function addRound(segment) {
-            var point = segment._point;
+            bounds = bounds.unite(
-            bounds = bounds.unite(joinBounds.setCenter(matrix
+                    joinBounds.setCenter(segment._point.transform(matrix)));
-                    ? matrix._transformPoint(point) : point));
         }
 
         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(),
+        var hor = new Point(radius, 0).transform(matrix),
-            hor = mx.transform(new Point(radius, 0)),
+            ver = new Point(0, radius).transform(matrix),
-            ver = mx.transform(new Point(0, radius)),
             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(
+            var corner = new Line(point.add(normal1),
-                    point.add(normal1),
                     new Point(-normal1.y, normal1.x), true
-                ).intersect(new Line(
+                ).intersect(new Line(point.add(normal2),
-                    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));
     },

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');
+});
+

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();