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

rework admin boundaries and admin labels on lower zooms #1938

Merged
merged 5 commits into from Nov 23, 2015

Conversation

Projects
None yet
9 participants
@nebulon42
Contributor

nebulon42 commented Oct 31, 2015

Fixes #907, #1571.
Adresses part of #622 (the part that is similar to #907, #1571)

Changes are concerned with reduction of complexity and better rendering of country/state labels as well as boundary lines, especially in maintaining a visual hierarchy of admin levels 2,3,4.

Details:

  • country lines are thinner on z1,2,3 but get thicker quickly and maintain a hierarchy with level 3 and 4
  • level 3 and 4 are much thinner earlier
  • line thickness of admin entities on higher zooms is unchanged
  • line rendering only uses data derived from relations (osm_id < 0) which seems to avoid some duplicate rendering
  • simplification of admin lines starting at z4 using ST_simplify with a initial tolerance of ~ 7800 and decreasing linearly with scale denominator until the layer no longer gets rendered at z10
  • omission of admin levels 3,4 for smaller countries on z4, using a way_area factor of 9000000000000, which should only include Russia, Canada, China (?), United States of America, Brazil, Australia and Greenland (only if their country geometries are correct and therefore in the polygon table)
  • placename labels (city, town)
    • are dropped from z3 as I think it is too early to show them
  • country labels
    • are dropped from z2 (here e.g. continent labels could be rendered)
    • are darker with more char and line spacing
    • have now a semi-transparent halo
    • they wrap earlier and have larger font-size from z3
    • generally they stay bigger than state labels all the time
    • they are rendered for smaller countries earlier and stay longer (I tuned it so that the US is visible on z4)
  • state labels
    • have a semi-transparent halo now
    • wrap earlier
  • all boundary lines use now bevel joins as suggested by @imagico

Problems:

  • usage of ST_simplify is not undisputed (see #907), but I think it is a usable possibility until we have something better
  • performance of SQL query deciding if subnational entities should be rendered on z4 seems suboptimal fixed, thanks @pnorman, @imagico
  • since country boundaries are thicker later on sea boundaries are further emphasised

possible Improvements:

  • exponential decline of tolerance parameter when using ST_simplify - if somebody wants to experiment with it

Previews (only after the changes, available data: admin entities incl. names and place names [city, town]):

z1
world_z1

z2
world_z2

z3
world_z3

z4
Europe
europe_z4

Africa
africa_z4

Americas
americas_z4

unfortunately the USA geometry seems to be broken, but see this other screenshot in #907 (comment).

Oceania/Asia
oceania_z4

z5 (from here roads are missing)
europe_z5

z6
europe_z6

Testing:
If you need test data you can download it from [link no longer active]. This file contains global admin boundaries of level 2,3,4 as well as place nodes of cities and towns. Some country geometries like France, Belgium ... and unfortunately also the US are broken so their name (and sub-national entities at z4 in case of US) won't appear.

@imagico

This comment has been minimized.

Show comment
Hide comment
@imagico

imagico Oct 31, 2015

Collaborator

usage of ST_simplify is not undisputed (see #907), but I think it is a usable possibility until we have something better

A general remark here: While this is an understandable approach in principle it is important to also keep in mind the likely negative consequences:

  • establishing a superficial solution to a problem will make it less likely that someone will develop a more thorough one in the future
  • the disadvantages of the presented approach are not immediately apparent. It affects readability but this is not clearly visible because the mechanisms are subtle and non-obvious. A map user will, when presented with a better rendering, likely immediately see the improvement but will usually not be able to articulate the need for it a priori.

Specific suggestions:

  • use another line-join - since the corners of the boundaries are artificial anyway after ST_simplify there is no need to add even more high frequency noise.
  • although not directly related it would be a good opportunity to give important cities priority over subnational admin unit labels.
  • performance: i will leave the detailed analysis to others but ST_Within(b.way, c.way) is likely unneccessarily complicated, you'd just need to check the centroid of the level 3/4 polygon being within the level 2 polygon. I am not sure if there is a way to use the simplified level 2 polygons you generate anyway for the test.
  • What's with China at z4?
Collaborator

imagico commented Oct 31, 2015

usage of ST_simplify is not undisputed (see #907), but I think it is a usable possibility until we have something better

A general remark here: While this is an understandable approach in principle it is important to also keep in mind the likely negative consequences:

  • establishing a superficial solution to a problem will make it less likely that someone will develop a more thorough one in the future
  • the disadvantages of the presented approach are not immediately apparent. It affects readability but this is not clearly visible because the mechanisms are subtle and non-obvious. A map user will, when presented with a better rendering, likely immediately see the improvement but will usually not be able to articulate the need for it a priori.

Specific suggestions:

  • use another line-join - since the corners of the boundaries are artificial anyway after ST_simplify there is no need to add even more high frequency noise.
  • although not directly related it would be a good opportunity to give important cities priority over subnational admin unit labels.
  • performance: i will leave the detailed analysis to others but ST_Within(b.way, c.way) is likely unneccessarily complicated, you'd just need to check the centroid of the level 3/4 polygon being within the level 2 polygon. I am not sure if there is a way to use the simplified level 2 polygons you generate anyway for the test.
  • What's with China at z4?
@kocio-pl

This comment has been minimized.

Show comment
Hide comment
@kocio-pl

kocio-pl Oct 31, 2015

Collaborator

In general I like z4 with just the country borders. I suppose USA label should be visible if not the broken geometry you mentioned. However why Australia is different on that zoom level from the rest of the world (sub-country borders are visible)?

Collaborator

kocio-pl commented Oct 31, 2015

In general I like z4 with just the country borders. I suppose USA label should be visible if not the broken geometry you mentioned. However why Australia is different on that zoom level from the rest of the world (sub-country borders are visible)?

Show outdated Hide outdated project.yaml
Show outdated Hide outdated project.yaml
Show outdated Hide outdated project.yaml
@pnorman

This comment has been minimized.

Show comment
Hide comment
@pnorman

pnorman Nov 1, 2015

Collaborator

I'd probably do the line SQL something like this for lines

(SELECT
    *
  FROM planet_osm_roads l
  WHERE boundary = 'administrative'
    AND (admin_level = '2'
      OR (admin_level IN ('3', '4')
        AND EXISTS 
          (SELECT 1 
            FROM planet_osm_polygon p
            WHERE admin_level = '2'
              AND way_area > !pixel_width!::real*!pixel_height!::real*something
              AND ST_Within(l.way, p.way))
  ORDER BY admin_level ASC, way_area DESC
) AS admin_very_low_zoom
Collaborator

pnorman commented Nov 1, 2015

I'd probably do the line SQL something like this for lines

(SELECT
    *
  FROM planet_osm_roads l
  WHERE boundary = 'administrative'
    AND (admin_level = '2'
      OR (admin_level IN ('3', '4')
        AND EXISTS 
          (SELECT 1 
            FROM planet_osm_polygon p
            WHERE admin_level = '2'
              AND way_area > !pixel_width!::real*!pixel_height!::real*something
              AND ST_Within(l.way, p.way))
  ORDER BY admin_level ASC, way_area DESC
) AS admin_very_low_zoom
@pnorman

This comment has been minimized.

Show comment
Hide comment
@pnorman

pnorman Nov 1, 2015

Collaborator

If you're returning functions(way) AS way you need to add && conditions to appropriate WHERE clauses to check for intersection with !bbox! And if !bbox! appears in the query string, Mapnik will not add an outer bbox check so you have to make sure to add all of the checks you need

Collaborator

pnorman commented Nov 1, 2015

If you're returning functions(way) AS way you need to add && conditions to appropriate WHERE clauses to check for intersection with !bbox! And if !bbox! appears in the query string, Mapnik will not add an outer bbox check so you have to make sure to add all of the checks you need

@nebulon42

This comment has been minimized.

Show comment
Hide comment
@nebulon42

nebulon42 Nov 1, 2015

Contributor

re SQL: seems I was thinking a bit to complicated when it came to separating the less costly parts from the more expensive ones, thanks @pnorman
Using only polygon centroids speeded it up also, thanks @imagico

establishing a superficial solution to a problem will make it less likely that someone will develop a more thorough one in the future

I don't disagree with your arguments, but waiting for the perfect solution, when the problem has been there for years already is also not desirable IMO. But of course the maintainers are free to delay or decline this, if they think there will be better solutions. Also, anybody with more expertise is free to step forward.

use another line-join

I have now switched to bevel joins for all boundaries. Maybe better anyway since it helps to avoid something like this: miterjoin

it would be a good opportunity to give important cities priority over subnational admin unit labels

I decided against including it as there would be too many changes, but it is on my list.

What's with China at z4?

Sub-national entities should be displayed, but in the last two planet extracts I obtained China was not in the polygon table, same with Russia.

why Australia is different on that zoom level from the rest of the world (sub-country borders are visible)?

While Australia is a bit smaller than Brazil (which I have selected as cut-off country) according to https://en.wikipedia.org/wiki/List_of_countries_and_dependencies_by_area it is either the sea boundaries or the projection or both that distorts the ranking.

If I query the database for the biggest countries (way_area) I get the following list from the data upon which the previews are based (smallest on top):

"Brasil";9.29986e+12
"United States of America";1.0177e+13
"Australia";1.06476e+13
"Kalaallit Nunaat";4.31275e+13
"Canada";7.36066e+13

You see that China and Russia are missing, USA is too small because only Alaska made it into the table and also Greenland is in the list because it extends into low latitudes. I personally have no problem with Australia getting sub-national entities on z4. We would have to hardcode names or (if we would have hstore) iso codes otherwise, but that would be getting close to country specific renderings, which I wanted to avoid.

Contributor

nebulon42 commented Nov 1, 2015

re SQL: seems I was thinking a bit to complicated when it came to separating the less costly parts from the more expensive ones, thanks @pnorman
Using only polygon centroids speeded it up also, thanks @imagico

establishing a superficial solution to a problem will make it less likely that someone will develop a more thorough one in the future

I don't disagree with your arguments, but waiting for the perfect solution, when the problem has been there for years already is also not desirable IMO. But of course the maintainers are free to delay or decline this, if they think there will be better solutions. Also, anybody with more expertise is free to step forward.

use another line-join

I have now switched to bevel joins for all boundaries. Maybe better anyway since it helps to avoid something like this: miterjoin

it would be a good opportunity to give important cities priority over subnational admin unit labels

I decided against including it as there would be too many changes, but it is on my list.

What's with China at z4?

Sub-national entities should be displayed, but in the last two planet extracts I obtained China was not in the polygon table, same with Russia.

why Australia is different on that zoom level from the rest of the world (sub-country borders are visible)?

While Australia is a bit smaller than Brazil (which I have selected as cut-off country) according to https://en.wikipedia.org/wiki/List_of_countries_and_dependencies_by_area it is either the sea boundaries or the projection or both that distorts the ranking.

If I query the database for the biggest countries (way_area) I get the following list from the data upon which the previews are based (smallest on top):

"Brasil";9.29986e+12
"United States of America";1.0177e+13
"Australia";1.06476e+13
"Kalaallit Nunaat";4.31275e+13
"Canada";7.36066e+13

You see that China and Russia are missing, USA is too small because only Alaska made it into the table and also Greenland is in the list because it extends into low latitudes. I personally have no problem with Australia getting sub-national entities on z4. We would have to hardcode names or (if we would have hstore) iso codes otherwise, but that would be getting close to country specific renderings, which I wanted to avoid.

@nebulon42

This comment has been minimized.

Show comment
Hide comment
@nebulon42

nebulon42 Nov 1, 2015

Contributor

@pnorman I cannot quite follow your last remark, can you point me towards some examples or documentation about it?

Contributor

nebulon42 commented Nov 1, 2015

@pnorman I cannot quite follow your last remark, can you point me towards some examples or documentation about it?

@pnorman

This comment has been minimized.

Show comment
Hide comment
@pnorman

pnorman Nov 1, 2015

Collaborator

https://github.com/mapnik/mapnik/wiki/PostGIS#bbox-token

An outer filter on way will be unable to use indexes because the way returned by the query is computed, and there's no suitable indexes

Collaborator

pnorman commented Nov 1, 2015

https://github.com/mapnik/mapnik/wiki/PostGIS#bbox-token

An outer filter on way will be unable to use indexes because the way returned by the query is computed, and there's no suitable indexes

@pnorman

This comment has been minimized.

Show comment
Hide comment
@pnorman

pnorman Nov 1, 2015

Collaborator

How's this going to work in countries with multiple disjoint areas like the US or Russia?

Collaborator

pnorman commented Nov 1, 2015

How's this going to work in countries with multiple disjoint areas like the US or Russia?

@nebulon42

This comment has been minimized.

Show comment
Hide comment
@nebulon42

nebulon42 Nov 1, 2015

Contributor

I'll read up on it.

Next good point raised. Currently it would work only for those parts which exceed the threshold. So e.g. the overseas territories that belong to the USA would not get sub-entities if they have some. Alaska has none at z4 anyway. I'm not familiar with the structure of the Russian Federation so I'm not sure if this would be a problem.

Contributor

nebulon42 commented Nov 1, 2015

I'll read up on it.

Next good point raised. Currently it would work only for those parts which exceed the threshold. So e.g. the overseas territories that belong to the USA would not get sub-entities if they have some. Alaska has none at z4 anyway. I'm not familiar with the structure of the Russian Federation so I'm not sure if this would be a problem.

@imagico

This comment has been minimized.

Show comment
Hide comment
@imagico

imagico Nov 2, 2015

Collaborator

I don't disagree with your arguments, but waiting for the perfect solution, when the problem has been there for years already is also not desirable IMO.

This is not what i am suggesting. But introducing an approach that can definitely not be recommended to other map designers in the vague hope that someone will some time in the future come up with something better is equally not desirable IMO.

I consider my objection essentially to be in line with what @gravitystorm told me when i made my first suggestion to deal with the Antarctic ice sheet in #646 (comment) - this essentially amounts to the demand that a solution should offer a certain level of applicability beyond the precise situation it is used in.

Please don't view this as a blanket criticism of your work here - this is an interesting approach to the problem - but it seems it only works given the very special circumstances here, i.e. rendering as polygons, styling with fairly thick lines and using the trick in https://github.com/gravitystorm/openstreetmap-carto/blob/master/admin.mss#L56

Beyond these general remarks it also occurred to me that since you apply ST_Simplify up to z10 this change would seriously affect mapper feedback by showing a modified geometry at scales where the difference is already going to be clearly visible, like here:

http://www.openstreetmap.org/#map=10/69.6781/30.6299

I'm not familiar with the structure of the Russian Federation so I'm not sure if this would be a problem.

None of the Russian arctic islands is a separate level 4 entity i think, neither is the western hemisphere part. Labeling however is probably an issue (i.e Alaska etc. would not get a correct label) But admin unit labeling is already a complete mess anyway so this will probably hardly be noticeable.

Collaborator

imagico commented Nov 2, 2015

I don't disagree with your arguments, but waiting for the perfect solution, when the problem has been there for years already is also not desirable IMO.

This is not what i am suggesting. But introducing an approach that can definitely not be recommended to other map designers in the vague hope that someone will some time in the future come up with something better is equally not desirable IMO.

I consider my objection essentially to be in line with what @gravitystorm told me when i made my first suggestion to deal with the Antarctic ice sheet in #646 (comment) - this essentially amounts to the demand that a solution should offer a certain level of applicability beyond the precise situation it is used in.

Please don't view this as a blanket criticism of your work here - this is an interesting approach to the problem - but it seems it only works given the very special circumstances here, i.e. rendering as polygons, styling with fairly thick lines and using the trick in https://github.com/gravitystorm/openstreetmap-carto/blob/master/admin.mss#L56

Beyond these general remarks it also occurred to me that since you apply ST_Simplify up to z10 this change would seriously affect mapper feedback by showing a modified geometry at scales where the difference is already going to be clearly visible, like here:

http://www.openstreetmap.org/#map=10/69.6781/30.6299

I'm not familiar with the structure of the Russian Federation so I'm not sure if this would be a problem.

None of the Russian arctic islands is a separate level 4 entity i think, neither is the western hemisphere part. Labeling however is probably an issue (i.e Alaska etc. would not get a correct label) But admin unit labeling is already a complete mess anyway so this will probably hardly be noticeable.

@sommerluk

This comment has been minimized.

Show comment
Hide comment
@sommerluk

sommerluk Nov 2, 2015

Collaborator

Looks clean and much more organized than the current rendering!

My suggestions:
– Simply drop country border at z1 in favour of continent labels
– Continent labels z1–z2
– On z5, for sub-national entities, use the abbreviated name instead of the normal name (like at z4)
– And, as @imagico said: give important cities priority over subnational admin unit labels

Collaborator

sommerluk commented Nov 2, 2015

Looks clean and much more organized than the current rendering!

My suggestions:
– Simply drop country border at z1 in favour of continent labels
– Continent labels z1–z2
– On z5, for sub-national entities, use the abbreviated name instead of the normal name (like at z4)
– And, as @imagico said: give important cities priority over subnational admin unit labels

@nebulon42

This comment has been minimized.

Show comment
Hide comment
@nebulon42

nebulon42 Nov 2, 2015

Contributor

I'm actually not quite attached to the simplification. I see the main improvements with the labels, line styles and also with removing subnational units from z4. We could also leave out the simplification, but the map would still look quite ugly.

you apply ST_Simplify up to z10

I see the problem, that's why I wrote: "exponential decline of tolerance parameter when using ST_simplify". Still have to think about it, input welcome.

Please do not expect too much out of this PR. At this stage I'm not going to add more features, it is more likely that some changes are removed.

Simply drop country border at z1 in favour of continent labels

Might be a good idea, but I only want to do that when we render something to replace them (e.g. those labels).

On z5, for sub-national entities, use the abbreviated name instead of the normal name (like at z4)

I'm not sure, will have to test this.

Contributor

nebulon42 commented Nov 2, 2015

I'm actually not quite attached to the simplification. I see the main improvements with the labels, line styles and also with removing subnational units from z4. We could also leave out the simplification, but the map would still look quite ugly.

you apply ST_Simplify up to z10

I see the problem, that's why I wrote: "exponential decline of tolerance parameter when using ST_simplify". Still have to think about it, input welcome.

Please do not expect too much out of this PR. At this stage I'm not going to add more features, it is more likely that some changes are removed.

Simply drop country border at z1 in favour of continent labels

Might be a good idea, but I only want to do that when we render something to replace them (e.g. those labels).

On z5, for sub-national entities, use the abbreviated name instead of the normal name (like at z4)

I'm not sure, will have to test this.

@pnorman

This comment has been minimized.

Show comment
Hide comment
@pnorman

pnorman Nov 2, 2015

Collaborator

I would drop the simplification. It adds complexity, and I'm not certain the results are what we really want. We can look at it independently.

I wouldn't worry much about z0-z1. The use-cases for that are extremely limited, and it's always going to be a bad map at such a low zoom thanks to Mercator.

I am still concerned about complexity, but it might be okay with those changes.

Collaborator

pnorman commented Nov 2, 2015

I would drop the simplification. It adds complexity, and I'm not certain the results are what we really want. We can look at it independently.

I wouldn't worry much about z0-z1. The use-cases for that are extremely limited, and it's always going to be a bad map at such a low zoom thanks to Mercator.

I am still concerned about complexity, but it might be okay with those changes.

@dieterdreist

This comment has been minimized.

Show comment
Hide comment
@dieterdreist

dieterdreist Nov 3, 2015

sent from a phone

Am 02.11.2015 um 22:14 schrieb Michael Glanznig notifications@github.com:

Simply drop country border at z1 in favour of continent labels

I did this on my own rendering but switched to a small shapefile because of phantasy continents popping up from time to time.

dieterdreist commented Nov 3, 2015

sent from a phone

Am 02.11.2015 um 22:14 schrieb Michael Glanznig notifications@github.com:

Simply drop country border at z1 in favour of continent labels

I did this on my own rendering but switched to a small shapefile because of phantasy continents popping up from time to time.

@kocio-pl

This comment has been minimized.

Show comment
Hide comment
@kocio-pl

kocio-pl Nov 3, 2015

Collaborator

@sommerluk I also made a quick list of low zoom levels based on French style few months ago, maybe there are things we could use.

Collaborator

kocio-pl commented Nov 3, 2015

@sommerluk I also made a quick list of low zoom levels based on French style few months ago, maybe there are things we could use.

@HolgerJeromin

This comment has been minimized.

Show comment
Hide comment
@HolgerJeromin

HolgerJeromin Nov 3, 2015

Contributor

Rendering false continents is a good idea. At least if a fix in the database is rerendered faster than once a month (current render cycle on low zoom)

Contributor

HolgerJeromin commented Nov 3, 2015

Rendering false continents is a good idea. At least if a fix in the database is rerendered faster than once a month (current render cycle on low zoom)

@nebulon42

This comment has been minimized.

Show comment
Hide comment
@nebulon42

nebulon42 Nov 18, 2015

Contributor

I have removed the controversial simplification.

Contributor

nebulon42 commented Nov 18, 2015

I have removed the controversial simplification.

rework admin boundaries and admin labels on lower zooms, render count…
…ry subdivisions only for the biggest countries on z4
@matthijsmelissen

This comment has been minimized.

Show comment
Hide comment
@matthijsmelissen

matthijsmelissen Nov 18, 2015

Collaborator

I haven't had time to look at it yet.

Collaborator

matthijsmelissen commented Nov 18, 2015

I haven't had time to look at it yet.

@pnorman

This comment has been minimized.

Show comment
Hide comment
@pnorman

pnorman Nov 22, 2015

Collaborator

The cartography looks fine. I'm checking into some z3/z4 performance issues with a whole planet DB.

Collaborator

pnorman commented Nov 22, 2015

The cartography looks fine. I'm checking into some z3/z4 performance issues with a whole planet DB.

Show outdated Hide outdated project.yaml
Show outdated Hide outdated project.yaml
Show outdated Hide outdated project.yaml

pnorman added some commits Nov 22, 2015

Add SQL conditions for low level admin lines and labels
This helps significantly with the use of appropriate partial indexes
Don't use centroid for low-zoom admin calcs
This avoids getting it wrong in cases of highly-concave regions.
@pnorman

This comment has been minimized.

Show comment
Hide comment
@pnorman

pnorman Nov 22, 2015

Collaborator

Did the PR nebulon42#1 to fix some issues. There's still an admin 0,1,2,3,4 vs 2,4 mismatch to be fixed, but I think that's the last of the technical issues.

When those are fixed, I'm okay with it.

We're adding more layers, but I'm okay with that. It comes with our choices about how to handle layers.

The admin 4 in large admin 2 logic is a bit annoying to have to add, but it's not a complexity cost we're imposing on others. Most styles will use Natural Earth and special case iso_code2='USA' and a couple of other large places.

Collaborator

pnorman commented Nov 22, 2015

Did the PR nebulon42#1 to fix some issues. There's still an admin 0,1,2,3,4 vs 2,4 mismatch to be fixed, but I think that's the last of the technical issues.

When those are fixed, I'm okay with it.

We're adding more layers, but I'm okay with that. It comes with our choices about how to handle layers.

The admin 4 in large admin 2 logic is a bit annoying to have to add, but it's not a complexity cost we're imposing on others. Most styles will use Natural Earth and special case iso_code2='USA' and a couple of other large places.

@nebulon42

This comment has been minimized.

Show comment
Hide comment
@nebulon42

nebulon42 Nov 22, 2015

Contributor

Thanks @pnorman. I have addressed the remaining issues.

Contributor

nebulon42 commented Nov 22, 2015

Thanks @pnorman. I have addressed the remaining issues.

@matthijsmelissen

This comment has been minimized.

Show comment
Hide comment
@matthijsmelissen

matthijsmelissen Nov 22, 2015

Collaborator

Thanks @nebulon42. I haven't studied the code in detail, but the output certainly looks much better.

Collaborator

matthijsmelissen commented Nov 22, 2015

Thanks @nebulon42. I haven't studied the code in detail, but the output certainly looks much better.

@pnorman

This comment has been minimized.

Show comment
Hide comment
@pnorman

pnorman Nov 23, 2015

Collaborator

I used the index CREATE INDEX planet_osm_polygon_low_admin ON planet_osm_polygon USING gist (way) WITH (fillfactor=100) WHERE boundary = 'administrative' AND osm_id < 0 AND admin_level IN ('0','1','2','3','4'); while working on this - since in development you're constantly re-rendering low zooms instead of caching them for a month.

Collaborator

pnorman commented Nov 23, 2015

I used the index CREATE INDEX planet_osm_polygon_low_admin ON planet_osm_polygon USING gist (way) WITH (fillfactor=100) WHERE boundary = 'administrative' AND osm_id < 0 AND admin_level IN ('0','1','2','3','4'); while working on this - since in development you're constantly re-rendering low zooms instead of caching them for a month.

pnorman added a commit that referenced this pull request Nov 23, 2015

Merge pull request #1938 from nebulon42/admin
rework admin boundaries and admin labels on lower zooms

@pnorman pnorman merged commit d0d9afa into gravitystorm:master Nov 23, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@nebulon42 nebulon42 deleted the nebulon42:admin branch Nov 23, 2015

@matthijsmelissen

This comment has been minimized.

Show comment
Hide comment
@matthijsmelissen

matthijsmelissen Nov 23, 2015

Collaborator

Is there a reason to keep the #necountries layer and shapefile, or would we be able to get rid of this and render the low-level admin borders based on osm-data?

Collaborator

matthijsmelissen commented Nov 23, 2015

Is there a reason to keep the #necountries layer and shapefile, or would we be able to get rid of this and render the low-level admin borders based on osm-data?

@nebulon42

This comment has been minimized.

Show comment
Hide comment
@nebulon42

nebulon42 Nov 23, 2015

Contributor

I think it would be great to replace the Natural Earth data with OSM data. But this would need generalization and should be done with a shapefile since Douglas-Peucker (ST_Simplify) was not well-received. This would need some logic similar to coastline processing and could be hosted on openstreetmapdata.com (?), but someone would need to provide the update and error logic.

Contributor

nebulon42 commented Nov 23, 2015

I think it would be great to replace the Natural Earth data with OSM data. But this would need generalization and should be done with a shapefile since Douglas-Peucker (ST_Simplify) was not well-received. This would need some logic similar to coastline processing and could be hosted on openstreetmapdata.com (?), but someone would need to provide the update and error logic.

@pnorman

This comment has been minimized.

Show comment
Hide comment
@pnorman

pnorman Nov 23, 2015

Collaborator

think it would be great to replace the Natural Earth data with OSM data. But this would need generalization and should be done with a shapefile since Douglas-Peucker (ST_Simplify) was not well-received. This would need some logic similar to coastline processing and could be hosted on openstreetmapdata.com (?), but someone would need to provide the update and error logic

We don't use dashed lines at the even lower zooms, so the issues with dashes don't apply here. But we can handle this in a new issue.

Collaborator

pnorman commented Nov 23, 2015

think it would be great to replace the Natural Earth data with OSM data. But this would need generalization and should be done with a shapefile since Douglas-Peucker (ST_Simplify) was not well-received. This would need some logic similar to coastline processing and could be hosted on openstreetmapdata.com (?), but someone would need to provide the update and error logic

We don't use dashed lines at the even lower zooms, so the issues with dashes don't apply here. But we can handle this in a new issue.

@nebulon42

This comment has been minimized.

Show comment
Hide comment
@nebulon42

nebulon42 Nov 23, 2015

Contributor

We don't use dashed lines at the even lower zooms, so the issues with dashes don't apply here. But we can handle this in a new issue.

Yes, we should do that. But I think at least @imagico's concerns were not mainly about dashes.

Contributor

nebulon42 commented Nov 23, 2015

We don't use dashed lines at the even lower zooms, so the issues with dashes don't apply here. But we can handle this in a new issue.

Yes, we should do that. But I think at least @imagico's concerns were not mainly about dashes.

@matthijsmelissen

This comment has been minimized.

Show comment
Hide comment
@matthijsmelissen

matthijsmelissen Dec 1, 2015

Collaborator

Is it just me, or is zoom level 4 not being properly updated with the new rendering on the openstreetmap.org servers? Is this a glitch on the servers or in the stylesheet?

Collaborator

matthijsmelissen commented Dec 1, 2015

Is it just me, or is zoom level 4 not being properly updated with the new rendering on the openstreetmap.org servers? Is this a glitch on the servers or in the stylesheet?

@gravitystorm

This comment has been minimized.

Show comment
Hide comment
@gravitystorm

gravitystorm Dec 1, 2015

Owner

I had to revert this pull request, since it was bringing the OSMF servers to a halt (see the revert PR #1987 ). This is the query that had been running for more than 14 hours for one request:

(SELECT
way,
admin_level
FROM planet_osm_roads o
WHERE boundary = 'administrative'
AND (admin_level IN ('0', '1', '2')
OR (admin_level IN ('3', '4')
AND EXISTS
(SELECT 1
FROM planet_osm_polygon i
WHERE boundary = 'administrative'
AND admin_level = '2'
AND way_area > 9e+12
AND ST_Within(o.way, i.way)
AND osm_id < 0
)
)
)
AND osm_id < 0
ORDER BY admin_level DESC
) AS admin_very_low_zoom

I suspect this needs to be tested further with databases containing more representative data - it appears the query runs fine when there's a small amount of boundary-only data, but not when the tables contain lots of non-boundary data too. I think @pnorman is making some investigations regarding the query and the behaviour of the query planner.

Owner

gravitystorm commented Dec 1, 2015

I had to revert this pull request, since it was bringing the OSMF servers to a halt (see the revert PR #1987 ). This is the query that had been running for more than 14 hours for one request:

(SELECT
way,
admin_level
FROM planet_osm_roads o
WHERE boundary = 'administrative'
AND (admin_level IN ('0', '1', '2')
OR (admin_level IN ('3', '4')
AND EXISTS
(SELECT 1
FROM planet_osm_polygon i
WHERE boundary = 'administrative'
AND admin_level = '2'
AND way_area > 9e+12
AND ST_Within(o.way, i.way)
AND osm_id < 0
)
)
)
AND osm_id < 0
ORDER BY admin_level DESC
) AS admin_very_low_zoom

I suspect this needs to be tested further with databases containing more representative data - it appears the query runs fine when there's a small amount of boundary-only data, but not when the tables contain lots of non-boundary data too. I think @pnorman is making some investigations regarding the query and the behaviour of the query planner.

@imagico

This comment has been minimized.

Show comment
Hide comment
@imagico

imagico Dec 1, 2015

Collaborator

The shapefile layer changes which were positive on their own could be restored independently by the way.

Collaborator

imagico commented Dec 1, 2015

The shapefile layer changes which were positive on their own could be restored independently by the way.

@pnorman

This comment has been minimized.

Show comment
Hide comment
@pnorman

pnorman Dec 1, 2015

Collaborator

One of the longest queries was

SELECT ST_AsBinary("way") AS geom,"admin_level" FROM (SELECT
    way,
    admin_level
  FROM planet_osm_roads o
  WHERE boundary = 'administrative'
    AND (admin_level IN ('0', '1', '2')
      OR (admin_level IN ('3', '4')
        AND EXISTS
          (SELECT 1
            FROM planet_osm_polygon i
            WHERE boundary = 'administrative'
              AND admin_level = '2'
              AND way_area > 9e+12
              AND ST_Within(o.way, i.way)
              AND osm_id < 0
          )
      )
    )
    AND osm_id < 0
  ORDER BY admin_level DESC
  ) AS admin_very_low_zoom WHERE "way" && ST_SetSRID('BOX3D(-1252344.271424999 -1252344.271424999,20037508 20037508)'::box3d, 900913);

This is the z4 query, which gets z2 admin or z4 admin in large z2.

The query plan on one of the servers was http://explain.depesz.com/s/VgNj

The query plan is moderately stupid, involving a bitmap and of a 20GB gist and 5GB btree index for the subplan, and isn't great for the roads part of the plan, though indexes are smaller there.

Collaborator

pnorman commented Dec 1, 2015

One of the longest queries was

SELECT ST_AsBinary("way") AS geom,"admin_level" FROM (SELECT
    way,
    admin_level
  FROM planet_osm_roads o
  WHERE boundary = 'administrative'
    AND (admin_level IN ('0', '1', '2')
      OR (admin_level IN ('3', '4')
        AND EXISTS
          (SELECT 1
            FROM planet_osm_polygon i
            WHERE boundary = 'administrative'
              AND admin_level = '2'
              AND way_area > 9e+12
              AND ST_Within(o.way, i.way)
              AND osm_id < 0
          )
      )
    )
    AND osm_id < 0
  ORDER BY admin_level DESC
  ) AS admin_very_low_zoom WHERE "way" && ST_SetSRID('BOX3D(-1252344.271424999 -1252344.271424999,20037508 20037508)'::box3d, 900913);

This is the z4 query, which gets z2 admin or z4 admin in large z2.

The query plan on one of the servers was http://explain.depesz.com/s/VgNj

The query plan is moderately stupid, involving a bitmap and of a 20GB gist and 5GB btree index for the subplan, and isn't great for the roads part of the plan, though indexes are smaller there.

@pnorman

This comment has been minimized.

Show comment
Hide comment
@pnorman

pnorman Dec 1, 2015

Collaborator

Running on my server with only standard indexes, I get

http://explain.depesz.com/s/laJE

30 minute query time isn't great, but there are only 4 z4 metatiles, and this is the big one that covers most of Europe.

The query on production with the different plan was running for >14h before terminated.

Collaborator

pnorman commented Dec 1, 2015

Running on my server with only standard indexes, I get

http://explain.depesz.com/s/laJE

30 minute query time isn't great, but there are only 4 z4 metatiles, and this is the big one that covers most of Europe.

The query on production with the different plan was running for >14h before terminated.

@matthijsmelissen

This comment has been minimized.

Show comment
Hide comment
@matthijsmelissen

matthijsmelissen Dec 1, 2015

Collaborator

Is that a server with full planet loaded?

Collaborator

matthijsmelissen commented Dec 1, 2015

Is that a server with full planet loaded?

@pnorman

This comment has been minimized.

Show comment
Hide comment
@pnorman

pnorman Dec 1, 2015

Collaborator

Yes. My setup has a few differences

  • Single-user, faster CPU, but much less RAM. Probably not significant
  • PostgreSQL 9.4 instead of 9.1 (?). (Actually, it's the same version)
  • No btree index on planet_osm_roads (osm_id). I do have one on planet_osm_polygon (osm_id)
  • Data from this summer. Not a significant difference
  • No slim tables. Doesn't matter
  • No updates to the render tables. Not significant, though it does make it probably 25-75% faster without bloat

Since the query takes >14h to run on production, I'm not going to get any selectivity estimates to see why it's doing the BitmapAnd plan or if it's a good plan, but my instincts are telling me it's a horrible plan.

My inclination is to fix this through #207, because a suitable partial index brings query times down into the 1 minute range, and even an ill-suited one is way better.

Collaborator

pnorman commented Dec 1, 2015

Yes. My setup has a few differences

  • Single-user, faster CPU, but much less RAM. Probably not significant
  • PostgreSQL 9.4 instead of 9.1 (?). (Actually, it's the same version)
  • No btree index on planet_osm_roads (osm_id). I do have one on planet_osm_polygon (osm_id)
  • Data from this summer. Not a significant difference
  • No slim tables. Doesn't matter
  • No updates to the render tables. Not significant, though it does make it probably 25-75% faster without bloat

Since the query takes >14h to run on production, I'm not going to get any selectivity estimates to see why it's doing the BitmapAnd plan or if it's a good plan, but my instincts are telling me it's a horrible plan.

My inclination is to fix this through #207, because a suitable partial index brings query times down into the 1 minute range, and even an ill-suited one is way better.

@nebulon42

This comment has been minimized.

Show comment
Hide comment
@nebulon42

nebulon42 Dec 1, 2015

Contributor

Oh dear, I will provide a new PR with just the style updates as I think these are worthwhile to have in also without the new logic on z4.

Contributor

nebulon42 commented Dec 1, 2015

Oh dear, I will provide a new PR with just the style updates as I think these are worthwhile to have in also without the new logic on z4.

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