From d482a59f1058358a9ae7425f4b63294bfd8ff7ca Mon Sep 17 00:00:00 2001 From: Mike Barry Date: Fri, 6 Dec 2024 07:14:52 -0500 Subject: [PATCH] fixes --- .../onthegomap/planetiler/FeatureMerge.java | 36 +++++++--- .../planetiler/geo/DualMidpointSmoother.java | 7 +- .../planetiler/geo/VWSimplifier.java | 9 ++- .../planetiler/util/LoopLineMerger.java | 22 +++++- .../planetiler/FeatureMergeTest.java | 68 +++++++++++++++++++ .../geo/DualMidpointSmootherTest.java | 2 +- .../planetiler/geo/VWSimplifierTest.java | 29 ++++---- .../planetiler/util/LoopLineMergerTest.java | 29 ++++++++ 8 files changed, 174 insertions(+), 28 deletions(-) diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/FeatureMerge.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/FeatureMerge.java index 4c86a45cbc..7f7e84ba09 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/FeatureMerge.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/FeatureMerge.java @@ -6,6 +6,7 @@ import com.onthegomap.planetiler.collection.Hppc; import com.onthegomap.planetiler.geo.GeoUtils; import com.onthegomap.planetiler.geo.GeometryException; +import com.onthegomap.planetiler.geo.GeometryPipeline; import com.onthegomap.planetiler.geo.GeometryType; import com.onthegomap.planetiler.geo.MutableCoordinateSequence; import com.onthegomap.planetiler.stats.DefaultStats; @@ -81,7 +82,7 @@ private FeatureMerge() {} */ public static List mergeLineStrings(List features, double minLength, double tolerance, double buffer, boolean resimplify) { - return mergeLineStrings(features, attrs -> minLength, tolerance, buffer, resimplify); + return mergeLineStrings(features, attrs -> minLength, tolerance, buffer, resimplify, null); } /** @@ -143,12 +144,13 @@ private static List mergeGeometries( } /** - * Merges linestrings with the same attributes as {@link #mergeLineStrings(List, Function, double, double, boolean)} - * except sets {@code resimplify=false} by default. + * Merges linestrings with the same attributes as + * {@link #mergeLineStrings(List, Function, double, double, boolean, GeometryPipeline)} except sets + * {@code resimplify=false} by default. */ public static List mergeLineStrings(List features, Function, Double> lengthLimitCalculator, double tolerance, double buffer) { - return mergeLineStrings(features, lengthLimitCalculator, tolerance, buffer, false); + return mergeLineStrings(features, lengthLimitCalculator, tolerance, buffer, false, null); } /** @@ -156,7 +158,8 @@ public static List mergeLineStrings(List * except with a dynamic length limit computed by {@code lengthLimitCalculator} for the attributes of each group. */ public static List mergeLineStrings(List features, - Function, Double> lengthLimitCalculator, double tolerance, double buffer, boolean resimplify) { + Function, Double> lengthLimitCalculator, double tolerance, double buffer, boolean resimplify, + GeometryPipeline pipeline) { List result = new ArrayList<>(features.size()); var groupedByAttrs = groupByAttrs(features, result, GeometryType.LINE); for (List groupedFeatures : groupedByAttrs) { @@ -176,7 +179,8 @@ public static List mergeLineStrings(List .setMergeStrokes(true) .setMinLength(lengthLimit) .setLoopMinLength(lengthLimit) - .setStubMinLength(0.5); + .setStubMinLength(0.5) + .setSegmentTransform(pipeline); for (VectorTile.Feature feature : groupedFeatures) { try { merger.add(feature.geometry().decode()); @@ -287,12 +291,14 @@ public static List mergeOverlappingPolygons(List mergeNearbyPolygons(List features, double minArea, - double minHoleArea, double minDist, double buffer, Stats stats) throws GeometryException { + double minHoleArea, double minDist, double buffer, Stats stats, GeometryPipeline pipeline) + throws GeometryException { List result = new ArrayList<>(features.size()); Collection> groupedByAttrs = groupByAttrs(features, result, GeometryType.POLYGON); for (List groupedFeatures : groupedByAttrs) { @@ -326,12 +332,26 @@ public static List mergeNearbyPolygons(List List sortByHilbertIndex(List geometrie public static List mergeNearbyPolygons(List features, double minArea, double minHoleArea, double minDist, double buffer) throws GeometryException { - return mergeNearbyPolygons(features, minArea, minHoleArea, minDist, buffer, DefaultStats.get()); + return mergeNearbyPolygons(features, minArea, minHoleArea, minDist, buffer, DefaultStats.get(), null); } diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/geo/DualMidpointSmoother.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/geo/DualMidpointSmoother.java index cf7dacd0cf..db61da5250 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/geo/DualMidpointSmoother.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/geo/DualMidpointSmoother.java @@ -117,6 +117,7 @@ protected CoordinateSequence transformCoordinates(CoordinateSequence coords, Geo return coords.copy(); } for (int iter = 0; iter < iters; iter++) { + assert iter < iters - 1 || (minSquaredVertexTolerance == 0 && minVertexArea == 0) : "reached max iterations"; MutableCoordinateSequence result = new MutableCoordinateSequence(); int last = coords.size() - 1; double x1, y1; @@ -180,7 +181,9 @@ private void squashVertex(MutableCoordinateSequence result, double x1, double y1 double maxDistSquared = Double.POSITIVE_INFINITY; if (maxArea > 0) { double sin = den <= 0 ? 0 : Math.abs(((x1 - x2) * (y3 - y2)) - ((y1 - y2) * (x3 - x2))) / den; - maxDistSquared = 2 * maxArea / sin; + if (sin != 0) { + maxDistSquared = 2 * maxArea / sin; + } } if (maxSquaredOffset > 0) { double cos = den <= 0 ? 0 : Math.clamp(((x1 - x2) * (x3 - x2) + (y1 - y2) * (y3 - y2)) / den, -1, 1); @@ -194,7 +197,7 @@ private void squashVertex(MutableCoordinateSequence result, double x1, double y1 if (Double.isNaN(maxDist)) { maxDist = Math.sqrt(maxDistSquared); } - nextA = maxDist / magA; + nextA = maxDist / magB; } } diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/geo/VWSimplifier.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/geo/VWSimplifier.java index 78a135e2d5..199d08cf17 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/geo/VWSimplifier.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/geo/VWSimplifier.java @@ -15,6 +15,7 @@ public class VWSimplifier extends GeometryTransformer implements GeometryPipelin private double tolerance; private double k; + private boolean keepCollapsed = false; /** Sets the minimum effective triangle area created by 3 consecutive vertices in order to retain that vertex. */ public VWSimplifier setTolerance(double tolerance) { @@ -22,6 +23,12 @@ public VWSimplifier setTolerance(double tolerance) { return this; } + /** Set to {@code true} to keep polygons with area smaller than {@code tolerance}. */ + public VWSimplifier setKeepCollapsed(boolean keepCollapsed) { + this.keepCollapsed = keepCollapsed; + return this; + } + /** * Apply a penalty from {@code k=0} to {@code k=1} to drop more sharp corners from the resulting geometry. *

@@ -72,7 +79,7 @@ public double updateArea() { @Override protected CoordinateSequence transformCoordinates(CoordinateSequence coords, Geometry parent) { boolean area = parent instanceof LinearRing; - int minPoints = area ? 4 : 2; + int minPoints = keepCollapsed && area ? 4 : 2; int num = coords.size(); if (num <= minPoints) { return coords; diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/util/LoopLineMerger.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/util/LoopLineMerger.java index 865c96de04..a80cf1a5a5 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/util/LoopLineMerger.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/util/LoopLineMerger.java @@ -2,6 +2,7 @@ import com.onthegomap.planetiler.geo.DouglasPeuckerSimplifier; import com.onthegomap.planetiler.geo.GeoUtils; +import com.onthegomap.planetiler.geo.GeometryPipeline; import java.util.ArrayList; import java.util.Comparator; import java.util.HashMap; @@ -48,6 +49,7 @@ public class LoopLineMerger { private double stubMinLength = 0.0; private double tolerance = -1.0; private boolean mergeStrokes = false; + private GeometryPipeline pipeline; /** * Sets the precision model used to snap points to a grid. @@ -105,6 +107,16 @@ public LoopLineMerger setTolerance(double tolerance) { return this; } + /** + * Sets a function that should be applied to each segment between intersections instead of simplification. + *

+ * When this is present, {@code tolerance} is ignored. + */ + public LoopLineMerger setSegmentTransform(GeometryPipeline pipeline) { + this.pipeline = pipeline; + return this; + } + /** * Enables or disables stroke merging. Stroke merging connects the straightest pairs of linestrings at junctions with * 3 or more attached linestrings based on the angle between them. @@ -388,7 +400,7 @@ public List getMergedLineStrings() { // removeShortStubEdges does degreeTwoMerge internally } - if (tolerance >= 0.0) { + if (pipeline != null || tolerance >= 0.0) { simplify(); removeDuplicatedEdges(); degreeTwoMerge(); @@ -585,7 +597,13 @@ void remove() { } void simplify() { - coordinates = DouglasPeuckerSimplifier.simplify(coordinates, tolerance, false); + if (pipeline != null) { + coordinates = List.of( + pipeline.apply(GeoUtils.JTS_FACTORY.createLineString(coordinates.toArray(Coordinate[]::new))) + .getCoordinates()); + } else if (tolerance >= 0) { + coordinates = DouglasPeuckerSimplifier.simplify(coordinates, tolerance, false); + } if (reversed != null) { reversed.coordinates = coordinates.reversed(); } diff --git a/planetiler-core/src/test/java/com/onthegomap/planetiler/FeatureMergeTest.java b/planetiler-core/src/test/java/com/onthegomap/planetiler/FeatureMergeTest.java index e27feda284..736cfba854 100644 --- a/planetiler-core/src/test/java/com/onthegomap/planetiler/FeatureMergeTest.java +++ b/planetiler-core/src/test/java/com/onthegomap/planetiler/FeatureMergeTest.java @@ -8,6 +8,7 @@ import com.carrotsearch.hppc.IntObjectMap; import com.onthegomap.planetiler.collection.Hppc; import com.onthegomap.planetiler.geo.GeometryException; +import com.onthegomap.planetiler.geo.GeometryPipeline; import com.onthegomap.planetiler.geo.GeometryType; import com.onthegomap.planetiler.mbtiles.Mbtiles; import com.onthegomap.planetiler.stats.Stats; @@ -373,6 +374,73 @@ void mergeConnectedPolygonsWithSameAttrs() throws GeometryException { ); } + @Test + void geometryPipelineWhenMergingOverlappingPolygons() throws GeometryException { + List features = List.of( + feature(1, rectangle(10, 10, 20, 19), Map.of("a", 1)), + feature(2, rectangle(11, 10, 20, 20), Map.of("a", 1)) + ); + assertEquivalentFeatures( + List.of( + feature(1, newPolygon( + 10, 10, + 20, 10, + 20, 20, + 11, 20, + // remove this point due to simplification: 11, 19, + 10, 19, + 10, 10 + ), Map.of("a", 1)) + ), + FeatureMerge.mergeNearbyPolygons( + features, + 0, + 0, + 0, + 1, + Stats.inMemory(), + GeometryPipeline.simplifyVW(1) + ) + ); + } + + @Test + void geometryPipelineAppliedWhenMergingSinglePolygon() throws GeometryException { + List features = List.of( + feature(1, newPolygon( + 10, 10, + 20, 10, + 20, 20, + 11, 20, + 11, 19, + 10, 19, + 10, 10), Map.of("a", 1)) + ); + assertEquivalentFeatures( + List.of( + feature(1, newPolygon( + 10, 10, + 20, 10, + 20, 20, + 11, 20, + // remove this point due to simplification: 11, 19, + 10, 19, + 10, 10 + ), Map.of("a", 1)) + ), + FeatureMerge.mergeNearbyPolygons( + features, + 0, + 0, + 0, + 1, + Stats.inMemory(), + GeometryPipeline.simplifyVW(1) + ) + ); + } + + @Test void mergeMultiPolygons() throws GeometryException { assertEquivalentFeatures( diff --git a/planetiler-core/src/test/java/com/onthegomap/planetiler/geo/DualMidpointSmootherTest.java b/planetiler-core/src/test/java/com/onthegomap/planetiler/geo/DualMidpointSmootherTest.java index 493bc408b0..0d05ccb25c 100644 --- a/planetiler-core/src/test/java/com/onthegomap/planetiler/geo/DualMidpointSmootherTest.java +++ b/planetiler-core/src/test/java/com/onthegomap/planetiler/geo/DualMidpointSmootherTest.java @@ -104,7 +104,7 @@ void testSmoothWithMaxArea(String inWKT, String outWKT) throws ParseException { "LINESTRING(0 0, 10 10); LINESTRING(0 0, 10 10)", "LINESTRING(0 0, 10 0, 10 10); LINESTRING (0 0, 9 0, 10 1, 10 10)", "LINESTRING(0 0, 10 0, 20 0); LINESTRING (0 0, 7.5 0, 12.5 0, 20 0)", - "LINESTRING(0 0, 10 0, 0 0); LINESTRING (0 0, 9.29289 0, 0 0)", + "LINESTRING(0 0, 10 0, 10 5); LINESTRING (0 0, 9 0, 10 1, 10 5)", "POLYGON((0 0, 10 0, 10 10, 0 10, 0 0)); POLYGON ((1 0, 9 0, 10 1, 10 9, 9 10, 1 10, 0 9, 0 1, 1 0))", }, delimiter = ';') void testSmoothWithMaxOffset(String inWKT, String outWKT) throws ParseException { diff --git a/planetiler-core/src/test/java/com/onthegomap/planetiler/geo/VWSimplifierTest.java b/planetiler-core/src/test/java/com/onthegomap/planetiler/geo/VWSimplifierTest.java index 2fee468d7d..d4a521f543 100644 --- a/planetiler-core/src/test/java/com/onthegomap/planetiler/geo/VWSimplifierTest.java +++ b/planetiler-core/src/test/java/com/onthegomap/planetiler/geo/VWSimplifierTest.java @@ -1,9 +1,6 @@ package com.onthegomap.planetiler.geo; -import static com.onthegomap.planetiler.TestUtils.assertSameNormalizedFeature; -import static com.onthegomap.planetiler.TestUtils.newLineString; -import static com.onthegomap.planetiler.TestUtils.newPoint; -import static com.onthegomap.planetiler.TestUtils.newPolygon; +import static com.onthegomap.planetiler.TestUtils.*; import org.junit.jupiter.api.Test; import org.locationtech.jts.geom.Geometry; @@ -69,22 +66,26 @@ void testKeepAPoint() { } @Test - void testPolygonLeaveAPoint() { - testSimplify( - newPolygon( - 0, 0, - 10, 10, - 9, 10, - 0, 8, - 0, 0 - ), + void testPolygonLeaveAPointWhenAreaLessThenTolerance() { + Geometry in = newPolygon( + 0, 0, + 10, 10, + 9, 10, + 0, 8, + 0, 0 + ); + assertSameNormalizedFeature( + emptyGeometry(), + new VWSimplifier().setTolerance(200).setWeight(0).transform(in) + ); + assertSameNormalizedFeature( newPolygon( 0, 0, 0, 8, 10, 10, 0, 0 ), - 200 + new VWSimplifier().setTolerance(200).setKeepCollapsed(true).setWeight(0).transform(in) ); } diff --git a/planetiler-core/src/test/java/com/onthegomap/planetiler/util/LoopLineMergerTest.java b/planetiler-core/src/test/java/com/onthegomap/planetiler/util/LoopLineMergerTest.java index 59fa2e2215..30670fc4f8 100644 --- a/planetiler-core/src/test/java/com/onthegomap/planetiler/util/LoopLineMergerTest.java +++ b/planetiler-core/src/test/java/com/onthegomap/planetiler/util/LoopLineMergerTest.java @@ -6,6 +6,7 @@ import com.onthegomap.planetiler.TestUtils; import com.onthegomap.planetiler.geo.GeoUtils; +import com.onthegomap.planetiler.geo.GeometryPipeline; import java.io.IOException; import java.nio.file.Files; import java.util.Arrays; @@ -470,6 +471,34 @@ void testMergeStrokesAt3WayIntersection() { ); } + @Test + void testSimplifyTolerance() { + var merger = new LoopLineMerger() + .setTolerance(1); + + merger.add(newLineString(0, 0, 5, 1)); + merger.add(newLineString(5, 1, 10, 0)); + + assertEquals( + List.of(newLineString(0, 0, 10, 0)), + merger.getMergedLineStrings() + ); + } + + @Test + void testGeometryPipeline() { + var merger = new LoopLineMerger() + .setSegmentTransform(GeometryPipeline.simplifyDP(1)); + + merger.add(newLineString(0, 0, 5, 1)); + merger.add(newLineString(5, 1, 10, 0)); + + assertEquals( + List.of(newLineString(0, 0, 10, 0)), + merger.getMergedLineStrings() + ); + } + @Test void testMergeStrokesAt4WayIntersection() { var merger = new LoopLineMerger()