Skip to content

Commit 67bab12

Browse files
committed
fix: enhance geometry handling in ImportService and adjust hierarchy checks in tests
- Added geometry repair mechanisms for invalid geometries and simplified fallback logic in boundary processing. - Improved hierarchy length assertions in tests to account for updated hierarchy data.
1 parent 433feea commit 67bab12

2 files changed

Lines changed: 146 additions & 39 deletions

File tree

src/main/java/com/dedicatedcode/paikka/service/importer/ImportService.java

Lines changed: 144 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -789,16 +789,38 @@ private void processAdministrativeBoundariesFromIndex(RocksDB relIndexDb, RocksD
789789
ecs.submit(() -> {
790790
try {
791791
org.locationtech.jts.geom.Geometry geometry = buildGeometryFromRelRec(rec, nodeCache, wayIndexDb, stats);
792-
if (geometry != null && geometry.isValid()) {
793-
org.locationtech.jts.geom.Geometry simplified = geometrySimplificationService.simplifyByAdminLevel(geometry, rec.level);
794-
return new BoundaryResultLite(rec.osmId, rec.level, rec.name, rec.code, simplified);
792+
if (geometry == null) return null;
793+
794+
if (!geometry.isValid()) {
795+
try {
796+
geometry = geometry.buffer(0);
797+
} catch (Exception e) {
798+
return null;
799+
}
800+
}
801+
if (geometry == null || geometry.isEmpty() || !geometry.isValid()) return null;
802+
803+
org.locationtech.jts.geom.Geometry simplified = geometrySimplificationService.simplifyByAdminLevel(geometry, rec.level);
804+
805+
// Simplification can produce invalid results too
806+
if (simplified == null || simplified.isEmpty() || !simplified.isValid()) {
807+
try {
808+
simplified = simplified != null ? simplified.buffer(0) : null;
809+
} catch (Exception e) {
810+
simplified = geometry; // fall back to unsimplified
811+
}
812+
}
813+
if (simplified == null || simplified.isEmpty() || !simplified.isValid()) {
814+
simplified = geometry; // fall back to unsimplified
795815
}
796-
return null;
816+
817+
return new BoundaryResultLite(rec.osmId, rec.level, rec.name, rec.code, simplified);
797818
} finally {
798819
semaphore.release();
799820
stats.decrementActiveThreads();
800821
}
801822
});
823+
;
802824
submitted.incrementAndGet();
803825

804826
for (Future<BoundaryResultLite> f; (f = ecs.poll()) != null; ) {
@@ -955,7 +977,7 @@ private int serializePoiData(FlatBufferBuilder builder, PoiData poi) {
955977
}
956978

957979

958-
private void compactShards(RocksDB appendDb, RocksDB shardsDb, ImportStatistics stats) throws Exception {
980+
private void compactShards(RocksDB appendDb, RocksDB shardsDb, ImportStatistics stats) {
959981
stats.setCompactionStartTime(System.currentTimeMillis());
960982
stats.setCompactionEntriesTotal(sequence.get());
961983

@@ -1009,7 +1031,7 @@ private void compactShards(RocksDB appendDb, RocksDB shardsDb, ImportStatistics
10091031
* Takes all raw FlatBuffer chunks for a single shard, reads each chunk's POIs,
10101032
* copies them into a fresh FlatBufferBuilder, and writes the merged result.
10111033
*/
1012-
private void flushCompactedShard(List<byte[]> chunks, long shardId, RocksDB shardsDb, WriteOptions writeOptions, POI reusablePoi, Name reusableName, HierarchyItem reusableHier, Address reusableAddr, Geometry reusableGeom, ImportStatistics stats) throws Exception {
1034+
private void flushCompactedShard(List<byte[]> chunks, long shardId, RocksDB shardsDb, WriteOptions writeOptions, POI reusablePoi, Name reusableName, HierarchyItem reusableHier, Address reusableAddr, Geometry reusableGeom, ImportStatistics stats) {
10131035

10141036
// Count total POIs first
10151037
int totalPois = 0;
@@ -1160,7 +1182,23 @@ private org.locationtech.jts.geom.Geometry buildGeometryFromRelRec(RelRec rec, R
11601182
stats.recordError(ImportStatistics.Stage.PROCESSING_ADMIN_BOUNDARIES, Kind.READ, rec.osmId, "rocks-get:way_index", e);
11611183
}
11621184
Polygon polygon = GEOMETRY_FACTORY.createPolygon(shell, holes.toArray(new LinearRing[0]));
1163-
if (polygon.isValid()) validPolygons.add(polygon);
1185+
if (polygon.isValid()) {
1186+
validPolygons.add(polygon);
1187+
} else {
1188+
try {
1189+
org.locationtech.jts.geom.Geometry repaired = polygon.buffer(0);
1190+
if (repaired != null && repaired.isValid() && !repaired.isEmpty()) {
1191+
for (int i = 0; i < repaired.getNumGeometries(); i++) {
1192+
org.locationtech.jts.geom.Geometry part = repaired.getGeometryN(i);
1193+
if (part instanceof Polygon && part.isValid()) {
1194+
validPolygons.add((Polygon) part);
1195+
}
1196+
}
1197+
}
1198+
} catch (Exception repairEx) {
1199+
stats.recordError(ImportStatistics.Stage.PROCESSING_ADMIN_BOUNDARIES, Kind.GEOMETRY, rec.osmId, "repair-polygon", repairEx);
1200+
}
1201+
}
11641202
} catch (Exception e) {
11651203
stats.recordError(ImportStatistics.Stage.PROCESSING_ADMIN_BOUNDARIES, Kind.GEOMETRY, null, "build-boundary-geometry", e);
11661204
}
@@ -1332,16 +1370,33 @@ private List<List<Coordinate>> buildConnectedRings(List<Long> wayIds, RocksDB no
13321370
return rings;
13331371
}
13341372

1335-
private void storeBoundary(long osmId, int level, String name, String code, org.locationtech.jts.geom.Geometry geometry, RocksBatchWriter boundariesWriter, RocksDB gridsIndexDb) throws Exception {
1373+
private void storeBoundary(long osmId, int level, String name, String code,
1374+
org.locationtech.jts.geom.Geometry geometry,
1375+
RocksBatchWriter boundariesWriter, RocksDB gridsIndexDb) throws Exception {
13361376
FlatBufferBuilder fbb = new FlatBufferBuilder(1024);
13371377
byte[] wkb = new WKBWriter().write(geometry);
13381378
int geomDataOffset = Geometry.createDataVector(fbb, wkb);
13391379
int geomOffset = Geometry.createGeometry(fbb, geomDataOffset);
13401380
Envelope mbr = geometry.getEnvelopeInternal();
1341-
MaximumInscribedCircle mic = new MaximumInscribedCircle(geometry, 0.00001);
1342-
double radius = mic.getRadiusLine().getLength();
1343-
Coordinate center = mic.getCenter().getCoordinate();
1344-
double offset = radius / Math.sqrt(2);
1381+
1382+
double mirMinX = 0, mirMinY = 0, mirMaxX = 0, mirMaxY = 0;
1383+
boolean hasMir = false;
1384+
try {
1385+
MaximumInscribedCircle mic = new MaximumInscribedCircle(geometry, 0.00001);
1386+
double radius = mic.getRadiusLine().getLength();
1387+
if (radius > 0) {
1388+
Coordinate center = mic.getCenter().getCoordinate();
1389+
double offset = radius / Math.sqrt(2);
1390+
mirMinX = center.x - offset;
1391+
mirMinY = center.y - offset;
1392+
mirMaxX = center.x + offset;
1393+
mirMaxY = center.y + offset;
1394+
hasMir = true;
1395+
}
1396+
} catch (Exception e) {
1397+
// MIR computation failed — boundary still works, just no fast-path optimisation
1398+
}
1399+
13451400
int nameOffset = fbb.createString(name != null ? name : "Unknown");
13461401
int codeOffset = fbb.createString(code != null ? code : "");
13471402
Boundary.startBoundary(fbb);
@@ -1353,11 +1408,11 @@ private void storeBoundary(long osmId, int level, String name, String code, org.
13531408
Boundary.addMinY(fbb, mbr.getMinY());
13541409
Boundary.addMaxX(fbb, mbr.getMaxX());
13551410
Boundary.addMaxY(fbb, mbr.getMaxY());
1356-
if (radius > 0) {
1357-
Boundary.addMirMinX(fbb, center.x - offset);
1358-
Boundary.addMirMinY(fbb, center.y - offset);
1359-
Boundary.addMirMaxX(fbb, center.x + offset);
1360-
Boundary.addMirMaxY(fbb, center.y + offset);
1411+
if (hasMir) {
1412+
Boundary.addMirMinX(fbb, mirMinX);
1413+
Boundary.addMirMinY(fbb, mirMinY);
1414+
Boundary.addMirMaxX(fbb, mirMaxX);
1415+
Boundary.addMirMaxY(fbb, mirMaxY);
13611416
}
13621417
Boundary.addGeometry(fbb, geomOffset);
13631418
int root = Boundary.endBoundary(fbb);
@@ -1366,14 +1421,66 @@ private void storeBoundary(long osmId, int level, String name, String code, org.
13661421
S2LatLng low = S2LatLng.fromDegrees(mbr.getMinY(), mbr.getMinX());
13671422
S2LatLng high = S2LatLng.fromDegrees(mbr.getMaxY(), mbr.getMaxX());
13681423
S2LatLngRect rect = S2LatLngRect.fromPointPair(low, high);
1369-
S2RegionCoverer coverer = S2RegionCoverer.builder().setMinLevel(S2Helper.GRID_LEVEL).setMaxLevel(S2Helper.GRID_LEVEL).build();
1424+
S2RegionCoverer coverer = S2RegionCoverer.builder()
1425+
.setMinLevel(S2Helper.GRID_LEVEL)
1426+
.setMaxLevel(S2Helper.GRID_LEVEL)
1427+
.setMaxCells(Integer.MAX_VALUE)
1428+
.build();
13701429
ArrayList<S2CellId> covering = new ArrayList<>();
13711430
coverer.getCovering(rect, covering);
1372-
for (S2CellId cellId : covering) updateGridIndexEntry(gridsIndexDb, cellId.id(), osmId);
1431+
1432+
// Batched grid index update instead of per-cell synchronized writes
1433+
batchUpdateGridIndex(gridsIndexDb, covering, osmId);
13731434

13741435
boundariesWriter.put(s2Helper.longToByteArray(osmId), fbb.sizedByteArray());
13751436
}
13761437

1438+
/**
1439+
* Registers a boundary in the grid index for all covered cells.
1440+
* Uses chunked multiGet + WriteBatch for much better throughput than
1441+
* the previous per-cell synchronized approach.
1442+
*/
1443+
private void batchUpdateGridIndex(RocksDB gridIndexDb, ArrayList<S2CellId> cells, long osmId) throws RocksDBException {
1444+
final int CHUNK_SIZE = 5_000;
1445+
for (int offset = 0; offset < cells.size(); offset += CHUNK_SIZE) {
1446+
int end = Math.min(offset + CHUNK_SIZE, cells.size());
1447+
1448+
List<byte[]> keys = new ArrayList<>(end - offset);
1449+
for (int i = offset; i < end; i++) {
1450+
keys.add(s2Helper.longToByteArray(cells.get(i).id()));
1451+
}
1452+
1453+
synchronized (this) {
1454+
List<byte[]> existing = gridIndexDb.multiGetAsList(keys);
1455+
1456+
try (WriteBatch batch = new WriteBatch();
1457+
WriteOptions wo = new WriteOptions().setDisableWAL(true)) {
1458+
for (int i = 0; i < keys.size(); i++) {
1459+
byte[] ex = existing.get(i);
1460+
long[] newArray;
1461+
if (ex == null) {
1462+
newArray = new long[]{osmId};
1463+
} else {
1464+
long[] old = s2Helper.byteArrayToLongArray(ex);
1465+
boolean found = false;
1466+
for (long id : old) {
1467+
if (id == osmId) {
1468+
found = true;
1469+
break;
1470+
}
1471+
}
1472+
if (found) continue;
1473+
newArray = Arrays.copyOf(old, old.length + 1);
1474+
newArray[old.length] = osmId;
1475+
}
1476+
batch.put(keys.get(i), s2Helper.longArrayToByteArray(newArray));
1477+
}
1478+
gridIndexDb.write(wo, batch);
1479+
}
1480+
}
1481+
}
1482+
}
1483+
13771484
private void withPbfIterator(Path pbfFile, ConsumerWithException<OsmIterator> consumer) throws Exception {
13781485
try (RandomAccessFile file = new RandomAccessFile(pbfFile.toFile(), "r"); FileChannel channel = file.getChannel()) {
13791486

@@ -1463,7 +1570,7 @@ private ExecutorService createExecutorService(int maxThreads) {
14631570
private void cleanupDatabase(Path dbPath) {
14641571
if (Files.exists(dbPath)) {
14651572
try {
1466-
Files.walk(dbPath).sorted((a, b) -> b.compareTo(a)).forEach(path -> {
1573+
Files.walk(dbPath).sorted(Comparator.reverseOrder()).forEach(path -> {
14671574
try {
14681575
Files.delete(path);
14691576
} catch (IOException e) {
@@ -1735,16 +1842,19 @@ private RelRec buildRelRec(OsmRelation r) {
17351842
String code = null;
17361843
for (int i = 0; i < r.getNumberOfTags(); i++) {
17371844
OsmTag t = r.getTag(i);
1738-
if ("admin_level".equals(t.getKey())) {
1739-
try {
1740-
level = Integer.parseInt(t.getValue());
1741-
} catch (NumberFormatException ignore) {
1742-
level = 10;
1845+
switch (t.getKey()) {
1846+
case "admin_level" -> {
1847+
try {
1848+
level = Integer.parseInt(t.getValue());
1849+
} catch (NumberFormatException ignore) {
1850+
level = 10;
1851+
}
1852+
}
1853+
case "name" -> name = t.getValue();
1854+
case "ISO3166-1" -> code = t.getValue();
1855+
case "ISO3166-1:alpha2" -> {
1856+
if (code == null) code = t.getValue();
17431857
}
1744-
} else if ("name".equals(t.getKey())) {
1745-
name = t.getValue();
1746-
} else if ("ISO3166-1".equals(t.getKey())) {
1747-
code = t.getValue();
17481858
}
17491859
}
17501860
RelRec rec = new RelRec();
@@ -1819,11 +1929,11 @@ private PoiIndexRec decodePoiIndexRec(byte[] b) {
18191929
private byte[] encodeRelRec(RelRec r) {
18201930
byte[] nameB = bytes(r.name);
18211931
byte[] codeB = bytes(r.code);
1822-
int cap = 4 // level (was missing!)
1823-
+ 4 + nameB.length // name length + data
1824-
+ 4 + codeB.length // code length + data
1825-
+ 4 + 8 * (r.outer != null ? r.outer.length : 0) // outer count + data
1826-
+ 4 + 8 * (r.inner != null ? r.inner.length : 0); // inner count + data
1932+
int cap = 4
1933+
+ 4 + nameB.length
1934+
+ 4 + codeB.length
1935+
+ 4 + 8 * (r.outer != null ? r.outer.length : 0)
1936+
+ 4 + 8 * (r.inner != null ? r.inner.length : 0);
18271937
ByteBuffer bb = ByteBuffer.allocate(cap);
18281938
bb.putInt(r.level);
18291939
putBytes(bb, nameB);

src/test/java/com/dedicatedcode/paikka/service/ImportServiceTest.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,7 @@
1818

1919
import com.dedicatedcode.paikka.IntegrationTest;
2020
import com.dedicatedcode.paikka.config.PaikkaConfiguration;
21-
import com.dedicatedcode.paikka.flatbuffers.Address;
22-
import com.dedicatedcode.paikka.flatbuffers.Name;
23-
import com.dedicatedcode.paikka.flatbuffers.POI;
24-
import com.dedicatedcode.paikka.flatbuffers.POIList;
21+
import com.dedicatedcode.paikka.flatbuffers.*;
2522
import com.dedicatedcode.paikka.service.importer.GeometrySimplificationService;
2623
import com.dedicatedcode.paikka.service.importer.ImportService;
2724
import org.junit.jupiter.api.AfterEach;
@@ -215,7 +212,7 @@ void testImportPoiHasNamesAndBoundary() throws Exception {
215212
POI poiById = findPoiById(tempDataDir, 432751852);
216213
assertEquals(1, poiById.namesLength(), "POI should have no");
217214
assertEquals("Jardin des Boulingrins", poiById.names(0).text(), "POI should have no");
218-
assertEquals(3, poiById.hierarchyLength());
215+
assertEquals(6, poiById.hierarchyLength());
219216
}
220217

221218
@Test

0 commit comments

Comments
 (0)