Skip to content
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

Merged
merged 16 commits into from
Apr 2, 2024

Conversation

Nakaner
Copy link
Contributor

@Nakaner Nakaner commented Feb 23, 2024

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:

  • Add missing features classes to layer landcover (natural=dune, natural=bare_rock/scree/scrub/shrubbery/tundra).
  • Add missing features to layer place (place=province/borough/quarter/island/isolated_dwelling).
  • Add support for place=island mapped as polygons.
  • Add attribute subclass for railway features, set correct attribute `class.
  • Do not write railway features to layer transportation_name.
  • Refactor layers transportation_name, transportation_name_mid, transportation_name_detail.
  • Do not write roads without names or reference number to layer transportation_name.
  • Do not write unnamed ferries to layer transportation_name.
  • Set correct minzoom values for all roads in the transportation and transportation_name layer.
  • Add highway=raceway.
  • Fix handling of roads under construction.
  • Do not set attribute mtb_scale if the tag is not set on the OSM way.
  • Add aerialways to layer transportation.
  • Do not set attribute expressway if the tag is missing.
  • Add areas for pedestrians and platforms to layer transportation.
  • Add man_made=bridge to layer transportation.

Some of the bugs were found by a client using a fork of the schema.

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]
Copy link
Contributor

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?

Copy link
Contributor Author

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=*.

Copy link
Contributor

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?

Copy link
Contributor Author

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.

@hyperknot
Copy link

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.

@systemed
Copy link
Owner

@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.

@Nakaner
Copy link
Contributor Author

Nakaner commented Feb 26, 2024

@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:

  • the place ranking mentioned by @systemed
  • usage of route relations for transportation_name layer
  • unsure: disputed boundaries (I did not bother)

@systemed systemed merged commit 2b68d2c into systemed:master Apr 2, 2024
8 checks passed
@systemed
Copy link
Owner

systemed commented Apr 2, 2024

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants