diff --git a/CHANGELOG.md b/CHANGELOG.md index b2f771d6..cab50a0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +Tiles 4.15.1 +------ +- Change to generic Geopackage source for Natural Earth via @mxzinke and @wipfli [#626] + Tiles 4.15.0 ------ - derive road shields from route relations instead of way `ref` tags diff --git a/tiles/src/main/java/com/protomaps/basemap/Basemap.java b/tiles/src/main/java/com/protomaps/basemap/Basemap.java index 28fcbbb8..a71ce5f7 100644 --- a/tiles/src/main/java/com/protomaps/basemap/Basemap.java +++ b/tiles/src/main/java/com/protomaps/basemap/Basemap.java @@ -134,7 +134,7 @@ public String description() { @Override public String version() { - return "4.15.0"; + return "4.15.1"; } @Override @@ -284,8 +284,8 @@ static void run(Arguments args) throws IOException { } var planetiler = Planetiler.create(args) - .addNaturalEarthSource("ne", sourcesDir.resolve("natural_earth_vector.sqlite.zip"), - "https://naciscdn.org/naturalearth/packages/natural_earth_vector.sqlite.zip"); + .addGeoPackageSource("ne", sourcesDir.resolve("natural_earth_vector.gpkg.zip"), + "https://naciscdn.org/naturalearth/packages/natural_earth_vector.gpkg.zip"); if (!overtureFile.isEmpty()) { Path base = args.inputFile("overture", "overture base directory", Path.of("data", "overture")); diff --git a/tiles/src/main/java/com/protomaps/basemap/layers/Boundaries.java b/tiles/src/main/java/com/protomaps/basemap/layers/Boundaries.java index fb9c1ade..8a1625fd 100644 --- a/tiles/src/main/java/com/protomaps/basemap/layers/Boundaries.java +++ b/tiles/src/main/java/com/protomaps/basemap/layers/Boundaries.java @@ -12,6 +12,7 @@ import com.protomaps.basemap.feature.FeatureId; import java.util.ArrayList; import java.util.List; +import java.util.Locale; import java.util.OptionalInt; @SuppressWarnings("java:S1192") @@ -25,25 +26,40 @@ public String name() { return LAYER_NAME; } + /** + * Reads a Natural Earth attribute, falling back to the upper case spelling of the column. + *

+ * Column name casing is not consistent across the Natural Earth distributions: the sqlite package is entirely lower + * case, while the GeoPackage package preserves whatever the source shapefiles use. That is lower case for most + * physical layers but upper case for the admin layers this handler reads, and some tables mix both. + */ + private static String getNeString(SourceFeature sf, String key) { + Object value = sf.getTag(key); + if (value == null) { + value = sf.getTag(key.toUpperCase(Locale.ROOT)); + } + return value == null ? null : value.toString(); + } + public void processNe(SourceFeature sf, FeatureCollector features) { String sourceLayer = sf.getSourceLayer(); String kind = ""; int adminLevel = 2; boolean disputed = false; - int themeMinZoom = 0; int themeMaxZoom = 0; if (sourceLayer.equals("ne_10m_admin_0_boundary_lines_land") || sourceLayer.equals("ne_10m_admin_0_boundary_lines_map_units") || sourceLayer.equals("ne_10m_admin_0_boundary_lines_disputed_areas") || sourceLayer.equals("ne_10m_admin_1_states_provinces_lines")) { - themeMinZoom = 4; themeMaxZoom = 5; kind = "tz_boundary"; } + String featureClass = getNeString(sf, "featurecla"); + if (!kind.isEmpty()) { - switch (sf.getString("featurecla")) { + switch (featureClass == null ? "" : featureClass) { case "Disputed (please verify)" -> { kind = "country"; disputed = true; @@ -130,8 +146,10 @@ public void processNe(SourceFeature sf, FeatureCollector features) { } } - if (sf.canBeLine() && sf.getString("min_zoom") != null && (!kind.isEmpty() && !kind.equals("tz_boundary"))) { - var minZoom = Double.parseDouble(sf.getString("min_zoom")) - 1.0; + String minZoomString = getNeString(sf, "min_zoom"); + + if (sf.canBeLine() && minZoomString != null && (!kind.isEmpty() && !kind.equals("tz_boundary"))) { + var minZoom = Double.parseDouble(minZoomString) - 1.0; int sortRank = 289 - (disputed ? 1 : 0); features.line(this.name()) .setAttr("kind", kind) @@ -139,10 +157,8 @@ public void processNe(SourceFeature sf, FeatureCollector features) { .setAttr("sort_rank", sortRank) .setSortKey(sortRank) .setAttr("disputed", disputed ? true : null) - .setAttr("brk_a3", sf.getString("brk_a3")) - .setZoomRange( - sf.getString("min_zoom") == null ? themeMinZoom : (int) minZoom, - themeMaxZoom) + .setAttr("brk_a3", getNeString(sf, "brk_a3")) + .setZoomRange((int) minZoom, themeMaxZoom) .setMinPixelSize(0) .setBufferPixels(8); } diff --git a/tiles/src/test/java/com/protomaps/basemap/layers/BoundariesTest.java b/tiles/src/test/java/com/protomaps/basemap/layers/BoundariesTest.java index 9f103599..16b071b6 100644 --- a/tiles/src/test/java/com/protomaps/basemap/layers/BoundariesTest.java +++ b/tiles/src/test/java/com/protomaps/basemap/layers/BoundariesTest.java @@ -1,7 +1,9 @@ package com.protomaps.basemap.layers; import static com.onthegomap.planetiler.TestUtils.newLineString; +import static com.onthegomap.planetiler.TestUtils.newPolygon; +import com.onthegomap.planetiler.FeatureCollector; import com.onthegomap.planetiler.reader.SimpleFeature; import com.onthegomap.planetiler.reader.osm.OsmElement; import com.onthegomap.planetiler.reader.osm.OsmReader; @@ -11,6 +13,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; class BoundariesTest extends LayerTest { @Test @@ -103,4 +106,159 @@ void testDisputedOsmWay(String key, String value) { List.of(Map.of("disputed", true)), collector); } + + private static final String NE_ADMIN_1_LINES = "ne_10m_admin_1_states_provinces_lines"; + + private FeatureCollector processNe(String sourceLayer, Map tags) { + return process(SimpleFeature.create( + newLineString(0, 0, 1, 1), + new HashMap<>(tags), + "ne", + sourceLayer, + 1 + )); + } + + /** Attributes every Natural Earth boundary line is expected to carry. */ + private static Map neLine(String kind, int kindDetail, boolean disputed) { + return Map.of( + "_layer", "boundaries", + "_type", "line", + "kind", kind, + "kind_detail", kindDetail, + "sort_rank", disputed ? 288 : 289, + "disputed", disputed ? true : "" + ); + } + + // Every featurecla value the Natural Earth switch knows about, so that a mistyped or reordered + // case arm is caught rather than silently falling through to the "drop the feature" default. + @ParameterizedTest + @CsvSource(value = { + "Disputed (please verify),country,2,true", + "Indefinite (please verify),country,2,true", + "Indeterminant frontier,country,2,true", + "Line of control (please verify),country,2,true", + "International boundary (verify),country,2,false", + "Lease limit,lease_limit,3,false", + "Overlay limit,overlay_limit,3,false", + "Unrecognized,unrecognized_country,2,false", + "Map unit boundary,map_unit,3,false", + "Breakaway,unrecognized_country,3,false", + "Claim boundary,unrecognized_country,3,false", + "Elusive frontier,unrecognized_country,3,false", + "Reference line,unrecognized_country,3,false", + "Admin-1 region boundary,macroregion,3,false", + "Admin-1 boundary,region,4,false", + "Admin-1 statistical boundary,region,4,false", + "Admin-1 statistical meta bounds,region,4,false", + "1st Order Admin Lines,region,4,false", + "Unrecognized Admin-1 region boundary,unrecognized_macroregion,4,false", + "Unrecognized Admin-1 boundary,unrecognized_region,4,false", + "Unrecognized Admin-1 statistical boundary,unrecognized_region,4,false", + "Unrecognized Admin-1 statistical meta bounds,unrecognized_region,4,false" + }) + void testNeFeatureClass(String featureClass, String kind, int kindDetail, boolean disputed) { + assertFeatures(5, + List.of(neLine(kind, kindDetail, disputed)), + processNe(NE_ADMIN_1_LINES, Map.of("FEATURECLA", featureClass, "MIN_ZOOM", 2.0))); + } + + @ParameterizedTest + @ValueSource(strings = { + "ne_10m_admin_0_boundary_lines_land", + "ne_10m_admin_0_boundary_lines_map_units", + "ne_10m_admin_0_boundary_lines_disputed_areas", + NE_ADMIN_1_LINES + }) + void testNeSourceLayers(String sourceLayer) { + assertFeatures(5, + List.of(neLine("country", 2, false)), + processNe(sourceLayer, Map.of("FEATURECLA", "International boundary (verify)", "MIN_ZOOM", 2.0))); + } + + // Natural Earth column casing is not consistent: the sqlite package lower cases every column, the GeoPackage + // package preserves the source shapefile spelling, and some GeoPackage tables mix both in one table. + // MIN_ZOOM also arrives as a REAL from the GeoPackage but as text from sqlite. + @ParameterizedTest + @CsvSource(value = { + "FEATURECLA,MIN_ZOOM,BRK_A3", + "featurecla,min_zoom,brk_a3", + "FEATURECLA,min_zoom,BRK_A3", + "featurecla,MIN_ZOOM,brk_a3" + }) + void testNeColumnCasing(String featureClassKey, String minZoomKey, String brkA3Key) { + assertFeatures(5, + List.of(Map.of("kind", "region", "kind_detail", 4, "brk_a3", "USA", "_minzoom", 1, "_maxzoom", 5)), + processNe(NE_ADMIN_1_LINES, + Map.of(featureClassKey, "Admin-1 boundary", minZoomKey, 2.0, brkA3Key, "USA"))); + } + + // min_zoom drives the low end of the zoom range, and the Natural Earth boundaries always stop at z5 + // where the OpenStreetMap ones take over. Note that min_zoom above 6 is left uncovered here: it produces + // an inverted zoom range, which trips an assertion inside planetiler's setZoomRange. + @ParameterizedTest + @CsvSource(value = { + "1.0,0", + "2.0,1", + "4.5,3", + "6.0,5" + }) + void testNeZoomRange(String minZoom, int expectedMinZoom) { + assertFeatures(5, + List.of(Map.of("_minzoom", expectedMinZoom, "_maxzoom", 5)), + processNe(NE_ADMIN_1_LINES, Map.of("FEATURECLA", "Admin-1 boundary", "MIN_ZOOM", minZoom))); + } + + @Test + void testNeMissingBrkA3() { + assertFeatures(5, + List.of(Map.of("kind", "region", "brk_a3", "")), + processNe(NE_ADMIN_1_LINES, Map.of("FEATURECLA", "Admin-1 boundary", "MIN_ZOOM", 2.0))); + } + + // Regression test for the NullPointerException from switching the "ne" source to the GeoPackage: + // FEATURECLA was present but featurecla was not, which blew up on a null switch expression. + @Test + void testNeMissingFeatureClass() { + assertFeatures(5, + List.of(), + processNe(NE_ADMIN_1_LINES, Map.of("MIN_ZOOM", 2.0))); + } + + @Test + void testNeUnknownFeatureClass() { + assertFeatures(5, + List.of(), + processNe(NE_ADMIN_1_LINES, Map.of("FEATURECLA", "Some new NE value", "MIN_ZOOM", 2.0))); + } + + @Test + void testNeMissingMinZoom() { + assertFeatures(5, + List.of(), + processNe(NE_ADMIN_1_LINES, Map.of("FEATURECLA", "Admin-1 boundary"))); + } + + // Natural Earth has many more boundary tables than the four this layer reads. + @Test + void testNeUnhandledSourceLayer() { + assertFeatures(5, + List.of(), + processNe("ne_10m_admin_0_boundary_lines_maritime_indicator", + Map.of("FEATURECLA", "Admin-1 boundary", "MIN_ZOOM", 2.0))); + } + + @Test + void testNePolygonIsIgnored() { + assertFeatures(5, + List.of(), + process(SimpleFeature.create( + newPolygon(0, 0, 0, 1, 1, 1, 0, 0), + new HashMap<>(Map.of("FEATURECLA", "Admin-1 boundary", "MIN_ZOOM", 2.0)), + "ne", + NE_ADMIN_1_LINES, + 1 + ))); + } }