-
Notifications
You must be signed in to change notification settings - Fork 659
Rasters: Add tileSize
to TileJSON
#1559
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
Rasters: Add tileSize
to TileJSON
#1559
Conversation
src/serve_rendered.js
Outdated
@@ -1005,6 +1005,7 @@ export const serve_rendered = { | |||
); | |||
} | |||
const info = clone(item.tileJSON); | |||
info.tileSize = (tileSize != undefined ? tileSize : 256) |
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.
cf. Line 997 where tileSize
is set
const tileSize = parseInt(req.params.tileSize, 10) || undefined
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.
ya, I made this default to undefined for a reason. When I added 512 tiles and added tileSize, I wanted to preserve the old behavior when the user went to the old /style/id.json endpoint urls (which returned a 256px tile before). When it is undefined, getTileUrls returns a tile url that does not have a tilesize included (like it did before). When tileSize is defined, the size gets included in the tiles url.
I am not really concerned with adding tileSize to our tilejson. The spec seems to say as long as the required fields are included that the tilejson is valid. the client is supposed to ignore the unknow key values, but obviously some clients are able to use this already (like maplibre) and just having that information would be helpful (since if it doesn't read it directly they may need to know the size so they can set it themeselfs) |
can you run |
@@ -1005,7 +1005,7 @@ export const serve_rendered = { | |||
); | |||
} | |||
const info = clone(item.tileJSON); | |||
info.tileSize = (tileSize != undefined ? tileSize : 256); | |||
info.tileSize = tileSize != undefined ? tileSize : 256; |
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.
Thanks to @acalcutt for catching & running Lint for me. I had been doing development via Docker
Add
tileSize
to TileJSON Output for Raster endpointsi.
tileSize
i. There is discussion at tilejson-spec#26.
ii. The Sources spec for Mapbox & MapLibre make use of
tileSize
.Tests
Using the default Zurich tile set with a basic preview. Paths are mentioned in the docs at Rendered Tiles.
http://localhost:8080/styles/basic-preview.json
http://localhost:8080/styles/256/basic-preview.json
http://localhost:8080/styles/512/basic-preview.json
Note: the path to the TileJSON does not exactly match the pattern for the path the PNG, but those are the verified paths.
Note on TileJSON
2.0.0
v. TileJSON3.0.0
++2.0.0
#2. File format discusses how to deal with extra keys: "implementations MUST expose unknown key/values in their API so that API users can optionally handle these keys"