-
Notifications
You must be signed in to change notification settings - Fork 240
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
Reduce differences between official OpenMapTiles tiles and Tilemaker implementation #686
Conversation
They are already documented but have been missing in the implementation.
Minzoom for railway features depends on railway=*, service=* and usage=*. Only railway features with specific railway=* values are added to the vector tiles. This means, railway=construction/disused/… and railway=fancy_value are ignored. The OpenMapTiles schema defines a subclass field for railways as well. This commit adds this field.
It makes vector tiles smaller if we do not add railway features without names to the vector tiles.
* Drop layers transportation_name_mid and transportation_name_detail, use MinZoom() instead. * Do not write roads without names or ref into the vector tiles.
* Set correct minzoom values for all roads in the transportation and transportation_name layer. * Add highway=raceway * Align handling of roads under construction to OpenMapTiles.
* Do not write polygons other than infrastructure for pedestrians and piers. * Set minzoom according to area size and absolute minzoom.
* Add man_made=bridge to layer transportation. * Set brunnel attribute for all polygon features on that layer.
else | ||
MinZoom(9) | ||
end | ||
local class = railwayClasses[railway] |
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 noticed that railway=proposed also ends up in the tiles. Am I right in thinking that this now takes care of it?
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.
Neither the schema documentation nor the official implementation nor the tiles served by MapTiler contain railway=proposed
.
railwayClasses
contains all accepted values of railway=*
.
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.
That was my point. It ended up in the tiles before this PR and presumably doesn't anymore, correct?
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.
Yes, I will not end up in the tiles any more.
I am curious about the licensing situation here. From what I see, OpenMapTiles development in the official repo has stopped (let's call it OMT 1.x) and now MapTiler is pushing into their private fork of it (let's call it OMT 2.x). In my understanding, this PR is essentially reverse engineering some changes from OMT 2.x and includes those back into OMT 1.x, but wired inside this repo. But doing this might be entirely legal, depending on the license. Which is something I really don't know. So the big questions is if MapTiler pulled a Mapbox here, or we are still allowed to somehow include the changes. |
@Nakaner: This is great, thank you. I'll take more of a look at it shortly but very happy to see this. @hyperknot: I don't particularly follow what OpenMapTiles is doing so I'll believe you on the development direction. My reading is that the reasonably bare-bones implementation of the schema that we provide is unlikely to be copyrightable: we are essentially just stuffing OSM tags into some fairly obvious layers with minimal additional processing. We simply don't do the more complex stuff that they do and which is more likely to attract copyright protection. But OMT are of course aware that tilemaker exists and includes this implementation, and I'm sure they'd be in contact if they had an issue with it. In the medium term, I envisage that we might move to Shortbread as the "endorsed" tilemaker schema and perhaps provide some tooling around extending it. |
@hyperknot I did not know which versions of OpenMapTiles exist at all. I found the sparse documentation that lacks many details. I had a client asking for these tiles. I produced the tiles with Tilemaker, they reported incompatibilities. I looked into the implementation and it answered some questions/confirmed some complaints. But the implementation was so difficult to read that I ended up looking in some vector tiles served by Maptiler (i.e. I reverse engineered it). There are still a few differences left:
|
Thank you! |
This pull request reduces the differences between official OpenMapTiles tiles as served by MapTiler (the schema documentation lacks details, therefore I inspected their tiles) and the implementation of Tilemaker.
Changes:
natural=dune
,natural=bare_rock/scree/scrub/shrubbery/tundra
).place=province/borough/quarter/island/isolated_dwelling
).place=island
mapped as polygons.subclass
for railway features, set correct attribute `class.highway=raceway
.mtb_scale
if the tag is not set on the OSM way.expressway
if the tag is missing.man_made=bridge
to layer transportation.Some of the bugs were found by a client using a fork of the schema.