Skip to content
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

Use rtree spatial index from PlanetilerConfig bounds by default for Geopackages #635

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Use rtree spatial index from PlanetilerConfig bounds by default for Geopackages #635

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions planetiler-core/src/main/java/com/onthegomap/planetiler/Planetiler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ public Planetiler addShapefileSource(String projection, String name, Path defaul
* @see Downloader
*/
public Planetiler addGeoPackageSource(String projection, String name, Path defaultPath, String defaultUrl) {

Path path = getPath(name, "geopackage", defaultPath, defaultUrl);
boolean keepUnzipped = getKeepUnzipped(name);
return addStage(name, "Process features in " + path,
Expand Down
30 changes: 27 additions & 3 deletions planetiler-core/src/main/java/com/onthegomap/planetiler/reader/GeoPackageReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,20 @@
import java.util.HashMap;
import java.util.List;
import java.util.function.Consumer;
import mil.nga.geopackage.BoundingBox;
import mil.nga.geopackage.GeoPackage;
import mil.nga.geopackage.GeoPackageManager;
import mil.nga.geopackage.features.index.FeatureIndexManager;
import mil.nga.geopackage.features.index.FeatureIndexResults;
import mil.nga.geopackage.features.index.FeatureIndexType;
import mil.nga.geopackage.features.user.FeatureColumns;
import mil.nga.geopackage.features.user.FeatureDao;
import mil.nga.geopackage.features.user.FeatureRow;
import mil.nga.geopackage.geom.GeoPackageGeometryData;
import org.geotools.geometry.jts.JTS;
import org.geotools.geometry.jts.WKBReader;
import org.geotools.referencing.CRS;
import org.locationtech.jts.geom.Envelope;
import org.locationtech.jts.geom.Geometry;
import org.opengis.referencing.FactoryException;
import org.opengis.referencing.operation.MathTransform;
Expand All @@ -39,9 +45,14 @@ public class GeoPackageReader extends SimpleReader<SimpleFeature> {
private final GeoPackage geoPackage;
private final MathTransform coordinateTransform;

GeoPackageReader(String sourceProjection, String sourceName, Path input, Path tmpDir, boolean keepUnzipped) {
private final Envelope bounds;

GeoPackageReader(String sourceProjection, String sourceName, Path input, Path tmpDir, boolean keepUnzipped,
Envelope bounds) {

super(sourceName);
this.keepUnzipped = keepUnzipped;
this.bounds = bounds;

if (sourceProjection != null) {
try {
Expand Down Expand Up @@ -105,7 +116,7 @@ public static void process(String sourceProjection, String sourceName, List<Path
SourceFeatureProcessor.processFiles(
sourceName,
sourcePaths,
path -> new GeoPackageReader(sourceProjection, sourceName, path, tmpDir, keepUnzipped),
path -> new GeoPackageReader(sourceProjection, sourceName, path, tmpDir, keepUnzipped, config.bounds().latLon()),
writer, config, profile, stats
);
}
Expand Down Expand Up @@ -138,7 +149,20 @@ public void readFeatures(Consumer<SimpleFeature> next) throws Exception {
MathTransform transform = (coordinateTransform != null) ? coordinateTransform :
CRS.findMathTransform(CRS.decode("EPSG:" + srsId), latLonCRS);

for (var feature : features.queryForAll()) {
FeatureIndexManager indexer = new FeatureIndexManager(geoPackage,
features);
indexer.setIndexLocation(FeatureIndexType.RTREE);
Comment on lines +152 to +154
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's been a while, but if I remember correctly, this will actually build the index if it doesn't already exist in the DB before running the query.

Looking at the PR where I initially added this, should be possible to check for the presence of the index before trying to use it: #413 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems awkward and unexpected behavior to modify the database in-place, can we disable that? If it's being unzipped every time it could be slow for a large geopackage.

Maybe detect the absence of the index and ignore?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nvm, I see you mentioned exactly that behavior I suggested) so the next step is addressing possible projection mismatches; we need to find some geopackages in the wild that are neither WGS84 or webmerc


BoundingBox boundingBox = BoundingBox.worldWGS84();

if (this.bounds != null) {
var l = this.bounds;

boundingBox = new BoundingBox(l.getMinX(), l.getMinY(), l.getMaxX(), l.getMaxY());
}
FeatureIndexResults indexResults = indexer.query(boundingBox);

for (FeatureRow feature : indexResults) {
GeoPackageGeometryData geometryData = feature.getGeometry();
byte[] wkb;
if (geometryData == null || (wkb = geometryData.getWkb()).length == 0) {
Expand Down
39 changes: 37 additions & 2 deletions planetiler-core/src/test/java/com/onthegomap/planetiler/reader/GeoPackageReaderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.junit.jupiter.api.io.TempDir;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.locationtech.jts.geom.Envelope;
import org.locationtech.jts.geom.Geometry;

class GeoPackageReaderTest {
Expand All @@ -37,7 +38,7 @@ void testReadGeoPackage(boolean keepUnzipped) throws IOException {
for (var path : List.of(pathOutsideZip, pathInZip)) {
for (var proj : projections) {
try (
var reader = new GeoPackageReader(proj, "test", path, tmpDir, keepUnzipped)
var reader = new GeoPackageReader(proj, "test", path, tmpDir, keepUnzipped, null)
) {
for (int iter = 0; iter < 2; iter++) {
String id = "path=" + path + " proj=" + proj + " iter=" + iter;
Expand Down Expand Up @@ -66,13 +67,47 @@ void testReadGeoPackage(boolean keepUnzipped) throws IOException {
}
}


@Test
@Timeout(30)
void testReadGeoPackageSpatialIndex() throws IOException {
Path pathOutsideZip = TestUtils.pathToResource("geopackage.gpkg");
Path zipPath = TestUtils.pathToResource("geopackage.gpkg.zip");
Path pathInZip = FileUtils.walkPathWithPattern(zipPath, "*.gpkg").get(0);

var projections = new String[]{null, "EPSG:4326"};

for (var path : List.of(pathOutsideZip, pathInZip)) {
for (var proj : projections) {
try (
var reader =
new GeoPackageReader(proj, "test", path, tmpDir, false, new Envelope(-77.0306, -77.0192, 38.8894, 38.9014))
) {
for (int iter = 0; iter < 2; iter++) {
String id = "path=" + path + " proj=" + proj + " iter=" + iter;
assertEquals(86, reader.getFeatureCount(), id);
List<Geometry> points = new ArrayList<>();
List<String> names = new ArrayList<>();
WorkerPipeline.start("test", Stats.inMemory())
.fromGenerator("geopackage", reader::readFeatures, 1)
.addBuffer("reader_queue", 100, 1)
.sinkToConsumer("counter", 1, elem -> {
points.add(elem.latLonGeometry());
}).await();
assertEquals(4, points.size(), id);
}
}
}
}
}

@Test
@Timeout(30)
void testReadEmptyGeoPackage() throws IOException {
Path path = TestUtils.pathToResource("empty-geom.gpkg");

try (
var reader = new GeoPackageReader(null, "test", path, tmpDir, false)
var reader = new GeoPackageReader(null, "test", path, tmpDir, false, null)
) {
for (int iter = 0; iter < 2; iter++) {
String id = "iter=" + iter;
Expand Down