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 <[email protected]> Date: Sat Mar 10 14:59:48 2018 -0300 Add end-to-end test and docs commit efa4a8fa30e3873df9640986b1d456b244eea827 Author: Mike Fogel <[email protected]> Date: Sat Mar 10 14:56:19 2018 -0300 Remove degenerate rings/polys from input commit 4ba5ece7538861613297d3bd02fa9bc98ac37b4e Author: Mike Fogel <[email protected]> Date: Sat Mar 10 14:38:24 2018 -0300 Remove colinear points from input commit 74a3107b16551366eabf1e3344eeab8932d4bc2f Author: Mike Fogel <[email protected]> Date: Sat Mar 10 14:18:31 2018 -0300 Move 'remove dup input points' to clean-input.js
mfogel · Mar 10, 2018 · 750cb66 · 750cb66
1 parent acdeeee
commit 750cb66
Show file tree

Hide file tree

Showing 8 changed files with 197 additions and 30 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,8 @@
+{
+  "type": "Feature",
+  "properties": null,
+  "geometry": {
+    "type": "MultiPolygon",
+    "coordinates": []
+  }
+}
diff --git 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', () => {