Modify

Opened 4 weeks ago

Closed 11 days ago

#18847 closed defect (fixed)

[Patch RFC] Circle Arc tool creates unconnected nodes

Reported by: Hb--- Owned by: GerdP
Priority: normal Milestone:
Component: Plugin utilsplugin2 Version:
Keywords: arc node Cc:

Description (last modified by skyper)

What steps will reproduce the problem?

  1. Draw a way with some nodes.
  2. Select non-adjacent nodes like 2, 4 and 5 and the way.
  3. Invoke Circle Arc tool (Shift+C)

What is the expected result?

A circle arc formed between the segments 2, 3 and 4.

What happens instead?

Some free nodes gets created (under node 3 in the screenshot).

The current behavior does not fit to the "Method 3" from the help and not to the authors comments in the class header.


URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2020-02-29 00:28:12 +0100 (Sat, 29 Feb 2020)
Build-Date:2020-02-29 02:31:03
Revision:15958
Relative:URL: ^/trunk

Identification: JOSM/1.5 (15958 en) Windows 7 64-Bit
OS Build number: Windows 7 Professional (7601)
Memory Usage: 1450 MB / 3604 MB (1269 MB allocated, but free)
Java version: 1.8.0_212-b03, AdoptOpenJDK, OpenJDK 64-Bit Server VM
Screen: \Display0 1280x1024
Maximum Screen Size: 1280x1024
VM arguments: [-Djosm.home=testrun]
Program arguments: [--language=en]
Dataset consistency test: No problems found

Plugins:
+ utilsplugin2 (35334)

Last errors/warnings:
- W: Failed to parse external taginfo data at https://josm.openstreetmap.de/remote/geofabrik-index-v1-nogeom.json: Invalid token=EOF at (line no=6119, column no=2252, offset=359085). Expected tokens are: [CURLYOPEN, SQUAREOPEN, STRING, NUMBER, TRUE, FALSE, NULL]
- E: Failed to locate image 'https://josm.openstreetmap.de/browser/trunk/images/copy.svg?format=raw'

Attachments (4)

unconnected-nodes.png (3.1 KB) - added by Hb--- 4 weeks ago.
circle-arcs.osm (1.7 KB) - added by GerdP 2 weeks ago.
18847.1.patch (18.9 KB) - added by GerdP 12 days ago.
18847.2.patch (23.2 KB) - added by GerdP 12 days ago.

Download all attachments as: .zip

Change History (15)

Changed 4 weeks ago by Hb---

Attachment: unconnected-nodes.png added

comment:1 Changed 4 weeks ago by skyper

Description: modified (diff)
Keywords: arc node added

comment:2 Changed 4 weeks ago by GerdP

Seems the algo calculates the points at the right place but fails to join the way to them.

comment:3 Changed 2 weeks ago by GerdP

IIGTR the action should just show a notification saying e.g. "select three consecutive way nodes" in this case?

comment:4 in reply to:  3 Changed 2 weeks ago by GerdP

Owner: changed from team to GerdP
Status: newassigned

Replying to GerdP:

IIGTR the action should just show a notification saying e.g. "select three consecutive way nodes" in this case?

Forget that. The help clearly states what should happen.

Changed 2 weeks ago by GerdP

Attachment: circle-arcs.osm added

comment:5 Changed 2 weeks ago by GerdP

Looking at the code I don't think that it ever worked as described in the pictures for "One way and three of its nodes selected"

I wonder what to do when other ways are connected to the selected nodes. Try my modified example and watch how way w2 changes its position.
What should happen when a way is connected to a node which is not selected but between the selected nodes?

comment:6 Changed 2 weeks ago by skyper

It should be similar to complete circles. Have a look at roundabouts. There the nodes with other parents are moved as less as possible and the nodes without another parent are squeezed around them. Better even change the order of the nodes in the way than moving nodes with additional parents a lot.

comment:7 Changed 2 weeks ago by GerdP

OK, sounds reasonable. I've never used that action.

Changed 12 days ago by GerdP

Attachment: 18847.1.patch added

comment:8 Changed 12 days ago by GerdP

The patch changes the code to work similar to "Align Nodes in Circle" but it adds nodes to produce a circle.
The patched code still uses preference circlearc.angle-separation which has a default of 20 while actions like "Create Circle" calculate a value near 4 for large circles and much higher value for small circles (see #10777)
I think it would be better to use the same calculation and ignore circlearc.angle-separation.

Changed 12 days ago by GerdP

Attachment: 18847.2.patch added

comment:9 Changed 12 days ago by GerdP

Summary: Circle Arc tool creates unconnected nodes[Patch RFC] Circle Arc tool creates unconnected nodes

18847.2.patch removes the evaluation of circlearc.angle-separation as suggested in comment:8
A lot of the code is a copy of CreateCircleAction or AlignInCircleAction.
If nobody complains I'll commit this tomorrow.

comment:10 in reply to:  8 Changed 12 days ago by skyper

Replying to GerdP:

I think it would be better to use the same calculation and ignore circlearc.angle-separation.

+1

comment:11 Changed 11 days ago by GerdP

Resolution: fixed
Status: assignedclosed

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.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.