Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
atiannicelli opened this issue Oct 6, 2022 · 5 comments · May be fixed by #10003
Open

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

atiannicelli opened this issue Oct 6, 2022 · 5 comments · May be fixed by #10003
Labels
bug A bug - let's fix this!

Comments

@atiannicelli
Copy link

atiannicelli commented Oct 6, 2022

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.
  1. 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
Copy link
Collaborator

1ec5 commented Oct 6, 2022

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 tyrasd added the bug A bug - let's fix this! label Oct 11, 2022
tyrasd added a commit to openstreetmap/id-tagging-schema that referenced this issue Oct 11, 2022
@tyrasd
Copy link
Member

tyrasd commented Oct 11, 2022

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:

  • prevent the deletion of the node -> prevents degenerate geometry from being created
  • delete the whole way (like when the third-last node of an area is deleted)
  • convert the closed way into an open one with two nodes A-B
  • (allow the creation of such a degenerate A-B-A line)

@DujaOSM
Copy link

DujaOSM commented Oct 18, 2022

* (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
Copy link
Author

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
Copy link

DujaOSM commented Oct 28, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug - let's fix this!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants