Allow self-crossing rings using even-odd rule

Closes #30 Squashed commit of the following: commit d58120f Author: Mike Fogel <[email protected]> Date: Sun Aug 19 17:43:35 2018 -0300 Readme updates commit 587b74e Author: Mike Fogel <[email protected]> Date: Sun Aug 19 17:32:45 2018 -0300 Remove logic that threw for self-crossing rings commit 3307efd Author: Mike Fogel <[email protected]> Date: Sun Aug 19 17:28:28 2018 -0300 Test to enforce even-odd rule, not non-zero rule Adpated from the graphic on the wikipedia page https://en.wikipedia.org/wiki/Nonzero-rule commit e8b0ecf Author: Mike Fogel <[email protected]> Date: Sun Aug 19 17:19:25 2018 -0300 Change error tests to working tests These tests fail at this point, going to fix that shortly.
mfogel · Aug 19, 2018 · b35adf9 · b35adf9
1 parent d2f6e6f
commit b35adf9
Show file tree

Hide file tree

Showing 11 changed files with 133 additions and 141 deletions.
diff --git a/README.md b/README.md
@@ -43,10 +43,10 @@ Each positional argument (`<geom>`) may be either a Polygon or a MultiPolygon.
 Follows the [GeoJSON Polygon spec](https://tools.ietf.org/html/rfc7946#section-3.1.6), with the following notes/modifications:
 * rings of the polygon 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).
 * winding order of rings of Polygon does not matter
 * interior rings may extend outside exterior rings (portion of interior ring outside exterior ring is dropped)
 * interior rings may touch or overlap each other
-* rings may touch themselves, but may **not** cross themselves. If a self-crossing ring is found, an exception will be thrown. To clean up self-crossing rings, you may want to use the [non-zero rule](https://en.wikipedia.org/wiki/Nonzero-rule) or the [even-odd rule](https://en.wikipedia.org/wiki/Even%E2%80%93odd_rule).
 
 #### MultiPolygon
 
@@ -80,6 +80,7 @@ The Martinez-Rueda-Feito polygon clipping algorithm is used to compute the resul
 
 ### vNext (in development)
 
+ * Allow self-crossing rings using [even-odd rule](https://en.wikipedia.org/wiki/Even%E2%80%93odd_rule) ([#30](https://github.com/mfogel/polygon-clipping/issues/30))
  * Fix bug with nearly vertical segments being split ([#29](https://github.com/mfogel/polygon-clipping/issues/29))
  * Fix bug with coincident segments being split slightly differently ([#22](https://github.com/mfogel/polygon-clipping/issues/22))
 

diff --git a/src/clean-input.js b/src/clean-input.js
@@ -144,36 +144,3 @@ export const cleanRing = ring => {
   // drop it
   while (ring.length < 4 && ring.length > 0) ring.pop()
 }
-
-/* Scan the already-linked events of the segments for any
- * self-intersecting input rings (which are not supported) */
-export const errorOnSelfIntersectingRings = segments => {
-  for (let i = 0, iMax = segments.length; i < iMax; i++) {
-    const seg = segments[i]
-
-    const evt = seg.flowIntoSE
-
-    if (evt.linkedEvents.length > 2) {
-      const evtsThisRing = evt.linkedEvents.filter(
-        other => other.segment.ringIn === seg.ringIn
-      )
-      if (evtsThisRing.length > 2) {
-        evtsThisRing.sort(evt.getLeftmostComparator(evt.otherSE))
-        const leftMostEvt = evtsThisRing[1] // skip ourself
-        const rightMostEvt = evtsThisRing[evtsThisRing.length - 1]
-
-        // both the segment on our immediate left and right will flow
-        // 'out' in intersection point was a touch and not a crossing
-        if (
-          leftMostEvt.segment.flowIntoSE === leftMostEvt ||
-          rightMostEvt.segment.flowIntoSE === rightMostEvt
-        ) {
-          throw new Error(
-            `Self-intersecting, crossing input ring found at ` +
-              `[${evt.point.x}, ${evt.point.y}]`
-          )
-        }
-      }
-    }
-  }
-}
diff --git a/src/index.js b/src/index.js
@@ -46,9 +46,6 @@ export default function doIt (operationType, geom, moreGeoms) {
     }
   }
 
-  /* Error on self-crossing input rings */
-  cleanInput.errorOnSelfIntersectingRings(sweepLine.segments)
-
   /* Collect and compile segments we're keeping into a multipolygon */
   const ringsOut = geomOut.Ring.factory(sweepLine.segments)
   const result = new geomOut.MultiPoly(ringsOut)

diff --git a/src/segment.js b/src/segment.js
@@ -71,9 +71,8 @@ export default class Segment {
     )
   }
 
-  constructor (ringIn, flowL2R) {
+  constructor (ringIn) {
     this.ringIn = ringIn
-    this.flowL2R = flowL2R
     this.leftSE = null
     this.rightSE = null
     this.ringOut = null
@@ -85,22 +84,19 @@ export default class Segment {
     const ptCmp = cmpPoints(point1, point2)
     let lp
     let rp
-    let flowL2R
     if (ptCmp < 0) {
       lp = point1
       rp = point2
-      flowL2R = true
     } else if (ptCmp > 0) {
       lp = point2
       rp = point1
-      flowL2R = false
     } else {
       throw new Error(
         `Tried to create degenerate segment at [${point1.x}, ${point1.y}]`
       )
     }
 
-    const seg = new Segment(ring, flowL2R)
+    const seg = new Segment(ring)
     seg.leftSE = new SweepEvent(lp, seg)
     seg.rightSE = new SweepEvent(rp, seg)
 
@@ -128,11 +124,6 @@ export default class Segment {
     return cmp(this.leftSE.point.x, this.rightSE.point.x) === 0
   }
 
-  /* In the original ringIn, which event came second */
-  get flowIntoSE () {
-    return this.flowL2R ? this.rightSE : this.leftSE
-  }
-
   swapEvents () {
     const tmp = this.leftSE
     this.leftSE = this.rightSE
@@ -241,7 +232,7 @@ export default class Segment {
     }
 
     const point = points.shift()
-    const newSeg = new Segment(this.ringIn, this.flowL2R)
+    const newSeg = new Segment(this.ringIn)
     newSeg.leftSE = new SweepEvent(point, newSeg)
     newSeg.rightSE = this.rightSE
     this.rightSE.segment = newSeg

diff --git a/test/end-to-end-errors.test.js b/test/end-to-end-errors.test.js
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
@@ -0,0 +1,15 @@
+{
+  "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/even-odd-rule-not-non-zero-winding/args.geojson b/test/end-to-end/even-odd-rule-not-non-zero-winding/args.geojson
@@ -0,0 +1,27 @@
+{
+  "type": "FeatureCollection",
+  "features": [
+    {
+      "type": "Feature",
+      "properties": {},
+      "geometry": {
+        "type": "Polygon",
+        "coordinates": [
+          [
+            [0, 0],
+            [4, 0],
+            [4, 3],
+            [1, 3],
+            [1, -1],
+            [2, -1],
+            [2, 2],
+            [3, 2],
+            [3, 1],
+            [0, 1],
+            [0, 0]
+          ]
+        ]
+      }
+    }
+  ]
+}
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
@@ -0,0 +1,30 @@
+{
+  "type": "Feature",
+  "properties": {},
+  "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]]
+      ],
+      [
+        [[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/end-to-end/multipoly-with-self-crossing-rings/args.geojson b/test/end-to-end/multipoly-with-self-crossing-rings/args.geojson
@@ -0,0 +1,55 @@
+{
+  "type": "FeatureCollection",
+  "features": [
+    {
+      "type": "Feature",
+      "properties": {},
+      "geometry": {
+        "type": "MultiPolygon",
+        "coordinates": [
+          [[[0, 0], [4, 0], [1, 3], [1, 1], [4, 4], [0, 4], [0, 0]]],
+          [
+            [
+              [0, 10],
+              [4, 10],
+              [2, 12],
+              [1, 13],
+              [1, 11],
+              [2, 12],
+              [4, 14],
+              [0, 14],
+              [0, 10]
+            ]
+          ],
+          [
+            [
+              [10, 0],
+              [14, 0],
+              [12, 2],
+              [11, 3],
+              [11, 1],
+              [14, 4],
+              [10, 4],
+              [10, 0]
+            ]
+          ],
+          [
+            [
+              [10, 10],
+              [14, 10],
+              [14, 14],
+              [10, 10],
+              [13, 11],
+              [12, 11],
+              [10, 10]
+            ]
+          ],
+          [
+            [[20, 0], [26, 0], [26, 6], [20, 6], [20, 0]],
+            [[21, 1], [21, 5], [25, 5], [22, 2], [22, 4], [25, 1], [21, 1]]
+          ]
+        ]
+      }
+    }
+  ]
+}
diff --git a/test/index.test.js b/test/index.test.js
@@ -66,17 +66,4 @@ describe('doIt calls the right stuff', () => {
     expect(cleanInput.cleanMultiPoly).toHaveBeenCalledWith(mp2Ob)
     expect(cleanInput.cleanMultiPoly).toHaveBeenCalledWith(mp3Ob)
   })
-
-  test('errorOnSelfIntersectingRings() called', () => {
-    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]]]]
-
-    cleanInput.errorOnSelfIntersectingRings = jest.fn(
-      cleanInput.errorOnSelfIntersectingRings
-    )
-    doIt(operation.types.UNION, mp1, [mp2, mp3])
-
-    expect(cleanInput.errorOnSelfIntersectingRings).toHaveBeenCalledTimes(1)
-  })
 })
diff --git a/test/segment.test.js b/test/segment.test.js
@@ -5,10 +5,8 @@ import Segment from '../src/segment'
 describe('constructor', () => {
   test('general', () => {
     const ringIn = {}
-    const flowL2R = {}
-    const seg = new Segment(ringIn, flowL2R)
+    const seg = new Segment(ringIn)
     expect(seg.ringIn).toEqual(ringIn)
-    expect(seg.flowL2R).toEqual(flowL2R)
     expect(seg.leftSE).toBeNull()
     expect(seg.rightSE).toBeNull()
     expect(seg.ringOut).toBeNull()
@@ -23,7 +21,6 @@ describe('fromRing', () => {
     const seg = Segment.fromRing(p1, p2)
     expect(seg.leftSE.point).toEqual(p1)
     expect(seg.rightSE.point).toEqual(p2)
-    expect(seg.flowL2R).toBeTruthy()
   })
 
   test('correct point on left and right 1', () => {
@@ -32,7 +29,6 @@ describe('fromRing', () => {
     const seg = Segment.fromRing(p1, p2)
     expect(seg.leftSE.point).toEqual(p2)
     expect(seg.rightSE.point).toEqual(p1)
-    expect(seg.flowL2R).toBeFalsy()
   })
 
   test('attempt create segment with same poitns', () => {