organicmaps/organicmaps-tmp
Archived
13
Fork 1
Code Issues2.7k Pull requests182 Projects Releases65 Wiki Activity Actions

[styles] Hide icons for unnamed gardens forests parks #3386

Merged
mpawelski merged 3 commits from hide-icons-for-unnamed-gardens-forests-parks into master 2022-10-29 11:31:26 +00:00
Conversation 34 Commits 3 Files changed 62 +7395 -7214
mpawelski commented 2022-09-10 23:29:48 +00:00 (Migrated from github.com)

Currently we show icons for landuse=forest, leisure=park, landuse=garden areas. But this areas are often pretty small which means we will show a lot of icons on a screen. In my area this is quite big problem especially for landuse=garden which is used often as general "maintaned greenery" area and it looks very bad with a lot of icons displayed. But even for small "forest" areas I don't see much value in displaying this icon (you can clearly see this is some "green stuff" and if you want to know more you can select it).

Recently this problem was a bit mitigated by not showing icons for "residential gardens" () but in my experience garden:type=residential is often not set.

I propose to only show icons for "named" green areas. Very often this are parks, some bigger gardens and forests. Something of bigger significance that can justify icon. This was already suggested here and I completely agree with it.

Here are some before/after examples:

before:

after:

However, we still show areas with a "name" tag. Examples:
garden with name

park with name

Currently we show icons for `landuse=forest`, `leisure=park`, `landuse=garden` areas. But this areas are often pretty small which means we will show a lot of icons on a screen. In my area this is quite big problem especially for `landuse=garden` which is used often as general "maintaned greenery" area and it looks very bad with a lot of icons displayed. But even for small "forest" areas I don't see much value in displaying this icon (you can clearly see this is some "green stuff" and if you want to know more you can select it). Recently this problem was a bit mitigated by not showing icons for "residential gardens" (https://git.omaps.dev/organicmaps/organicmaps/issues/1746) but in my experience `garden:type=residential` is often not set. I propose to only show icons for "named" green areas. Very often this are parks, some bigger gardens and forests. Something of bigger significance that can justify icon. This was already suggested [here](https://git.omaps.dev/organicmaps/organicmaps/issues/1746#issuecomment-1044940739) and I completely agree with it. Here are some before/after examples: before: - <img src="https://user-images.githubusercontent.com/2056282/189504934-60883114-7506-4f9f-b9f1-9494c7aecf28.png" width="300"> <img src="https://user-images.githubusercontent.com/2056282/189504943-789d16e1-0818-4584-aaf9-036e71bdb369.png" width="300"> after: - <img src="https://user-images.githubusercontent.com/2056282/189504955-3f1a6406-8e6a-4681-8279-fa342c1190e1.png" width="300"> <img src="https://user-images.githubusercontent.com/2056282/189504961-70147fb9-f55c-4118-8751-23aafa6eccde.png" width="300"> However, we still show areas with a "name" tag. Examples: garden with name <img src="https://user-images.githubusercontent.com/2056282/189504982-05f4bfb2-a9dc-4a06-b3a8-91d637b3b0a8.png" width="300"> park with name <img src="https://user-images.githubusercontent.com/2056282/189504985-a483a14c-d87a-4379-8019-40072397b440.png" width="300">
👍 4 ❤️ 5
biodranik commented 2022-09-11 22:42:04 +00:00 (Migrated from github.com)

Thanks, generally, the map looks cleaner.

As I mentioned in the referenced issue, what about motivating users to edit/add missing data by showing clickable icons?

What about hiding icons if the area of the feature is below a certain threshold?

Thanks, generally, the map looks cleaner. As I mentioned in the referenced issue, what about motivating users to edit/add missing data by showing clickable icons? What about hiding icons if the area of the feature is below a certain threshold?
mpawelski commented 2022-09-13 10:07:08 +00:00 (Migrated from github.com)

Thanks @biodranik :)
I replied in to keep the discussion in one place

Thanks @biodranik :) I replied in https://git.omaps.dev/organicmaps/organicmaps/issues/1746 to keep the discussion in one place
biodranik (Migrated from github.com) approved these changes 2022-09-13 21:58:36 +00:00
biodranik (Migrated from github.com) left a comment

@vng PTAL

@vng PTAL
vng commented 2022-09-14 06:03:35 +00:00 (Migrated from github.com)

Well, this is a trade-off change and looks like it's the best currently available option.
Some areas become cleaner, but some areas also will be a bit empty.

The ideal fix is to have some dynamic icons filtering mechanics depending on POIs density. Some day in future.

Well, this is a _trade-off_ change and looks like it's the best currently available option. Some areas become cleaner, but some areas also will be a bit empty. The ideal fix is to have some dynamic icons filtering mechanics depending on POIs density. Some day in future.
biodranik commented 2022-09-14 09:56:01 +00:00 (Migrated from github.com)

It would be sad if large remote forest icons will disappear after this change. Maybe it's scope can be narrowed to city gardens only?

It would be sad if large remote forest icons will disappear after this change. Maybe it's scope can be narrowed to city gardens only?
biodranik commented 2022-09-14 09:56:39 +00:00 (Migrated from github.com)

It would be better to test it and compare it in some beta version first, without merging to the master.

It would be better to test it and compare it in some beta version first, without merging to the master.
mpawelski commented 2022-09-14 18:13:22 +00:00 (Migrated from github.com)

It would be better to test it and compare it in some beta version first, without merging to the master.

Should I rebase it on beta branch then?

Should contributing PRs go to master or beta? I didn't found such information in CONTRIBUTING.md file.

> It would be better to test it and compare it in some beta version first, without merging to the master. Should I rebase it on `beta` branch then? Should contributing PRs go to `master` or `beta`? I didn't found such information in CONTRIBUTING.md file.
biodranik commented 2022-09-25 07:05:58 +00:00 (Migrated from github.com)

I think this issue can be solved by introducing textures for area objects. Right?

At the moment, round icons for natural things do not follow the consistency rule that we try to follow: icons with circles are related to some businesses.

What if (as an experiment) we try to remove circles for these icons? Maybe also for forests and parks? Will the map become cleaner?

I think this issue can be solved by introducing textures for area objects. Right? At the moment, round icons for natural things do not follow the consistency rule that we try to follow: icons with circles are related to some businesses. What if (as an experiment) we try to remove circles for these icons? Maybe also for forests and parks? Will the map become cleaner?
biodranik commented 2022-09-25 07:06:26 +00:00 (Migrated from github.com)

Regarding branches, you can make PR to the beta branch. I've actualized it with the current master.

Regarding branches, you can make PR to the beta branch. I've actualized it with the current master.
mpawelski commented 2022-09-25 11:58:45 +00:00 (Migrated from github.com)

I think this issue can be solved by introducing textures for area objects. Right?

I would love to have some textures on OM.

osm.org have them and in some cases it makes it very clear what area represents (like "forest", "scrub", "cementary", etc), some are more arbitrary (like the landuse=garden, landuse=allotments) where I would not guess what it represent without learning it.
mapy.cz uses textures much less often but still for some areas they use them to make it much clearer what you see ("cementary", "wetland", "swamp")

But IMO in this case (landuse=forest, leisure=park, landuse=garden areas) not having textures is not a big issue. I just think for this types of areas, when they don't have a name, it's good enough to render them as "some shade of green" area. Most people don't care about concrete type of such greenery and they don't need and icon for it.

But I see you care about distinguishing this kind of areas with an icon or a texture. IMO texture is 100 times better that icon for such areas.

What if (as an experiment) we try to remove circles for these icons? Maybe also for forests and parks? Will the map become cleaner?

I think if will be much cleaner. IMO even if we remove icons completely for this areas it would be an improvement.

Regarding branches, you can make PR to the beta branch. I've actualized it with the current master.

ok, I'll rebase commits from this PR to beta branch

> I think this issue can be solved by introducing textures for area objects. Right? I would *love* to have some textures on OM. [osm.org](https://www.openstreetmap.org) have them and in some cases it makes it very clear what area represents (like "forest", "scrub", "cementary", etc), some are more arbitrary (like the `landuse=garden`, `landuse=allotments`) where I would not guess what it represent without learning it. [mapy.cz](https://en.mapy.cz) uses textures much less often but still for some areas they use them to make it much clearer what you see ("cementary", "wetland", "swamp") But IMO in this case (`landuse=forest`, `leisure=park`, `landuse=garden` areas) not having textures is not a big issue. I just think for this types of areas, when they don't have a name, it's good enough to render them as "some shade of green" area. Most people don't care about concrete type of such greenery and they don't need and icon for it. But I see you care about distinguishing this kind of areas with an icon or a texture. IMO texture is 100 times better that icon for such areas. > What if (as an experiment) we try to remove circles for these icons? Maybe also for forests and parks? Will the map become cleaner? I think if will be much cleaner. IMO even if we remove icons completely for this areas it would be an improvement. > Regarding branches, you can make PR to the beta branch. I've actualized it with the current master. ok, I'll rebase commits from this PR to beta branch
Misalf-git commented 2022-09-25 14:29:20 +00:00 (Migrated from github.com)

Using textures instead of icons sounds like a great idea as some areas can't be differentiated (like some greenery). However, I'd suggest to choose only subtle indications for areas to keep the map "clean". And only where really necessary.

Using textures instead of icons sounds like a great idea as some areas can't be differentiated (like some greenery). However, I'd suggest to choose only subtle indications for areas to keep the map "clean". And only where really necessary.
👍 2
RedAuburn commented 2022-09-25 14:42:34 +00:00
Member

Agreed, I quite like that OM uses so few textures, it makes it look very clean. If textures are used, they should be very subtle

Agreed, I quite like that OM uses so few textures, it makes it look very clean. If textures are used, they should be very subtle
👍 1
mpawelski commented 2022-09-27 10:56:49 +00:00 (Migrated from github.com)

I agree with @Misalf-git and @endim8.
I'm not a big fan of heavy texture map like on osm.org. I mentioned stuff like "cementery", "wetlands", because for me this are good examples when subtle texture improve the map visibility and makes it "cleaner".
Examples:
landuse=cementery

natural=wetland

But when it comes to areas of different types of greenery I prefer just solid colors. When I see light green I can guess it's most probably grass-like area (landuse=grass, natural=grassland, landuse=meadow, etc), anything with darker green is probably something with trees, bushes or some maintained greenery (landuse=forest, natural=wood, leisure=garden, leisure=park, natural=scrub, natural=shrubbery, landuse=village_green, etc). I'm not interested in details.

I think @biodranik have different opinion on that:

It would be sad if large remote forest icons will disappear after this change. Maybe it's scope can be narrowed to city gardens only?

For me if such remote forest icon would disappear then it would still be an improvement. This icon for me doesn't provide any value.

However, if we would get rid of icons for such green areas I think it's worth to think if we can improve the UX of selecting an area.

Right now you can easily tap on map any icon and any building. But for other areas you need to touch and hold for some time to select and area. This IMO is super undiscoverable UX. Why we can't easily select any areas? The fact that you can easily tap the screen to select any building (and for example see that it's actually a "garage") seems a bit arbitrary. Having possibility to easily tap on green area to check that it's a forest would be great for those that want that information (but as I said I think not many people want it anyway)

I agree with @Misalf-git and @endim8. I'm not a big fan of heavy texture map like on osm.org. I mentioned stuff like "cementery", "wetlands", because for me this are good examples when subtle texture improve the map visibility and makes it "cleaner". Examples: `landuse=cementery` <img src="https://user-images.githubusercontent.com/2056282/192506934-24645501-326f-4d15-9ee4-ec5389821e2d.png" width=400/> <img src="https://user-images.githubusercontent.com/2056282/192506946-fcc616e6-1cba-4008-8903-d914f11a36b7.png" width=400/> `natural=wetland` <img src="https://user-images.githubusercontent.com/2056282/192507263-9cbc8b50-b29b-4d41-ab2c-c16d382b2d58.png" /> <img src="https://user-images.githubusercontent.com/2056282/192507267-6713ea3d-e94a-4b27-aef4-3fe15902b6ab.png"/> But when it comes to areas of different types of greenery I prefer just solid colors. When I see light green I can guess it's most probably grass-like area (`landuse=grass`, `natural=grassland`, `landuse=meadow`, etc), anything with darker green is probably something with trees, bushes or some maintained greenery (`landuse=forest`, `natural=wood`, `leisure=garden`, `leisure=park`, `natural=scrub`, `natural=shrubbery`, `landuse=village_green`, etc). I'm not interested in details. I think @biodranik have different opinion on that: > It would be sad if large remote forest icons will disappear after this change. Maybe it's scope can be narrowed to city gardens only? For me if such remote forest icon would disappear then it would still be an improvement. This icon for me doesn't provide any value. However, if we would get rid of icons for such green areas I think it's worth to think if we can improve the UX of selecting an area. Right now you can easily tap on map any icon and any building. But for other areas you need to touch and hold for some time to select and area. This IMO is super undiscoverable UX. Why we can't easily select any areas? The fact that you can easily tap the screen to select any building (and for example see that it's actually a "garage") seems a bit arbitrary. Having possibility to easily tap on green area to check that it's a forest would be great for those that want that information (but as I said I think not many people want it anyway)
biodranik commented 2022-09-27 12:03:54 +00:00 (Migrated from github.com)

We tried the "single tap selects everything" strategy, and got many complaints from users who accidentally selected something, and Place Info Page appeared. There is also an existing feature on a single tap on an empty/remote area, that toggles the fullscreen mode. There should be some better way, let's brainstorm it.

We tried the "single tap selects everything" strategy, and got many complaints from users who accidentally selected something, and Place Info Page appeared. There is also an existing feature on a single tap on an empty/remote area, that toggles the fullscreen mode. There should be some better way, let's brainstorm it.
mpawelski commented 2022-09-27 15:15:37 +00:00 (Migrated from github.com)

We tried the "single tap selects everything" strategy, and got many complaints from users who accidentally selected something, and Place Info Page appeared.

Ah, this damn users with their fat fingers! 😡 😉

More seriously though, for me this PR is actually quite a good solution. You can click green areas that have name easily and I don't care about these areas that don't. I would also like to change the icon to not have round background to make the map a bit "cleaner".

But to summarize other suggested options and cons that some people raised (it's all subjective so some cons can be actually pros to some 😉):

  • Don't render icons for areas without name tag. Basically this PR.
    Cons: icons won't be shown for things like large remote forest
  • Render textures instead of plain colors to distinguish areas more easily.
    Cons: map will not look that "clean"
  • Don't show icon for small areas (decide on some threshold)
    cons: there are some small named areas that are worth to show
  • Just remove circle background from icons
    Cons: better that nothing, but IMO map will be still not "clean" enough.

I'm also using mapy.cz app the most (after OM of course) and did some analysis of how they render OSM green areas for comparison/inspiration:

  • they render icons for named leisure=garden areas so you can click it, they don't show icons for unnamed gardens, just render green area. It's basically the same behaviour that this PR introduce.
  • the same situation with leisure=park, landuse=recreation_ground Render area and icon for areas with a name tag, no icons for areas without name tag.
    BTW, OM doesn't render an icon for named landuse=recreation_ground but display clickable label.
  • sometimes they don't render leisure=garden and landuse=grass at all. I'm Not sure why. I guess they don't want to render too many small areas like that in a city. This is something I dislike a lot. Map doesn't look that detailed because of that.
  • for landuse=forest they don't render icon at all, for named forest they display the name but you can't click it

Looks like they mostly came up with similar solution like this PR, except they decide to sometimes not render green areas at all in a city and they don't show clickable icons for forests.

> We tried the "single tap selects everything" strategy, and got many complaints from users who accidentally selected something, and Place Info Page appeared. Ah, this damn users with their fat fingers! 😡 😉 More seriously though, for me this PR is actually quite a good solution. You can click green areas that have name easily and I don't care about these areas that don't. I would also like to change the icon to not have round background to make the map a bit "cleaner". But to summarize other suggested options and cons that some people raised (it's all subjective so some cons can be actually pros to some 😉): - Don't render icons for areas without `name` tag. Basically this PR. Cons: icons won't be shown for things like large remote forest - Render textures instead of plain colors to distinguish areas more easily. Cons: map will not look that "clean" - Don't show icon for small areas (decide on some threshold) cons: there are some small named areas that are worth to show - Just remove circle background from icons Cons: better that nothing, but IMO map will be still not "clean" enough. --- I'm also using [mapy.cz](https://play.google.com/store/apps/details?id=cz.seznam.mapy) app the most (after OM of course) and did some analysis of how they render OSM green areas for comparison/inspiration: - they render icons for named `leisure=garden` areas so you can click it, they don't show icons for unnamed gardens, just render green area. It's basically the same behaviour that this PR introduce. - the same situation with `leisure=park`, `landuse=recreation_ground` Render area and icon for areas with a `name` tag, no icons for areas without `name` tag. BTW, OM doesn't render an icon for named `landuse=recreation_ground` but display clickable label. - sometimes they don't render `leisure=garden` and `landuse=grass` at all. I'm Not sure why. I guess they don't want to render too many small areas like that in a city. This is something I dislike a lot. Map doesn't look that detailed because of that. - for `landuse=forest` they don't render icon at all, for named forest they display the name but you can't click it Looks like they mostly came up with similar solution like this PR, except they decide to sometimes not render green areas at all in a city and they don't show clickable icons for forests.
biodranik commented 2022-09-28 20:45:50 +00:00 (Migrated from github.com)

There are many good points mentioned in the conversation.

Regarding this PR, @mpawelski can you please try to remove circles for icons and compare screenshots? In theory, the map should become way cleaner.

There are many good points mentioned in the conversation. Regarding this PR, @mpawelski can you please try to remove circles for icons and compare screenshots? In theory, the map should become way cleaner.
biodranik commented 2022-09-30 21:25:01 +00:00 (Migrated from github.com)

Let me summarize:

  1. Removing the forests icon is a no-go blocker. They are very important for orienteering.
  2. Carefully designing textures can help a lot (e.g. render them only when you zoomed in, in this case, some flowers may appear and make it clear and recognizable). But adding textures is a complex task.
  3. Threshold idea won't work in many cases.
  4. Imagine someone wants to map a garden. But after adding it the icon... does not appear... Bad!
  5. Removing the circle but leaving the icon is the easiest and less destructive option at the moment.
Let me summarize: 1. Removing the forests icon is a no-go blocker. They are very important for orienteering. 2. Carefully designing textures can help a lot (e.g. render them only when you zoomed in, in this case, some flowers may appear and make it clear and recognizable). But adding textures is a complex task. 3. Threshold idea won't work in many cases. 4. Imagine someone wants to map a garden. But after adding it the icon... does not appear... Bad! 5. Removing the circle but leaving the icon is the easiest and less destructive option at the moment.
mpawelski commented 2022-10-04 11:48:38 +00:00 (Migrated from github.com)

I removed background from forest, park, garden icons. However, I was afraid the contrast of "dark green icon" on "light green area" might be not enough to distinguish for some people (currently the icons with background have small outline/border around the circle - white with 60% opacity) so I also tested version with similar outline around the shape of an icon without background.

Here are the icons I used:

  • landuse=garden

garden-m garden-m-no-bg-outline garden-m-no-bg

  • leisure=park

park-m park-m-no-bg-outline park-m-no-bg

  • landuse=forest

nparkF-m nparkF-m-no-bg-outline nparkF-m-no-bg

Screenshots

(I recommend to zoom out a bit in browser to view it 😉 )

  1. 52.431639, 16.943598
    lots of small gardens

Screenshot_20221003_203259
Screenshot_20221003_224256
Screenshot_20221003_233046

  1. 52.409347, 16.913122
    another small gardens, on zoom 18, this areas were even smaller then the icon

Screenshot_20221003_203559
Screenshot_20221003_231138
Screenshot_20221003_233234

  1. 52.451107, 16.942539
    here we can see small forest areas

Screenshot_20221003_205013
Screenshot_20221003_224604
Screenshot_20221003_233335

  1. 52.409288, 16.931286
    garden with a name

Screenshot_20221003_205811
Screenshot_20221003_224728
Screenshot_20221003_233434

  1. 53.504118, 19.749705
    "node garden" - garden mapped with a single node, not as area

Screenshot_20221003_210113
Screenshot_20221003_224946
Screenshot_20221003_233631

  1. 52.89343,16.55155
    some named park

Screenshot_20221003_215233
Screenshot_20221003_225051
Screenshot_20221003_233720

  1. 52.45625,16.93457
    some named landuse=forest and leisure=nature_reserve

Screenshot_20221003_220018
Screenshot_20221003_225206
Screenshot_20221003_233858

My observations

  • icons without circle background looks better
  • "green icon" on "green area" looks quite readable
  • outline around icon shape is not necessary.
    • though I would consider using icon with outline for named objects displayed with a label because this label itself has a white outline and IMO the icon feels a bit "disconnected" to the text
  • I still really don't like the amount of garden and forest icons in some areas. It's an improvement, but IMO it still clutters the map a lot.

Reply to @biodranik

Let me summarize:

  1. Removing the forests icon is a no-go blocker. They are very important for orienteering.

We have different opinion on that. But that's ok. You clearly want to have some indication that there are trees on area. IMO adding subtle texture would be the best solution for it.

I don't think forest icons are that usable because, IMO, from user perspective they can be positioned at seemingly random places where there are not immediately visible and are actually quite hard to find.

This is especially annoying for bigger areas where I have zoom level where such icon is rendered, but the area is quite big so the icon is not displayed "at the edge" of area where I'm probably standing so to find this icon I need to pan the screen and actively search for this icon. And when I zoom out to see the whole shape of this big area the icon is disappearing because we don't show such icons on smaller zoom level. In practice the best solution to verify that this is a forest is to touch and hold to select the area.

That's why I think if we really want to make it clear that area is forest the textures is the only way. As an intermediate step we can also consider changing the color of forests to something else to make it more distinguish from parks and garden. The icon itself is not that useful.

  1. Carefully designing textures can help a lot (e.g. render them only when you zoomed in, in this case, some flowers may appear and make it clear and recognizable). But adding textures is a complex task.

So currently OMaps doesn't have any possibility to render textures similar to osm.org?

  1. Threshold idea won't work in many cases.

Actually, I gave a second though on this idea. My previous objection to threshold was that there are small named areas that should be displayed. But we can decide to always render icons for named areas and provide a threshold logic for those that doesn't have a name.

  1. Imagine someone wants to map a garden. But after adding it the icon... does not appear... Bad!

You mean adding garden in OM? Can we do that?

For me this whole PR and discussions is about areas. leisure=garden can be a node according to wiki but I see mapping it as an area is vastly more popular (taginfo).

I think we can simply decide to always render an icon for "node garden".

BTW, osm.org doesn't render anything for "node gardens", OM does :)

  1. Removing the circle but leaving the icon is the easiest and less destructive option at the moment.

Yep, but maybe we can consider again threshold option for one last time?

Summary. What I propose.

  1. Use icon without "circle backgrounds" for garden, park and forest

  2. (optionally) for consistency use "backgroundless" icon but with slight outline for named areas.

    Cons: this is very small styling change not worth the added complexity in styling rules and having additional icon files.

  3. Always render icons for named forest, garden and parks areas but for unnamed ones we should not render icon for really small areas (figure out some threshold)

What you think @biodranik?

I removed background from forest, park, garden icons. However, I was afraid the contrast of "dark green icon" on "light green area" might be not enough to distinguish for some people (currently the icons with background have small outline/border around the circle - white with 60% opacity) so I also tested version with similar outline around the shape of an icon without background. Here are the icons I used: - `landuse=garden` <img alt="garden-m" src="https://user-images.githubusercontent.com/2056282/193806987-18db6d59-576f-4ba8-9fd1-0f7587a2ad40.svg" width=50 /> <img alt="garden-m-no-bg-outline" src="https://user-images.githubusercontent.com/2056282/193807007-4b168f0d-ace7-4f5c-b35d-e1d3c659d8e0.svg" width=50 /> <img alt="garden-m-no-bg" src="https://user-images.githubusercontent.com/2056282/193807023-c31659ec-6577-4c01-967a-08f04d26168a.svg" width=50 /> - `leisure=park` <img alt="park-m" src="https://user-images.githubusercontent.com/2056282/193807280-e93ef0be-c199-484d-aad8-b7e048d72d7b.svg" width=50 /> <img alt="park-m-no-bg-outline" src="https://user-images.githubusercontent.com/2056282/193807285-f939f93e-e70c-4108-a0c2-bffc1ca10c35.svg" width=50 /> <img alt="park-m-no-bg" src="https://user-images.githubusercontent.com/2056282/193807263-11aca8b8-1550-4baf-9813-11b0a6b42d7c.svg" width=50 /> - `landuse=forest` <img alt="nparkF-m" src="https://user-images.githubusercontent.com/2056282/193807418-1051389b-dfa7-480e-a3b5-f137b0ec57fb.svg" width=50 /> <img alt="nparkF-m-no-bg-outline" src="https://user-images.githubusercontent.com/2056282/193807421-31d76233-1b78-4f3c-b246-7cc7ed989919.svg" width=50 /> <img alt="nparkF-m-no-bg" src="https://user-images.githubusercontent.com/2056282/193807415-20f3ec3c-ef70-4f8a-8c70-e8d0745dad21.svg" width=50 /> # Screenshots (I recommend to zoom out a bit in browser to view it 😉 ) 1. [52.431639, 16.943598](https://www.openstreetmap.org/#map=19/52.43164/16.94407) lots of small gardens ![Screenshot_20221003_203259](https://user-images.githubusercontent.com/2056282/193808889-35692579-3611-4713-a126-dc4d541ee057.png) ![Screenshot_20221003_224256](https://user-images.githubusercontent.com/2056282/193808928-b01e3116-01a1-4651-b0b8-6a57fa00a07a.png) ![Screenshot_20221003_233046](https://user-images.githubusercontent.com/2056282/193808973-476f0d51-9349-4a9d-b243-e528ceb5a7f2.png) 2. [52.409347, 16.913122](https://www.openstreetmap.org/#map=19/52.40935/16.91359) another small gardens, on zoom 18, this areas were even smaller then the icon ![Screenshot_20221003_203559](https://user-images.githubusercontent.com/2056282/193809007-7cc781c9-d4aa-4542-8577-691af688c8d0.png) ![Screenshot_20221003_231138](https://user-images.githubusercontent.com/2056282/193809048-8846898a-fcc9-44f2-bed7-c35e2bb5fdd4.png) ![Screenshot_20221003_233234](https://user-images.githubusercontent.com/2056282/193809063-77785ba1-7677-4b94-9d54-339574cc4282.png) 3. [52.451107, 16.942539](https://www.openstreetmap.org/#map=19/52.45111/16.94254) here we can see small forest areas ![Screenshot_20221003_205013](https://user-images.githubusercontent.com/2056282/193809138-cec47ea2-08fc-4df9-bdb9-e1a99e754f6b.png) ![Screenshot_20221003_224604](https://user-images.githubusercontent.com/2056282/193809161-d778d2e0-d285-4d1a-8b9a-5936c70f0031.png) ![Screenshot_20221003_233335](https://user-images.githubusercontent.com/2056282/193809178-57189807-44d0-48c0-8f58-cd48b9ff4818.png) 4. [52.409288, 16.931286](https://www.openstreetmap.org/#map=19/52.409288/16.931286) garden with a name ![Screenshot_20221003_205811](https://user-images.githubusercontent.com/2056282/193809941-93bf81a6-c4ac-444c-8bc1-fb369d8b3601.png) ![Screenshot_20221003_224728](https://user-images.githubusercontent.com/2056282/193809245-10d08200-53b2-41fc-81f8-ea8ffed71945.png) ![Screenshot_20221003_233434](https://user-images.githubusercontent.com/2056282/193809280-c6c7c407-05f7-4d65-a90b-dc953cc36592.png) 5. [53.504118, 19.749705](https://www.openstreetmap.org/#map=19/53.504118/19.749705) "node garden" - garden mapped with a single node, not as area ![Screenshot_20221003_210113](https://user-images.githubusercontent.com/2056282/193809317-f0e9a3c4-9439-41cf-873c-37105ad8d1a3.png) ![Screenshot_20221003_224946](https://user-images.githubusercontent.com/2056282/193809340-fd1fd451-9c58-41be-8583-1d55a8dbc899.png) ![Screenshot_20221003_233631](https://user-images.githubusercontent.com/2056282/193809387-af930664-8a7f-4e64-96f9-c289d516c390.png) 6. [52.89343,16.55155](https://www.openstreetmap.org/#map=19/52.89343/16.55155) some named park ![Screenshot_20221003_215233](https://user-images.githubusercontent.com/2056282/193809601-f0647d89-eddd-4e46-84ef-877a9757d7f9.png) ![Screenshot_20221003_225051](https://user-images.githubusercontent.com/2056282/193809615-a6b8a4e8-0c7b-4f41-ba8e-27d4004629d4.png) ![Screenshot_20221003_233720](https://user-images.githubusercontent.com/2056282/193809634-c8210a6b-4ca4-41ba-8730-f71bb44306ac.png) 7. [52.45625,16.93457](https://www.openstreetmap.org/#map=17/52.45625/16.93457) some named `landuse=forest` and `leisure=nature_reserve` ![Screenshot_20221003_220018](https://user-images.githubusercontent.com/2056282/193809662-927256ea-537f-41c0-a0b7-447aa9cc4303.png) ![Screenshot_20221003_225206](https://user-images.githubusercontent.com/2056282/193809688-af656c88-883e-45b6-ad31-faeebbdbe687.png) ![Screenshot_20221003_233858](https://user-images.githubusercontent.com/2056282/193809718-0cf1a4f8-414d-464b-8440-523c5003d370.png) # My observations - icons without circle background looks better - "green icon" on "green area" looks quite readable - outline around icon shape is not necessary. - though I would consider using icon with outline for named objects displayed with a label because this label itself has a white outline and IMO the icon feels a bit "disconnected" to the text - I *still* really **don't like** the amount of garden and forest icons in some areas. It's an improvement, but IMO it still clutters the map a lot. # Reply to @biodranik > Let me summarize: > > 1. Removing the forests icon is a no-go blocker. They are very important for orienteering. We have different opinion on that. But that's ok. You clearly want to have some indication that there are trees on area. IMO adding *subtle* texture would be the best solution for it. I don't think forest icons are that usable because, IMO, from user perspective they can be positioned at seemingly random places where there are not immediately visible and are actually quite hard to find. This is especially annoying for bigger areas where I have zoom level where such icon is rendered, but the area is quite big so the icon is not displayed "at the edge" of area where I'm probably standing so to find this icon I need to pan the screen and actively search for this icon. And when I zoom out to see the whole shape of this big area the icon is disappearing because we don't show such icons on smaller zoom level. In practice the best solution to verify that this is a forest is to touch and hold to select the area. That's why I think if we really want to make it clear that area is forest the textures is the only way. As an intermediate step we can also consider changing the color of forests to something else to make it more distinguish from parks and garden. The icon itself is not that useful. > 2. Carefully designing textures can help a lot (e.g. render them only when you zoomed in, in this case, some flowers may appear and make it clear and recognizable). But adding textures is a complex task. So currently OMaps doesn't have any possibility to render textures similar to osm.org? > 3. Threshold idea won't work in many cases. Actually, I gave a second though on this idea. My previous objection to threshold was that there are small named areas that should be displayed. But we can decide to always render icons for named areas and provide a threshold logic for those that doesn't have a name. > 4. Imagine someone wants to map a garden. But after adding it the icon... does not appear... Bad! You mean adding garden in OM? Can we do that? For me this whole PR and discussions is about areas. `leisure=garden` can be a node according to [wiki](https://wiki.openstreetmap.org/wiki/Tag:leisure%3Dgarden) but I see mapping it as an area is vastly more popular ([taginfo](https://taginfo.openstreetmap.org/tags/leisure=garden#overview)). I think we can simply decide to always render an icon for "node garden". BTW, osm.org doesn't render anything for "node gardens", OM does :) > 5. Removing the circle but leaving the icon is the easiest and less destructive option at the moment. Yep, but maybe we can consider again threshold option for one last time? # Summary. What I propose. 1. Use icon without "circle backgrounds" for garden, park and forest 2. (optionally) for consistency use "backgroundless" icon but with slight outline for named areas. Cons: this is very small styling change not worth the added complexity in styling rules and having additional icon files. 3. Always render icons for named forest, garden and parks areas but for unnamed ones we should not render icon for really small areas (figure out some threshold) What you think @biodranik?
biodranik commented 2022-10-06 22:03:08 +00:00 (Migrated from github.com)

First, thank you for the detailed investigation and comparison, that's a lot of work! I really appreciate your approach to solving issues :)

Second, I mostly agree with your observations:

  1. Circles should be removed.
  2. Outline is not needed.
  3. Optional outline where the text also has an outline will look better (but more bright white outline, similar to the text's). It would be great if we can programmatically style/outline icons. That is planned after adding SVG support into the core and will make styling very easy.
  4. Map cluttering is an issue, another possible option is to use smaller icons on some zooms or for some cases.
  5. Forest icons help to easily connect the green color with the forest. Otherwise, there could be several different shades of green areas nearby without a clear distinction.
  6. But an even better option is to use textures, agree? It will allow removing the forest icon :) So now we can focus on smaller changes that will improve the map until the really important improvement comes. Textures can be used on different zoom levels to avoid cluttering.
  7. Threshold requires some coding, and it won't work consistently if we draw node icons and filter area icons using a threshold. It would be more efficient to code textures :)

What do you think about these priorities/tasks?

  1. Test night mode icons without circles
  2. Implement icons without circles everywhere.
  3. Try outlined icons for outlined texts if it's easy (but make an outline more white).
  4. Focus on textures support. I think we can add some textures manually at the moment in the code by defining them as a bitmap matrix of zeroes and ones and adding some type checks for rendering.
First, thank you for the detailed investigation and comparison, that's a lot of work! I really appreciate your approach to solving issues :) Second, I mostly agree with your observations: 1. Circles should be removed. 2. Outline is not needed. 3. Optional outline where the text also has an outline will look better (but more bright white outline, similar to the text's). It would be great if we can programmatically style/outline icons. That is planned after adding SVG support into the core and will make styling very easy. 4. Map cluttering is an issue, another possible option is to use smaller icons on some zooms or for some cases. 5. Forest icons help to easily connect the green color with the forest. Otherwise, there could be several different shades of green areas nearby without a clear distinction. 6. But an even better option is to use textures, agree? It will allow removing the forest icon :) So now we can focus on smaller changes that will improve the map until the really important improvement comes. Textures can be used on different zoom levels to avoid cluttering. 7. Threshold requires some coding, and it won't work consistently if we draw node icons and filter area icons using a threshold. It would be more efficient to code textures :) What do you think about these priorities/tasks? 1. Test night mode icons without circles 2. Implement icons without circles everywhere. 3. Try outlined icons for outlined texts if it's easy (but make an outline more white). 4. Focus on textures support. I think we can add some textures manually at the moment in the code by defining them as a bitmap matrix of zeroes and ones and adding some type checks for rendering.
mpawelski commented 2022-10-07 11:06:43 +00:00 (Migrated from github.com)

First, thank you for the detailed investigation and comparison, that's a lot of work! I really appreciate your approach to solving issues :)

thanks a lot for the kind words :)

Second, I mostly agree with your observations:

  1. Circles should be removed.

  2. Outline is not needed.

  3. Optional outline where the text also has an outline will look better (but more bright white outline, similar to the text's).
    It would be great if we can programmatically style/outline icons.

For icons without "holes" (like forest, park) it was actually easy to add this outline to SVG: just add a "stroke" with proper width to the "path" element and set "paint-order" to "stroke fill marker".

However, for icon with hole inside (like garden) I didn't like the outline inside the "flower" circle so in the end I removed it manually ("Convert strokes to path" in Inkscape, and did some manual modifications to get rid of outline inside).

So, I'm not sure how hard it would be to add such outlines programmatically, but I agree this would be great if we do it :)

BTW, I had to convert strokes to path even for forest and park icon in the end because "./tools/unix/generate_symbols.sh" didn't generate proper "symbols.png" file from them. Just FYI.

That is planned after adding SVG support into the core and will make styling very easy.

Yep, I also read about this plan somewhere. That's why I was a bit hesitant about providing additional icons with outline because I guess the point is actually to minimize the amount of icons we have to one "basic" icon and programmatically add "circle background" or "outline" when necessary.

But I guess adding this "core SVG support" is a bit long term plan, and in the meantime we can live with many icons variants.

  1. Map cluttering is an issue, another possible option is to use smaller icons on some zooms or for some cases.

I could experiment with some smaller icons. But I'm actually more leaning toward this threshold approach and setting this threshold to pretty low to get rid of this icons abundance in places where there are many really small green areas like in example 1 and 2. At least for bigger areas this icons are not that much distracting after removing background from them.

  1. Forest icons help to easily connect the green color with the forest. Otherwise, there could be several different shades of green areas nearby without a clear distinction.

  2. But an even better option is to use textures, agree? It will allow removing the forest icon :) So now we can focus on smaller changes that will improve the map until the really important improvement comes. Textures can be used on different zoom levels to avoid cluttering.

I agree, textures seems to solve most of our problems (get rid of "icons abundance" while making it clear what the area actually represents). We would just need to make sure this textures are informative, yet subtle to make the map feel "clear". But this is a normal "styling nitpicking" ;)

7. Threshold requires some coding,

I was hoping threshold approach could be achieved just by changing some .mappcss files. I see styles like that in the project:

area|z10-[landuse=reservoir][bbox_area>=4000000],
area|z10-[natural=water][bbox_area>=80000000],

and was hoping to use this "bbox_area" feature. Won't it work?

and it won't work consistently if we draw node icons and filter area icons using a threshold. It would be more efficient to code textures :)

Yep, it won't be consistent but at least there will still be some indication there there is some "green stuff" (icon for node, green area for small areas). Node gardens are not that popular anyway (10k vs 940k). I agree having textures would be the best.

What do you think about these priorities/tasks?

  1. Test night mode icons without circles

  2. Implement icons without circles everywhere.

  3. Try outlined icons for outlined texts if it's easy (but make an outline more white).

  4. Focus on textures support. I think we can add some textures manually at the moment in the code by defining them as a bitmap matrix of zeroes and ones and adding some type checks for rendering.

1,2,3 - Sure, I will check the night mode and experiment with outline icons for outlined text.

Will post the result here when I'll have time to work on it!

4 - Sounds good. But I will leave this to someone else. I guess I'm not that versed in C++ and the OM codebase yet to even approach it.

I was also thinking about testing "threshold" or "smaller icons" approach but I think I will now focus on 1,2,3 tasks because this will be an improvement anyway and I want to see it in OM sooner than later :)

> First, thank you for the detailed investigation and comparison, that's a lot of work! I really appreciate your approach to solving issues :) thanks a lot for the kind words :) > Second, I mostly agree with your observations: > >1. Circles should be removed. > >2. Outline is not needed. > >3. Optional outline where the text also has an outline will look better (but more bright white outline, similar to the text's). > **It would be great if we can programmatically style/outline icons.** For icons without "holes" (like forest, park) it was actually easy to add this outline to SVG: just add a "stroke" with proper width to the "path" element and set "paint-order" to "stroke fill marker". However, for icon with hole inside (like garden) I didn't like the outline inside the "flower" circle so in the end I removed it manually ("Convert strokes to path" in Inkscape, and did some manual modifications to get rid of outline inside). So, I'm not sure how hard it would be to add such outlines programmatically, but I agree this would be great if we do it :) BTW, I had to convert strokes to path even for forest and park icon in the end because "./tools/unix/generate_symbols.sh" didn't generate proper "symbols.png" file from them. Just FYI. > That is planned after adding SVG support into the core and will make styling very easy. Yep, I also read about this plan somewhere. That's why I was a bit hesitant about providing additional icons with outline because I guess the point is actually to minimize the amount of icons we have to one "basic" icon and programmatically add "circle background" or "outline" when necessary. But I guess adding this "core SVG support" is a bit long term plan, and in the meantime we can live with many icons variants. >4. Map cluttering is an issue, another possible option is to use smaller icons on some zooms or for some cases. I could experiment with some smaller icons. But I'm actually more leaning toward this threshold approach and setting this threshold to pretty low to get rid of this icons abundance in places where there are many really small green areas like in example 1 and 2. At least for bigger areas this icons are not _that much_ distracting after removing background from them. >5. Forest icons help to easily connect the green color with the forest. Otherwise, there could be several different shades of green areas nearby without a clear distinction. > >6. But an even better option is to use textures, agree? It will allow removing the forest icon :) So now we can focus on smaller changes that will improve the map until the really important improvement comes. Textures can be used on different zoom levels to avoid cluttering. I agree, textures seems to solve most of our problems (get rid of "icons abundance" while making it clear what the area actually represents). We would just need to make sure this textures are informative, yet subtle to make the map feel "clear". But this is a normal "styling nitpicking" ;) > > 7. Threshold requires some coding, I was hoping threshold approach could be achieved just by changing some .mappcss files. I see styles like that in the project: ``` area|z10-[landuse=reservoir][bbox_area>=4000000], area|z10-[natural=water][bbox_area>=80000000], ``` and was hoping to use this "bbox_area" feature. Won't it work? > and it won't work consistently if we draw node icons and filter area icons using a threshold. It would be more efficient to code textures :) Yep, it won't be consistent but at least there will still be some indication there there is some "green stuff" (icon for node, green area for small areas). Node gardens are not that popular anyway ([10k vs 940k](https://taginfo.openstreetmap.org/tags/leisure=garden#overview)). I agree having textures would be the best. > > What do you think about these priorities/tasks? > >1. Test night mode icons without circles > >2. Implement icons without circles everywhere. > >3. Try outlined icons for outlined texts if it's easy (but make an outline more white). > >4. Focus on textures support. I think we can add some textures manually at the moment in the code by defining them as a bitmap matrix of zeroes and ones and adding some type checks for rendering. 1,2,3 - Sure, I will check the night mode and experiment with outline icons for outlined text. Will post the result here when I'll have time to work on it! 4 - Sounds good. But I will leave this to someone else. I guess I'm not that versed in C++ and the OM codebase yet to even approach it. I was also thinking about testing "threshold" or "smaller icons" approach but I think I will now focus on 1,2,3 tasks because this will be an improvement anyway and I want to see it in OM sooner than later :)
biodranik commented 2022-10-07 18:12:54 +00:00 (Migrated from github.com)

BTW, I had to convert strokes to path even for forest and park icon in the end because "./tools/unix/generate_symbols.sh" didn't generate proper "symbols.png" file from them. Just FYI.

What kind of error did you get? That should be fixed. What is your version of Qt in the system?

But I guess adding this "core SVG support" is a bit long term plan, and in the meantime we can live with many icons variants.

Right.

and was hoping to use this "bbox_area" feature. Won't it work?

You may try it. But it is not the best approach. The filter should not filter based on one feature's area, but filter based on the number of nearby icons. And that's a more complex task.

Regarding textures, you can help by providing a pattern matrix with 0 (transparent pixel) and 1 (non-transparent one) for some popular patterns. Something like that: https://www.pixilart.com/draw/texture-694e374ce4

> BTW, I had to convert strokes to path even for forest and park icon in the end because "./tools/unix/generate_symbols.sh" didn't generate proper "symbols.png" file from them. Just FYI. What kind of error did you get? That should be fixed. What is your version of Qt in the system? > But I guess adding this "core SVG support" is a bit long term plan, and in the meantime we can live with many icons variants. Right. > and was hoping to use this "bbox_area" feature. Won't it work? You may try it. But it is not the best approach. The filter should not filter based on one feature's area, but filter based on the number of nearby icons. And that's a more complex task. Regarding textures, you can help by providing a pattern matrix with 0 (transparent pixel) and 1 (non-transparent one) for some popular patterns. Something like that: https://www.pixilart.com/draw/texture-694e374ce4
mpawelski commented 2022-10-07 22:43:32 +00:00 (Migrated from github.com)

BTW, I had to convert strokes to path even for forest and park icon in the end because "./tools/unix/generate_symbols.sh" didn't generate proper "symbols.png" file from them. Just FYI.

What kind of error did you get? That should be fixed. What is your version of Qt in the system?

I'm using WSL on windows. this is what qmake --version command tells me:

QMake version 3.1
Using Qt version 5.15.3 in /usr/lib/x86_64-linux-gnu

No error. Just different looking icon in "symbols.png".

Ok, I did some more test and concluded that SVG strokes are indeed recognized and rendered in "symbols.png", but the paint-order="stroke fill markers" is not recognized (so it looks as if the default "fill stroke marker" order was taken).

But this might be limitation of QT. It says that it support "SVG 1.2 Tiny." and I don't see any "paint-order" mentioned there so it just might be not supported.

Examples

For this svg (original file from the project with manually removed background "by hand" by removing <circle> element):
nparkF-m
I get this ".\data\resources-xxxhdpi_clear\symbols.png" file after running ./tools/unix/generate_symbols.sh:
symbols
When I modify svg and add stroke="#fff" stroke-opacity=".6" stroke-width="2" paint-order="stroke fill markers" to it to get this svg:
nparkF-m
after running ./tools/unix/generate_symbols.sh i get this ".\data\resources-xxxhdpi_clear\symbols.png" file:
symbols

It looks that the stroke is also rendered "inside" the shape just like in this svg (like previous but without paint-order="stroke fill markers":):

nparkF-m

For which the same "symbols.png" is generated:
symbols

and was hoping to use this "bbox_area" feature. Won't it work?

You may try it. But it is not the best approach. The filter should not filter based on one feature's area, but filter based on the number of nearby icons. And that's a more complex task.

Yep, this would be filtering icons by "POIs density" mentioned by vng. But maybe filtering just by this "bbox_area" will be good enough. I'll try to do some experiment with it.

> > BTW, I had to convert strokes to path even for forest and park icon in the end because "./tools/unix/generate_symbols.sh" didn't generate proper "symbols.png" file from them. Just FYI. > > What kind of error did you get? That should be fixed. What is your version of Qt in the system? I'm using WSL on windows. this is what ` qmake --version` command tells me: > QMake version 3.1 > Using Qt version 5.15.3 in /usr/lib/x86_64-linux-gnu No error. Just different looking icon in "symbols.png". Ok, I did some more test and concluded that SVG strokes are indeed recognized and rendered in "symbols.png", but the `paint-order="stroke fill markers"` is not recognized (so it looks as if the [default](https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/paint-order) "fill stroke marker" order was taken). But this might be limitation of QT. It [says](https://doc.qt.io/qt-6/svgrendering.html) that it support "[SVG 1.2 Tiny](http://www.w3.org/TR/SVGMobile12)." and I don't see any "paint-order" mentioned there so it just might be not supported. <details> <summary> Examples </summary> For this svg (original file from the project with manually removed background "by hand" by removing `<circle>` element): <img alt="nparkF-m" src="https://user-images.githubusercontent.com/2056282/194668934-46851e86-71f6-4a35-945a-cf1b794f49bc.svg" width=50/> I get this ".\data\resources-xxxhdpi_clear\symbols.png" file after running `./tools/unix/generate_symbols.sh`: ![symbols](https://user-images.githubusercontent.com/2056282/194669369-5ccc0118-5504-4dfa-af09-3f3f1ee02a8f.png) When I modify svg and add `stroke="#fff" stroke-opacity=".6" stroke-width="2" paint-order="stroke fill markers"` to it to get this svg: <img alt="nparkF-m" src="https://user-images.githubusercontent.com/2056282/194669514-0c872e1f-ca74-497e-b7a0-ec749d453f09.svg" width=50/> after running `./tools/unix/generate_symbols.sh` i get this ".\data\resources-xxxhdpi_clear\symbols.png" file: ![symbols](https://user-images.githubusercontent.com/2056282/194669844-5fd4e2d8-6770-404e-bc56-bc39239832a2.png) It looks that the stroke is also rendered "inside" the shape just like in this svg (like previous but without `paint-order="stroke fill markers"`:): <img alt="nparkF-m" src="https://user-images.githubusercontent.com/2056282/194670510-76b7e0e8-8e53-4b27-951e-3a27be27295f.svg" width=50/> For which the same "symbols.png" is generated: ![symbols](https://user-images.githubusercontent.com/2056282/194671758-61f0d282-2daf-452f-aa08-87bdaf4ccdbd.png) </details> --- > > and was hoping to use this "bbox_area" feature. Won't it work? > > You may try it. But it is not the best approach. The filter should not filter based on one feature's area, but filter based on the number of nearby icons. And that's a more complex task. Yep, this would be filtering icons by "POIs density" [mentioned](https://git.omaps.dev/organicmaps/organicmaps/pulls/3386#issuecomment-1246283178) by vng. But maybe filtering just by this "bbox_area" will be good enough. I'll try to do some experiment with it.
biodranik commented 2022-10-08 08:03:47 +00:00 (Migrated from github.com)

SVG 1.1 is a W3C Recommendation and forms the core of the current SVG developments in Qt.

Please try to use a compatible SVG format. It may be worth mentioning it in our documentation somewhere.

> SVG 1.1 is a W3C Recommendation and forms the core of the current SVG developments in Qt. Please try to use a compatible SVG format. It may be worth mentioning it in our documentation somewhere.
mpawelski commented 2022-10-10 21:02:11 +00:00 (Migrated from github.com)

ok, I did some work on it but first I want to ask about some strange behavior of changing label's color for some nature icons. It looks that on zoom up to 16 the label is green in day and night theme . After zoom 17+ it changes to dark label in day theme and white label in night theme.

Is it intentional? Should label just have the same color on all zooms? Here's how the video to show what I mean:

https://user-images.githubusercontent.com/2056282/194949677-a1df4d9b-e559-4178-9f4f-53b48d36ae5c.mp4

Looks like a bug in styles to me. First we define that it should be green, later in different file it's (probably accidentally) overriden for zoom 17+.

I'm asking because currently I prepared icons for night theme with white outline but I'm thinking if it would be better to fix this label styling to always have green color with dark outline in night theme and change the outline to also be black (so icon will be green with dark outline, just like the text). What do you guys think?

Anyway here are screens with what I have so far (with white outline in dark mode):

Screens

1L
1N
2L
2N
3L
3N
4L
4N
5L
5N

ok, I did some work on it but first I want to ask about some strange behavior of changing label's color for _some_ nature icons. It looks that on zoom up to 16 the label is green in day and night theme . After zoom 17+ it changes to dark label in day theme and white label in night theme. Is it intentional? Should label just have the same color on all zooms? Here's how the video to show what I mean: https://user-images.githubusercontent.com/2056282/194949677-a1df4d9b-e559-4178-9f4f-53b48d36ae5c.mp4 Looks like a bug in styles to me. [First](https://github.com/organicmaps/organicmaps/blob/24e5164/data/styles/clear/include/Basemap_label.mapcss#L460-L461) we define that it should be green, [later](https://github.com/organicmaps/organicmaps/blob/master/data/styles/clear/include/Icons.mapcss#L1520) in different file it's (probably accidentally) overriden for zoom 17+. I'm asking because currently I prepared icons for night theme with white outline but I'm thinking if it would be better to fix this label styling to always have green color with dark outline in night theme and change the outline to also be black (so icon will be green with dark outline, just like the text). What do you guys think? Anyway here are screens with what I have so far (with white outline in dark mode): <details> <summary> Screens</summary> ![1L](https://user-images.githubusercontent.com/2056282/194951319-175fea0a-f242-4b17-9228-55bae7220998.png) ![1N](https://user-images.githubusercontent.com/2056282/194951325-c998b883-51ff-4a3f-962a-4968bdc8726b.png) ![2L](https://user-images.githubusercontent.com/2056282/194951331-f1835049-644f-4570-8d55-69b68b7e4e47.png) ![2N](https://user-images.githubusercontent.com/2056282/194951340-1e63e17f-7f28-4775-acaa-2973329b6c75.png) ![3L](https://user-images.githubusercontent.com/2056282/194951346-89ec8c95-0593-4bdf-8c0b-a31bef6c63e1.png) ![3N](https://user-images.githubusercontent.com/2056282/194951350-b8ba1d00-04f3-4a99-853f-4080d1c904d3.png) ![4L](https://user-images.githubusercontent.com/2056282/194951354-549aba91-1ea9-4e38-9157-01626469a7d0.png) ![4N](https://user-images.githubusercontent.com/2056282/194951360-150db37d-807e-4931-bfab-48551149f0ec.png) ![5L](https://user-images.githubusercontent.com/2056282/194951364-584a5d83-1ad0-4e79-993a-4b111a4323b5.png) ![5N](https://user-images.githubusercontent.com/2056282/194951367-0e909e2d-6582-408a-9713-2976b1e742c4.png) </details>
biodranik commented 2022-10-10 21:10:43 +00:00 (Migrated from github.com)

Changing labels' color looks like a bug. You can confirm it by checking git history using git blame locally or here, on Github.

Changing labels' color looks like a bug. You can confirm it by checking git history using git blame locally or here, on Github.
mpawelski commented 2022-10-11 19:49:14 +00:00 (Migrated from github.com)

Changing labels' color looks like a bug. You can confirm it by checking git history using git blame locally or here, on Github.

It's hard to tell anything from git history. this node|z17-[leisure], line is from refactoring commit by @Zverik that introduced this file. Probably the bug was introduced by accident in this big refactor 6 years ago.

I will try to fix it. The easiest fix would probably be just replacing this node|z17-[leisure] with all the leisure=value tag we support (we can take it from "mapcss-mapping.csv" right?), except the nature ones where it breaks the styling (leisure=park,anduse=forest and leisure=garden) . @biodranik do you think it's acceptable fix?

It would mean that if we add new leisure tag support then we need to add styling rule for it. But I think it's acceptable, you need to test how new POI is rendered anyway.

The other approach would probably require big refactoring of styles. Analyzing what other styles without value (like [leisure], [sport], etc) can override previous style with a value (like [leisure=garden]) and moving this styles first. Sound like a lot of work, not sure if it's worth it.

> Changing labels' color looks like a bug. You can confirm it by checking git history using git blame locally or here, on Github. It's hard to tell anything from git history. this `node|z17-[leisure],` line is from refactoring [commit](https://git.omaps.dev/organicmaps/organicmaps/commit/f40a61a1242fc22e69a66dbd89f401a366a709e2) by @Zverik that introduced this file. Probably the bug was introduced by accident in this big refactor 6 years ago. I will try to fix it. The easiest fix would probably be just replacing this `node|z17-[leisure]` with all the `leisure=value` tag we support (we can take it from "mapcss-mapping.csv" right?), _except_ the nature ones where it breaks the styling (`leisure=park`,`anduse=forest` and `leisure=garden`) . @biodranik do you think it's acceptable fix? It would mean that if we add new `leisure` tag support then we need to add styling rule for it. But I think it's acceptable, you need to test how new POI is rendered anyway. The other approach would probably require big refactoring of styles. Analyzing what other styles without value (like [`leisure]`, [`sport`], etc) can override previous style with a value (like [`leisure=garden]`) and moving this styles first. Sound like a lot of work, not sure if it's worth it.
biodranik commented 2022-10-11 21:56:36 +00:00 (Migrated from github.com)

@vng can you please suggest something?

@vng can you please suggest something?
vng commented 2022-10-13 12:26:06 +00:00 (Migrated from github.com)

79 commits ahead? Something is wrong here, please, make correct rebase above master.

79 commits ahead? Something is wrong here, please, make correct rebase above master.
mpawelski commented 2022-10-13 12:42:34 +00:00 (Migrated from github.com)

While we're deciding how to fix this labels styling I would like to get some feedback about styling and help with crashing android app.

I changed white outline in dark mode to dark outline and fixed changing outline in label for leisure=park and leisure=garden to test how it will look.

Examples

Screenshot_20221013_134742
Screenshot_20221013_134804
Screenshot_20221013_135401
Screenshot_20221013_140000
Screenshot_20221013_140051

How do you like it? For me it looks quite good. The dark outlines are more subtle than in day theme (I didn't changed them since last time, you can see the day theme examples in my previous post) because of lower contrast.
But I don't want to make just the outline for dark theme bigger, because I want to keep day and night icons mostly the same, just with different color values. Though, I would also consider making garden outline a big bigger in day and night theme if you think it's not enough.

I fixed the mentioned node|z17-[leisure] line with the approach I described before (replaced it with [leisure=value] selectors of all the tags that we support, minus the conflicting nature ones, and minus the one handled in this Icons.mapcss file). If we want to be sure that these selectors for leisure=value are unnecessary then we would probably have to check it manually POI by POI.

However I have different problem. The application is crashing when I change the theme in android emulator. I changed just the styling files and added some icons so I have no idea how it affects android code. Maybe someone can help?

It crashes when I go back from settings menu to map after changing theme:
crash_when_changing_theme

logs:

2022-10-13 14:02:38.217 12853-12853/app.organicmaps.debug D/AndroidRuntime: Shutting down VM
2022-10-13 14:02:38.232 12853-12853/app.organicmaps.debug E/AndroidRuntime: FATAL EXCEPTION: main
    Process: app.organicmaps.debug, PID: 12853
    java.lang.IllegalStateException: Fragment MapButtonsController{6872b0f} (a85e2ada-b875-44a9-96a7-3ab5600d045b) not attached to a context.
        at androidx.fragment.app.Fragment.requireContext(Fragment.java:967)
        at com.mapswithme.maps.maplayer.MapButtonsController.updateMenuBadge(MapButtonsController.java:184)
        at com.mapswithme.maps.maplayer.MapButtonsController$1.onGlobalLayout(MapButtonsController.java:96)
        at android.view.ViewTreeObserver.dispatchOnGlobalLayout(ViewTreeObserver.java:1061)
        at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:3352)
        at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:2286)
        at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:8948)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1231)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1239)
        at android.view.Choreographer.doCallbacks(Choreographer.java:899)
        at android.view.Choreographer.doFrame(Choreographer.java:832)
        at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1214)
        at android.os.Handler.handleCallback(Handler.java:942)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loopOnce(Looper.java:201)
        at android.os.Looper.loop(Looper.java:288)
        at android.app.ActivityThread.main(ActivityThread.java:7898)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:936)
2022-10-13 14:02:38.280 12853-12853/app.organicmaps.debug I/Process: Sending signal. PID: 12853 SIG: 9

It's throwing error in MapButtonsController.java, look like pretty new code. However I don't get this crash on latest master, just in my branch (that's based on latest master). This is really confusing for me because my changes are basically in styling only.

79 commits ahead? Something is wrong here, please, make correct rebase above master.

@vng yep, sorry, I messed up a bit, should be good now. Now it's based on master and PR is to master (the previous @biodranik comment about merging it to beta was when this PR was about removing unnamed garden icons, but the code changed to just replacing icons to the one without circle background, so maybe it can be merged to master now?)

While we're deciding how to fix this labels styling I would like to get some feedback about styling and help with crashing android app. I changed white outline in dark mode to dark outline and fixed changing outline in label for `leisure=park` and `leisure=garden` to test how it will look. <details> <summary> Examples </summary> ![Screenshot_20221013_134742](https://user-images.githubusercontent.com/2056282/195594470-267a5a64-af94-4e47-a5db-d204f4926047.png) ![Screenshot_20221013_134804](https://user-images.githubusercontent.com/2056282/195594485-b75515d2-0712-4242-bf4b-07ecbf61a7f5.png) ![Screenshot_20221013_135401](https://user-images.githubusercontent.com/2056282/195594491-40044e1e-7974-4f5b-a52d-39702d5b43f3.png) ![Screenshot_20221013_140000](https://user-images.githubusercontent.com/2056282/195594505-080770b0-8570-4d61-a4bc-8385ba4e4bb0.png) ![Screenshot_20221013_140051](https://user-images.githubusercontent.com/2056282/195594517-52e8291f-5cb1-4260-814e-da2432d87e6e.png) </details> How do you like it? For me it looks quite good. The dark outlines are more subtle than in day theme (I didn't changed them since last time, you can see the day theme examples [in my previous post](https://git.omaps.dev/organicmaps/organicmaps/pulls/3386#issuecomment-1273816503)) because of lower contrast. But I don't want to make just the outline for dark theme bigger, because I want to keep day and night icons mostly the same, just with different color values. Though, I would also consider making garden outline a big bigger in day and night theme if you think it's not enough. I fixed the mentioned `node|z17-[leisure]` line with the approach I described before (replaced it with `[leisure=value]` selectors of all the tags that we support, minus the conflicting nature ones, and minus the one handled in this Icons.mapcss file). If we want to be sure that these selectors for `leisure=value` are unnecessary then we would probably have to check it manually POI by POI. However I have different problem. The application is crashing when I change the theme in android emulator. I changed just the styling files and added some icons so I have no idea how it affects android code. Maybe someone can help? It crashes when I go back from settings menu to map after changing theme: ![crash_when_changing_theme](https://user-images.githubusercontent.com/2056282/195595561-80b590a2-ee6a-4a27-ac77-a280c036c3bc.gif) logs: ``` 2022-10-13 14:02:38.217 12853-12853/app.organicmaps.debug D/AndroidRuntime: Shutting down VM 2022-10-13 14:02:38.232 12853-12853/app.organicmaps.debug E/AndroidRuntime: FATAL EXCEPTION: main Process: app.organicmaps.debug, PID: 12853 java.lang.IllegalStateException: Fragment MapButtonsController{6872b0f} (a85e2ada-b875-44a9-96a7-3ab5600d045b) not attached to a context. at androidx.fragment.app.Fragment.requireContext(Fragment.java:967) at com.mapswithme.maps.maplayer.MapButtonsController.updateMenuBadge(MapButtonsController.java:184) at com.mapswithme.maps.maplayer.MapButtonsController$1.onGlobalLayout(MapButtonsController.java:96) at android.view.ViewTreeObserver.dispatchOnGlobalLayout(ViewTreeObserver.java:1061) at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:3352) at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:2286) at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:8948) at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1231) at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1239) at android.view.Choreographer.doCallbacks(Choreographer.java:899) at android.view.Choreographer.doFrame(Choreographer.java:832) at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1214) at android.os.Handler.handleCallback(Handler.java:942) at android.os.Handler.dispatchMessage(Handler.java:99) at android.os.Looper.loopOnce(Looper.java:201) at android.os.Looper.loop(Looper.java:288) at android.app.ActivityThread.main(ActivityThread.java:7898) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:936) 2022-10-13 14:02:38.280 12853-12853/app.organicmaps.debug I/Process: Sending signal. PID: 12853 SIG: 9 ``` It's throwing error in _MapButtonsController.java_, look like pretty new code. However I don't get this crash on latest master, just in my branch (that's based on latest master). This is really confusing for me because my changes are basically in styling only. > 79 commits ahead? Something is wrong here, please, make correct rebase above master. @vng yep, sorry, I messed up a bit, should be good now. Now it's based on master and PR is to master (the previous @biodranik comment about merging it to beta was when this PR was about removing unnamed garden icons, but the code changed to just replacing icons to the one without circle background, so maybe it can be merged to master now?)
mpawelski commented 2022-10-13 12:54:01 +00:00 (Migrated from github.com)

ok, now, it's ready to review if you want to take a look...

Would be great if someone could test this branch if they also have crashes in android when changing theme.

ok, _now_, it's ready to review if you want to take a look... Would be great if someone could test this branch if they also have crashes in android when changing theme.
jimcarst (Migrated from github.com) approved these changes 2022-10-17 16:15:39 +00:00
LaoshuBaby commented 2022-10-17 16:31:36 +00:00 (Migrated from github.com)

I received review request for translation, but I haven't found where this PR change string?

I received review request for translation, but I haven't found where this PR change string?
rtsisyk commented 2022-10-18 06:29:46 +00:00
Owner

I received review request for translation, but I haven't found where this PR change string?

It looks like that PR touched all translations initially, but removed this change later. Sorry for the noise.

> I received review request for translation, but I haven't found where this PR change string? It looks like that PR touched all translations initially, but removed this change later. Sorry for the noise.
vng commented 2022-10-21 10:05:13 +00:00 (Migrated from github.com)

Will check later. Have some ideas how to fix this leisure

Will check later. Have some ideas how to fix this _leisure_
vng commented 2022-10-28 08:43:32 +00:00 (Migrated from github.com)

@mpawelski Updated your branch. Please, check that it's exactly what you wanted to achieve.

@mpawelski Updated your branch. Please, check that it's exactly what you wanted to achieve.
mpawelski commented 2022-10-29 11:28:20 +00:00 (Migrated from github.com)

@vng Great, your styles changes are better, you even fix my copy-paste error ("park-outline-outline-m.svg" 😅).

I checked the styles on desktop app and it works how I want. :) For me we can merge it.

Though i didn't check if my previously mentioned issues with crashing android app when changing theme is still occurring.

@vng Great, your styles changes are better, you even fix my copy-paste error ("park-outline-outline-m.svg" 😅). I checked the styles on desktop app and it works how I want. :) For me we can merge it. Though i didn't check if my [previously mentioned](https://git.omaps.dev/organicmaps/organicmaps/pulls/3386#issuecomment-1277549819) issues with crashing android app when changing theme is still occurring.
This repo is archived. You cannot comment on pull requests.
No reviewers
biodranik
jimcarst
No labels
Accessibility
Accessibility
Address
Address
Android
Android
Android Auto
Android Auto
Android Automotive (AAOS)
Android Automotive (AAOS)
API
API
AppGallery
AppGallery
AppStore
AppStore
Battery and Performance
Battery and Performance
Blocker
Blocker
Bookmarks and Tracks
Bookmarks and Tracks
Borders
Borders
Bug
Bug
Build
Build
CarPlay
CarPlay
Classificator
Classificator
Community
Community
Core
Core
CrashReports
CrashReports
Cycling
Cycling
Desktop
Desktop
DevEx
DevEx
DevOps
DevOps
dev_sandbox
dev_sandbox
Directions
Directions
Documentation
Documentation
Downloader
Downloader
Drape
Drape
Driving
Driving
Duplicate
Duplicate
Editor
Editor
Elevation
Elevation
Enhancement
Enhancement
Epic
Epic
External Map Datasets
External Map Datasets
F-Droid
F-Droid
Fonts
Fonts
Frequently User Reported
Frequently User Reported
Fund
Fund
Generator
Generator
Good first issue
Good first issue
Google Play
Google Play
GPS
GPS
GSoC
GSoC
iCloud
iCloud
Icons
Icons
iOS
iOS
Legal
Legal
Linux Desktop
Linux Desktop
Linux packaging
Linux packaging
Linux Phone
Linux Phone
Mac OS
Mac OS
Map Data
Map Data
Metro
Metro
Navigation
Navigation
Need Feedback
Need Feedback
Night Mode
Night Mode
NLnet 2024-06-281
NLnet 2024-06-281
No Feature Parity
No Feature Parity
Opening Hours
Opening Hours
Outdoors
Outdoors
POI Info
POI Info
Privacy
Privacy
Public Transport
Public Transport
Raw Idea
Raw Idea
Refactoring
Refactoring
Regional
Regional
Regression
Regression
Releases
Releases
RoboTest
RoboTest
Route Planning
Route Planning
Routing
Routing
Ruler
Ruler
Search
Search
Security
Security
Styles
Styles
Tests
Tests
Track Recording
Track Recording
Translations
Translations
TTS
TTS
UI
UI
UX
UX
Walk Navigation
Walk Navigation
Watches
Watches
Web
Web
Wikipedia
Wikipedia
Windows
Windows
Won't fix
Won't fix
World Map
World Map
No milestone
No project
No assignees
3 participants
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: organicmaps/organicmaps-tmp#3386
No description provided.
Delete branch "hide-icons-for-unnamed-gardens-forests-parks"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?