From 55d525b8a305cfb17e6515c931f1fc2d1d1f80a0 Mon Sep 17 00:00:00 2001 From: Mike Fogel Date: Sun, 24 Feb 2019 13:19:15 -0300 Subject: [PATCH 01/12] Rewrite of Segment.compare() --- src/segment.js | 181 +++++++++++++++++++++++-------------------- test/segment.test.js | 22 +++--- 2 files changed, 109 insertions(+), 94 deletions(-) diff --git a/src/segment.js b/src/segment.js index fc4a27d..ccf2c98 100644 --- a/src/segment.js +++ b/src/segment.js @@ -9,105 +9,120 @@ import { closestPoint, intersection, verticalIntersection } from './vector' let segmentId = 0 export default class Segment { + + /* This compare() function is for ordering segments in the sweep + * line tree, and does so according to the following criteria: + * + * Consider the vertical line that lies an infinestimal step to the + * right of the right-more of the two left endpoints of the input + * segments. Imagine slowly moving a point up from negative infinity + * in the increasing y direction. Which of the two segments will that + * point intersect first? That segment comes 'before' the other one. + * + * If neither segment would be intersected by such a line, (if one + * or more of the segments are vertical) then the line to be considered + * is directly on the right-more of the two left inputs. + */ static compare (a, b) { const alx = a.leftSE.point.x - const aly = a.leftSE.point.y const blx = b.leftSE.point.x - const bly = b.leftSE.point.y const arx = a.rightSE.point.x const brx = b.rightSE.point.x // check if they're even in the same vertical plane - if (cmp(brx, alx) < 0) return 1 - if (cmp(arx, blx) < 0) return -1 - - // check for a consumption relationship. if present, - // avoid the segment angle calculations (can yield - // inconsistent results after splitting) - let aConsumedBy = a - let bConsumedBy = b - while (aConsumedBy.consumedBy) aConsumedBy = aConsumedBy.consumedBy - while (bConsumedBy.consumedBy) bConsumedBy = bConsumedBy.consumedBy - - // for segment angle comparisons - let aCmpBLeft, aCmpBRight, bCmpALeft, bCmpARight - - if (aConsumedBy === bConsumedBy) { - // are they identical? - if (a === b) return 0 - - // colinear segments with matching left-endpoints, fall back - // on creation order as a tie-breaker - if (a.id < b.id) return -1 - if (a.id > b.id) return 1 - - } else if ( - // are a and b colinear? - (aCmpBLeft = a.comparePoint(b.leftSE.point)) === 0 && - (aCmpBRight = a.comparePoint(b.rightSE.point)) === 0 && - (bCmpALeft = b.comparePoint(a.leftSE.point)) === 0 && - (bCmpARight = b.comparePoint(a.rightSE.point)) === 0 - ) { - // a & b are colinear - - // colinear segments with non-matching left-endpoints, consider - // the more-left endpoint to be earlier - const cmpLX = cmp(alx, blx) - if (cmpLX !== 0) return cmpLX - - // NOTE: we do not use segment length to break a tie here, because - // when segments are split their length changes - - // colinear segments with matching left-endpoints, fall back - // on creation order as a tie-breaker - if (a.id < b.id) return -1 - if (a.id > b.id) return 1 + if (brx < alx) return 1 + if (arx < blx) return -1 - } else { - // a & b are not colinear - - const cmpLX = cmp(alx, blx) - // if the left endpoints are not in the same vertical line, - // consider the placement of the left event of the right-more segment - // with respect to the left-more segment. - if (cmpLX < 0) { - if (aCmpBLeft > 0) return -1 - if (aCmpBLeft < 0) return 1 - // NOTE: fall-through is necessary here. why? Can that be avoided? - } - if (cmpLX > 0) { - if (bCmpALeft === undefined) bCmpALeft = b.comparePoint(a.leftSE.point) - if (bCmpALeft !== 0) return bCmpALeft - // NOTE: fall-through is necessary here. why? Can that be avoided? - } + const aly = a.leftSE.point.y + const bly = b.leftSE.point.y + const ary = a.rightSE.point.y + const bry = b.rightSE.point.y + + // is left endpoint of segment B the right-more? + if (alx < blx) { + // are the two segments in the same horizontal plane? + if (bly < aly && bly < ary) return 1 + if (bly > aly && bly > ary) return -1 + + // is the B left endpoint colinear to segment A? + const aCmpBLeft = a.comparePoint(b.leftSE.point) + if (aCmpBLeft < 0) return 1 + if (aCmpBLeft > 0) return -1 + + // is the A right endpoint colinear to segment B ? + const bCmpARight = b.comparePoint(a.rightSE.point) + if (bCmpARight !== 0) return bCmpARight + + // colinear segments, consider the one with left-more + // left endpoint to be first (arbitrary?) + return -1 + } - const cmpLY = cmp(aly, bly) - // if left endpoints are in the same vertical line, lower means ealier - if (cmpLY !== 0) return cmpLY - // left endpoints match exactly + // is left endpoint of segment A the right-more? + if (alx > blx) { + if (aly < bly && aly < bry) return -1 + if (aly > bly && aly > bry) return 1 - // special case verticals due to rounding errors - // part of https://github.com/mfogel/polygon-clipping/issues/29 - const aVert = a.isVertical() - if (aVert !== b.isVertical()) return aVert ? 1 : -1 + // is the A left endpoint colinear to segment B? + const bCmpALeft = b.comparePoint(a.leftSE.point) + if (bCmpALeft !== 0) return bCmpALeft - // sometimes, because one segment is longer than the other, - // one of these comparisons will return 0 and the other won't - if (aCmpBRight === undefined) aCmpBRight = a.comparePoint(b.rightSE.point) - if (aCmpBRight > 0) return -1 + // is the B right endpoint colinear to segment A? + const aCmpBRight = a.comparePoint(b.rightSE.point) if (aCmpBRight < 0) return 1 - if (bCmpARight === undefined) bCmpARight = b.comparePoint(a.rightSE.point) + if (aCmpBRight > 0) return -1 + + // colinear segments, consider the one with left-more + // left endpoint to be first (arbitrary?) + return 1 + } + + // if we get here, the two left endpoints are in the same + // vertical plane, ie alx === blx + + // consider the lower left-endpoint to come first + if (aly < bly) return -1 + if (aly > bly) return 1 + + // left endpoints are identical + // check for colinearity by using the left-more right endpoint + + // is the A right endpoint more left-more? + if (arx < brx) { + const bCmpARight = b.comparePoint(a.rightSE.point) if (bCmpARight !== 0) return bCmpARight + + // colinear segments with matching left endpoints, + // consider the one with more left-more right endpoint to be first + return -1 } - throw new Error( - 'Segment comparison of ' + - `[${a.leftSE.point.x}, ${a.leftSE.point.y}] -> [${a.rightSE.point.x}, ${a.rightSE.point.y}] ` + - 'against ' + - `[${b.leftSE.point.x}, ${b.leftSE.point.y}] -> [${b.rightSE.point.x}, ${b.rightSE.point.y}] ` + - 'failed. Please submit a bug report.' - ) + // is the B right endpoint more left-more? + if (arx > brx) { + const aCmpBRight = a.comparePoint(b.rightSE.point) + if (aCmpBRight < 0) return 1 + if (aCmpBRight > 0) return -1 + + // colinear segments with matching left endpoints, + // consider the one with more left-more right endpoint to be first + return 1 + } + + // if we get here, two two right endpoints are in the same + // vertical plane, ie arx === brx + + // consider the lower right-endpoint to come first + if (ary < bry) return -1 + if (ary > bry) return 1 + + // right endpoints identical as well, so the segments are idential + // fall back on creation order as consistent tie-breaker + if (a.id < b.id) return -1 + if (a.id > b.id) return 1 + + // identical segment, ie a === b + return 0 } /* Warning: a reference to ringsIn input will be stored, diff --git a/test/segment.test.js b/test/segment.test.js index 68bcf51..eb07141 100644 --- a/test/segment.test.js +++ b/test/segment.test.js @@ -628,15 +628,15 @@ describe('compare segments', () => { test('intersect on left from above', () => { const seg1 = Segment.fromRing({ x: 0, y: 0 }, { x: 4, y: 0 }) const seg2 = Segment.fromRing({ x: -2, y: 2 }, { x: 2, y: -2 }) - expect(Segment.compare(seg1, seg2)).toBe(-1) - expect(Segment.compare(seg2, seg1)).toBe(1) + expect(Segment.compare(seg1, seg2)).toBe(1) + expect(Segment.compare(seg2, seg1)).toBe(-1) }) test('intersect on left from below', () => { const seg1 = Segment.fromRing({ x: 0, y: 0 }, { x: 4, y: 0 }) const seg2 = Segment.fromRing({ x: -2, y: -2 }, { x: 2, y: 2 }) - expect(Segment.compare(seg1, seg2)).toBe(1) - expect(Segment.compare(seg2, seg1)).toBe(-1) + expect(Segment.compare(seg1, seg2)).toBe(-1) + expect(Segment.compare(seg2, seg1)).toBe(1) }) test('intersect on left from vertical', () => { @@ -709,8 +709,8 @@ describe('compare segments', () => { test('one segment thinks theyre colinear, but the other says no', () => { const seg1 = Segment.fromRing({ x: -60.6876, y: -40.83428174062278 }, { x: -60.6841701, y: -40.83491 }) const seg2 = Segment.fromRing({ x: -60.6876, y: -40.83428174062278 }, { x: -60.6874, y: -40.83431837489067 }) - expect(Segment.compare(seg1, seg2)).toBe(-1) - expect(Segment.compare(seg2, seg1)).toBe(1) + expect(Segment.compare(seg1, seg2)).toBe(1) + expect(Segment.compare(seg2, seg1)).toBe(-1) }) }) @@ -736,12 +736,12 @@ describe('compare segments', () => { expect(Segment.compare(seg2, seg1)).toBe(-1) }) - test('left endpoints match - should be sorted by ring id', () => { + test('left endpoints match - should be length', () => { const seg1 = Segment.fromRing({ x: 0, y: 0 }, { x: 4, y: 4 }, { id: 1 }) const seg2 = Segment.fromRing({ x: 0, y: 0 }, { x: 3, y: 3 }, { id: 2 }) const seg3 = Segment.fromRing({ x: 0, y: 0 }, { x: 5, y: 5 }, { id: 3 }) - expect(Segment.compare(seg1, seg2)).toBe(-1) - expect(Segment.compare(seg2, seg1)).toBe(1) + expect(Segment.compare(seg1, seg2)).toBe(1) + expect(Segment.compare(seg2, seg1)).toBe(-1) expect(Segment.compare(seg2, seg3)).toBe(-1) expect(Segment.compare(seg3, seg2)).toBe(1) @@ -769,8 +769,8 @@ describe('compare segments', () => { test('segment consistency - from #60', () => { const seg1 = Segment.fromRing({ x: -131.57153657554915, y: 55.01963125 }, { x: -131.571478, y: 55.0187174 }) const seg2 = Segment.fromRing({ x: -131.57153657554915, y: 55.01963125 }, { x: -131.57152375603846, y: 55.01943125 }) - expect(Segment.compare(seg1, seg2)).toBe(-1) - expect(Segment.compare(seg2, seg1)).toBe(1) + expect(Segment.compare(seg1, seg2)).toBe(1) + expect(Segment.compare(seg2, seg1)).toBe(-1) }) test('ensure transitive - part of issue 60', () => { From 99fdeb99bea05c0913c960df562b517353c938d3 Mon Sep 17 00:00:00 2001 From: Mike Fogel Date: Sun, 24 Feb 2019 13:46:16 -0300 Subject: [PATCH 02/12] Rewrite SweepEvent.compare() --- src/segment.js | 22 +------------- src/sweep-event.js | 63 +++++++--------------------------------- test/segment.test.js | 5 +--- test/sweep-event.test.js | 8 ++--- 4 files changed, 17 insertions(+), 81 deletions(-) diff --git a/src/segment.js b/src/segment.js index ccf2c98..87a8c00 100644 --- a/src/segment.js +++ b/src/segment.js @@ -2,7 +2,7 @@ import operation from './operation' import SweepEvent from './sweep-event' import { isInBbox, touchesBbox, getBboxOverlap } from './bbox' import { cmp, cmpPoints, touchPoints } from './flp' -import { closestPoint, intersection, verticalIntersection } from './vector' +import { closestPoint, intersection } from './vector' // Give segments unique ID's to get consistent sorting of // segments and sweep events when all else is identical @@ -183,10 +183,6 @@ export default class Segment { } } - isVertical () { - return cmp(this.leftSE.point.x, this.rightSE.point.x) === 0 - } - isAnEndpoint (point) { return ( cmpPoints(point, this.leftSE.point) === 0 || @@ -218,22 +214,6 @@ export default class Segment { return 0 } - /* Compare point vertically with segment. - * 1: point is below segment - * 0: segment appears to be vertical - * -1: point is above segment */ - compareVertically (point) { - if (this.isAnEndpoint(point)) return 0 - const interPt = verticalIntersection(this.leftSE.point, this.vector(), point.x) - - // Trying to be as exact as possible here, hence not using flp comparisons - if (interPt !== null) { - if (point.y < interPt.y) return -1 - if (point.y > interPt.y) return 1 - } - return 0 - } - /* Does the point in question touch the given segment? * Greedy - essentially a 2 * Number.EPSILON comparison. * If it's not possible to add an independent point between the diff --git a/src/sweep-event.js b/src/sweep-event.js index d207de2..4727d27 100644 --- a/src/sweep-event.js +++ b/src/sweep-event.js @@ -1,68 +1,27 @@ import { cmp } from './flp' +import Segment from './segment' import { cosineOfAngle, sineOfAngle } from './vector' export default class SweepEvent { static compare (a, b) { - // if the events are already linked, then we know the points are equal - if (a.point !== b.point) { + // favor event with a point that the sweep line hits first + const cmpX = cmp(a.point.x, b.point.x) + if (cmpX !== 0) return cmpX - // favor event with a point that the sweep line hits first - const cmpX = cmp(a.point.x, b.point.x) - if (cmpX !== 0) return cmpX + const cmpY = cmp(a.point.y, b.point.y) + if (cmpY !== 0) return cmpY - const cmpY = cmp(a.point.y, b.point.y) - if (cmpY !== 0) return cmpY - - // Points are equal, so go ahead and link these events. - a.link(b) - } + // the points are the same, so link them if needed + if (a.point !== b.point) a.link(b) // favor right events over left if (a.isLeft !== b.isLeft) return a.isLeft ? 1 : -1 - // are they identical? - if (a === b) return 0 - - // The calcuations of relative segment angle below can give different - // results after segment splitting due to rounding errors. - // To maintain sweep event queue ordering, we thus skip these calculations - // if we already know the segements to be colinear (one of the requirements - // of the 'consumedBy' relationship). - let aConsumedBy = a - let bConsumedBy = b - while (aConsumedBy.consumedBy) aConsumedBy = aConsumedBy.consumedBy - while (bConsumedBy.consumedBy) bConsumedBy = bConsumedBy.consumedBy - if (aConsumedBy !== bConsumedBy) { - - // favor vertical segments for left events, and non-vertical for right - // https://github.com/mfogel/polygon-clipping/issues/29 - const aVert = a.segment.isVertical() - const bVert = b.segment.isVertical() - if (aVert && ! bVert) return a.isLeft ? 1 : -1 - if (! aVert && bVert) return a.isLeft ? -1 : 1 - - // Favor events where the line segment is lower. - // Sometimes, because one segment is longer than the other, - // one of these comparisons will return 0 and the other won't. - const pointSegCmp = a.segment.compareVertically(b.otherSE.point) - if (pointSegCmp === 1) return -1 - if (pointSegCmp === -1) return 1 - const otherPointSegCmp = b.segment.compareVertically(a.otherSE.point) - if (otherPointSegCmp !== 0) return otherPointSegCmp - - // NOTE: We don't sort on segment length because that changes - // as segments are divided. - } - - // as a tie-breaker, favor lower segment creation id - if (a.segment.id < b.segment.id) return -1 - if (a.segment.id > b.segment.id) return 1 - - throw new Error( - `SweepEvent comparison failed at [${a.point.x}, ${a.point.y}]` - ) + // we have two matching left or right endpoints + // ordering of this case is the same as for their segments + return Segment.compare(a.segment, b.segment) } // Warning: 'point' input will be modified and re-used (for performance) diff --git a/test/segment.test.js b/test/segment.test.js index eb07141..18a4d26 100644 --- a/test/segment.test.js +++ b/test/segment.test.js @@ -109,26 +109,23 @@ describe('split', () => { }) }) -describe('simple properties - bbox, vector, points, isVertical', () => { +describe('simple properties - bbox, vector', () => { test('general', () => { const seg = Segment.fromRing({ x: 1, y: 2 }, { x: 3, y: 4 }) expect(seg.bbox()).toEqual({ ll: { x: 1, y: 2 }, ur: { x: 3, y: 4 } }) expect(seg.vector()).toEqual({ x: 2, y: 2 }) - expect(seg.isVertical()).toBeFalsy() }) test('horizontal', () => { const seg = Segment.fromRing({ x: 1, y: 4 }, { x: 3, y: 4 }) expect(seg.bbox()).toEqual({ ll: { x: 1, y: 4 }, ur: { x: 3, y: 4 } }) expect(seg.vector()).toEqual({ x: 2, y: 0 }) - expect(seg.isVertical()).toBeFalsy() }) test('vertical', () => { const seg = Segment.fromRing({ x: 3, y: 2 }, { x: 3, y: 4 }) expect(seg.bbox()).toEqual({ ll: { x: 3, y: 2 }, ur: { x: 3, y: 4 } }) expect(seg.vector()).toEqual({ x: 0, y: 2 }) - expect(seg.isVertical()).toBeTruthy() }) }) diff --git a/test/sweep-event.test.js b/test/sweep-event.test.js index 87e1758..ea96df1 100644 --- a/test/sweep-event.test.js +++ b/test/sweep-event.test.js @@ -53,8 +53,8 @@ describe('sweep event compare', () => { test('and favor barely lower segment', () => { const seg1 = Segment.fromRing({ x: -75.725, y: 45.357 }, { x: -75.72484615384616, y: 45.35723076923077 }) const seg2 = Segment.fromRing({ x: -75.725, y: 45.357 }, { x: -75.723, y: 45.36 }) - expect(SweepEvent.compare(seg1.leftSE, seg2.leftSE)).toBe(1) - expect(SweepEvent.compare(seg2.leftSE, seg1.leftSE)).toBe(-1) + expect(SweepEvent.compare(seg1.leftSE, seg2.leftSE)).toBe(-1) + expect(SweepEvent.compare(seg2.leftSE, seg1.leftSE)).toBe(1) }) test('then favor lower ring id', () => { @@ -97,8 +97,8 @@ describe('sweep event compare', () => { // harvested from https://github.com/mfogel/polygon-clipping/issues/62 const seg1 = Segment.fromRing({ x: -71.0390933353125, y: 41.504475 }, { x: -71.0389879, y: 41.5037842 }) const seg2 = Segment.fromRing({ x: -71.0390933353125, y: 41.504475 }, { x: -71.03906280974431, y: 41.504275 }) - expect(SweepEvent.compare(seg1.leftSE, seg2.leftSE)).toBe(-1) - expect(SweepEvent.compare(seg2.leftSE, seg1.leftSE)).toBe(1) + expect(SweepEvent.compare(seg1.leftSE, seg2.leftSE)).toBe(1) + expect(SweepEvent.compare(seg2.leftSE, seg1.leftSE)).toBe(-1) }) }) From ab4cf5d7bec39dc87f3c6efaa67d4559f13789e5 Mon Sep 17 00:00:00 2001 From: Mike Fogel Date: Sun, 24 Feb 2019 16:49:22 -0300 Subject: [PATCH 03/12] Rounder() module --- src/rounder.js | 56 ++++++++++++++++++++++++++++++++++++++++++++ test/rounder.test.js | 43 ++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+) create mode 100644 src/rounder.js create mode 100644 test/rounder.test.js diff --git a/src/rounder.js b/src/rounder.js new file mode 100644 index 0000000..8a17dfc --- /dev/null +++ b/src/rounder.js @@ -0,0 +1,56 @@ +import { cmp } from './flp' +import SplayTree from 'splaytree' + +/** + * This class rounds incoming values sufficiently so that + * floating points problems are, for the most part, avoided. + * + * Incoming points are have their x & y values tested against + * all previously seen x & y values. If either is 'too close' + * to a previously seen value, it's value is 'snapped' to the + * previously seen value. + * + * All points should be rounded by this class before being + * stored in any data structures in the rest of this algorithm. + */ + +class Rounder { + constructor () { + this.reset() + } + + reset () { + this.xVals = new SplayTree() + this.yVals = new SplayTree() + } + + round (x, y) { + return { + x: this.roundCoord(x, this.xVals), + y: this.roundCoord(y, this.yVals), + } + } + + roundCoord (coord, tree) { + const node = tree.add(coord) + + const prevNode = tree.prev(node) + if (prevNode !== null && cmp(node.key, prevNode.key) === 0) { + tree.remove(coord) + return prevNode.key + } + + const nextNode = tree.next(node) + if (nextNode !== null && cmp(node.key, nextNode.key) === 0) { + tree.remove(coord) + return nextNode.key + } + + return coord + } +} + +// singleton available by import +const rounder = new Rounder() + +export default rounder diff --git a/test/rounder.test.js b/test/rounder.test.js new file mode 100644 index 0000000..c2b5a00 --- /dev/null +++ b/test/rounder.test.js @@ -0,0 +1,43 @@ +/* eslint-env jest */ + +import rounder from '../src/rounder' + +describe('rounder.round()', () => { + test('no overlap', () => { + rounder.reset() + const pt1 = {x: 3, y: 4} + const pt2 = {x: 4, y: 5} + const pt3 = {x: 5, y: 5} + expect(rounder.round(pt1.x, pt1.y)).toEqual(pt1) + expect(rounder.round(pt2.x, pt2.y)).toEqual(pt2) + expect(rounder.round(pt3.x, pt3.y)).toEqual(pt3) + }) + + test('exact overlap', () => { + rounder.reset() + const pt1 = {x: 3, y: 4} + const pt2 = {x: 4, y: 5} + const pt3 = {x: 3, y: 4} + expect(rounder.round(pt1.x, pt1.y)).toEqual(pt1) + expect(rounder.round(pt2.x, pt2.y)).toEqual(pt2) + expect(rounder.round(pt3.x, pt3.y)).toEqual(pt3) + }) + + test('rounding one coordinate', () => { + rounder.reset() + const pt1 = {x: 3, y: 4} + const pt2 = {x: 3 + Number.EPSILON, y: 4} + const pt3 = {x: 3, y: 4 + Number.EPSILON} + expect(rounder.round(pt1.x, pt1.y)).toEqual(pt1) + expect(rounder.round(pt2.x, pt2.y)).toEqual(pt1) + expect(rounder.round(pt3.x, pt3.y)).toEqual(pt1) + }) + + test('rounding both coordinates', () => { + rounder.reset() + const pt1 = {x: 3, y: 4} + const pt2 = {x: 3 + Number.EPSILON, y: 4 + Number.EPSILON} + expect(rounder.round(pt1.x, pt1.y)).toEqual(pt1) + expect(rounder.round(pt2.x, pt2.y)).toEqual(pt1) + }) +}) From c50eda10ac9be5415d7baad9272a1ed4c2c8e63b Mon Sep 17 00:00:00 2001 From: Mike Fogel Date: Sun, 24 Feb 2019 17:21:11 -0300 Subject: [PATCH 04/12] Use the rounder --- src/clean-input.js | 5 +++-- src/operation.js | 4 +++- src/segment.js | 6 ++--- test/end-to-end/issue-37/intersection.geojson | 6 ++--- test/end-to-end/issue-60-7/union.geojson | 4 ++-- .../thin-vertical-triangle/args.geojson | 22 +++++++++++++++++++ .../thin-vertical-triangle/union.geojson | 8 +++++++ 7 files changed, 44 insertions(+), 11 deletions(-) create mode 100644 test/end-to-end/thin-vertical-triangle/args.geojson create mode 100644 test/end-to-end/thin-vertical-triangle/union.geojson diff --git a/src/clean-input.js b/src/clean-input.js index ea74c78..9b342a2 100644 --- a/src/clean-input.js +++ b/src/clean-input.js @@ -1,5 +1,6 @@ import { cmpPoints } from './flp' import { compareVectorAngles } from './vector' +import rounder from './rounder' /* Given input geometry as a standard array-of-arrays geojson-style * geometry, return one that uses objects as points, for better perf */ @@ -30,7 +31,7 @@ export const pointsAsObjects = geom => { 'Only 2-dimensional polygons supported.' ) } - output[i][j].push({ x: geom[i][j][k][0], y: geom[i][j][k][1] }) + output[i][j].push(rounder.round(geom[i][j][k][0], geom[i][j][k][1])) } } else { // polygon if (geom[i][j].length < 2) { @@ -42,7 +43,7 @@ export const pointsAsObjects = geom => { 'Only 2-dimensional polygons supported.' ) } - output[i].push({ x: geom[i][j][0], y: geom[i][j][1] }) + output[i].push(rounder.round(geom[i][j][0], geom[i][j][1])) } } } diff --git a/src/operation.js b/src/operation.js index 100c78f..d56550e 100644 --- a/src/operation.js +++ b/src/operation.js @@ -2,14 +2,16 @@ import SplayTree from 'splaytree' import * as cleanInput from './clean-input' import * as geomIn from './geom-in' import * as geomOut from './geom-out' +import rounder from './rounder' import SweepEvent from './sweep-event' import SweepLine from './sweep-line' export class Operation { run (type, geom, moreGeoms) { operation.type = type + rounder.reset() - /* Make a copy of the input geometry with points as objects, for perf */ + /* Make a copy of the input geometry with rounded points as objects */ const geoms = [cleanInput.pointsAsObjects(geom)] for (let i = 0, iMax = moreGeoms.length; i < iMax; i++) { geoms.push(cleanInput.pointsAsObjects(moreGeoms[i])) diff --git a/src/segment.js b/src/segment.js index 87a8c00..cfd2b5e 100644 --- a/src/segment.js +++ b/src/segment.js @@ -3,6 +3,7 @@ import SweepEvent from './sweep-event' import { isInBbox, touchesBbox, getBboxOverlap } from './bbox' import { cmp, cmpPoints, touchPoints } from './flp' import { closestPoint, intersection } from './vector' +import rounder from './rounder' // Give segments unique ID's to get consistent sorting of // segments and sweep events when all else is identical @@ -305,9 +306,8 @@ export default class Segment { // is the intersection found between the lines not on the segments? if (!isInBbox(bboxOverlap, pt)) return null - // We don't need to check if we need to 'snap' to an endpoint, - // because the endpoint cmps we did eariler were greedy - return pt + // round the the computed point if needed + return rounder.round(pt.x, pt.y) } /** diff --git a/test/end-to-end/issue-37/intersection.geojson b/test/end-to-end/issue-37/intersection.geojson index e343cff..07f5c8a 100644 --- a/test/end-to-end/issue-37/intersection.geojson +++ b/test/end-to-end/issue-37/intersection.geojson @@ -7,10 +7,10 @@ "coordinates": [ [ [ - [0.523985, 51.281651], + [0.5239850000000027, 51.281651], [0.5241, 51.2816], - [0.5240213684210527, 51.28168736842105], - [0.523985, 51.281651] + [0.5240213684210545, 51.28168736842105], + [0.5239850000000027, 51.281651] ] ] ] diff --git a/test/end-to-end/issue-60-7/union.geojson b/test/end-to-end/issue-60-7/union.geojson index 8e1cf03..ae32bb6 100644 --- a/test/end-to-end/issue-60-7/union.geojson +++ b/test/end-to-end/issue-60-7/union.geojson @@ -14,10 +14,10 @@ ], [ [ - [-10.000000000000016, 1.44], + [-10.000000000000018, 1.44], [-9, 1.5], [-10.00000000000001, 1.75], - [-10.000000000000016, 1.44] + [-10.000000000000018, 1.44] ] ] ] diff --git a/test/end-to-end/thin-vertical-triangle/args.geojson b/test/end-to-end/thin-vertical-triangle/args.geojson new file mode 100644 index 0000000..49049c0 --- /dev/null +++ b/test/end-to-end/thin-vertical-triangle/args.geojson @@ -0,0 +1,22 @@ +{ + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "properties": null, + "geometry": { + "type": "MultiPolygon", + "coordinates": [ + [ + [ + [-10.000000000000005, -6], + [-10.000000000000007, -5], + [-10.000000000000009, -7], + [-10.000000000000005, -6] + ] + ] + ] + } + } + ] +} diff --git a/test/end-to-end/thin-vertical-triangle/union.geojson b/test/end-to-end/thin-vertical-triangle/union.geojson new file mode 100644 index 0000000..db4825c --- /dev/null +++ b/test/end-to-end/thin-vertical-triangle/union.geojson @@ -0,0 +1,8 @@ +{ + "type": "Feature", + "properties": null, + "geometry": { + "type": "MultiPolygon", + "coordinates": [] + } +} From 1b43cdacc803c5919486a0b627ecf053fada5693 Mon Sep 17 00:00:00 2001 From: Mike Fogel Date: Sun, 24 Feb 2019 18:04:16 -0300 Subject: [PATCH 05/12] Remove most of the use of cmp() No need to do fuzzy comparisons here anymore --- src/bbox.js | 26 +++++++++++++------------- src/clean-input.js | 9 +++++---- src/segment.js | 44 +++++++++++++++++++++++++++++--------------- src/sweep-event.js | 8 ++++---- test/bbox.test.js | 6 +++--- 5 files changed, 54 insertions(+), 39 deletions(-) diff --git a/src/bbox.js b/src/bbox.js index f79fff0..4cadbdb 100644 --- a/src/bbox.js +++ b/src/bbox.js @@ -1,4 +1,4 @@ -import { cmp, touch } from './flp' +import { touch } from './flp' /** * A bounding box has the format: @@ -9,10 +9,10 @@ import { cmp, touch } from './flp' export const isInBbox = (bbox, point) => { return ( - cmp(bbox.ll.x, point.x) <= 0 && - cmp(point.x, bbox.ur.x) <= 0 && - cmp(bbox.ll.y, point.y) <= 0 && - cmp(point.y, bbox.ur.y) <= 0 + (bbox.ll.x <= point.x) && + (point.x <= bbox.ur.x) && + (bbox.ll.y <= point.y) && + (point.y <= bbox.ur.y) ) } @@ -22,10 +22,10 @@ export const isInBbox = (bbox, point) => { * - it 'touches' one of the sides (another greedy comparison) */ export const touchesBbox = (bbox, point) => { return ( - (cmp(bbox.ll.x, point.x) <= 0 || touch(bbox.ll.x, point.x)) && - (cmp(point.x, bbox.ur.x) <= 0 || touch(point.x, bbox.ur.x)) && - (cmp(bbox.ll.y, point.y) <= 0 || touch(bbox.ll.y, point.y)) && - (cmp(point.y, bbox.ur.y) <= 0 || touch(point.y, bbox.ur.y)) + ((bbox.ll.x <= point.x) || touch(bbox.ll.x, point.x)) && + ((point.x <= bbox.ur.x) || touch(point.x, bbox.ur.x)) && + ((bbox.ll.y <= point.y) || touch(bbox.ll.y, point.y)) && + ((point.y <= bbox.ur.y) || touch(point.y, bbox.ur.y)) ) } @@ -35,10 +35,10 @@ export const touchesBbox = (bbox, point) => { export const getBboxOverlap = (b1, b2) => { // check if the bboxes overlap at all if ( - cmp(b2.ur.x, b1.ll.x) < 0 || - cmp(b1.ur.x, b2.ll.x) < 0 || - cmp(b2.ur.y, b1.ll.y) < 0 || - cmp(b1.ur.y, b2.ll.y) < 0 + b2.ur.x < b1.ll.x || + b1.ur.x < b2.ll.x || + b2.ur.y < b1.ll.y || + b1.ur.y < b2.ll.y ) return null // find the middle two X values diff --git a/src/clean-input.js b/src/clean-input.js index 9b342a2..cf737d4 100644 --- a/src/clean-input.js +++ b/src/clean-input.js @@ -1,4 +1,3 @@ -import { cmpPoints } from './flp' import { compareVectorAngles } from './vector' import rounder from './rounder' @@ -117,11 +116,13 @@ export const cleanMultiPoly = multipoly => { * WARN: input modified directly */ export const cleanRing = ring => { if (ring.length === 0) return - if (cmpPoints(ring[0], ring[ring.length - 1]) === 0) ring.pop() + const firstPt = ring[0] + const lastPt = ring[ring.length - 1] + if (firstPt.x === lastPt.x && firstPt.y === lastPt.y) ring.pop() const isPointUncessary = (prevPt, pt, nextPt) => - cmpPoints(prevPt, pt) === 0 || - cmpPoints(pt, nextPt) === 0 || + (prevPt.x === pt.x && prevPt.y === pt.y) || + (nextPt.x === pt.x && nextPt.y === pt.y) || compareVectorAngles(pt, prevPt, nextPt) === 0 let i = 0 diff --git a/src/segment.js b/src/segment.js index cfd2b5e..4f98a85 100644 --- a/src/segment.js +++ b/src/segment.js @@ -142,20 +142,34 @@ export default class Segment { // this.ringOut, this.consumedBy, this.prev } - static fromRing(point1, point2, ring) { - let leftSE, rightSE - const ptCmp = cmpPoints(point1, point2) - if (ptCmp < 0) { - leftSE = new SweepEvent(point1, true) - rightSE = new SweepEvent(point2, false) - } else if (ptCmp > 0) { - leftSE = new SweepEvent(point2, true) - rightSE = new SweepEvent(point1, false) - } else { - throw new Error( - `Tried to create degenerate segment at [${point1.x}, ${point2.y}]` + static fromRing(pt1, pt2, ring) { + let leftPt, rightPt + + // ordering the two points according to sweep line ordering + // TODO: should probably break this logic out into it's own func + if (pt1.x < pt2.x) { + leftPt = pt1 + rightPt = pt2 + } + else if (pt1.x > pt2.x) { + leftPt = pt2 + rightPt = pt1 + } + else { // pt1.x === pt2.x + if (pt1.y < pt2.y) { + leftPt = pt1 + rightPt = pt2 + } + else if (pt1.y > pt2.y) { + leftPt = pt2 + rightPt = pt1 + } + else throw new Error( + `Tried to create degenerate segment at [${pt1.x}, ${pt1.y}]` ) } + const leftSE = new SweepEvent(leftPt, true) + const rightSE = new SweepEvent(rightPt, false) return new Segment(leftSE, rightSE, [ring]) } @@ -184,10 +198,10 @@ export default class Segment { } } - isAnEndpoint (point) { + isAnEndpoint (pt) { return ( - cmpPoints(point, this.leftSE.point) === 0 || - cmpPoints(point, this.rightSE.point) === 0 + (pt.x === this.leftSE.point.x && pt.y === this.leftSE.point.y) || + (pt.x === this.rightSE.point.x && pt.y === this.rightSE.point.y) ) } diff --git a/src/sweep-event.js b/src/sweep-event.js index 4727d27..4bf1c49 100644 --- a/src/sweep-event.js +++ b/src/sweep-event.js @@ -7,11 +7,11 @@ export default class SweepEvent { static compare (a, b) { // favor event with a point that the sweep line hits first - const cmpX = cmp(a.point.x, b.point.x) - if (cmpX !== 0) return cmpX + if (a.point.x < b.point.x) return -1 + if (a.point.x > b.point.x) return 1 - const cmpY = cmp(a.point.y, b.point.y) - if (cmpY !== 0) return cmpY + if (a.point.y < b.point.y) return -1 + if (a.point.y > b.point.y) return 1 // the points are the same, so link them if needed if (a.point !== b.point) a.link(b) diff --git a/test/bbox.test.js b/test/bbox.test.js index a62a2be..0364d98 100644 --- a/test/bbox.test.js +++ b/test/bbox.test.js @@ -26,9 +26,9 @@ describe('is in bbox', () => { test('barely inside & outside', () => { const bbox = { ll: { x: 1, y: 0.8 }, ur: { x: 1.2, y: 6 } } - expect(isInBbox(bbox, { x: 1.2 + Number.EPSILON, y: 6 })).toBeTruthy() - expect(isInBbox(bbox, { x: 1.2 + 2 * Number.EPSILON, y: 6 })).toBeFalsy() - expect(isInBbox(bbox, { x: 1, y: 0.8 - Number.EPSILON / 2 })).toBeTruthy() + expect(isInBbox(bbox, { x: 1.2 - Number.EPSILON, y: 6 })).toBeTruthy() + expect(isInBbox(bbox, { x: 1.2 + Number.EPSILON, y: 6 })).toBeFalsy() + expect(isInBbox(bbox, { x: 1, y: 0.8 + Number.EPSILON })).toBeTruthy() expect(isInBbox(bbox, { x: 1, y: 0.8 - Number.EPSILON })).toBeFalsy() }) }) From 6b186f743a7eaedc869dd4ebc56814c350433164 Mon Sep 17 00:00:00 2001 From: Mike Fogel Date: Sun, 24 Feb 2019 18:14:37 -0300 Subject: [PATCH 06/12] Make segment.comparePoint() more exact --- src/segment.js | 22 ++++++++++++---------- test/segment.test.js | 4 ++-- test/sweep-event.test.js | 8 ++++---- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/segment.js b/src/segment.js index 4f98a85..2955200 100644 --- a/src/segment.js +++ b/src/segment.js @@ -1,7 +1,7 @@ import operation from './operation' import SweepEvent from './sweep-event' import { isInBbox, touchesBbox, getBboxOverlap } from './bbox' -import { cmp, cmpPoints, touchPoints } from './flp' +import { cmpPoints, touchPoints } from './flp' import { closestPoint, intersection } from './vector' import rounder from './rounder' @@ -213,19 +213,21 @@ export default class Segment { if (this.isAnEndpoint(point)) return 0 const interPt = closestPoint(this.leftSE.point, this.rightSE.point, point) - const cmpY = cmp(point.y, interPt.y) - if (cmpY !== 0) return cmpY - - const cmpX = cmp(point.x, interPt.x) - const segCmpX = cmp(this.leftSE.point.y, this.rightSE.point.y) + if (point.y < interPt.y) return -1 + if (point.y > interPt.y) return 1 // depending on if our segment angles up or down, // the x coord comparison means oppposite things - if (cmpX > 0) return segCmpX - if (cmpX < 0) { - if (segCmpX > 0) return -1 - if (segCmpX < 0) return 1 + if (point.x < interPt.x) { + if (this.leftSE.point.y < this.rightSE.point.y) return 1 + if (this.leftSE.point.y > this.rightSE.point.y) return -1 + } + if (point.x > interPt.x) { + if (this.leftSE.point.y < this.rightSE.point.y) return -1 + if (this.leftSE.point.y > this.rightSE.point.y) return 1 } + + // on the line return 0 } diff --git a/test/segment.test.js b/test/segment.test.js index 18a4d26..92a6a37 100644 --- a/test/segment.test.js +++ b/test/segment.test.js @@ -766,8 +766,8 @@ describe('compare segments', () => { test('segment consistency - from #60', () => { const seg1 = Segment.fromRing({ x: -131.57153657554915, y: 55.01963125 }, { x: -131.571478, y: 55.0187174 }) const seg2 = Segment.fromRing({ x: -131.57153657554915, y: 55.01963125 }, { x: -131.57152375603846, y: 55.01943125 }) - expect(Segment.compare(seg1, seg2)).toBe(1) - expect(Segment.compare(seg2, seg1)).toBe(-1) + expect(Segment.compare(seg1, seg2)).toBe(-1) + expect(Segment.compare(seg2, seg1)).toBe(1) }) test('ensure transitive - part of issue 60', () => { diff --git a/test/sweep-event.test.js b/test/sweep-event.test.js index ea96df1..87e1758 100644 --- a/test/sweep-event.test.js +++ b/test/sweep-event.test.js @@ -53,8 +53,8 @@ describe('sweep event compare', () => { test('and favor barely lower segment', () => { const seg1 = Segment.fromRing({ x: -75.725, y: 45.357 }, { x: -75.72484615384616, y: 45.35723076923077 }) const seg2 = Segment.fromRing({ x: -75.725, y: 45.357 }, { x: -75.723, y: 45.36 }) - expect(SweepEvent.compare(seg1.leftSE, seg2.leftSE)).toBe(-1) - expect(SweepEvent.compare(seg2.leftSE, seg1.leftSE)).toBe(1) + expect(SweepEvent.compare(seg1.leftSE, seg2.leftSE)).toBe(1) + expect(SweepEvent.compare(seg2.leftSE, seg1.leftSE)).toBe(-1) }) test('then favor lower ring id', () => { @@ -97,8 +97,8 @@ describe('sweep event compare', () => { // harvested from https://github.com/mfogel/polygon-clipping/issues/62 const seg1 = Segment.fromRing({ x: -71.0390933353125, y: 41.504475 }, { x: -71.0389879, y: 41.5037842 }) const seg2 = Segment.fromRing({ x: -71.0390933353125, y: 41.504475 }, { x: -71.03906280974431, y: 41.504275 }) - expect(SweepEvent.compare(seg1.leftSE, seg2.leftSE)).toBe(1) - expect(SweepEvent.compare(seg2.leftSE, seg1.leftSE)).toBe(-1) + expect(SweepEvent.compare(seg1.leftSE, seg2.leftSE)).toBe(-1) + expect(SweepEvent.compare(seg2.leftSE, seg1.leftSE)).toBe(1) }) }) From 504f46e178ad82a435e3ebec39003885edd2b0d1 Mon Sep 17 00:00:00 2001 From: Mike Fogel Date: Sun, 24 Feb 2019 18:26:45 -0300 Subject: [PATCH 07/12] Exact angle comparison when building result --- src/sweep-event.js | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/sweep-event.js b/src/sweep-event.js index 4bf1c49..2e8abcc 100644 --- a/src/sweep-event.js +++ b/src/sweep-event.js @@ -1,4 +1,3 @@ -import { cmp } from './flp' import Segment from './segment' import { cosineOfAngle, sineOfAngle } from './vector' @@ -108,12 +107,24 @@ export default class SweepEvent { const { sine: asine, cosine: acosine } = cache.get(a) const { sine: bsine, cosine: bcosine } = cache.get(b) - const cmpZeroASine = cmp(asine, 0) - const cmpZeroBSine = cmp(bsine, 0) + // both on or above x-axis + if (asine >= 0 && bsine >= 0) { + if (acosine < bcosine) return 1 + if (acosine > bcosine) return -1 + return 0 + } + + // both below x-axis + if (asine < 0 && bsine < 0) { + if (acosine < bcosine) return -1 + if (acosine > bcosine) return 1 + return 0 + } - if (cmpZeroASine >= 0 && cmpZeroBSine >= 0) return cmp(bcosine, acosine) - if (cmpZeroASine < 0 && cmpZeroBSine < 0) return cmp(acosine, bcosine) - return cmp(bsine, asine) + // one above x-axis, one below + if (bsine < asine) return -1 + if (bsine > asine) return 1 + return 0 } } } From 5fe5b73e395317894af2622ed5321a5ead30b70f Mon Sep 17 00:00:00 2001 From: Mike Fogel Date: Mon, 25 Feb 2019 13:53:39 -0300 Subject: [PATCH 08/12] Streamline rounder, pressed with 0 --- src/rounder.js | 39 +++++++++++++++++++++++++++------------ test/rounder.test.js | 8 ++++++++ 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/src/rounder.js b/src/rounder.js index 8a17dfc..9f2bf60 100644 --- a/src/rounder.js +++ b/src/rounder.js @@ -14,35 +14,50 @@ import SplayTree from 'splaytree' * stored in any data structures in the rest of this algorithm. */ -class Rounder { +class PtRounder { constructor () { this.reset() } reset () { - this.xVals = new SplayTree() - this.yVals = new SplayTree() + this.xRounder = new CoordRounder() + this.yRounder = new CoordRounder() } round (x, y) { return { - x: this.roundCoord(x, this.xVals), - y: this.roundCoord(y, this.yVals), + x: this.xRounder.round(x), + y: this.yRounder.round(y), } } +} + +class CoordRounder { + constructor () { + this.tree = new SplayTree() + // preseed with 0 so we don't end up with values < Number.EPSILON + this.round(0) + } - roundCoord (coord, tree) { - const node = tree.add(coord) + // Note: this can rounds input values backwards or forwards. + // You might ask, why not restrict this to just rounding + // forwards? Wouldn't that allow left endpoints to always + // remain left endpoints during splitting (never change to + // right). No - it wouldn't, because we snap intersections + // to endpoints (to establish independence from the segment + // angle for t-intersections). + round (coord) { + const node = this.tree.add(coord) - const prevNode = tree.prev(node) + const prevNode = this.tree.prev(node) if (prevNode !== null && cmp(node.key, prevNode.key) === 0) { - tree.remove(coord) + this.tree.remove(coord) return prevNode.key } - const nextNode = tree.next(node) + const nextNode = this.tree.next(node) if (nextNode !== null && cmp(node.key, nextNode.key) === 0) { - tree.remove(coord) + this.tree.remove(coord) return nextNode.key } @@ -51,6 +66,6 @@ class Rounder { } // singleton available by import -const rounder = new Rounder() +const rounder = new PtRounder() export default rounder diff --git a/test/rounder.test.js b/test/rounder.test.js index c2b5a00..2f14a5e 100644 --- a/test/rounder.test.js +++ b/test/rounder.test.js @@ -40,4 +40,12 @@ describe('rounder.round()', () => { expect(rounder.round(pt1.x, pt1.y)).toEqual(pt1) expect(rounder.round(pt2.x, pt2.y)).toEqual(pt1) }) + + test('preseed with 0', () => { + rounder.reset() + const pt1 = { x: Number.EPSILON / 2, y: -Number.EPSILON / 2 } + expect(pt1.x).not.toEqual(0) + expect(pt1.y).not.toEqual(0) + expect(rounder.round(pt1.x, pt1.y)).toEqual({ x: 0, y: 0 }) + }) }) From 8d72b504801eb28e41be9ddee7a023fd4118c741 Mon Sep 17 00:00:00 2001 From: Mike Fogel Date: Mon, 25 Feb 2019 14:05:02 -0300 Subject: [PATCH 09/12] Eliminate a few more uneeded fuzzy comparisons --- src/segment.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/segment.js b/src/segment.js index 2955200..e66e809 100644 --- a/src/segment.js +++ b/src/segment.js @@ -291,7 +291,7 @@ export default class Segment { // does this left endpoint matches (other doesn't) if (touchesThisLSE) { // check for segments that just intersect on opposing endpoints - if (touchesOtherRSE && cmpPoints(this.leftSE.point, other.rightSE.point) === 0) return null + if (touchesOtherRSE && touchPoints(this.leftSE.point, other.rightSE.point)) return null // t-intersection on left endpoint return this.leftSE.point } @@ -299,7 +299,7 @@ export default class Segment { // does other left endpoint matches (this doesn't) if (touchesOtherLSE) { // check for segments that just intersect on opposing endpoints - if (touchesThisRSE && cmpPoints(this.rightSE.point, other.leftSE.point) === 0) return null + if (touchesThisRSE && touchPoints(this.rightSE.point, other.leftSE.point)) return null // t-intersection on left endpoint return other.leftSE.point } @@ -349,7 +349,7 @@ export default class Segment { for (let i = 0, iMax = points.length; i < iMax; i++) { const point = points[i] // skip repeated points - if (prevPoint && cmpPoints(prevPoint, point) === 0) continue + if (prevPoint && prevPoint.x === point.x && prevPoint.y === point.y) continue const alreadyLinked = point.events !== undefined const newLeftSE = new SweepEvent(point, true) From fac3719386faba8aa39822115a6e8111669cd4b4 Mon Sep 17 00:00:00 2001 From: Mike Fogel Date: Mon, 25 Feb 2019 14:25:18 -0300 Subject: [PATCH 10/12] Simplify interface of Segment.split() --- src/segment.js | 50 ++++++++++++++++---------------------------- src/sweep-line.js | 38 +++++++++++++++++++++++++-------- test/segment.test.js | 9 +++++--- 3 files changed, 53 insertions(+), 44 deletions(-) diff --git a/src/segment.js b/src/segment.js index e66e809..c52f5ae 100644 --- a/src/segment.js +++ b/src/segment.js @@ -1,7 +1,7 @@ import operation from './operation' import SweepEvent from './sweep-event' import { isInBbox, touchesBbox, getBboxOverlap } from './bbox' -import { cmpPoints, touchPoints } from './flp' +import { touchPoints } from './flp' import { closestPoint, intersection } from './vector' import rounder from './rounder' @@ -338,38 +338,24 @@ export default class Segment { * * Warning: input array of points is modified */ - split (points) { - // sort the points in sweep line order - points.sort(cmpPoints) - - let prevSeg = this - let prevPoint = null - + split (point) { const newEvents = [] - for (let i = 0, iMax = points.length; i < iMax; i++) { - const point = points[i] - // skip repeated points - if (prevPoint && prevPoint.x === point.x && prevPoint.y === point.y) continue - const alreadyLinked = point.events !== undefined - - const newLeftSE = new SweepEvent(point, true) - const newRightSE = new SweepEvent(point, false) - const oldRightSE = prevSeg.rightSE - prevSeg.replaceRightSE(newRightSE) - newEvents.push(newRightSE) - newEvents.push(newLeftSE) - - prevSeg = new Segment(newLeftSE, oldRightSE, prevSeg.ringsIn.slice()) - - // in the point we just used to create new sweep events with was already - // linked to other events, we need to check if either of the affected - // segments should be consumed - if (alreadyLinked) { - newLeftSE.checkForConsuming() - newRightSE.checkForConsuming() - } - - prevPoint = point + const alreadyLinked = point.events !== undefined + + const newLeftSE = new SweepEvent(point, true) + const newRightSE = new SweepEvent(point, false) + const oldRightSE = this.rightSE + this.replaceRightSE(newRightSE) + newEvents.push(newRightSE) + newEvents.push(newLeftSE) + new Segment(newLeftSE, oldRightSE, this.ringsIn.slice()) + + // in the point we just used to create new sweep events with was already + // linked to other events, we need to check if either of the affected + // segments should be consumed + if (alreadyLinked) { + newLeftSE.checkForConsuming() + newRightSE.checkForConsuming() } return newEvents diff --git a/src/sweep-line.js b/src/sweep-line.js index 00d45cb..bdf8fbf 100644 --- a/src/sweep-line.js +++ b/src/sweep-line.js @@ -62,15 +62,13 @@ export default class SweepLine { } if (event.isLeft) { - // TODO: would it make sense to just stop and bail out at the first time we're split? - // rather than split ourselves multiple times? - const mySplitters = [] // Check for intersections against the previous segment in the sweep line + let prevMySplitter = null if (prevSeg) { const prevInter = prevSeg.getIntersection(segment) if (prevInter !== null) { - if (!segment.isAnEndpoint(prevInter)) mySplitters.push(prevInter) + if (!segment.isAnEndpoint(prevInter)) prevMySplitter = prevInter if (!prevSeg.isAnEndpoint(prevInter)) { const newEventsFromSplit = this._splitSafely(prevSeg, prevInter) for (let i = 0, iMax = newEventsFromSplit.length; i < iMax; i++) { @@ -81,10 +79,11 @@ export default class SweepLine { } // Check for intersections against the next segment in the sweep line + let nextMySplitter = null if (nextSeg) { const nextInter = nextSeg.getIntersection(segment) if (nextInter !== null) { - if (!segment.isAnEndpoint(nextInter)) mySplitters.push(nextInter) + if (!segment.isAnEndpoint(nextInter)) nextMySplitter = nextInter if (!nextSeg.isAnEndpoint(nextInter)) { const newEventsFromSplit = this._splitSafely(nextSeg, nextInter) for (let i = 0, iMax = newEventsFromSplit.length; i < iMax; i++) { @@ -94,15 +93,36 @@ export default class SweepLine { } } - // split ourselves if need be - if (mySplitters.length > 0) { + // For simplicity, even if we find more than one intersection we only + // spilt on the 'earliest' (sweep-line style) of the intersections. + // The other intersection will be handled in a future process(). + if (prevMySplitter !== null || nextMySplitter !== null) { + + // TODO: this sweep-line ordering logic should be pulled + // out to it's own module. Overlaps with sweep event ordering + let mySplitter = null + if (prevMySplitter === null) mySplitter = nextMySplitter + else if (nextMySplitter === null) mySplitter = prevMySplitter + else { + // choose the ealiest in the sweep line pass + if (nextMySplitter.x < prevMySplitter.x) mySplitter = nextMySplitter + else if (nextMySplitter.x > prevMySplitter.x) mySplitter = prevMySplitter + else { + if (nextMySplitter.y < prevMySplitter.y) mySplitter = nextMySplitter + else if (nextMySplitter.y > prevMySplitter.y) mySplitter = prevMySplitter + else { + // the two splitters are the exact same point + mySplitter = prevMySplitter + } + } + } // Rounding errors can cause changes in ordering, // so remove afected segments and right sweep events before splitting this.queue.remove(segment.rightSE) newEvents.push(segment.rightSE) - const newEventsFromSplit = segment.split(mySplitters) + const newEventsFromSplit = segment.split(mySplitter) for (let i = 0, iMax = newEventsFromSplit.length; i < iMax; i++) { newEvents.push(newEventsFromSplit[i]) } @@ -160,7 +180,7 @@ export default class SweepLine { this.tree.remove(seg) const rightSE = seg.rightSE this.queue.remove(rightSE) - const newEvents = seg.split([pt]) + const newEvents = seg.split(pt) newEvents.push(rightSE) // splitting can trigger consumption if (seg.consumedBy === undefined) this.tree.insert(seg) diff --git a/test/segment.test.js b/test/segment.test.js index 92a6a37..f9ff996 100644 --- a/test/segment.test.js +++ b/test/segment.test.js @@ -57,7 +57,7 @@ describe('split', () => { test('on interior point', () => { const seg = Segment.fromRing({ x: 0, y: 0 }, { x: 10, y: 10 }, true) const pt = { x: 5, y: 5 } - const evts = seg.split([pt]) + const evts = seg.split(pt) expect(evts[0].segment).toBe(seg) expect(evts[0].point).toEqual(pt) expect(evts[0].isLeft).toBe(false) @@ -73,7 +73,7 @@ describe('split', () => { test('on close-to-but-not-exactly interior point', () => { const seg = Segment.fromRing({ x: 0, y: 10 }, { x: 10, y: 0 }, false) const pt = { x: 5 + Number.EPSILON, y: 5 } - const evts = seg.split([pt]) + const evts = seg.split(pt) expect(evts[0].segment).toBe(seg) expect(evts[0].point).toEqual(pt) expect(evts[0].isLeft).toBe(false) @@ -88,7 +88,10 @@ describe('split', () => { const [sPt1, sPt2, sPt3] = [{ x: 2, y: 2 }, { x: 4, y: 4 }, { x: 6, y: 6 }] const [orgLeftEvt, orgRightEvt] = [seg.leftSE, seg.rightSE] - const newEvts = seg.split([sPt3, sPt1, sPt2]) + const newEvts3 = seg.split(sPt3) + const newEvts2 = seg.split(sPt2) + const newEvts1 = seg.split(sPt1) + const newEvts = [].concat(newEvts1, newEvts2, newEvts3) expect(newEvts.length).toBe(6) From 3100bee7c5113847f6e54a1f00b245522382e28a Mon Sep 17 00:00:00 2001 From: Mike Fogel Date: Mon, 25 Feb 2019 14:51:49 -0300 Subject: [PATCH 11/12] Eliminate cmpPoints() --- src/flp.js | 57 ++++++++++++------------------------------------ src/segment.js | 2 +- test/flp.test.js | 46 +------------------------------------- 3 files changed, 16 insertions(+), 89 deletions(-) diff --git a/src/flp.js b/src/flp.js index 275215c..a9e8a77 100644 --- a/src/flp.js +++ b/src/flp.js @@ -29,47 +29,6 @@ export const cmp = (a, b) => { return a < b ? -1 : 1 } -/* FLP point comparator, favors point encountered first by sweep line */ -export const cmpPoints = (aPt, bPt) => { - if (aPt === bPt) return 0 - - // fist compare X, then compare Y - let a = aPt.x - let b = bPt.x - - // inlined version of cmp() for performance boost - if ( - a <= -epsilon || - epsilon <= a || - b <= -epsilon || - epsilon <= b - ) { - const diff = a - b - if (diff * diff >= EPSILON_SQ * a * b) { - return a < b ? -1 : 1 - } - } - - a = aPt.y - b = bPt.y - - // inlined version of cmp() for performance boost - if ( - a <= -epsilon || - epsilon <= a || - b <= -epsilon || - epsilon <= b - ) { - const diff = a - b - if (diff * diff >= EPSILON_SQ * a * b) { - return a < b ? -1 : 1 - } - } - - // they're the same - return 0 -} - /* Greedy comparison. Two numbers are defined to touch * if their midpoint is indistinguishable from either. */ export const touch = (a, b) => { @@ -80,6 +39,18 @@ export const touch = (a, b) => { /* Greedy comparison. Two points are defined to touch * if their midpoint is indistinguishable from either. */ export const touchPoints = (aPt, bPt) => { - const mPt = { x: (aPt.x + bPt.x) / 2, y: (aPt.y + bPt.y) / 2 } - return cmpPoints(mPt, aPt) === 0 || cmpPoints(mPt, bPt) === 0 + // call directly to (skip touch()) cmp() for performance boost + const mx = (aPt.x + bPt.x) / 2 + const aXMiss = cmp(mx, aPt.x) !== 0 + if (aXMiss && cmp(mx, bPt.x) !== 0) return false + + const my = (aPt.y + bPt.y) / 2 + const aYMiss = cmp(my, aPt.y) !== 0 + if (aYMiss && cmp(my, bPt.y) !== 0) return false + + // we have touching on both x & y, we have to make sure it's + // not just on opposite points thou + if (aYMiss && aYMiss) return true + if (!aYMiss && !aYMiss) return true + return false } diff --git a/src/segment.js b/src/segment.js index c52f5ae..290aa55 100644 --- a/src/segment.js +++ b/src/segment.js @@ -1,7 +1,7 @@ import operation from './operation' import SweepEvent from './sweep-event' import { isInBbox, touchesBbox, getBboxOverlap } from './bbox' -import { touchPoints } from './flp' +import { touch, touchPoints } from './flp' import { closestPoint, intersection } from './vector' import rounder from './rounder' diff --git a/test/flp.test.js b/test/flp.test.js index 1d5e5ca..4cbd7cd 100644 --- a/test/flp.test.js +++ b/test/flp.test.js @@ -1,6 +1,6 @@ /* eslint-env jest */ -import { cmp, cmpPoints, touch, touchPoints } from '../src/flp' +import { cmp, touch, touchPoints } from '../src/flp' describe('compare', () => { test('exactly equal', () => { @@ -52,50 +52,6 @@ describe('compare', () => { }) }) -describe('compare points', () => { - test('earlier X coord', () => { - const a = { x: -1, y: 1 } - const b = { x: 0, y: 0 } - expect(cmpPoints(a, b)).toBe(-1) - }) - - test('later X coord', () => { - const a = { x: 1, y: 0 } - const b = { x: 0, y: 1 } - expect(cmpPoints(a, b)).toBe(1) - }) - - test('earlier Y coord', () => { - const a = { x: 0, y: -1 } - const b = { x: 0, y: 0 } - expect(cmpPoints(a, b)).toBe(-1) - }) - - test('later Y coord', () => { - const a = { x: 0, y: 1 } - const b = { x: 0, y: 0 } - expect(cmpPoints(a, b)).toBe(1) - }) - - test('equal coord', () => { - const a = { x: 1, y: 1 } - const b = { x: 1, y: 1 } - expect(cmpPoints(a, b)).toBe(0) - }) - - test('barely equal', () => { - const a = { x: 1, y: 0 } - const b = { x: 1 + Number.EPSILON, y: 0 } - expect(cmpPoints(a, b)).toBe(0) - }) - - test('barely unequal', () => { - const a = { x: 1, y: 0 } - const b = { x: 1 - Number.EPSILON, y: 0 } - expect(cmpPoints(a, b)).toBe(1) - }) -}) - describe('touch()', () => { test('exactly equal', () => { expect(touch(1, 1)).toBe(true) From 0d6c2d715ae57e5c781af684fccfeb2ad71ac844 Mon Sep 17 00:00:00 2001 From: Mike Fogel Date: Mon, 25 Feb 2019 15:02:26 -0300 Subject: [PATCH 12/12] Consolidate sweep line point ordering logic --- src/segment.js | 25 ++++++++----------------- src/sweep-event.js | 19 ++++++++++++++----- src/sweep-line.js | 19 ++++++------------- 3 files changed, 28 insertions(+), 35 deletions(-) diff --git a/src/segment.js b/src/segment.js index 290aa55..394b3e8 100644 --- a/src/segment.js +++ b/src/segment.js @@ -1,7 +1,7 @@ import operation from './operation' import SweepEvent from './sweep-event' import { isInBbox, touchesBbox, getBboxOverlap } from './bbox' -import { touch, touchPoints } from './flp' +import { touchPoints } from './flp' import { closestPoint, intersection } from './vector' import rounder from './rounder' @@ -146,28 +146,19 @@ export default class Segment { let leftPt, rightPt // ordering the two points according to sweep line ordering - // TODO: should probably break this logic out into it's own func - if (pt1.x < pt2.x) { + const cmpPts = SweepEvent.comparePoints(pt1, pt2) + if (cmpPts < 0) { leftPt = pt1 rightPt = pt2 } - else if (pt1.x > pt2.x) { + else if (cmpPts > 0) { leftPt = pt2 rightPt = pt1 } - else { // pt1.x === pt2.x - if (pt1.y < pt2.y) { - leftPt = pt1 - rightPt = pt2 - } - else if (pt1.y > pt2.y) { - leftPt = pt2 - rightPt = pt1 - } - else throw new Error( - `Tried to create degenerate segment at [${pt1.x}, ${pt1.y}]` - ) - } + else throw new Error( + `Tried to create degenerate segment at [${pt1.x}, ${pt1.y}]` + ) + const leftSE = new SweepEvent(leftPt, true) const rightSE = new SweepEvent(rightPt, false) return new Segment(leftSE, rightSE, [ring]) diff --git a/src/sweep-event.js b/src/sweep-event.js index 2e8abcc..56d4cbd 100644 --- a/src/sweep-event.js +++ b/src/sweep-event.js @@ -3,14 +3,12 @@ import { cosineOfAngle, sineOfAngle } from './vector' export default class SweepEvent { + // for ordering sweep events in the sweep event queue static compare (a, b) { // favor event with a point that the sweep line hits first - if (a.point.x < b.point.x) return -1 - if (a.point.x > b.point.x) return 1 - - if (a.point.y < b.point.y) return -1 - if (a.point.y > b.point.y) return 1 + const ptCmp = SweepEvent.comparePoints(a.point, b.point) + if (ptCmp !== 0) return ptCmp // the points are the same, so link them if needed if (a.point !== b.point) a.link(b) @@ -23,6 +21,17 @@ export default class SweepEvent { return Segment.compare(a.segment, b.segment) } + // for ordering points in sweep line order + static comparePoints (aPt, bPt) { + if (aPt.x < bPt.x) return -1 + if (aPt.x > bPt.x) return 1 + + if (aPt.y < bPt.y) return -1 + if (aPt.y > bPt.y) return 1 + + return 0 + } + // Warning: 'point' input will be modified and re-used (for performance) constructor (point, isLeft) { if (point.events === undefined) point.events = [this] diff --git a/src/sweep-line.js b/src/sweep-line.js index bdf8fbf..4a77245 100644 --- a/src/sweep-line.js +++ b/src/sweep-line.js @@ -1,5 +1,6 @@ import SplayTree from 'splaytree' import Segment from './segment' +import SweepEvent from './sweep-event' /** * NOTE: We must be careful not to change any segments while @@ -98,23 +99,15 @@ export default class SweepLine { // The other intersection will be handled in a future process(). if (prevMySplitter !== null || nextMySplitter !== null) { - // TODO: this sweep-line ordering logic should be pulled - // out to it's own module. Overlaps with sweep event ordering let mySplitter = null if (prevMySplitter === null) mySplitter = nextMySplitter else if (nextMySplitter === null) mySplitter = prevMySplitter else { - // choose the ealiest in the sweep line pass - if (nextMySplitter.x < prevMySplitter.x) mySplitter = nextMySplitter - else if (nextMySplitter.x > prevMySplitter.x) mySplitter = prevMySplitter - else { - if (nextMySplitter.y < prevMySplitter.y) mySplitter = nextMySplitter - else if (nextMySplitter.y > prevMySplitter.y) mySplitter = prevMySplitter - else { - // the two splitters are the exact same point - mySplitter = prevMySplitter - } - } + const cmpSplitters = SweepEvent.comparePoints(prevMySplitter, nextMySplitter) + if (cmpSplitters < 0) mySplitter = prevMySplitter + if (cmpSplitters > 0) mySplitter = nextMySplitter + // the two splitters are the exact same point + mySplitter = prevMySplitter } // Rounding errors can cause changes in ordering,