Modify

Opened 5 years ago

Closed 5 years ago

#17768 closed enhancement (fixed)

Use code in MultipolygonTest to improve CreateMultipolygonAction

Reported by: GerdP Owned by: GerdP
Priority: normal Milestone: 19.06
Component: Core Version:
Keywords: Cc:

Description

A few things which are not good:

  • Performance is poor when you update a complex multipolygon (MP) since MultipolygonBuilder uses complex area intersections
  • In special cases a created or updated MP might be flagged as incorrect by validator (MultipolygonTest)

My idea: The calculations in MultipolygonTest are using a faster implementaion to detect the inner and outer ways (as well as geometry errors), it only requires a few lines of code to implement a method makeFromWays(Collection<Way> ways) similar to that in MultipolygonBuilder and all the validation result (list of TestErrors) comes for free.
In a further step I plan to use this to improve Selector.CrossingFinder.

Attachments (2)

17768-beta.patch (11.2 KB ) - added by GerdP 5 years ago.
Proof of concept, I am not yet 100% sure how to handle update of an invalid MP (e.g. node as member)
17768-v0.patch (12.4 KB ) - added by GerdP 5 years ago.
improved version

Download all attachments as: .zip

Change History (9)

by GerdP, 5 years ago

Attachment: 17768-beta.patch added

Proof of concept, I am not yet 100% sure how to handle update of an invalid MP (e.g. node as member)

comment:1 by GerdP, 5 years ago

Owner: changed from team to GerdP
Status: newassigned

comment:2 by GerdP, 5 years ago

In 15141/josm:

see #17767 and #17768: add non regression unit tests
Test testTicket17768() fails with current code and is ignored for now.

comment:3 by GerdP, 5 years ago

I am stepping through the unit test samples in data_nodist\multipolygon.osm
I think the "Update multipolygon" action should also fix some erros, esp.

  • all member roles should be set correctly (okay)
  • area style tags on outer ways should be moved to the mp relation (if correct old style mp) (okay)
  • duplicate members should be removed (not done without the patch)
  • non-Way members should be removed, although they are allowed in type=boundary relations (not done without the patch)
  • invalid geometries should produce a message (okay)
  • relation "06/04 - Mismatching way styles" should produce a warning (not done, also not with the patch)

by GerdP, 5 years ago

Attachment: 17768-v0.patch added

improved version

comment:4 by GerdP, 5 years ago

Resolution: fixed
Status: assignedclosed

In 15160/josm:

fix #17768: Use code in MultipolygonTest to improve CreateMultipolygonAction

  • implement new method makeFromWays() in MultipolygonTest
  • let MultipolygonBuilder.makeFromWays() use the new method, it is much faster and detects more cases where the geometry is invalid.
  • remove now obsolete code in MultipolygonBuilder, add javadoc for some public fields
  • let CreateMultipolygonAction use the new method and add some code to improve user feedback when update removes members or when nothing was changed but the relation is still not a valid multipolygon.
  • let Geometry use final field JoinedPolygon.nodes instead of method JoinedPolygon.getNodes() as this is not a getter but a calculation routine which should be changed later.

comment:5 by GerdP, 5 years ago

In 15162/josm:

see #17768: activate unit test

comment:6 by GerdP, 5 years ago

Resolution: fixed
Status: closedreopened

Create / update multipolygon no longer works when inner rings share segments.

comment:7 by GerdP, 5 years ago

Resolution: fixed
Status: reopenedclosed

In 15183/josm:

fix #17768: Create / Update multipolygon did not work wich touching inner rings

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain GerdP.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.