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

Change scrub color and pattern #3529

Merged
merged 1 commit into from Nov 30, 2018

Conversation

Projects
None yet
4 participants
@jeisenbe
Copy link
Contributor

jeisenbe commented Nov 26, 2018

Fixes #2098 - "[Intermittent] Stream is Barely Visible on Scrub"
Related to #2069, However does not yet change the color of golf courses, which use the same color as current scrub but without a pattern. This should be addressed soon, perhaps by changing golf to use the leisure color.

Changes proposed in this pull request:
The current scrub color is: #b5e3b5 = Lch(86,29,143), hsla(120,45%,80%,1)

  1. Change scrub color to: #c8d7ab = Lch(84,24,122), hsla(67,41%,78%,1)

_The current scrub pattern uses: #8ece8f = Lch(77,41,143)
2. Change scrub pattern to use: #b0be93 = Lch(75,24,122)

  1. Update scrub.md so that it links to the new pattern from http://www.imagico.de/map/jsdotpattern.php

Test rendering with links to the example places:
http://www.openstreetmap.org/#map=15/20.7490/-156.8960
Hawaii z15
Before
hawaii-z15-before
After
hawaii-z15-scrub-only

Hawaii z16 http://www.openstreetmap.org/#map=16/20.7490/-156.8960
Before
hawaii-z16-before
After
hawaii-z16-scrub-only

Test area z15
Before
test1-before=
After
test-scrub-only-z15

@jeisenbe

This comment has been minimized.

Copy link
Contributor

jeisenbe commented Nov 26, 2018

Here are some lower zoom levels to compare with farmland and heath:

Goodwick, Wales
http://www.openstreetmap.org/#map=13/52.0101/-4.9899
z12 Before
goodwick-z12-master
After
goodwick-z12-scrub-only

z13 Before
goodwick-z13-master
After
z13-goodwick-scrub-only

z14 Before
goodwick-z14-master
After
z14-goodwick-scrub-only

Graig Isaf, Wales
With bog and marsh: http://www.openstreetmap.org/#map=7/52.81443/-4.01972
Before
bog-marsh-master
After
bog-marsh-scrub-only

@jeisenbe

This comment has been minimized.

Copy link
Contributor

jeisenbe commented Nov 26, 2018

Some areas with detailed landcover mapping in the Azores Islands, including scrub, forest, heath, grass, marsh, intermittent lake and intermittent streams.

Caldeira, Azores Islands
http://www.openstreetmap.org/#map=16/38.5847/-28.7164
Before
caldeira-master
After
caldeira-scrub-only-

Estrada da Caldeira, Azores
http://www.openstreetmap.org/#map=17/38.57280/-28.69047
Before
estrada-master
After
estrada-scrub-only

Sicosta, Azores
Villages with allotments, farmyards, orchards.
http://www.openstreetmap.org/#map=17/38.55036/-28.65888
Before
sicosta-master
After
sicosta-scrub-only

@Tomasz-W

This comment has been minimized.

Copy link

Tomasz-W commented Nov 27, 2018

@jeisenbe The original proposition was c8d6ab so why it has changed to c8d7ab here?

PS. Please do not put certain colour values in PRs names as we often change them during PR discussion and it might be confusing.

@kocio-pl

This comment has been minimized.

Copy link
Collaborator

kocio-pl commented Nov 27, 2018

Yes, the names of tickets should be short and succint in general, the rest should be inside.

@jeisenbe

This comment has been minimized.

Copy link
Contributor

jeisenbe commented Nov 27, 2018

It was always #c8d7ab in the test images that I was showing on issue #2098, but perhaps I mistyped the code in the comments? #c8d7ab is #d1e0b4 is 3% darker, by Lch.

There is no perceptible difference between c8d6ab and c8d7ab.
The chroma or saturation is only 1 or 2% different, by hsla or Lch, otherwise they are identical:

c8d6ab-vs-c8d7ab

Sorry about the title, I'll shorten it.

@jeisenbe jeisenbe changed the title Change scrub color to #c8d7ab and use new scrub pattern in #b0be93 to match Change scrub color and pattern Nov 27, 2018

@kocio-pl kocio-pl merged commit 40b20c1 into gravitystorm:master Nov 30, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kocio-pl

This comment has been minimized.

Copy link
Collaborator

kocio-pl commented Nov 30, 2018

I will certainly need to adjust to this new brownish-green rendering of scrub, but it is still green and it really solves the water rendering problem. We have some more green areas changes ahead and let's see how they play together.

Thanks for the coding work and analysis for all the people involved in this!

@jeisenbe jeisenbe deleted the jeisenbe:scrub-only branch Dec 1, 2018

@Tomasz-W Tomasz-W referenced this pull request Dec 1, 2018

Closed

Good first issues [meta-ticket] #3298

26 of 26 tasks complete
@Adamant36

This comment has been minimized.

Copy link
Contributor

Adamant36 commented Dec 24, 2018

@jeisenbe, I happened to notice that the new scrub pattern disappears one zoom level lower then forest. Could it perhaps be tweaked so they both disappear at the same level? (I can't remember if it did the same thing before the change or not, but I thought it might be due to the new density or something).

@jeisenbe

This comment has been minimized.

Copy link
Contributor

jeisenbe commented Dec 24, 2018

@Adamant36

This comment has been minimized.

Copy link
Contributor

Adamant36 commented Dec 24, 2018

Bummer. Maybe I'll do an issue to see if there's any will to change it then.

@jeisenbe

This comment has been minimized.

Copy link
Contributor

jeisenbe commented Dec 24, 2018

@Adamant36

This comment has been minimized.

Copy link
Contributor

Adamant36 commented Dec 24, 2018

I say if it works we do it. I don't see why it wouldn't on larger areas. Although a 16x16 pixel difference is rather large.

@Tomasz-W

This comment has been minimized.

Copy link

Tomasz-W commented Dec 24, 2018

I was going to notice this problem. It can be annoying with large scrub areas like here:

https://www.openstreetmap.org//#map=13/34.2229/-117.8636

please fix it.

@Adamant36

This comment has been minimized.

Copy link
Contributor

Adamant36 commented Dec 24, 2018

Wow, that's a huge scrub area. There's no reason the pattern shouldn't still display at z13 with a place like that.

@jeisenbe

This comment has been minimized.

Copy link
Contributor

jeisenbe commented Dec 24, 2018

@Tomasz-W

This comment has been minimized.

Copy link

Tomasz-W commented Dec 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment