Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

updates to placename rendering #2314

Merged
merged 5 commits into from
Sep 23, 2016

Conversation

nebulon42
Copy link
Contributor

@nebulon42 nebulon42 commented Aug 29, 2016

Fixes #1883.
Tries to partly fix #1391 for captial cities.

This PR leads to the following changes:

  • up to (excluding) z8 place names are marked with a dot, double dot for capital cities
  • on z4 only bigger capital cities are rendered, other place names are dropped
  • state labels can no longer obscure capital cities, because they got higher priority

Previews - outdated

z3
z3
(note that capitals do not obscure country labels, missing country labels are due to broken polygons in my snapshot)

z4
z4

z5
z5

z6
z6

z7
z7

z8 (here my database starts deviating from OSM production servers)
z8

This PR has still some issues:

  • changes of larger labels for placenames #2181 are not included since I prepared it earlier, I will fine-tune to incorporate this change if necessary
  • I noticed fewer place labels at z6 and will have to further investigate that
  • Fürth shows up instead of Nürnberg, this should not happen
  • If dots should be shown <= z8 or < z8 still needs to be investigated further

Another item for discussion is of course if the label placement should be NE,SE,NW or just N,S or E,W.

way,
name,
CASE
WHEN population ~ '^-?\d+(\.\d+)?$' THEN population::NUMERIC ELSE 0
Copy link
Collaborator

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

@sommerluk
Copy link
Collaborator

Especially the fact that capitals get priority over state names is a big advance.

😊

  • Indeed I think the changes of larger labels for placenames #2181 should be added.
  • As this PR touches yet various placenames labels at lower zoom levels, could you fine-tune also the labels for state names, so that they get a minimal font size of 10? Maybe move rendering for state labels one zoom level higher to prevent to clutter the map?
  • Personally, I would not show capitals at z3 at all, and at z4 only the big capitals (like in the current PR for z3 with [population > 600000]).

@boothym
Copy link
Contributor

boothym commented Aug 30, 2016

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.

@nebulon42
Copy link
Contributor Author

@boothym Belfast has capital=yes but no admin_level set. I first did require admin level 2 or no admin level at all to be more inclusive. If we are more strict and require admin_level=2 there might be the need to update some capitals. Maybe this is desirable anyway.

@nebulon42
Copy link
Contributor Author

I have made some updates:

  • no capitals at z3
  • fewer capitals at z4
  • be more strict with capitals (capital=yes and admin_level=2)
  • change population expression
  • no dots at z8
  • label placement only above and beneath the dot

There are still fewer place names rendered around z6 than I would expect.

z3
z3

z4
z4

z5
z5

z6
z6

z7
z7

@sommerluk I think we should fine-tune state labels afterwards if necessary.

@imagico
Copy link
Collaborator

imagico commented Aug 30, 2016

A few observations:

  • there seem to be much less city labels in France and Italy at z6/7 in your examples than previously and compared to other countries.
  • metatile discontinuities are probably a bigger problem with this change. Is it possible to tell Mapnik to avoid placing labels across metatile boundaries?
  • switching from dot + off-center label to center labels from z7 to z8 will create a significant visual discontinuity. It makes sense to switch from dot symbols to builtup area rendering at some point but currently builtup areas are based on poor data making things look even stranger.
  • only allowing N/S labels seems a bit strange - for example in your z6 example Bern could be labeled in W direction but is not.
  • rendering capitals only at z4 will look somewhat strange, especially with large countries with small capitals and other big cities.

@nebulon42
Copy link
Contributor Author

nebulon42 commented Aug 31, 2016

Note to myself: experiment with marking the capitals also after z7 (e.g. underline), ref #2222
edit: text underlines are currently not possible with Mapnik, an attempt at a hack with overlines (https://en.wikipedia.org/wiki/Overline) had the drawback that overlines have fixed width whereas different letters could have different width. So word and line would not necessarily match.

@dieterdreist
Copy link

dieterdreist commented Aug 31, 2016

I assume admin_level should be added to the node?

capital=4

@dieterdreist
Copy link

sent from a phone

Il giorno 30 ago 2016, alle ore 21:32, Michael Glanznig [email protected] ha scritto:

I first did require admin level 2 or no admin level at all to be more inclusive. If we are more strict and require admin_level=2 there might be the need to update some capitals. Maybe this is desirable anyway.

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.

@dieterdreist
Copy link

sent from a phone

Il giorno 30 ago 2016, alle ore 23:47, Christoph Hormann [email protected] ha scritto:

switching from dot + off-center label to center labels from z7 to z8 will create a significant visual discontinuity. It makes sense to switch from dot symbols to builtup area rendering at some point but currently builtup areas are based on poor data making things look even stranger.

+1, we should rather push for place polygons for these instead of ne shapefiles

@jojo4u
Copy link

jojo4u commented Aug 31, 2016

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.

It's not documented but done: 83% of all capital=* have admin_level

@dieterdreist
Copy link

sent from a phone

Il giorno 31 ago 2016, alle ore 12:33, jojo4u [email protected] ha scritto:

It's not documented but done: 83% of all capital=* have admin_level

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:
https://taginfo.openstreetmap.org/keys/capital#values
if you add those capital=yes that are country capitals to the above figures, you even get over 99% following this approach

@nebulon42
Copy link
Contributor Author

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 capital=yes won't produce satisfactory results but the false positives could be cleaned up later on. I'm tending towards being explicit about the admin level by either using capital=2 (currently only 11) or admin_level=2 (190 occurrences in combination with capital=yes). admin_level may be redundant, but since it is used it may be worth considering it.

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.

@dieterdreist
Copy link

2016-08-31 19:59 GMT+02:00 Michael Glanznig [email protected]:

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.

I can't completely understand you here. 100% are using capital=*, the very
most of them capital=yes for national capitals and
capital=<admin_level_number> for other admin levels. A significant amount
is alternatively using capital=2 for national capitals, no problem here (we
check for yes or 2). Those using capital=yes for places that are not
national capitals are very few (judging from usage numbers without looking
which kind of capital it is) and are doing this against the documented
tagging practice, so I would recommend these instances to be changed in
order to be in line (because numbers are very low this seams reasonable).
Those additional admin_levels on places are not documented regarding their
meaning and sometimes differ from the value in capital, so I don't know why
we should pay attention, we don't even know what it means, and it adds no
additional information (it's only 87% compared to 100% with capital and
according to the meaning you presume it's the same).

@mboeringa
Copy link

The question about which tagging is appropriate for country capitals is valid.
...
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.

... 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".

@kocio-pl
Copy link
Collaborator

kocio-pl commented Aug 31, 2016

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.

@nebulon42
Copy link
Contributor Author

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".

@kocio-pl
Copy link
Collaborator

kocio-pl commented Sep 1, 2016

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.

@StyXman
Copy link
Contributor

StyXman commented Sep 1, 2016

@kocio-pl: you don't seem to need much. Just hack your import.style file with the fields you need: name, popultion, capital, admin_level, place, ref, boundary and probably a couple more.

@pnorman
Copy link
Collaborator

pnorman commented Sep 1, 2016

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.

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.

@dieterdreist
Copy link

sent from a phone

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.

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)

@nebulon42
Copy link
Contributor Author

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)
@kocio-pl
Copy link
Collaborator

kocio-pl commented Sep 2, 2016

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:

  • the data can be as fresh as latest PBF (useful if we would need to fix tagging)
  • I can test just one macroregion/continent at the time
  • it makes it much easier to swap imports
  • the import from PBF is more standard/less fragile than database dump (also because in case of failure I can repeat any of 2 stages, each 3h long, instead of one big 27h stage)

@kocio-pl
Copy link
Collaborator

kocio-pl commented Sep 2, 2016

South America on z4 also shows similar problems like Europe - only 3 capitals visible (maybe it has something to do with too strong filtering? or tagging maybe?):
v8vrpcum

z5 (click to enlarge):
bmto9vo1

@nebulon42
Copy link
Contributor Author

nebulon42 commented Sep 2, 2016

I plan to try to address all the remaining problems in another update.
But one thing that is important to know is (like I said above): Country labels block capitals like e.g. Perú and Lima.

@pnorman pnorman changed the title updates to placename rendering [WIP] updates to placename rendering Sep 2, 2016
…rict with capitals, no dots at z8, N,S,E,W placement, correction of query
@nebulon42
Copy link
Contributor Author

nebulon42 commented Sep 2, 2016

Europe
z4
z4
Note: apparently London is missing because the metatile boundary on z4 exactly passes through it.

z5
z5

z6
z6

z7
z7

z8
z8

z9
z9

US
z4
am_z4

z5
am_z5

z6
am_z6

z7
am_z7

@nebulon42 nebulon42 changed the title [WIP] updates to placename rendering updates to placename rendering Sep 2, 2016
@imagico
Copy link
Collaborator

imagico commented Sep 2, 2016

Looks good to me.

Two more observations:

  • for some reason Caracas is missing in the US z4 sample which is odd.
  • as a secondary effect of this change display of level 4 admin unit labels now often seems arbitrary which should probably be reviewed but can be done separately of course.

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.

@nebulon42
Copy link
Contributor Author

nebulon42 commented Sep 2, 2016

for some reason Caracas is missing in the US z4 sample which is odd.

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.

display of level 4 admin unit labels now often seems arbitrary

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.

Did you observe any cases where labels vanish from z7 to z8 due to the reduced placement flexibility?

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.

@kocio-pl
Copy link
Collaborator

kocio-pl commented Sep 2, 2016

Let's test some basic things - new code at z4, without country and state labels and population limits to test capitals rendering alone. Lisbon is not shown until z7, but that was a tagging problem (I have fixed it now), Helsinki are eclipsing Tallinn and Vienna is eclipsing Bratislava (I don't know why):
ul6l8emq

The same, but with 100k limit instead of 600k (another big problem: Athens needed fixing) - why is this limit so high?:
bdbwj_0b

I have also the question about dots - why are they always rendered with city label? At least for capitals I would use them as a fallback to mark city position even if the label can't be shown.

[EDIT:] What about showing only dots at z4 and the names from z5?

@imagico
Copy link
Collaborator

imagico commented Sep 2, 2016

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.

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')
Copy link
Collaborator

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')

???

Copy link
Contributor Author

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.

@nebulon42
Copy link
Contributor Author

I have also the question about dots - why are they always rendered with city label? At least for capitals I would use them as a fallback to mark city position even if the label can't be shown.
[EDIT:] What about showing only dots at z4 and the names from z5?

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.

Do you mean only capitals are prioritized over level 4 admin units?

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.

@pnorman
Copy link
Collaborator

pnorman commented Sep 3, 2016

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.

👍

@kocio-pl
Copy link
Collaborator

kocio-pl commented Sep 3, 2016

For me it does not make much sense to show a dot if you don't see which city that is.

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.

@mboeringa
Copy link

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.

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.

@HolgerJeromin
Copy link
Contributor

A city dot is useless but is a capital dot useless too?

@dieterdreist
Copy link

sent from a phone

Il giorno 04 set 2016, alle ore 18:22, mboeringa [email protected] ha scritto:

At Z4, the "Paris" label is the width of the entire Netherlands

given the size of the Netherlands, that's not extremely impressive ;-)

@mboeringa
Copy link

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...

@nebulon42
Copy link
Contributor Author

A city dot is useless but is a capital dot useless too?

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.

@kocio-pl
Copy link
Collaborator

kocio-pl commented Sep 5, 2016

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
Copy link
Collaborator

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed it because I moved country capitals to their own layer. Only capitals with level 4 are still handled by this.

@matthijsmelissen matthijsmelissen merged commit 52d85dd into gravitystorm:master Sep 23, 2016
@naoliv
Copy link

naoliv commented Sep 23, 2016

Really sorry if this is not related at all with this PR, but is it possible to query for the admin_centre node of admin_level=2 relations to get their capitals, instead relying in capital=yes + admin_level=2? (or some other similar test)

I am asking this because Brasilia, for example, is at the same time the admin_centre of an admin_level=2, admin_level=4 and an admin_level=8 relations, and adding admin_level=2 to its node won't properly represent what it is.

This is also the same problem in the Humanitarian style hotosm/HDM-CartoCSS#281

Is there a better place to discuss this, please?

@nebulon42
Copy link
Contributor Author

Thanks for raising awareness of this issue. Currently it is not possible to query for admin_centre roles. This would need changes to the rendering database. But it might be worth to do it in the future. I suggest opening a separate ticket for it, so that it can be discussed independently. Please also again reference the Humanitarian style.

@dieterdreist
Copy link

sent from a phone

Il giorno 23 set 2016, alle ore 21:04, Nelson A. de Oliveira [email protected] ha scritto:

I am asking this because Brasilia, for example, is at the same time the admin_centre of an admin_level=2, admin_level=4 and an admin_level=8 relations, and adding admin_level=2 to its node won't properly represent what it is.

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)

@kocio-pl
Copy link
Collaborator

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):

z4
snf3sanp

z5
6e4txvrj

z6
h6dhdqbm

z7
3qattfh6

@jojo4u
Copy link

jojo4u commented Oct 2, 2016

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet