openstreetmap / iD

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

Create duplicate line segments by deleting node on line. #9329

Open atiannicelli opened 2 years ago

atiannicelli commented 2 years ago

The URL is not great because I fixed the issue there, but if you follow the steps to reproduce you will see the issue.

URL

https://www.openstreetmap.org/way/545586121

How to reproduce the issue?

  1. Click on "line"
  2. create a triangle so that the last node is the same as the first node.
  3. delete one of the nodes. 4) Choose a type for the feature (e.g. "tree row").

At this point you will have a line that references the same node twice.

The change file shows a the two new nodes and one node being referenced twice.

Screenshot(s) or anything else?

<osmChange version="0.6" generator="iD">
    <create>
        <node id="-24" lon="116.36601429790252" lat="39.98403808057159" version="0"/>
        <node id="-25" lon="116.36629195628717" lat="39.98402975391612" version="0"/>
        <way id="-4" version="0"><nd ref="-24"/><nd ref="-25"/><nd ref="-24"/>
            <tag k="natural" v="tree_row"/>
        </way>
    </create>
    <modify/>
    <delete if-unused="true"/>
</osmChange>

Which deployed environments do you see the issue in?

Released version at openstreetmap.org/edit, RapiD version at mapwith.ai/rapid

What version numbers does this issue effect?

2.22.0

Which browsers are you seeing this problem on?

Chrome

1ec5 commented 2 years ago

Incidentally, I had been taking advantage of this behavior to map double-sided curbs, but apparently OSM Inspector complained, causing some mappers to turn them into single-sided curbs. I wound up splitting the way so that there are two coincident ways facing in opposite directions. However, now I notice that there’s a two_sided=yes key. We should add a field and two-sided rendering for that key if we take away the ability to degenerate a way in this manner.

tyrasd commented 2 years ago

the question that remains is what iD should do in such a situation with the closed way triangle A-B-C-A when deleting node C:

DujaOSM commented 2 years ago
* (allow the creation of such a degenerate A-B-A line)

This, but then it has to have a proper geometry validation that won't let you commit the changeset. iD still lets many invalid area geometries to be committed, and they can only be detected post-hoc by QA tools such as OSM Inspectors. I reported similar ones at #9154 and #8167.

atiannicelli commented 2 years ago

Ya, IF there is the ability to disallow the commit of a malformed feature then I vote for

(allow the creation of such a degenerate A-B-A line)

If, however, it is difficult for the editor to disallow this type of thing the next best is to

delete the whole way (like when the third-last node of an area is deleted)

If a user deletes a node and sees the entire object get deleted they will either, 1) be fine with that and move on or 2) undo the delete to figure out what went wrong. I guess there is a third option where they don't realize that the object was deleted... hmmm.

DujaOSM commented 2 years ago

Here's another variation on the theme (your #9328 is yet another):

Start with a concave but valid polygon area, such as the one on the left. Now, delete the bottom node. As the result, you get a self-intersecting area like the one on the right. iD will let you commit. InvalidPolygon