From 5c5704eb7c45eaabec2880349e6d67dc7e4d4721 Mon Sep 17 00:00:00 2001 From: Michael Barry Date: Wed, 5 Oct 2022 06:18:28 -0400 Subject: [PATCH] Don't merge transportation segments with oneway tag (#40) --- .../openmaptiles/layers/Transportation.java | 83 +++++++++++-------- .../layers/AbstractLayerTest.java | 16 ++-- .../layers/TransportationTest.java | 12 ++- 3 files changed, 66 insertions(+), 45 deletions(-) diff --git a/src/main/java/org/openmaptiles/layers/Transportation.java b/src/main/java/org/openmaptiles/layers/Transportation.java index 10b0913..dd5e161 100644 --- a/src/main/java/org/openmaptiles/layers/Transportation.java +++ b/src/main/java/org/openmaptiles/layers/Transportation.java @@ -149,12 +149,14 @@ public class Transportation implements .thenComparing(routeRelation -> coalesce(routeRelation.network(), "")) .thenComparingInt(r -> r.ref().length()) .thenComparing(RouteRelation::ref); + private static final Set ONEWAY_VALUES = Set.of(-1, 1); + private static final String LIMIT_MERGE_TAG = "__limit_merge"; private final AtomicBoolean loggedNoGb = new AtomicBoolean(false); private final boolean z13Paths; - private PreparedGeometry greatBritain = null; private final Map MINZOOMS; private final Stats stats; private final PlanetilerConfig config; + private PreparedGeometry greatBritain = null; public Transportation(Translations translations, PlanetilerConfig config, Stats stats) { this.config = config; @@ -179,23 +181,6 @@ public class Transportation implements ); } - @Override - public void processNaturalEarth(String table, SourceFeature feature, - FeatureCollector features) { - if ("ne_10m_admin_0_countries".equals(table) && feature.hasTag("iso_a2", "GB")) { - // multiple threads call this method concurrently, GB polygon *should* only be found - // once, but just to be safe synchronize updates to that field - synchronized (this) { - try { - Geometry boundary = feature.polygon().buffer(GeoUtils.metersToPixelAtEquator(0, 10_000) / 256d); - greatBritain = PreparedGeometryFactory.prepare(boundary); - } catch (GeometryException e) { - LOGGER.error("Failed to get Great Britain Polygon: " + e); - } - } - } - } - /** Returns a value for {@code surface} tag constrained to a small set of known values from raw OSM data. */ private static String surface(String value) { return value == null ? null : SURFACE_PAVED_VALUES.contains(value) ? FieldValues.SURFACE_PAVED : @@ -250,19 +235,20 @@ public class Transportation implements return "bridge".equals(manMade) || "pier".equals(manMade); } - enum RouteNetwork { - - US_INTERSTATE("us-interstate"), - US_HIGHWAY("us-highway"), - US_STATE("us-state"), - CA_TRANSCANADA("ca-transcanada"), - GB_MOTORWAY("gb-motorway"), - GB_TRUNK("gb-trunk"); - - final String name; - - RouteNetwork(String name) { - this.name = name; + @Override + public void processNaturalEarth(String table, SourceFeature feature, + FeatureCollector features) { + if ("ne_10m_admin_0_countries".equals(table) && feature.hasTag("iso_a2", "GB")) { + // multiple threads call this method concurrently, GB polygon *should* only be found + // once, but just to be safe synchronize updates to that field + synchronized (this) { + try { + Geometry boundary = feature.polygon().buffer(GeoUtils.metersToPixelAtEquator(0, 10_000) / 256d); + greatBritain = PreparedGeometryFactory.prepare(boundary); + } catch (GeometryException e) { + LOGGER.error("Failed to get Great Britain Polygon: " + e); + } + } } } @@ -534,8 +520,39 @@ public class Transportation implements public List postProcess(int zoom, List items) { double tolerance = config.tolerance(zoom); double minLength = coalesce(MIN_LENGTH.apply(zoom), 0).doubleValue(); - // TODO preserve direction for one-way? - return FeatureMerge.mergeLineStrings(items, minLength, tolerance, BUFFER_SIZE); + + // don't merge road segments with oneway tag + // TODO merge preserving oneway instead ignoring + int onewayId = 1; + for (var item : items) { + var oneway = item.attrs().get(Fields.ONEWAY); + if (oneway instanceof Integer i && ONEWAY_VALUES.contains(i)) { + item.attrs().put(LIMIT_MERGE_TAG, onewayId++); + } + } + + var merged = FeatureMerge.mergeLineStrings(items, minLength, tolerance, BUFFER_SIZE); + + for (var item : merged) { + item.attrs().remove(LIMIT_MERGE_TAG); + } + return merged; + } + + enum RouteNetwork { + + US_INTERSTATE("us-interstate"), + US_HIGHWAY("us-highway"), + US_STATE("us-state"), + CA_TRANSCANADA("ca-transcanada"), + GB_MOTORWAY("gb-motorway"), + GB_TRUNK("gb-trunk"); + + final String name; + + RouteNetwork(String name) { + this.name = name; + } } /** Information extracted from route relations to use when processing ways in that relation. */ diff --git a/src/test/java/org/openmaptiles/layers/AbstractLayerTest.java b/src/test/java/org/openmaptiles/layers/AbstractLayerTest.java index 1f688cb..68db9b1 100644 --- a/src/test/java/org/openmaptiles/layers/AbstractLayerTest.java +++ b/src/test/java/org/openmaptiles/layers/AbstractLayerTest.java @@ -176,20 +176,20 @@ public abstract class AbstractLayerTest { ); } - protected void testMergesLinestrings(Map attrs, String layer, - double length, int zoom) throws GeometryException { + protected void testMergesLinestrings(Map attrs, String layer, double length, int zoom) + throws GeometryException { var line1 = new VectorTile.Feature( layer, 1, VectorTile.encodeGeometry(newLineString(0, 0, length / 2, 0)), - attrs, + new HashMap<>(attrs), 0 ); var line2 = new VectorTile.Feature( layer, 1, VectorTile.encodeGeometry(newLineString(length / 2, 0, length, 0)), - attrs, + new HashMap<>(attrs), 0 ); var connected = new VectorTile.Feature( @@ -206,20 +206,20 @@ public abstract class AbstractLayerTest { ); } - protected void testDoesNotMergeLinestrings(Map attrs, String layer, - double length, int zoom) throws GeometryException { + protected void testDoesNotMergeLinestrings(Map attrs, String layer, double length, int zoom) + throws GeometryException { var line1 = new VectorTile.Feature( layer, 1, VectorTile.encodeGeometry(newLineString(0, 0, length / 2, 0)), - attrs, + new HashMap<>(attrs), 0 ); var line2 = new VectorTile.Feature( layer, 1, VectorTile.encodeGeometry(newLineString(length / 2, 0, length, 0)), - attrs, + new HashMap<>(attrs), 0 ); diff --git a/src/test/java/org/openmaptiles/layers/TransportationTest.java b/src/test/java/org/openmaptiles/layers/TransportationTest.java index 3c6ff27..31cafa5 100644 --- a/src/test/java/org/openmaptiles/layers/TransportationTest.java +++ b/src/test/java/org/openmaptiles/layers/TransportationTest.java @@ -950,13 +950,17 @@ class TransportationTest extends AbstractLayerTest { } @Test - void testMergesDisconnectedRoadFeatures() throws GeometryException { - testMergesLinestrings(Map.of("class", "motorway"), Transportation.LAYER_NAME, 10, 14); + void testMergesDisconnectedRoadNameFeatures() throws GeometryException { + testMergesLinestrings(Map.of("class", "motorway"), TransportationName.LAYER_NAME, 10, 14); } @Test - void testMergesDisconnectedRoadNameFeatures() throws GeometryException { - testMergesLinestrings(Map.of("class", "motorway"), TransportationName.LAYER_NAME, 10, 14); + void testMergesDisconnectedRoadFeaturesUnlessOneway() throws GeometryException { + String layer = Transportation.LAYER_NAME; + testMergesLinestrings(Map.of("class", "motorway", "oneway", 0), layer, 10, 14); + testMergesLinestrings(Map.of("class", "motorway"), layer, 10, 14); + testDoesNotMergeLinestrings(Map.of("class", "motorway", "oneway", 1), layer, 10, 14); + testDoesNotMergeLinestrings(Map.of("class", "motorway", "oneway", -1), layer, 10, 14); } @Test