From af797df5ba1fb1f82e8f098932c27ffc0d2a05eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Lehni?= Date: Thu, 7 Jan 2016 11:18:46 +0100 Subject: [PATCH] Remove includeOverlaps parameter from getCrossings() And write better comments about how overlaps should be dealt with ideally. --- src/path/PathItem.Boolean.js | 7 +++---- src/path/PathItem.js | 14 ++++++++------ 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/path/PathItem.Boolean.js b/src/path/PathItem.Boolean.js index a121812f..7a011a35 100644 --- a/src/path/PathItem.Boolean.js +++ b/src/path/PathItem.Boolean.js @@ -92,9 +92,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( - // Only handle overlaps when not self-intersecting - _path1.getCrossings(_path2, !!_path2))), + var crossings = divideLocations( + CurveLocation.expand(_path1.getCrossings(_path2))), segments = [], // Aggregate of all curves in both operands, monotonic in y. monoCurves = [], @@ -156,7 +155,7 @@ PathItem.inject(new function() { return null; var _path1 = preparePath(path1, false), _path2 = preparePath(path2, false), - crossings = _path1.getCrossings(_path2, true), + crossings = _path1.getCrossings(_path2), sub = operator.subtract, paths = []; diff --git a/src/path/PathItem.js b/src/path/PathItem.js index f10e22aa..79210825 100644 --- a/src/path/PathItem.js +++ b/src/path/PathItem.js @@ -146,16 +146,18 @@ var PathItem = Item.extend(/** @lends PathItem# */{ * crossing each other, as opposed to simply touching. * * @param {PathItem} path the other item to find the crossings with - * @param {Boolean} includeOverlaps whether to also count overlaps as - * crossings * @see #getIntersections(path) */ - getCrossings: function(path, includeOverlaps) { + 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 first. - // Check overlap first since it's the cheaper test between the two. - return includeOverlaps && inter._overlap || inter.isCrossing(); + // 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._overlap || inter.isCrossing(); }); },