Better labels in general #37
Conversation
kent1D
commented
May 4, 2018
kent1D
commented
- better icons that should fix City icon size consistency #27
- better management of cities level, the use of capitals didn't work so well because of the SQL sorting (look at Villeurbanne / Lyon in France currently), should fix La comparaison sur la capitale ne peut fonctionner correctement #36
- better display of admin_level 4 regions / provinces :
- do not show labels for osm_id > 0 like in the main osm map, should fix Strange labels on regions #35
- show regions label earlier and in a better way
- some corrections on cities labels directions
- some corrections on countries sovereign code (UK is better understood than RU)
Other small corrections
Sov countries lowercase and smaller
…te.git into better_countries
…tter_countries
better the administrative capital, Porto-Novo.
- Black stars for cities with embassies. - White stars with black border for capital cities without french embassies
and cities are both squares : - Black square for cities with french presence - White square with black border for other cities
Changing shield-text-dx and shield-text-dy for towns because of the new icon Allow the capital cities without embassies to overlap also, like embassies (mainly for Benin, Cotonou and Porto-Novo) Reduce the shield-size of embassies, capitals and intermediate a bit for small zooms to prepare the visibility of the regions soon. On zoom 9 and 10 make bigger the shield size for all
Smaller size on zoom 6 - 8 Bigger wrapping to allow more visibility Add a label position tolerance, label of provinces are not POIs, their placement can be moved a bit At zoom 9 : Bigger size for font and bold font, really better visibility
#place[type='city'] and #place[type='town'] without shield should begin after zoom 10 Only show villages at zoom >= 11. Avoid to mix label with points and labels with texts for human settlements. We don't really need to show them up before zomm 11. Better size at different zoom levels
There was a problem hiding this comment.
Hey amigo, sorry for the delay for the review :(
It's a bit too broad for my taste: when I see a PR with 25 commits and 17 changed files and labels plus admin boundary affected, I put a flag "needs calm time to review properly" on the task and it immediately move down on the list :(
The other issue is that it makes a whole pack to be merged or not merged all in once, while some of the changes here are clearly needed, while I think some others need more thinking.
Basically, if I have a quick look on the dataset currently loaded on my machine, I can see the labels changes need more thinking. At least for two reasons:
- I see more cut labels (I haven't tracked down everything now so I'm not sure why, certainly margin missing and such)
- bigger labels also means less room for other infos
- not specific to this PR, but even with the current changes, I keep a bit dissatisfied about the icons consistency (size and impact)
Two examples at zoom 10 on IDF to illustrate my remarks:
Not sure how to move forward from here and how I can help.
Best suggestion I have is to extract everything but the labels changes in smaller PRs, so we can then focus on the tricky part more easily.
Thoughts? :) (Don't hesitate to tell me what I can do to help and make your task easier)
| shield-size: 11; | ||
| shield-face-name: @regular; | ||
| shield-halo-radius: 1; | ||
| shield-wrap-width: 50; | ||
| shield-fill: @town_text; | ||
| shield-halo-fill: @halo; | ||
| shield-placement-type: simple; | ||
| shield-placements: 'NE,SW,NW,SE,E,W'; | ||
| shield-placements: 'NE,SW,NW,SE,E,W,N,S'; |
There was a problem hiding this comment.
A shield plus a north or south position does not seem right to me: the icon will end up in the middle of the name, so the name will likely appear as related to another icon on crowded areas.
| } | ||
| [type='intermediate'], [type='embassy'], [type='capital'] { | ||
| text-face-name: @bold; | ||
| text-size:16; |
| text-size: 15; | ||
| text-size: 16; | ||
| } | ||
| [type='intermediate'], [type='embassy'], [type='capital'] { |
There was a problem hiding this comment.
Seems we end up with two rules for this no?
| @@ -325,11 +326,12 @@ Layer: | |||
| geometry, '{lang}' as lang, capital, | |||
| CASE WHEN type='city' THEN 1 WHEN type='town' THEN 2 ELSE 100 END AS prio, | |||
| CASE WHEN type IN ('hamlet', 'suburb', 'isolated_dwelling', 'neighbourhood', 'allotments', 'city_block') THEN 'minor' ELSE type END AS type, | |||
| population, COALESCE(NULLIF(name{lang}, ''), name) as name | |||
| CASE WHEN capital='yes' THEN 1 WHEN capital IN ('4','5','6','7','8','9','10') THEN capital::INT ELSE 100 END AS capital, | |||
There was a problem hiding this comment.
Seems we have one extra "capital" column here ;)
| <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 6 6"><path class="st0" fill="#FFF" stroke="#525252" d="M.5.5h5v5h-5v-5z"/></svg> |
There was a problem hiding this comment.
Seems its white filled now, not sure it's on purpose.
| } | ||
| [zoom>=10] { | ||
| text-size: 14; | ||
| text-face-name: @bold; |
| @@ -338,7 +340,8 @@ Layer: | |||
| FROM | |||
| osm_admin | |||
| WHERE | |||
| admin_level = 3 OR admin_level = 4 | |||
| (admin_level = 3 OR admin_level = 4) | |||
| AND osm_id < 0 | |||
There was a problem hiding this comment.
DB indexes will need an update I think.
Maybe it's time to create a generalized_table instead? I guess it's a quite small dataset given those filters.
|
I will close this PR asap and do it step by step ;) |
