From 352e238bae75ae438bed3bd3dc474480a6359656 Mon Sep 17 00:00:00 2001 From: Martin Davis <mtnclimb@gmail.com> Date: Wed, 1 Sep 2021 18:03:03 -0700 Subject: [PATCH 1/2] Update comments Signed-off-by: Martin Davis <mtnclimb@gmail.com> --- .../java/org/locationtech/jts/geom/util/GeometryFixer.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/core/src/main/java/org/locationtech/jts/geom/util/GeometryFixer.java b/modules/core/src/main/java/org/locationtech/jts/geom/util/GeometryFixer.java index 1f592f779b..626327a523 100644 --- a/modules/core/src/main/java/org/locationtech/jts/geom/util/GeometryFixer.java +++ b/modules/core/src/main/java/org/locationtech/jts/geom/util/GeometryFixer.java @@ -299,9 +299,8 @@ private Geometry fixHoles(Polygon geom) { } private Geometry fixRing(LinearRing ring) { - //-- always execute fix, since it may remove repeated coords etc + //-- always execute fix, since it may remove repeated/invalid coords etc Geometry poly = factory.createPolygon(ring); - // TOD: check if buffer removes invalid coordinates return BufferOp.bufferByZero(poly, true); } From 612cc6775b91a1c4f307558f683f0f76149b5f9e Mon Sep 17 00:00:00 2001 From: Martin Davis <mtnclimb@gmail.com> Date: Thu, 2 Sep 2021 15:43:01 -0700 Subject: [PATCH 2/2] Change behaviour of GeometryFixer for holes outside polygons Signed-off-by: Martin Davis <mtnclimb@gmail.com> --- .../jts/geom/util/GeometryFixer.java | 102 +++++++++++++++--- .../jts/geom/util/GeometryFixerTest.java | 5 + .../perf/geom/util/GeometryFixerFuzzer.java | 58 +++++++--- 3 files changed, 134 insertions(+), 31 deletions(-) diff --git a/modules/core/src/main/java/org/locationtech/jts/geom/util/GeometryFixer.java b/modules/core/src/main/java/org/locationtech/jts/geom/util/GeometryFixer.java index 626327a523..94a9ba5181 100644 --- a/modules/core/src/main/java/org/locationtech/jts/geom/util/GeometryFixer.java +++ b/modules/core/src/main/java/org/locationtech/jts/geom/util/GeometryFixer.java @@ -26,6 +26,8 @@ import org.locationtech.jts.geom.MultiPolygon; import org.locationtech.jts.geom.Point; import org.locationtech.jts.geom.Polygon; +import org.locationtech.jts.geom.prep.PreparedGeometry; +import org.locationtech.jts.geom.prep.PreparedGeometryFactory; import org.locationtech.jts.operation.buffer.BufferOp; import org.locationtech.jts.operation.overlayng.OverlayNG; import org.locationtech.jts.operation.overlayng.OverlayNGRobust; @@ -45,18 +47,23 @@ * <li>Empty atomic geometries are valid and are returned unchanged</li> * <li>Empty elements are removed from collections</li> * <li><code>Point</code>: keep valid coordinate, or EMPTY</li> - * <li><code>LineString</code>: fix coordinate list</li> - * <li><code>LinearRing</code>: fix coordinate list, return as valid ring or else <code>LineString</code></li> + * <li><code>LineString</code>: coordinates are fixed</li> + * <li><code>LinearRing</code>: coordinates are fixed. Keep valid ring, or else convert into <code>LineString</code></li> * <li><code>Polygon</code>: transform into a valid polygon, - * preserving as much of the extent and vertices as possible</li> - * <li><code>MultiPolygon</code>: fix each polygon, - * then ensure result is non-overlapping (via union)</li> - * <li><code>GeometryCollection</code>: fix each element</li> + * preserving as much of the extent and vertices as possible. + * <ul> + * <li>Rings are fixed to ensure they are valid</li> + * <li>Holes intersecting the shell are subtracted from the shell</li> + * <li>Holes outside the shell are converted into polygons</li> + * </ul></li> + * <li><code>MultiPolygon</code>: each polygon is fixed, + * then result made non-overlapping (via union)</li> + * <li><code>GeometryCollection</code>: each element is fixed</li> * <li>Collapsed lines and polygons are handled as follows, * depending on the <code>keepCollapsed</code> setting: * <ul> - * <li><code>false</code>: (default) collapses are converted to empty geometries</li> - * <li><code>true</code>: collapses are converted to a valid geometry of lower dimension</li> + * <li><code>false</code>: (default) collapses are converted to empty geometries</li> + * <li><code>true</code>: collapses are converted to a valid geometry of lower dimension</li> * </ul> * </li> * </ol> @@ -263,25 +270,85 @@ private Geometry fixPolygonElement(Polygon geom) { if (isKeepCollapsed) { return fixLineString(shell); } - //-- if not allowing collapses then return empty polygon + //--- if not allowing collapses then return empty polygon return null; } - // if no holes then done + //--- if no holes then done if (geom.getNumInteriorRing() == 0) { return fixShell; } - Geometry fixHoles = fixHoles(geom); - Geometry result = removeHoles(fixShell, fixHoles); + + //--- fix holes, classify, and construct shell-true holes + List<Geometry> holesFixed = fixHoles(geom); + List<Geometry> holes = new ArrayList<Geometry>(); + List<Geometry> shells = new ArrayList<Geometry>(); + classifyHoles(fixShell, holesFixed, holes, shells); + Geometry polyWithHoles = difference(fixShell, holes); + if (shells.size() == 0) { + return polyWithHoles; + } + + //--- if some holes converted to shells, union all shells + shells.add(polyWithHoles); + Geometry result = union(shells); return result; } - private Geometry removeHoles(Geometry shell, Geometry holes) { - if (holes == null) + private List<Geometry> fixHoles(Polygon geom) { + List<Geometry> holes = new ArrayList<Geometry>(); + for (int i = 0; i < geom.getNumInteriorRing(); i++) { + Geometry holeRep = fixRing(geom.getInteriorRingN(i)); + if (holeRep != null) { + holes.add(holeRep); + } + } + return holes; + } + + private void classifyHoles(Geometry shell, List<Geometry> holesFixed, List<Geometry> holes, List<Geometry> shells) { + PreparedGeometry shellPrep = PreparedGeometryFactory.prepare(shell); + for (Geometry hole : holesFixed) { + if (shellPrep.intersects(hole)) { + holes.add(hole); + } + else { + shells.add(hole); + } + } + } + + /** + * Subtracts a list of polygonal geometries from a polygonal geometry. + * + * @param shell polygonal geometry for shell + * @param holes polygonal geometries to subtract + * @return the result geometry + */ + private Geometry difference(Geometry shell, List<Geometry> holes) { + if (holes == null || holes.size() == 0) return shell; - return OverlayNGRobust.overlay(shell, holes, OverlayNG.DIFFERENCE); + Geometry holesUnion = union(holes); + return OverlayNGRobust.overlay(shell, holesUnion, OverlayNG.DIFFERENCE); + } + + /** + * Unions a list of polygonal geometries. + * Optimizes case of zero or one input geometries. + * Requires that the inputs are net new objects. + * + * @param polys the polygonal geometries to union + * @return the union of the inputs + */ + private Geometry union(List<Geometry> polys) { + if (polys.size() == 0) return factory.createPolygon(); + if (polys.size() == 1) { + return polys.get(0); + } + // TODO: replace with holes.union() once OverlayNG is the default + return OverlayNGRobust.union(polys); } - private Geometry fixHoles(Polygon geom) { + private Geometry OLDfixHoles(Polygon geom) { List<Geometry> holes = new ArrayList<Geometry>(); for (int i = 0; i < geom.getNumInteriorRing(); i++) { Geometry holeRep = fixRing(geom.getInteriorRingN(i)); @@ -300,6 +367,7 @@ private Geometry fixHoles(Polygon geom) { private Geometry fixRing(LinearRing ring) { //-- always execute fix, since it may remove repeated/invalid coords etc + // TODO: would it be faster to check ring validity first? Geometry poly = factory.createPolygon(ring); return BufferOp.bufferByZero(poly, true); } @@ -317,7 +385,7 @@ private Geometry fixMultiPolygon(MultiPolygon geom) { return factory.createMultiPolygon(); } // TODO: replace with polys.union() once OverlayNG is the default - Geometry result = OverlayNGRobust.union(polys); + Geometry result = union(polys); return result; } diff --git a/modules/core/src/test/java/org/locationtech/jts/geom/util/GeometryFixerTest.java b/modules/core/src/test/java/org/locationtech/jts/geom/util/GeometryFixerTest.java index 4b049b5832..4f82b08baf 100644 --- a/modules/core/src/test/java/org/locationtech/jts/geom/util/GeometryFixerTest.java +++ b/modules/core/src/test/java/org/locationtech/jts/geom/util/GeometryFixerTest.java @@ -253,6 +253,11 @@ public void testPolygonHoleKeepCollapse() { "POLYGON ((10 10, 10 90, 90 90, 90 10, 10 10))"); } + public void testPolygonHoleOverlapAndOutsideOverlap() { + checkFix("POLYGON ((50 90, 80 90, 80 10, 50 10, 50 90), (70 80, 90 80, 90 20, 70 20, 70 80), (40 80, 40 50, 0 50, 0 80, 40 80), (30 40, 10 40, 10 60, 30 60, 30 40), (60 70, 80 70, 80 30, 60 30, 60 70))", + "MULTIPOLYGON (((10 40, 10 50, 0 50, 0 80, 40 80, 40 50, 30 50, 30 40, 10 40)), ((70 80, 70 70, 60 70, 60 30, 70 30, 70 20, 80 20, 80 10, 50 10, 50 90, 80 90, 80 80, 70 80)))"); + } + //---------------------------------------- public void testMultiPolygonEmpty() { diff --git a/modules/core/src/test/java/test/jts/perf/geom/util/GeometryFixerFuzzer.java b/modules/core/src/test/java/test/jts/perf/geom/util/GeometryFixerFuzzer.java index 1bd5d20c4e..e93de13d27 100644 --- a/modules/core/src/test/java/test/jts/perf/geom/util/GeometryFixerFuzzer.java +++ b/modules/core/src/test/java/test/jts/perf/geom/util/GeometryFixerFuzzer.java @@ -15,11 +15,15 @@ import org.locationtech.jts.geom.Geometry; import org.locationtech.jts.geom.GeometryFactory; import org.locationtech.jts.geom.LinearRing; +import org.locationtech.jts.geom.Point; +import org.locationtech.jts.geom.Polygon; import org.locationtech.jts.geom.util.GeometryFixer; public class GeometryFixerFuzzer { + private static final int GEOM_EXTENT_SIZE = 100; private static final int NUM_ITER = 10000; + private static final boolean IS_VERBOSE = false; public static void main(String[] args) { GeometryFixerFuzzer.run(); @@ -37,33 +41,38 @@ public GeometryFixerFuzzer() { } private void run(int numIter) { + System.out.println("GeometryFixer fuzzer: iterations = " + numIter); for (int i = 0; i < numIter; i++) { int numHoles = (int) (10 * Math.random()); - Geometry invalidPoly = createRandomPoly(100, numHoles); + //Geometry invalidPoly = createRandomLinePoly(100, numHoles); + Geometry invalidPoly = createRandomCirclePoly(100, numHoles); Geometry result = GeometryFixer.fix(invalidPoly); boolean isValid = result.isValid(); - String status = isValid ? "valid" : "INVALID"; - String msg = String.format("%d: Pts - input %d, output %d - %s", - i, invalidPoly.getNumPoints(), result.getNumPoints(), status); - //System.out.println(invalidPoly); - if (! isValid) { - System.out.println(msg); - System.out.println(invalidPoly); - } + report(i, invalidPoly, result, isValid); } } - private Geometry createRandomPoly(int numPoints, int numHoles) { + private void report(int i, Geometry invalidPoly, Geometry result, boolean isValid) { + String status = isValid ? "valid" : "INVALID"; + String msg = String.format("%d: Pts - input %d, output %d - %s", + i, invalidPoly.getNumPoints(), result.getNumPoints(), status); + if (IS_VERBOSE || ! isValid) { + System.out.println(msg); + System.out.println(invalidPoly); + } + } + + private Geometry createRandomLinePoly(int numPoints, int numHoles) { int numRingPoints = numPoints / (numHoles + 1); - LinearRing shell = createRandomRing(numRingPoints); + LinearRing shell = createRandomLineRing(numRingPoints); LinearRing[] holes = new LinearRing[numHoles]; for (int i = 0; i < numHoles; i++) { - holes[i] = createRandomRing(numRingPoints); + holes[i] = createRandomLineRing(numRingPoints); } return factory.createPolygon(shell, holes); } - private LinearRing createRandomRing(int numPoints) { + private LinearRing createRandomLineRing(int numPoints) { return factory.createLinearRing(createRandomPoints(numPoints)); } @@ -78,7 +87,28 @@ private Coordinate[] createRandomPoints(int numPoints) { } private double randOrd() { - double ord = 100 * Math.random(); + double ord = GEOM_EXTENT_SIZE * Math.random(); return ord; } + + private Geometry createRandomCirclePoly(int numPoints, int numHoles) { + int numRingPoints = numPoints / (numHoles + 1); + LinearRing shell = ceateRandomCircleRing(numRingPoints); + LinearRing[] holes = new LinearRing[numHoles]; + for (int i = 0; i < numHoles; i++) { + holes[i] = ceateRandomCircleRing(numRingPoints); + } + return factory.createPolygon(shell, holes); + } + + private LinearRing ceateRandomCircleRing(int numPoints) { + int numQuadSegs = (numPoints / 4) + 1; + if (numQuadSegs < 3) numQuadSegs = 3; + + Coordinate p = new Coordinate(randOrd(), randOrd()); + Point pt = factory.createPoint( p ); + double radius = GEOM_EXTENT_SIZE * Math.random() / 2; + Polygon buffer = (Polygon) pt.buffer(radius, numQuadSegs); + return buffer.getExteriorRing(); + } }