From d317a5e32c8c7ad5188a02e5d22238f075d65d7a Mon Sep 17 00:00:00 2001 From: Brian Sperlongano Date: Sat, 29 Nov 2025 16:04:30 -0500 Subject: [PATCH 1/7] Upgrade tiny trunk segments for merging --- .../openmaptiles/layers/Transportation.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/main/java/org/openmaptiles/layers/Transportation.java b/src/main/java/org/openmaptiles/layers/Transportation.java index 299d3f7e..15475896 100644 --- a/src/main/java/org/openmaptiles/layers/Transportation.java +++ b/src/main/java/org/openmaptiles/layers/Transportation.java @@ -168,6 +168,8 @@ public class Transportation implements .put(6, 100) .put(5, 500) .put(4, 1_000); + private static final ZoomFunction.MeterToPixelThresholds TRUNK_UPGRADE_LENGTH = ZoomFunction.meterThresholds() + .put(5, 1_000); // ORDER BY network_type, network, LENGTH(ref), ref) private static final Comparator RELATION_ORDERING = Comparator .comparingInt( @@ -700,6 +702,30 @@ public List postProcess(int zoom, List i if (oneway instanceof Number n && ONEWAY_VALUES.contains(n.intValue())) { item.tags().put(LIMIT_MERGE_TAG, onewayId++); } + + // At zoom 5 only, upgrade tiny trunk sections to motorway for merging purposes. + if (zoom == 5) { + try { + Geometry geom = item.geometry().decode(); + if (geom instanceof LineString lineString) { + double pixelLength = lineString.getLength(); + + if (pixelLength < TRUNK_UPGRADE_LENGTH.apply(zoom).doubleValue()) { + Object classValue = item.tags().get(Fields.CLASS); + if (classValue instanceof String currentClass) { + if (FieldValues.CLASS_TRUNK.equals(currentClass)) { + item.tags().put(Fields.CLASS, FieldValues.CLASS_MOTORWAY); + } + if (FieldValues.CLASS_TRUNK_CONSTRUCTION.equals(currentClass)) { + item.tags().put(Fields.CLASS, FieldValues.CLASS_MOTORWAY_CONSTRUCTION); + } + } + } + } + } catch (GeometryException e) { + // Do nothing: skip problematic geometry for trunk upgrade + } + } } var merged = FeatureMerge.mergeLineStrings(items, minLength, tolerance, BUFFER_SIZE); From 423a2814498e1c3f2604a5b030f28752e2f07948 Mon Sep 17 00:00:00 2001 From: Brian Sperlongano Date: Sat, 29 Nov 2025 23:40:41 -0500 Subject: [PATCH 2/7] Revert --- .../openmaptiles/layers/Transportation.java | 26 +------------------ 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/src/main/java/org/openmaptiles/layers/Transportation.java b/src/main/java/org/openmaptiles/layers/Transportation.java index 15475896..7fa78603 100644 --- a/src/main/java/org/openmaptiles/layers/Transportation.java +++ b/src/main/java/org/openmaptiles/layers/Transportation.java @@ -168,7 +168,7 @@ public class Transportation implements .put(6, 100) .put(5, 500) .put(4, 1_000); - private static final ZoomFunction.MeterToPixelThresholds TRUNK_UPGRADE_LENGTH = ZoomFunction.meterThresholds() + private static final ZoomFunction.MeterToPixelThresholds TRUNK_UPGRADE_LENGTH = ZoomFunction.meterThresholds() .put(5, 1_000); // ORDER BY network_type, network, LENGTH(ref), ref) private static final Comparator RELATION_ORDERING = Comparator @@ -702,30 +702,6 @@ public List postProcess(int zoom, List i if (oneway instanceof Number n && ONEWAY_VALUES.contains(n.intValue())) { item.tags().put(LIMIT_MERGE_TAG, onewayId++); } - - // At zoom 5 only, upgrade tiny trunk sections to motorway for merging purposes. - if (zoom == 5) { - try { - Geometry geom = item.geometry().decode(); - if (geom instanceof LineString lineString) { - double pixelLength = lineString.getLength(); - - if (pixelLength < TRUNK_UPGRADE_LENGTH.apply(zoom).doubleValue()) { - Object classValue = item.tags().get(Fields.CLASS); - if (classValue instanceof String currentClass) { - if (FieldValues.CLASS_TRUNK.equals(currentClass)) { - item.tags().put(Fields.CLASS, FieldValues.CLASS_MOTORWAY); - } - if (FieldValues.CLASS_TRUNK_CONSTRUCTION.equals(currentClass)) { - item.tags().put(Fields.CLASS, FieldValues.CLASS_MOTORWAY_CONSTRUCTION); - } - } - } - } - } catch (GeometryException e) { - // Do nothing: skip problematic geometry for trunk upgrade - } - } } var merged = FeatureMerge.mergeLineStrings(items, minLength, tolerance, BUFFER_SIZE); From 6ab67975b23572f90ffc0f0a94becebc55e4e3f2 Mon Sep 17 00:00:00 2001 From: Brian Sperlongano Date: Sun, 30 Nov 2025 19:52:21 -0500 Subject: [PATCH 3/7] Implement trunk merging at z5 --- .../openmaptiles/layers/Transportation.java | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/main/java/org/openmaptiles/layers/Transportation.java b/src/main/java/org/openmaptiles/layers/Transportation.java index 7fa78603..aa78dc32 100644 --- a/src/main/java/org/openmaptiles/layers/Transportation.java +++ b/src/main/java/org/openmaptiles/layers/Transportation.java @@ -287,6 +287,21 @@ private static boolean isTrunkForZ5(String highway, List routeRel .anyMatch(Z5_TRUNK_BY_NETWORK::contains); } + /** + * Checks if a trunk segment is small enough to be processed at z5 so it can merge with surrounding motorways. + * @param element the highway linestring element to check + * @return true if the trunk segment length is less than the threshold, false otherwise + */ + private boolean isTrunkZ5MergeableLength(Tables.OsmHighwayLinestring element) { + try { + return element.source().length() < TRUNK_UPGRADE_LENGTH.apply(5).doubleValue(); + } catch (GeometryException e) { + e.log(stats, "omt_transportation_trunk_length", + "Unable to get feature length for trunk upgrade: " + element.source().id()); + return false; + } + } + private static boolean isMotorwayWithNetworkForZ4(List routeRelations) { // All roads in network included in osm_national_network except gb-trunk and us-highway return routeRelations.stream() @@ -572,6 +587,12 @@ int getMinzoom(Tables.OsmHighwayLinestring element, String highwayClass) { (z13Paths || !nullOrEmpty(element.name()) || routeRank <= 2 || !nullOrEmpty(element.sacScale())) ? 13 : 14; case FieldValues.CLASS_TRUNK -> { boolean z5trunk = isTrunkForZ5(highway, routeRelations); + + //Allow small trunk segments to be processed at z5 so they can merge with surrounding motorways + if (isTrunkZ5MergeableLength(element)) { + z5trunk = true; + } + // and if it is good for Z5, it may be good also for Z4 (see CLASS_MOTORWAY bellow): String clazz = FieldValues.CLASS_TRUNK; if (z5trunk && isMotorwayWithNetworkForZ4(routeRelations)) { @@ -704,6 +725,19 @@ public List postProcess(int zoom, List i } } + //Upgrade small trunk segments at z5 so they can merge with surrounding motorways + if(zoom == 5) { + for (var item : items) { + var highway = item.tags().get(Fields.CLASS); + if (highway instanceof String highwayStr && highwayStr.equals(FieldValues.CLASS_TRUNK)) { + item.tags().put(Fields.CLASS, FieldValues.CLASS_MOTORWAY); + } + if (highway instanceof String highwayStr && highwayStr.equals(FieldValues.CLASS_TRUNK_CONSTRUCTION)) { + item.tags().put(Fields.CLASS, FieldValues.CLASS_MOTORWAY_CONSTRUCTION); + } + } + } + var merged = FeatureMerge.mergeLineStrings(items, minLength, tolerance, BUFFER_SIZE); for (var item : merged) { From 0b3eb141775883f5392a3dc3ab660eb45a231fb6 Mon Sep 17 00:00:00 2001 From: Brian Sperlongano Date: Mon, 1 Dec 2025 08:01:17 -0500 Subject: [PATCH 4/7] Update test cases --- .../layers/AbstractLayerTest.java | 47 +++++++++++++- .../layers/TransportationTest.java | 64 +++++++++++++------ 2 files changed, 89 insertions(+), 22 deletions(-) diff --git a/src/test/java/org/openmaptiles/layers/AbstractLayerTest.java b/src/test/java/org/openmaptiles/layers/AbstractLayerTest.java index e3f649e3..da48ed38 100644 --- a/src/test/java/org/openmaptiles/layers/AbstractLayerTest.java +++ b/src/test/java/org/openmaptiles/layers/AbstractLayerTest.java @@ -26,6 +26,9 @@ import java.util.List; import java.util.Map; import java.util.stream.StreamSupport; +import org.locationtech.jts.geom.Coordinate; +import org.locationtech.jts.geom.GeometryFactory; +import org.locationtech.jts.geom.LineString; import org.openmaptiles.OpenMapTilesProfile; import org.openmaptiles.util.Utils; @@ -137,9 +140,28 @@ SourceFeature lineFeature(Map props) { ); } + /** + * Creates a {@link SourceFeature} representing a linestring of the specified real-world length in meters, + * using a vertical line starting at (0, 0) in lat/lon coordinates. + * + * @param length Length of the line in meters. + * @param props Properties to include in the feature. + * @return A {@link SourceFeature} of the requested length and properties. + */ SourceFeature lineFeatureWithLength(double length, Map props) { + + // Convert meters to degrees of latitude (at equator: 1 degree ≈ 111,320 meters) + // Create a line in lat/lon coordinates going north from (0, 0) + // Using JTS to create the geometry directly in lat/lon (longitude, latitude order) + double lengthInDegrees = length / 111320.0; + GeometryFactory geometryFactory = new GeometryFactory(); + Coordinate[] coordinates = new Coordinate[]{ + new Coordinate(0.0, 0.0), // Start at (lon=0, lat=0) + new Coordinate(0.0, lengthInDegrees) // End at (lon=0, lat=lengthInDegrees) + }; + LineString lineString = geometryFactory.createLineString(coordinates); return SimpleFeature.create( - GeoUtils.worldToLatLonCoords(newLineString(0, 0, 0, length)), + lineString, new HashMap<>(props), OpenMapTilesProfile.OSM_SOURCE, null, @@ -186,6 +208,29 @@ protected SimpleFeature lineFeatureWithRelation(List relationIn ); } + protected SimpleFeature lineFeatureWithRelationAndLength(List relationInfos, + Map map, double length) { + // Convert meters to degrees of latitude (at equator: 1 degree ≈ 111,320 meters) + // Create a line in lat/lon coordinates going north from (0, 0) + // Using JTS to create the geometry directly in lat/lon (longitude, latitude order) + double lengthInDegrees = length / 111320.0; + GeometryFactory geometryFactory = new GeometryFactory(); + Coordinate[] coordinates = new Coordinate[]{ + new Coordinate(0.0, 0.0), // Start at (lon=0, lat=0) + new Coordinate(0.0, lengthInDegrees) // End at (lon=0, lat=lengthInDegrees) + }; + LineString lineString = geometryFactory.createLineString(coordinates); + return SimpleFeature.createFakeOsmFeature( + lineString, + map, + OpenMapTilesProfile.OSM_SOURCE, + null, + 0, + (relationInfos == null ? List.of() : relationInfos).stream() + .map(r -> new OsmReader.RelationMember<>("", r)).toList() + ); + } + protected void testMergesLinestrings(Map attrs, String layer, double length, int zoom) throws GeometryException { var line1 = new VectorTile.Feature( diff --git a/src/test/java/org/openmaptiles/layers/TransportationTest.java b/src/test/java/org/openmaptiles/layers/TransportationTest.java index 3e0f6b4d..86721057 100644 --- a/src/test/java/org/openmaptiles/layers/TransportationTest.java +++ b/src/test/java/org/openmaptiles/layers/TransportationTest.java @@ -379,7 +379,7 @@ void testDuplicateRoute() { rel2.setTag("ref", "104"); rel2.setTag("direction", "south"); - FeatureCollector features = process(lineFeatureWithRelation( + FeatureCollector features = process(lineFeatureWithRelationAndLength( Stream.concat( profile.preprocessOsmRelation(rel2).stream(), profile.preprocessOsmRelation(rel1).stream() @@ -390,13 +390,14 @@ void testDuplicateRoute() { "lanes", 5, "maxspeed", "55 mph", "expressway", "no" - ))); + ), + 2000.0)); assertFeatures(13, List.of(mapOf( "_layer", "transportation", "class", "trunk", "network", "us-state", - "_minzoom", 6 + "_minzoom", 5 ), Map.of( "_layer", "transportation_name", "class", "trunk", @@ -1207,17 +1208,18 @@ void testTransCanadaProvincialCaOnPrimaryRefOther() { rel.setTag("network", "CA:ON:primary"); rel.setTag("ref", "85"); - FeatureCollector features = process(lineFeatureWithRelation( + FeatureCollector features = process(lineFeatureWithRelationAndLength( profile.preprocessOsmRelation(rel), Map.of( "highway", "trunk" - ))); + ), + 2000.0)); assertFeatures(13, List.of(Map.of( "_layer", "transportation", "class", "trunk", "network", "ca-provincial", - "_minzoom", 6 + "_minzoom", 5 ), Map.of( "_layer", "transportation_name", "class", "trunk", @@ -1261,17 +1263,18 @@ void testTransCanadaProvincialCaMbPthRefOther() { rel.setTag("network", "CA:MB:PTH"); rel.setTag("ref", "77"); - FeatureCollector features = process(lineFeatureWithRelation( + FeatureCollector features = process(lineFeatureWithRelationAndLength( profile.preprocessOsmRelation(rel), Map.of( "highway", "trunk" - ))); + ), + 2000.0)); assertFeatures(13, List.of(Map.of( "_layer", "transportation", "class", "trunk", "network", "ca-provincial", - "_minzoom", 6 + "_minzoom", 5 ), Map.of( "_layer", "transportation_name", "class", "trunk", @@ -1315,17 +1318,18 @@ void testTransCanadaProvincialCaAbPrimaryRefOther() { rel.setTag("network", "CA:AB:primary"); rel.setTag("ref", "10"); - FeatureCollector features = process(lineFeatureWithRelation( + FeatureCollector features = process(lineFeatureWithRelationAndLength( profile.preprocessOsmRelation(rel), Map.of( "highway", "trunk" - ))); + ), + 2000.0)); assertFeatures(13, List.of(Map.of( "_layer", "transportation", "class", "trunk", "network", "ca-provincial", - "_minzoom", 6 + "_minzoom", 5 ), Map.of( "_layer", "transportation_name", "class", "trunk", @@ -1369,17 +1373,18 @@ void testTransCanadaProvincialCaBcRefOther() { rel.setTag("network", "CA:BC"); rel.setTag("ref", "10"); - FeatureCollector features = process(lineFeatureWithRelation( + FeatureCollector features = process(lineFeatureWithRelationAndLength( profile.preprocessOsmRelation(rel), Map.of( "highway", "trunk" - ))); + ), + 2000.0)); assertFeatures(13, List.of(Map.of( "_layer", "transportation", "class", "trunk", "network", "ca-provincial", - "_minzoom", 6 + "_minzoom", 5 ), Map.of( "_layer", "transportation_name", "class", "trunk", @@ -1395,16 +1400,17 @@ void testTransCanadaProvincialCaOther() { rel.setTag("route", "road"); rel.setTag("network", "CA:yellowhead"); - FeatureCollector features = process(lineFeatureWithRelation( + FeatureCollector features = process(lineFeatureWithRelationAndLength( profile.preprocessOsmRelation(rel), Map.of( "highway", "trunk" - ))); + ), + 2000.0)); assertFeatures(13, List.of(Map.of( "_layer", "transportation", "class", "trunk", - "_minzoom", 6 + "_minzoom", 5 )), features); boolean caProvPresent = StreamSupport.stream(features.spliterator(), false) .flatMap(f -> f.getAttrsAtZoom(13).entrySet().stream()) @@ -2181,18 +2187,19 @@ void testARoad() { rel.setTag("network", "AsianHighway"); rel.setTag("ref", "AH11"); - FeatureCollector features = process(lineFeatureWithRelation( + FeatureCollector features = process(lineFeatureWithRelationAndLength( profile.preprocessOsmRelation(rel), Map.of( "highway", "trunk", "name", "National Highway 7", "ref", "7" - ))); + ), + 2000.0)); assertFeatures(13, List.of(Map.of( "_layer", "transportation", "class", "trunk", - "_minzoom", 6 + "_minzoom", 5 ), Map.of( "_layer", "transportation_name", "class", "trunk", @@ -2227,4 +2234,19 @@ void testERoad() { "route_1_ref", "E 77" )), features); } + + @Test + void testShortTrunkSegmentWithoutNetwork() { + // Short trunk segment (< 1000m) should appear at z5 to allow merging with surrounding motorways + FeatureCollector features = process(lineFeatureWithLength(500.0, Map.of( + "highway", "trunk" + ))); + + assertFeatures(13, List.of(Map.of( + "_layer", "transportation", + "class", "trunk", + "_minzoom", 5 + )), features); + } + } From 1f0230b9ced9c99f691853cc33b2b112c7692a37 Mon Sep 17 00:00:00 2001 From: Brian Sperlongano Date: Mon, 1 Dec 2025 21:32:03 -0500 Subject: [PATCH 5/7] Formatting --- .../org/openmaptiles/layers/Transportation.java | 5 +++-- .../org/openmaptiles/layers/AbstractLayerTest.java | 14 +++++++------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/openmaptiles/layers/Transportation.java b/src/main/java/org/openmaptiles/layers/Transportation.java index aa78dc32..c051141c 100644 --- a/src/main/java/org/openmaptiles/layers/Transportation.java +++ b/src/main/java/org/openmaptiles/layers/Transportation.java @@ -168,7 +168,7 @@ public class Transportation implements .put(6, 100) .put(5, 500) .put(4, 1_000); - private static final ZoomFunction.MeterToPixelThresholds TRUNK_UPGRADE_LENGTH = ZoomFunction.meterThresholds() + private static final ZoomFunction.MeterToPixelThresholds TRUNK_UPGRADE_LENGTH = ZoomFunction.meterThresholds() .put(5, 1_000); // ORDER BY network_type, network, LENGTH(ref), ref) private static final Comparator RELATION_ORDERING = Comparator @@ -289,6 +289,7 @@ private static boolean isTrunkForZ5(String highway, List routeRel /** * Checks if a trunk segment is small enough to be processed at z5 so it can merge with surrounding motorways. + * * @param element the highway linestring element to check * @return true if the trunk segment length is less than the threshold, false otherwise */ @@ -726,7 +727,7 @@ public List postProcess(int zoom, List i } //Upgrade small trunk segments at z5 so they can merge with surrounding motorways - if(zoom == 5) { + if (zoom == 5) { for (var item : items) { var highway = item.tags().get(Fields.CLASS); if (highway instanceof String highwayStr && highwayStr.equals(FieldValues.CLASS_TRUNK)) { diff --git a/src/test/java/org/openmaptiles/layers/AbstractLayerTest.java b/src/test/java/org/openmaptiles/layers/AbstractLayerTest.java index da48ed38..a5bcb9b1 100644 --- a/src/test/java/org/openmaptiles/layers/AbstractLayerTest.java +++ b/src/test/java/org/openmaptiles/layers/AbstractLayerTest.java @@ -141,12 +141,12 @@ SourceFeature lineFeature(Map props) { } /** - * Creates a {@link SourceFeature} representing a linestring of the specified real-world length in meters, - * using a vertical line starting at (0, 0) in lat/lon coordinates. + * Creates a {@link SourceFeature} representing a linestring of the specified real-world length in meters, using a + * vertical line starting at (0, 0) in lat/lon coordinates. * * @param length Length of the line in meters. * @param props Properties to include in the feature. - * @return A {@link SourceFeature} of the requested length and properties. + * @return A {@link SourceFeature} of the requested length and properties. */ SourceFeature lineFeatureWithLength(double length, Map props) { @@ -156,8 +156,8 @@ SourceFeature lineFeatureWithLength(double length, Map props) { double lengthInDegrees = length / 111320.0; GeometryFactory geometryFactory = new GeometryFactory(); Coordinate[] coordinates = new Coordinate[]{ - new Coordinate(0.0, 0.0), // Start at (lon=0, lat=0) - new Coordinate(0.0, lengthInDegrees) // End at (lon=0, lat=lengthInDegrees) + new Coordinate(0.0, 0.0), // Start at (lon=0, lat=0) + new Coordinate(0.0, lengthInDegrees) // End at (lon=0, lat=lengthInDegrees) }; LineString lineString = geometryFactory.createLineString(coordinates); return SimpleFeature.create( @@ -216,8 +216,8 @@ protected SimpleFeature lineFeatureWithRelationAndLength(List r double lengthInDegrees = length / 111320.0; GeometryFactory geometryFactory = new GeometryFactory(); Coordinate[] coordinates = new Coordinate[]{ - new Coordinate(0.0, 0.0), // Start at (lon=0, lat=0) - new Coordinate(0.0, lengthInDegrees) // End at (lon=0, lat=lengthInDegrees) + new Coordinate(0.0, 0.0), // Start at (lon=0, lat=0) + new Coordinate(0.0, lengthInDegrees) // End at (lon=0, lat=lengthInDegrees) }; LineString lineString = geometryFactory.createLineString(coordinates); return SimpleFeature.createFakeOsmFeature( From f18ea8db75a6622de985855ae5d592eb460812f3 Mon Sep 17 00:00:00 2001 From: Brian Sperlongano Date: Mon, 1 Dec 2025 22:14:07 -0500 Subject: [PATCH 6/7] Update tests to be specific to this PR case --- .../layers/AbstractLayerTest.java | 47 +------- .../layers/TransportationTest.java | 104 ++++++++++++------ 2 files changed, 73 insertions(+), 78 deletions(-) diff --git a/src/test/java/org/openmaptiles/layers/AbstractLayerTest.java b/src/test/java/org/openmaptiles/layers/AbstractLayerTest.java index a5bcb9b1..e3f649e3 100644 --- a/src/test/java/org/openmaptiles/layers/AbstractLayerTest.java +++ b/src/test/java/org/openmaptiles/layers/AbstractLayerTest.java @@ -26,9 +26,6 @@ import java.util.List; import java.util.Map; import java.util.stream.StreamSupport; -import org.locationtech.jts.geom.Coordinate; -import org.locationtech.jts.geom.GeometryFactory; -import org.locationtech.jts.geom.LineString; import org.openmaptiles.OpenMapTilesProfile; import org.openmaptiles.util.Utils; @@ -140,28 +137,9 @@ SourceFeature lineFeature(Map props) { ); } - /** - * Creates a {@link SourceFeature} representing a linestring of the specified real-world length in meters, using a - * vertical line starting at (0, 0) in lat/lon coordinates. - * - * @param length Length of the line in meters. - * @param props Properties to include in the feature. - * @return A {@link SourceFeature} of the requested length and properties. - */ SourceFeature lineFeatureWithLength(double length, Map props) { - - // Convert meters to degrees of latitude (at equator: 1 degree ≈ 111,320 meters) - // Create a line in lat/lon coordinates going north from (0, 0) - // Using JTS to create the geometry directly in lat/lon (longitude, latitude order) - double lengthInDegrees = length / 111320.0; - GeometryFactory geometryFactory = new GeometryFactory(); - Coordinate[] coordinates = new Coordinate[]{ - new Coordinate(0.0, 0.0), // Start at (lon=0, lat=0) - new Coordinate(0.0, lengthInDegrees) // End at (lon=0, lat=lengthInDegrees) - }; - LineString lineString = geometryFactory.createLineString(coordinates); return SimpleFeature.create( - lineString, + GeoUtils.worldToLatLonCoords(newLineString(0, 0, 0, length)), new HashMap<>(props), OpenMapTilesProfile.OSM_SOURCE, null, @@ -208,29 +186,6 @@ protected SimpleFeature lineFeatureWithRelation(List relationIn ); } - protected SimpleFeature lineFeatureWithRelationAndLength(List relationInfos, - Map map, double length) { - // Convert meters to degrees of latitude (at equator: 1 degree ≈ 111,320 meters) - // Create a line in lat/lon coordinates going north from (0, 0) - // Using JTS to create the geometry directly in lat/lon (longitude, latitude order) - double lengthInDegrees = length / 111320.0; - GeometryFactory geometryFactory = new GeometryFactory(); - Coordinate[] coordinates = new Coordinate[]{ - new Coordinate(0.0, 0.0), // Start at (lon=0, lat=0) - new Coordinate(0.0, lengthInDegrees) // End at (lon=0, lat=lengthInDegrees) - }; - LineString lineString = geometryFactory.createLineString(coordinates); - return SimpleFeature.createFakeOsmFeature( - lineString, - map, - OpenMapTilesProfile.OSM_SOURCE, - null, - 0, - (relationInfos == null ? List.of() : relationInfos).stream() - .map(r -> new OsmReader.RelationMember<>("", r)).toList() - ); - } - protected void testMergesLinestrings(Map attrs, String layer, double length, int zoom) throws GeometryException { var line1 = new VectorTile.Feature( diff --git a/src/test/java/org/openmaptiles/layers/TransportationTest.java b/src/test/java/org/openmaptiles/layers/TransportationTest.java index 86721057..c0e8b2f4 100644 --- a/src/test/java/org/openmaptiles/layers/TransportationTest.java +++ b/src/test/java/org/openmaptiles/layers/TransportationTest.java @@ -4,9 +4,12 @@ import static com.onthegomap.planetiler.TestUtils.newPoint; import static com.onthegomap.planetiler.TestUtils.rectangle; import static java.util.Map.entry; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import com.onthegomap.planetiler.FeatureCollector; +import com.onthegomap.planetiler.VectorTile; import com.onthegomap.planetiler.config.Arguments; import com.onthegomap.planetiler.config.PlanetilerConfig; import com.onthegomap.planetiler.geo.GeometryException; @@ -16,6 +19,7 @@ import com.onthegomap.planetiler.reader.osm.OsmRelationInfo; import com.onthegomap.planetiler.stats.Stats; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.stream.Stream; @@ -379,7 +383,7 @@ void testDuplicateRoute() { rel2.setTag("ref", "104"); rel2.setTag("direction", "south"); - FeatureCollector features = process(lineFeatureWithRelationAndLength( + FeatureCollector features = process(lineFeatureWithRelation( Stream.concat( profile.preprocessOsmRelation(rel2).stream(), profile.preprocessOsmRelation(rel1).stream() @@ -390,8 +394,7 @@ void testDuplicateRoute() { "lanes", 5, "maxspeed", "55 mph", "expressway", "no" - ), - 2000.0)); + ))); assertFeatures(13, List.of(mapOf( "_layer", "transportation", @@ -1208,12 +1211,11 @@ void testTransCanadaProvincialCaOnPrimaryRefOther() { rel.setTag("network", "CA:ON:primary"); rel.setTag("ref", "85"); - FeatureCollector features = process(lineFeatureWithRelationAndLength( + FeatureCollector features = process(lineFeatureWithRelation( profile.preprocessOsmRelation(rel), Map.of( "highway", "trunk" - ), - 2000.0)); + ))); assertFeatures(13, List.of(Map.of( "_layer", "transportation", @@ -1263,12 +1265,11 @@ void testTransCanadaProvincialCaMbPthRefOther() { rel.setTag("network", "CA:MB:PTH"); rel.setTag("ref", "77"); - FeatureCollector features = process(lineFeatureWithRelationAndLength( + FeatureCollector features = process(lineFeatureWithRelation( profile.preprocessOsmRelation(rel), Map.of( "highway", "trunk" - ), - 2000.0)); + ))); assertFeatures(13, List.of(Map.of( "_layer", "transportation", @@ -1318,12 +1319,11 @@ void testTransCanadaProvincialCaAbPrimaryRefOther() { rel.setTag("network", "CA:AB:primary"); rel.setTag("ref", "10"); - FeatureCollector features = process(lineFeatureWithRelationAndLength( + FeatureCollector features = process(lineFeatureWithRelation( profile.preprocessOsmRelation(rel), Map.of( "highway", "trunk" - ), - 2000.0)); + ))); assertFeatures(13, List.of(Map.of( "_layer", "transportation", @@ -1373,12 +1373,11 @@ void testTransCanadaProvincialCaBcRefOther() { rel.setTag("network", "CA:BC"); rel.setTag("ref", "10"); - FeatureCollector features = process(lineFeatureWithRelationAndLength( + FeatureCollector features = process(lineFeatureWithRelation( profile.preprocessOsmRelation(rel), Map.of( "highway", "trunk" - ), - 2000.0)); + ))); assertFeatures(13, List.of(Map.of( "_layer", "transportation", @@ -1400,12 +1399,11 @@ void testTransCanadaProvincialCaOther() { rel.setTag("route", "road"); rel.setTag("network", "CA:yellowhead"); - FeatureCollector features = process(lineFeatureWithRelationAndLength( + FeatureCollector features = process(lineFeatureWithRelation( profile.preprocessOsmRelation(rel), Map.of( "highway", "trunk" - ), - 2000.0)); + ))); assertFeatures(13, List.of(Map.of( "_layer", "transportation", @@ -2187,14 +2185,13 @@ void testARoad() { rel.setTag("network", "AsianHighway"); rel.setTag("ref", "AH11"); - FeatureCollector features = process(lineFeatureWithRelationAndLength( + FeatureCollector features = process(lineFeatureWithRelation( profile.preprocessOsmRelation(rel), Map.of( "highway", "trunk", "name", "National Highway 7", "ref", "7" - ), - 2000.0)); + ))); assertFeatures(13, List.of(Map.of( "_layer", "transportation", @@ -2235,18 +2232,61 @@ void testERoad() { )), features); } - @Test - void testShortTrunkSegmentWithoutNetwork() { - // Short trunk segment (< 1000m) should appear at z5 to allow merging with surrounding motorways - FeatureCollector features = process(lineFeatureWithLength(500.0, Map.of( - "highway", "trunk" - ))); + private VectorTile.Feature createLineFeature(String layer, double x1, double y1, double x2, double y2, + String roadClass) { + return new VectorTile.Feature( + layer, + 1, + VectorTile.encodeGeometry(newLineString(x1, y1, x2, y2)), + new HashMap<>(Map.of("class", roadClass)), + 0 + ); + } - assertFeatures(13, List.of(Map.of( - "_layer", "transportation", - "class", "trunk", - "_minzoom", 5 - )), features); + @Test + void testTrunkMotorwayAlternatingMergesToMotorwayAtZoom5() throws GeometryException { + // Test that alternating trunk and motorway segments merge into a single motorway at zoom 5 + String layer = Transportation.LAYER_NAME; + double segmentLength = 1000.0; // Each segment is 1000m + + // Create 4 segments: motorway, trunk, motorway, trunk + var motorway1 = createLineFeature(layer, 0, 0, segmentLength, 0, "motorway"); + var trunk1 = createLineFeature(layer, segmentLength, 0, 2 * segmentLength, 0, "trunk"); + var motorway2 = createLineFeature(layer, 2 * segmentLength, 0, 3 * segmentLength, 0, "motorway"); + var trunk2 = createLineFeature(layer, 3 * segmentLength, 0, 4 * segmentLength, 0, "trunk"); + + // At zoom 5, trunk segments should be upgraded to motorway and all segments should merge + List resultZ5 = + profile.postProcessLayerFeatures(layer, 5, List.of(motorway1, trunk1, motorway2, trunk2)); + + // Should result in a single merged motorway feature + assertEquals(1, resultZ5.size(), "Should merge into a single feature at zoom 5"); + VectorTile.Feature mergedFeatureZ5 = resultZ5.get(0); + assertEquals("motorway", mergedFeatureZ5.tags().get("class"), "Merged feature should be motorway class at zoom 5"); + assertEquals(layer, mergedFeatureZ5.layer(), "Layer should be transportation"); + + // At zoom 6, trunk segments should NOT be upgraded to motorway + // FeatureMerge.mergeLineStrings merges based on geometry connectivity, not class + // So all connected segments will merge into one feature regardless of class + List resultZ6 = + profile.postProcessLayerFeatures(layer, 6, List.of(motorway1, trunk1, motorway2, trunk2)); + + // At zoom 6, all segments merge into one because they're geometrically connected + // The class will be one of the classes (likely the first one processed) + assertEquals(1, resultZ6.size(), + "At zoom 6, all connected segments merge into one feature (FeatureMerge merges by geometry, not class)"); + + // Verify that trunk segments were NOT upgraded to motorway at zoom 6 + // The merged feature should have either "motorway" or "trunk" class, but not both + VectorTile.Feature mergedFeatureZ6 = resultZ6.get(0); + String mergedClassZ6 = (String) mergedFeatureZ6.tags().get("class"); + assertTrue("motorway".equals(mergedClassZ6) || "trunk".equals(mergedClassZ6), + "Merged feature should have either motorway or trunk class at zoom 6, but not a mix"); + + // The key difference from z5: at z6, trunk segments are NOT upgraded before merging + // So the merged feature might have "trunk" class if trunk segments are processed first, + // or "motorway" if motorway segments are processed first + // The important thing is that trunk segments keep their "trunk" class at z6 } } From daef06526bb9865b6e1ff020b758eb0aeb52e3de Mon Sep 17 00:00:00 2001 From: Brian Sperlongano Date: Wed, 3 Dec 2025 20:15:14 -0500 Subject: [PATCH 7/7] Fix z5/z6 test cases --- .../layers/TransportationTest.java | 84 +++++++++---------- 1 file changed, 38 insertions(+), 46 deletions(-) diff --git a/src/test/java/org/openmaptiles/layers/TransportationTest.java b/src/test/java/org/openmaptiles/layers/TransportationTest.java index c0e8b2f4..d648ab02 100644 --- a/src/test/java/org/openmaptiles/layers/TransportationTest.java +++ b/src/test/java/org/openmaptiles/layers/TransportationTest.java @@ -6,7 +6,6 @@ import static java.util.Map.entry; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; import com.onthegomap.planetiler.FeatureCollector; import com.onthegomap.planetiler.VectorTile; @@ -2232,61 +2231,54 @@ void testERoad() { )), features); } - private VectorTile.Feature createLineFeature(String layer, double x1, double y1, double x2, double y2, - String roadClass) { - return new VectorTile.Feature( + @Test + void testShortTrunkMerge() throws GeometryException { + var layer = Transportation.LAYER_NAME; + + var motorwayZ6 = new VectorTile.Feature( layer, 1, - VectorTile.encodeGeometry(newLineString(x1, y1, x2, y2)), - new HashMap<>(Map.of("class", roadClass)), + VectorTile.encodeGeometry(newLineString(0, 0, 10, 10)), + new HashMap<>(Map.of("class", "motorway")), + 0 + ); + var trunkZ6 = new VectorTile.Feature( + layer, + 2, + VectorTile.encodeGeometry(newLineString(10, 10, 10, 11)), + new HashMap<>(Map.of("class", "trunk")), + 0 + ); + var motorwayZ5 = new VectorTile.Feature( + layer, + 1, + VectorTile.encodeGeometry(newLineString(0, 0, 10, 10)), + new HashMap<>(Map.of("class", "motorway")), + 0 + ); + var trunkZ5 = new VectorTile.Feature( + layer, + 2, + VectorTile.encodeGeometry(newLineString(10, 10, 10, 11)), + new HashMap<>(Map.of("class", "trunk")), 0 ); - } - - @Test - void testTrunkMotorwayAlternatingMergesToMotorwayAtZoom5() throws GeometryException { - // Test that alternating trunk and motorway segments merge into a single motorway at zoom 5 - String layer = Transportation.LAYER_NAME; - double segmentLength = 1000.0; // Each segment is 1000m - // Create 4 segments: motorway, trunk, motorway, trunk - var motorway1 = createLineFeature(layer, 0, 0, segmentLength, 0, "motorway"); - var trunk1 = createLineFeature(layer, segmentLength, 0, 2 * segmentLength, 0, "trunk"); - var motorway2 = createLineFeature(layer, 2 * segmentLength, 0, 3 * segmentLength, 0, "motorway"); - var trunk2 = createLineFeature(layer, 3 * segmentLength, 0, 4 * segmentLength, 0, "trunk"); + List inputZ6 = List.of(motorwayZ6, trunkZ6); + List inputZ5 = List.of(motorwayZ5, trunkZ5); - // At zoom 5, trunk segments should be upgraded to motorway and all segments should merge - List resultZ5 = - profile.postProcessLayerFeatures(layer, 5, List.of(motorway1, trunk1, motorway2, trunk2)); + List resultZ6 = profile.postProcessLayerFeatures(layer, 6, inputZ6); + List resultZ5 = profile.postProcessLayerFeatures(layer, 5, inputZ5); - // Should result in a single merged motorway feature + assertEquals(2, resultZ6.size(), "Should be separate features at zoom 6"); assertEquals(1, resultZ5.size(), "Should merge into a single feature at zoom 5"); + VectorTile.Feature mergedFeatureZ5 = resultZ5.get(0); assertEquals("motorway", mergedFeatureZ5.tags().get("class"), "Merged feature should be motorway class at zoom 5"); - assertEquals(layer, mergedFeatureZ5.layer(), "Layer should be transportation"); - - // At zoom 6, trunk segments should NOT be upgraded to motorway - // FeatureMerge.mergeLineStrings merges based on geometry connectivity, not class - // So all connected segments will merge into one feature regardless of class - List resultZ6 = - profile.postProcessLayerFeatures(layer, 6, List.of(motorway1, trunk1, motorway2, trunk2)); - - // At zoom 6, all segments merge into one because they're geometrically connected - // The class will be one of the classes (likely the first one processed) - assertEquals(1, resultZ6.size(), - "At zoom 6, all connected segments merge into one feature (FeatureMerge merges by geometry, not class)"); - // Verify that trunk segments were NOT upgraded to motorway at zoom 6 - // The merged feature should have either "motorway" or "trunk" class, but not both - VectorTile.Feature mergedFeatureZ6 = resultZ6.get(0); - String mergedClassZ6 = (String) mergedFeatureZ6.tags().get("class"); - assertTrue("motorway".equals(mergedClassZ6) || "trunk".equals(mergedClassZ6), - "Merged feature should have either motorway or trunk class at zoom 6, but not a mix"); - - // The key difference from z5: at z6, trunk segments are NOT upgraded before merging - // So the merged feature might have "trunk" class if trunk segments are processed first, - // or "motorway" if motorway segments are processed first - // The important thing is that trunk segments keep their "trunk" class at z6 + List classesZ6 = resultZ6.stream() + .map(f -> (String) f.tags().get("class")) + .toList(); + assertEquals(List.of("motorway", "trunk"), classesZ6, "At zoom 6, should have motorway and trunk classes"); } - }