areaToMinZoom refactor

src/main/java/org/openmaptiles/layers/Poi.java

@@ -155,12 +155,7 @@ private int minzoom(String subclass, String mappingKey) {
 
   public static int uniAreaToMinZoom(double areaWorld) {
     double oneSideWorld = Math.sqrt(areaWorld);
-    // adjusted formula from `Utils.getMinZoomForLength()`, given that 1/10 does not match `1 / (2^<something>)`
     double zoom = -(Math.log(oneSideWorld * SQRT10) / LOG2);
-
-    // Say Z13.01 means bellow threshold, Z13.00 is exactly threshold, Z12.99 is over threshold,
-    // hence Z13.01 and Z13.00 will be rounded to Z14 and Z12.99 to Z13 (e.g. `floor() + 1`).
-    // And to accommodate for some precision errors (observed for Z9-Z11) we do also `- 0.1e-10`.
     int result = (int) Math.floor(zoom - 0.1e-10) + 1;
 
     return Math.clamp(result, 10, 14);

src/main/java/org/openmaptiles/layers/Transportation.java

@@ -74,7 +74,6 @@ OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 import org.openmaptiles.OpenMapTilesProfile;
 import org.openmaptiles.generated.OpenMapTilesSchema;
 import org.openmaptiles.generated.Tables;
-import org.openmaptiles.util.Utils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -166,6 +165,7 @@ public class Transportation implements
   private static final Set<Integer> ONEWAY_VALUES = Set.of(-1, 1);
   private final Map<String, Integer> MINZOOMS;
   private static final String LIMIT_MERGE_TAG = "__limit_merge";
+  private static final double LOG2 = Math.log(2);
   private final AtomicBoolean loggedNoGb = new AtomicBoolean(false);
   private final AtomicBoolean loggedNoIreland = new AtomicBoolean(false);
   private final boolean z13Paths;
@@ -425,10 +425,6 @@ public void process(Tables.OsmHighwayLinestring element, FeatureCollector featur
       }
       int minzoom = getMinzoom(element, highwayClass);
 
-      String brunnelValue = brunnel(element.isBridge(), element.isTunnel(), element.isFord());
-      int brunnelMinzoom = brunnelValue != null ? getBrunnelMinzoom(element) : minzoom;
-      int layerMinzoom = Math.max(brunnelMinzoom, 9);
-
       if (minzoom > config.maxzoom()) {
         return;
       }
@@ -442,11 +438,11 @@ public void process(Tables.OsmHighwayLinestring element, FeatureCollector featur
         .setAttr(Fields.CLASS, highwayClass)
         .setAttr(Fields.SUBCLASS, highwaySubclass(highwayClass, element.publicTransport(), highway))
         .setAttr(Fields.NETWORK, networkType != null ? networkType.name : null)
-        .setAttrWithMinzoom(Fields.BRUNNEL, brunnelValue, brunnelMinzoom)
+        .setAttrWithMinSize(Fields.BRUNNEL, brunnel(element.isBridge(), element.isTunnel(), element.isFord()), 4, 4, 12)
         // z8+
         .setAttrWithMinzoom(Fields.EXPRESSWAY, element.expressway() && !"motorway".equals(highway) ? 1 : null, 8)
         // z9+
-        .setAttrWithMinzoom(Fields.LAYER, nullIfLong(element.layer(), 0), layerMinzoom)
+        .setAttrWithMinSize(Fields.LAYER, nullIfLong(element.layer(), 0), 4, 9, 12)
         .setAttrWithMinzoom(Fields.BICYCLE, nullIfEmpty(element.bicycle()), 9)
         .setAttrWithMinzoom(Fields.FOOT, nullIfEmpty(element.foot()), 9)
         .setAttrWithMinzoom(Fields.HORSE, nullIfEmpty(element.horse()), 9)
@@ -511,17 +507,6 @@ int getMinzoom(Tables.OsmHighwayLinestring element, String highwayClass) {
     return minzoom;
   }
 
-  int getBrunnelMinzoom(Tables.OsmHighwayLinestring element) {
-    try {
-      return Utils.getClippedMinZoomForLength(element.source().length(), 6, 4, 12);
-    } catch (GeometryException e) {
-      e.log(stats, "omt_brunnel_minzoom",
-        "Unable to calculate brunnel minzoom for " + element.source().id());
-      // brunnel is optional (depends on feature size) for Z4-Z11, it is always present for Z12+, hence 12 as fallback
-      return 12;
-    }
-  }
-
   private boolean isPierPolygon(Tables.OsmHighwayLinestring element) {
     if ("pier".equals(element.manMade())) {
       try {
@@ -598,13 +583,20 @@ public void process(Tables.OsmShipwayLinestring element, FeatureCollector featur
       .setMinZoom(getFerryMinzoom(element));
   }
 
+  /*
+   * Ferries are supposed to be included in Z4-Z10 depending on their length, for Z11+ always. This implements
+   * the equivalent of `sql_filter: ST_Length(geometry)>2*ZRES0` in OpenMapTiles to make sure that only longer ferries
+   * make it into lower zoom. That is needed since ferries are not tagged as highways based on importance, hence length
+   * is a substitute for importance.
+   */
   int getFerryMinzoom(Tables.OsmShipwayLinestring element) {
     try {
-      return Utils.getClippedMinZoomForLength(element.source().length(), 3, 4, 11);
+      double zoom = -(Math.log(element.source().length()) / LOG2) - 3;
+      return Math.clamp((int) Math.floor(zoom - 0.1e-10) + 1, 4, 11);
     } catch (GeometryException e) {
       e.log(stats, "omt_ferry_minzoom",
         "Unable to calculate ferry minzoom for " + element.source().id());
-      // ferries are supposed to be included in Z4-Z10 depending on their length (=this min. zoom calculation), for Z11+ always, hence 11 as fallback
+      // for Z11+ always, hence 11 as fallback
       return 11;
     }
   }

src/main/java/org/openmaptiles/layers/TransportationName.java

@@ -279,9 +279,9 @@ public void process(Tables.OsmHighwayLinestring element, FeatureCollector featur
     }
 
     if (brunnel) {
-      String brunnelValue = brunnel(element.isBridge(), element.isTunnel(), element.isFord());
-      int brunnelMinzoom = brunnelValue != null ? transportation.getBrunnelMinzoom(element) : minzoom;
-      feature.setAttrWithMinzoom(Fields.BRUNNEL, brunnelValue, brunnelMinzoom);
+      // from OMT: "Drop brunnel if length of way < 2% of tile width (less than 3 pixels)"
+      feature.setAttrWithMinSize(Fields.BRUNNEL, brunnel(element.isBridge(), element.isTunnel(), element.isFord()),
+        3, 4, 12);
     }
 
     /*

src/main/java/org/openmaptiles/layers/WaterName.java

@@ -56,7 +56,6 @@ OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 import org.openmaptiles.generated.OpenMapTilesSchema;
 import org.openmaptiles.generated.Tables;
 import org.openmaptiles.util.OmtLanguageUtils;
-import org.openmaptiles.util.Utils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -167,6 +166,11 @@ public void process(Tables.OsmMarinePoint element, FeatureCollector features) {
       if ("ocean".equals(element.place())) {
         minZoom = 0;
       } else if (rank != null) {
+        // FIXME: While this looks like matching properly stuff in https://github.com/openmaptiles/openmaptiles/pull/1457/files#diff-201daa1c61c99073fe3280d440c9feca5ed2236b251ad454caa14cc203f952d1R74 ,
+        // it includes not just https://www.openstreetmap.org/relation/13360255 but also https://www.openstreetmap.org/node/1385157299 (and some others).
+        // Hence check how that OpenMapTiles code works for "James Bay" and:
+        // a) if same as here then, fix there and then here
+        // b) if OK (while here NOK), fix only here
         minZoom = rank;
       } else if ("bay".equals(element.natural())) {
         minZoom = 13;
@@ -185,49 +189,35 @@ public void process(Tables.OsmMarinePoint element, FeatureCollector features) {
   @Override
   public void process(Tables.OsmWaterPolygon element, FeatureCollector features) {
     if (nullIfEmpty(element.name()) != null) {
-      try {
-        Geometry centerlineGeometry = lakeCenterlines.get(element.source().id());
-        FeatureCollector.Feature feature;
-        int minzoom = 9;
-        String place = element.place();
-        String clazz;
-        if ("bay".equals(element.natural())) {
-          clazz = FieldValues.CLASS_BAY;
-        } else if ("sea".equals(place)) {
-          clazz = FieldValues.CLASS_SEA;
-        } else {
-          clazz = FieldValues.CLASS_LAKE;
-          minzoom = 3;
-        }
-        if (centerlineGeometry != null) {
-          // prefer lake centerline if it exists
-          feature = features.geometry(LAYER_NAME, centerlineGeometry)
-            .setMinPixelSizeBelowZoom(13, 6d * element.name().length());
-        } else {
-          // otherwise just use a label point inside the lake
-          feature = features.pointOnSurface(LAYER_NAME);
-          double area = element.source().area();
-          if (place != null && SEA_OR_OCEAN_PLACE.contains(place)) {
-            minzoom = areaToMinZoom(area, 0);
-          } else {
-            minzoom = areaToMinZoom(area, 3);
-          }
-        }
-        feature
-          .setAttr(Fields.CLASS, clazz)
-          .setBufferPixels(BUFFER_SIZE)
-          .putAttrs(OmtLanguageUtils.getNames(element.source().tags(), translations))
-          .setAttr(Fields.INTERMITTENT, element.isIntermittent() ? 1 : 0)
-          .setMinZoom(minzoom);
-      } catch (GeometryException e) {
-        e.log(stats, "omt_water_polygon", "Unable to get geometry for water polygon " + element.source().id());
+      Geometry centerlineGeometry = lakeCenterlines.get(element.source().id());
+      FeatureCollector.Feature feature;
+      int minzoom = 9;
+      String place = element.place();
+      String clazz;
+      if ("bay".equals(element.natural())) {
+        clazz = FieldValues.CLASS_BAY;
+      } else if ("sea".equals(place)) {
+        clazz = FieldValues.CLASS_SEA;
+      } else {
+        clazz = FieldValues.CLASS_LAKE;
+        minzoom = 3;
       }
+      if (centerlineGeometry != null) {
+        // prefer lake centerline if it exists
+        feature = features.geometry(LAYER_NAME, centerlineGeometry)
+          .setMinPixelSizeBelowZoom(13, 6d * element.name().length());
+      } else {
+        // otherwise just use a label point inside the lake
+        feature = features.pointOnSurface(LAYER_NAME)
+          .setMinZoom(place != null && SEA_OR_OCEAN_PLACE.contains(place) ? 0 : 3)
+          .setMinPixelSize(128); // tiles are 256x256, so 128x128 is 1/4 of a tile
+      }
+      feature
+        .setAttr(Fields.CLASS, clazz)
+        .setBufferPixels(BUFFER_SIZE)
+        .putAttrs(OmtLanguageUtils.getNames(element.source().tags(), translations))
+        .setAttr(Fields.INTERMITTENT, element.isIntermittent() ? 1 : 0)
+        .setMinZoom(minzoom);
     }
   }
-
-  public static int areaToMinZoom(double areaWorld, int minLimit) {
-    double oneSideWorld = Math.sqrt(areaWorld);
-    // OMT does "feature area is 1/4 of tile area", which is same as "feature side is 1/2 of tile side"
-    return Utils.getClippedMinZoomForLength(oneSideWorld, 1, minLimit, 14);
-  }
 }

src/main/java/org/openmaptiles/util/Utils.java

@@ -76,44 +76,4 @@ public static String brunnel(boolean isBridge, boolean isTunnel) {
   public static String brunnel(boolean isBridge, boolean isTunnel, boolean isFord) {
     return isBridge ? "bridge" : isTunnel ? "tunnel" : isFord ? "ford" : null;
   }
-
-  /**
-   * Calculate minzoom for a feature with given length based on threshold: if a feature's length is least
-   * {@code 1 / (2^threshold)} of a tile at certain zoom level, it will be shown at that and higher zoom levels, e.g.
-   * that particular zoom level is minzoom.
-   * <p>
-   * {@code threshold} is calculated in such way to avoid calculating log(2) os it during the runtime. Use 1 for 1/2, 2
-   * for 1/4, 3 for 1/8, etc.
-   * 
-   * @param length    length of the feature
-   * @param threshold threshold
-   * @return minzoom for a feature with given length and given threshold
-   */
-  public static int getMinZoomForLength(double length, double threshold) {
-    // Say threshold is 1/8 (threshold variable = 8) of tile size, hence ...
-    // ... from pixels to world coord, for say Z14, the minimum length is:
-    //  PORTION_OF_TILE_SIDE = (256d / 8) / Math.pow(2d, 14d + 8d);
-    // ... and then minimum length for some lower zoom:
-    //  PORTION_OF_TILE_SIDE * Math.pow(2, 14 - zoom);
-    // all this then reversed and simplified to:
-    double zoom = -(Math.log(length) / LOG2) - threshold;
-
-    // Say Z13.01 means bellow threshold, Z13.00 is exactly threshold, Z12.99 is over threshold,
-    // hence Z13.01 and Z13.00 will be rounded to Z14 and Z12.99 to Z13 (e.g. `floor() + 1`).
-    // And to accommodate for some precision errors (observed for Z9-Z11) we do also `- 0.1e-10`.
-    return (int) Math.floor(zoom - 0.1e-10) + 1;
-  }
-
-  /**
-   * Same as {@link #getMinZoomForLength(double, double)} but with result within the given minimum and maximim.
-   * 
-   * @param length    length of the feature
-   * @param threshold threshold
-   * @param min       clip the result to this value if lower
-   * @param max       clip the result to this value if higher
-   * @return minzoom for a feature with given length and given threshold clipped to not exceed given minimum and maximum
-   */
-  public static int getClippedMinZoomForLength(double length, double threshold, int min, int max) {
-    return Math.clamp(getMinZoomForLength(length, threshold), min, max);
-  }
 }

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

@@ -2020,54 +2020,6 @@ private record TestEntry(
     int expectedZoom
   ) {}
 
-  private void createBrunnelForMinZoomTest(List<TestEntry> testEntries, double length, int expectedZoom, String name) {
-    var feature = lineFeatureWithLength(length, Map.of(
-      "name", name,
-      "bridge", "yes",
-      "highway", "motorway"
-    ));
-    testEntries.add(new TestEntry(
-      feature,
-      Math.clamp(expectedZoom, 4, 12)
-    ));
-  }
-
-  @Test
-  void testGetBrunnelMinzoom() throws GeometryException {
-    final List<TestEntry> testEntries = new ArrayList<>();
-    for (int zoom = 14; zoom >= 0; zoom--) {
-      double testLength = Math.pow(2, -zoom - 6);
-
-      // slightly bellow the threshold
-      createBrunnelForMinZoomTest(testEntries, testLength * 0.999, zoom + 1, "brunnel-");
-      // precisely at the threshold
-      createBrunnelForMinZoomTest(testEntries, testLength, zoom, "brunnel=");
-      // slightly over the threshold
-      createBrunnelForMinZoomTest(testEntries, testLength * 1.001, zoom, "brunnel+");
-    }
-
-    for (var entry : testEntries) {
-      var result = process(entry.feature);
-
-      assertFeatures(entry.expectedZoom, List.of(Map.of(
-        "_layer", "transportation",
-        "_type", "line",
-        "class", "motorway",
-        "brunnel", "bridge"
-      ), Map.of(
-        "_layer", "transportation_name",
-        "_type", "line"
-      )), result);
-
-      assertFeatures(entry.expectedZoom - 1, List.of(Map.of(
-        "_layer", "transportation",
-        "brunnel", "<null>"
-      ), Map.of(
-        "_layer", "transportation_name"
-      )), result);
-    }
-  }
-
   private void createFerryForMinZoomTest(List<TestEntry> testEntries, double length, int expectedZoom, String name) {
     var feature = lineFeatureWithLength(length, Map.of(
       "name", name,
@@ -2103,9 +2055,6 @@ void testGetFerryMinzoom() throws GeometryException {
         "_layer", "transportation_name",
         "_type", "line"
       )), process(entry.feature));
-      /*
-      process(entry.feature);
-       */
     }
   }