diff --git a/src/item/Item.js b/src/item/Item.js index e1df195c..761489cd 100644 --- a/src/item/Item.js +++ b/src/item/Item.js @@ -157,7 +157,7 @@ new function() { // Injection scope for various item event handlers this._setProject(project); } else { (hasProps && props.parent || project) - ._insertItem(undefined, this, true, true); + ._insertItem(undefined, this, true); // _created = true } // Filter out Item.NO_INSERT before _set(), for performance reasons. if (hasProps && props !== Item.NO_INSERT) { @@ -1373,9 +1373,9 @@ new function() { // Injection scope for various item event handlers return this._children; }, - setChildren: function(items, _preserve) { + setChildren: function(items) { this.removeChildren(); - this.addChildren(items, _preserve); + this.addChildren(items); }, /** @@ -2287,8 +2287,8 @@ new function() { // Injection scope for hit-test functions shared with project * @param {Item} item the item to be added as a child * @return {Item} the added item, or `null` if adding was not possible */ - addChild: function(item, _preserve) { - return this.insertChild(undefined, item, _preserve); + addChild: function(item) { + return this.insertChild(undefined, item); }, /** @@ -2300,8 +2300,8 @@ new function() { // Injection scope for hit-test functions shared with project * @param {Item} item the item to be inserted as a child * @return {Item} the inserted item, or `null` if inserting was not possible */ - insertChild: function(index, item, _preserve) { - var res = item ? this.insertChildren(index, [item], _preserve) : null; + insertChild: function(index, item) { + var res = item ? this.insertChildren(index, [item]) : null; return res && res[0]; }, @@ -2313,8 +2313,8 @@ new function() { // Injection scope for hit-test functions shared with project * @param {Item[]} items the items to be added as children * @return {Item[]} the added items, or `null` if adding was not possible */ - addChildren: function(items, _preserve) { - return this.insertChildren(this._children.length, items, _preserve); + addChildren: function(items) { + return this.insertChildren(this._children.length, items); }, /** @@ -2327,9 +2327,7 @@ new function() { // Injection scope for hit-test functions shared with project * @return {Item[]} the inserted items, or `null` if inserted was not * possible */ - insertChildren: function(index, items, _preserve) { - // CompoundPath#insertChildren() requires _preserve and _type: - // _preserve avoids changing of the children's path orientation + insertChildren: function(index, items) { var children = this._children; if (children && items && items.length > 0) { // We need to clone items because it may be an Item#children array. @@ -2386,7 +2384,7 @@ new function() { // Injection scope for hit-test functions shared with project * @param {Number} offset the offset at which the item should be inserted * @return {Item} the inserted item, or `null` if inserting was not possible */ - _insertAt: function(item, offset, _preserve) { + _insertAt: function(item, offset) { var res = this; if (res !== item) { var owner = item && item._getOwner(); @@ -2394,7 +2392,7 @@ new function() { // Injection scope for hit-test functions shared with project // Notify parent of change. Don't notify item itself yet, // as we're doing so when adding it to the new owner below. res._remove(false, true); - owner._insertItem(item._index + offset, res, _preserve); + owner._insertItem(item._index + offset, res); } else { res = null; } @@ -2408,8 +2406,8 @@ new function() { // Injection scope for hit-test functions shared with project * @param {Item} item the item above which it should be inserted * @return {Item} the inserted item, or `null` if inserting was not possible */ - insertAbove: function(item, _preserve) { - return this._insertAt(item, 1, _preserve); + insertAbove: function(item) { + return this._insertAt(item, 1); }, /** @@ -2418,8 +2416,8 @@ new function() { // Injection scope for hit-test functions shared with project * @param {Item} item the item below which it should be inserted * @return {Item} the inserted item, or `null` if inserting was not possible */ - insertBelow: function(item, _preserve) { - return this._insertAt(item, 0, _preserve); + insertBelow: function(item) { + return this._insertAt(item, 0); }, /** diff --git a/src/item/Project.js b/src/item/Project.js index afd7fe56..70dbf97d 100644 --- a/src/item/Project.js +++ b/src/item/Project.js @@ -378,13 +378,13 @@ var Project = PaperScopeItem.extend(/** @lends Project# */{ // Project#_insertItem() and Item#_insertItem() are helper functions called // in Item#copyTo(), and through _getOwner() in the various Item#insert*() // methods. They are called the same to facilitate so duck-typing. - _insertItem: function(index, item, _preserve, _created) { + _insertItem: function(index, item, _created) { item = this.insertLayer(index, item) // Anything else than layers needs to be added to a layer first. // If none exists yet, create one now, then add the item to it. || (this._activeLayer || this._insertItem(undefined, - new Layer(Item.NO_INSERT), true, true)) - .insertChild(index, item, _preserve); + new Layer(Item.NO_INSERT), true)) // _created = true + .insertChild(index, item); // If a layer was newly created, also activate it. if (_created && item.activate) item.activate(); diff --git a/src/path/CompoundPath.js b/src/path/CompoundPath.js index 47adf7cb..51b69571 100644 --- a/src/path/CompoundPath.js +++ b/src/path/CompoundPath.js @@ -106,7 +106,7 @@ var CompoundPath = PathItem.extend(/** @lends CompoundPath# */{ } }, - insertChildren: function insertChildren(index, items, _preserve) { + insertChildren: function insertChildren(index, items) { // If we're passed a segment array describing a simple path instead of a // compound-path, wrap it in another array to turn it into the array // notation for compound-paths. @@ -124,28 +124,13 @@ var CompoundPath = PathItem.extend(/** @lends CompoundPath# */{ if (list === items && !(item instanceof Path)) list = Base.slice(list); if (Array.isArray(item)) { - var path = new Path({ segments: item, insert: false }); - // Fix natural clockwise value, so it's not automatically - // determined when inserted into the compound-path. - // TODO: Remove reorientation code instead. - path.setClockwise(path.isClockwise()); - list[i] = path; + list[i] = new Path({ segments: item, insert: false }); } else if (item instanceof CompoundPath) { list.splice.apply(list, [i, 1].concat(item.removeChildren())); item.remove(); } } - list = insertChildren.base.call(this, index, list, _preserve); - // All children except for the bottom one (first one in list) are set - // to anti-clockwise orientation, so that they appear as holes, but - // only if their orientation was not already specified before - // (= _clockwise is defined). - for (var i = 0, l = !_preserve && list && list.length; i < l; i++) { - var item = list[i]; - if (item._clockwise === undefined) - item.setClockwise(item._index === 0); - } - return list; + return insertChildren.base.call(this, index, list); }, // DOCS: reduce() @@ -167,22 +152,6 @@ var CompoundPath = PathItem.extend(/** @lends CompoundPath# */{ return reduce.base.call(this); }, - /** - * Specifies whether the compound path is oriented clock-wise. - * - * @bean - * @type Boolean - */ - isClockwise: function() { - var child = this.getFirstChild(); - return child && child.isClockwise(); - }, - - setClockwise: function(clockwise) { - if (this.isClockwise() ^ !!clockwise) - this.reverse(); - }, - /** * Specifies whether the compound-path is fully closed, meaning all its * contained sub-paths are closed path. diff --git a/src/path/Path.js b/src/path/Path.js index e8b95439..333b72d8 100644 --- a/src/path/Path.js +++ b/src/path/Path.js @@ -134,17 +134,12 @@ var Path = PathItem.extend(/** @lends Path# */{ copyContent: function(source) { this.setSegments(source._segments); this._closed = source._closed; - var clockwise = source._clockwise; - if (clockwise !== undefined) - this._clockwise = clockwise; }, _changed: function _changed(flags) { _changed.base.call(this, flags); if (flags & /*#=*/ChangeFlag.GEOMETRY) { - // Clockwise state becomes undefined as soon as geometry changes. - // Also clear cached mono curves used for winding calculations. - this._length = this._area = this._clockwise = undefined; + this._length = this._area = undefined; if (flags & /*#=*/ChangeFlag.SEGMENTS) { this._version++; // See CurveLocation } else if (this._curves) { @@ -854,29 +849,6 @@ var Path = PathItem.extend(/** @lends Path# */{ return area; }, - /** - * Specifies whether the path is oriented clock-wise. - * - * @bean - * @type Boolean - */ - isClockwise: function() { - if (this._clockwise !== undefined) - return this._clockwise; - return this.getArea() >= 0; - }, - - setClockwise: function(clockwise) { - // Only revers the path if its clockwise orientation is not the same - // as what it is now demanded to be. - // On-the-fly conversion to boolean: - if (this.isClockwise() != (clockwise = !!clockwise)) - this.reverse(); - // Reverse only flips _clockwise state if it was already set, so let's - // always set this here now. - this._clockwise = clockwise; - }, - /** * Specifies whether an path is selected and will also return `true` if the * path is partially selected, i.e. one or more of its segments is selected. @@ -1084,9 +1056,7 @@ var Path = PathItem.extend(/** @lends Path# */{ path = this; } else { path = new Path(Item.NO_INSERT); - // Pass true for _preserve, in case of CompoundPath, to avoid - // reversing of path direction, which would mess with segments! - path.insertAbove(this, true); + path.insertAbove(this); path.copyAttributes(this); } path._add(segs, 0); @@ -1264,9 +1234,6 @@ var Path = PathItem.extend(/** @lends Path# */{ } // Clear curves since it all has changed. this._curves = null; - // Flip clockwise state if it's defined - if (this._clockwise !== undefined) - this._clockwise = !this._clockwise; this._changed(/*#=*/Change.GEOMETRY); }, diff --git a/src/path/PathItem.Boolean.js b/src/path/PathItem.Boolean.js index 547efc19..6cb9c6c3 100644 --- a/src/path/PathItem.Boolean.js +++ b/src/path/PathItem.Boolean.js @@ -55,7 +55,9 @@ PathItem.inject(new function() { .transform(null, true, true); if (closed) res.setClosed(true); - return closed ? res.resolveCrossings().reorient(true) : res; + return closed + ? res.resolveCrossings().reorient(res.getFillRule() === 'nonzero') + : res; } function createResult(ctor, paths, reduce, path1, path2) { @@ -990,11 +992,8 @@ PathItem.inject(new function() { var length = paths.length, item; if (length > 1 && children) { - if (paths !== children) { - // TODO: Fix automatic child-orientation in CompoundPath, - // and stop passing true for _preserve. - this.setChildren(paths, true); // Preserve orientation - } + if (paths !== children) + this.setChildren(paths); item = this; } else if (length === 1 && !children) { if (paths[0] !== this) @@ -1005,7 +1004,7 @@ PathItem.inject(new function() { // and attempt to replace this item with it. if (!item) { item = new CompoundPath(Item.NO_INSERT); - item.addChildren(paths, true); // Preserve orientation + item.addChildren(paths); item = item.reduce(); item.copyAttributes(this); this.replaceWith(item); @@ -1015,49 +1014,44 @@ PathItem.inject(new function() { /** * Fixes the orientation of the sub-paths of a compound-path, assuming - * that non of its sub-paths intersect, by reorienting sub-paths so that - * they are of different winding direction than their containing path, - * except for disjoint sub-paths, i.e. islands, which are reoriented so + * that non of its sub-paths intersect, by reorienting them so that they + * are of different winding direction than their containing paths, + * except for disjoint sub-paths, i.e. islands, which are oriented so * that they have the same winding direction as the the biggest path. * - * Additionally, if the compound-path has the `'nonzero'` - * {@link #getFillRule()}, the winding of each nested path is counted, - * and sub-paths that do not contribute to the final result are - * discarded. - * - * @param {Boolean} [sort=false] controls if the sub-paths should be - * sorted according to their area from largest to smallest, or if - * normal sequence should be preserved + * @param {Boolean} [nonZero=false] controls if the non-zero fill-rule + * is to be applied, by counting the winding of each nested path and + * discarding sub-paths that do not contribute to the final result * @return {PahtItem} a reference to the item itself, reoriented - * @see #getFillRule() */ - reorient: function(sort) { + reorient: function(nonZero) { var children = this._children, length = children && children.length; if (length > 1) { - children = this.removeChildren(); - // First order the paths by their areas. - var sorted = children.slice().sort(function (a, b) { + // Build a lookup table with information for each path's + // original index and winding contribution. + var lookup = Base.each(children, function(path, i) { + this[path._id] = { + winding: path.isClockwise() ? 1 : -1, + index: i + }; + }, {}), + // Now sort the paths by their areas, from large to small. + sorted = this.removeChildren().sort(function (a, b) { return abs(b.getArea()) - abs(a.getArea()); }), + // Get reference to the first, largest path and insert it + // already. first = sorted[0], - paths = [first], - isNonZero = this.getFillRule() === 'nonzero', - // We only need to build a lookup table with information for - // each path if we process with non-zero fill-rule, or if we - // are to preserve the original sequence in the result. - lookup = (isNonZero || !sort) && Base.each(children, - function(path, i) { - this[path._id] = { - winding: path.isClockwise() ? 1 : -1, - index: i - }; - }, {}); - // Walk through sorted paths, from largest to smallest. - // The first, largest path can be skipped. + paths = []; + // Always insert paths at their original index. With exclusion, + // this produces null entries, but #setChildren() handles those. + paths[lookup[first._id].index] = first; + // Walk through the sorted paths, from largest to smallest. + // Skip the first path, as it is already added. for (var i1 = 1; i1 < length; i1++) { var path1 = sorted[i1], - entry1 = lookup && lookup[path1._id], + entry1 = lookup[path1._id], point = path1.getInteriorPoint(), isContained = false, container = null, @@ -1073,8 +1067,8 @@ PathItem.inject(new function() { // contains the current path, and then set the // orientation to the opposite of the containing path. if (path2.contains(point)) { - var entry2 = lookup && lookup[path2._id]; - if (isNonZero && !isContained) { + var entry2 = lookup[path2._id]; + if (nonZero && !isContained) { entry1.winding += entry2.winding; // Remove path if rule is nonzero and winding // of path and containing path is not zero. @@ -1086,7 +1080,7 @@ PathItem.inject(new function() { isContained = true; // If the containing path is not excluded, we're // done searching for the orientation defining path. - container = !(entry2 && entry2.exclude) && path2; + container = !entry2.exclude && path2; } } if (!exclude) { @@ -1096,18 +1090,10 @@ PathItem.inject(new function() { path1.setClockwise(container ? !container.isClockwise() : first.isClockwise()); - if (!sort) { - // If asked to preserve sequence (not sort children - // according to their area), insert back at their - // original index. With exclusion this produces null - // entries, but #setChildren() can handle those. - paths[entry1.index] = path1; - } else { - paths.push(path1); - } + paths[entry1.index] = path1; } } - this.setChildren(paths, true); // Preserve orientation + this.setChildren(paths); } return this; }, diff --git a/src/path/PathItem.js b/src/path/PathItem.js index c5625ef7..e3a37113 100644 --- a/src/path/PathItem.js +++ b/src/path/PathItem.js @@ -90,6 +90,24 @@ var PathItem = Item.extend(/** @lends PathItem# */{ return this; }, + /** + * Specifies whether the path is oriented clock-wise. + * + * @bean + * @type Boolean + */ + isClockwise: function() { + return this.getArea() >= 0; + }, + + setClockwise: function(clockwise) { + // Only revers the path if its clockwise orientation is not the same + // as what it is now demanded to be. + // On-the-fly conversion to boolean: + if (this.isClockwise() != (clockwise = !!clockwise)) + this.reverse(); + }, + /** * The path's geometry, formatted as SVG style path data. * @@ -97,7 +115,6 @@ var PathItem = Item.extend(/** @lends PathItem# */{ * @bean * @type String */ - setPathData: function(data) { // NOTE: #getPathData() is defined in CompoundPath / Path // This is a very compact SVG Path Data parser that works both for Path diff --git a/test/tests/CompoundPath.js b/test/tests/CompoundPath.js index a89d6158..31c19364 100644 --- a/test/tests/CompoundPath.js +++ b/test/tests/CompoundPath.js @@ -34,10 +34,10 @@ test('moveTo() / lineTo()', function() { }, 2); }); -test('CompoundPath() and default clockwise settings', function() { - var path1 = new Path.Rectangle([200, 200], [100, 100]); +test('CompoundPath#reorient()', function() { + var path1 = new Path.Rectangle([300, 300], [100, 100]); var path2 = new Path.Rectangle([50, 50], [200, 200]); - var path3 = new Path.Rectangle([0, 0], [400, 400]); + var path3 = new Path.Rectangle([0, 0], [500, 500]); equals(function() { return path1.clockwise; @@ -49,7 +49,9 @@ test('CompoundPath() and default clockwise settings', function() { return path3.clockwise; }, true); - var compound = new CompoundPath(path1, path2, path3); + var compound = new CompoundPath({ + children: [path1, path2, path3], + }).reorient(); equals(function() { return compound.lastChild == path3; @@ -59,29 +61,10 @@ test('CompoundPath() and default clockwise settings', function() { }, true); equals(function() { return path1.clockwise; - }, true); + }, false); equals(function() { return path2.clockwise; }, false); - equals(function() { - return path3.clockwise; - }, false); -}); - -test('CompoundPath() and predefined clockwise settings', function() { - var path1 = new Path.Rectangle([200, 200], [100, 100]); - var path2 = new Path.Rectangle([50, 50], [200, 200]); - var path3 = new Path.Rectangle([0, 0], [400, 400]); - path1.clockwise = false; - path2.clockwise = true; - path3.clockwise = true; - var compound = new CompoundPath(path1, path2, path3); - equals(function() { - return path1.clockwise; - }, false); - equals(function() { - return path2.clockwise; - }, true); equals(function() { return path3.clockwise; }, true); diff --git a/test/tests/HitResult.js b/test/tests/HitResult.js index db3490de..c0052a35 100644 --- a/test/tests/HitResult.js +++ b/test/tests/HitResult.js @@ -696,7 +696,8 @@ test('hit-testing compound-paths', function() { }); var compoundPath = new CompoundPath({ children: [path1, path2], - fillColor: 'blue' + fillColor: 'blue', + fillRule: 'evenodd' }); // When hit-testing a side, we should get a result on the torus equals(function() { diff --git a/test/tests/PathItem_Contains.js b/test/tests/PathItem_Contains.js index 2c5ccd2c..d3083805 100644 --- a/test/tests/PathItem_Contains.js +++ b/test/tests/PathItem_Contains.js @@ -96,40 +96,50 @@ test('CompoundPath#contains() (donut)', function() { new Path.Circle([0, 0], 25) ]); - testPoint(path, new Point(0, -50), true, - 'The top center point of the outer circle should be inside the donut.'); - testPoint(path, new Point(0, 0), false, - 'The center point should be outside the donut.'); - testPoint(path, new Point(-35, 0), true, - 'A vertically centered point on the left side should be inside the donut.'); - testPoint(path, new Point(35, 0), true, - 'A vertically centered point on the right side should be inside the donut.'); - testPoint(path, new Point(0, 49), true, - 'The near bottom center point of the outer circle should be inside the donut.'); - testPoint(path, new Point(0, 50), true, - 'The bottom center point of the outer circle should be inside the donut.'); - testPoint(path, new Point(0, 51), false, - 'The near bottom center point of the outer circle should be outside the donut.'); - testPoint(path, new Point({ length: 50, angle: 30 }), true, - 'A random point on the periphery of the outer circle should be inside the donut.'); - testPoint(path, new Point(-25, 0), true, - 'The left center point of the inner circle should be inside the donut.'); - testPoint(path, new Point(0, -25), true, - 'The top center point of the inner circle should be inside the donut.'); - testPoint(path, new Point(25, 0), true, - 'The right center point of the inner circle should be inside the donut.'); - testPoint(path, new Point(0, 25), true, - 'The bottom center point of the inner circle should be inside the donut.'); - testPoint(path, new Point(-50, -50), false, - 'The top left point of bounding box should be outside the donut.'); - testPoint(path, new Point(50, -50), false, - 'The top right point of the bounding box should be outside the donut.'); - testPoint(path, new Point(-50, 50), false, - 'The bottom left point of bounding box should be outside the donut.'); - testPoint(path, new Point(50, 50), false, - 'The bottom right point of the bounding box should be outside the donut.'); - testPoint(path, new Point(-45, 45), false, - 'The near bottom left point of bounding box should be outside the donut.'); + function testDonut(path, title) { + title = 'fillRule = ' + title + ': '; + testPoint(path, new Point(0, -50), true, title + + 'The top center point of the outer circle should be inside the donut.'); + testPoint(path, new Point(0, 0), false, title + + 'The center point should be outside the donut.'); + testPoint(path, new Point(-35, 0), true, title + + 'A vertically centered point on the left side should be inside the donut.'); + testPoint(path, new Point(35, 0), true, title + + 'A vertically centered point on the right side should be inside the donut.'); + testPoint(path, new Point(0, 49), true, title + + 'The near bottom center point of the outer circle should be inside the donut.'); + testPoint(path, new Point(0, 50), true, title + + 'The bottom center point of the outer circle should be inside the donut.'); + testPoint(path, new Point(0, 51), false, title + + 'The near bottom center point of the outer circle should be outside the donut.'); + testPoint(path, new Point({ length: 50, angle: 30 }), true, title + + 'A random point on the periphery of the outer circle should be inside the donut.'); + testPoint(path, new Point(-25, 0), true, title + + 'The left center point of the inner circle should be inside the donut.'); + testPoint(path, new Point(0, -25), true, title + + 'The top center point of the inner circle should be inside the donut.'); + testPoint(path, new Point(25, 0), true, title + + 'The right center point of the inner circle should be inside the donut.'); + testPoint(path, new Point(0, 25), true, title + + 'The bottom center point of the inner circle should be inside the donut.'); + testPoint(path, new Point(-50, -50), false, title + + 'The top left point of bounding box should be outside the donut.'); + testPoint(path, new Point(50, -50), false, title + + 'The top right point of the bounding box should be outside the donut.'); + testPoint(path, new Point(-50, 50), false, title + + 'The bottom left point of bounding box should be outside the donut.'); + testPoint(path, new Point(50, 50), false, title + + 'The bottom right point of the bounding box should be outside the donut.'); + testPoint(path, new Point(-45, 45), false, title + + 'The near bottom left point of bounding box should be outside the donut.'); + } + + path.fillRule = 'evenodd'; + // testDonut(path, '\'evenodd\''); + path.reorient(); + testDonut(path, '\'evenodd\' + reorient()'); + path.fillRule = 'nonzero'; + testDonut(path, '\'nonzero\' + reorient()'); }); test('Shape#contains()', function() { diff --git a/test/tests/Path_Boolean.js b/test/tests/Path_Boolean.js index 77c1a237..190f7a37 100644 --- a/test/tests/Path_Boolean.js +++ b/test/tests/Path_Boolean.js @@ -350,7 +350,7 @@ test('#870', function() { }); test('#875', function() { - var cp = new Path({ + var p1 = new Path({ segments: [ [158.7, 389.3, 0, 0, -4.95, 4.95], [158.7, 407.2, -4.95, -4.95, 4.95, 4.95], @@ -360,26 +360,29 @@ test('#875', function() { ], closed: true }); - var p = new Path.Circle(260, 320, 100); + var p2 = new Path.Circle(260, 320, 100); - compareBoolean(function() { return cp.subtract(p); }, + compareBoolean(function() { return p1.subtract(p2); }, 'M158.7,407.2c4.95,4.95 12.95,4.95 17.9,0c4.95,-4.95 4.95,-12.95 0,-17.9c-4.95,-4.95 -12.95,-4.95 -17.9,0c-4.95,4.95 -4.95,12.95 0,17.9z'); }); test('#877', function() { - var cp = new CompoundPath([ - new Path.Circle(100, 60, 50), - new Path.Circle(100, 60, 30), - new Path({ - segments: [ - [120, 140], - [150, 140], - [150, 190], - [120, 190] - ], - closed: true - }) - ]); + var cp = new CompoundPath({ + children: [ + new Path.Circle(100, 60, 50), + new Path.Circle(100, 60, 30), + new Path({ + segments: [ + [120, 140], + [150, 140], + [150, 190], + [120, 190] + ], + closed: true + }), + ], + fillRule: 'evenodd' + }); var p = new Path({ segments: [