Fix buffer to use largest enclosed area for invalid rings (#655)

* Fix buffer to use largest enclosed area for invalid rings 

Signed-off-by: Martin Davis <[email protected]>

modules/core/src/main/java/org/locationtech/jts/algorithm/Orientation.java

@@ -21,7 +21,8 @@
  * Orientation is a fundamental property of planar geometries 
  * (and more generally geometry on two-dimensional manifolds).
  * <p>
- * Orientation is notoriously subject to numerical precision errors
+ * Determining triangle orientation 
+ * is notoriously subject to numerical precision errors
  * in the case of collinear or nearly collinear points.  
  * JTS uses extended-precision arithmetic to increase
  * the robustness of the computation.
@@ -104,7 +105,7 @@ public static int index(Coordinate p1, Coordinate p2, Coordinate q)
   }
 
   /**
-   * Computes whether a ring defined by an array of {@link Coordinate}s is
+   * Tests if a ring defined by an array of {@link Coordinate}s is
    * oriented counter-clockwise.
    * <ul>
    * <li>The list of points is assumed to have the first and last points equal.
@@ -129,7 +130,7 @@ public static boolean isCCW(Coordinate[] ring)
   }
 
   /**
-   * Computes whether a ring defined by a {@link CoordinateSequence} is
+   * Tests if a ring defined by a {@link CoordinateSequence} is
    * oriented counter-clockwise.
    * <ul>
    * <li>The list of points is assumed to have the first and last points equal.
@@ -234,4 +235,32 @@ public static boolean isCCW(CoordinateSequence ring)
       return delX < 0;
     }
   }
+
+  /**
+   * Tests if a ring defined by an array of {@link Coordinate}s is
+   * oriented counter-clockwise, using the signed area of the ring.
+   * <ul>
+   * <li>The list of points is assumed to have the first and last points equal.
+   * <li>This handles coordinate lists which contain repeated points.
+   * <li>This handles rings which contain collapsed segments 
+   *     (in particular, along the top of the ring).
+   * <li>This handles rings which are invalid due to self-intersection
+   * </ul>
+   * This algorithm is guaranteed to work with valid rings.
+   * For invalid rings (containing self-intersections),   
+   * the algorithm determines the orientation of
+   * the largest enclosed area (including overlaps).
+   * This provides a more useful result in some situations, such as buffering.
+   * <p>
+   * However, this approach may be less accurate in the case of 
+   * rings with almost zero area.
+   * (Note that the orientation of rings with zero area is essentially
+   * undefined, and hence non-deterministic.)
+   * 
+   * @param ring an array of Coordinates forming a ring (with first and last point identical)
+   * @return true if the ring is oriented counter-clockwise.
+   */
+  public static boolean isCCWArea(Coordinate[] ring) {
+    return Area.ofRingSigned(ring) < 0;
+  }
 }

modules/core/src/main/java/org/locationtech/jts/operation/buffer/BufferOp.java

@@ -22,7 +22,6 @@
 import org.locationtech.jts.math.MathUtil;
 import org.locationtech.jts.noding.Noder;
 import org.locationtech.jts.noding.ScaledNoder;
-import org.locationtech.jts.noding.snapround.MCIndexSnapRounder;
 import org.locationtech.jts.noding.snapround.SnapRoundingNoder;
 
 //import debug.*;
@@ -63,13 +62,19 @@
  * <li>{@link BufferParameters#JOIN_BEVEL} - corners are beveled (clipped off).
  * </ul>
  * <p>
- * The buffer algorithm can perform simplification on the input to increase performance.
+ * The buffer algorithm may perform simplification on the input to increase performance.
  * The simplification is performed a way that always increases the buffer area 
  * (so that the simplified input covers the original input).
  * The degree of simplification can be {@link BufferParameters#setSimplifyFactor(double) specified},
  * with a {@link BufferParameters#DEFAULT_SIMPLIFY_FACTOR default} used otherwise.
  * Note that if the buffer distance is zero then so is the computed simplify tolerance, 
  * no matter what the simplify factor.
+ * <p>
+ * Buffer results are always valid geometry.
+ * Given this, computing a zero-width buffer of an invalid polygonal geometry is
+ * an effective way to "validify" the geometry.
+ * Note however that in the case of self-intersecting "bow-tie" geometries,
+ * only the largest enclosed area will be retained.
  *
  * @version 1.7
  */

modules/core/src/main/java/org/locationtech/jts/operation/buffer/OffsetCurveSetBuilder.java

@@ -241,8 +241,18 @@ private void addRingSide(Coordinate[] coord, double offsetDistance, int side, in
 
     int leftLoc  = cwLeftLoc;
     int rightLoc = cwRightLoc;
+    /*
+     * Use area-based orientation test, 
+     * to ensure that for invalid rings the largest enclosed area
+     * is used to determine orientation.
+     * This produces a more sensible result especially when
+     * used for validifying polygonal geometry via buffer-by-zero.
+     * For buffering use, the lower robustness of ccw-by-area
+     * doesn't matter, since very narrow or flat rings
+     * produce an acceptable offset curve for either orientation.
+     */
     if (coord.length >= LinearRing.MINIMUM_VALID_SIZE 
-        && Orientation.isCCW(coord)) {
+      && Orientation.isCCWArea(coord)) {
       leftLoc = cwRightLoc;
       rightLoc = cwLeftLoc;
       side = Position.opposite(side);

modules/core/src/test/java/org/locationtech/jts/algorithm/IsCCWTest.java → modules/core/src/test/java/org/locationtech/jts/algorithm/OrientationIsCCWTest.java

@@ -15,24 +15,21 @@
 import org.locationtech.jts.geom.CoordinateSequence;
 import org.locationtech.jts.geom.Geometry;
 import org.locationtech.jts.geom.Polygon;
-import org.locationtech.jts.io.ParseException;
-import org.locationtech.jts.io.WKTReader;
 
-import junit.framework.TestCase;
 import junit.textui.TestRunner;
 import test.jts.GeometryTestCase;
 
 /**
- * Tests CGAlgorithms.isCCW
+ * Tests Orientation isCCW
  * @version 1.7
  */
-public class IsCCWTest extends GeometryTestCase {
+public class OrientationIsCCWTest extends GeometryTestCase {
 
   public static void main(String args[]) {
-    TestRunner.run(IsCCWTest.class);
+    TestRunner.run(OrientationIsCCWTest.class);
   }
 
-  public IsCCWTest(String name) { super(name); }
+  public OrientationIsCCWTest(String name) { super(name); }
 
   public void testTooFewPoints() {
     Coordinate[] pts = new Coordinate[] {
@@ -51,71 +48,83 @@ public void testTooFewPoints() {
   }
 
   public void testCCW() {
-    checkOrientationCCW(true, "POLYGON ((60 180, 140 120, 100 180, 140 240, 60 180))");
+    checkCCW(true, "POLYGON ((60 180, 140 120, 100 180, 140 240, 60 180))");
   }
 
   public void testRingCW() {
-    checkOrientationCCW(false, "POLYGON ((60 180, 140 240, 100 180, 140 120, 60 180))");
+    checkCCW(false, "POLYGON ((60 180, 140 240, 100 180, 140 120, 60 180))");
   }
 
   public void testCCWSmall() {
-    checkOrientationCCW(true, "POLYGON ((1 1, 9 1, 5 9, 1 1))");
+    checkCCW(true, "POLYGON ((1 1, 9 1, 5 9, 1 1))");
   }
 
   public void testDuplicateTopPoint() {
-    checkOrientationCCW(true, "POLYGON ((60 180, 140 120, 100 180, 140 240, 140 240, 60 180))");
+    checkCCW(true, "POLYGON ((60 180, 140 120, 100 180, 140 240, 140 240, 60 180))");
   }
 
   public void testFlatTopSegment() {
-    checkOrientationCCW(false, "POLYGON ((100 200, 200 200, 200 100, 100 100, 100 200))");
+    checkCCW(false, "POLYGON ((100 200, 200 200, 200 100, 100 100, 100 200))");
   }
 
   public void testFlatMultipleTopSegment() {
-    checkOrientationCCW(false, "POLYGON ((100 200, 127 200, 151 200, 173 200, 200 200, 100 100, 100 200))");
+    checkCCW(false, "POLYGON ((100 200, 127 200, 151 200, 173 200, 200 200, 100 100, 100 200))");
   }
 
   public void testDegenerateRingHorizontal() {
-    checkOrientationCCW(false, "POLYGON ((100 200, 100 200, 200 200, 100 200))");
+    checkCCW(false, "POLYGON ((100 200, 100 200, 200 200, 100 200))");
   }
 
   public void testDegenerateRingAngled() {
-    checkOrientationCCW(false, "POLYGON ((100 100, 100 100, 200 200, 100 100))");
+    checkCCW(false, "POLYGON ((100 100, 100 100, 200 200, 100 100))");
   }
 
   public void testDegenerateRingVertical() {
-    checkOrientationCCW(false, "POLYGON ((200 100, 200 100, 200 200, 200 100))");
+    checkCCW(false, "POLYGON ((200 100, 200 100, 200 200, 200 100))");
   }
 
   /**
    * This case is an invalid ring, so answer is a default value
    */
   public void testTopAngledSegmentCollapse() {
-    checkOrientationCCW(false, "POLYGON ((10 20, 61 20, 20 30, 50 60, 10 20))");
+    checkCCW(false, "POLYGON ((10 20, 61 20, 20 30, 50 60, 10 20))");
   }
 
   public void testABATopFlatSegmentCollapse() {
-    checkOrientationCCW(true, "POLYGON ((71 0, 40 40, 70 40, 40 40, 20 0, 71 0))");
+    checkCCW(true, "POLYGON ((71 0, 40 40, 70 40, 40 40, 20 0, 71 0))");
   }
 
   public void testABATopFlatSegmentCollapseMiddleStart() {
-    checkOrientationCCW(true, "POLYGON ((90 90, 50 90, 10 10, 90 10, 50 90, 90 90))");
+    checkCCW(true, "POLYGON ((90 90, 50 90, 10 10, 90 10, 50 90, 90 90))");
   }
 
   public void testMultipleTopFlatSegmentCollapseSinglePoint() {
-    checkOrientationCCW(true, "POLYGON ((100 100, 200 100, 150 200, 170 200, 200 200, 100 200, 150 200, 100 100))");
+    checkCCW(true, "POLYGON ((100 100, 200 100, 150 200, 170 200, 200 200, 100 200, 150 200, 100 100))");
   }
 
   public void testMultipleTopFlatSegmentCollapseFlatTop() {
-    checkOrientationCCW(true, "POLYGON ((10 10, 90 10, 70 70, 90 70, 10 70, 30 70, 50 70, 10 10))");
+    checkCCW(true, "POLYGON ((10 10, 90 10, 70 70, 90 70, 10 70, 30 70, 50 70, 10 10))");
   }
 
-  private void checkOrientationCCW(boolean expectedCCW, String wkt) {
+  /**
+   * Signed-area orientation returns orientation of largest enclosed area
+   */
+  public void testBowTieByArea() {
+    checkCCWArea(false, "POLYGON ((10 10, 50 10, 25 35, 35 35, 10 10))");
+  }
+
+  private void checkCCW(boolean expectedCCW, String wkt) {
     Coordinate[] pts2x = getCoordinates(wkt);
     assertEquals("Coordinate array isCCW: ", expectedCCW, Orientation.isCCW(pts2x) );
     CoordinateSequence seq2x = getCoordinateSequence(wkt);
     assertEquals("CoordinateSequence isCCW: ", expectedCCW, Orientation.isCCW(seq2x) );
   }
 
+  private void checkCCWArea(boolean expectedCCW, String wkt) {
+    Coordinate[] pts = getCoordinates(wkt);
+    assertEquals("Coordinate array isCCW: ", expectedCCW, Orientation.isCCW(pts) );
+  }
+
   private Coordinate[] getCoordinates(String wkt)
   {
     Geometry geom = read(wkt);

modules/core/src/test/java/org/locationtech/jts/operation/buffer/BufferTest.java

@@ -495,7 +495,7 @@ public void testFloatingPrecision14() throws Exception {
       .test();
   }
 
-  public void testQuickPolygonUnion() throws Exception {
+  public void testQuickPolygonUnion() {
     Geometry a = read("POLYGON((0 0, 100 0, 100 100, 0 100, 0 0))");
     Geometry b = read("POLYGON((50 50, 150 50, 150 150, 50 150, 50 50))");
     Geometry[] polygons = new Geometry[] {a, b};
@@ -504,4 +504,24 @@ public void testQuickPolygonUnion() throws Exception {
     //System.out.println(union);
     assertEquals("POLYGON ((0 0, 0 100, 50 100, 50 150, 150 150, 150 50, 100 50, 100 0, 0 0))", union.toString());
   }
+
+  /**
+   * This now works since buffer ring orientation is changed to use signed-area test.
+   */
+  public void testBowtiePolygonLargestAreaRetained() {
+    Geometry a = read("POLYGON ((10 10, 50 10, 25 35, 35 35, 10 10))");
+    Geometry result = a.buffer(0);
+    Geometry expected = read("POLYGON ((10 10, 30 30, 50 10, 10 10))");
+    checkEqual(expected, result);
+  }
+
+  /**
+   * This now works since buffer ring orientation is changed to use signed-area test.
+   */
+  public void testBowtiePolygonHoleLargestAreaRetained() {
+    Geometry a = read("POLYGON ((0 40, 60 40, 60 0, 0 0, 0 40), (10 10, 50 10, 25 35, 35 35, 10 10))");
+    Geometry result = a.buffer(0);
+    Geometry expected = read("POLYGON ((0 40, 60 40, 60 0, 0 0, 0 40), (10 10, 50 10, 30 30, 10 10))");
+    checkEqual(expected, result);
+  }
 }

modules/core/src/test/java/org/locationtech/jts/simplify/DouglasPeuckerSimplifierTest.java

@@ -17,13 +17,14 @@
 import org.locationtech.jts.io.WKTReader;
 
 import junit.framework.TestCase;
+import test.jts.GeometryTestCase;
 
 
 /**
  * @version 1.7
  */
 public class DouglasPeuckerSimplifierTest
-    extends TestCase
+    extends GeometryTestCase
 {
   public DouglasPeuckerSimplifierTest(String name) {
     super(name);
@@ -152,6 +153,27 @@ public void testGeometryCollection() throws Exception {
       ,10.0))
         .test();
   }
+
+  /**
+   * Test that a polygon made invalid by simplification
+   * is fixed in a sensible way.
+   * Fixed by buffer(0) area-base orientation
+   * See https://github.com/locationtech/jts/issues/498
+   */
+  public void testInvalidPolygonFixed() {
+    checkDP(
+        "POLYGON ((21.32686 47.78723, 21.32386 47.79023, 21.32186 47.80223, 21.31486 47.81023, 21.32786 47.81123, 21.33986 47.80223, 21.33886 47.81123, 21.32686 47.82023, 21.32586 47.82723, 21.32786 47.82323, 21.33886 47.82623, 21.34186 47.82123, 21.36386 47.82223, 21.40686 47.81723, 21.32686 47.78723))", 
+        0.0036,
+        "POLYGON ((21.32686 47.78723, 21.31486 47.81023, 21.32786 47.81123, 21.33986 47.80223, 21.328068201892744 47.823286782334385, 21.33886 47.82623, 21.34186 47.82123, 21.40686 47.81723, 21.32686 47.78723))"
+        );
+  }
+
+  private void checkDP(String wkt, double tolerance, String wktExpected) {
+    Geometry geom = read(wkt);
+    Geometry result = DouglasPeuckerSimplifier.simplify(geom, tolerance);
+    Geometry expected = read(wktExpected);
+    checkEqual(expected, result);
+  }
 }
 
 class DPSimplifierResult