Fix compare segments endpoint touching

src/compare_segments.js

@@ -12,22 +12,36 @@ export default function compareSegments(le1, le2) {
   if (le1 === le2) return 0;
 
   // Segments are not collinear
-  if (signedArea(le1.point, le1.otherEvent.point, le2.point) !== 0 ||
-    signedArea(le1.point, le1.otherEvent.point, le2.otherEvent.point) !== 0) {
+   if (signedArea(le1.point, le1.otherEvent.point, le2.point) !== 0 ||
+       signedArea(le1.point, le1.otherEvent.point, le2.otherEvent.point) !== 0) {
 
-    // If they share their left endpoint use the right endpoint to sort
+    // Left endpoints exactly identical? Use the right endpoint to sort
     if (equals(le1.point, le2.point)) return le1.isBelow(le2.otherEvent.point) ? -1 : 1;
 
-    // Different left endpoint: use the left endpoint to sort
+    // Left endpoints identical in x, but different in y? Sort by y
     if (le1.point[0] === le2.point[0]) return le1.point[1] < le2.point[1] ? -1 : 1;
 
-    // has the line segment associated to e1 been inserted
-    // into S after the line segment associated to e2 ?
-    if (compareEvents(le1, le2) === 1) return le2.isAbove(le1.point) ? -1 : 1;
+    // Default case:
+    // - Determine which segment is older, i.e., has been inserted before in the sweep line.
+    // - Project the left/start point of the new segment onto the existing segment.
+    // - If this point falls exactly onto the existing segment, use the right point to sort.
+    var oldEvt, newEvt, invertSignedArea;
+    if (compareEvents(le1, le2) === -1) {
+      oldEvt = le1;
+      newEvt = le2;
+      invertSignedArea = -1;
+    } else {
+      oldEvt = le2;
+      newEvt = le1;
+      invertSignedArea = 1;
+    }
 
-    // The line segment associated to e2 has been inserted
-    // into S after the line segment associated to e1
-    return le1.isBelow(le2.point) ? -1 : 1;
+    let cmpNewLeftPoint = invertSignedArea * signedArea(newEvt.point, oldEvt.point, oldEvt.otherEvent.point);
+    if (cmpNewLeftPoint !== 0) {
+      return cmpNewLeftPoint;
+    } else {
+      return invertSignedArea * signedArea(newEvt.otherEvent.point, oldEvt.point, oldEvt.otherEvent.point);
+    }
   }
 
   if (le1.isSubject === le2.isSubject) { // same polygon
@@ -36,10 +50,11 @@ export default function compareSegments(le1, le2) {
       p1 = le1.otherEvent.point; p2 = le2.otherEvent.point;
       if (p1[0] === p2[0] && p1[1] === p2[1]) return 0;
       else return le1.contourId > le2.contourId ? 1 : -1;
+    } else {
+      return compareEvents(le1, le2) === 1 ? 1 : -1;
     }
   } else { // Segments are collinear, but belong to separate polygons
     return le1.isSubject ? -1 : 1;
   }
 
-  return compareEvents(le1, le2) === 1 ? 1 : -1;
 }

test/compare_segments.test.js

@@ -117,5 +117,38 @@ tap.test('compare segments', (main) => {
     t.end();
   });
 
+  main.test('T-shaped cases', (t) => {
+    // Ensures that segments touching at endpoints are ordered correctly.
+    let se1, se2;
+
+    // shape: \/
+    //         \
+    se1 = new SweepEvent([0, 1],  true, new SweepEvent([1, 0], false));
+    se2 = new SweepEvent([0.5, 0.5], true, new SweepEvent([1, 1], false));
+    t.equal(compareSegments(se1, se2), -1);
+    t.equal(compareSegments(se2, se1), +1);
+
+    // shape:  /
+    //        /\
+    se1 = new SweepEvent([0, 0],  true, new SweepEvent([1, 1], false));
+    se2 = new SweepEvent([0.5, 0.5], true, new SweepEvent([1, 0], false));
+    t.equal(compareSegments(se1, se2), +1);
+    t.equal(compareSegments(se2, se1), -1);
+
+    // shape: T
+    se1 = new SweepEvent([0, 1],  true, new SweepEvent([1, 1], false));
+    se2 = new SweepEvent([0.5, 1], true, new SweepEvent([0.5, 0], false));
+    t.equal(compareSegments(se1, se2), +1);
+    t.equal(compareSegments(se2, se1), -1);
+
+    // shape: T upside down
+    se1 = new SweepEvent([0, 0],  true, new SweepEvent([1, 0], false));
+    se2 = new SweepEvent([0.5, 1], true, new SweepEvent([0.5, 0], false));
+    t.equal(compareSegments(se1, se2), -1);
+    t.equal(compareSegments(se2, se1), +1);
+
+    t.end();
+  });
+
   main.end();
 });

test/edge_cases.test.js

@@ -217,6 +217,9 @@ tap.test('Edge cases', (main) => {
   });
 
   main.test('infinite loop crash', (t) => {
+    // This test case currently only ensures that there is no infinite loop.
+    // The specified result is still invalid. See discussion at:
+    // https://github.com/w8r/martinez/issues/114
     const p1 = [[
       [180.60987101280907, 22.943242898435663],
       [280.6098710128091, 22.943242898435663],
@@ -242,11 +245,10 @@ tap.test('Edge cases', (main) => {
       [-5.65625, 110.828125]
     ]];
     const r = [[[
-      [198.56943964860557, 22.943242898435663],
-      [253.09375, 40.984375],
-      [260.125, 59.265625],
-      [280.6098710128091, 60.657800699899646],
-      [280.6098710128091, 62.94324289843566]
+      [180.60987101280907, 22.943242898435663],
+      [180.60987101280907, 62.94324289843566],
+      [280.6098710128091, 62.94324289843566],
+      [280.6098710128091, 22.943242898435663],
     ]]];
 
     t.deepEqual(martinez.intersection(p1, p2), r);
@@ -475,5 +477,45 @@ tap.test('Edge cases', (main) => {
     t.end();
   });
 
+  main.test('issue #97', (t) => {
+    const p1 = [[
+      [0, 0],
+      [1, 0],
+      [1, 1],
+      [0, 1],
+      [0, 0]
+    ]];
+    const p2 = [[
+      [0.5, 0.0],
+      [0.6, 0.5],
+      [0.7, 0.5],
+      [0.5, 0.0]
+    ]];
+    const result = martinez.intersection(p1, p2);
+    t.deepEqual(result, [[[
+      [0.5, 0],
+      [0.7, 0.5],
+      [0.6, 0.5],
+      [0.5, 0],
+    ]]]);
+    t.end();
+  });
+
+  main.test('issue #110', (t) => {
+    const p1 = [[[115, 96], [140, 206], [120, 210], [125, 250], [80, 300], [115, 96]]];
+    const p2 = [[[111, 228], [129, 192], [309, 282], [111, 228]]];
+    const result = martinez.union(p1, p2);
+    t.deepEqual(result, [[[
+      [80, 300],
+      [115, 96],
+      [137.82051282051282, 196.4102564102564],
+      [309, 282],
+      [122.6470588235294, 231.1764705882353],
+      [125, 250],
+      [80, 300],
+    ]]]);
+    t.end();
+  });
+
   main.end();
 });

test/fixtures/issue110.geojson

@@ -5,14 +5,14 @@
     "properties": {},
     "geometry": {
       "type": "Polygon",
-      "coordinates": [[[115,96], [140,206], [120,210], [125,250], [80,300]]]
+      "coordinates": [[[115,96], [140,206], [120,210], [125,250], [80,300], [115,96]]]
     }
   },{
     "type": "Feature",
     "properties": {},
     "geometry": {
       "type": "Polygon",
-      "coordinates": [[[111,228], [129,192], [309,282]]]
+      "coordinates": [[[111,228], [129,192], [309,282], [111,228]]]
     }
   }]
 }