-
Notifications
You must be signed in to change notification settings - Fork 819
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
updates to placename rendering #2314
updates to placename rendering #2314
Conversation
way, | ||
name, | ||
CASE | ||
WHEN population ~ '^-?\d+(\.\d+)?$' THEN population::NUMERIC ELSE 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest to use the following code instead:
WHEN (population ~ '^[0-9]{1,8}$') THEN population::INTEGER
ELSE 0
Reasons:
- Tagging: Negative population values and non-integer floating point population count are not usefull values for the population tag. I think they should not be considered.
- Security: The digit limit
{1,8}
avoids SQL errors for extremely long (wrong) values like “54651654564564564564564564564564179875464661646”. Indeed this is less an issue with your code because you use NUMERIC and not INTEGER, but I would feel more comfortable when the digit count is limited. - Consistency: This is the way it is done in placenames-medium
Especially the fact that capitals get priority over state names is a big advance. 😊
|
I notice at z4 Belfast is shown as a capital when it is not a country capital: https://www.openstreetmap.org/node/1418701024 - I assume admin_level should be added to the node? Although you should probably check for its existence before rendering it. I agree on no capitals at z3, and also applying some logic to showing capitals at z4 - places like Luxembourg, Andorra, Saint Helier etc. are too small to be shown. |
@boothym Belfast has |
I have made some updates:
There are still fewer place names rendered around z6 than I would expect. @sommerluk I think we should fine-tune state labels afterwards if necessary. |
A few observations:
|
Note to myself: experiment with marking the capitals also after z7 (e.g. underline), ref #2222 |
capital=4 |
sent from a phone
admin_levels on place nodes are not what is documented or done, there's rather capital=yes for country capitals and capital= for other principal towns/cities. |
sent from a phone
+1, we should rather push for place polygons for these instead of ne shapefiles |
It's not documented but done: 83% of all capital=* have admin_level |
sent from a phone
right, there are people who like this redundancy, but 96,5% of all capital=* are admin level numbers, unlike the former, this is documented and it is still significantly more: |
The question about which tagging is appropriate for country capitals is valid. I forgot that I ported this code from a fork of mine where I focussed on what worked for rendering and not on tagging policy. Since what we will render will likely become the de-facto standard here it is worth reflecting about it. Currently using only On the other hand, this style tried to avoid imposing tagging decisions on the community but to instead follow what is being used. Unfortunately, the wiki proposal is not in good shape and the wiki page is not good either. |
2016-08-31 19:59 GMT+02:00 Michael Glanznig [email protected]:
I can't completely understand you here. 100% are using capital=*, the very |
... but general convention, as you found out is capital=yes + admin_level=2. So I don't think using it amounts to "imposing a tagging decision". |
Great, for me it's important update for mid-zoom! I don't understand why Rome and Warsaw are not visible on z4, even when there is plenty of space to render these labels. United Kingdom and Ireland are also strange for me - their names disappear as expected, but Dublin and Belfast labels are still not rendered. |
Rome and Warsaw are blocked by country labels "Italia" and "Polska" on z4. Capitals only get priority over state labels but not country labels. Unfortunately, in my planet snapshot some country polygons are broken. This is why quite some country labels are missing. If they would be rendered there would be less capitals shown. I know that this might be a bit confusing. Belfast does not appear because it is not a country capital. Dublin does not appear because it does not exceed the threshold of 600.000 inhabitants. I agree with @imagico that it would be important to still show other important cities on z4. Warsaw on z5 puzzles me a bit. I think it should appear, but instead the state label is rendered. However, it works for Berlin and "Brandenburg". |
I also don't understand some other capitals missing, like Prague and Lisbon. I guess when the capital label has no space to be rendered, we should render at least the circle. Would it be possible to prepare simplified database with only admin borders and cities? Maybe just a script to do it myself? Importing the whole planet or just the Europe (18 GB in PBF!) would be overkill for my computer, but I'd like to test this code. |
@kocio-pl: you don't seem to need much. Just hack your |
https://lists.openstreetmap.org/pipermail/dev/2016-May/029245.html has more than just that, but is 26GB. It's a pg_dump file so you don't need osm2pgsql, which is the big time saver, more so than dropping most buildings and residential roads. |
sent from a phone
you could prepare such an extract yourself with osmosis or osmfilter, and import this extract with osm2pgsql. Although downloading and filtering will take some time, it surely will take much fewer time than importing the planet unfiltered. Alternatively download the extract with overpass api (not sure if heavy usage like this is allowed) |
I'm using https://github.com/gmgeo/osm-carto-lowzoom if that's any help. You still need to download the planet (or any other extract), process it and import it. So don't expect it to be quick. But it is manageable. |
…ing, do not let state labels obscure captial city names (fixes gravitystorm#1883)
0e954bd
to
226a9f1
Compare
Thanks for all the hints! First test with osm-carto-lowzoom was pretty promising - South America (PBF size about 1GB) was reduced to just 60 MB in about 5 minutes. Naive estimation tells that for the whole planet (33 GB) it would be less than 3 hours. Import of resulting lowzoom South America file took 160 s, so the second stage for the whole planet could be even shorter, but even 6 hours in total sounds reasonable for me. I was trying the dump once, but it took about 27 hours, IIRC. It was on the old laptop, but still 4-5 times longer than with osm-carto-lowzoom and the bottleneck was probably the same HDD. And it probably contains some data I don't need in this case. For this task I will try osm-carto-lowzoom:
|
I plan to try to address all the remaining problems in another update. |
…rict with capitals, no dots at z8, N,S,E,W placement, correction of query
226a9f1
to
3747894
Compare
Looks good to me. Two more observations:
With my remark on discontinuity between zoom levels i did not mean to object to this change. I just wanted to point out this does happen and should be kept in mind. Most maps make this switch from z8 to z9, some earlier, some later, some do it in steps (keeping dots for smaller settlements up to higher zoom levels). Did you observe any cases where labels vanish from z7 to z8 due to the reduced placement flexibility? Probably very rare but in principle this could occur. |
I checked and the node is not in my rendering database. The reason or rather the solution may be found in https://www.openstreetmap.org/changeset/41312341.
Can you name some cases where you noticed that? I thought since capitals occur not that often state labels should not be that affected. I suspect it might be rather due to a number of admin boundaries being broken.
Yes, I noticed that. For example in the case of Amsterdam and Haarlem. But I guess there is not much we can do about it. |
Do you mean only capitals are prioritized over level 4 admin units? You could be right that the effect is not significantly worse than before. |
ELSE 1 | ||
END) | ||
) AS score | ||
FROM planet_osm_point | ||
WHERE place IN ('city', 'town') | ||
AND name IS NOT NULL | ||
AND capital IS NULL OR capital != '2' OR (capital = 'yes' AND admin_level != '2') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the counter-part for the capital-names SQL querry?
At capital-names, there is:
WHERE capital = 'yes'
AND admin_level = '2'
So I guess the counter-part should be
AND capital IS NULL OR capital != 'yes' OR (capital = 'yes' AND admin_level != '2')
???
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you are right. I will change that.
I don't agree here. Tying dots and city labels together was an intentional decision. For me it does not make much sense to show a dot if you don't see which city that is.
Yes currently it is like that. I also thought of prioritizing other big cities, but I would suggest to roll out the change like that and optimize later on when we have seen what became better and what still can be optimized. |
👍 |
But why? We do it for other features. It makes a lot of sense to me to show "here is something important in this context" and later "...and its name is x" as a detail. I consider it to be good generalization. In the context of countries we have some basic properties like the shape and the capital. If there's enough space, we can show their names, but we don't have to bind the shape and the location of administrative center with their names. |
I think this is not entirely comparable. We are talking of completely different scales / zoom levels. Adding points without labels at something like Z4-6 just wastes space that could be used to label another city, the dot may prevent other labels being placed. Look at the images. At Z4, the "Paris" label is the width of the entire Netherlands... Space is valuable at these scales. Additionally, a supermarket shop icon without a name, is still rather informative. I think a city dot is less so. |
A city dot is useless but is a capital dot useless too? |
sent from a phone
given the size of the Netherlands, that's not extremely impressive ;-) |
Well, I am sure there are plenty of people in the Netherlands who would wish the Netherlands was one big Paris "arrondissement"... ;-) Well, actually, Napoléon thought that as well! He even kicked his brother Louis Napoléon Bonaparte out who he had put on the throne as King of Holland to achieve it, because his brother was a bit to independent and felt at home in his own kingdom... |
I'm inclined to answer yes. Location without label (even with category) carries too little meaning for me. But let's look at it from the technical side: I'm not going to remove the shield logic that ties dots and labels together because then we would get labels without dots too, which is the situation I wanted to get rid of. To achieve what you want would mean to add dots in a second pass. Since this PR has enough to review already I would leave that for a follow-up PR. Anybody who thinks the situation could be further improved should file follow-up issues and/or PRs. But let's not try to pack too much into this one. |
I'm OK with resolving the more general problem first and tuning the concept later. |
@@ -1545,14 +1589,14 @@ Layer: | |||
END) | |||
* | |||
(CASE | |||
WHEN (capital = 'yes') THEN 3 | |||
WHEN (capital = '4') THEN 2 | |||
WHEN (capital = '4' OR (capital = 'yes' AND admin_level = '4')) THEN 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why has this code been changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed it because I moved country capitals to their own layer. Only capitals with level 4 are still handled by this.
Really sorry if this is not related at all with this PR, but is it possible to query for the I am asking this because Brasilia, for example, is at the same time the This is also the same problem in the Humanitarian style hotosm/HDM-CartoCSS#281 Is there a better place to discuss this, please? |
Thanks for raising awareness of this issue. Currently it is not possible to query for |
sent from a phone
the tag for this is "capital", simply add the lowest number, that's the most significant one (or yes for country capitals). There's nothing special with Brasilia, many (most?) country capitals are also admin centres of lower administrative entities at the same time, the idea of capital is to give an idea about administrative importance, not modeling all the details (use the relations for this) |
Thanks for creating it and merging! I think it's worth fine tune it more or at least find out what are the problems with some places. Does it make sense to open issue for that or it's enough to talk here until new PR? Current data rendering in Europe (with Lisbon and Athens fixed) looks like this (remember to open the images, since they are scaled down by default): |
I also want to thank the developers, especially @nebulon42! The zoom-in from Z4 to Z9 on Germany and later Stuttgart is now really natural compared to before where Stuttgart somehow got eclipsed. And the capital dot is awesome! |
Fixes #1883.
Tries to partly fix #1391 for captial cities.
This PR leads to the following changes:
Previews - outdated
z3
(note that capitals do not obscure country labels, missing country labels are due to broken polygons in my snapshot)
z4
z5
z6
z7
z8 (here my database starts deviating from OSM production servers)
This PR has still some issues:
Another item for discussion is of course if the label placement should be NE,SE,NW or just N,S or E,W.