From c82e5d41f7074533f3037553286ba35e442cb0f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Lehni?= Date: Sat, 14 Dec 2019 19:40:00 +0100 Subject: [PATCH] Improve fix for nested group matrix reset Closes #1711 --- src/basic/Matrix.js | 7 +++--- src/item/Item.js | 61 ++++++++++++++++++++++++--------------------- test/tests/Group.js | 31 ++++++++++++++++++++--- 3 files changed, 63 insertions(+), 36 deletions(-) diff --git a/src/basic/Matrix.js b/src/basic/Matrix.js index 5ac409ab..c155bfba 100644 --- a/src/basic/Matrix.js +++ b/src/basic/Matrix.js @@ -185,15 +185,14 @@ var Matrix = Base.extend(/** @lends Matrix# */{ * Attempts to apply the matrix to the content of item that it belongs to, * meaning its transformation is baked into the item's content or children. * - * @param {Boolean} [recursively=true] controls whether to apply transformations - * recursively on children + * @param {Boolean} [recursively=true] controls whether to apply + * transformations recursively on children * @return {Boolean} {@true if the matrix was applied} */ apply: function(recursively, _setApplyMatrix) { var owner = this._owner; if (owner) { - owner.transform(null, true, Base.pick(recursively, true), - _setApplyMatrix); + owner.transform(null, Base.pick(recursively, true), _setApplyMatrix); // If the matrix was successfully applied, it will be reset now. return this.isIdentity(); } diff --git a/src/item/Item.js b/src/item/Item.js index 3a45d61e..9e6de474 100644 --- a/src/item/Item.js +++ b/src/item/Item.js @@ -3509,19 +3509,22 @@ new function() { // Injection scope for hit-test functions shared with project // @param {String[]} flags array of any of the following: 'objects', // 'children', 'fill-gradients', 'fill-patterns', 'stroke-patterns', // 'lines'. Default: ['objects', 'children'] - transform: function(matrix, _applyMatrix, _applyRecursively, - _setApplyMatrix) { + transform: function(matrix, _applyRecursively, _setApplyMatrix) { var _matrix = this._matrix, - // If no matrix is provided, or the matrix is the identity, we might - // still have some work to do in case _applyMatrix is true transformMatrix = matrix && !matrix.isIdentity(), - applyMatrix = (_applyMatrix || this._applyMatrix) + // If no matrix is provided, or the matrix is the identity, we might + // still have some work to do: _setApplyMatrix or _applyRecursively. + applyMatrix = ( + _setApplyMatrix && this._canApplyMatrix || + this._applyMatrix && ( // Don't apply _matrix if the result of concatenating with // matrix would be identity. - && ((!_matrix.isIdentity() || transformMatrix) - // Even if it's an identity matrix, we still need to - // recursively apply the matrix to children. - || _applyMatrix && _applyRecursively && this._children); + transformMatrix || !_matrix.isIdentity() || + // Even if it's an identity matrix, we may still need to + // recursively apply the matrix to children. + _applyRecursively && this._children + ) + ); // Bail out if there is nothing to do. if (!transformMatrix && !applyMatrix) return this; @@ -3549,25 +3552,25 @@ new function() { // Injection scope for hit-test functions shared with project if (strokeColor) strokeColor.transform(matrix); } - if (applyMatrix) { - // Set the internal _applyMatrix flag to true if we're told to do so. + // Call #_transformContent() now, if we need to directly apply the + // internal _matrix transformations to the item's content. + // Application is not possible on Raster, PointText, SymbolItem, since + // the matrix is where the actual transformation state is stored. + + if (applyMatrix && (applyMatrix = this._transformContent( + _matrix, _applyRecursively, _setApplyMatrix))) { + // Pivot is provided in the parent's coordinate system, so transform + // it along too. + var pivot = this._pivot; + if (pivot) + _matrix._transformPoint(pivot, pivot, true); + // Reset the internal matrix to the identity transformation if + // it was possible to apply it, but do not notify owner of change. + _matrix.reset(true); + // Set the internal _applyMatrix flag to true if we're told to + // do so if (_setApplyMatrix && this._canApplyMatrix) this._applyMatrix = true; - // Call #_transformContent() now, if we need to directly apply the - // internal _matrix transformations to the item's content. - // Application is not possible on Raster, PointText, SymbolItem, since - // the matrix is where the actual transformation state is stored. - if (this._applyMatrix && (applyMatrix = this._transformContent(_matrix, - _applyRecursively, _setApplyMatrix))) { - // Pivot is provided in the parent's coordinate system, so transform - // it along too. - var pivot = this._pivot; - if (pivot) - _matrix._transformPoint(pivot, pivot, true); - // Reset the internal matrix to the identity transformation if - // it was possible to apply it, but do not notify owner of change. - _matrix.reset(true); - } } // Calling _changed will clear _bounds and _position, but depending // on matrix we can calculate and set them again, so preserve them. @@ -3619,9 +3622,9 @@ new function() { // Injection scope for hit-test functions shared with project _transformContent: function(matrix, applyRecursively, setApplyMatrix) { var children = this._children; if (children) { - for (var i = 0, l = children.length; i < l; i++) - children[i].transform(matrix, true, applyRecursively, - setApplyMatrix); + for (var i = 0, l = children.length; i < l; i++) { + children[i].transform(matrix, applyRecursively, setApplyMatrix); + } return true; } }, diff --git a/test/tests/Group.js b/test/tests/Group.js index 55ab101a..5b9bc1b3 100644 --- a/test/tests/Group.js +++ b/test/tests/Group.js @@ -192,10 +192,35 @@ test( test('group.matrix with parent matrix applied (#1711)', function() { var child = new Group({ applyMatrix: false }); - var parent = new Group([child]); + var parent = new Group({ applyMatrix: true, children: [child] }); var scale = 1.1; var initial = child.scaling.x; parent.scale(scale); - var final = child.scaling.x; - equals(final, initial * scale); + equals(child.scaling.x, initial * scale); +}); + +test('Nested group.matrix.apply(true, true) with matrices not applied', function() { + var path = new Path({ applyMatrix: false }); + var group = new Group({ applyMatrix: false, children: [path] }); + var parent = new Group({ applyMatrix: false, children: [group] }); + var grandParent = new Group({ applyMatrix: false, children: [parent] }); + equals(function() { + return grandParent.applyMatrix; + }, false); + equals(function() { + return group.applyMatrix; + }, false); + equals(function() { + return path.applyMatrix; + }, false); + grandParent.matrix.apply(true, true); + equals(function() { + return grandParent.applyMatrix; + }, true); + equals(function() { + return group.applyMatrix; + }, true); + equals(function() { + return path.applyMatrix; + }, true); });