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

Add color fill for mangrove, reedbed and saltmarsh wetlands #3807

Merged
merged 5 commits into from
Aug 30, 2019

Conversation

jeisenbe
Copy link
Collaborator

@jeisenbe jeisenbe commented Jun 15, 2019

Fixes #2013
Fixes #2025
Fixes #3799

Changes proposed in this pull request:

  • Add @scrub color fill for wetland=mangrove
  • Add @grass color fill for wetland=reedbed
  • Add @grass color fill with a white dot pattern for wetland=saltmarsh

1) Mangrove @scrub fill - #c8d7ab

z13 Potowai

Before
https://www.openstreetmap.org/#map=13/-4.2983/134.9890
z13-potowai-mangroves-before
After
z13-potowai-mangroves-scrub-fill-after

z15 mangroves by tidalflat

https://www.openstreetmap.org/#map=15/-4.4014/135.0908
Before
z15-mangroves-tidalflat-beach-before
After
z15-mangroves-scrub-fill-tidalflat-beach-aftrer

2) Saltmarsh with white dot pattern and @grass fill - #cdebb0

A. Salt marsh (left) and marsh (right) - before

https://www.openstreetmap.org/#map=13/51.3590/4.1393
z13-schelde-saltmarsh-vs-marsh-before
After
z13-schelde-saltmarsh-dots2

B. Before - Small salt marshes: one over sea water, one over land in database

https://www.openstreetmap.org/#map=15/51.4552/3.6555
z15-slufter-saltmarshes-zeeland-before
After
z15-slufter-saltmarsh-dots2

C. Before - saltmarsh (left) next to marsh (center), mouth of the Eel river, California

z14-eel-river-saltmarsh-marsh-before
After
z14-eel-river-marsh-saltmarsh-dot2

3) Reedbed @grass fill #cdebb0

z18 reedbeds vs marshes before

https://www.openstreetmap.org/#map=18/51.73020/3.72768
18-reedbeds-vs-marshes-before
After
z18-reedbeds-marshes-grass-color-after

z16 reedbed farm before

https://www.openstreetmap.org/#map=16/51.4514/3.9198
z16-reedbed-farm-before
After
z16-reedbed-farm-grass-color

Copy link
Collaborator

@imagico imagico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and works fine overall.

I think it would be better to only render the grain pattern for saltmarsh on the higher zoom levels where wetlands are also differentiated through the overlay (that is z13 and above).

It might also be worth trying the reef pattern (in bright color obviously) as alternative to the pattern you designed. I could imagine it might look a bit less noisy.

@imagico
Copy link
Collaborator

imagico commented Jun 19, 2019

By the way @jeisenbe - i sent you two emails about two weeks ago and i have not received a reply - could you check your spam filter?

@jeisenbe
Copy link
Collaborator Author

By the way @jeisenbe - i sent you two emails about two weeks ago and i have not received a reply - could you check your spam filter?

I never got those emails @imagico, even after checking the spam filter. Did you every try resending them?

@jeisenbe
Copy link
Collaborator Author

I've made the suggested change to limit the pattern to z13 and above only.

And I've also tested using a pattern based on reef.svg but in white with partial opacity as suggested:

  1. Current commit

  2. Reef dots - 100% opacity white
    z13-verdronken-saltmarsh-after-with-reef-100-percent-opacity

  3. Reef dots - 75% opacity
    z13-verdronken-saltmarsh-after-with-reef-75-percent-opacity

  4. Reef dots - 50% opacity
    z13-verdronken-saltmarsh-after-with-reef-50-percent-opacity

The 100% opacity version is ok, but the others seem too subtle to me.

@imagico
Copy link
Collaborator

imagico commented Jul 28, 2019

I never got those emails @imagico, even after checking the spam filter. Did you every try resending them?

I sent them again on June 20 - from two different mail clients. That Google has an arrogant attitude in terms of what email they regard as legitimate and what not is known but i have never seen this in such extreme form with both mail from my gmx.de and from imagico.de being silently ignored (which is not ok by any measure - they'd either have to reject it or deliver it).

I tried using the OSM messaging system now - which should work no matter how much google tries to prevent it from working.

@jeisenbe
Copy link
Collaborator Author

Here are tests at https://www.openstreetmap.org/#map=#14/40.6192/-124.2910 with salt marsh and marsh in the same view:

  1. Current commit

  2. 100% opacity reef pattern in white
    z14-eel-river-marshes-reef-dots-100percent

  3. 75%
    z14-eel-river-marshes-reef-dots-75percent

  4. 50%
    z14-eel-river-marshes-reef-dots-50percent

@jeisenbe
Copy link
Collaborator Author

@imagico thoughts on the current pattern vs the more subtle reef.svg white dot pattern?

@imagico
Copy link
Collaborator

imagico commented Aug 21, 2019

The reef pattern seems a bit weak as is - you could try to make it heavier but ultimately the current version is fine as well.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Aug 21, 2019 via email

Copy link
Collaborator

@imagico imagico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, i am fine with this now.

@jeisenbe
Copy link
Collaborator Author

I'm going to merge this now, since it's been approved by another maintainer and there were no other concerns. Just checking: is it best to "squash and merge" the 5 commits into one, or just rebase and merge the 5 separate commits?

It seems like it would help keep the development history more concise to use squash and merge.

@imagico
Copy link
Collaborator

imagico commented Aug 29, 2019

I don't really mind but others have indicated to prefer squash and merge.

@kocio-pl
Copy link
Collaborator

The motivation was that squash and merge keeps the code changes clean, so people outside the project can see what changes, not the inner technical details of the change. It also makes writing Changelog easier.

@jeisenbe
Copy link
Collaborator Author

Should I update the changelog when merging this PR, or is that done later?

@jeisenbe jeisenbe merged commit 8b8e392 into gravitystorm:master Aug 30, 2019
@jeisenbe jeisenbe deleted the mangrove-reedbed-saltmarsh branch August 30, 2019 13:24
jeisenbe added a commit that referenced this pull request Aug 31, 2019
Also added PR# to each item in this release
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants