protomaps / go-pmtiles

Single-file executable tool for working with PMTiles archives
https://docs.protomaps.com/pmtiles/cli
BSD 3-Clause "New" or "Revised" License
373 stars 51 forks source link

Update convert to support martin mbtiles #106

Closed crb04c closed 9 months ago

crb04c commented 10 months ago

Martin generates mbtiles with a format of MVT and does not supply a compression metadata field

bdon commented 10 months ago

This won't be forward compatible if Martin adds the option of Brotli compression, or no compression (like tippecanoe does with --no-tile-compression. Since Martin is actively developed can we instead add the compression metadata field to Martin?

crb04c commented 10 months ago

Yea that would probably be preferred, and if the format of mvt is not officially supported in MBTiles- changing martin to produce a format of pbf as well. We can abandon this PR and I'll just use my fork in the meantime. Thanks.

nyurik commented 10 months ago

@crb04c note that I just modified martin-cp to set format=pbf instead of format=mvt. This makes it more inline with other tools like QGIS that weren't able to read it otherwise. Would that cover this issue?

crb04c commented 10 months ago

@nyurik It will handle everything but compression-

@bdon however according to the spec for MBTiles I'd say it makes sense to default to gzip if none is provided pbf as a format refers to gzip-compressed vector tile data in [Mapbox Vector Tile](https://github.com/mapbox/vector-tile-spec/) format.

https://github.com/mapbox/mbtiles-spec/blob/master/1.3/spec.md

nyurik commented 10 months ago

@crb04c I am not sure i understood. My thought was to keep as close to the original as possible "by default" - i.e. format=pbf would mean gzip encoding, and for other formats like png to use identity encoding by default.

Going forward, we should support additional modes like encoding=identity or encoding=br to do non-compressed or brotli-encoded compression (i am following the HTTP Encoding header here to avoid creating a new standard) -- to override the default behavior of the legacy mbtiles implementations.

bdon commented 10 months ago

@crb04c yes, good observation, let's default to assuming that pbf is gzipped per spec 1.3

What error were you running into that needs this PR? The finalize function will already set the header: https://github.com/protomaps/go-pmtiles/blob/main/pmtiles/convert.go#L412

crb04c commented 10 months ago

@bdon This is in relation to the command line conversion of mbtiles- before the custom build I had to add a step to edit the header of the pmtiles to set compression for MVT/PBF by default.

bdon commented 10 months ago

@crb04c can you provide the Martin-created mbtiles archive that failed to convert?

crb04c commented 10 months ago

example.zip Here is an example of a an mbtiles generated by martin

Output PMTiles image

Output PMTiles with changes image

nyurik commented 10 months ago

Please include the command you used to generate it, and the version of the tool. Thanks!

crb04c commented 10 months ago

Martin v0.11.4

.\martin-cp.exe postgresql://postgres:****@localhost:5432/postgis_test --output-file example.mbtiles --min-zoom 0 --max-zoom 10 --source "PointTest"

bdon commented 10 months ago

This should be behave correctly as-is if Martin uses format=pbf instead of format=mvt. @crb04c Can you verify with the latest version of Martin?

bdon commented 9 months ago

Please reopen this PR if it doesn't work with the latest version of Martin.