From ed57b82b197138a8bb6fb82c5a7b20b7b9332cfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Lehni?= Date: Tue, 21 Feb 2017 22:05:38 +0100 Subject: [PATCH] Boolean: Improve handling of multiple crossings on the same curve. --- src/path/CurveLocation.js | 15 +++++------- src/path/Path.js | 7 +++--- src/path/PathItem.Boolean.js | 44 ++++++++++++++++++++---------------- src/path/PathItem.js | 2 +- 4 files changed, 35 insertions(+), 33 deletions(-) diff --git a/src/path/CurveLocation.js b/src/path/CurveLocation.js index db6b35f4..9fe7af31 100644 --- a/src/path/CurveLocation.js +++ b/src/path/CurveLocation.js @@ -89,10 +89,10 @@ var CurveLocation = Base.extend(/** @lends CurveLocation# */{ */ getSegment: function() { // Request curve first, so _segment gets invalidated if it's out of sync - var curve = this.getCurve(), - segment = this._segment; + var segment = this._segment; if (!segment) { - var time = this.getTime(); + var curve = this.getCurve(), + time = this.getTime(); if (time === 0) { segment = curve._segment1; } else if (time === 1) { @@ -119,10 +119,9 @@ var CurveLocation = Base.extend(/** @lends CurveLocation# */{ var path = this._path, that = this; if (path && path._version !== this._version) { - // If the path's segments have changed in the meantime, clear the - // internal _time value and force re-fetching of the correct - // curve again here. - this._time = this._curve = this._offset = null; + // If the path's segments have changed, clear the cached time and + // offset values and force re-fetching of the correct curve. + this._time = this._offset = this._curve = null; } // If path is out of sync, access current curve objects through segment1 @@ -134,7 +133,6 @@ var CurveLocation = Base.extend(/** @lends CurveLocation# */{ if (curve && (that._time = curve.getTimeOf(that._point)) != null) { // Fetch path again as it could be on a new one through split() that._setCurve(curve); - that._segment = segment; return curve; } } @@ -142,7 +140,6 @@ var CurveLocation = Base.extend(/** @lends CurveLocation# */{ return this._curve || trySegment(this._segment) || trySegment(this._segment1) - || trySegment(this._segment1.getNext()) || trySegment(this._segment2.getPrevious()); }, diff --git a/src/path/Path.js b/src/path/Path.js index d282608b..3931347c 100644 --- a/src/path/Path.js +++ b/src/path/Path.js @@ -1199,10 +1199,9 @@ var Path = PathItem.extend(/** @lends Path# */{ this._add([segments[0]]); path.remove(); } - // Close the resulting path and merge first and last segment if they - // touch, meaning the touched at path ends. Also do this if no path - // argument was provided, in which cases the path is joined with itself - // only if its ends touch. + // If the first and last segment touch, close the resulting path and + // merge the end segments. Also do this if no path argument was provided + // in which cases the path is joined with itself only if its ends touch. var first = this.getFirstSegment(), last = this.getLastSegment(); if (first !== last && first._point.isClose(last._point, epsilon)) { diff --git a/src/path/PathItem.Boolean.js b/src/path/PathItem.Boolean.js index 4fafa2ba..c7d33635 100644 --- a/src/path/PathItem.Boolean.js +++ b/src/path/PathItem.Boolean.js @@ -148,7 +148,10 @@ PathItem.inject(new function() { } else { // When there are no crossings, the result can be determined through // a much faster call to reorientPaths(): - paths = reorientPaths(paths2 ? paths1.concat(paths2) : paths1, + paths = reorientPaths( + // Make sure reorientPaths() never works on original + // _children arrays by calling paths1.slice() + paths2 ? paths1.concat(paths2) : paths1.slice(), function(w) { return !!operator[w]; }); @@ -357,6 +360,7 @@ PathItem.inject(new function() { // be changed to the scaled value after splitting previously. // See CurveLocation#getCurve(), #resolveCrossings() time = loc._time, + origTime = time, exclude = include && !include(loc), // Retrieve curve after calling include(), because it may cause // a change in the cached location values, see above. @@ -371,12 +375,12 @@ PathItem.inject(new function() { // renormalization within the curve. renormalizeLocs = []; prevTime = null; + prevCurve = curve; } else if (prevTime >= tMin) { // Rescale curve-time when we are splitting the same curve // multiple times, if splitting was done previously. - loc._time /= prevTime; + time /= prevTime; } - prevCurve = curve; } if (exclude) { // Store excluded locations for later renormalization, in case @@ -387,8 +391,7 @@ PathItem.inject(new function() { } else if (include) { results.unshift(loc); } - prevTime = time; - time = loc._time; + prevTime = origTime; if (time < tMin) { segment = curve._segment1; } else if (time > tMax) { @@ -1121,7 +1124,7 @@ PathItem.inject(new function() { var hasOverlaps = false, hasCrossings = false, intersections = this.getIntersections(null, function(inter) { - return inter._overlap && (hasOverlaps = true) || + return inter.hasOverlap() && (hasOverlaps = true) || inter.isCrossing() && (hasCrossings = true); }), // We only need to keep track of curves that need clearing @@ -1132,7 +1135,7 @@ PathItem.inject(new function() { // First divide in all overlaps, and then remove the inside of // the resulting overlap ranges. var overlaps = divideLocations(intersections, function(inter) { - return inter._overlap; + return inter.hasOverlap(); }, clearCurves); for (var i = overlaps.length - 1; i >= 0; i--) { var seg = overlaps[i]._segment, @@ -1160,20 +1163,23 @@ PathItem.inject(new function() { // Check both involved curves to see if they're still valid, // meaning they are still part of their paths. var curve1 = inter.getCurve(), - // Do not call getCurve() on the other intersection yet, - // as it too is in the intersections array and will be - // divided later. But do check if its current curve is - // still valid. This is required by some very rare edge + seg1 = inter.getSegment(), + // Do not call getCurve() and getSegment() on the other + // intersection yet, as it too is in the intersections + // array and will be divided later. But check if its + // current curve is valid, as required by some rare edge // cases, related to intersections on the same curve. - curve2 = inter._intersection._curve, - seg = inter._segment; - if (curve1 && curve2 && curve1._path && curve2._path) { + other = inter._intersection, + curve2 = other._curve, + seg2 = other._segment; + if (curve1 && curve2 && curve1._path && curve2._path) return true; - } else if (seg) { - // Remove all intersections that were involved in the - // handling of overlaps, to not confuse tracePaths(). - seg._intersection = null; - } + // Remove all intersections that were involved in the + // handling of overlaps, to not confuse tracePaths(). + if (seg1) + seg1._intersection = null; + if (seg2) + seg2._intersection = null; }, clearCurves); if (clearCurves) clearCurveHandles(clearCurves); diff --git a/src/path/PathItem.js b/src/path/PathItem.js index b1909cd4..4614c3b9 100644 --- a/src/path/PathItem.js +++ b/src/path/PathItem.js @@ -355,7 +355,7 @@ var PathItem = Item.extend(/** @lends PathItem# */{ // as it should in the known use cases. // The ideal implementation would deal with it in a way outlined in: // https://github.com/paperjs/paper.js/issues/874#issuecomment-168332391 - return inter._overlap || inter.isCrossing(); + return inter.hasOverlap() || inter.isCrossing(); }); },