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 noise of swimming pools at low zoom #3470

Merged
merged 2 commits into from
Oct 26, 2018
Merged

Reduce noise of swimming pools at low zoom #3470

merged 2 commits into from
Oct 26, 2018

Conversation

jragusa
Copy link
Contributor

@jragusa jragusa commented Oct 21, 2018

Fixes #585

Changes proposed in this pull request:

  • Remove outline of swimming pools between z14 and z17
  • Lighten colour of outline

Test rendering with links to the example places:
https://www.openstreetmap.org/#map=15/43.7081/7.3168
Before
z15
swimming_poolfinal_before_z15

z17
swimming_poolfinal_before_z17

z19
swimming_poolfinal_before_z19

After
z15
swimming_poolfinal_after_z15

z17
swimming_poolfinal_after_z17

z19
swimming_poolfinal_after_z19

@matthijsmelissen
Copy link
Collaborator

Thanks, so much better on z16-!

I'm not sure about toning down the outline so much on z17+, in my mind the outline also serves to distinguish pools from regular ponds. What do you think?

@jeisenbe
Copy link
Collaborator

jeisenbe commented Oct 21, 2018 via email

@jeisenbe
Copy link
Collaborator

jeisenbe commented Oct 21, 2018 via email

@jragusa
Copy link
Contributor Author

jragusa commented Oct 24, 2018

with outline from z14
z15
swimming_poolfinal2_after_z15

z17
swimming_poolfinal2_after_z17

@matthijsmelissen
Copy link
Collaborator

I think it would be good to keep the outline (perhaps slightly toned down) on the higher zoomlevels.

@Tomasz-W
Copy link

Tomasz-W commented Oct 24, 2018

@jragusa I think you confused chosen one values. In the ticket we decided on line-color: saturate(darken(@water-color, 20%), 20%); and in this PR it's different (outlines are less visible)

@jragusa
Copy link
Contributor Author

jragusa commented Oct 26, 2018

@Tomasz-W I indeed pushed wrong value for outline. See below same pictures with corrected outline. They are better visible.
z15
swimming_poolfinal2_after_z15
z17
swimming_poolfinal2_after_z17

@matthijsmelissen matthijsmelissen merged commit 59c47c4 into gravitystorm:master Oct 26, 2018
@matthijsmelissen
Copy link
Collaborator

Thanks!

@kocio-pl
Copy link
Collaborator

@matthijsmelissen Did you use "Squash and merge"? I guess this is a way to prevent to see every single commit in the merging history.

@matthijsmelissen
Copy link
Collaborator

I didn't - thanks for the pointer.

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.

5 participants