feat(geojson): add mvt tiles generation from GeoJSON #2098

styrowolf · 2025-08-28T22:48:02Z

This is a quickly hacked PR to implement MVT tile generation from GeoJSON files (#2054). I used geojson-vt-rs to slice GeoJSON into tiles and integrated the mvt module from geozero for encoding those tiles in MVT format. I needed access to some of the internals from geozero's mvt, which were not public -- that's why I decided to vendor in the module independently.

There are still rough edges and lots of TODOs left on this PR. In my initial testing, the tile endpoint was abnormally slow. I did not yet get around to why that is the case.

I also had to remove #![forbid(unsafe_code)] from the library for an unsafe trait impl in the mvt module. It might be a better idea to just break out that module into a seperate crate.

I would like to get guidance on how to move forward with this PR. Let me know what works and what does not. Thank you for your time.

nyurik · 2025-08-28T23:41:45Z

this is great, thank you! One of us will definitely review it shortly. Being a fairly complex feature, i'm sure there will be some back and forth about it, so please don't be dissuaded if it doesn't get merged right away :)

CommanderStorm

Firstly, this is nice work 👍🏻
Thanks for tackling this!
This is very straightforward code you wrote.

I needed access to some of the internals from geozero's mvt, which were not public

Instead of effectively forking, let's try to just make the fields public that we need.
For an experimental impl, current version is fine.
For getting merged, I would like to see a PR to make the values we need pub in geozero.

In my initial testing, the tile endpoint was abnormally slow. I did not yet get around to why that is the case.

Just throwing it out there: did you run it in release mode?. This burnt me once 😉

To benchmark things, we have the following two functionalities. If you think performance could be an issue, benchmarking it is probably a good idea.

martin/justfile

Lines 41 to 59 in 872a0e7

    
           # Run benchmark tests 
        
           bench: 
        
               cargo bench --bench bench 
        
               open target/criterion/report/index.html 
        
           # Run HTTP requests benchmark using OHA tool. Use with `just bench-server` 
        
           bench-http:  (cargo-install 'oha') 
        
               @echo "ATTENTION: Make sure Martin was started with    just bench-server" 
        
               @echo "Warming up..." 
        
               oha --latency-correction -z 5s --no-tui http://localhost:3000/function_zxy_query/18/235085/122323 > /dev/null 
        
               oha --latency-correction -z 60s         http://localhost:3000/function_zxy_query/18/235085/122323 
        
               oha --latency-correction -z 5s --no-tui http://localhost:3000/png/0/0/0 > /dev/null 
        
               oha --latency-correction -z 60s         http://localhost:3000/png/0/0/0 
        
               oha --latency-correction -z 5s --no-tui http://localhost:3000/stamen_toner__raster_CC-BY-ODbL_z3/0/0/0 > /dev/null 
        
               oha --latency-correction -z 60s         http://localhost:3000/stamen_toner__raster_CC-BY-ODbL_z3/0/0/0 
        
           # Start release-compiled Martin server and a test database 
        
           bench-server: start 
        
               cargo run --release -- tests/fixtures/mbtiles tests/fixtures/pmtiles

I would like to get guidance on how to move forward with this PR.
Let me know what works and what does not.

There are three larger aspects:

Testing. We try to test what we can. This prevents unwanted surprises later on and makes working generally more pleasant. Lets

add integration tests in test.sh
- add a minimal GeoJSON fixture with a polygons, points, and lines (each with one over tile border, one in tile border)
- test if it does the splitting at tile borders works as expected
- if the options (see inline comment) are correctly passed.
add unit tests for that
- file.geojson CLI argument parsing works as expected
- the config file is serialised as expected
- since there is not much "actual code" (= code which can have bugs without the dependency having a bug), I don't think we need much in terms of unit testing

Upstreaming the geozero-pub change necessary.
Vendoring (=forking) is a last resort, and we should not do this, unless as a last resort.
You can change where a dependency points to via git = "..", branch = "..".

Adjusting the existing documentation (README and /docs, for the individual source and the config file respectively)
=> we should tell people about the capability you are adding 😉

martin/src/geojson/mod.rs

martin/build.rs

CommanderStorm · 2025-08-29T00:23:01Z

martin/src/geojson/mod.rs

+            .map_err(|e| FileError::InvalidFilePath(path.clone()))?;
+
+        // TODO: see if options default is goood enough
+        let inner = GeoJSONVT::from_geojson(&geojson, &Options::default());


There are a few options which might be nice to have not hard-coded => added to GeoJsonConfig and passed to GeoJsonSource::new.
We can just expose these options in the config file.

I am not sure if index_max_points works like our existing option, or what that index is.

Options { max_zoom: 18, // max zoom to preserve detail on; can't be higher than 24 index_max_zoom: 5, // max zoom in the tile index index_max_points: 100000, // max number of points per tile in the tile index generate_id: false, // whether to generate feature ids, overriding existing ids tile: TileOptions { tolerance: 3., // simplification tolerance (higher means simpler) extent: 4096, // tile extent buffer: 64, // tile buffer on each side line_metrics: false, // enable line metrics tracking for LineString/MultiLineString features } }

More a personal interest:
I would be interested if this changes the performance of the code as the compiler now can no longer constant-fold things.

CommanderStorm · 2025-08-29T00:39:15Z

martin/src/geojson/mod.rs

+        let tilejson = tilejson! {
+            tiles: vec![],
+            minzoom: 0,
+            maxzoom: 18,
+        };


This looks a bit sparse. I am not sure if this tilejson works, as it does not specify what is contained features.. 🤔

I looked at the cog module's one and took that as an example. That's just like this let tilejson = tilejson! { tiles: vec![], minzoom: min_zoom, maxzoom: max_zoom };

I'll look into this.

Added the required vector_layers field as per https://github.com/mapbox/tilejson-spec/blob/master/3.0.0/README.md#332-fields. I viewed it in Maputnik editor and it seems to be working. We should probably add bbox etc. some time.

martin/src/geojson/mod.rs

martin/Cargo.toml

styrowolf · 2025-08-29T07:25:37Z

Thanks for the quick review. I've taken out mvt module code and replaced it with geozero with its internals exposed (I put the dependency from my fork for now to restructure the code, I'll put the PR in and I'll replace the dep when the changes are upstream).

GeoJSON

styrowolf · 2025-08-29T10:31:26Z

@CommanderStorm I've replaced the GeoJSONVT struct with the function. geojson_to_tile reprojects the entire dataset on every call, which might be a significant overhead. Maybe we could do a PR to that crate so that we can project once and keep that as state in GeoJsonSource.

CommanderStorm · 2025-08-29T11:09:24Z

Yea, reprojecting on every tile would not be great for performance

styrowolf · 2025-08-29T21:33:53Z

Current TODOs for this PR

Expose geojson_vt_rs::TileOptions to GeoJsonConfig
upstream and use MVT feature writing in geozero partial reimplementation of MvtWriter to support feature writing and outputting layers georust/geozero#264
Return info for more TileJSON fields (especially bounds)
Reproject once, tile multiple times feat: preprocess GeoJSON for faster tiling maxammann/geojson-vt-rs#3
Add GeoJSON error type and fix error handling
Add GeoJSON fixtures
Unit tests
Integration tests
Benchmarking

CommanderStorm · 2025-08-29T21:59:47Z

there are multiple ways of accomplishing this

I would prefer option 1 of using geozero to do the feature writing, if this is okay with you.
Having this code live here does seem fairly hacky.
This code living in geozero has ecosystem advantages for other people too 😉

I would prefer geozero over mvt, but I have not looked into this. That is mostly based around size of the community and "trust"/having talked with the people running it.

I know that this is a big ask.
If you want me to take over some part (besides encoding, you have dug into this already ^^), feel free to task me away.
I currently have semester breaks.

styrowolf · 2025-08-29T22:21:48Z

I would prefer geozero over mvt, but I have not looked into this. That is mostly based around size of the community and "trust".

I fully understand and agree with your point, though mvt also seems like a pretty popular crate with around 74K downloads -- the file headers also indicate Copyright (c) 2019–2024 Minnesota Department of Transportation.

I would prefer option 1 of using geozero to do the feature writing, if this is okay with you. Having this code live here does seem fairly hacky. This code living in geozero has ecosystem advantages for other people too 😉

I agree with you on the ecosystem advantages part. I've briefly looked over some geozero code to see how to approach this. I'm currently thinking of implementing something like a MvtLayerWriter which implements FeatureProcessor.

If you want me to take over some part (besides encoding, you have dug into this already ^^), feel free to task me away.
I currently have semester breaks.

Thank you so much! I really appreciate the help. I’ll stay in touch as I put together a basic plan to move forward.

CommanderStorm · 2025-08-29T23:30:22Z

Since writing docs is something that takes iterations, I thought you might appreciate me doing the legwork and writing up a first rough draft to iterate on later => c3b1728

for more information, see https://pre-commit.ci

styrowolf · 2025-09-02T11:06:16Z

I'm currently waiting on two PRs: geozero MVT feature-writing georust/geozero#264 and geojson-vt-rs preprocessed geojson maxammann/geojson-vt-rs#3. I will update this PR as those PRs move forward.

@nyurik

…outputting layers (#264) - [x] I agree to follow the project's [code of conduct](https://github.com/georust/.github/blob/main/CODE_OF_CONDUCT.md). - [x] I added an entry to the project's change log file if knowledge of this change could be valuable to users. - Usually called `CHANGES.md` or `CHANGELOG.md` - Prefix changelog entries for breaking changes with "BREAKING: " --- This is an implementation of feature writing for MVTs, as discussed in #263. This PR builds upon @nyurik's previous WIP reimplementation #181. It passes all of the `mvt_writer` tests. I am not sure if I understood the API exactly correctly but this seems to be a pretty good starting point, as individual feature/geom can be converted to a MVT feature with the `ToMvt` trait as well as `MvtWriter` can process multiple feature and output them to a layer. [Here is an example](https://github.com/maplibre/martin/blob/20ff5e36cc89bb07aefaf39a6f513df6b65a38cc/martin/src/geojson/mod.rs#L124) of feature writing and layer output with MvtWriter (from GeoJSON to MVT tiles PR, maplibre/martin#2098) Let me know how we can move forward with this PR. --------- Co-authored-by: Yuri Astrakhan <[email protected]>

styrowolf · 2025-09-12T22:02:32Z

Both of those PRs got merged, so I'll try to continue work on this again.

CommanderStorm · 2025-09-15T16:55:46Z

We have done a bit of light refactoring to support the v1 release via martin-core.
I have merged the changes into your PR, a few files have changed where they are located. Nothing big.

styrowolf · 2025-09-16T22:27:07Z

Thanks Frank. I just pushed a few (rough) commits to show how the code is with the new PRs. I still have to test, add fixtures, and improve error handling. The function naming is a bit rough too. Looking forward to what you think.

CommanderStorm · 2025-10-22T01:58:43Z

I rebased it again as our changes (should now be mostly done) in main were kind of touching this code.

I and @nyurik discussed this offline and came to the conclusion that

vendoring geozero, or
your original version are good.

The current path that we all hoped/assumed we can get geozero to make a release via internal chanels is apparently more a longer term plan 😞

We can also rename the feature geojson-unstable (default-disabled) and merge the done parts (f.ex. configuration) to reduce the PRs' scope.

styrowolf · 2025-11-22T01:35:31Z

My academic term has finished, so I have more time to work on this PR. I merged the main branch and did some refactoring. Let me know how can I continue, or whether we should shelve the PR.

initial geojson support

7a32f08

CommanderStorm requested changes Aug 29, 2025

View reviewed changes

styrowolf added 4 commits August 29, 2025 08:59

switch mvt module internals with forked geozero dependency

b875bc6

run just fmt-toml, a few edits

950469a

forbid unsafe code again

7e55e3a

clean up build.rs

ea69dc4

styrowolf mentioned this pull request Aug 29, 2025

mvt: expose Command, CommandInteger, ParameterInteger from the module georust/geozero#263

Closed

2 tasks

fill in vector_layer field in TileJSON, use geojson_to_tile for tiling

34cd961

GeoJSON

concurrent scraping is better

6a5a1dd

CommanderStorm changed the title ~~Generate MVT tiles from GeoJSON (#2054)~~ feat(goejson): add mvt tiles generation from GeoJSON Aug 29, 2025

styrowolf changed the title ~~feat(goejson): add mvt tiles generation from GeoJSON~~ feat(geojson): add mvt tiles generation from GeoJSON Aug 29, 2025

Merge branch 'main' into feat-geojson

5ed3ff5

styrowolf added 3 commits August 30, 2025 11:08

expose geojson_vt_rs::TileOptions to GeoJsonConfig

f75cc02

Merge remote-tracking branch 'styro/feat-geojson' into feat-geojson

72af52c

implement TileJSON bounds for GeoJsonSource

c0432d3

styrowolf mentioned this pull request Aug 30, 2025

feat: preprocess GeoJSON for faster tiling maxammann/geojson-vt-rs#3

Merged

CommanderStorm and others added 3 commits August 31, 2025 23:21

Merge branch 'main' into feat-geojson

0cc4af9

add a first draft for documentation

825a2ca

[pre-commit.ci] auto fixes from pre-commit.com hooks

5b7cee4

for more information, see https://pre-commit.ci

styrowolf mentioned this pull request Sep 1, 2025

partial reimplementation of MvtWriter to support feature writing and outputting layers georust/geozero#264

Merged

2 tasks

CommanderStorm added 4 commits September 15, 2025 18:40

Merge branch 'main' into feat-geojson

78ddfad

fix merge issue

dddfae9

add doc comments

ede6af6

fix more merge isssues

11c0bce

styrowolf added 2 commits September 17, 2025 00:14

use PreprocessedGeoJSON to generate tiles

62a4096

use new geozero version to write MVTs

b182c6f

Merge branch 'main' into feat-geojson

0397a4c

styrowolf and others added 4 commits November 22, 2025 00:07

refactor utility functions into seperate module

5e3028c

Merge remote-tracking branch 'origin/main' into feat-geojson

4563f6b

fix formatting

4306b1a

chore(fmt): apply pre-commit formatting fixes

6a0bdc9

	# Run benchmark tests
	bench:
	cargo bench --bench bench
	open target/criterion/report/index.html

	# Run HTTP requests benchmark using OHA tool. Use with `just bench-server`
	bench-http: (cargo-install 'oha')
	@echo "ATTENTION: Make sure Martin was started with just bench-server"
	@echo "Warming up..."
	oha --latency-correction -z 5s --no-tui http://localhost:3000/function_zxy_query/18/235085/122323 > /dev/null
	oha --latency-correction -z 60s http://localhost:3000/function_zxy_query/18/235085/122323
	oha --latency-correction -z 5s --no-tui http://localhost:3000/png/0/0/0 > /dev/null
	oha --latency-correction -z 60s http://localhost:3000/png/0/0/0
	oha --latency-correction -z 5s --no-tui http://localhost:3000/stamen_toner__raster_CC-BY-ODbL_z3/0/0/0 > /dev/null
	oha --latency-correction -z 60s http://localhost:3000/stamen_toner__raster_CC-BY-ODbL_z3/0/0/0

	# Start release-compiled Martin server and a test database
	bench-server: start
	cargo run --release -- tests/fixtures/mbtiles tests/fixtures/pmtiles

Uh oh!

feat(geojson): add mvt tiles generation from GeoJSON #2098

Are you sure you want to change the base?

feat(geojson): add mvt tiles generation from GeoJSON #2098

Uh oh!

Conversation

styrowolf commented Aug 28, 2025

Uh oh!

nyurik commented Aug 28, 2025

Uh oh!

CommanderStorm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CommanderStorm Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

CommanderStorm Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

styrowolf Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

styrowolf Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

styrowolf commented Aug 29, 2025

Uh oh!

styrowolf commented Aug 29, 2025

Uh oh!

CommanderStorm commented Aug 29, 2025

Uh oh!

styrowolf commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CommanderStorm commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

styrowolf commented Aug 29, 2025

Uh oh!

CommanderStorm commented Aug 29, 2025

Uh oh!

styrowolf commented Sep 2, 2025

Uh oh!

styrowolf commented Sep 12, 2025

Uh oh!

CommanderStorm commented Sep 15, 2025

Uh oh!

styrowolf commented Sep 16, 2025

Uh oh!

CommanderStorm commented Oct 22, 2025

Uh oh!

styrowolf commented Nov 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

styrowolf commented Aug 29, 2025 •

edited

Loading

CommanderStorm commented Aug 29, 2025 •

edited

Loading