Modify

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#10821 closed defect (fixed)

[PATCH] ImproveWayAccuracy produces self-intersecting ways

Reported by: akks Owned by: team
Priority: normal Milestone: 14.11
Component: Core Version:
Keywords: ImproveWayAccuracy Cc:

Description

Reported by Vitalts on forum:
http://forum.openstreetmap.org/viewtopic.php?pid=469490#p469490

How to reproduce:

  1. Open attached .osm file
  2. Enter Improve Way Accuracy mode W and hold Ctrl
  3. Move mouse near the east node (see animated gif by Vitalts)

Self-intersecting ways are not expected to appear.

Solution:
There is some rounding error in sin/cos based Geometry.getSegmentAltituteIntersection (transferred from IWA
plugin? misprint also...)

The function does actually the same as closesPointTo but returns null sometimes. It is used only in IWA mode and in NanoLog plugin.

  • One solution is to remove this function and use closestPointToSegment and additional checks.
  • Another solution is to keep separate function but use the code from closestPointToSegment (no other class changing will be neeeded, patch attached)

@team: could someone check the patch with the second solution?

Attachments (4)

IWAbug.osm (747 bytes) - added by akks 6 years ago.
Improve Way Accuracy.gif (145.1 KB) - added by akks 6 years ago.
iwa.patch (3.1 KB) - added by akks 6 years ago.
iwa2.patch (4.0 KB) - added by akks 6 years ago.

Download all attachments as: .zip

Change History (13)

Changed 6 years ago by akks

Attachment: IWAbug.osm added

Changed 6 years ago by akks

Attachment: Improve Way Accuracy.gif added

Changed 6 years ago by akks

Attachment: iwa.patch added

comment:1 Changed 6 years ago by stoecker

If it does the same, drop the function and fix plugin calls. Why keep a duplicate function?

comment:2 Changed 6 years ago by stoecker

The name is anyway wrong "altitute" :-)

comment:3 Changed 6 years ago by akks

Yes, the misprint is also a problem :)

However, closestPointToSegment does not tell if the nearest point is inside the segment (extra unneeded operations to check?), so maybe the separate function is still needed...

Last edited 6 years ago by akks (previous) (diff)

comment:4 Changed 6 years ago by stoecker

If it's only used at one place in core and there the other function is better, then there is no reason to keep it except you find a good reason.

comment:5 Changed 6 years ago by akks

OK, then I'll delete the Geometry.getSegmentAltituteIntersection and fix call in NanoLog this evening.
The patch follows.

Last edited 6 years ago by akks (previous) (diff)

Changed 6 years ago by akks

Attachment: iwa2.patch added

comment:6 Changed 6 years ago by stoecker

Otherwise ьeasurу the angle

Should be "Otherwise measure the angle". But ь isn't m in cyrillic. Simply a typo? :-)

comment:7 Changed 6 years ago by akks

Resolution: fixed
Status: newclosed

In 7776/josm:

fix #10821: ImproveWayAccuracy produced self-intersecting ways
remove Geometry.getSegmentAltituteIntersection method (use closestPointToSegment)

comment:8 Changed 6 years ago by akks

Thank you, fixed the typo and committed. Will fix NanoLog now...

comment:9 Changed 6 years ago by Don-vip

Milestone: 14.11

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.