gravitystorm / openstreetmap-carto

A general-purpose OpenStreetMap mapnik style, in CartoCSS
Other
1.53k stars 819 forks source link

Relationship boundaries are rendered twice #344

Closed Circeus closed 9 years ago

Circeus commented 10 years ago

From the code in admin.mss, I can't figure out why, but (as far as I can tell) boundary relationships will be rendered regardless of the tagging on the member ways. Since tagging ways separately with boundary tags will also cause these to be rendered, in many instances you'll get ways that render as boundaries twice. The result is that it is essentially impossible to have meaningful boundary relationships without making it impossible to have consistent rendering between ways with identical tags.

For a simple example: http://www.openstreetmap.org/#map=17/33.81944/-118.16976

The issue is not limited to the interaction of way tags and relationships, since a way that belongs to multiple boundary relationships will still suffer the same problem. Is there any way to limit the rendering at the way level (i.e. check for boundary tags OR membership in a boundary=administrative relationship) rather than rendering from both sides?

dieterdreist commented 10 years ago

2014-02-19 23:26 GMT+01:00 Circeus notifications@github.com:

From the code in admin.mss, I can't figure out why, but boundary relationships will be rendered regardless of the tagging on the member ways. Since tagging ways separately with boundary tags will also cause these to be rendered, in many instances you'll get ways that render as boundaries twice.

maybe they also get rendered three times: in the case of adjacent boundaries (i.e. not a coast boundary) you will get a rendering for each of the adjacent adminitrative entities plus eventually of the way itself (if it has rendered boundary/admin tags as well).

matthijsmelissen commented 10 years ago

I didn't follow the discussion, is this tagging as seen as correct?

Circeus commented 10 years ago

maybe they also get rendered three times: in the case of adjacent boundaries (i.e. not a coast boundary) you will get a rendering for each of the adjacent administrative entities plus eventually of the way itself (if it has rendered boundary/admin tags as well).

Indeed. I revised the post and ended up accidentally deleting a mention of that.

I didn't follow the discussion, is this tagging as seen as correct?

Whether or not tagging the ways is correct is irrelevant since neighboring relationships do generate the issue (though of course having it render 3 times instead of 2 makes it that much more likely to have render issues). Relationships are essentially a required feature for nominatim and related services to work. Having boundary tags on the ways is appropriate because otherwise, you have elements in the database with no tags of their own, which I'm fairly sure is considered a major no-no (and there are many places where relationships have not been implemented, or incompletely etc.)!

matthijsmelissen commented 10 years ago

So that leaves us with solving it in osm2pgsql, the sql queries, or the cartocss, right? Any preferences?

pnorman commented 10 years ago

What is commonly viewed as correct tagging for two administrative entities sharing one boundary is

The ways may have a name tag as well, but this is not the name of the country but the name of the boundary. For example, way 2 is the A/B boundary, the name is not A or B. The usefulness of these is disputed, and their names probably shouldn't be rendered on a general purpose map.

I would probably go with rendering dashed lines from the ways in planet_osm_line. If we decide to render administrative areas, those would be from planet_osm_polygon. I'm not convinced about lower-level administrative areas, having them and place nodes might get confusing.

Circeus commented 10 years ago

The ways may have a name tag as well, but this is not the name of the country but the name of the boundary.

You mean in cases, mostly, where the boundary happens to be another feature (e.g. a road or a river), right?

I'm not convinced about lower-level administrative boundaries, having them and place nodes might get confusing.

Currently, Mapnik renders them all the way down to admin_level=10 and no one complains. The highest levels are usually not dashed, so do not have the issue in the first place! If e choose to not render any boundary below a certain level, this issue (which starts around, I believe, admin_level=6 and lower ranks) becomes moot entirely, but I suspect a bevy of users will complain very loudly.

So that leaves us with solving it in osm2pgsql, the sql queries, or the cartocss, right? Any preferences?

I'm assuming the relationships are rendered from something else than cartocss in the first palce, since (afaict) the CSS makes no reference whatsoever to relations

If we want to promote tags on the boundary segments, the simplest method is brute force: render solely based on way tags and ignore relations (only the administrative ones, leave other type of boundaries alone) as far rendering is concerned. This also solves the related issue that we currently can't prevent a boundary relation name rendering at the centroid (I believe?) even if there is a place= tag connected to that relation Issue (105, I believe). See e.g. here (twin Morsang-sur-orge labels) or here (twin Beverly Hills labels). I'm aware this brings up an issue in cases where there rendering of city names was left entirely to rely on boundary relations, but since those labels not only ignore the place= rendering hierarchy entirely anyway (they only show up, I believe, at zoom 14 and higher!), they are currently an undesirable method of rendering place names.

Otherwise, a method that can detect either the tags on the way or membership in a boundary relationship, and turn on rendering if either condition is met would be needed. I do not have the technical knowledge as to whether tat is possible.

pnorman commented 10 years ago

The ways may have a name tag as well, but this is not the name of the country but the name of the boundary. You mean in cases, mostly, where the boundary happens to be another feature (e.g. a road or a river), right?

It would never be only the name of the country. In the example above, it would be the A/B border, if it were tagged.

I'm not convinced about lower-level administrative boundaries, having them and place nodes might get confusing. Currently, Mapnik renders them all the way down to admin_level=10 and no one complains.

Whoops - I was speaking of administrative areas, and have corrected my earlier message. They're not currently rendered except when they trigger the catch-all.

I'd support removing admin boundaries from the catchall after we implement #103, but #104 is the issue for that while this is the issue for the lines, so let's carry any discussion about rendering the areas over there.

I'm assuming the relationships are rendered from something else than cartocss in the first palce, since (afaict) the CSS makes no reference whatsoever to relations

No - all boundaries are from cartocss and the osm2pgsql database, the same way roads and such are, with the exception of boundaries at low zooms (z1-z3?)

Fixing the dashed lines shouldn't be too hard, but the names on the lines which are also a mess require more work, since they're from the catchall afaik.

Circeus commented 10 years ago

Honestly I don't care much for the names-on-line issue (which is as much a poor tagging issue as it is a rendering issue).

No - all boundaries are from cartocss and the osm2pgsql database, the same way roads and such are, with the exception of boundaries at low zooms (z1-z3?)

Then tell me with part of admin.mss is responsible for causing boundaries defined solely by relationships (with no boundary tags on the way) to render? I'm curious, since I couldn't figure out at all looking at the code 9which refers only to ways AFAICT). This comment clearly proves that not all rendering instructions are to be found in the CSS.

cquest commented 10 years ago

Improving boundary rendering has been on my agenda for some time (and still is).

Currently, boundaries are rendered from planet_osm_polygon, not from planet_osm_line and these polygons are coming from osm2pgsql processing of relations while lines are coming from ways.

This causes 2 problems:

To improve it, switching the the planet_osm_line is a option, but you'll face:

The way to solve this I've explored, is to postprocess the boundary polygons to recreate linestrings. I've worked on this for another purpose (create a topologically correct simplified boundary set for french municipalities), but the results my be reused. It could be maintained up-to-date using a trigger (not tested).

This kind of pre/post processing is a nice thing to do on renderings that aim the best graphical output despite data inconsistency, but I doubt this should be the goal of the OSM default rendering that needs to be showing data as the are and not as they should be.

pnorman commented 10 years ago

This kind of pre/post processing is a nice thing to do on renderings that aim the best graphical output despite data inconsistency, but I doubt this should be the goal of the OSM default rendering that needs to be showing data as the are and not as they should be.

Yes - that's my view.

pnorman commented 10 years ago

So I had a try in tilemill, the trick seems to add an osm_id > 0 restriction. PR soon.

gravitystorm commented 10 years ago

I must confess to having read and re-read the conversation and I'm frequently lost as to what is going on.

I've made various changes today that affect the rendering of boundaries. Perhaps if I just state my goals it will make things clearer:

For example

    Canada
 -- 2 -- 2 -- 2 -- 2 --
   United States

I don't want the text written on top of the boundary, and I don't want the boundary line to be mis-mash of different admin_levels drawn over the top of one another.

Now how we manage this is a different topic :-)

pnorman commented 10 years ago

348 solves the line problem. I'll rebase it and re-test

I can probably get us POLYGONs instead of LINESTRINGs for the labels if that would help

Circeus commented 10 years ago

Each boundary should be rendered once, with the highest admin level (i.e. small number) of that particular line, whether from tags on ways or tags on relations

That was indeed the basic gist of my original report. I got sidetracked along the way because there are multiple issues related to boundary rendering (the whole generating labels from relations, for example, which I believe will never be viable without a way to custom-place that label), but your comment captures exactly what I hoped to get.

gravitystorm commented 10 years ago

That was indeed the basic gist of my original report.

Good stuff. This is all fixed as a combination of b0842bd7a34d7616 84daae0fc859ac7 and f0d0c9e33e1e

Circeus commented 10 years ago

If the commit isn't going through, then his is NOT closed in any way or form and needs to be reopened.

matthijsmelissen commented 10 years ago

Is this the same issue as https://trac.openstreetmap.org/ticket/2094?

Circeus commented 10 years ago

Yes, clearly so.

Circeus commented 10 years ago

Accidentally used the wrong button >>;;;

pnorman commented 10 years ago

Looked at this a bit. Currently a lot of boundaries are missing admin_level tagging on the ways, posing problems for only rendering from the linear features.

I do not believe slicing admin boundaries at run time to reconstruct this missing information is possible to do for performance reasons.

Mapbox takes the approach of pre-processing to reconstruct what should be the way tags. The approach could be adapted, but then we're introducing another process like coastlines, and of similar complexity.

Thoughts?

pnorman commented 10 years ago

http://www.itoworld.com/map/2 indicates where boundaries are broken - anywhere in grey is an admin boundary without an amin_level tag. It looks like Poland is the major country with missing tags on ways.

@balrog-kun, thoughts on how to fix the data, as you were the reporter of problems in #348?

balrog-kun commented 10 years ago

Just saw the comment, sorry.

With the slim tables it is possible to create an index telling you the lowest admin_level of all relations containing given way, something similar is done for hiking route relations for drawing all the route's colours as a parallel lines. But I guess the generic style should not assume slim mode tables being present.

Here's a slightly hacky idea that should help with the dashed lines appearing one on top of another but I can't test it at this time.

pnorman commented 10 years ago

Ya, we can't depend on slim mode.

But what I was wondering about was a strategy for fixing the data in the regions with missing tags on ways.

balrog-kun commented 10 years ago

@pnorman Sorry didn't realise that admin_levels on ways were considered correct tagging. The difference in Poland probably is a result of people documenting this on the forums as incorrect tagging and not encouraging it (I had stopped mapping boundaries earlier). Adding these tags as a big mechanical edit is technically easy, even with just JOSM and its search tool, not sure about other aspects of mechanical edits.

matthijsmelissen commented 9 years ago

Solved by #1107 some time ago.