Skip to content

When merging features, assign IDs ending in 0 to the result #1397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+142 −22
Open

When merging features, assign IDs ending in 0 to the result #1397

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0364aaf
When merging features, assign IDs ending in 0 to the result
jake-low Nov 17, 2025
af32513
Add VectorTile.Feature.copyWithIdAndGeometry helper function
jake-low Nov 21, 2025
e26a61f
Temporarily exclude failing OMT profile tests
jake-low Nov 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 43 additions & 3 deletions planetiler-core/src/main/java/com/onthegomap/planetiler/FeatureMerge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
* <p>
* 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;
}
Copy link
Contributor

@msbarry msbarry Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this! I'm realizing this will update IDs on all elements, not just OSM elements. I'm not sure that is too important as the IDs are less meaningful in other sources.

I just double checked and confirmed that all features (even non-osm ones) get multiplied by config.featureSourceIdMultiplier() so it should be safe to do this for all features without causing unwanted collisions. Could we use that instead of a hardcoded 10 constant though?


/**
* 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}.
Expand Down Expand Up @@ -141,7 +155,13 @@ private static List<VectorTile.Feature> mergeGeometries(
.sorted(BY_HILBERT_INDEX)
.map(d -> d.feature.geometry())
.forEachOrdered(combined);
result.add(feature1.copyWithNewGeometry(combined.finish()));
result.add(new VectorTile.Feature(
feature1.layer(),
makeMergedId(feature1.id()),
combined.finish(),
feature1.tags(),
feature1.group()
));
}
}
return result;
Expand Down Expand Up @@ -204,7 +224,17 @@ public static List<VectorTile.Feature> mergeLineStrings(List<VectorTile.Feature>
if (!outputSegments.isEmpty()) {
outputSegments = sortByHilbertIndex(outputSegments);
Geometry newGeometry = GeoUtils.combineLineStrings(outputSegments);
result.add(feature1.copyWithNewGeometry(newGeometry));
if (groupedFeatures.size() > 1) {
result.add(new VectorTile.Feature(
feature1.layer(),
makeMergedId(feature1.id()),
VectorTile.encodeGeometry(newGeometry),
feature1.tags(),
feature1.group()
));
} else {
result.add(feature1.copyWithNewGeometry(newGeometry));
}
}
}
}
Expand Down Expand Up @@ -363,7 +393,17 @@ public static List<VectorTile.Feature> mergeNearbyPolygons(List<VectorTile.Featu
if (!outPolygons.isEmpty()) {
outPolygons = sortByHilbertIndex(outPolygons);
Geometry combined = GeoUtils.combinePolygons(outPolygons);
result.add(feature1.copyWithNewGeometry(combined));
if (groupedFeatures.size() > 1) {
result.add(new VectorTile.Feature(
feature1.layer(),
makeMergedId(feature1.id()),
VectorTile.encodeGeometry(combined),
feature1.tags(),
feature1.group()
));
} else {
result.add(feature1.copyWithNewGeometry(combined));
}
}
}
return result;
Expand Down
109 changes: 91 additions & 18 deletions planetiler-core/src/test/java/com/onthegomap/planetiler/FeatureMergeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -231,7 +231,7 @@ void mergeLineStringIgnoreNonLineString() {
void mergeLineStringRemoveDetailOutsideTile() {
assertEquals(
List.of(
feature(1, newMultiLineString(
feature(0, newMultiLineString(
newLineString(
10, 10,
-10, 20,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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(
Expand All @@ -383,7 +383,7 @@ void geometryPipelineWhenMergingOverlappingPolygons() throws GeometryException {
);
assertEquivalentFeatures(
List.of(
feature(1, newPolygon(
feature(0, newPolygon(
10, 10,
20, 10,
20, 20,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down Expand Up @@ -844,7 +844,7 @@ <S extends Geometry, M extends GeometryCollection> 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(
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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");
}
}
2 changes: 1 addition & 1 deletion ...tiler-examples/src/test/java/com/onthegomap/planetiler/examples/BikeRouteOverlayTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down
Loading