osmandapp / OsmAnd

OsmAnd
https://osmand.net
Other
4.64k stars 1.01k forks source link

Discard erroneous layer=* tags when generating vector maps #1615

Closed ghost closed 9 years ago

ghost commented 9 years ago

The layer=* tag is defined in the wiki to be valid only in combination with certain other tags such as bridge, tunnel etc. I have been testing and enforcing this rule for over a year and can confirm that over 99 percent of cases where the rule is broken are mapping errors. Those are ignored in mapnik but in OsmAnd they cause "extraneous" tunnels or bridges to appear in places where there aren't any.

The erroneous tags are easy to mechanically filter out (I have osmosis and josm rules) and doing so would fix https://github.com/osmandapp/Osmand/issues/606 as well as slightly reduce size of generated maps.

JOSM search patterns: http://wiki.openstreetmap.org/wiki/User:RicoZ#layer_cleanup

layer=* in wiki: http://wiki.openstreetmap.org/wiki/Key:layer

vshcherb commented 9 years ago

We already did some simplification of layer=* tags, lets ask xmd5a in what situation we ended up. It would be really cool if it could be simplified.

ghost commented 9 years ago

There is still some very low hanging fruit especially with waterways ("help why is my river in a tunnel" - http://osmand.net/go?lat=47.570892&lon=11.430225&z=18) . This is immediately doable, will slightly reduce size of vector maps and improve rendering to match what mapnik does in such cases.

Ready to help out, even some coding but no idea where to start.

xmd5a2 commented 9 years ago

Already done commit, but this change needs careful testing. https://github.com/osmandapp/OsmAnd-resources/commit/2905bd68736c338eb9d1efcbf47dbba3a1d1b4bc

ghost commented 9 years ago

The render.xml changes are obvious, don't know enough about rendering_types.xml at the moment.

There is also the possibility to discard the irrelevant layer tags with a simple osmosis -tt filter as a preprocessing step to map generation - this would reduce size of maps and for waterways I have it 100% tested to result in mapnik behavior. This is something that we developed long time ago for OpenAndroMaps:

<?xml version="1.0"?>
<translations>
        <translation>
                <name>Fix Layers on waterways </name>
                <description>Adjust </description>
                <match type="way" mode="and">
                        <tag k="waterway" v=".*"/>
                        <tag k="layer" v="-1"/>
                        <notag k="culvert" v="yes"/>
                        <notag k="covered" v="yes"/>
                        <notag k="pipeline" v="yes"/>
                        <notag k="location" v="underground"/>
                        <notag k="tunnel" v=".*"/>
                </match>
                <output>
                        <copy-all/>
                        <tag k="layer" v="0"/>
                </output>
        </translation>
</translations>

It can be generalized to other layers and high/railways but this will already catch a vast amount of useless data.

xmd5a2 commented 9 years ago

We can use layer for rendering order. Let's keep it.

ghost commented 9 years ago

Those are invalid layer tags that would get filtered out, I routinely use such filter to clean areas in OSM and it never broke anything. The default rendering - highways over waterways - will kick in and result in better rendering than with those erroneous layer tags.

Granted, when doing this on OSM data I am careful to review the changes and fix unrelated errors that I notice but data consumers like Osmand can do it mechanically on their copy of data without breaking anything.

aceman444 commented 9 years ago

Well, I think OsmAnd drawing something resembling a bridge just because there is layer=1 on the object is kinda a strange decision that is not based on the wiki. Also rendering e.g. waterways with layer=-1 as dashed (as if underground or in a tunnel) is incorrect assumption. Yes, that is issue #606. But that does not mean some data/tagging is incorrect. But of course filtering out data that OsmAnd does not need/support to shrink maps is fine with me (unless custom rendering styles want to see those original tags).

ghost commented 9 years ago

The strange rendering should be fixed with the already comitted patch - is it in the "normal" nightly builds so everyone could easily test it?

The "isolated" layer tags are indeed incorrect, the layer wiki page says in the overview "With some exceptions, layer=* on ways should be used only in combination with one of tunnel=, bridge=, highway=steps, highway=elevator, covered=* or indoor=yes. For Area areas, it could be used in combination with tags such as man_made=bridge, building=* and similar. "

For waterways the layer=-1 tags not matching these criteria can be safely discarded because they are meaningless and the renderer will give much better results when using the default way over waterway layering. In fact I am pretty sure for waterways even ALL "correct" layer=-1 tags could be safely discarded because the renderer would still do the right thing and draw them bellow highways/railways resulting in more significant savings. Mapnik does something like that, for other reasons.

For ways other than waterways those can not be quite as easily discarded because in some cases a bridge/tunnel may be missing but the layer tag correct, in that situation the renderer would be missing valuable information. It is also much rarer to find stray layer tags on highways or railways so any kind of space saving would be totally irelevant in such cases.

aceman444 commented 9 years ago

If you discard layer=-1 on waterways you must add code in the renderer to be sure bridges are draws on top of the river, as in that case usually the bridges have no layer specified. So you will still have to keep special handling somewhere.

ghost commented 9 years ago

No special code needed - it is already there because it is a very common case. Consider http://osmand.net/go?lat=46.40669&lon=11.247631&z=21 the bridge http://www.openstreetmap.org/way/348171636#map=19/46.40649/11.24823&layers=D

Neither the bridge nor the waterway have a layer tag and Osmand is still displaying them as expected.

xmd5a2 commented 9 years ago

These tiles from osmand.net are rendered using mapnik. But yes, OsmAnd itself renders it correctly.

aceman444 commented 9 years ago

What just means the special code is already in. Something must say to draw a road over a river, if layers are equal. On all other objects having same layer, the order (what is on top) could be undefined.

xmd5a2 commented 9 years ago

Yes, highways with layer=1 have more priority https://github.com/osmandapp/OsmAnd-resources/blob/master/rendering_styles/default.render.xml#L1879

aceman444 commented 9 years ago

My example was that the river has layer=-1 and then the roads have no layer set (layer=0 is assumed), in that case, the order is defined. But if you automatically remove the layer=-1 now suddenly both have layer=0 and you probably need to have special code that says, if 2 objects have the same layer, but one is a waterway and one is a road, then draw road on top. It no longer automatically infers from the different layers set.

aceman444 commented 9 years ago

Setting of layer=-1 on the whole river is wrong per the wiki, but people often use it exactly to save putting layer=1 on all the bridges over it.

ghost commented 9 years ago

@aceman444 : you would be surprised how utterly useless "layer" really is for rendering and especially waterways. Consider the bridge http://www.openstreetmap.org/way/37264089#map=19/45.86529/11.17610&layers=D the bridge has layer=-1, while the stream "below" it has no layer (hence layer=0)

Surprisingly both mapnik and Osmand draw this situation correctly - fully ignoring the grossly incorrect layer of the bridge. My common sense tells me it happens to be rendered correctly because every sane renderer draws features similar like this: (area,way,node) X (tunnel, (landcover&landuse&natural), waterway, (highway&railway), (man_made&buidling), bridge).

meaning points over ways over areas AND for overlapping areas or ways rightmost wins

Layer comes into effect only long after this if the above ordering is not sufficient - commonly eg two bridges or two tunnels crossing.

So removing layer=-1 from waterways would affect exclusively the case where two waterways are crossing, neither has a tunnel or bridge tag, and one of them happens to have a "correct" layer tag. However, in that case Osmand will draw the two crossing waterways in a way where it is completely indistinguishable which one is above or bellow whether or not it has a valid layer - you won't see a difference unless there is an aqueduct or tunnel and in that case the aqueduct or tunnel would "overrule" the layer tag anyway.

ghost commented 9 years ago

Sorry, someone fixed that example after I linked it in http://wiki.openstreetmap.org/wiki/Key:layer#Data_consumers but you can see it in the history and it will take a few days till Osmand maps will be updated. Besides, there several thousands other bridges with layer=-1 in Italy.

aceman444 commented 9 years ago

Yeah, so mapnik again hides incorrect data and tries to guess what was meant. Then no wonder there are so many errors than again renderers need to discard data in specific cases because half of it is bad. I do use OsmAnd to spot bad data when in the field so I can survey it immediately so it is good for me if it is shown. And the feature to show FIXMEs is great :)

ghost commented 9 years ago

Osmand displays it just as well as mapnik and did so long time before the recent layer related patch.

It is nice to have a way to spot errors but those are (or should be) easily visible using osmose/keepright. No need to deliberately degrade Osmand performance. If anyone, the desktop should see the bugs but unfortunately it is always so that projects like Osmand, Navit, Mapsforger suffer.

aceman444 commented 9 years ago

Yeah, but OsmAnd is offline (so when in the field), while Osmose/keepright/OSM Inspector is online so I can't survey the reality while sitting at the desktop :)

It depends if OsmAnd wants to cater to the OSM editing users, which I think it does having the OSM edits plugin, showing FIXMEs etc. Or if it just wants to draw nice maps and hide errors (potentially incorrectly) for OSM consumers. Of course a toggle would be best :)

ghost commented 9 years ago

Well you can't use OsmAnd to discover this particular error (bridge & layer=-1) because it never displayed it anyway.

For very long time people could use OsmAnd (and Mapsfroge and similar) to spot the very frequent error with waterway & layer=-1. Despite that it was an extremely irritating error with those apps it does not appear more than 3 users actually went out fixing this in OSM.

Not surprising - OsmAnd and similar apps are not well suited to help this job. It won't display tag/values or raw xml of an object, not even the OSM object id.

What works much better - if I spot something that looks like a systematic error I try to think how it could be detected using rules and file tickets for JOSM validator and Osmose

aceman444 commented 9 years ago

If the bridge with layer=-1 was displayed as dashed (as a tunnel) in OsmAnd, it could have been spotted. Half of my country is tagged with layer=-1 on waterways. But it is not easy to fix because it takes time to mark all the bridges and also people (at least around here) are split on whether it is wrong tagging or not. Yes, the wiki says it is wrong.

ghost commented 9 years ago

Apparently it was not displayed that way in Osmand - I still have OsmAnd 2.1.1 on my phone. Look at http://www.openstreetmap.org/way/28590490#map=19/45.86651/10.93736&layers=D do you see in Osmand anything that would alert you of a grave error like a bridge under a river?

Perhaps the layer=-1 added a tunnel to the way in older Osmand versions which apparently was overridden by the explicit bridge=yes, then natural ordering resulted in "accidentally correct" rendering.

The guys from the Osmose team are very responsive and have better technical possibilities than a handheld renderer and better reach to find and initiate fixing such errors.