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

Render lines and labels only on route=ferry ways, not relations #3727

Merged
merged 1 commit into from Apr 4, 2019

Conversation

tpikonen
Copy link
Contributor

Only render route=ferry label and dashed line on ways with positive osm_id in database, i.e. real ways, not relations.

Fixes #3726, platforms on route=ferry relations are not rendered with labels and lines any more.

Test rendering with links to the example places:

Before
ferry-platform

After
fixed-ferry-route

@matkoniecz
Copy link
Contributor

matkoniecz commented Mar 24, 2019

Is both before and after rendered from the same data? Note canal/river that appeared near entrance on "after".

Where this place is located? Maybe "before" version is the wrong one.

@tpikonen
Copy link
Contributor Author

The location is here.

The before image is from live osm.org map, the after is from kosmtik with a bbbike.org extract from yesterday, which is apparently not up to date. The width of the gap between the pier and land is the only visible difference here though.

@matkoniecz
Copy link
Contributor

Can you try testing both on the same data? Something like that should not appear.

@tpikonen
Copy link
Contributor Author

Ok, here's both master and this PR from kosmtik and BBBike Helsinki extract data:

Before:
before-ferry-route

After (the same as previously):
after-ferry-route

@matkoniecz
Copy link
Contributor

OK, then at least it is not caused by this PR.

@matthijsmelissen
Copy link
Collaborator

Are there any instances in the database of route=ferry relations where the elements of the relation are not tagged route=ferry? These ferries would disappear with this PR.

@tpikonen
Copy link
Contributor Author

Are there any instances in the database of route=ferry relations where the elements of the relation are not tagged route=ferry?

This overpass query shows that there are some tagless ways part of route=ferry relations which would indeed disappear if this PR is applied. So not rendering route=ferry relation members is not a good idea.

I can make a new PR which filters ferry ways by their role, but this needs the planet_osm_rels table to be present in the database. It's not used in any of the project.mml SQL queries at the moment. I hope it's there in the osm.org DB layout...(?)

@imagico
Copy link
Collaborator

imagico commented Mar 25, 2019

This overpass query shows that there are some tagless ways part of route=ferry relations which would indeed disappear if this PR is applied. So not rendering route=ferry relation members is not a good idea.

That is not necessarily the case. We never made the explicit decision to render route relations tagged route=ferry as multilinstrings. This just happens to be the default. So the question we need to decide on is if we actually want to do that and this does not only depend on if features exist with this tagging but also in particular on:

  • if this is established tagging
  • if this tagging is applied consistently by mappers
  • if it is properly documented

Your overpass query returns 576 ways (which is very few compared to the >20k features in total). Many of these are clearly poor or ambiguous mapping like

https://www.openstreetmap.org/way/550313539
https://www.openstreetmap.org/relation/5660063

Another large fraction of the features in question is where there is a waterway tag on the way and therefore the name would be ambiguous if it was tagged route=ferry as well. The route relation there is incorrectly used as a way equivalent to avoid creating a second way with the mapper assuming this to work just like a closed way and a multipolygon relation as two alternative ways to map the same thing.

But you can't have it both ways - either a route relation is just an alternative and more flexible method to map a (multi)linestring geometry, then all way members are part of this multilinestring - or it is a specific type or relation with members having different roles, then you can't use it as a makeshift multilinestring feature to avoid creating another way geometry.

And we have the obligation not to lure mappers into ambiguous mapping so IMO we should drop rendering route relations tagged route=ferry as multilinestrings independent of the specific issue shown here.

I can make a new PR which filters ferry ways by their role, but this needs the planet_osm_rels table to be present in the database. It's not used in any of the project.mml SQL queries at the moment. I hope it's there in the osm.org DB layout...(?)

That is not an option.

Copy link
Collaborator

@imagico imagico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note we probably should more often specifically not render geometries based on route relations generated by osm2pgsql unless we explicitly want to show them.

@imagico
Copy link
Collaborator

imagico commented Mar 30, 2019

I would merge this but i don't want to cut short discussion on rendering route relations tagged route=ferry (which this change would stop doing). Please speak up if you have objections to this.

@imagico imagico merged commit fc9bc71 into gravitystorm:master Apr 4, 2019
@jengelh
Copy link

jengelh commented Sep 23, 2019

Hm this broke the rendering of the routes from Stockholm (routes on way 694006263); should those relation routes all be independent overlapping ways?

Nakaner added a commit to Nakaner/openstreetmap-carto that referenced this pull request Sep 27, 2019
Pull request gravitystorm#3727 introduced the new condition osm_id > 0 but did
not update the condition for the indexes inteded to be used by the SQL
queries of these layers.
@jeisenbe
Copy link
Collaborator

OT:

should those relation routes all be independent overlapping ways?

No, but way 694006263 should be tagged with route=ferry, according to the wiki. It's currently just tagged with "ferry=trunk" and "ref=E 20" but lacks a main feature tag. That's like a railway which is just included in type=route relations but has no railway=rail tag.

https://wiki.openstreetmap.org/wiki/Tag:route%3Dferry - " use the tag on a single way way drawn along the whole route. In addition, the tag can also be added to a route relation relation containing all the ways."

pnorman pushed a commit that referenced this pull request Feb 13, 2020
Pull request #3727 introduced the new condition osm_id > 0 but did
not update the condition for the indexes inteded to be used by the SQL
queries of these layers.
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.

platforms ways on route=ferry relations rendered identically to route ways
7 participants