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 religious landuse and place of worship lighter #3493

Merged
merged 31 commits into from
Nov 10, 2018

Conversation

jeisenbe
Copy link
Collaborator

@jeisenbe jeisenbe commented Nov 2, 2018

Fixes #3457
Fixes #3322

Changes proposed in this pull request:

  • Change the fill color for religious landuse and places of worship to a lighter shade of gray

  • Change the outline to 30% darker than the new shade; significantly lighter than the previous outline

Test rendering with links to the example places:

Wamena, Indonesia
http://openstreetmap.org/#map=17/-4.09339/138.94656
z15 Before:

After:

z16 Before

After:

z17 Before

After:

YWAM campus in Kona, Hawaii
(This is a large religious campus)
http://openstreetmap.org/#map=16/19.6287/-155.9750

z15 Before:

After:

z16 Before:
ywamz16original

After:

jeisenbe and others added 22 commits October 17, 2018 22:06
update with changes from master in past month
Follows pattern of amenity=bank
Followed pattern of amenity=bank
Added internet_cafe to amenitypoints and project.mss,  and added icon to symbol/amenity


Merging local copy with changes made previously online only. Added username and email
…p-carto

Updating my fork with the recent changes in the original master
Merging new changes from gravitystorm/openstreetmap-carto.
merge upstream changes into fork
@jeisenbe jeisenbe changed the title Religious Religious areas in lighter gray Nov 2, 2018
@kocio-pl kocio-pl changed the title Religious areas in lighter gray Render religious landuse and place of worship lighter Nov 2, 2018
@kocio-pl
Copy link
Collaborator

kocio-pl commented Nov 2, 2018

Did you try lighter alternatives? I guess that it could be still less dark.

@polarbearing
Copy link
Contributor

The commit list is very long, with unrelated items, for changing just two colour values?

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 2, 2018 via email

@kocio-pl
Copy link
Collaborator

kocio-pl commented Nov 3, 2018

Maybe you made the branch from some other branch, not from the current master? Squashing works, so this is not a big problem.

@kocio-pl
Copy link
Collaborator

kocio-pl commented Nov 6, 2018

Could you post how does it look like with this new shade? One simple "after" image would do.

@kocio-pl
Copy link
Collaborator

kocio-pl commented Nov 6, 2018

BTW: denomination=foursquare? 😄

https://www.openstreetmap.org/way/607212018

taghistory 24

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 7, 2018 via email

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 7, 2018

Here are two options.

1) Add landuse_religious to the current rendering for amenity_place_of_worship,
plus a new line to allow lower zoom levels with waypixels >3000, as with other similar features

Kona z14
kona-z14-standard

Kona z15
kona-z16-standard

Kona z16
kona-z16-standard

2) Change rendering to match that of other landuses and societal amenities

Kona z14
kona-z14

Kona z15
kona-z15

Kona z16
kona-z16

Code 1):

  [feature = 'amenity_place_of_worship'],
  [feature = 'landuse_religious'] {
    [zoom >= 10][way_pixels > 3000][is_building = 'no'],
    [zoom >= 17] {
      text-name: "[name]";
      text-size: @standard-font-size;
      text-wrap-width: @standard-wrap-width;
      text-line-spacing: @standard-line-spacing-size;
      text-fill: #000033;
      text-dy: 12;
      text-face-name: @standard-font;
      text-halo-radius: @standard-halo-radius;
      text-halo-fill: @standard-halo-fill;
      text-placement: interior;
    }
  }

Code 2):

  [feature = 'amenity_place_of_worship'],
  [feature = 'landuse_religious'] {
    [zoom >= 10][way_pixels > 3000][is_building = 'no'],
    [zoom >= 17] {
      text-name: "[name]";
      text-size: @landcover-font-size;
      text-wrap-width: @landcover-wrap-width-size;
      text-line-spacing: @landcover-line-spacing-size;
      text-fill: #333333;
      text-dy: 12;
      [way_pixels > 12000] {
        text-size: @landcover-font-size-big;
        text-wrap-width: @landcover-wrap-width-size-big;
        text-line-spacing: @landcover-line-spacing-size-big;
      }
      [way_pixels > 48000] {
        text-size: @landcover-font-size-bigger;
        text-wrap-width: @landcover-wrap-width-size-bigger;
        text-line-spacing: @landcover-line-spacing-size-bigger;
      }
      text-face-name: @landcover-face-name;
      text-halo-radius: @standard-halo-radius;
      text-halo-fill: @standard-halo-fill;
      text-placement: interior;
    }
  }

@kocio-pl
Copy link
Collaborator

kocio-pl commented Nov 7, 2018

It would be interesting to see how it works with churches and other temples visible currently.

@kocio-pl
Copy link
Collaborator

kocio-pl commented Nov 7, 2018

Kona campus should be tagged as amenity=university by the way.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 7, 2018

Hope Chapel
1)Standard code
hopechapel-z16-standard
2)Landuse code
hopechapel-z16-landuse

Hawaiian Place of Worship (temple ruins)

  1. Standard
    hawaiian-pow-z17-standard
  2. Landuse
    hawaiian-pow-z17-landuse

Bhuddist POW

  1. Standard
    mission-z17-standard

  2. Landuse
    mission-z17-landuse

@kocio-pl
Copy link
Collaborator

kocio-pl commented Nov 7, 2018

I meant checking the situation when both temple and landuse have their respective names. Won't the name of the landuse eclipse name+icon of a temple?

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 7, 2018 via email

@polarbearing
Copy link
Contributor

Adding the label is open in #3322 so it is a good idea to fix that en passant.

The grey in the recent examples is very close to residential. It might be distinguishable in contrastive situations, but might be mis-recognised if standing alone.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 8, 2018 via email

Render landuse_religious in same text color as place_of_worship but in style of other landuses. Also render place_of_worship from z16 when waypixels greater than 3000
@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 8, 2018

Fixes #3322

Amenity=place_of_worship labels will continue to be rendered the same as currently.

Landuse=religious name label will be rendered the same as other landuse labels.

I've done the renderings with #d0d0d0 as the color for the area, and darkened it 50% for the religious landuse label.

I added the names to some areas in my town on JOSM for testing

z17 Landuse=religious with 2 place_of_worship buildings and a node, in town.
z17-gkip-new-png

z18
z18-gkip-new

z19
z19-gkip-new

Church in the country (it's a round building)
z17
z17-katolik-new

z18
z18-katolik-new

Test with large religious area with place_of_worship building almost at the center, made to see how the conflicting labels would resolve. It behaves as expected.
z15
z15-kecandian-new

z16
z16-kecandian-new

z17
z17-kecandian-new

z18
z18-kecandian-new

[feature = 'amenity_place_of_worship'][zoom >= 17] {
text-name: "[name]";
text-size: @standard-font-size;
text-wrap-width: @standard-wrap-width;
text-line-spacing: @standard-line-spacing-size;
text-fill: #000033;
text-fill: #000033;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a space to remove at the end.

Render landuse_religious in same text color as place_of_worship but in style of other landuses. Also render place_of_worship from z16 when waypixels greater than 3000. Changed fill color to #d0d0d0 as in test renderings. Amended commit to fix whitespace errors and duplicate code error.
@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 9, 2018 via email

@kocio-pl
Copy link
Collaborator

kocio-pl commented Nov 9, 2018

I'm happy with the results. Please just remove the extra space you have introduced by accident and this will be ready to merge - see #3493 (comment).

@polarbearing
Copy link
Contributor

yes d0d0d0 is fine for me

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 9, 2018 via email

@kocio-pl kocio-pl merged commit c724770 into gravitystorm:master Nov 10, 2018
@kocio-pl
Copy link
Collaborator

Thanks for the change and adding the label (thanks, @polarbearing!).

I'm not able to give more attention to the OSM Carto than I already do, sorry about that. I'm not sure what was the reason and it takes some time to investigate, but in the end the most important thing is that I can squash and merge your code properly.

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.

4 participants