-
-
Notifications
You must be signed in to change notification settings - Fork 406
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.
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
Add PMTiles support #938
base: main
Are you sure you want to change the base?
Add PMTiles support #938
Conversation
Thanks for taking the time to open this PR. |
I saw your comment there, but comparing PMTiles with MBTiles didn't make sense for me. PMTiles can be used as local and remote datasource like mvt files when using an HTTP server with Range Request support. |
But you still need a http server with range support... From my point if view using a tile server or worker is not a big lift. |
There are not many HTTP servers without Range Request support ( Switching the tile source in a style created with Maputnik is a one-liner like |
I could argue that you won't need to run the above script at all and using Martin or tileserver-gl (using docker, or even without) is not that complicated... |
@pka It might be helpful if you could write your full use-case in #807. Where is the pmtiles file you want to reference? Is it on your local machine or hosted somewhere? For background I also use maputnik + PMTiles but with a separate tool that runs both. If we can all share our use-cases, that could help reveal pain points and potential solutions. |
@nyurik @lseelenbinder Could you maybe help formulating use cases for styling maps with PMTiles storage? |
here is the necessary changes to add PMTiles for vector sources to the UI: This works fine with remote sources e.g. you can paste in However, the "inspect" panel doesn't work yet because it relies on MapLibre internals to fetch TileJSON. The inspect panel will work once protomaps/PMTiles#247 is implemented. (EDIT: I implemented 247 above, so inspect works now.) |
a11dc3e
to
d3d4c99
Compare
There is a UI issue right now where the |
hello what's needed to get this forward? I would really like to use this since my vector tiles are stored in the pmtiles format, which is simpler for me to deploy than mbtiles as well as even being smaller in size. Hosting a special-made server that can extract and serve the tiles (either trying to obtain replacement mbtiles or using Right now my setup with pmtiles looks like this: in a folder with e.g.
to have a compatibility for the tiles JSON, as well as
to get a local reverse proxy that adds HTTPS with a self-signed and trusted certificate. Then I can use If this PR was merged, I wouldn't need And in the future this might even allow me to just select a pmtiles file from disk and without needing to upload it anywhere or host any server, it would just read from it in-memory. (e.g. using a HTML file picker and then using Blob.slice instead of range requests) |
I agree this should be added to maputnik. At minimum, we need to rebase this PR to be able to merge it. i don't see any substantial blockers to be honest, as it is indeed a highly useful mode of operation, and we shouldn't require users to host their own tiles anywhere if they have a pmtiles on some s3 or another range-supporting hosting |
Still contains old bugs Co-authored-by: Brandon Liu <[email protected]> Co-authored-by: Pirmin Kalberer <[email protected]>
* add PMTiles mode to source editing dialog * App.tsx detects PMTiles sources for fetching vector_layers (inspect does not yet work)
@WebFreak001 thanks for fixing the bug in the modal data sources editor. I've added your commits also to this PR, to keep the full history. |
As mentioned in the other PR, It would be better to have a test to cover this scenario/use case/flow. |
We could test e.g. diff --git a/cypress/e2e/modals.cy.ts b/cypress/e2e/modals.cy.ts
index aade75f..5eeb0c6 100644
--- a/cypress/e2e/modals.cy.ts
+++ b/cypress/e2e/modals.cy.ts
@@ -83,6 +83,20 @@ describe("modals", () => {
});
});
+ it("add new pmtiles source", () => {
+ let sourceId = "pmtilestest";
+ when.setValue("modal:sources.add.source_id", sourceId);
+ when.select("modal:sources.add.source_type", "pmtiles_vector");
+ when.setValue("modal:sources.add.source_url", "https://pmtiles.io/stamen_toner(raster)CC-BY+ODbL_z3.pmtiles");
+ when.click("modal:sources.add.add_source");
+ when.wait(200);
+ then(
+ get.styleFromLocalStorage().then((style) => style.sources[sourceId])
+ ).shouldInclude({
+ url: "pmtiles://https://pmtiles.io/stamen_toner(raster)CC-BY+ODbL_z3.pmtiles",
+ });
+ });
+
it("add new raster source", () => {
let sourceId = "rastertest";
when.setValue("modal:sources.add.source_id", sourceId); Is there a way to address the source_url input field? |
get.styleFromLocalStorage().then((style) => style.sources[sourceId]) | ||
).shouldInclude({ | ||
type: "vector", | ||
// url: "pmtiles://https://data.source.coop/protomaps/openstreetmap/v4.pmtiles", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like some assertion is missing here to properly test this...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently the onChange
trigger code is not executed in Cypress, so url
can't be tested for a new value. Someone more knowledgeable about Cypress maybe knows a workaround?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CC: @ShellyDCMS, any chance you can take a look at this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@HarelM , checking
Add support for pmtiles sources. Solves #807
There is still an error, probably when adding the source to the Maputnik sources:
@bdon How did you solve this for https://editor.protomaps.com/ ?