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

Drop 'highway=steps' from z=13 #3825

Merged
merged 2 commits into from
Aug 22, 2019

Conversation

btwhite92
Copy link
Contributor

Fixes #3808

Changes proposed in this pull request

  • Drop steps from z13

Description

Footway rendering was dropped with #3467, steps should likewise be dropped. For future reference, if more cleanup/adjustments happens to z13:

When taking the current status of z13 otherwise as a given removing steps would be an improvement. But i like to point out that #3467 (and to some extent already #747) left z13 and the whole zoom level progression in a fairly inconsistent state with several contradicting design ideas being thrown together. I have my doubts that in the long term a z13 design and a zoom level progression with no footways (and accordingly no steps) at z13 makes a lot of sense. #3467 did not really present a vision how this is supposed to work outside European urban settings and even there it has its problems.

So yes, removing the steps from z13 would be consequential in the current situation but it should not prejudice the long term strategy for z13 and around.

Originally posted by @imagico in #3808 (comment)

Test rendering with links to the example places

Before

steps-ex-old

(Vitória, Espírito Santo, Brasil) - thanks @LucFreitas for the excellent example location

After

steps-ex-new

@kocio-pl
Copy link
Collaborator

Thanks for taking care of this. It looks to me like pretty obvious change at this moment. Unfortunately my testing environment is broken currently and I have not too much time to fix it, so I hope that maybe somebody else would be able to test and merge it.

@jeisenbe
Copy link
Collaborator

I've reviewed this PR. The code works as expected. Here are some test images from Singapore. It's on the equator, so z13 looks like z12 in northern Europe. There are a number of steps in the two hilly parks here:

z14: Before/After (unchanged):
z14-singapore-no-steps-after

z13: Before
z13-singapore-with-steps-before

z13: After
z13-singapore-no-steps-after

Certainly this change works with the scheme after #3467 - although I would also like to reconsider some of the changes from that PR.

It does not fix the problems with the red dotted footways and paths on z14 or 15 - these might be improved by changing the rendering style for paths and footways, perhaps using something similar to the German style.

However, overall I would approve the PR as a reasonable step to take at the moment.

@imagico imagico merged commit 5fd5107 into gravitystorm:master Aug 22, 2019
@imagico
Copy link
Collaborator

imagico commented Aug 22, 2019

Thanks - and sorry for the delay.

As already said - this is consequential to #3467 - but z13 is in any case after #3467 quite out of balance and not really usable outside urban environments so requires overall work - which might ultimately also mean getting back footway rendering (and consequently steps).

@btwhite92 btwhite92 deleted the steps-z13-removal branch August 25, 2019 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drop highway=steps from z13
4 participants