Don't merge transportation segments with oneway tag (#40)

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<Integer> 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<String, Integer> 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 {
+  @Override
-
+  public void processNaturalEarth(String table, SourceFeature feature,
-    US_INTERSTATE("us-interstate"),
+    FeatureCollector features) {
-    US_HIGHWAY("us-highway"),
+    if ("ne_10m_admin_0_countries".equals(table) && feature.hasTag("iso_a2", "GB")) {
-    US_STATE("us-state"),
+      // multiple threads call this method concurrently, GB polygon *should* only be found
-    CA_TRANSCANADA("ca-transcanada"),
+      // once, but just to be safe synchronize updates to that field
-    GB_MOTORWAY("gb-motorway"),
+      synchronized (this) {
-    GB_TRUNK("gb-trunk");
+        try {
-
+          Geometry boundary = feature.polygon().buffer(GeoUtils.metersToPixelAtEquator(0, 10_000) / 256d);
-    final String name;
+          greatBritain = PreparedGeometryFactory.prepare(boundary);
-
+        } catch (GeometryException e) {
-    RouteNetwork(String name) {
+          LOGGER.error("Failed to get Great Britain Polygon: " + e);
-      this.name = name;
+        }
+      }
     }
   }
 
@@ -534,8 +520,39 @@ public class Transportation implements
   public List<VectorTile.Feature> postProcess(int zoom, List<VectorTile.Feature> 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. */

src/test/java/org/openmaptiles/layers/AbstractLayerTest.java

@@ -176,20 +176,20 @@ public abstract class AbstractLayerTest {
     );
   }
 
-  protected void testMergesLinestrings(Map<String, Object> attrs, String layer,
+  protected void testMergesLinestrings(Map<String, Object> attrs, String layer, double length, int zoom)
-    double length, int zoom) throws GeometryException {
+    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<String, Object> attrs, String layer,
+  protected void testDoesNotMergeLinestrings(Map<String, Object> attrs, String layer, double length, int zoom)
-    double length, int zoom) throws GeometryException {
+    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
     );
 

src/test/java/org/openmaptiles/layers/TransportationTest.java

@@ -950,13 +950,17 @@ class TransportationTest extends AbstractLayerTest {
   }
 
   @Test
-  void testMergesDisconnectedRoadFeatures() throws GeometryException {
+  void testMergesDisconnectedRoadNameFeatures() throws GeometryException {
-    testMergesLinestrings(Map.of("class", "motorway"), Transportation.LAYER_NAME, 10, 14);
+    testMergesLinestrings(Map.of("class", "motorway"), TransportationName.LAYER_NAME, 10, 14);
   }
 
   @Test
-  void testMergesDisconnectedRoadNameFeatures() throws GeometryException {
+  void testMergesDisconnectedRoadFeaturesUnlessOneway() throws GeometryException {
-    testMergesLinestrings(Map.of("class", "motorway"), TransportationName.LAYER_NAME, 10, 14);
+    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