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 a154a84618..a32cd22f90 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/FeatureMerge.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/FeatureMerge.java @@ -68,6 +68,20 @@ public class FeatureMerge { /** Don't instantiate */ private FeatureMerge() {} + /** + * Modifies a feature ID to end in 0, indicating the feature was created by merging and doesn't correspond to a + * single source element. + *

+ * For OSM features, IDs are encoded as {@code osm_id * 10 + element_type} where element_type is 1 for nodes, 2 for + * ways, and 3 for relations. IDs ending in 0 are reserved for features that don't map to a single OSM element. + * + * @param id the original feature ID + * @return the modified ID ending in 0 + */ + private static long makeMergedId(long id) { + return (id / 10) * 10; + } + /** * Combines linestrings with the same set of attributes into a multilinestring where segments with touching endpoints * are merged by {@link LineMerger}, removing any linestrings under {@code minLength}. @@ -141,7 +155,7 @@ private static List mergeGeometries( .sorted(BY_HILBERT_INDEX) .map(d -> d.feature.geometry()) .forEachOrdered(combined); - result.add(feature1.copyWithNewGeometry(combined.finish())); + result.add(feature1.copyWithIdAndGeometry(makeMergedId(feature1.id()), combined.finish())); } } return result; @@ -204,7 +218,12 @@ public static List mergeLineStrings(List if (!outputSegments.isEmpty()) { outputSegments = sortByHilbertIndex(outputSegments); Geometry newGeometry = GeoUtils.combineLineStrings(outputSegments); - result.add(feature1.copyWithNewGeometry(newGeometry)); + if (groupedFeatures.size() > 1) { + result.add(feature1.copyWithIdAndGeometry(makeMergedId(feature1.id()), + VectorTile.encodeGeometry(newGeometry))); + } else { + result.add(feature1.copyWithNewGeometry(newGeometry)); + } } } } @@ -363,7 +382,12 @@ public static List mergeNearbyPolygons(List 1) { + result.add(feature1.copyWithIdAndGeometry(makeMergedId(feature1.id()), + VectorTile.encodeGeometry(combined))); + } else { + result.add(feature1.copyWithNewGeometry(combined)); + } } } return result; diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/VectorTile.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/VectorTile.java index b7c6f7dcd2..cbcc8157dc 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/VectorTile.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/VectorTile.java @@ -1137,6 +1137,19 @@ public Feature copyWithNewGeometry(VectorGeometry newGeometry) { ); } + /** + * Returns a copy of this feature with {@code id} and {@code geometry} replaced. + */ + public Feature copyWithIdAndGeometry(long newId, VectorGeometry newGeometry) { + return new Feature( + layer, + newId, + newGeometry, + tags, + group + ); + } + /** Returns a copy of this feature with {@code extraAttrs} added to {@code attrs}. */ public Feature copyWithExtraAttrs(Map extraAttrs) { return new Feature( 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 8aaaa143f8..f3f7ada5c4 100644 --- a/planetiler-core/src/test/java/com/onthegomap/planetiler/FeatureMergeTest.java +++ b/planetiler-core/src/test/java/com/onthegomap/planetiler/FeatureMergeTest.java @@ -82,7 +82,7 @@ void mergeMergeOneLineStrings() { void dontMergeDisconnectedLineStrings() { assertEquals( List.of( - feature(1, newMultiLineString( + feature(0, newMultiLineString( newLineString(10, 10, 20, 20), newLineString(30, 30, 40, 40) ), Map.of()) @@ -122,7 +122,7 @@ void dontMergeConnectedLineStringsDifferentAttr() { void mergeConnectedLineStringsSameAttrs() { assertEquals( List.of( - feature(1, newLineString(10, 10, 30, 30), Map.of("a", 1)) + feature(0, newLineString(10, 10, 30, 30), Map.of("a", 1)) ), FeatureMerge.mergeLineStrings( List.of( @@ -188,7 +188,7 @@ void simplifyLineStringIfToleranceIsSet() { void mergeMultiLineString() { assertEquals( List.of( - feature(1, newLineString(10, 10, 40, 40), Map.of("a", 1)) + feature(0, newLineString(10, 10, 40, 40), Map.of("a", 1)) ), FeatureMerge.mergeLineStrings( List.of( @@ -211,7 +211,7 @@ void mergeLineStringIgnoreNonLineString() { List.of( feature(3, newPoint(5, 5), Map.of("a", 1)), feature(4, rectangle(50, 60), Map.of("a", 1)), - feature(1, newLineString(10, 10, 30, 30), Map.of("a", 1)) + feature(0, newLineString(10, 10, 30, 30), Map.of("a", 1)) ), FeatureMerge.mergeLineStrings( List.of( @@ -231,7 +231,7 @@ void mergeLineStringIgnoreNonLineString() { void mergeLineStringRemoveDetailOutsideTile() { assertEquals( List.of( - feature(1, newMultiLineString( + feature(0, newMultiLineString( newLineString( 10, 10, -10, 20, @@ -274,7 +274,7 @@ void mergeLineStringRemoveDetailOutsideTile() { void mergeLineStringMinLength() { assertEquals( List.of( - feature(2, newLineString(20, 20, 20, 25), Map.of("a", 1)) + feature(0, newLineString(20, 20, 20, 25), Map.of("a", 1)) ), FeatureMerge.mergeLineStrings( List.of( @@ -318,7 +318,7 @@ void mergePolygonEmptyList() throws GeometryException { void dontMergeDisconnectedPolygons() throws GeometryException { assertEquivalentFeatures( List.of( - feature(1, newMultiPolygon( + feature(0, newMultiPolygon( rectangle(10, 20), rectangle(22, 10, 30, 20) ), Map.of("a", 1)) @@ -360,7 +360,7 @@ void dontMergeConnectedPolygonsWithDifferentAttrs() throws GeometryException { void mergeConnectedPolygonsWithSameAttrs() throws GeometryException { assertEquivalentFeatures( List.of( - feature(1, rectangle(10, 10, 30, 20), Map.of("a", 1)) + feature(0, rectangle(10, 10, 30, 20), Map.of("a", 1)) ), FeatureMerge.mergeNearbyPolygons( List.of( @@ -383,7 +383,7 @@ void geometryPipelineWhenMergingOverlappingPolygons() throws GeometryException { ); assertEquivalentFeatures( List.of( - feature(1, newPolygon( + feature(0, newPolygon( 10, 10, 20, 10, 20, 20, @@ -446,7 +446,7 @@ void geometryPipelineAppliedWhenMergingSinglePolygon() throws GeometryException void mergeMultiPolygons() throws GeometryException { assertEquivalentFeatures( List.of( - feature(1, rectangle(10, 10, 40, 20), Map.of("a", 1)) + feature(0, rectangle(10, 10, 40, 20), Map.of("a", 1)) ), FeatureMerge.mergeNearbyPolygons( List.of( @@ -488,7 +488,7 @@ void mergePolygonsIgnoreNonPolygons() throws GeometryException { void mergePolygonsWithinMinDist() throws GeometryException { assertEquivalentFeatures( List.of( - feature(1, rectangle(10, 10, 30, 20), Map.of("a", 1)) + feature(0, rectangle(10, 10, 30, 20), Map.of("a", 1)) ), FeatureMerge.mergeNearbyPolygons( List.of( @@ -507,7 +507,7 @@ void mergePolygonsWithinMinDist() throws GeometryException { void mergePolygonsInsideEachother() throws GeometryException { assertEquivalentFeatures( List.of( - feature(1, rectangle(10, 40), Map.of("a", 1)) + feature(0, rectangle(10, 40), Map.of("a", 1)) ), FeatureMerge.mergeNearbyPolygons( List.of( @@ -526,7 +526,7 @@ void mergePolygonsInsideEachother() throws GeometryException { void dontMergePolygonsAboveMinDist() throws GeometryException { assertEquivalentFeatures( List.of( - feature(1, newMultiPolygon( + feature(0, newMultiPolygon( rectangle(10, 20), rectangle(21.1, 10, 30, 20) ), Map.of("a", 1)) @@ -566,7 +566,7 @@ void removePolygonsBelowMinSize() throws GeometryException { void allowPolygonsAboveMinSize() throws GeometryException { assertEquivalentFeatures( List.of( - feature(1, newMultiPolygon( + feature(0, newMultiPolygon( rectangle(10, 20), rectangle(30, 10, 40, 20) ), Map.of("a", 1)) @@ -745,7 +745,7 @@ void mergeMultiPolygon() throws GeometryException { Collections.reverse(innerRing); assertTopologicallyEquivalentFeatures( List.of( - feature(1, newPolygon(rectangleCoordList(10, 22), List.of(innerRing)), Map.of("a", 1)) + feature(0, newPolygon(rectangleCoordList(10, 22), List.of(innerRing)), Map.of("a", 1)) ), FeatureMerge.mergeNearbyPolygons( List.of( @@ -767,7 +767,7 @@ void mergeMultiPolygonExcludeSmallInnerRings() throws GeometryException { Collections.reverse(innerRing); assertTopologicallyEquivalentFeatures( List.of( - feature(1, rectangle(10, 22), Map.of("a", 1)) + feature(0, rectangle(10, 22), Map.of("a", 1)) ), FeatureMerge.mergeNearbyPolygons( List.of( @@ -844,7 +844,7 @@ void testMultigeometryMerger( assertTopologicallyEquivalentFeatures( List.of( feature(4, otherGeometry, Map.of("a", 1)), - feature(1, combineJTS.apply(List.of(geom1, geom2, geom3, geom4)), Map.of("a", 1)), + feature(0, combineJTS.apply(List.of(geom1, geom2, geom3, geom4)), Map.of("a", 1)), feature(3, geom5, Map.of("a", 2)) ), merge.apply( @@ -1016,7 +1016,7 @@ void mergeNormalizeOuterRing() throws GeometryException { void mergeFillPolygonsDoesNotNormalizeIrregularFill() throws GeometryException { assertEquivalentFeatures( List.of( - feature(1, newPolygon( + feature(0, newPolygon( -2, -2, 200, -2, 200, -1, @@ -1062,4 +1062,77 @@ void mergeLineStringZeroMinLength(double minLength, double minTolerance, double assertEquals(input.geometry().decode(), actualSingle.geometry().decode()); assertEquals(List.of(input), actual); } + + @Test + void mergedLineStringIdEndsInZero() { + var result = FeatureMerge.mergeLineStrings( + List.of( + feature(123, newLineString(10, 10, 20, 20), Map.of("a", 1)), + feature(456, newLineString(20, 20, 30, 30), Map.of("a", 1)) + ), + 0, 0, 0 + ); + assertEquals(1, result.size()); + assertEquals(120, result.getFirst().id(), "merged feature ID should end in 0"); + } + + @Test + void mergedMultiPointIdEndsInZero() { + var result = FeatureMerge.mergeMultiPoint( + List.of( + feature(123, newPoint(10, 10), Map.of("a", 1)), + feature(456, newPoint(20, 20), Map.of("a", 1)) + ) + ); + assertEquals(1, result.size()); + assertEquals(120, result.getFirst().id(), "merged feature ID should end in 0"); + } + + @Test + void mergedMultiPolygonIdEndsInZero() { + var result = FeatureMerge.mergeMultiPolygon( + List.of( + feature(123, rectangle(10, 20), Map.of("a", 1)), + feature(456, rectangle(30, 40), Map.of("a", 1)) + ) + ); + assertEquals(1, result.size()); + assertEquals(120, result.getFirst().id(), "merged feature ID should end in 0"); + } + + @Test + void mergedMultiLineStringIdEndsInZero() { + var result = FeatureMerge.mergeMultiLineString( + List.of( + feature(123, newLineString(10, 10, 20, 20), Map.of("a", 1)), + feature(456, newLineString(30, 30, 40, 40), Map.of("a", 1)) + ) + ); + assertEquals(1, result.size()); + assertEquals(120, result.getFirst().id(), "merged feature ID should end in 0"); + } + + @Test + void mergedNearbyPolygonsIdEndsInZero() throws GeometryException { + var result = FeatureMerge.mergeNearbyPolygons( + List.of( + feature(123, rectangle(10, 20), Map.of("a", 1)), + feature(456, rectangle(20, 10, 30, 20), Map.of("a", 1)) + ), + 0, 0, 0, 1 + ); + assertEquals(1, result.size()); + assertEquals(120, result.getFirst().id(), "merged feature ID should end in 0"); + } + + @Test + void singleFeatureIdUnchanged() { + var result = FeatureMerge.mergeMultiPoint( + List.of( + feature(123, newPoint(10, 10), Map.of("a", 1)) + ) + ); + assertEquals(1, result.size()); + assertEquals(123, result.getFirst().id(), "single feature ID should not be modified"); + } } diff --git a/planetiler-core/src/test/resources/exclude.txt b/planetiler-core/src/test/resources/exclude.txt index 4792dc26d1..0418d1efbf 100644 --- a/planetiler-core/src/test/resources/exclude.txt +++ b/planetiler-core/src/test/resources/exclude.txt @@ -1,2 +1,9 @@ +# Temporarily disable tests broken by https://github.com/onthegomap/planetiler/pull/1397 +org.openmaptiles.layers.BoundaryTest#testMergesDisconnectedLineFeatures +org.openmaptiles.layers.WaterwayTest#testOsmWaterwayRelation +org.openmaptiles.layers.WaterwayTest#testWaterwayImportantRiverPostProcess +org.openmaptiles.layers.TransportationTest#testMergesDisconnectedRoadNameFeatures +org.openmaptiles.layers.TransportationTest#testMergesDisconnectedRoadFeaturesUnlessOneway +org.openmaptiles.layers.TransportationTest#testMergesDisconnectedRoadFeaturesUnlessOnewayLong # Temporarily disable test broken by this fix: https://github.com/onthegomap/planetiler/pull/1404 org.openmaptiles.util.OmtLanguageUtilsTest#testLatinFallbacks diff --git a/planetiler-examples/src/test/java/com/onthegomap/planetiler/examples/BikeRouteOverlayTest.java b/planetiler-examples/src/test/java/com/onthegomap/planetiler/examples/BikeRouteOverlayTest.java index bacc61b73e..20b4b5ff84 100644 --- a/planetiler-examples/src/test/java/com/onthegomap/planetiler/examples/BikeRouteOverlayTest.java +++ b/planetiler-examples/src/test/java/com/onthegomap/planetiler/examples/BikeRouteOverlayTest.java @@ -98,7 +98,7 @@ void testTilePostProcessingMergesConnectedLines() { attrs ); // merged: (0, 0) to (20, 0) - var connected = new VectorTile.Feature(layer, 1, // id + var connected = new VectorTile.Feature(layer, 0, // id VectorTile.encodeGeometry(newLineString(0, 0, 20, 0)), attrs );