Modify

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#9605 closed defect (fixed)

[patch] Align Nodes in Line moves outer node instead of middle one

Reported by: dforsi Owned by: team
Priority: normal Milestone: 14.03
Component: Core Version: latest
Keywords: Cc:

Description

The attached .osm file contains a way with 3 nodes, no matter in which order I select them, when I use the "L" tool, the topmost node (id 2373880100) gets moved and aligned with the other two, creating what seems to be a self-intersecting way, while I would expect that the middle node (id 2556897398) gets moved; expected behaviour happens if I move the nodes around.

I tried with version 6502 (tested) and 6750 (latest).

Attachments (5)

linearize.osm (937 bytes) - added by dforsi 6 years ago.
test case for bug #9605
bug9605.patch (3.1 KB) - added by Balaitous 6 years ago.
Update of patch after r6893
test_case_9605_2.osm (1.3 KB) - added by oligo 6 years ago.
bug9605_2.patch (8.0 KB) - added by oligo 6 years ago.
AlignInLineActionTest.java (6.7 KB) - added by oligo 6 years ago.

Download all attachments as: .zip

Change History (17)

Changed 6 years ago by dforsi

Attachment: linearize.osm added

test case for bug #9605

comment:1 Changed 6 years ago by Don-vip

Milestone: 14.02

comment:2 Changed 6 years ago by simon04

From source:trunk/src/org/openstreetmap/josm/actions/AlignInLineAction.java:

        // Find from the selected nodes two that are the furthest apart.

So code-wise this behaviour is intended. Not sure whether that is very useful or not …

comment:3 Changed 6 years ago by dforsi

In my opinion it isn't useful if you have 3 consecutive nodes belonging to the same way (like in the file that I attached in the first comment) because the anchors are by definition the nodes at the extremities, because if you choose the middle node as an anchor then you get a self overlapping way.
Not sure how it extends to more complex situations, but it seems to me that no matter how many nodes are selected, if they belong to the same way it is desiderable that their new coordinates are related to the order in which the nodes appear in the way.

comment:4 Changed 6 years ago by Don-vip

Milestone: 14.0214.03

did not have enough time to work on this one.

comment:5 Changed 6 years ago by Balaitous

Summary: Align Nodes in Line moves outer node instead of middle one[patch] Align Nodes in Line moves outer node instead of middle one

Changed 6 years ago by Balaitous

Attachment: bug9605.patch added

Update of patch after r6893

comment:6 Changed 6 years ago by bastiK

Resolution: fixed
Status: newclosed

In 6894/josm:

applied #9605 - Align Nodes in Line moves outer node instead of middle one (patch by Balaitous)

comment:7 Changed 6 years ago by bastiK

I like the improvements, you are doing!

Changed 6 years ago by oligo

Attachment: test_case_9605_2.osm added

Changed 6 years ago by oligo

Attachment: bug9605_2.patch added

Changed 6 years ago by oligo

Attachment: AlignInLineActionTest.java added

comment:8 Changed 6 years ago by oligo

Resolution: fixed
Status: closedreopened

This patch causes a regression when aligning nodes on closed ways (I tested on version latest).
I attached test_case_9605_2.osm to reproduce: select nodes ABD and align. I would expect that node A be centered between B and D. Instead node B is moved away.

Here is the patch: bug9605_2.patch and some non-regression tests: AlignInLineActionTest.java (maybe the setUp() method needs some adjustments as I had difficulties with launching it as a single unit-test in Eclipse).

comment:9 Changed 6 years ago by skyper

See #10050

comment:10 in reply to:  8 Changed 6 years ago by skyper

Replying to oligo:

Here is the patch: bug9605_2.patch and some non-regression tests: AlignInLineActionTest.java (maybe the setUp() method needs some adjustments as I had difficulties with launching it as a single unit-test in Eclipse).

Did anyone look at above mentioned patch, yet ?

comment:11 Changed 6 years ago by Don-vip

Resolution: fixed
Status: reopenedclosed

In 7850/josm:

fix #9605, fix #10050 - align nodes in line moves outer instead of inner node (patch by oligo)

comment:12 Changed 6 years ago by Don-vip

Thank you very much! This is a really great patch! :)

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.