Test coverage for building heights [#205]

CHANGELOG.md

@@ -1,3 +1,9 @@
+Tiles v3.2.0
+------
+- Add `village_green` and `allotments` to landuse layer via @lenalebt [#206]
+- Remove non-deterministic ordering ID from POIs
+- stricter parsing of building height values [#205]
+
 Tiles v3.1.0
 ------
 - Boundaries admin_level 3 and 5 are included along with 4 and 6, respectively [#189]

tiles/src/main/java/com/protomaps/basemap/Basemap.java

@@ -92,7 +92,7 @@ public String description() {
 
   @Override
   public String version() {
-    return "3.1.0";
+    return "3.2.0";
   }
 
   @Override

tiles/src/main/java/com/protomaps/basemap/layers/Buildings.java

@@ -12,14 +12,42 @@
 import com.protomaps.basemap.feature.FeatureId;
 import com.protomaps.basemap.postprocess.Area;
 import java.util.List;
+import java.util.regex.Pattern;
 
 public class Buildings implements ForwardingProfile.FeatureProcessor, ForwardingProfile.FeaturePostProcessor {
 
+  static final String HEIGHT_KEY = "height";
+  static final String MIN_HEIGHT_KEY = "height";
+
   @Override
   public String name() {
     return "buildings";
   }
 
+  public record Height(Double height, Double min_height) {}
+
+
+  static final Pattern pattern = Pattern.compile("^\\d+(\\.\\d)?$");
+
+  static Double parseWellFormedDouble(String s) {
+    if (pattern.matcher(s).matches()) {
+      return parseDoubleOrNull(s);
+    }
+    return null;
+  }
+
+  static Height parseHeight(String osmHeight, String osmLevels, String osmMinHeight) {
+    var height = parseDoubleOrNull(osmHeight);
+    if (height == null) {
+      Double levels = parseDoubleOrNull(osmLevels);
+      if (levels != null) {
+        height = Math.max(levels, 1) * 3 + 2;
+      }
+    }
+
+    return new Height(height, parseDoubleOrNull(osmMinHeight));
+  }
+
   static int quantizeVal(double val, int step) {
     // special case: if val is very small, we don't want it rounding to zero, so
     // round the smallest values up to the first step.
@@ -34,8 +62,8 @@ static int quantizeVal(double val, int step) {
   public void processFeature(SourceFeature sf, FeatureCollector features) {
     if (sf.canBePolygon() && ((sf.hasTag("building") && !sf.hasTag("building", "no")) ||
       (sf.hasTag("building:part") && !sf.hasTag("building:part", "no")))) {
-      Double height = parseDoubleOrNull(sf.getString("height"));
-      Double minHeight = parseDoubleOrNull(sf.getString("min_height"));
+
+      var height = parseHeight(sf.getString(HEIGHT_KEY), sf.getString("building:levels"), sf.getString(MIN_HEIGHT_KEY));
       Integer minZoom = 11;
       String kind = "building";
 
@@ -46,27 +74,20 @@ public void processFeature(SourceFeature sf, FeatureCollector features) {
         minZoom = 14;
       }
 
-      if (height == null) {
-        Double levels = parseDoubleOrNull(sf.getString("building:levels"));
-        if (levels != null) {
-          height = Math.max(levels, 1) * 3 + 2;
-        }
-      }
-
       var feature = features.polygon(this.name())
         .setId(FeatureId.create(sf))
         // Core Tilezen schema properties
         .setAttr("pmap:kind", kind)
         // Core OSM tags for different kinds of places
         .setAttrWithMinzoom("layer", Parse.parseIntOrNull(sf.getString("layer")), 13)
         // NOTE: Height is quantized by zoom in a post-process step
-        .setAttr("height", height)
+        .setAttr(HEIGHT_KEY, height.height())
         .setZoomRange(minZoom, 15);
 
       if (kind.equals("building_part")) {
         // We don't need to set WithMinzoom because that's implicate with the ZoomRange
         feature.setAttr("pmap:kind_detail", sf.getString("building:part"));
-        feature.setAttr("min_height", minHeight);
+        feature.setAttr(MIN_HEIGHT_KEY, height.min_height());
       }
 
       // Names should mostly just be for POIs
@@ -87,8 +108,8 @@ public List<VectorTile.Feature> postProcess(int zoom, List<VectorTile.Feature> i
 
     // quantize height by zoom when less than max_zoom 15 to facilitate better feature merging
     for (var item : items) {
-      if (item.attrs().containsKey("height")) {
-        var height = (double) item.attrs().get("height");
+      if (item.attrs().containsKey(HEIGHT_KEY)) {
+        var height = (double) item.attrs().get(HEIGHT_KEY);
 
         // Protected against NULL values
         if (height > 0) {
@@ -105,12 +126,12 @@ public List<VectorTile.Feature> postProcess(int zoom, List<VectorTile.Feature> i
             height = quantizeVal(height, 5);
           }
 
-          item.attrs().put("height", height);
+          item.attrs().put(HEIGHT_KEY, height);
         }
       }
 
-      if (item.attrs().containsKey("min_height")) {
-        var minHeight = (double) item.attrs().get("min_height");
+      if (item.attrs().containsKey(MIN_HEIGHT_KEY)) {
+        var minHeight = (double) item.attrs().get(MIN_HEIGHT_KEY);
 
         // Protected against NULL values
         if (minHeight > 0) {
@@ -126,7 +147,7 @@ public List<VectorTile.Feature> postProcess(int zoom, List<VectorTile.Feature> i
             minHeight = quantizeVal(minHeight, 5);
           }
 
-          item.attrs().put("min_height", minHeight);
+          item.attrs().put(MIN_HEIGHT_KEY, minHeight);
         }
       }
     }

tiles/src/test/java/com/protomaps/basemap/layers/BuildingsTest.java

@@ -1,6 +1,8 @@
 package com.protomaps.basemap.layers;
 
 import static com.onthegomap.planetiler.TestUtils.newPolygon;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 
 import com.onthegomap.planetiler.reader.SimpleFeature;
 import java.util.HashMap;
@@ -24,4 +26,33 @@ void simple() {
         0
       )));
   }
+
+  @Test
+  void parseWellFormedDouble() {
+    var result = Buildings.parseWellFormedDouble("10.5");
+    assertEquals(10.5, result);
+    result = Buildings.parseWellFormedDouble("10");
+    assertEquals(10, result);
+    result = Buildings.parseWellFormedDouble("0");
+    assertEquals(0, result);
+    result = Buildings.parseWellFormedDouble("9,10,11,12");
+    assertNull(result);
+  }
+
+  @Test
+  void parseBuildingHeights() {
+    var result = Buildings.parseHeight("10.5", "12", null);
+    assertEquals(10.5, result.height());
+    result = Buildings.parseHeight("2", null, "1");
+    assertEquals(2, result.height());
+    assertEquals(1, result.min_height());
+  }
+
+  @Test
+  void parseBuildingHeightsFromLevels() {
+    var result = Buildings.parseHeight(null, "3", null);
+    assertEquals(11, result.height());
+    result = Buildings.parseHeight(null, "0.5", null);
+    assertEquals(5, result.height());
+  }
 }