Modify

Opened 8 years ago

Closed 6 years ago

Last modified 6 years ago

#7423 closed defect (fixed)

[patch] Align nodes in circle results are unstable

Reported by: alv Owned by: team
Priority: normal Milestone: 14.03
Component: Core Version: tested
Keywords: align node circle Cc:

Description

Steps to reproduce:

  • select any 3+ node way or enough nodes
  • hit O to align nodes in circle; shape is changed
  • hit O again to align nodes in circle: shape is changed again

What is the expected result:

  • second align nodes in circle should not move anything, because the nodes were just (should have been, anyway) already aligned in a circle.

Attachments (3)

7423_sample.osm.bz2 (550 bytes) - added by skyper 8 years ago.
example file
bug7423.patch (3.5 KB) - added by Balaitous 6 years ago.
since r6920
bug7423.2.patch (4.3 KB) - added by Balaitous 6 years ago.
since r6933

Download all attachments as: .zip

Change History (12)

comment:1 Changed 8 years ago by skyper

Priority: minornormal

I have a real nice example where this action does not work at all

Changed 8 years ago by skyper

Attachment: 7423_sample.osm.bz2 added

example file

comment:2 Changed 6 years ago by Balaitous

Problem is that center is computed as centroid of nodes.
Even if nodes are already aligned in circle, centroid may differ from center.
I propose a last square method based on bisector.

I will put an update of the patch after ticket #8431 will be done.

comment:3 Changed 6 years ago by Balaitous

Summary: Align nodes in circle results are unstable[patch working] Align nodes in circle results are unstable

comment:4 Changed 6 years ago by skyper

Keywords: align node added

comment:5 Changed 6 years ago by Balaitous

Summary: [patch working] Align nodes in circle results are unstable[patch] Align nodes in circle results are unstable

Patch is now OK (and I wish without bugs !!)
I use a least square method to compute center of circle, that ensure exact computation of center if nodes are already align in circle.

Changed 6 years ago by Balaitous

Attachment: bug7423.patch added

since r6920

comment:6 Changed 6 years ago by Don-vip

Milestone: 14.03

Good, but some remarks before committing:

  • I'd prefer a new method in Geometry class, with a clear distinction about the difference between getCentroid and getCenter
  • Can you rephrase the displayed error message to be more user friendly? Something about the number of required nodes for example.

Changed 6 years ago by Balaitous

Attachment: bug7423.2.patch added

since r6933

comment:7 Changed 6 years ago by Balaitous

I have put a new version. getCenter is now in Geometry class.

There remain a bug: result can be strange if you apply AlignInCircle action over a self crossing way.
But it is not a normal "use case", so I don't know if this bug is blocking.

In normal case, it is an improvement, since result is stable if you apply this action twice, and over an arc, the center is at the right place.

comment:8 Changed 6 years ago by Don-vip

Resolution: fixed
Status: newclosed

In 6934/josm:

fix #7423 - Align nodes in circle results are unstable (patch by Balaitous)

comment:9 Changed 6 years ago by Don-vip

thanks!

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.