-
-
Notifications
You must be signed in to change notification settings - Fork 93
Add encoding field to vector sources #1251
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1251 +/- ##
=======================================
Coverage 95.37% 95.37%
=======================================
Files 114 114
Lines 7398 7398
Branches 2327 2327
=======================================
Hits 7056 7056
Misses 342 342 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thanks for taking the time to open this PR! |
nyurik
left a comment
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.
Encoding focuses on "format", not the organization that produced it. mlt, mvt, ...
louwers
left a comment
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.
I prefer using mvt and mlt instead of mapbox and maplibre.
It is not unthinkable that either Mapbox or MapLibre releases another vector tile format in the future. Then these names would be very confusing. I already think mapbox is quite confusing because I personally don't associate MVTs with Mapbox even though they invented the format.
|
This looks good in general. |
|
@HarelM Let's merge? |
|
We need a new release of the spec package to update the docs, right? |
|
No need, looks like it is live. https://maplibre.org/maplibre-style-spec/sources/#encoding |
|
Nice! Wasn't sure if docs are updated only on release or every merge to main. DevOps is awesome 😎 |
|
I am probably a bit late here, but I was looking at the MLT maplibre-js PR to get an idea of how I would implement it in tileserver-gl and was wondering about this choice to use 'encloding' instead of 'format' for the type of tile in MLT. it seems like other tile types use 'format' metadata to distinguish between what is in the tile . "encoding" seems like we only use in terrain right now, and for example in tileserver-gl it is set to either mapbox or terrarium right now. This being a vector source it is probably not much of an issue, but it seems different from the convention we have been using. |
|
It's just for consistency with the other source definition. |
Adds an optional
encodingfield to vector sources, based on the one in raster sources, to allow the use of MapLibre Tiles.Implementation issues:
Resolves #1252
I don't think this has any effect on APIs, can someone confirm?
Launch Checklist
Include before/after visuals or gifs if this PR includes visual changes.Post benchmark scores.CHANGELOG.mdunder the## mainsection.