From 3e70a46c1cb9d74da140cade13dd887f47ea0630 Mon Sep 17 00:00:00 2001 From: Mike Fogel Date: Thu, 28 Mar 2019 16:50:36 -0300 Subject: [PATCH 1/6] Change testsuite to assert non-zero rule --- .../all.geojson | 15 ----------- .../non-zero-rule-not-even-odd/all.geojson | 25 +++++++++++++++++++ .../args.geojson | 0 3 files changed, 25 insertions(+), 15 deletions(-) delete mode 100644 test/end-to-end/even-odd-rule-not-non-zero-winding/all.geojson create mode 100644 test/end-to-end/non-zero-rule-not-even-odd/all.geojson rename test/end-to-end/{even-odd-rule-not-non-zero-winding => non-zero-rule-not-even-odd}/args.geojson (100%) diff --git a/test/end-to-end/even-odd-rule-not-non-zero-winding/all.geojson b/test/end-to-end/even-odd-rule-not-non-zero-winding/all.geojson deleted file mode 100644 index 0f42a8e..0000000 --- a/test/end-to-end/even-odd-rule-not-non-zero-winding/all.geojson +++ /dev/null @@ -1,15 +0,0 @@ -{ - "type": "Feature", - "properties": {}, - "geometry": { - "type": "MultiPolygon", - "coordinates": [ - [[[0, 0], [1, 0], [1, 1], [0, 1], [0, 0]]], - [[[1, -1], [2, -1], [2, 0], [1, 0], [1, -1]]], - [ - [[1, 1], [2, 1], [2, 0], [4, 0], [4, 3], [1, 3], [1, 1]], - [[2, 1], [2, 2], [3, 2], [3, 1], [2, 1]] - ] - ] - } -} diff --git a/test/end-to-end/non-zero-rule-not-even-odd/all.geojson b/test/end-to-end/non-zero-rule-not-even-odd/all.geojson new file mode 100644 index 0000000..d62d13a --- /dev/null +++ b/test/end-to-end/non-zero-rule-not-even-odd/all.geojson @@ -0,0 +1,25 @@ +{ + "type": "Feature", + "properties": {}, + "geometry": { + "type": "MultiPolygon", + "coordinates": [ + [ + [ + [0, 0], + [1, 0], + [1, -1], + [2, -1], + [2, 0], + [4, 0], + [4, 3], + [1, 3], + [1, 1], + [0, 1], + [0, 0] + ], + [[2, 1], [2, 2], [3, 2], [3, 1], [2, 1]] + ] + ] + } +} diff --git a/test/end-to-end/even-odd-rule-not-non-zero-winding/args.geojson b/test/end-to-end/non-zero-rule-not-even-odd/args.geojson similarity index 100% rename from test/end-to-end/even-odd-rule-not-non-zero-winding/args.geojson rename to test/end-to-end/non-zero-rule-not-even-odd/args.geojson From 31c3e30b6e22938383c3476a6dc883089d71bf79 Mon Sep 17 00:00:00 2001 From: Mike Fogel Date: Thu, 28 Mar 2019 16:51:38 -0300 Subject: [PATCH 2/6] Update documentation --- CHANGELOG.md | 1 + README.md | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff4d2a8..9c25c98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ This project adheres to [Semantic Versioning](https://semver.org/). ## vNext (in development) + * Change winding rule from [even-odd](https://en.wikipedia.org/wiki/Even%E2%80%93odd_rule) to [non-zero](https://en.wikipedia.org/wiki/Nonzero-rule) ([#57](https://github.com/mfogel/polygon-clipping/issues/57)) * Performance improvements ([#55](https://github.com/mfogel/polygon-clipping/issues/55)) * Bug fixes (more instances of [#60](https://github.com/mfogel/polygon-clipping/issues/60)) diff --git a/README.md b/README.md index a8ec490..5c8d1ab 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ Each positional argument (``) may be either a Polygon or a MultiPolygon. T * MultiPolygons may contain touching or overlapping Polygons. * rings are not required to be self-closing. * rings may contain repeated points, which are ignored. -* rings may be self-touching and/or self-crossing. Self-crossing rings will be interpreted using the [even-odd rule](https://en.wikipedia.org/wiki/Even%E2%80%93odd_rule). +* rings may be self-touching and/or self-crossing. Self-crossing rings will be interpreted using the [non-zero rule](https://en.wikipedia.org/wiki/Nonzero-rule). * winding order of rings does not matter. * inner rings may extend outside their outer ring. The portion of inner rings outside their outer ring is dropped. * inner rings may touch or overlap each other. From 1d5578a7d2bf37cb8285e6cac11844afe5e60a5f Mon Sep 17 00:00:00 2001 From: Mike Fogel Date: Thu, 28 Mar 2019 21:34:54 -0300 Subject: [PATCH 3/6] Implementation of non-zero winding rule --- src/segment.js | 75 ++++++++++++------- .../all.geojson | 23 ++---- test/segment.test.js | 6 +- 3 files changed, 55 insertions(+), 49 deletions(-) diff --git a/src/segment.js b/src/segment.js index 596c936..aa33365 100644 --- a/src/segment.js +++ b/src/segment.js @@ -134,9 +134,9 @@ export default class Segment { return 0 } - /* Warning: a reference to ringsIn input will be stored, + /* Warning: a reference to ringWindings input will be stored, * and possibly will be later modified */ - constructor (leftSE, rightSE, ringsIn) { + constructor (leftSE, rightSE, ringWindings) { this.id = ++segmentId this.leftSE = leftSE leftSE.segment = this @@ -144,24 +144,26 @@ export default class Segment { this.rightSE = rightSE rightSE.segment = this rightSE.otherSE = leftSE - this.ringsIn = ringsIn + this.ringWindings = ringWindings this._cache = {} // left unset for performance, set later in algorithm // this.ringOut, this.consumedBy, this.prev } static fromRing(pt1, pt2, ring) { - let leftPt, rightPt + let leftPt, rightPt, winding // ordering the two points according to sweep line ordering const cmpPts = SweepEvent.comparePoints(pt1, pt2) if (cmpPts < 0) { leftPt = pt1 rightPt = pt2 + winding = 1 } else if (cmpPts > 0) { leftPt = pt2 rightPt = pt1 + winding = -1 } else throw new Error( `Tried to create degenerate segment at [${pt1.x}, ${pt1.y}]` @@ -169,7 +171,7 @@ export default class Segment { const leftSE = new SweepEvent(leftPt, true) const rightSE = new SweepEvent(rightPt, false) - return new Segment(leftSE, rightSE, [ring]) + return new Segment(leftSE, rightSE, [[ring, winding]]) } /* When a segment is split, the rightSE is replaced with a new sweep event */ @@ -372,7 +374,7 @@ export default class Segment { this.replaceRightSE(newRightSE) newEvents.push(newRightSE) newEvents.push(newLeftSE) - const newSeg = new Segment(newLeftSE, oldRightSE, this.ringsIn.slice()) + const newSeg = new Segment(newLeftSE, oldRightSE, this.ringWindings.slice()) // when splitting a nearly vertical downward-facing segment, // sometimes one of the resulting new segments is vertical, in which @@ -402,9 +404,13 @@ export default class Segment { this.leftSE = tmpEvt this.leftSE.isLeft = true this.rightSE.isLeft = false + for (let i = 0, iMax = this.ringWindings.length; i < iMax; i++) { + const pair = this.ringWindings[i] + this.ringWindings[i] = [pair[0], -1 * pair[1]] + } } - /* Consume another segment. We take their ringsIn under our wing + /* Consume another segment. We take their ringWindings under our wing * and mark them as consumed. Use for perfectly overlapping segments */ consume (other) { let consumer = this @@ -429,10 +435,10 @@ export default class Segment { consumee = tmp } - for (let i = 0, iMax = consumee.ringsIn.length; i < iMax; i++) { - consumer.ringsIn.push(consumee.ringsIn[i]) + for (let i = 0, iMax = consumee.ringWindings.length; i < iMax; i++) { + consumer.ringWindings.push(consumee.ringWindings[i]) } - consumee.ringsIn = null + consumee.ringWindings = null consumee.consumedBy = consumer // mark sweep events consumed as to maintain ordering in sweep event queue @@ -453,32 +459,29 @@ export default class Segment { return this.prev.prevInResult() } - ringsBefore () { - const key = 'ringsBefore' + ringWindingsBefore () { + const key = 'ringWindingsBefore' if (this._cache[key] === undefined) this._cache[key] = this[`_${key}`]() return this._cache[key] } - _ringsBefore () { + _ringWindingsBefore () { if (! this.prev) return [] - return (this.prev.consumedBy || this.prev).ringsAfter() + return (this.prev.consumedBy || this.prev).ringWindingsAfter() } - ringsAfter () { - const key = 'ringsAfter' + ringWindingsAfter () { + const key = 'ringWindingsAfter' if (this._cache[key] === undefined) this._cache[key] = this[`_${key}`]() return this._cache[key] } - _ringsAfter () { - const rings = this.ringsBefore().slice(0) - for (let i = 0, iMax = this.ringsIn.length; i < iMax; i++) { - const ring = this.ringsIn[i] - const index = rings.indexOf(ring) - if (index === -1) rings.push(ring) - else rings.splice(index, 1) + _ringWindingsAfter () { + const ringWindingsAfter = this.ringWindingsBefore().slice(0) + for (let i = 0, iMax = this.ringWindings.length; i < iMax; i++) { + ringWindingsAfter.push(this.ringWindings[i]) } - return rings + return ringWindingsAfter } multiPolysBefore () { @@ -499,12 +502,28 @@ export default class Segment { } _multiPolysAfter () { - // first calcualte our polysAfter + // consolidate down the ringWindings we've accumulated + const rings = [] + const windings = [] + const ringWindingsAfter = this.ringWindingsAfter() + for (let i = 0, iMax = ringWindingsAfter.length; i < iMax; i++) { + const ring = ringWindingsAfter[i][0] + const winding = ringWindingsAfter[i][1] + const index = rings.indexOf(ring) + if (index === -1) { + rings.push(ring) + windings.push(winding) + } + else { + windings[index] += winding + } + } + // calcualte our polysAfter const polysAfter = [] const polysExclude = [] - const ringsAfter = this.ringsAfter() - for (let i = 0, iMax = ringsAfter.length; i < iMax; i++) { - const ring = ringsAfter[i] + for (let i = 0, iMax = rings.length; i < iMax; i++) { + if (windings[i] === 0) continue // non-zero rule + const ring = rings[i] const poly = ring.poly if (polysExclude.indexOf(poly) !== -1) continue if (ring.isExterior) polysAfter.push(poly) diff --git a/test/end-to-end/multipoly-with-self-crossing-rings/all.geojson b/test/end-to-end/multipoly-with-self-crossing-rings/all.geojson index 2f798e7..7436961 100644 --- a/test/end-to-end/multipoly-with-self-crossing-rings/all.geojson +++ b/test/end-to-end/multipoly-with-self-crossing-rings/all.geojson @@ -4,27 +4,14 @@ "geometry": { "type": "MultiPolygon", "coordinates": [ - [ - [[0, 0], [4, 0], [2, 2], [4, 4], [0, 4], [0, 0]], - [[2, 2], [1, 1], [1, 3], [2, 2]] - ], - [ - [[0, 10], [4, 10], [2, 12], [4, 14], [0, 14], [0, 10]], - [[2, 12], [1, 11], [1, 13], [2, 12]] - ], - [ - [[10, 0], [14, 0], [12, 2], [14, 4], [10, 4], [10, 0]], - [[12, 2], [11, 1], [11, 3], [12, 2]] - ], - [ - [[10, 10], [14, 10], [14, 14], [10, 10]], - [[10, 10], [12, 11], [13, 11], [10, 10]] - ], + [[[0, 0], [4, 0], [2, 2], [4, 4], [0, 4], [0, 0]]], + [[[0, 10], [4, 10], [2, 12], [4, 14], [0, 14], [0, 10]]], + [[[10, 0], [14, 0], [12, 2], [14, 4], [10, 4], [10, 0]]], + [[[10, 10], [14, 10], [14, 14], [10, 10]]], [ [[20, 0], [26, 0], [26, 6], [20, 6], [20, 0]], [[21, 1], [21, 5], [25, 5], [23, 3], [25, 1], [21, 1]] - ], - [[[23, 3], [22, 4], [22, 2], [23, 3]]] + ] ] } } diff --git a/test/segment.test.js b/test/segment.test.js index 94db376..c8716ab 100644 --- a/test/segment.test.js +++ b/test/segment.test.js @@ -7,9 +7,9 @@ describe('constructor', () => { test('general', () => { const leftSE = new SweepEvent({x: 0, y: 0}) const rightSE = new SweepEvent({x: 1, y: 1}) - const ringsIn = [] - const seg = new Segment(leftSE, rightSE, ringsIn) - expect(seg.ringsIn).toBe(ringsIn) + const ringWindings = [] + const seg = new Segment(leftSE, rightSE, ringWindings) + expect(seg.ringWindings).toBe(ringWindings) expect(seg.leftSE).toBe(leftSE) expect(seg.leftSE.otherSE).toBe(rightSE) expect(seg.rightSE).toBe(rightSE) From c9f615f089c623ec2456dfda921d0113baa34584 Mon Sep 17 00:00:00 2001 From: Mike Fogel Date: Thu, 28 Mar 2019 21:57:01 -0300 Subject: [PATCH 4/6] Remove the 'cache' on the segment object --- src/segment.js | 93 ++++++++++++--------------- test/geom-out.test.js | 146 +++++++++++++++++++++--------------------- test/segment.test.js | 1 - 3 files changed, 113 insertions(+), 127 deletions(-) diff --git a/src/segment.js b/src/segment.js index aa33365..a4dbf03 100644 --- a/src/segment.js +++ b/src/segment.js @@ -145,7 +145,6 @@ export default class Segment { rightSE.segment = this rightSE.otherSE = leftSE this.ringWindings = ringWindings - this._cache = {} // left unset for performance, set later in algorithm // this.ringOut, this.consumedBy, this.prev } @@ -448,60 +447,44 @@ export default class Segment { /* The first segment previous segment chain that is in the result */ prevInResult () { - const key = 'prevInResult' - if (this._cache[key] === undefined) this._cache[key] = this[`_${key}`]() - return this._cache[key] - } - - _prevInResult () { - if (! this.prev) return null - if (this.prev.isInResult()) return this.prev - return this.prev.prevInResult() + if (this._prevInResult !== undefined) return this._prevInResult + if (! this.prev) this._prevInResult = null + else if (this.prev.isInResult()) this._prevInResult = this.prev + else this._prevInResult = this.prev.prevInResult() + return this._prevInResult } ringWindingsBefore () { - const key = 'ringWindingsBefore' - if (this._cache[key] === undefined) this._cache[key] = this[`_${key}`]() - return this._cache[key] - } - - _ringWindingsBefore () { - if (! this.prev) return [] - return (this.prev.consumedBy || this.prev).ringWindingsAfter() + if (this._ringWindingBefore !== undefined) return this._ringWindingBefore + if (! this.prev) this._ringWindingBefore = [] + else { + const seg = this.prev.consumedBy || this.prev + this._ringWindingBefore = seg.ringWindingsAfter() + } + return this._ringWindingBefore } ringWindingsAfter () { - const key = 'ringWindingsAfter' - if (this._cache[key] === undefined) this._cache[key] = this[`_${key}`]() - return this._cache[key] - } - - _ringWindingsAfter () { - const ringWindingsAfter = this.ringWindingsBefore().slice(0) + if (this._ringWindingAfter !== undefined) return this._ringWindingAfter + this._ringWindingsAfter = this.ringWindingsBefore().slice(0) for (let i = 0, iMax = this.ringWindings.length; i < iMax; i++) { - ringWindingsAfter.push(this.ringWindings[i]) + this._ringWindingsAfter.push(this.ringWindings[i]) } - return ringWindingsAfter + return this._ringWindingsAfter } multiPolysBefore () { - const key = 'multiPolysBefore' - if (this._cache[key] === undefined) this._cache[key] = this[`_${key}`]() - return this._cache[key] - } - - _multiPolysBefore () { - if (! this.prev) return [] - return (this.prev.consumedBy || this.prev).multiPolysAfter() + if (this._multiPolysBefore !== undefined) return this._multiPolysBefore + if (! this.prev) this._multiPolysBefore = [] + else { + const seg = this.prev.consumedBy || this.prev + this._multiPolysBefore = seg.multiPolysAfter() + } + return this._multiPolysBefore } multiPolysAfter () { - const key = 'multiPolysAfter' - if (this._cache[key] === undefined) this._cache[key] = this[`_${key}`]() - return this._cache[key] - } - - _multiPolysAfter () { + if (this._multiPolysAfter !== undefined) return this._multiPolysAfter // consolidate down the ringWindings we've accumulated const rings = [] const windings = [] @@ -534,22 +517,20 @@ export default class Segment { } } // now calculate our multiPolysAfter - const mps = [] + this._multiPolysAfter = [] for (let i = 0, iMax = polysAfter.length; i < iMax; i++) { const mp = polysAfter[i].multiPoly - if (mps.indexOf(mp) === -1) mps.push(mp) + if (this._multiPolysAfter.indexOf(mp) === -1) { + this._multiPolysAfter.push(mp) + } } - return mps + return this._multiPolysAfter } /* Is this segment part of the final result? */ isInResult () { - const key = 'isInResult' - if (this._cache[key] === undefined) this._cache[key] = this[`_${key}`]() - return this._cache[key] - } + if (this._isInResult !== undefined) return this._isInResult - _isInResult () { // if we've been consumed, we're not in the result if (this.consumedBy) return false @@ -563,7 +544,8 @@ export default class Segment { // * On the other side there is 1 or more. const noBefores = mpsBefore.length === 0 const noAfters = mpsAfter.length === 0 - return noBefores !== noAfters + this._isInResult = noBefores !== noAfters + break } case 'intersection': { @@ -580,7 +562,8 @@ export default class Segment { least = mpsAfter.length most = mpsBefore.length } - return most === operation.numMultiPolys && least < most + this._isInResult = most === operation.numMultiPolys && least < most + break } case 'xor': { @@ -588,19 +571,23 @@ export default class Segment { // * the difference between the number of multipolys represented // with poly interiors on our two sides is an odd number const diff = Math.abs(mpsBefore.length - mpsAfter.length) - return diff % 2 === 1 + this._isInResult = diff % 2 === 1 + break } case 'difference': { // DIFFERENCE included iff: // * on exactly one side, we have just the subject const isJustSubject = mps => mps.length === 1 && mps[0].isSubject - return isJustSubject(mpsBefore) !== isJustSubject(mpsAfter) + this._isInResult = isJustSubject(mpsBefore) !== isJustSubject(mpsAfter) + break } default: throw new Error(`Unrecognized operation type found ${operation.type}`) } + + return this._isInResult } } diff --git a/test/geom-out.test.js b/test/geom-out.test.js index c532353..17f645c 100644 --- a/test/geom-out.test.js +++ b/test/geom-out.test.js @@ -17,9 +17,9 @@ describe('ring', () => { const seg2 = Segment.fromRing(p2, p3) const seg3 = Segment.fromRing(p3, p1) - seg1._cache['isInResult'] = true - seg2._cache['isInResult'] = true - seg3._cache['isInResult'] = true + seg1._isInResult = true + seg2._isInResult = true + seg3._isInResult = true const rings = RingOut.factory([seg1, seg2, seg3]) @@ -44,12 +44,12 @@ describe('ring', () => { const seg5 = Segment.fromRing(p5, p6) const seg6 = Segment.fromRing(p6, p4) - seg1._cache['isInResult'] = true - seg2._cache['isInResult'] = true - seg3._cache['isInResult'] = true - seg4._cache['isInResult'] = true - seg5._cache['isInResult'] = true - seg6._cache['isInResult'] = true + seg1._isInResult = true + seg2._isInResult = true + seg3._isInResult = true + seg4._isInResult = true + seg5._isInResult = true + seg6._isInResult = true const rings = RingOut.factory([seg1, seg2, seg3, seg4, seg5, seg6]) @@ -77,13 +77,13 @@ describe('ring', () => { const seg6 = Segment.fromRing(p6, p7) const seg7 = Segment.fromRing(p7, p5) - seg1._cache['isInResult'] = true - seg2._cache['isInResult'] = true - seg3._cache['isInResult'] = true - seg4._cache['isInResult'] = true - seg5._cache['isInResult'] = true - seg6._cache['isInResult'] = true - seg7._cache['isInResult'] = true + seg1._isInResult = true + seg2._isInResult = true + seg3._isInResult = true + seg4._isInResult = true + seg5._isInResult = true + seg6._isInResult = true + seg7._isInResult = true const rings = RingOut.factory([seg1, seg2, seg3, seg4, seg5, seg6, seg7]) @@ -113,14 +113,14 @@ describe('ring', () => { const seg7 = Segment.fromRing(p7, p8) const seg8 = Segment.fromRing(p8, p5) - seg1._cache['isInResult'] = true - seg2._cache['isInResult'] = true - seg3._cache['isInResult'] = true - seg4._cache['isInResult'] = true - seg5._cache['isInResult'] = true - seg6._cache['isInResult'] = true - seg7._cache['isInResult'] = true - seg8._cache['isInResult'] = true + seg1._isInResult = true + seg2._isInResult = true + seg3._isInResult = true + seg4._isInResult = true + seg5._isInResult = true + seg6._isInResult = true + seg7._isInResult = true + seg8._isInResult = true const segs = [seg1, seg2, seg3, seg4, seg5, seg6, seg7, seg8] const rings = RingOut.factory(segs) @@ -163,16 +163,16 @@ describe('ring', () => { const seg9 = Segment.fromRing(p9, p10) const seg10 = Segment.fromRing(p10, p8) - seg1._cache['isInResult'] = true - seg2._cache['isInResult'] = true - seg3._cache['isInResult'] = true - seg4._cache['isInResult'] = true - seg5._cache['isInResult'] = true - seg6._cache['isInResult'] = true - seg7._cache['isInResult'] = true - seg8._cache['isInResult'] = true - seg9._cache['isInResult'] = true - seg10._cache['isInResult'] = true + seg1._isInResult = true + seg2._isInResult = true + seg3._isInResult = true + seg4._isInResult = true + seg5._isInResult = true + seg6._isInResult = true + seg7._isInResult = true + seg8._isInResult = true + seg9._isInResult = true + seg10._isInResult = true const segs = [seg1, seg2, seg3, seg4, seg5, seg6, seg7, seg8, seg9, seg10] const rings = RingOut.factory(segs) @@ -208,15 +208,15 @@ describe('ring', () => { const seg8 = Segment.fromRing(p8, p9) const seg9 = Segment.fromRing(p9, p7) - seg1._cache['isInResult'] = true - seg2._cache['isInResult'] = true - seg3._cache['isInResult'] = true - seg4._cache['isInResult'] = true - seg5._cache['isInResult'] = true - seg6._cache['isInResult'] = true - seg7._cache['isInResult'] = true - seg8._cache['isInResult'] = true - seg9._cache['isInResult'] = true + seg1._isInResult = true + seg2._isInResult = true + seg3._isInResult = true + seg4._isInResult = true + seg5._isInResult = true + seg6._isInResult = true + seg7._isInResult = true + seg8._isInResult = true + seg9._isInResult = true const segs = [seg1, seg2, seg3, seg4, seg5, seg6, seg7, seg8, seg9] const rings = RingOut.factory(segs) @@ -252,15 +252,15 @@ describe('ring', () => { const seg8 = Segment.fromRing(p8, p9) const seg9 = Segment.fromRing(p9, p7) - seg1._cache['isInResult'] = true - seg2._cache['isInResult'] = true - seg3._cache['isInResult'] = true - seg4._cache['isInResult'] = true - seg5._cache['isInResult'] = true - seg6._cache['isInResult'] = true - seg7._cache['isInResult'] = true - seg8._cache['isInResult'] = true - seg9._cache['isInResult'] = true + seg1._isInResult = true + seg2._isInResult = true + seg3._isInResult = true + seg4._isInResult = true + seg5._isInResult = true + seg6._isInResult = true + seg7._isInResult = true + seg8._isInResult = true + seg9._isInResult = true const segs = [seg1, seg2, seg3, seg4, seg5, seg6, seg7, seg8, seg9] const rings = RingOut.factory(segs) @@ -280,9 +280,9 @@ describe('ring', () => { const seg2 = Segment.fromRing(p2, p3) const seg3 = Segment.fromRing(p3, p1) - seg1._cache['isInResult'] = true - seg2._cache['isInResult'] = true - seg3._cache['isInResult'] = false // broken ring + seg1._isInResult = true + seg2._isInResult = true + seg3._isInResult = false // broken ring expect(() => RingOut.factory([seg1, seg2, seg3])).toThrow() }) @@ -297,9 +297,9 @@ describe('ring', () => { const seg2 = Segment.fromRing(p2, p3) const seg3 = Segment.fromRing(p3, p1) - seg1._cache['isInResult'] = true - seg2._cache['isInResult'] = true - seg3._cache['isInResult'] = true + seg1._isInResult = true + seg2._isInResult = true + seg3._isInResult = true const ring = RingOut.factory([seg1, seg2, seg3])[0] @@ -317,9 +317,9 @@ describe('ring', () => { const seg2 = Segment.fromRing(p2, p3) const seg3 = Segment.fromRing(p3, p1) - seg1._cache['isInResult'] = true - seg2._cache['isInResult'] = true - seg3._cache['isInResult'] = true + seg1._isInResult = true + seg2._isInResult = true + seg3._isInResult = true const ring = RingOut.factory([seg1, seg2, seg3])[0] ring._isExteriorRing = false @@ -339,10 +339,10 @@ describe('ring', () => { const seg3 = Segment.fromRing(p3, p4) const seg4 = Segment.fromRing(p4, p1) - seg1._cache['isInResult'] = true - seg2._cache['isInResult'] = true - seg3._cache['isInResult'] = true - seg4._cache['isInResult'] = true + seg1._isInResult = true + seg2._isInResult = true + seg3._isInResult = true + seg4._isInResult = true const ring = RingOut.factory([seg1, seg2, seg3, seg4])[0] @@ -361,10 +361,10 @@ describe('ring', () => { const seg3 = Segment.fromRing(p3, p4) const seg4 = Segment.fromRing(p4, p1) - seg1._cache['isInResult'] = true - seg2._cache['isInResult'] = true - seg3._cache['isInResult'] = true - seg4._cache['isInResult'] = true + seg1._isInResult = true + seg2._isInResult = true + seg3._isInResult = true + seg4._isInResult = true const ring = RingOut.factory([seg1, seg2, seg3, seg4])[0] @@ -387,10 +387,10 @@ describe('ring', () => { const seg3 = Segment.fromRing(p3, p4) const seg4 = Segment.fromRing(p4, p1) - seg1._cache['isInResult'] = true - seg2._cache['isInResult'] = true - seg3._cache['isInResult'] = true - seg4._cache['isInResult'] = true + seg1._isInResult = true + seg2._isInResult = true + seg3._isInResult = true + seg4._isInResult = true const ring = RingOut.factory([seg1, seg2, seg3, seg4])[0] diff --git a/test/segment.test.js b/test/segment.test.js index c8716ab..279b174 100644 --- a/test/segment.test.js +++ b/test/segment.test.js @@ -14,7 +14,6 @@ describe('constructor', () => { expect(seg.leftSE.otherSE).toBe(rightSE) expect(seg.rightSE).toBe(rightSE) expect(seg.rightSE.otherSE).toBe(leftSE) - expect(seg._cache).toEqual({}) expect(seg.ringOut).toBe(undefined) expect(seg.prev).toBe(undefined) expect(seg.consumedBy).toBe(undefined) From ab52e6455b0c795972f238cc2d0c780e69c29eaa Mon Sep 17 00:00:00 2001 From: Mike Fogel Date: Thu, 28 Mar 2019 23:01:38 -0300 Subject: [PATCH 5/6] Slightly more performant nonzero rule impl. --- src/segment.js | 106 ++++++++++++++++++++++++++----------------- test/segment.test.js | 8 ++-- 2 files changed, 70 insertions(+), 44 deletions(-) diff --git a/src/segment.js b/src/segment.js index a4dbf03..43fc955 100644 --- a/src/segment.js +++ b/src/segment.js @@ -136,7 +136,7 @@ export default class Segment { /* Warning: a reference to ringWindings input will be stored, * and possibly will be later modified */ - constructor (leftSE, rightSE, ringWindings) { + constructor (leftSE, rightSE, rings, windings) { this.id = ++segmentId this.leftSE = leftSE leftSE.segment = this @@ -144,7 +144,8 @@ export default class Segment { this.rightSE = rightSE rightSE.segment = this rightSE.otherSE = leftSE - this.ringWindings = ringWindings + this.rings = rings + this.windings = windings // left unset for performance, set later in algorithm // this.ringOut, this.consumedBy, this.prev } @@ -170,7 +171,7 @@ export default class Segment { const leftSE = new SweepEvent(leftPt, true) const rightSE = new SweepEvent(rightPt, false) - return new Segment(leftSE, rightSE, [[ring, winding]]) + return new Segment(leftSE, rightSE, [ring], [winding]) } /* When a segment is split, the rightSE is replaced with a new sweep event */ @@ -373,7 +374,9 @@ export default class Segment { this.replaceRightSE(newRightSE) newEvents.push(newRightSE) newEvents.push(newLeftSE) - const newSeg = new Segment(newLeftSE, oldRightSE, this.ringWindings.slice()) + const newSeg = new Segment( + newLeftSE, oldRightSE, this.rings.slice(), this.windings.slice() + ) // when splitting a nearly vertical downward-facing segment, // sometimes one of the resulting new segments is vertical, in which @@ -403,13 +406,12 @@ export default class Segment { this.leftSE = tmpEvt this.leftSE.isLeft = true this.rightSE.isLeft = false - for (let i = 0, iMax = this.ringWindings.length; i < iMax; i++) { - const pair = this.ringWindings[i] - this.ringWindings[i] = [pair[0], -1 * pair[1]] + for (let i = 0, iMax = this.windings.length; i < iMax; i++) { + this.windings[i] *= -1 } } - /* Consume another segment. We take their ringWindings under our wing + /* Consume another segment. We take their rings under our wing * and mark them as consumed. Use for perfectly overlapping segments */ consume (other) { let consumer = this @@ -434,10 +436,18 @@ export default class Segment { consumee = tmp } - for (let i = 0, iMax = consumee.ringWindings.length; i < iMax; i++) { - consumer.ringWindings.push(consumee.ringWindings[i]) + for (let i = 0, iMax = consumee.rings.length; i < iMax; i++) { + const ring = consumee.rings[i] + const winding = consumee.windings[i] + const index = consumer.rings.indexOf(ring) + if (index === -1) { + consumer.rings.push(ring) + consumer.windings.push(winding) + } + else consumer.windings[index] += winding } - consumee.ringWindings = null + consumee.rings = null + consumee.windings = null consumee.consumedBy = consumer // mark sweep events consumed as to maintain ordering in sweep event queue @@ -454,23 +464,51 @@ export default class Segment { return this._prevInResult } - ringWindingsBefore () { - if (this._ringWindingBefore !== undefined) return this._ringWindingBefore - if (! this.prev) this._ringWindingBefore = [] + _calcRingWindingsBefore () { + if (! this.prev) { + this._ringsBefore = [] + this._windingsBefore = [] + } else { const seg = this.prev.consumedBy || this.prev - this._ringWindingBefore = seg.ringWindingsAfter() + this._ringsBefore = seg.ringsAfter() + this._windingsBefore = seg.windingsAfter() } - return this._ringWindingBefore } - ringWindingsAfter () { - if (this._ringWindingAfter !== undefined) return this._ringWindingAfter - this._ringWindingsAfter = this.ringWindingsBefore().slice(0) - for (let i = 0, iMax = this.ringWindings.length; i < iMax; i++) { - this._ringWindingsAfter.push(this.ringWindings[i]) + ringsBefore () { + if (this._ringsBefore === undefined) this._calcRingWindingsBefore() + return this._ringsBefore + } + + windingsBefore() { + if (this._windingsBefore === undefined) this._calcRingWindingsBefore() + return this._windingsBefore + } + + _calcRingWindingsAfter () { + this._ringsAfter = this.ringsBefore().slice(0) + this._windingsAfter = this.windingsBefore().slice(0) + for (let i = 0, iMax = this.rings.length; i < iMax; i++) { + const ring = this.rings[i] + const winding = this.windings[i] + const index = this._ringsAfter.indexOf(ring) + if (index === -1) { + this._ringsAfter.push(ring) + this._windingsAfter.push(winding) + } + else this._windingsAfter[index] += winding } - return this._ringWindingsAfter + } + + ringsAfter () { + if (this._ringsAfter === undefined) this._calcRingWindingsAfter() + return this._ringsAfter + } + + windingsAfter() { + if (this._windingsAfter === undefined) this._calcRingWindingsAfter() + return this._windingsAfter } multiPolysBefore () { @@ -485,28 +523,14 @@ export default class Segment { multiPolysAfter () { if (this._multiPolysAfter !== undefined) return this._multiPolysAfter - // consolidate down the ringWindings we've accumulated - const rings = [] - const windings = [] - const ringWindingsAfter = this.ringWindingsAfter() - for (let i = 0, iMax = ringWindingsAfter.length; i < iMax; i++) { - const ring = ringWindingsAfter[i][0] - const winding = ringWindingsAfter[i][1] - const index = rings.indexOf(ring) - if (index === -1) { - rings.push(ring) - windings.push(winding) - } - else { - windings[index] += winding - } - } // calcualte our polysAfter const polysAfter = [] const polysExclude = [] - for (let i = 0, iMax = rings.length; i < iMax; i++) { - if (windings[i] === 0) continue // non-zero rule - const ring = rings[i] + const ringsAfter = this.ringsAfter() + const windingsAfter = this.windingsAfter() + for (let i = 0, iMax = ringsAfter.length; i < iMax; i++) { + if (windingsAfter[i] === 0) continue // non-zero rule + const ring = ringsAfter[i] const poly = ring.poly if (polysExclude.indexOf(poly) !== -1) continue if (ring.isExterior) polysAfter.push(poly) diff --git a/test/segment.test.js b/test/segment.test.js index 279b174..f00db96 100644 --- a/test/segment.test.js +++ b/test/segment.test.js @@ -7,9 +7,11 @@ describe('constructor', () => { test('general', () => { const leftSE = new SweepEvent({x: 0, y: 0}) const rightSE = new SweepEvent({x: 1, y: 1}) - const ringWindings = [] - const seg = new Segment(leftSE, rightSE, ringWindings) - expect(seg.ringWindings).toBe(ringWindings) + const rings = [] + const windings = [] + const seg = new Segment(leftSE, rightSE, rings, windings) + expect(seg.rings).toBe(rings) + expect(seg.windings).toBe(windings) expect(seg.leftSE).toBe(leftSE) expect(seg.leftSE.otherSE).toBe(rightSE) expect(seg.rightSE).toBe(rightSE) From a7c781d252cbce63b09d64b0ecf3f2922b83d6d4 Mon Sep 17 00:00:00 2001 From: Mike Fogel Date: Fri, 29 Mar 2019 15:00:07 -0300 Subject: [PATCH 6/6] Pull together segment sweep line state Performance boost --- src/segment.js | 91 +++++++++++++++++++------------------------------- 1 file changed, 34 insertions(+), 57 deletions(-) diff --git a/src/segment.js b/src/segment.js index 43fc955..02c7864 100644 --- a/src/segment.js +++ b/src/segment.js @@ -464,70 +464,48 @@ export default class Segment { return this._prevInResult } - _calcRingWindingsBefore () { - if (! this.prev) { - this._ringsBefore = [] - this._windingsBefore = [] + beforeState() { + if (this._beforeState !== undefined) return this._beforeState + if (! this.prev) this._beforeState = { + rings: [], + windings: [], + multiPolys: [], } else { const seg = this.prev.consumedBy || this.prev - this._ringsBefore = seg.ringsAfter() - this._windingsBefore = seg.windingsAfter() + this._beforeState = seg.afterState() } + return this._beforeState } - ringsBefore () { - if (this._ringsBefore === undefined) this._calcRingWindingsBefore() - return this._ringsBefore - } + afterState () { + if (this._afterState !== undefined) return this._afterState - windingsBefore() { - if (this._windingsBefore === undefined) this._calcRingWindingsBefore() - return this._windingsBefore - } + const beforeState = this.beforeState() + this._afterState = { + rings: beforeState.rings.slice(0), + windings: beforeState.windings.slice(0), + multiPolys: [] + } + const ringsAfter = this._afterState.rings + const windingsAfter = this._afterState.windings + const mpsAfter = this._afterState.multiPolys - _calcRingWindingsAfter () { - this._ringsAfter = this.ringsBefore().slice(0) - this._windingsAfter = this.windingsBefore().slice(0) + // calculate ringsAfter, windingsAfter for (let i = 0, iMax = this.rings.length; i < iMax; i++) { const ring = this.rings[i] const winding = this.windings[i] - const index = this._ringsAfter.indexOf(ring) + const index = ringsAfter.indexOf(ring) if (index === -1) { - this._ringsAfter.push(ring) - this._windingsAfter.push(winding) + ringsAfter.push(ring) + windingsAfter.push(winding) } - else this._windingsAfter[index] += winding - } - } - - ringsAfter () { - if (this._ringsAfter === undefined) this._calcRingWindingsAfter() - return this._ringsAfter - } - - windingsAfter() { - if (this._windingsAfter === undefined) this._calcRingWindingsAfter() - return this._windingsAfter - } - - multiPolysBefore () { - if (this._multiPolysBefore !== undefined) return this._multiPolysBefore - if (! this.prev) this._multiPolysBefore = [] - else { - const seg = this.prev.consumedBy || this.prev - this._multiPolysBefore = seg.multiPolysAfter() + else windingsAfter[index] += winding } - return this._multiPolysBefore - } - multiPolysAfter () { - if (this._multiPolysAfter !== undefined) return this._multiPolysAfter - // calcualte our polysAfter + // calcualte polysAfter const polysAfter = [] const polysExclude = [] - const ringsAfter = this.ringsAfter() - const windingsAfter = this.windingsAfter() for (let i = 0, iMax = ringsAfter.length; i < iMax; i++) { if (windingsAfter[i] === 0) continue // non-zero rule const ring = ringsAfter[i] @@ -540,26 +518,25 @@ export default class Segment { if (index !== -1) polysAfter.splice(index, 1) } } - // now calculate our multiPolysAfter - this._multiPolysAfter = [] + + // calculate multiPolysAfter for (let i = 0, iMax = polysAfter.length; i < iMax; i++) { const mp = polysAfter[i].multiPoly - if (this._multiPolysAfter.indexOf(mp) === -1) { - this._multiPolysAfter.push(mp) - } + if (mpsAfter.indexOf(mp) === -1) mpsAfter.push(mp) } - return this._multiPolysAfter + + return this._afterState } /* Is this segment part of the final result? */ isInResult () { - if (this._isInResult !== undefined) return this._isInResult - // if we've been consumed, we're not in the result if (this.consumedBy) return false - const mpsBefore = this.multiPolysBefore() - const mpsAfter = this.multiPolysAfter() + if (this._isInResult !== undefined) return this._isInResult + + const mpsBefore = this.beforeState().multiPolys + const mpsAfter = this.afterState().multiPolys switch (operation.type) { case 'union': {