Modify

Opened 3 years ago

Closed 3 years ago

#12377 closed defect (fixed)

validator does not detect intersecting rings in multipolygon

Reported by: cmuelle8 Owned by: team
Priority: normal Milestone: 16.02
Component: Core validator Version: latest
Keywords: template_report multipolygon intersecting Cc:

Description

Find test case attached. Additionally, touching inner rings should be warned about, as this will lead to "boundary" segments of the area not bounding the area, i.e. segments that have "outside" on both sides and hence are not boundaries. This contradicts every reasonable understanding of what an area is.

The least common denominator is, that an area has an exact outline enclosing it (it does not matter if there are more than one patches, or even holes, for this). By declaring touching inner rings to be a valid part of multipolygons, we define outline parts that cannot be an outline to be a valid outline - which is, erm, complete nonsense.

Build-Date:2016-01-14 22:06:58
Revision:9449
Is-Local-Build:true

Identification: JOSM/1.5 (9449 SVN de) Linux Ubuntu 15.10
Memory Usage: 630 MB / 1609 MB (113 MB allocated, but free)
Java version: 1.8.0_66-internal-b17, Oracle Corporation, OpenJDK 64-Bit Server VM
Dataset consistency test: No problems found

Plugins:
- PicLayer (31895)
- RoadSigns (31895)
- alignways (31895)
- apache-commons (31895)
- editgpx (31106)
- imagery_offset_db (31895)
- kendzi3d (1.0.189)
- kendzi3d-jogl (41)
- kendzi3d-resources (0.0.1)
- log4j (31895)
- openvisible (31106)
- pbf (31772)
- photo_geotagging (31895)
- photoadjust (31963)
- poly (31772)
- print (31895)
- reverter (31926)
- turnlanes (31772)
- turnrestrictions (31895)
- utilsplugin2 (31895)
- wikipedia (31917)

Attachments (4)

invalid-mp-not-detected-by-validator.osm (2.4 KB) - added by cmuelle8 3 years ago.
invalid-mp-not-detected-by-validator.osm
intersecting-touching-inner-rings-within-mp_undetected-by-validator.osm (2.8 KB) - added by cmuelle8 3 years ago.
intersecting-touching-inner-rings-within-mp_undetected-by-validator.osm
intersecting-touching-inner-rings-within-mp_assigned-wrong-error-group-by-validator.osm (2.8 KB) - added by cmuelle8 3 years ago.
intersecting-touching-inner-rings-within-mp_assigned-wrong-error-group-by-validator.osm
josm-mp-outer-intersect.osm (1.7 KB) - added by cmuelle8 3 years ago.
josm-mp-outer-intersect.osm

Download all attachments as: .zip

Change History (19)

Changed 3 years ago by cmuelle8

invalid-mp-not-detected-by-validator.osm

comment:1 Changed 3 years ago by stoecker

Touching inner rings are a valid part of the multipolygon definition.

comment:2 Changed 3 years ago by Don-vip

Resolution: invalid
Status: newclosed

comment:3 Changed 3 years ago by stoecker

Resolution: invalid
Status: closedreopened

That was too early. The test case contains intersecting inner rings and that's clearly invalid.

comment:4 Changed 3 years ago by Don-vip

Milestone: 16.02

Exact.

comment:5 Changed 3 years ago by Don-vip

In 9490/josm:

see #12377 - add multipolygon unit test

comment:6 in reply to:  1 ; Changed 3 years ago by cmuelle8

Replying to stoecker:

Touching inner rings are a valid part of the multipolygon definition.

Earth is flat, a flat. And beauty is in the eye of the beholder.

comment:7 in reply to:  6 Changed 3 years ago by stoecker

Replying to cmuelle8:

Replying to stoecker:

Touching inner rings are a valid part of the multipolygon definition.

Earth is flat, a flat. And beauty is in the eye of the beholder.

And the chance that what you say is seriously considered are going down to zero.

Changed 3 years ago by cmuelle8

intersecting-touching-inner-rings-within-mp_undetected-by-validator.osm

Changed 3 years ago by cmuelle8

intersecting-touching-inner-rings-within-mp_assigned-wrong-error-group-by-validator.osm

comment:8 Changed 3 years ago by cmuelle8

I've added a couple more test cases to test a potential fix against.

They only differ in one respect: which of the inner rings contains the other.

We would expect that both cases are handled equally by the validator, but they are not:

  1. The first example is detected valid, no warnings or errors shown.
  1. The second, very similar example, suggests the role of the smaller inner ring to be 'outer'.

I do not, among others in the community, endorse MPs with touching inner rings. Ideally the people that want to have them should fix the validator to produce a usable classification themselves. This should be no problem, because "The [current] multipolygon definition [read:documentation] was designed, accepted and implemented in a long process." with "no vote [or friendly comments] required." ( Citations are taken from http://wiki.openstreetmap.org/wiki/Talk:Relation:multipolygon#Touching_inner_rings )

comment:9 Changed 3 years ago by stoecker

In 9553/josm:

see #12377 - added relevant way to the validator message

comment:10 Changed 3 years ago by Don-vip

In 9582/josm:

see #12377 - code refactoring to cleanup multipolygon test

comment:11 Changed 3 years ago by Don-vip

In 9583/josm:

see #12377 - simplify multipolygon geometry checks

comment:12 Changed 3 years ago by Don-vip

Resolution: fixed
Status: reopenedclosed

In 9584/josm:

fix #12377 - detect intersecting rings in multipolygon validator test

comment:13 Changed 3 years ago by cmuelle8

Thanks for your efforts, the code is very readable now.

However it still feels not right, to catch intersecting outer polygons using the MultipolygonBuilder,
but then test intersecting inner polygons in the validator.

comment:14 Changed 3 years ago by cmuelle8

Resolution: fixed
Status: closedreopened

Actually the validator has to check intersecting outer rings as well.

Even though the multipolygon builder (Strg+B) will refuse to create them, a valid MP may be invalidated by e.g. moving a node of a valid outer polygon into another outer polygon. The validator does not recognize this currently. We also need to include this case into unit test, it's very basic.

Changed 3 years ago by cmuelle8

Attachment: josm-mp-outer-intersect.osm added

josm-mp-outer-intersect.osm

comment:15 Changed 3 years ago by Don-vip

Resolution: fixed
Status: reopenedclosed

In 9900/josm:

fix #12377 - detect intersecting outer members in multipolygons

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.