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

Use usual subway bridge style for subway construction bridges #3571

Merged

Conversation

tpetillon
Copy link
Contributor

Changes proposed in this pull request:

  • Make subway bridges under construction thinner and only appear from zoom 14 on.

Currently, all railway construction bridges are rendered the same. However not all (non-construction) railway bridges are identical.

Namely, subway bridges are thinner and appear later (zoom 14 instead of 13) than standard railways.

This PR unifies the appearance of subway bridges, whether under construction or not.

Some other railway types have their own rules for bridges (light rail, funicular, narrow gauge, etc.). They are not handled in this PR, but could be if there is interest.

Examples

(Area: https://www.openstreetmap.org/#map=14/48.1131/-1.6690)

Zoom 13

Before
image

After
image

Zoom 14

(This one is more subtle.)

Before
image
image

After
image
image

@matkoniecz
Copy link
Contributor

From what I see in code it affects also trams. (Changing also tramway construction bridges is not a blocker for this PR, rather an idea for a new potential one).

@matkoniecz
Copy link
Contributor

Fun fact: there are exactly 126 ways representing subway bridges under construction in OSM:http://overpass-turbo.eu/s/EzD

@tpetillon
Copy link
Contributor Author

Actually, a bit more. I get 261 ways by querying for other values of the key bridge. (https://overpass-turbo.eu/s/EzL) I admit it's still a tiny number though. But because such bridges tend to appear in larger cities and stay in that state for a few years, I figured that it may be valuable.

Indeed currently tram bridges under construction use the style of standard railways. I can include them in the changes if you want.

@matkoniecz
Copy link
Contributor

Right. I forgot about other values of bridge key.

And to be clear: this is a very good change, I left this comment partially because I am in the middle of reviewing this PR.

Fixing team construction bridges may be done as part of this PR or as the next one, both are perfectly fine.

@matkoniecz matkoniecz merged commit eba9849 into gravitystorm:master Dec 17, 2018
@matkoniecz
Copy link
Contributor

Thanks for fixing this inconsistency!

In case that you are would be interested in making other improvements: #2872 is a nice railway related one - code is relatively simple but it requires a bit thinking over what should be unified

@tpetillon
Copy link
Contributor Author

Thanks for the merge, and thanks to everyone who made it easy to setup the project. With Docker it's a breeze. :)

I'm indeed interested in making further changes. I need to familiarise myself with the code first.

@tpetillon tpetillon deleted the subway-construction-bridges branch December 17, 2018 20:36
@kocio-pl
Copy link
Collaborator

Great to hear that! OSM Carto is big and more coders might help to cover more problems.

@matkoniecz
Copy link
Contributor

@tpetillon If you are stuck with something - open an unfinished PR and ask for help, hopefully it will be something that we were stuck before and we can help :)

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.

3 participants