Skip to content

Commit

Permalink
Merge pull request #52 from phanecak-maptiler/omt_3_15_0-updates_base…
Browse files Browse the repository at this point in the history 
…d_on_review-3

Updates based on review - round 3
  • Loading branch information
@phanecak-maptiler
phanecak-maptiler authored Apr 18, 2024
2 parents ebf965d + 94e868b commit 17714ea
Show file tree
Hide file tree
Showing 5 changed files with 324 additions and 46 deletions.
8 changes: 7 additions & 1 deletion src/main/java/org/openmaptiles/layers/Boundary.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ public class Boundary implements

private static final Logger LOGGER = LoggerFactory.getLogger(Boundary.class);
private static final double COUNTRY_TEST_OFFSET = GeoUtils.metersToPixelAtEquator(0, 10) / 256d;
private static final String COUNTRY_KE = "Kenya";
private static final String COUNTRY_SS = "South Sudan";
private final Stats stats;
private final boolean addCountryNames;
private final boolean onlyOsmBoundaries;
Expand Down Expand Up @@ -184,7 +186,11 @@ record BoundaryInfo(int adminLevel, int minzoom, int maxzoom) {}
if (disputed) {
String left = feature.getString("adm0_left");
String right = feature.getString("adm0_right");
isDisputedSouthSudanAndKenya = "South Sudan".equals(left) && "Kenya".equals(right);
if (COUNTRY_SS.equals(left)) {
isDisputedSouthSudanAndKenya = COUNTRY_KE.equals(right);
} else if (COUNTRY_KE.equals(left)) {
isDisputedSouthSudanAndKenya = COUNTRY_SS.equals(right);
}
}
yield isDisputedSouthSudanAndKenya ? new BoundaryInfo(2, 1, 4) :
feature.hasTag("featurecla", "Lease limit") ? null :
Expand Down
72 changes: 31 additions & 41 deletions src/main/java/org/openmaptiles/layers/Water.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
import com.onthegomap.planetiler.config.PlanetilerConfig;
import com.onthegomap.planetiler.expression.MultiExpression;
import com.onthegomap.planetiler.geo.GeometryException;
import com.onthegomap.planetiler.geo.PolygonIndex;
import com.onthegomap.planetiler.reader.SimpleFeature;
import com.onthegomap.planetiler.reader.SourceFeature;
import com.onthegomap.planetiler.stats.Stats;
Expand All @@ -51,11 +52,9 @@ OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Consumer;
import org.locationtech.jts.geom.Envelope;
import org.locationtech.jts.geom.Geometry;
import org.locationtech.jts.geom.TopologyException;
import org.locationtech.jts.geom.util.GeometryFixer;
import org.locationtech.jts.index.strtree.STRtree;
import org.openmaptiles.OpenMapTilesProfile;
import org.openmaptiles.generated.OpenMapTilesSchema;
import org.openmaptiles.generated.Tables;
Expand Down Expand Up @@ -89,7 +88,7 @@ public class Water implements
private final MultiExpression.Index<String> classMapping;
private final PlanetilerConfig config;
private final Stats stats;
private final Map<String, STRtree> neLakeIndexes = new ConcurrentHashMap<>();
private PolygonIndex<LakeInfo> neLakeIndex = PolygonIndex.create();
private final Map<String, Map<String, LakeInfo>> neLakeNameMaps = new ConcurrentHashMap<>();
private final List<LakeInfo> neAllLakeInfos = new ArrayList<>();

Expand Down Expand Up @@ -125,14 +124,13 @@ record WaterInfo(int minZoom, int maxZoom, String clazz) {}
lakeInfo.geom = geom;
lakeInfo.name = feature.getString("name");

var index = neLakeIndexes.computeIfAbsent(table, t -> new STRtree());
var neLakeNameMap = neLakeNameMaps.computeIfAbsent(table, t -> new ConcurrentHashMap<>());

// need to externally synchronize inserts into the STRTree and ArrayList
// need to externally synchronize inserts into ArrayList
synchronized (this) {
index.insert(geom.getEnvelopeInternal(), lakeInfo);
neAllLakeInfos.add(lakeInfo);
}
neLakeIndex.put(geom, lakeInfo);
if (lakeInfo.name != null) {
if (!neLakeNameMap.containsKey(lakeInfo.name) ||
lakeInfo.geom.getArea() > neLakeNameMap.get(lakeInfo.name).geom.getArea()) {
Expand Down Expand Up @@ -184,62 +182,55 @@ public void process(Tables.OsmWaterPolygon element, FeatureCollector features) {

void fillOsmIdIntoNeLake(Tables.OsmWaterPolygon element) {
try {
// if OSM lake is too small for Z6 (e.g. area bellow ~4px) we assume there is no matching NE lake
Geometry geom = element.source().worldGeometry();
if (geom.getArea() < OSM_ID_MATCH_AREA_LIMIT) {
return;
}

// match by name:
boolean match = false;
if (element.name() != null) {
for (var map : neLakeNameMaps.values()) {
var lakeInfo = map.get(element.name());
if (lakeInfo != null) {
match = true;
fillOsmIdIntoNeLake(element, element.source().worldGeometry(), lakeInfo);
fillOsmIdIntoNeLake(element, element.source().worldGeometry(), lakeInfo, true);
}
}
}
if (match) {
return;
}

// if OSM lake is too small for Z6 (e.g. area bellow ~4px) we assume there is no matching NE lake
Geometry geom = element.source().worldGeometry();
if (geom.getArea() < OSM_ID_MATCH_AREA_LIMIT) {
return;
}

// match by intersection:
Envelope envelope = geom.getEnvelopeInternal();

for (var index : neLakeIndexes.values()) {
var items = index.query(envelope);
if (items.size() == 1) {
if (items.getFirst()instanceof LakeInfo lakeInfo) {
fillOsmIdIntoNeLake(element, geom, lakeInfo);
}
} else if (!items.isEmpty()) {
for (var item : items) {
if (item instanceof LakeInfo lakeInfo) {
fillOsmIdIntoNeLake(element, geom, lakeInfo);
}
}
}
var items = neLakeIndex.getIntersecting(geom);
for (var lakeInfo : items) {
fillOsmIdIntoNeLake(element, geom, lakeInfo, false);
}
} catch (GeometryException e) {
e.log(stats, "omt_water",
"Error getting geometry for OSM feature " + element.source().id());
}
}

void fillOsmIdIntoNeLake(Tables.OsmWaterPolygon element, Geometry geom, LakeInfo lakeInfo) throws GeometryException {
/*
* When we match lakes with `neLakeIndexes` then `intersetsCheckNeeded` should be `false`,
* otherwise `true`, to make sure we DO check the intersection but to avoid checking it twice.
*/
void fillOsmIdIntoNeLake(Tables.OsmWaterPolygon element, Geometry geom, LakeInfo lakeInfo,
boolean intersetsCheckNeeded) throws GeometryException {
Geometry neGeom = lakeInfo.geom;
Geometry intersection;
try {
if (!neGeom.intersects(geom)) {
if (intersetsCheckNeeded && !neGeom.intersects(geom)) {
return;
}
intersection = neGeom.intersection(geom);
} catch (TopologyException e) {
try {
Geometry fixedGeom = GeometryFixer.fix(geom);
if (!neGeom.intersects(fixedGeom)) {
if (intersetsCheckNeeded && !neGeom.intersects(fixedGeom)) {
return;
}
intersection = neGeom.intersection(fixedGeom);
Expand All @@ -249,11 +240,13 @@ void fillOsmIdIntoNeLake(Tables.OsmWaterPolygon element, Geometry geom, LakeInfo
}
}

// should match following in OpenMapTiles: Distinct on keeps just the first occurence -> order by 'area_ratio DESC'
double areaRatio = intersection.getArea() / neGeom.getArea();
if (areaRatio > lakeInfo.areaRatio) {
// Should match following in OpenMapTiles: Distinct on keeps just the first occurence -> order by 'area_ratio DESC'
// With a twist: NE geometry is always the same, hence we can make it a little bit faster by dropping "ratio"
// and compare only the intersection area: bigger area -> bigger ratio.
double area = intersection.getArea();
if (area > lakeInfo.area) {
lakeInfo.osmId = element.source().id();
lakeInfo.areaRatio = areaRatio;
lakeInfo.area = area;
}
}

Expand All @@ -262,9 +255,6 @@ public void finish(String sourceName, FeatureCollector.Factory featureCollectors
Consumer<FeatureCollector.Feature> emit) {
if (OpenMapTilesProfile.NATURAL_EARTH_SOURCE.equals(sourceName)) {
var timer = stats.startStage("ne_lake_index");
for (var index : neLakeIndexes.values()) {
index.build();
}
timer.stop();
} else if (OpenMapTilesProfile.OSM_SOURCE.equals(sourceName)) {
var timer = stats.startStage("ne_lakes");
Expand All @@ -276,7 +266,7 @@ public void finish(String sourceName, FeatureCollector.Factory featureCollectors
}
}
neLakeNameMaps.clear();
neLakeIndexes.clear();
neLakeIndex = null;
neAllLakeInfos.clear();
timer.stop();
}
Expand All @@ -297,15 +287,15 @@ private static class LakeInfo {
String clazz;
Geometry geom;
Long osmId;
double areaRatio;
double area;

public LakeInfo(int minZoom, int maxZoom, String clazz) {
this.name = null;
this.minZoom = minZoom;
this.maxZoom = maxZoom;
this.clazz = clazz;
this.osmId = null;
this.areaRatio = 0;
this.area = 0;
}
}
}
5 changes: 2 additions & 3 deletions src/main/java/org/openmaptiles/layers/WaterName.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ public void process(Tables.OsmWaterPolygon element, FeatureCollector features) {
minzoomCL = MINZOOM_LAKE;
}
if (centerlineGeometry != null) {
// prefer lake centerline if it exists, but point will be also used if minzoom bellow 9 is calculated from area
// prefer lake centerline if it exists, but point will be also used if minzoom below 9 is calculated from area
// note: Here we're diverging from OpenMapTiles: For bays with minzoom (based on area) point is used between
// minzoom and Z8 and for Z9+ centerline is used, while OpenMaptiles sticks with points.
setupOsmWaterPolygonFeature(
Expand All @@ -241,8 +241,7 @@ public void process(Tables.OsmWaterPolygon element, FeatureCollector features) {
}

// Show a label if a water feature covers at least 1/4 of a tile or z14+
Geometry geometry = element.source().worldGeometry();
double area = geometry.getArea();
final double area = element.source().worldGeometry().getArea();
int minzoom = (int) Math.floor(-1d - Math.log(Math.sqrt(area)) / LOG2);
if (place != null && SEA_OR_OCEAN_PLACE.contains(place)) {
minzoom = Math.clamp(minzoom, MINZOOM_SEA_AND_OCEAN, 14);
Expand Down
38 changes: 38 additions & 0 deletions src/test/java/org/openmaptiles/layers/BoundaryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,44 @@ void testNaturalEarthCountryBoundaries() {
)));
}

@Test
void testNaturalEarthCountryKeSsBoundaryReversed() {
assertFeatures(0, List.of(Map.of(
"_layer", "boundary",
"_minzoom", 1,
"admin_level", 2
)), process(SimpleFeature.create(
newLineString(0, 0, 1, 1),
Map.of(
"featurecla", "Disputed (please verify)",
"adm0_right", "South Sudan",
"adm0_left", "Kenya"
),
OpenMapTilesProfile.NATURAL_EARTH_SOURCE,
"ne_10m_admin_0_boundary_lines_land",
0
)));
}

@Test
void testNaturalEarthCountryNotKeSsBoundary() {
assertFeatures(0, List.of(Map.of(
"_layer", "boundary",
"_minzoom", 4,
"admin_level", 2
)), process(SimpleFeature.create(
newLineString(0, 0, 1, 1),
Map.of(
"featurecla", "Disputed (please verify)",
"adm0_left", "South Sudan",
"adm0_right", "Uganda"
),
OpenMapTilesProfile.NATURAL_EARTH_SOURCE,
"ne_10m_admin_0_boundary_lines_land",
0
)));
}

@Test
void testNaturalEarthStateBoundaries() {
assertFeatures(0, List.of(Map.of(
Expand Down
Loading

0 comments on commit 17714ea

Please sign in to comment.