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 icons for memorial subtags #3356

Merged
merged 21 commits into from
Nov 7, 2018
Merged

Add icons for memorial subtags #3356

merged 21 commits into from
Nov 7, 2018

Conversation

jragusa
Copy link
Contributor

@jragusa jragusa commented Aug 21, 2018

Fixes #3088

Changes proposed in this pull request:

  • Add icons for memorial subtags bust, stone and man_made=obelisk.
  • Associate blue_plaque and stolperstein to plaque
  • Replace generic icon of memorial
  • Move all related icons to the historic folder
  • Use a new icon statue for both memorial=statue and tourism=artwork + artwork_type=statue

Test rendering with links to the example places:
generic memorial tag
https://www.openstreetmap.org/node/5051735103
monument_left

memorial=bust
https://www.openstreetmap.org/node/5501258534
memorial_bust

memorial=statue
https://www.openstreetmap.org/node/3822475193
memorial_statue

memorial=stone
https://www.openstreetmap.org/node/4987880673
memorial_stone

man_made=obelisk
https://www.openstreetmap.org/way/72937686
memorial_obelisk

@kocio-pl
Copy link
Collaborator

Also related to #1066.

@Tomasz-W Tomasz-W mentioned this pull request Aug 21, 2018
26 tasks
@Tomasz-W
Copy link

Tomasz-W commented Aug 21, 2018

@jragusa Please edit description of first test rendering image. A bench icon was for historic=memorial + amenity=bench combination only, we stay with current generic icon for historic=memorial.
Can you take care of mentioned above #1066 by the way?

@kocio-pl
Copy link
Collaborator

My questions:

Associate blue_plaque and stolperstein to plaque

  1. Which stolperstein tagging do you want to render and what's your rationale?

https://wiki.openstreetmap.org/wiki/Tag:memorial%3Dstolperstein
https://wiki.openstreetmap.org/wiki/Tag:memorial:type%3Dstolperstein

taghistory 25
taghistory 26

Replace generic icon of memorial

  1. What do you mean - replace with what and what with memorial objects without specific categories?

@jragusa
Copy link
Contributor Author

jragusa commented Aug 21, 2018

@Tomasz-W Ok there is a misunderstood from my side. I initially though we change the generic icon to the bench. So I will change it and make tests. Do you have a location for test ?

@kocio-pl Concerning the tagging scheme, I only use memorial=stolperstein to remain consistent with the other tags. Since memorial:type=stolperstein is more frequent, do you prefer I include both or only the memorial:type ? In any case, I'm investigating tomorrow the respective count for each tagging scheme.

@Tomasz-W
Copy link

Example of memorial bench: https://www.openstreetmap.org/node/3648628350

@kocio-pl
Copy link
Collaborator

We can render them in a special way both, only one of them, or don't change anything. There's no clear answer yet for such problems, so it's good to think a moment and decide.

This style has been evolving mostly case by case, but with complex tags (very broad categories) like towers or castles we touch a new ground. I like systematic approach and having the same scheme would be nice, on the other hand so many uses make visual clutter already.

@Tomasz-W
Copy link

I vote for memorial=stolperstein, because:

@dieterdreist
Copy link

dieterdreist commented Aug 21, 2018 via email

@jragusa
Copy link
Contributor Author

jragusa commented Aug 22, 2018

There is a clear predominance of the memorial scheme. Note there is also some minor memorial_type tags.

tag memorial memorial:type
blue_plaque 181 1
bust 2 602 95
plaque 9 293 559
statue 7449 818
stele 4 385 171
stone 4 224 230
stolperstein 129 19 943
war_memorial 13 719 124
total 45 632 23 627

@dieterdreist
Copy link

dieterdreist commented Aug 22, 2018 via email

@kocio-pl
Copy link
Collaborator

Interesting trends:
taghistory 27

@dieterdreist
Copy link

dieterdreist commented Aug 22, 2018 via email

@kocio-pl
Copy link
Collaborator

Since this is German community decision and practice that is different than all the rest, I just asked them about possible change of stolperstein tagging scheme:

https://forum.openstreetmap.org/viewtopic.php?id=63458

@jragusa
Copy link
Contributor Author

jragusa commented Aug 22, 2018

That would be a good starting point to have an agreement with the German community about this tag.

In the meantime, I have difficulties to render memorial bench. I have errors from project.mml. Do we have other equivalents than the combination of amenity=bench + historic=memorial ?

@SomeoneElseOSM
Copy link
Contributor

Do we have other equivalents than the combination of amenity=bench + historic=memorial ?

If it helps, when I was going through historic and memorial combinations in taginfo in the UK about a month ago I didn't find any other historic bench combinations. You can see what I did find at https://github.com/SomeoneElseOSM/SomeoneElse-style/blob/master/style.lua#L2661 .

@Tomasz-W
Copy link

I think our goal should be to promote simple and understandable tagging schemes without exceptions, not some different subtypes mixes based on usages. If we have 8 subtypes and only 1 of them is different, and it actually includes only one country, I think that community of this country should repair it and make it matching to accepted subtypes tagging scheme.

@kocio-pl
Copy link
Collaborator

kocio-pl commented Aug 22, 2018

This conflict is a direct, but otherwise unrelated consequence of merging #3345.

Initial responses from German community look encouraging and indicate support for historic=memorial + memorial=stolperstein scheme. This would help to clean up historic=memorial scheme at last, since Stolpersteine is the main force behind memorial/memorial:type split:

taghistory 28

Next big thing would be moving memorial=war_memorial into some other subkey then, as this has been already agreed upon:

https://lists.openstreetmap.org/pipermail/tagging/2017-October/thread.html#33696

@Tomasz-W
Copy link

Ok, so remaining changes in this PR are to:

  • left current generic historic=memorial icon, and use bench icon only for combination with amenity=bench
  • ignore memorial=war_memorial tag, and treat it just as historic=memorial

@kocio-pl
Copy link
Collaborator

Could the statue icon be tuned so it's a bit more different than artwork? Maybe base could be stronger?

@Tomasz-W
Copy link

Well, actualy both icons are presenting the same thing (statue), so trying to make them different from each other is doomed to fail. As I think that statue shape is not a good symbol for most of artworks (they are usually some weird, experimental constructions), I've proposed to change an artwork icon (see #3088 (comment)) to avoid the problem.

@kocio-pl
Copy link
Collaborator

Hm, artwork can be anything and looking for the right shape it became clear to me that this icon should be small and unobtrusive, so I'm not sure these propositions could work.

On the other hand worst examples were like park with a lot of statues (#2236 (comment)), so maybe it might. It's a tagging problem to decide what is artwork and what is memorial (for example contemporary or ancient Greek religious figures) and it's outside the scope of styling data.

@Tomasz-W
Copy link

@kocio-pl What do you think about column/ totem icon for artworks? As current icon it's also vertical, but simpler shape is giving a bigger freedom of interpretation.

tourism artwork

@jragusa Can you test it?
Gist: https://gist.github.com/Tomasz-W/125758342433c6880afed2f3c955f2d4

PS. Current artwork icon leads to tagging for renderer, see eg. St. Peter Square in Vatican:
https://www.openstreetmap.org/#map=19/41.90222/12.45660

https://previews.123rf.com/images/marinv/marinv1509/marinv150900049/44850743-colonnades-that-surround-st-peter-s-square-in-rome-vatican-city-with-many-statues-on-top.jpg

They are not artworks, but statues of certain saints (historic=memorial + memorial=statue) , but they are tagged as tourism=artwork for osm-carto

@kocio-pl
Copy link
Collaborator

At first sight I like the totem idea, however on our scale it looks too close to new obelisk icon. What do you think about making top wider (http://pluspng.com/img-png/png-totem-pole-traditionally-totem-poles-represented-a-group-of-individual-spirits-events-in-time-clans-or-stories-1000.png) or add more horizontal elements (like this one: https://commons.wikimedia.org/wiki/File:%22Totem%22,_Carl%C3%A9.jpg)? Typical totem wings might look like cross (https://d30y9cdsu7xlg0.cloudfront.net/png/20257-200.png), but you might get a bit creative. 😄

As of Vatican etc. - they were already there when I was testing different artwork icons, so they definitely were not tagged for rendering, because artwork rendering was not existing yet while memorial rendering was there already. And this is your interpretation that these figures are memorials - religious objects might not been historic in general, so it's hard to know how should they be really tagged. That's the problem I still don't know how to solve.

@Tomasz-W
Copy link

@dieterdreist
Copy link

dieterdreist commented Aug 25, 2018 via email

@Tomasz-W
Copy link

Every memorial is an artwork, but not every artwork is a memorial.
On provided example photo from Vatican, these are statues of specific persons who used to live in previous centuries. It makes these figures closer to historic=memorial (the main function is to honour and keep a memory about these people) than an tourism=artwork (which they also are, but it's not a main function).

@kocio-pl
Copy link
Collaborator

Every memorial is an artwork, but not every artwork is a memorial.

👍 - thanks, this helps me a bit. So the artwork could be always an option and if something is double tagged (memorial+artwork), we might skip the artwork as the obvious part.

Memorial definition:

A feature for tagging smaller memorials, usually remembering special persons, peoples lost their lives in the wars, past events or missing places.

The question is how does it work for Zeus for example? Is such a statue made for remembering him? It's easy to say about historic things from past few hundred years, but what is the main function of religious statues?

Maybe memorial definition should be extended somehow, to make it cover all the religious depictions, no matter if they're historic or not (or it is disputed)? Or maybe we need special key like religious=*?

Maybe this is a harder question and should go to the Tagging list, but we need some basic understanding how do we treat them here, because it might be too hard to know. For example we could do something like with forest/wood - maybe we could render all the statues the same, no matter if this is tagged as memorial (memorial=statue) or an artwork (artwork_type=statue).

@jragusa
Copy link
Contributor Author

jragusa commented Oct 19, 2018

@kocio-pl I have updated the code following comment of @Tomasz-W and removed memorial_bench.

Below you can see the different rendering of artwork_type=statue:
https://www.openstreetmap.org/node/4720792741 (scultpure)
https://www.openstreetmap.org/node/4720792726 (statue)
artwork_statue

@Tomasz-W
Copy link

@jragusa Please update stone icon file, I missed it during pixel-aligning other icons.

https://gist.github.com/Tomasz-W/f1306db9c0086f0d209c50a9bf30ec73

Sorry for the problem ;)

@jragusa
Copy link
Contributor Author

jragusa commented Oct 19, 2018

@Tomasz-W done, no worries ;)

@kocio-pl
Copy link
Collaborator

kocio-pl commented Oct 20, 2018

I would remove stolperstein support also, since this is not yet decided and I'm not sure how to proceed with that. We can get back to it later, as far as I've seen all such complex solutions need some fixing later on (it was true for towers, castles and information amenities).

For now I will try to make independent tests. If I will find a time I will try to make a code for a memorial bench, but it also might be added later. This way or another I think next release will contain this code.

@@ -765,21 +769,34 @@
marker-clip: false;
}

[feature = 'historic_memorial'][zoom >= 17] {
[feature = 'historic_memorial'][zoom >= 17],
Copy link
Collaborator

Choose a reason for hiding this comment

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

You need to exclude here all defined memorials which are to be shown on z18 or z19, because now they first look like a generic memorial.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What would be displayed at z17 ? If I exclude all the features listed below, we will highlight from z17 historic=memorial without memorial subtags which are not the most important ones.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems that I should be more accurate:

  • I was thinking about z17 for generic memorials and statue
  • plaque should stay at z19

This way we have all three zoom levels used, which helps keep the map clean and distinct.

Generic memorials can be very different and have different size, for me generic memorial on z17 makes sense after generic monument at z16.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made corrections, I have tested on memorial=stele and memorial=bust and will check the rest tomorrow

@@ -765,21 +769,34 @@
marker-clip: false;
}

[feature = 'historic_memorial'][zoom >= 17] {
[feature = 'historic_memorial'][zoom >= 17],
[feature = 'historic_memorial_stele'][zoom >= 19] {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have historic_memorial_stele defined in the project.mml? I guess no and we use simple memorial check now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I also think this should be rather z18+.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

both fixed

[memorial = 'plaque'][zoom >= 19] {
marker-file: url('symbols/historic/plaque.svg');
}
[memorial = 'statue'][zoom >= 19] {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this should be still z17+.

Copy link
Contributor Author

@jragusa jragusa Oct 24, 2018

Choose a reason for hiding this comment

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

fixed, I left blue_plaque to z19

[memorial = 'statue'][zoom >= 19] {
marker-file: url('symbols/historic/statue.svg');
}
[memorial = 'stone'][zoom >= 19] {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this should be z18+.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

stone also to z18 ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I think so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@jragusa
Copy link
Contributor Author

jragusa commented Oct 26, 2018

@kocio-pl do you want additional screenshots with last modifications ? I checked each features and everything seems correct.

@kocio-pl
Copy link
Collaborator

No need to, I will check it soon. However not all elements have proper initial zoom level, my take is to change:

  • [memorial = 'plaque'][zoom >= 17] -> z19+ (the same as blue plaque)
  • [memorial = 'bust'][zoom >= 19] -> z18+ (bigger than plaques, smaller than full posture/statue)
  • there's also missing stele image rendering (which is present in labels rendering) - I would make it z18+.

I'm deeply sorry for the time it takes, I should probably start with listing all initial zoom levels. Multiple objects changes are hard to manage and test... It's quite a new challenge for me.

@jragusa
Copy link
Contributor Author

jragusa commented Oct 29, 2018

@kocio-pl don't be sorry. I'm not in the hurry.
1/ fixed
2/ fixed
3/ memorial=stele is not hardcoded because it has the same icon than generic memorial and filtering in the header allows them to render from z18.

I took the opportunity to clean the code and hence added a dedicated line to memorial=stele for clarity. What do you think ?

[feature = 'historic_memorial'][memorial = 'stone'][zoom >= 18],
[feature = 'historic_memorial'][memorial = 'blue_plaque'][zoom >= 19],
[feature = 'historic_memorial'][memorial = 'plaque'][zoom >= 19],
[feature = 'historic_memorial'][zoom >= 19] {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The last issue: I think this should be z17+, since we have no idea what can it be, so it's kind of generic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are your sure for memorial=blue_plaque ? They seem to be not so large. Take a look at some examples from the Guardian.

I would put the same zoom level than stolperstein.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I meant all the memorial values different than what we have on z18+ and z19+. Look at the similar case here:

[feature = 'man_made_tower']["tower:type" != 'cooling']["tower:type" != 'lighting']["tower:type" != 'bell_tower']["tower:type" != 'watchtower'] {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the fix satisfy you ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks, it looks promising and I will test it soon. I just think that excluding statue is not necessary, because it will be rendered on z17+ anyway (that's why I told only about excluding z18+ and z19+ objects). It adds a bit more of clarity, so it's not an error, but I think this line is long enough.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have found the problem - this war memorial is not showing any icon at all (instead of a standard one), while the label is still visible from z17:

https://www.openstreetmap.org/node/3183626423#map=19/52.26992/20.99322

Copy link
Contributor Author

@jragusa jragusa Nov 7, 2018

Choose a reason for hiding this comment

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

I resolved both problems. The second one was related to a missing comma.

@kocio-pl kocio-pl merged commit 67327f3 into gravitystorm:master Nov 7, 2018
@kocio-pl
Copy link
Collaborator

kocio-pl commented Nov 7, 2018

Thanks! As far as i could test it, it's OK, but rendering memorials etc. is so complex, that it will need an update anyway (especially after stolperstein will be resolved, but also a memorial bench should be properly depicted).

It's good to see more familiar shapes and distribution among multiple zoom levels, because memorials are similar only as a tagging idea, in reality the world of commemorating is much more diverse.

@Tomasz-W
Copy link

Tomasz-W commented Nov 7, 2018

@kocio-pl @jragusa Do you need new issue with memorial benches to not forgot it?

@kocio-pl
Copy link
Collaborator

kocio-pl commented Nov 8, 2018

I don't need it.

@jragusa jragusa deleted the memorial branch November 25, 2018 13:48
@jeisenbe jeisenbe added new features Requests to render new features POI labels Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new features Requests to render new features POI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Distinguish different memorial types
10 participants