Skip to content

Commit

Permalink
Fix PathItem#isCrossing() to not return overlaps
Browse files Browse the repository at this point in the history
  • Loading branch information
lehni committed Jun 23, 2019
1 parent 7f49640 commit bba7090
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 11 deletions.
17 changes: 14 additions & 3 deletions src/path/PathItem.Boolean.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,17 @@ PathItem.inject(new function() {
return result;
}

function filterIntersection(inter) {
// TODO: Change isCrossing() to also handle overlaps (hasOverlap())
// that are actually involved in a crossing! For this we need proper
// overlap range detection / merging first... But as we call
// #resolveCrossings() first in boolean operations, removing all
// self-touching areas in paths, this works for 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.hasOverlap() || inter.isCrossing();
}

function traceBoolean(path1, path2, operation, options) {
// Only support subtract and intersect operations when computing stroke
// based boolean operations (options.split = true).
Expand All @@ -121,8 +132,8 @@ PathItem.inject(new function() {
_path2.reverse();
// Split curves at crossings on both paths. Note that for self-
// intersection, path2 is null and getIntersections() handles it.
var crossings = divideLocations(
CurveLocation.expand(_path1.getCrossings(_path2))),
var crossings = divideLocations(CurveLocation.expand(
_path1.getIntersections(_path2, filterIntersection))),
paths1 = getPaths(_path1),
paths2 = _path2 && getPaths(_path2),
segments = [],
Expand Down Expand Up @@ -182,7 +193,7 @@ PathItem.inject(new function() {
function splitBoolean(path1, path2, operation) {
var _path1 = preparePath(path1),
_path2 = preparePath(path2),
crossings = _path1.getCrossings(_path2),
crossings = _path1.getIntersections(_path2, filterIntersection),
subtract = operation === 'subtract',
divide = operation === 'divide',
added = {},
Expand Down
9 changes: 1 addition & 8 deletions src/path/PathItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -351,14 +351,7 @@ var PathItem = Item.extend(/** @lends PathItem# */{
*/
getCrossings: function(path) {
return this.getIntersections(path, function(inter) {
// TODO: Only return overlaps that are actually crossings! For this
// we need proper overlap range detection / merging first...
// But as we call #resolveCrossings() first in boolean operations,
// removing all self-touching areas in paths, this currently works
// 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.hasOverlap() || inter.isCrossing();
return inter.isCrossing();
});
},

Expand Down
20 changes: 20 additions & 0 deletions test/tests/Path_Intersections.js
Original file line number Diff line number Diff line change
Expand Up @@ -380,3 +380,23 @@ test('#1262', function() {
{ point: { x: 567.05562, y: 634.62043 }, time: 1 }
]);
});

test('#1409', function() {
var path1 = new Path({
segments: [[20, 20], [20, 80], [80, 80], [80, 20]],
closed: true
});
var path2 = new Path({
segments: [[80, 20], [80, 80], [140, 80], [140, 20]],
closed: true
});
testIntersections(path1.getCrossings(path2), []);

var rect1 = new Path.Rectangle(new Point(100, 100), new Size(100, 100));
var rect2 = rect1.clone();
testIntersections(rect1.getCrossings(rect2), []);

var circ1 = new Path.Circle(new Point(300,300), 40);
var circ2 = circ1.clone();
testIntersections(circ1.getCrossings(circ2), []);
});

0 comments on commit bba7090

Please sign in to comment.