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

iD encouraging users to create duplicated nodes #9513

Open ltog opened 1 year ago

ltog commented 1 year ago

Description

When looking at POIs that I have created some time ago, I noticed that quite a few of them are duplicated nodes (i.e. two nodes sharing the exact same coordinates).

Example: Original node: https://www.openstreetmap.org/node/6854623478 Duplicated node: https://www.openstreetmap.org/node/7463112270

The process how they develop is like this:

  1. User A maps a building (using a closed way)
  2. User B (me) adds a POI (say a hairdresser) to one of the sides of the buildings as node and makes this node also part of the building way.
  3. User C opens the POI in iD and sees a message: Hairdresser <name_of_hairdresser> should be a standalone point based on its tags together with a yellow warning sign and an explanation Some features shouldn't be part of lines or areas (see screenshot).
  4. User C clicks Extract this point and saves the change.

I disagree on how iD handles this on two levels:

Fundamental level: I believe that attaching nodes to building outlines in many cases is good practice for two reaons:

  1. It makes deriving the postal address of the POI from the address of the building easier compared to the alternative.
  2. If it is necessary to move building and POI, e.g. after noticing an offset in aerial imagery, the relative position of building and POI to each other will typically stay intact. (If they are separate, extra care has to be taken to select and move POI(s) and building together.)

IMHO, iD should not report these cases as issue at all. Note: I searched the wiki but could not find any indication on what method is being preferred.

Implementation level: Even if one considers extracting nodes good practice, I think the way iD handles this, could be improved:

Remark: When iD extracts one or multiple nodes, their number will be visible in the changeset tag resolved:mismatched_geometry:point_as_vertex. This happened first in changeset 75421119 (from 2019-10-08). Until now (2023-02-20) the total number of extracted nodes worldwide is 105'937. The number of duplicated nodes will likely be similar, say 100k. This makes efforts to remove duplicated nodes, such as keepright, moot.

I used the following code to count the number of cases:

#!/usr/bin/env python3

import xml.etree.ElementTree as ET

filename = 'changesets-230220.osm'

tag_sum = 0

for event, elem in ET.iterparse(filename, events=("start", "end")):
    if event == "start":
        if elem.tag == "tag" and elem.attrib.get("k") == "resolved:mismatched_geometry:point_as_vertex":
            tag_sum += int(elem.attrib.get("v", 0))
    elem.clear()

print(tag_sum)

Screenshots

Screenshot at 2023-02-25 13-13-38

1ec5 commented 1 year ago

User B (me) adds a POI (say a hairdresser) to one of the sides of the buildings as node and makes this node also part of the building way. I believe that attaching nodes to building outlines in many cases is good practice for two reaons

I think the warning is specifically about attaching the POI to a vertex of the building area without saying why. It’s much more common to either map it as a standalone point or dual-tag the building (not always applicable, of course), so iD has always supported both mapping styles, as far as I can tell.

To reproduce the state depicted in the screenshot, you would have had to manually add the amenity=cafe tag using the Tags section, or you would’ve had to create it as a standalone node and drag it onto the building’s edge. So by this point, you’re already sort of “fighting against” iD, so to speak, and you may encounter increasingly suboptimal behaviors. This style of mapping isn’t unheard of, but it entails dual-tagging as well: if you intend to place the node at the entrance, then dual-tagging the node as entrance=* would suppress the warning. The UI would present it as an entrance, not a POI, since that’s what it is physically.

After extracting the node, it could be moved a tiny bit: Either automatically (e.g. towards the center/inside of the building) or the user could be prompted to do so.

This is a good idea, consistent with what happens when extracting a POI from a dual-tagged building. It might have to be a special case, because the extract operation can’t normally assume you intended to map something interior to the building. For example, if you map a wall-mounted flagpole but later discover that it’s actually freestanding on the exterior, you would use this same extract operation. To address both cases, iD could enter move mode, where the point floats around until you click on its new location.

ltog commented 1 year ago

I think the warning is specifically about attaching the POI to a vertex of the building area without saying why.

No, it isn't. I should have written this more clearly: User B (me) adds this node using JOSM or Vespucci and the screenshot depicts, what user C would see.

1ec5 commented 1 year ago

Either automatically (e.g. towards the center/inside of the building) or the user could be prompted to do so.

To shift the point automatically, the line below could take the entity’s parent if the entity is a vertex. That would move the node to the centroid, but instead it could move the point along a line perpendicular to the way by a certain amount. I’m not sure if the distance should be fixed or based on the distance to the building’s straight skeleton. Some mappers prefer to eyeball a centroid of the assumed indoor area, while others prefer to place the POI at an arbitrary distance into the building.[^indoor] Ultimately, in this mapping style, it would be up to the user to determine a reasonable location for themselves.

https://github.com/openstreetmap/iD/blob/45c746ef05293eacce52d24e686072da696d6765/modules/actions/extract.js#L49

[^indoor]: If the building is already indoor-mapped, then we wouldn’t be in this situation, because the POI would be dual-tagged on the room area or so.

petrarca-arezzo commented 1 year ago

I would always put that kind of POIs (e.g. hairdresser) within the outline of the building since this is where it is - the hairdresser is not part of the building walls but something within the building. If useful I would also add an entrance to the building.

If the satellite images are off by quite some distance and everything needs to be moved then this would typically affect a whole area (not just one building) and this would be a mass-change to OSM then.