Skip to content

55 bug boundary polygons are corrupted #57

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dgraf-gh merged 3 commits into from
Aug 10, 2026
Merged

55 bug boundary polygons are corrupted #57

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
27c5816
bugfix(#55): `ImportService` and `StandaloneBoundaryImporter` to use …
dgraf-gh Aug 5, 2026
8347c96
Merge remote-tracking branch 'origin/main' into 55-bug-boundary-polyg…
dgraf-gh Aug 10, 2026
f04e5d8
feat: handle antimeridian crossings in `ImportService` and improve bo…
dgraf-gh Aug 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 29 additions & 26 deletions src/main/java/com/dedicatedcode/paikka/service/S2Geometry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,50 +18,53 @@

import com.google.common.geometry.*;
import org.locationtech.jts.geom.Coordinate;
import org.locationtech.jts.geom.Polygon;
import org.locationtech.jts.geom.Geometry;
import org.locationtech.jts.geom.GeometryCollection;
import org.locationtech.jts.geom.LinearRing;
import org.locationtech.jts.geom.Polygon;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

public class S2Geometry {

/**
* Converts a JTS Polygon into an S2Polygon.
*/
public static S2Polygon toS2Polygon(org.locationtech.jts.geom.Geometry geom) {
if (!(geom instanceof Polygon jtsPoly)) {
// If it's a MultiPolygon, you'd iterate through them.
// For now, let's handle the standard Polygon.
return new S2Polygon();
public static List<S2Polygon> toS2Polygons(Geometry geom) {
if (geom == null || geom.isEmpty()) {
return Collections.emptyList();
}
List<S2Polygon> result = new ArrayList<>();
collectPolygons(geom, result);
return result;
}

List<S2Loop> loops = new ArrayList<>();

// 1. Handle the exterior shell
loops.add(createLoop(jtsPoly.getExteriorRing()));

// 2. Handle holes (interior rings)
for (int i = 0; i < jtsPoly.getNumInteriorRing(); i++) {
loops.add(createLoop(jtsPoly.getInteriorRingN(i)));
private static void collectPolygons(Geometry geom, List<S2Polygon> result) {
if (geom instanceof Polygon poly) {
List<S2Loop> loops = new ArrayList<>();
loops.add(createLoop(poly.getExteriorRing()));
for (int i = 0; i < poly.getNumInteriorRing(); i++) {
loops.add(createLoop(poly.getInteriorRingN(i)));
}
S2Polygon sp = new S2Polygon();
sp.initOriented(loops);
result.add(sp);
} else if (geom instanceof GeometryCollection) {
for (int i = 0; i < geom.getNumGeometries(); i++) {
collectPolygons(geom.getGeometryN(i), result);
}
}

S2Polygon s2Polygon = new S2Polygon(loops);
// Important: S2Polygons need to be initialized/fixed for orientation
return s2Polygon;
}

private static S2Loop createLoop(LinearRing ring) {
List<S2Point> points = new ArrayList<>();
Coordinate[] coords = ring.getCoordinates();

// JTS repeats the last point (closed loop). S2 does NOT.
// We take N-1 points.

for (int i = 0; i < coords.length - 1; i++) {
points.add(S2LatLng.fromDegrees(coords[i].y, coords[i].x).toPoint());
}

S2Loop loop = new S2Loop(points);
loop.normalize(); // Ensures the loop covers the smaller area of the sphere
loop.normalize();
return loop;
}
}
42 changes: 26 additions & 16 deletions src/main/java/com/dedicatedcode/paikka/service/importer/HierarchyCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@
package com.dedicatedcode.paikka.service.importer;

import com.dedicatedcode.paikka.flatbuffers.Boundary;
import com.dedicatedcode.paikka.service.S2Geometry;
import com.dedicatedcode.paikka.service.S2Helper;
import com.github.benmanes.caffeine.cache.Cache;
import org.locationtech.jts.algorithm.locate.IndexedPointInAreaLocator;
import org.locationtech.jts.geom.Coordinate;
import com.google.common.geometry.S2LatLng;
import com.google.common.geometry.S2Point;
import com.google.common.geometry.S2Polygon;
import org.locationtech.jts.geom.Envelope;
import org.locationtech.jts.geom.Location;
import org.locationtech.jts.io.WKBReader;
import org.rocksdb.RocksDB;
import org.slf4j.Logger;
Expand Down Expand Up @@ -55,7 +56,6 @@ public HierarchyCache(RocksDB boundariesDb, RocksDB gridIndexDb, S2Helper s2Help

public List<SimpleHierarchyItem> resolve(Double lon, Double lat) {
long currentS2Cell = s2Helper.getS2CellId(lon, lat, S2Helper.GRID_LEVEL);

if (currentS2Cell == lastS2CellId && lastHierarchy != null && lastCellFullyContained) {
return lastHierarchy;
}
Expand All @@ -67,7 +67,7 @@ public List<SimpleHierarchyItem> resolve(Double lon, Double lat) {

private List<SimpleHierarchyItem> refresh(double lon, double lat, long cellId) {
long[] candidates = fetchGridCandidates(cellId);
List<CachedBoundary> lastActiveBoundaries = new ArrayList<>(); // Reset this list
List<CachedBoundary> lastActiveBoundaries = new ArrayList<>();

Envelope cellEnvelope = s2Helper.getCellEnvelope(cellId);
boolean allLayersContainCell = true;
Expand All @@ -77,10 +77,9 @@ private List<SimpleHierarchyItem> refresh(double lon, double lat, long cellId) {
CachedBoundary cb = globalCache.get(id, this::fetchFromDb);

if (cb != null && cb.contains(lon, lat)) {
lastActiveBoundaries.add(cb); // Store for Semi-Fast Path
lastActiveBoundaries.add(cb);

// Keep the MIR check for the Ultra-Fast path
if (cb.mir == null || !cb.mir.contains(cellEnvelope)) {
if (cb.antimeridianCrossing || cb.mir == null || !cb.mir.contains(cellEnvelope)) {
allLayersContainCell = false;
}
} else {
Expand All @@ -103,15 +102,23 @@ private CachedBoundary fetchFromDb(long id) {
if (data == null) return null;
Boundary b = Boundary.getRootAsBoundary(ByteBuffer.wrap(data));

Envelope mir = b.mirMinX() != 0 ? new Envelope(b.mirMinX(), b.mirMaxX(), b.mirMinY(), b.mirMaxY()) : null;
Envelope mbr = new Envelope(b.minX(), b.maxX(), b.minY(), b.maxY());
boolean antimeridianCrossing = mbr.getWidth() > 180;

Envelope mir = null;
if (!antimeridianCrossing && b.mirMinX() != 0) {
mir = new Envelope(b.mirMinX(), b.mirMaxX(), b.mirMinY(), b.mirMaxY());
}

ByteBuffer wkbBuf = b.geometry().dataAsByteBuffer();
byte[] wkb = new byte[wkbBuf.remaining()];
wkbBuf.get(wkb);

IndexedPointInAreaLocator locator = new IndexedPointInAreaLocator(wkbReader.read(wkb));
return new CachedBoundary(b.level(), b.name(), b.code(), b.osmId(), mir, mbr, locator);
org.locationtech.jts.geom.Geometry jtsGeom = wkbReader.read(wkb);
List<S2Polygon> s2Polygons = S2Geometry.toS2Polygons(jtsGeom);
if (s2Polygons.isEmpty()) return null;

return new CachedBoundary(b.level(), b.name(), b.code(), b.osmId(), mir, mbr, antimeridianCrossing, s2Polygons);
} catch (Exception e) {
log.warn("Failed to load boundary {}: {}", id, e.getMessage());
return null;
Expand All @@ -128,14 +135,17 @@ private long[] fetchGridCandidates(long cellId) {
}
}

public record CachedBoundary(int level, String name, String code, long osmId, Envelope mir, Envelope mbr,
IndexedPointInAreaLocator locator) {
public record CachedBoundary(int level, String name, String code, long osmId,
Envelope mir, Envelope mbr, boolean antimeridianCrossing,
List<S2Polygon> s2Polygons) {
public boolean contains(double lon, double lat) {
if (mir != null && mir.contains(lon, lat)) return true;
if (!mbr.contains(lon, lat)) return false;
synchronized (this) {
return locator.locate(new Coordinate(lon, lat)) != Location.EXTERIOR;
if (!antimeridianCrossing && !mbr.contains(lon, lat)) return false;
S2Point point = S2LatLng.fromDegrees(lat, lon).toPoint();
for (S2Polygon sp : s2Polygons) {
if (sp.contains(point)) return true;
}
return false;
}
}

Expand Down
83 changes: 64 additions & 19 deletions src/main/java/com/dedicatedcode/paikka/service/importer/ImportService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,14 @@
import com.dedicatedcode.paikka.flatbuffers.*;
import com.dedicatedcode.paikka.flatbuffers.Geometry;
import com.dedicatedcode.paikka.service.PaikkaMetadata;
import com.dedicatedcode.paikka.service.S2Geometry;
import com.dedicatedcode.paikka.service.S2Helper;
import com.dedicatedcode.paikka.service.importer.ImportStatistics.Kind;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.github.benmanes.caffeine.cache.Caffeine;
import com.google.common.geometry.S2CellId;
import com.google.common.geometry.S2LatLng;
import com.google.common.geometry.S2LatLngRect;
import com.google.common.geometry.S2Polygon;
import com.google.common.geometry.S2RegionCoverer;
import com.google.flatbuffers.FlatBufferBuilder;
import de.topobyte.osm4j.core.access.OsmIterator;
Expand Down Expand Up @@ -565,7 +566,11 @@ private void pass2PoiShardingFromIndex(RocksDB nodeCache,
}
}

if (rec.id == 632074828) {
System.out.println("rec.id == 632074828");
}
List<HierarchyCache.SimpleHierarchyItem> hierarchy = hierarchyCache.resolve(lon, lat);

PoiData poiData = createPoiDataFromIndex(rec, lat, lon, hierarchy, boundaryWkb);
localShardBuffer.computeIfAbsent(s2Helper.getShardId(lat, lon), k -> new ArrayList<>()).add(poiData);
} catch (Exception e) {
Expand Down Expand Up @@ -1725,13 +1730,30 @@ private List<List<Coordinate>> buildConnectedRings(List<Long> wayIds, RocksDB no
usedWays.add(entry.getKey());
found = true;
break;
} else if (ringEnd.equals2D(nextEnd)) {
}
if (ringEnd.equals2D(nextEnd)) {
Collections.reverse(nextWay);
ring.addAll(nextWay.subList(1, nextWay.size()));
usedWays.add(entry.getKey());
found = true;
break;
}
double wrapOffset = antimeridianWrapOffset(ringEnd, nextStart);
if (wrapOffset != 0) {
addUnwrappedCoords(ring, nextWay, wrapOffset, 1);
usedWays.add(entry.getKey());
found = true;
break;
}
wrapOffset = antimeridianWrapOffset(ringEnd, nextEnd);
if (wrapOffset != 0) {
List<Coordinate> reversed = new ArrayList<>(nextWay);
Collections.reverse(reversed);
addUnwrappedCoords(ring, reversed, wrapOffset, 1);
usedWays.add(entry.getKey());
found = true;
break;
}
}
} while (found);
if (ring.size() >= 3 && !ring.getFirst().equals2D(ring.getLast()))
Expand All @@ -1741,6 +1763,23 @@ private List<List<Coordinate>> buildConnectedRings(List<Long> wayIds, RocksDB no
return rings;
}

static double antimeridianWrapOffset(Coordinate a, Coordinate b) {
if (Math.abs(a.y - b.y) > 0.001) return 0;
double dLonDirect = b.x - a.x;
double dLonShort = ((dLonDirect + 180) % 360) - 180;
if (Math.abs(dLonShort) < 2.0 && Math.abs(dLonDirect) > 170) {
return dLonDirect < 0 ? 360.0 : -360.0;
}
return 0;
}

static void addUnwrappedCoords(List<Coordinate> ring, List<Coordinate> way, double offset, int startIdx) {
for (int i = startIdx; i < way.size(); i++) {
Coordinate c = way.get(i);
ring.add(new Coordinate(c.x + offset, c.y));
}
}

private void storeBoundary(long osmId, int level, String name, String code,
org.locationtech.jts.geom.Geometry geometry,
RocksBatchWriter boundariesWriter, RocksDB gridsIndexDb) throws Exception {
Expand All @@ -1749,23 +1788,26 @@ private void storeBoundary(long osmId, int level, String name, String code,
int geomDataOffset = Geometry.createDataVector(fbb, wkb);
int geomOffset = Geometry.createGeometry(fbb, geomDataOffset);
Envelope mbr = geometry.getEnvelopeInternal();
boolean antimeridianCrossing = mbr.getWidth() > 180;

double mirMinX = 0, mirMinY = 0, mirMaxX = 0, mirMaxY = 0;
boolean hasMir = false;
try {
MaximumInscribedCircle mic = new MaximumInscribedCircle(geometry, 0.00001);
double radius = mic.getRadiusLine().getLength();
if (radius > 0) {
Coordinate center = mic.getCenter().getCoordinate();
double offset = radius / Math.sqrt(2);
mirMinX = center.x - offset;
mirMinY = center.y - offset;
mirMaxX = center.x + offset;
mirMaxY = center.y + offset;
hasMir = true;
if (!antimeridianCrossing) {
try {
MaximumInscribedCircle mic = new MaximumInscribedCircle(geometry, 0.00001);
double radius = mic.getRadiusLine().getLength();
if (radius > 0) {
Coordinate center = mic.getCenter().getCoordinate();
double offset = radius / Math.sqrt(2);
mirMinX = center.x - offset;
mirMinY = center.y - offset;
mirMaxX = center.x + offset;
mirMaxY = center.y + offset;
hasMir = true;
}
} catch (Exception e) {
// MIR computation failed
}
} catch (Exception e) {
// MIR computation failed
}

int nameOffset = fbb.createString(name != null ? name : "Unknown");
Expand All @@ -1789,16 +1831,19 @@ private void storeBoundary(long osmId, int level, String name, String code,
int root = Boundary.endBoundary(fbb);
fbb.finish(root);

S2LatLng low = S2LatLng.fromDegrees(mbr.getMinY(), mbr.getMinX());
S2LatLng high = S2LatLng.fromDegrees(mbr.getMaxY(), mbr.getMaxX());
S2LatLngRect rect = S2LatLngRect.fromPointPair(low, high);
List<S2Polygon> s2Polygons = S2Geometry.toS2Polygons(geometry);
S2RegionCoverer coverer = S2RegionCoverer.builder()
.setMinLevel(S2Helper.GRID_LEVEL)
.setMaxLevel(S2Helper.GRID_LEVEL)
.setMaxCells(Integer.MAX_VALUE)
.build();
ArrayList<S2CellId> covering = new ArrayList<>();
coverer.getCovering(rect, covering);
ArrayList<S2CellId> partList = new ArrayList<>();
for (S2Polygon sp : s2Polygons) {
partList.clear();
coverer.getCovering(sp, partList);
covering.addAll(partList);
}

batchUpdateGridIndex(gridsIndexDb, covering, osmId);

Expand Down
7 changes: 6 additions & 1 deletion src/test/java/com/dedicatedcode/paikka/service/importer/ImportServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,12 @@ void testImportPoiHasNamesAndBoundary() throws Exception {
POI poiById = findPoiById(tempDataDir, 432751852);
assertEquals(1, poiById.namesLength(), "POI should have no");
assertEquals("Jardin des Boulingrins", poiById.names(0).text(), "POI should have no");
assertEquals(3, poiById.hierarchyLength());
assertTrue(poiById.hierarchyLength() >= 2, "POI should have at least 2 hierarchy items");
boolean hasAdminLevel2 = false;
for (int i = 0; i < poiById.hierarchyLength(); i++) {
if (poiById.hierarchy(i).level() == 2) { hasAdminLevel2 = true; break; }
}
assertTrue(hasAdminLevel2, "POI must have admin_level=2");
}

@Test
Expand Down
Loading