From 750cb6658041bbf1f988119d1b00964b5df561d0 Mon Sep 17 00:00:00 2001 From: Mike Fogel Date: Sat, 10 Mar 2018 15:05:49 -0300 Subject: [PATCH] Handle very thin input polygons Improved cleaning of: * duplicate input points * colinear input points * degenerate rings & polygons Closes #6 Squashed commit of the following: commit 69117d7be871b1723ee688fb4099989e2711cff0 Author: Mike Fogel Date: Sat Mar 10 14:59:48 2018 -0300 Add end-to-end test and docs commit efa4a8fa30e3873df9640986b1d456b244eea827 Author: Mike Fogel Date: Sat Mar 10 14:56:19 2018 -0300 Remove degenerate rings/polys from input commit 4ba5ece7538861613297d3bd02fa9bc98ac37b4e Author: Mike Fogel Date: Sat Mar 10 14:38:24 2018 -0300 Remove colinear points from input commit 74a3107b16551366eabf1e3344eeab8932d4bc2f Author: Mike Fogel Date: Sat Mar 10 14:18:31 2018 -0300 Move 'remove dup input points' to clean-input.js --- README.md | 8 +- src/clean-input.js | 79 +++++++++++++++-- src/geom-in.js | 14 ++-- src/index.js | 2 +- test/clean-input.test.js | 84 ++++++++++++++++++- .../clean-degenerate-polygon/args.geojson | 20 +++++ .../clean-degenerate-polygon/union.geojson | 8 ++ test/index.test.js | 12 +-- 8 files changed, 197 insertions(+), 30 deletions(-) create mode 100644 test/end-to-end/clean-degenerate-polygon/args.geojson create mode 100644 test/end-to-end/clean-degenerate-polygon/union.geojson diff --git a/README.md b/README.md index 2c979cc..6d66e25 100644 --- a/README.md +++ b/README.md @@ -78,13 +78,17 @@ The Martinez-Rueda-Feito polygon clipping algorithm is used to compute the resul ## Changelog -### v0.5 +### v0.6 (in development) + + * Handle very thin input polygons ([#6](https://github.com/mfogel/polygon-clipping/issues/6)) + +### v0.5 (2018-03-01) * Remove `clean()` from module.exports ([#3](https://github.com/mfogel/polygon-clipping/issues/3)) * Expand `difference()` operation to optionally take multiple clippings ([#1](https://github.com/mfogel/polygon-clipping/issues/1)) * Use [splay-tree](https://github.com/w8r/splay-tree) instead of [avl](https://github.com/w8r/avl) to power the sweep line status tree ([#2](https://github.com/mfogel/polygon-clipping/issues/2)) -### v0.4 +### v0.4 (2018-02-27) * First release as new package after fork from [martinez](https://github.com/w8r/martinez) diff --git a/src/clean-input.js b/src/clean-input.js index b0743b3..3e04c73 100644 --- a/src/clean-input.js +++ b/src/clean-input.js @@ -1,4 +1,5 @@ const { arePointsEqual } = require('./flp') +const { compareVectorAngles } = require('./vector') /* WARN: input modified directly */ const forceMultiPoly = geom => { @@ -22,14 +23,73 @@ const forceMultiPoly = geom => { } /* WARN: input modified directly */ -const closeAllRings = multipoly => { - multipoly.forEach(poly => { - poly.forEach(ring => { - if (!arePointsEqual(ring[0], ring[ring.length - 1])) { - ring.push([...ring[0]]) // copy by value - } - }) - }) +const cleanMultiPoly = multipoly => { + let i = 0 + while (i < multipoly.length) { + const poly = multipoly[i] + if (poly.length === 0) { + multipoly.splice(i, 1) + continue + } + + const exteriorRing = poly[0] + cleanRing(exteriorRing) + // poly is dropped if exteriorRing is degenerate + if (exteriorRing.length === 0) { + multipoly.splice(i, 1) + continue + } + + let j = 1 + while (j < poly.length) { + const interiorRing = poly[j] + cleanRing(interiorRing) + if (interiorRing.length === 0) poly.splice(j, 1) + else j++ + } + + i++ + } +} + +/* Clean ring: + * - remove duplicate points + * - remove colinear points TODO + * - remove rings with no area (less than 3 distinct points) TODO + * - close rings (last point should equal first) + * + * WARN: input modified directly */ +const cleanRing = ring => { + if (ring.length === 0) return + if (!arePointsEqual(ring[0], ring[ring.length - 1])) { + ring.push([...ring[0]]) // copy by value + } + + const isPointUncessary = (prevPt, pt, nextPt) => + arePointsEqual(prevPt, pt) || + arePointsEqual(pt, nextPt) || + compareVectorAngles(pt, prevPt, nextPt) === 0 + + let i = 1 + while (i < ring.length - 1) { + const [prevPt, pt, nextPt] = [ring[i - 1], ring[i], ring[i + 1]] + if (isPointUncessary(prevPt, pt, nextPt)) ring.splice(i, 1) + else i++ + } + + // check the first/last point as well + while (ring.length > 2) { + const [prevPt, pt, nextPt] = [ring[ring.length - 2], ring[0], ring[1]] + if (!isPointUncessary(prevPt, pt, nextPt)) break + ring.splice(0, 1) + ring.splice(ring.length - 1, 1) + ring.push(ring[0]) + } + + // if our ring has less than 3 distinct points now (so is degenerate) + // shrink it down to the empty array to communicate to our caller to + // drop it + while (ring.length < 4 && ring.length > 0) ring.pop() } /* Scan the already-linked events of the segments for any @@ -48,7 +108,8 @@ const errorOnSelfIntersectingRings = segments => { } module.exports = { - closeAllRings, + cleanMultiPoly, + cleanRing, errorOnSelfIntersectingRings, forceMultiPoly } diff --git a/src/geom-in.js b/src/geom-in.js index c033b17..207a7e2 100644 --- a/src/geom-in.js +++ b/src/geom-in.js @@ -13,14 +13,12 @@ class Ring { this.isInterior = !isExterior this.segments = [] - geomRing.forEach((point, i, ring) => { - if (i === 0) return - const prevPoint = ring[i - 1] - - // repeated point in a ring? Skip over it - if (arePointsEqual(prevPoint, point)) return - - this.segments.push(new Segment(prevPoint, point, this)) + let prevPoint = null + geomRing.forEach(point => { + if (prevPoint !== null) { + this.segments.push(new Segment(prevPoint, point, this)) + } + prevPoint = point }) } diff --git a/src/index.js b/src/index.js index 401b8ac..31cb5d2 100644 --- a/src/index.js +++ b/src/index.js @@ -7,7 +7,7 @@ const SweepLine = require('./sweep-line') const doIt = (operationType, ...geoms) => { geoms.forEach(g => cleanInput.forceMultiPoly(g)) - geoms.forEach(g => cleanInput.closeAllRings(g)) + geoms.forEach(g => cleanInput.cleanMultiPoly(g)) const multipolys = geoms.map(geom => new geomIn.MultiPoly(geom)) diff --git a/test/clean-input.test.js b/test/clean-input.test.js index 54429e4..c4271d9 100644 --- a/test/clean-input.test.js +++ b/test/clean-input.test.js @@ -1,6 +1,10 @@ /* eslint-env jest */ -const { closeAllRings, forceMultiPoly } = require('../src/clean-input') +const { + cleanRing, + cleanMultiPoly, + forceMultiPoly +} = require('../src/clean-input') const deepCopyArray = input => { if (Array.isArray(input)) return input.map(deepCopyArray) @@ -47,7 +51,7 @@ describe('forceMultiPoly()', () => { }) }) -describe('closeAllRings()', () => { +describe('cleanMultiPoly()', () => { test('adds closing elements to rings', () => { const openRings = [ [[[0, 0], [1, 0], [0, 1]]], @@ -57,7 +61,7 @@ describe('closeAllRings()', () => { [[[0, 0], [1, 0], [0, 1], [0, 0]]], [[[0, 0], [2, 0], [0, 2], [0, 0]], [[0, 0], [1, 0], [0, 1], [0, 0]]] ] - closeAllRings(openRings) + cleanMultiPoly(openRings) expect(openRings).toEqual(closedRings) }) @@ -67,7 +71,79 @@ describe('closeAllRings()', () => { [[[0, 0], [2, 0], [0, 2], [0, 0]], [[0, 0], [1, 0], [0, 1], [0, 0]]] ] const stillAllGood = deepCopyArray(allGood) - closeAllRings(allGood) + cleanMultiPoly(allGood) expect(allGood).toEqual(stillAllGood) }) + + test('interior degenerate rings removed', () => { + const mpIn = [ + [[[0, 0], [4, 0], [0, 4], [0, 0]], [[0, 0], [1, 1], [1, 1], [0, 0]]] + ] + const mpExpected = [[[[0, 0], [4, 0], [0, 4], [0, 0]]]] + cleanMultiPoly(mpIn) + expect(mpIn).toEqual(mpExpected) + }) + + test('exterior degenerate ring removes polygon', () => { + const mpIn = [ + [[[0, 0], [4, 0], [4, 0], [0, 0]], [[0, 0], [1, 0], [0, 1], [0, 0]]] + ] + cleanMultiPoly(mpIn) + expect(mpIn).toEqual([]) + }) + + test('exterior empty ring removes polygon', () => { + const mpIn = [[[]]] + cleanMultiPoly(mpIn) + expect(mpIn).toEqual([]) + }) + + test('polygon with no rings removed', () => { + const mpIn = [[]] + cleanMultiPoly(mpIn) + expect(mpIn).toEqual([]) + }) +}) + +describe('cleanRing()', () => { + test('already standardized input unchanged', () => { + const allGood = [[0, 0], [1, 0], [0, 1], [0, 0]] + const stillAllGood = deepCopyArray(allGood) + cleanRing(allGood) + expect(allGood).toEqual(stillAllGood) + }) + + test('adds closing elements to rings', () => { + const openRing = [[0, 0], [1, 0], [0, 1]] + const closedRing = [[0, 0], [1, 0], [0, 1], [0, 0]] + cleanRing(openRing) + expect(openRing).toEqual(closedRing) + }) + + test('removes duplicate points', () => { + const ringBad = [[0, 0], [1, 0], [1, 0], [1, 0], [0, 1], [0, 1], [0, 0]] + const ringGood = [[0, 0], [1, 0], [0, 1], [0, 0]] + cleanRing(ringBad) + expect(ringBad).toEqual(ringGood) + }) + + test('removes colinear points', () => { + const ringBad = [[0, 0], [1, 0], [2, 0], [1, 0], [0, 2], [0, 1], [0, 0]] + const ringGood = [[0, 0], [1, 0], [0, 2], [0, 0]] + cleanRing(ringBad) + expect(ringBad).toEqual(ringGood) + }) + + test('removes first/last when colinear', () => { + const ringBad = [[0, 0], [1, 0], [0, 1], [-1, 0], [0, 0]] + const ringGood = [[1, 0], [0, 1], [-1, 0], [1, 0]] + cleanRing(ringBad) + expect(ringBad).toEqual(ringGood) + }) + + test('degenerate ring shrinks to empty array', () => { + const ringBad = [[0, 0], [1, 0], [1, 0], [0, 0], [0, 0]] + cleanRing(ringBad) + expect(ringBad).toEqual([]) + }) }) diff --git a/test/end-to-end/clean-degenerate-polygon/args.geojson b/test/end-to-end/clean-degenerate-polygon/args.geojson new file mode 100644 index 0000000..3967043 --- /dev/null +++ b/test/end-to-end/clean-degenerate-polygon/args.geojson @@ -0,0 +1,20 @@ +{ + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "properties": null, + "geometry": { + "type": "Polygon", + "coordinates": [ + [ + [-130.73862770644791, 54.92404227517194], + [-130.73939947890625, 54.91877729166476], + [-130.73939947890625, 54.918777291664775], + [-130.73862770644791, 54.92404227517194] + ] + ] + } + } + ] +} diff --git a/test/end-to-end/clean-degenerate-polygon/union.geojson b/test/end-to-end/clean-degenerate-polygon/union.geojson new file mode 100644 index 0000000..db4825c --- /dev/null +++ b/test/end-to-end/clean-degenerate-polygon/union.geojson @@ -0,0 +1,8 @@ +{ + "type": "Feature", + "properties": null, + "geometry": { + "type": "MultiPolygon", + "coordinates": [] + } +} diff --git a/test/index.test.js b/test/index.test.js index 9d9a549..351b0f0 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -6,22 +6,22 @@ const operation = require('../src/operation') const doIt = require('../src') afterEach(() => { - cleanInput.closeAllRings.mockClear() + cleanInput.cleanMultiPoly.mockClear() cleanInput.errorOnSelfIntersectingRings.mockClear() cleanInput.forceMultiPoly.mockClear() }) describe('doIt calls the right stuff', () => { - test('closeAllRings() called correctly', () => { + test('cleanMultiPoly() called correctly', () => { const mp1 = [[[[0, 0], [2, 0], [0, 2], [0, 0]]]] const mp2 = [[[[0, 0], [1, 0], [0, 1], [0, 0]]]] const mp3 = [[[[0, 0], [1, 0], [0, 1], [0, 0]]]] doIt(operation.types.UNION, mp1, mp2, mp3) - expect(cleanInput.closeAllRings).toHaveBeenCalledTimes(3) - expect(cleanInput.closeAllRings).toHaveBeenCalledWith(mp1) - expect(cleanInput.closeAllRings).toHaveBeenCalledWith(mp2) - expect(cleanInput.closeAllRings).toHaveBeenCalledWith(mp3) + expect(cleanInput.cleanMultiPoly).toHaveBeenCalledTimes(3) + expect(cleanInput.cleanMultiPoly).toHaveBeenCalledWith(mp1) + expect(cleanInput.cleanMultiPoly).toHaveBeenCalledWith(mp2) + expect(cleanInput.cleanMultiPoly).toHaveBeenCalledWith(mp3) }) test('forceMultiPoly() called correctly', () => {