Fix buffer inverted ring removal heuristic (#1038)

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

@@ -24,6 +24,7 @@
 import org.locationtech.jts.geom.Envelope;
 import org.locationtech.jts.geom.Geometry;
 import org.locationtech.jts.geom.GeometryCollection;
+import org.locationtech.jts.geom.LineSegment;
 import org.locationtech.jts.geom.LineString;
 import org.locationtech.jts.geom.LinearRing;
 import org.locationtech.jts.geom.Location;
@@ -327,57 +328,75 @@ private void addRingSide(Coordinate[] coord, double offsetDistance, int side, in
    * <p>
    * See https://github.com/locationtech/jts/issues/472
    * 
-   * @param inputPts the input ring
+   * @param inputRing the input ring
    * @param distance the buffer distance
-   * @param curvePts the generated offset curve
+   * @param curveRing the generated offset curve ring
    * @return true if the offset curve is inverted
    */
-  private static boolean isRingCurveInverted(Coordinate[] inputPts, double distance, Coordinate[] curvePts) {
+  private static boolean isRingCurveInverted(Coordinate[] inputRing, double distance, Coordinate[] curveRing) {
     if (distance == 0.0) return false;
     /**
      * Only proper rings can invert.
      */
-    if (inputPts.length <= 3) return false;
+    if (inputRing.length <= 3) return false;
    /**
      * Heuristic based on low chance that a ring with many vertices will invert.
      * This low limit ensures this test is fairly efficient.
      */
-    if (inputPts.length >= MAX_INVERTED_RING_SIZE) return false;
+    if (inputRing.length >= MAX_INVERTED_RING_SIZE) return false;
 
     /**
      * Don't check curves which are much larger than the input.
      * This improves performance by avoiding checking some concave inputs 
      * (which can produce fillet arcs with many more vertices)
      */
-    if (curvePts.length > INVERTED_CURVE_VERTEX_FACTOR * inputPts.length) return false;
+    if (curveRing.length > INVERTED_CURVE_VERTEX_FACTOR * inputRing.length) return false;
 
     /**
-     * Check if the curve vertices are all closer to the input ring
-     * than the buffer distance.
-     * If so, the curve is NOT a valid buffer curve.
+     * If curve contains points which are on the buffer, 
+     * it is not inverted and can be included in the raw curves.
      */
-    double distTol = NEARNESS_FACTOR * Math.abs(distance);
-    double maxDist = maxDistance(curvePts, inputPts);
-    boolean isCurveTooClose = maxDist < distTol;
-    return isCurveTooClose;
+    if (hasPointOnBuffer(inputRing, distance, curveRing))
+      return false;
+
+    //-- curve is inverted, so discard it
+    return true;
   }
 
   /**
-   * Computes the maximum distance out of a set of points to a linestring.
+   * Tests if there are points on the raw offset curve which may
+   * lie on the final buffer curve
+   * (i.e. they are (approximately) at the buffer distance from the input ring). 
+   * For efficiency this only tests a limited set of points on the curve.
    * 
-   * @param pts the points
-   * @param line the linestring vertices
-   * @return the maximum distance
+   * @param inputRing
+   * @param distance
+   * @param curveRing
+   * @return true if the curve contains points lying at the required buffer distance
    */
-  private static double maxDistance(Coordinate[] pts, Coordinate[] line) {
-    double maxDistance = 0;
-    for (Coordinate p : pts) {
-      double dist = Distance.pointToSegmentString(p, line);
-      if (dist > maxDistance) {
-        maxDistance = dist;
+  private static boolean hasPointOnBuffer(Coordinate[] inputRing, double distance, Coordinate[] curveRing) {
+    double distTol = NEARNESS_FACTOR * Math.abs(distance);
+
+    for (int i = 0; i < curveRing.length - 1; i++) {
+      Coordinate v = curveRing[i];
+
+      //-- check curve vertices
+      double dist = Distance.pointToSegmentString(v, inputRing);
+      if (dist > distTol) {
+        return true; 
+      }
+
+      //-- check curve segment midpoints
+      int iNext = (i < curveRing.length - 1) ? i + 1 : 0;
+      Coordinate vnext = curveRing[iNext];
+      Coordinate midPt = LineSegment.midPoint(v, vnext);
+
+      double distMid = Distance.pointToSegmentString(midPt, inputRing);
+      if (distMid > distTol) {
+        return true; 
       }
     }
-    return maxDistance;
+    return false;
   }
 
   /**

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

@@ -575,6 +575,22 @@ public void testLineClosedNoHole() {
     checkBufferHasHole(wkt, 70, false);
   }
 
+  public void testSmallPolygonNegativeBuffer_1() {
+    String wkt = "MULTIPOLYGON (((833454.7163917861 6312507.405413097, 833455.3726665961 6312510.208920742, 833456.301153878 6312514.207390314, 833492.2432584754 6312537.770332065, 833493.0901320165 6312536.098774815, 833502.6580673696 6312517.561360772, 833503.9404352929 6312515.0542803425, 833454.7163917861 6312507.405413097)))";
+    checkBuffer(wkt, -3.8, 
+        "POLYGON ((833459.9671068499 6312512.066918822, 833490.7876785189 6312532.272283619, 833498.1465258132 6312517.999574621, 833459.9671068499 6312512.066918822))");
+    checkBuffer(wkt, -7, 
+        "POLYGON ((833474.0912127121 6312517.50004999, 833489.5713439264 6312527.648521655, 833493.2674441456 6312520.479822435, 833474.0912127121 6312517.50004999))");
+  }
+
+  public void testSmallPolygonNegativeBuffer_2() {
+    String wkt = "POLYGON ((182719.04521570954238996 224897.14115349075291306, 182807.02887436276068911 224880.64421749324537814, 182808.47314301913138479 224877.25002362736267969, 182718.38701137207681313 224740.00115247094072402, 182711.82697281913715415 224742.08599378637154587, 182717.1393717635946814 224895.61432328051887453, 182719.04521570954238996 224897.14115349075291306))";
+    checkBuffer(wkt, -5, 
+        "POLYGON ((182717 224746.99999999997, 182722.00000000003 224891.5, 182801.99999999997 224876.49999999997, 182717 224746.99999999997))");
+    checkBuffer(wkt, -30, 
+        "POLYGON ((182745.07127364463 224835.32741176756, 182745.97926048582 224861.56823147752, 182760.5070499446 224858.844270954, 182745.07127364463 224835.32741176756))");
+  }
+
   /**
    * See GEOS PR https://github.com/libgeos/geos/pull/978
    */
@@ -623,6 +639,13 @@ private void checkBuffer(String wkt, double dist, BufferParameters param, String
     checkEqual(expected, result, 0.01);
   }
 
+  private void checkBuffer(String wkt, double dist, String wktExpected) {
+    Geometry geom = read(wkt);
+    Geometry result = BufferOp.bufferOp(geom, dist);
+    Geometry expected = read(wktExpected);
+    checkEqual(expected, result, 0.01);
+  }
+
   private void checkBufferEmpty(String wkt, double dist, boolean isEmptyExpected) {
     Geometry a = read(wkt);
     Geometry result = a.buffer(dist);