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 landuse with grass color from z5 instead of z10 #3701

Merged
merged 2 commits into from
Apr 4, 2019

Conversation

vholten
Copy link
Contributor

@vholten vholten commented Mar 2, 2019

Fixes #3696

Change proposed in this pull request:

  • Add landuse=grass to the landcover-low-zoom layer and render it from z5 instead of z10

This allows large grassy areas, such as in the Netherlands, to be visible at low zoom.

https://www.openstreetmap.org/#map=9/52.0322/4.7873
z9 before
z9-master

z9 after
z9-new

https://www.openstreetmap.org/#map=8/52.0322/4.7873
z8 before
z8-master

z8 after
z8-new

@jeisenbe
Copy link
Collaborator

jeisenbe commented Mar 2, 2019 via email

@matthijsmelissen
Copy link
Collaborator

Looks good to me. Some colors, like grass/forest, are hard to distinguish, but that's not the fault of this pull request.

@jeisenbe
Copy link
Collaborator

jeisenbe commented Mar 4, 2019 via email

@Adamant36
Copy link
Contributor

Adamant36 commented Mar 4, 2019

From what I remember with early discussions landuse=grass is one of those tags that's a little vague, over mapped in places where it probably shouldn't be due to that, and should probably be surface tag instead. More then likely in this case its over applied because managed nature of it. So landuse=meadow would probably be better, but there's still lots of other instances where rendering grass at z5 would be beneficial. Maybe even this one so people will see it and re-tag it properly if need be.

@imagico
Copy link
Collaborator

imagico commented Mar 4, 2019

I don't actually see a good reason for any starting zoom level differentiation here for features that are otherwise rendered identically at all zoom levels. This is unexpected and confusing for mappers. So the same should IMO be done for landuse=village_green as well.

@aceman444
Copy link

I'm also surprised there are such large areas of landuse=grass, when this is intented for small managed patches of grass in residential/highway-side areas. Shouldn't the large country-side areas be natural=grassland or landuse=meadow or even landuse=farmland (if the grass is seeded to be farmed later).

@imagico
Copy link
Collaborator

imagico commented Mar 25, 2019

@vholten - i would prefer to remove all differentiation in starting zoom levels between different grass color tags, i.e. also for landuse=village_green. If you can change that i would merge this.

@vholten
Copy link
Contributor Author

vholten commented Mar 25, 2019

@imagico That sounds good, I'll probably do it this weekend.

@vholten vholten changed the title Render landuse=grass from z5 instead of z10 Render landuse with grass color from z5 instead of z10 Mar 31, 2019
@vholten
Copy link
Contributor Author

vholten commented Mar 31, 2019

PR updated: landuse=village_green is now also rendered from z5 instead of z10. I couldn't find any village_green areas that are large enough to be visible below z10 however.

@imagico Do you also want leisure=garden rendered from z5? It has a grass color and is currently rendered from z10.

@imagico imagico merged commit f8dcb1f into gravitystorm:master Apr 4, 2019
@imagico
Copy link
Collaborator

imagico commented Apr 4, 2019

Thanks.

leisure=garden is a somewhat different matter since that is differentiated in rendering at higher zoom levels.

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.

6 participants