Use rtree spatial index from PlanetilerConfig bounds by default for Geopackages #635
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Kudos, SonarCloud Quality Gate passed! |
Full logs: https://github.com/onthegomap/planetiler/actions/runs/13371218015 |
erik
reviewed
Jul 27, 2023
planetiler-core/src/main/java/com/onthegomap/planetiler/reader/GeoPackageReader.java
Outdated
Show resolved
Hide resolved
bdon
force-pushed
the
bdon/gpkg-indexing
branch
from
c8d3fee to
0b2c894
Compare
|
If I use the 4326 land/water polygons this drops the protomaps basemap build from ~30 seconds to 10 seconds for Monaco. Once we can port Natural Earth to use GPKG it should be even faster. |
msbarry
reviewed
Feb 13, 2025
|
|
||
| Iterable<FeatureRow> results; | ||
|
|
||
| if (this.bounds != null && indexer.isIndexed() && srsId == 4326) { |
There was a problem hiding this comment.
Another option is to transform the bounds from 4326 to whatever CRS the geopackage source uses. See ShapefileReader as an example
There was a problem hiding this comment.
I wasn't sure if that handled non-conformal transformations correctly, but if so we can use that. How were you able to use the Shapefile filter without .sbx sidecar files?
There was a problem hiding this comment.
Geotools ReferencedEnvelope#transform drops a bunch of points along the edge of the bounding box in the old coordinate system, then computes the bounding box of those points in the new coordinate system. Also if it throws a TransformException then it just skips the filter.
I'm not 100% sure how the geotools filter predicate works internally. My guess/hope would be that it uses the spatial index if available, but then falls back to just applying it against each shape if not. I added it based off the description here: https://docs.geotools.org/latest/userguide/library/main/filter.html
planetiler-core/src/main/java/com/onthegomap/planetiler/reader/GeoPackageReader.java
Outdated
Show resolved
Hide resolved
Comment on lines
91
to
97
| 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(expectedCount, points.size(), id); |
There was a problem hiding this comment.
Do we need WorkerPipeline here? I think we could potentially simplify this to just:
reader.readFeatures(feature -> {
points.add(elem.latLonGeometry());
});
|
|
Thanks! This looks good to me. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a first attempt at addressing #478 .
The existing issue is that all Shapefile or Geopackage features are read by every run, even if the
--boundsargument is passed to limit the tile output or the bounds are determined implicitly based on theosm.pbf. To reduce the runtime we can:.fbn,.fbxand/orqixsidecars depending on what software created the Shapefile. Maybe GeoTools can read all of these, but I'm not familiar with its internals.rtree_indexes (A vanilla SQLite feature, no Spatialite required)ne_50m_admin_0_boundary_lines_landas tables we care about. IMO, this doesn't reduce the runtime by much relative to spatial indexing, so it may be an unworthy optimization.My proposal here is to not change any APIs, but change the default behavior of Geopackage to use the determined
PlanetilerConfigbounds and apply the spatial index. Remaining todos:rtree_indexes will always exist on gpkg or are optionalAnother relevant format would be FlatGeobuf, which has an optional spatial index as part of the single file, but I don't see key advantages of FGB over GPKG in most Planetiler use cases where data lives on disk.