Modify

Opened 18 months ago

Closed 14 months ago

Last modified 11 months ago

#13086 closed enhancement (fixed)

[Patch] Detect all self-intersecting ways (multipolygons, less common tags, etc.)

Reported by: naoliv Owned by: team
Priority: normal Milestone: 16.10
Component: Core validator Version:
Keywords: Cc: stephankn

Description

While searching for objects with invalid geometries (using ST_IsValid() with a database) I saw that some objects aren't reported as wrong/invalid in JOSM.

For example, validating the attached file in JOSM nothing is displayed, while I can see a Self-intersection at or near point -49.642125 -21.152750000000001 from ST_IsValid()

Search for problem=yes in the attached file and you should see the intersection.

Shouldn't JOSM catch intersections like this?

JOSM:

Build-Date:2016-06-28 00:41:59
Revision:10501
Is-Local-Build:true

Identification: JOSM/1.5 (10501 SVN pt_BR) Linux Debian GNU/Linux testing (stretch)
Memory Usage: 247 MB / 10206 MB (63 MB allocated, but free)
Java version: 1.8.0_91-8u91-b14-2-b14, Oracle Corporation, OpenJDK 64-Bit Server VM
VM arguments: [-Dawt.useSystemAAFontSettings=on]
Dataset consistency test: No problems found

Attachments (5)

self-intersect.osm (1.3 MB) - added by naoliv 18 months ago.
13086.patch (10.1 KB) - added by GerdP 16 months ago.
13086_v2.patch (10.5 KB) - added by GerdP 15 months ago.
first_and_last.osm (955 bytes) - added by GerdP 14 months ago.
13086_mod.patch (3.3 KB) - added by GerdP 14 months ago.

Download all attachments as: .zip

Change History (27)

Changed 18 months ago by naoliv

Attachment: self-intersect.osm added

comment:1 Changed 18 months ago by mdk

see also #12843

comment:2 Changed 16 months ago by Don-vip

Summary: Not detecting self-intersecting multipolygonsDetect all self-intersecting ways (multipolygons, less common tags, etc.)

comment:3 Changed 16 months ago by Don-vip

Ticket #12843 has been marked as a duplicate of this ticket.

comment:4 Changed 16 months ago by Don-vip

Ticket #13500 has been marked as a duplicate of this ticket.

comment:5 Changed 16 months ago by GerdP

I've already posted a patch for multipolygons, see #13307. I think we have to add a test similar to CrossingWays to the SelfIntersectingWay test and maybe filter those self-intersections in the other tests so that they are not reported twice.
Will look into this.

Changed 16 months ago by GerdP

Attachment: 13086.patch added

comment:6 Changed 16 months ago by GerdP

Summary: Detect all self-intersecting ways (multipolygons, less common tags, etc.)[Patch] Detect all self-intersecting ways (multipolygons, less common tags, etc.)

comment:7 Changed 16 months ago by GerdP

I've added a patch which
1) improves SelfIntersectingWay so that it also finds 2self intersections at first or last node, Unit test added
2) improves CrossingWays to check each way for self-intersection

One open problem: Should we use the same error code for both ? I think no, but did not yet find a good description.
Once the way crosses itself without a node at the intersection, once there is a node.

Will post a modified patch for #13307 as well.

Changed 15 months ago by GerdP

Attachment: 13086_v2.patch added

comment:8 Changed 15 months ago by GerdP

I've noticed that the first patch replaced a warning like "crossing barriers" by "Self-intersecting ways". This was not intended. The new patch fixes this and uses the message "Self-crossing ways" instead of "Self-intersecting ways".
I can also create a unit test similar to that for coastlines if needed.

comment:9 Changed 15 months ago by naoliv

Ticket #13602 has been marked as a duplicate of this ticket.

comment:10 Changed 15 months ago by naoliv

Cc: stephankn added

comment:11 Changed 14 months ago by bastiK

Resolution: fixed
Status: newclosed

In 11136/josm:

applied #13086 - Detect all self-intersecting ways (patch by Gerd Petermann)

comment:12 Changed 14 months ago by Klumbumbus

Milestone: 16.10

comment:13 Changed 14 months ago by Klumbumbus

Resolution: fixed
Status: closedreopened

There is a false positive validator warning for ways which end at a node, which is part of the way and not the first node. This is a common case for highway=* e.g. way/209238741

Since I can't remember to see this warning before I assume this patch introduced it.

comment:14 Changed 14 months ago by bastiK

Agreed, this is generally not an error.

@Gerd Petermann: I think we should revert the part of the patch for SelfIntersectingWay.java and adapt the tests.

comment:15 Changed 14 months ago by GerdP

Yes, I also wondered if highway=* should be treated special. I'll have a look at it tomorrow.

comment:16 Changed 14 months ago by Klumbumbus

This is not specific to highways. I think all linear features can have such a shape (railways, barrier,...).

For linear features it should not warn and for areas and multipolygons such a case will be catched by ohter warnings (uncloses area / unclosed multipolygon). So I don't see a need for a warning for this case.

Changed 14 months ago by GerdP

Attachment: first_and_last.osm added

comment:17 Changed 14 months ago by GerdP

Please comment the case in new attachment first_and_last.osm
Should this be flagged or not? I think if the p-shaped ways are okay than this is also no problem?

Changed 14 months ago by GerdP

Attachment: 13086_mod.patch added

comment:18 Changed 14 months ago by GerdP

The patch 13086_mod.patch allows p-shaped and 8-shaped (as in first_and_last.osm) linear ways when the intersection node is first or last node of the way (or both) and modifies the unit tests accordingly.

comment:19 Changed 14 months ago by bastiK

Resolution: fixed
Status: reopenedclosed

In 11149/josm:

applied #13086 - changed self-intersecting ways detection (patch by Gerd Petermann, minor javadoc changes)

comment:20 in reply to:  18 Changed 14 months ago by bastiK

Replying to Gerd Petermann <gpetermann_muenchen@…>:

The patch 13086_mod.patch allows p-shaped and 8-shaped (as in first_and_last.osm) linear ways when the intersection node is first or last node of the way (or both) and modifies the unit tests accordingly.

Thanks, I would say the 8-shape is unusual, but not necessarily wrong. Maybe a Validator report at INFO-level would be okay. (But not for the P-shaped highways and barriers, ... They are too common.)

comment:21 Changed 14 months ago by Klumbumbus

I think we should keep it as is now. I don't think we need the 8-shape info level warnig. I assume this would produce more false positives than real errors.

comment:22 Changed 11 months ago by Don-vip

Regression in #14202

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.