openstreetmap / iD

🆔 The easy-to-use OpenStreetMap editor in JavaScript.
https://www.openstreetmap.org/edit?editor=id
ISC License
3.35k stars 1.2k forks source link

Hiding landuse multipolygons will also hide shared features that make up the multipolygon #2887

Closed bagage closed 8 years ago

bagage commented 8 years ago

Some routes are borders of a landuse area (nodes are merged together). Unfortunately when you want to hide these areas, it will also hide the route. Steps to reproduce:

  1. Go to this position
  2. Open Map Features menu
  3. Uncheck Landuse Features
  4. Both the yellow (Scrub) area AND a part of Rue Pierre Latécoère way will disappear. I expected only the first one to disappear here?
slhh commented 8 years ago

Hiding landuse seem to hide all outer of landuse multipolygons even if these are tagged as highway or railway. I think the title of this issue is slightly wrong, not routes but simple highway ways seem to be meant.

bhousel commented 8 years ago

Yes the feature matching code is intentionally greedy, meaning that if a feature matches multiple filter rules, any of them can be used to hide the feature. For example to hide Abandoned Railways, you can hide it either by filtering "Past/Future" or by filtering "Rail Features".

In the example above, way w221066703 Rue Pierre Latécoère is tagged as both a highway=unclassified and also is used as part of the outer way of r2922707, a natural=scrub multipolygon. So either the "Minor Road" or "Landuse" rule will hide it.

slhh commented 8 years ago

It might be a side effect of matching code being intentional greedy, but it is still a severe bug which can led to adding duplicates to the database. It is not the behavior expected by the user. Hiding objects unexpectedly is risky, because the user might add the object again.

Your Abandoned Railway example is quite different because of two properties of the same feature may led to hiding the feature. In this example it is like the behaviour beeing expected by the user.

In the reported landuse/highway case there are two different features which are only sharing OSM objects for its definition. Hiding one of these features based on the property of the other feature is an unexpected behaviour. In addition this behaviour seems to be useless and bad for usability.

The reported landuse/highway case is more compareable to a highway and a railway sharing some nodes. In this case iD doesn't hide the shared nodes.

Because iD is en editor and not simply a map I suggest some deviation from the behavior expected by the user: If a potentially hidden feature is sharing a way or node with a feature which is not hidden, the potentially hidden feature should not be hidden but dimmed. This dimmed feature should not be selectable in general. Single click selection of the dimmed object might be enabled where no other active object is selectable. Connecting objects to the dimmed object should be prevented except of connecting to shared nodes or shared ways and maybe shared edges. In case of trying to connect to a shared edge, which is not part of a shared way, iD should preferably ask if the user wants to connect to dimmed objects too. This deviation from the behavior expected would be an improvent of usability preventing unintended side effects of changes to hidden objects. Showing objects unexpectedly seems to be much less risky than hiding objects unexpectedly, provided that shift+drag selection is prohibited.

bhousel commented 8 years ago

People should really tag each feature as a separate element rather than trying to create multipolygons out of different features.

http://wiki.openstreetmap.org/wiki/One_feature,_one_OSM_element

I can promise you it is easier to fix the multipolygon than it is to fix iD to do all that stuff you asked for.
For starters..

If a potentially hidden feature is sharing a way or node with a feature which is not hidden,

This is an unanswerable question. Knowing whether an OSM relation contains sub-members which should cause it to be shown / hidden / rendered differently / treated special is essentially the halting problem.

pnorman commented 8 years ago

I can promise you it is easier to fix the multipolygon than it is to fix iD to do all that stuff you asked for.

In the example above, way w221066703 Rue Pierre Latécoère is tagged as both a highway=unclassified and also is used as part of the outer way of r2922707, a natural=scrub multipolygon. So either the "Minor Road" or "Landuse" rule will hide it.

The multipolygon you've described is correct. I try to avoid using roads, but I commonly create a situation such as where you have w1 and w2 forming a closed loop, w1 with barrier=fence and a relation with members w1 and w2 and tags type=multipolygon landuse=residential. There are two features, two OSM objects.

bhousel commented 8 years ago

Reopening because I went for a run and thought about this some more, and I think I can put in an easy fix.. We already have code to handle multipolygon inners, can put the change in there.

bhousel commented 8 years ago

The multipolygon you've described is correct.

I agree that it's a valid multipolygon..

I try to avoid using roads, but I commonly create a situation such as where you have w1 and w2 forming a closed loop, w1 with barrier=fence and a relation with members w1 and w2 and tags type=multipolygon landuse=residential. There are two features, two OSM objects.

Yeah it makes sense that a landuse can abut a fence. Not so much a road centerline :wink:

pnorman commented 8 years ago

Yeah it makes sense that a landuse can abut a fence. Not so much a road centerline

Landuse is sometimes carried out all the way to the centerline. I don't like that style anymore since it is a pain to maintain, even if produces generally superior cartography. I might do it to a footpath or similar, but generally there's a fence around here, so it hasn't been something I've thought about.

In any case, the problem is generic and applies to both roads and fences ;)

See http://master.apis.dev.openstreetmap.org/edit?editor=id#map=17/49.24644/-122.94796 for a test area