Ensure output rings are not self-intersecting

Fixes #11 Squashed commit of the following: commit 6d8e70b Author: Mike Fogel <[email protected]> Date: Wed Mar 21 23:24:46 2018 -0300 Remove starting point from ring if superflous commit 360d7c4 Author: Mike Fogel <[email protected]> Date: Wed Mar 21 22:17:37 2018 -0300 Get Ring.factory() fullfilling its spec commit 7553870 Author: Mike Fogel <[email protected]> Date: Wed Mar 21 17:57:57 2018 -0300 Fill in Ring.factory() Also change the Ring constructor to expect an in-order array of sweep events. commit 6f0c5f5 Author: Mike Fogel <[email protected]> Date: Wed Mar 21 16:51:31 2018 -0300 Move ring-building process to within a factory() commit 9208549 Author: Mike Fogel <[email protected]> Date: Tue Mar 20 23:29:09 2018 -0300 Good test case
mfogel · Mar 22, 2018 · ede9262 · ede9262
1 parent 291f591
commit ede9262
Show file tree

Hide file tree

Showing 10 changed files with 471 additions and 93 deletions.
diff --git a/README.md b/README.md
@@ -80,6 +80,7 @@ The Martinez-Rueda-Feito polygon clipping algorithm is used to compute the resul
 
 ### v0.6 (in development)
 
+ * Ensure output rings are not self-intersecting ([#11](https://github.com/mfogel/polygon-clipping/issues/11))
  * Allow self-touching (but not crossing) input rings ([#10](https://github.com/mfogel/polygon-clipping/issues/10))
  * Support empty MultiPolygons as input
  * Performance improvements (reduced memory footprint and lower CPU time)

diff --git a/src/geom-out.js b/src/geom-out.js
@@ -1,12 +1,91 @@
-const { cmpPoints } = require('./flp')
 const { compareVectorAngles } = require('./vector')
 
 class Ring {
-  constructor (segment) {
-    this.firstSegment = segment
+  /* Given the segments from the sweep line pass, compute & return a series
+   * of closed rings from all the segments marked to be part of the result */
+  static factory (allSegments) {
+    const ringsOut = []
+
+    for (let i = 0, iMax = allSegments.length; i < iMax; i++) {
+      const segment = allSegments[i]
+      if (!segment.isInResult || segment.ringOut) continue
+
+      let prevEvent = null
+      let event = segment.leftSE
+      let nextEvent = segment.rightSE
+      const events = [event]
+
+      const startingLE = event.linkedEvents
+      const intersectionLEs = []
+
+      /* Walk the chain of linked events to form a closed ring */
+      while (true) {
+        prevEvent = event
+        event = nextEvent
+        events.push(event)
+
+        /* Is the ring complete? */
+        if (event.linkedEvents === startingLE) break
+
+        while (true) {
+          const availableLEs = event.getAvailableLinkedEvents()
+
+          /* Did we hit a dead end? This shouldn't happen. Indicates some earlier
+           * part of the algorithm malfunctioned... please file a bug report. */
+          if (availableLEs.length === 0) {
+            const firstPt = events[0].point
+            const lastPt = events[events.length - 1].point
+            throw new Error(
+              `Unable to complete output ring starting at [${firstPt}].` +
+                ` Last matching segment found ends at [${lastPt}].`
+            )
+          }
+
+          /* Only one way to go, so cotinue on the path */
+          if (availableLEs.length === 1) {
+            nextEvent = availableLEs[0].otherSE
+            break
+          }
+
+          /* We must have an intersection. Check for a completed loop */
+          let indexLE = null
+          for (let j = 0, jMax = intersectionLEs.length; j < jMax; j++) {
+            if (intersectionLEs[j].linkedEvents === event.linkedEvents) {
+              indexLE = j
+              break
+            }
+          }
+          /* Found a completed loop. Cut that off and make a ring */
+          if (indexLE !== null) {
+            const intersectionLE = intersectionLEs.splice(indexLE)[0]
+            const ringEvents = events.splice(intersectionLE.index)
+            ringEvents.unshift(ringEvents[0].otherSE)
+            ringsOut.push(new Ring(ringEvents.reverse()))
+            continue
+          }
+          /* register the intersection */
+          intersectionLEs.push({
+            index: events.length,
+            linkedEvents: event.linkedEvents
+          })
+          /* Choose the left-most option to continue the walk */
+          const comparator = event.getLeftmostComparator(prevEvent)
+          nextEvent = availableLEs.sort(comparator)[0].otherSE
+          break
+        }
+      }
+
+      ringsOut.push(new Ring(events))
+    }
+    return ringsOut
+  }
+
+  constructor (events) {
+    this.events = events
+    for (let i = 0, iMax = events.length; i < iMax; i++) {
+      events[i].segment.registerRingOut(this)
+    }
     this.poly = null
-    this._points = null
-    this._claimSegments()
     this._clearCache()
   }
 
@@ -16,18 +95,22 @@ class Ring {
 
   getGeom () {
     // Remove superfluous points (ie extra points along a straight line),
-    // Note that the starting/ending point doesn't need to be considered,
-    // as the sweep line trace gaurantees it to be not in the middle
-    // of a straight segment.
-    const points = [this._points[0]]
-    for (let i = 1, iMax = this._points.length - 1; i < iMax; i++) {
-      const prevPt = this._points[i - 1]
-      const pt = this._points[i]
-      const nextPt = this._points[i + 1]
+    const points = [this.events[0].point]
+    for (let i = 1, iMax = this.events.length - 1; i < iMax; i++) {
+      const prevPt = this.events[i - 1].point
+      const pt = this.events[i].point
+      const nextPt = this.events[i + 1].point
       if (compareVectorAngles(pt, prevPt, nextPt) === 0) continue
       points.push(pt)
     }
-    points.push(this._points[this._points.length - 1])
+
+    // check if the starting point is necessary
+    const prevPt = this.events[this.events.length - 2].point
+    const pt = this.events[0].point
+    const nextPt = this.events[1].point
+    if (compareVectorAngles(pt, prevPt, nextPt) === 0) points.shift()
+
+    points.push(points[0])
     return this.isExteriorRing ? points : points.reverse()
   }
 
@@ -51,41 +134,6 @@ class Ring {
     return this._cache[propName]
   }
 
-  /* Walk down the segments via the linked events, and claim the
-   * segments that will be part of this ring */
-  _claimSegments () {
-    const segment = this.firstSegment
-    let prevEvent = null
-    let event = segment.leftSE
-    let nextEvent = segment.rightSE
-    this._points = [event.point]
-
-    while (true) {
-      prevEvent = event
-      event = nextEvent
-
-      this._points.push(event.point)
-      event.segment.registerRingOut(this)
-
-      const linkedEvents = event.getAvailableLinkedEvents()
-      if (linkedEvents.length === 0) break
-      if (linkedEvents.length === 1) nextEvent = linkedEvents[0].otherSE
-      if (linkedEvents.length > 1) {
-        const comparator = event.getLeftmostComparator(prevEvent)
-        nextEvent = linkedEvents.sort(comparator)[0].otherSE
-      }
-    }
-
-    const firstPt = this._points[0]
-    const lastPt = this._points[this._points.length - 1]
-    if (cmpPoints(firstPt, lastPt) !== 0) {
-      throw new Error(
-        `Unable to complete output ring starting at [${firstPt}].` +
-          ` Last matching segment found ends at [${lastPt}].`
-      )
-    }
-  }
-
   _isExteriorRing () {
     if (!this.enclosingRing) return true
     if (!this.enclosingRing.enclosingRing) return false
@@ -95,7 +143,8 @@ class Ring {
 
   /* Returns the ring that encloses this one, if any */
   _enclosingRing () {
-    let prevSeg = this.firstSegment.prevInResult
+    let prevSeg = this.events[0].segment.prevInResult
+    while (prevSeg && prevSeg.ringOut === this) prevSeg = prevSeg.prevInResult
     let prevPrevSeg = prevSeg ? prevSeg.prevInResult : null
 
     while (true) {
@@ -162,7 +211,7 @@ class MultiPoly {
     const polys = []
     for (let i = 0, iMax = rings.length; i < iMax; i++) {
       const ring = rings[i]
-      if (ring.poly) return
+      if (ring.poly) continue
       if (ring.isExteriorRing) polys.push(new Poly(ring))
       else {
         if (!ring.enclosingRing.poly) polys.push(new Poly(ring.enclosingRing))

diff --git a/src/index.js b/src/index.js
@@ -7,13 +7,15 @@ const SweepEvent = require('./sweep-event')
 const SweepLine = require('./sweep-line')
 
 const doIt = (operationType, geom, moreGeoms) => {
+  /* Clean inputs */
   cleanInput.forceMultiPoly(geom)
   cleanInput.cleanMultiPoly(geom)
   for (let i = 0, iMax = moreGeoms.length; i < iMax; i++) {
     cleanInput.forceMultiPoly(moreGeoms[i])
     cleanInput.cleanMultiPoly(moreGeoms[i])
   }
 
+  /* Convert inputs to MultiPoly objects, mark subject & register operation */
   const multipolys = [new geomIn.MultiPoly(geom)]
   multipolys[0].markAsSubject()
   for (let i = 0, iMax = moreGeoms.length; i < iMax; i++) {
@@ -39,18 +41,11 @@ const doIt = (operationType, geom, moreGeoms) => {
     }
   }
 
-  /* Self-intersecting input rings are ambigious */
+  /* Error on self-crossing input rings */
   cleanInput.errorOnSelfIntersectingRings(sweepLine.segments)
 
-  /* Collect the segments we're keeping in a series of rings */
-  const ringsOut = []
-  for (let i = 0, iMax = sweepLine.segments.length; i < iMax; i++) {
-    const segment = sweepLine.segments[i]
-    if (!segment.isInResult || segment.ringOut) continue
-    ringsOut.push(new geomOut.Ring(segment))
-  }
-
-  /* Compile those rings into a multipolygon */
+  /* Collect and compile segments we're keeping into a multipolygon */
+  const ringsOut = geomOut.Ring.factory(sweepLine.segments)
   const result = new geomOut.MultiPoly(ringsOut)
   return result.getGeom()
 }

diff --git a/src/sweep-event.js b/src/sweep-event.js
@@ -37,13 +37,12 @@ class SweepEvent {
   }
 
   link (other) {
-    if (other.linkedEvents.length > 1) {
-      throw new Error('Cannot link an already-linked event')
+    const otherLE = other.linkedEvents
+    for (let i = 0, iMax = otherLE.length; i < iMax; i++) {
+      const evt = otherLE[i]
+      this.linkedEvents.push(evt)
+      evt.linkedEvents = this.linkedEvents
     }
-    for (let i = 0, iMax = other.linkedEvents.length; i < iMax; i++) {
-      this.linkedEvents.push(other.linkedEvents[i])
-    }
-    other.linkedEvents = this.linkedEvents
   }
 
   getAvailableLinkedEvents () {

diff --git a/test/end-to-end/clean-poly-with-interior-ring-touching-exterior/union.geojson b/test/end-to-end/clean-poly-with-interior-ring-touching-exterior/union.geojson
@@ -4,7 +4,10 @@
   "geometry": {
     "type": "MultiPolygon",
     "coordinates": [
-      [[[0, 0], [7, 0], [7, 2], [6, 1], [6, 3], [7, 2], [7, 7], [0, 7], [0, 0]]]
+      [
+        [[0, 0], [7, 0], [7, 7], [0, 7], [0, 0]],
+        [[7, 2], [6, 1], [6, 3], [7, 2]]
+      ]
     ]
   }
 }
diff --git a/test/end-to-end/no-self-intersecting-rings-output/args.geojson b/test/end-to-end/no-self-intersecting-rings-output/args.geojson
@@ -0,0 +1,29 @@
+{
+  "type": "FeatureCollection",
+  "features": [
+    {
+      "type": "Feature",
+      "properties": null,
+      "geometry": {
+        "type": "Polygon",
+        "coordinates": [[[0, 0], [2, 0], [3, 2], [2, 4], [0, 4], [0, 0]]]
+      }
+    },
+    {
+      "type": "Feature",
+      "properties": null,
+      "geometry": {
+        "type": "Polygon",
+        "coordinates": [[[1, 1], [3, 2], [1, 3], [1, 1]]]
+      }
+    },
+    {
+      "type": "Feature",
+      "properties": null,
+      "geometry": {
+        "type": "Polygon",
+        "coordinates": [[[3, 2], [4, 1], [4, 3], [3, 2]]]
+      }
+    }
+  ]
+}
diff --git a/test/end-to-end/no-self-intersecting-rings-output/xor.geojson b/test/end-to-end/no-self-intersecting-rings-output/xor.geojson
@@ -0,0 +1,14 @@
+{
+  "type": "Feature",
+  "properties": null,
+  "geometry": {
+    "type": "MultiPolygon",
+    "coordinates": [
+      [
+        [[0, 0], [2, 0], [3, 2], [2, 4], [0, 4], [0, 0]],
+        [[3, 2], [1, 1], [1, 3], [3, 2]]
+      ],
+      [[[3, 2], [4, 1], [4, 3], [3, 2]]]
+    ]
+  }
+}
diff --git a/test/end-to-end/self-intersects-but-doesnt-cross-2/union.geojson b/test/end-to-end/self-intersects-but-doesnt-cross-2/union.geojson
@@ -5,25 +5,10 @@
     "type": "MultiPolygon",
     "coordinates": [
       [
-        [
-          [0, -1],
-          [5, -1],
-          [5, 5],
-          [0, 5],
-          [0, 3],
-          [2, 2],
-          [1, 4],
-          [3, 4],
-          [2, 2],
-          [4, 3],
-          [4, 1],
-          [2, 2],
-          [3, 0],
-          [1, 0],
-          [2, 2],
-          [0, 1],
-          [0, -1]
-        ]
+        [[0, -1], [5, -1], [5, 5], [0, 5], [0, 3], [2, 2], [0, 1], [0, -1]],
+        [[2, 2], [1, 4], [3, 4], [2, 2]],
+        [[2, 2], [4, 3], [4, 1], [2, 2]],
+        [[2, 2], [3, 0], [1, 0], [2, 2]]
       ]
     ]
   }