Modify

Opened 5 years ago

Closed 5 years ago

Last modified 4 years 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--- 5 years ago.
circle-arcs.osm (1.7 KB ) - added by GerdP 5 years ago.
18847.1.patch (18.9 KB ) - added by GerdP 5 years ago.
18847.2.patch (23.2 KB ) - added by GerdP 5 years ago.

Download all attachments as: .zip

Change History (17)

by Hb---, 5 years ago

Attachment: unconnected-nodes.png added

comment:1 by skyper, 5 years ago

Description: modified (diff)
Keywords: arc node added

comment:2 by GerdP, 5 years ago

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

comment:3 by GerdP, 5 years ago

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

in reply to:  3 comment:4 by GerdP, 5 years ago

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.

by GerdP, 5 years ago

Attachment: circle-arcs.osm added

comment:5 by GerdP, 5 years ago

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 by skyper, 5 years ago

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 by GerdP, 5 years ago

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

by GerdP, 5 years ago

Attachment: 18847.1.patch added

comment:8 by GerdP, 5 years ago

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.

by GerdP, 5 years ago

Attachment: 18847.2.patch added

comment:9 by GerdP, 5 years ago

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.

in reply to:  8 comment:10 by skyper, 5 years ago

Replying to GerdP:

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

+1

comment:11 by GerdP, 5 years ago

Resolution: fixed
Status: assignedclosed

see [o35384:35385]

comment:12 by GerdP, 5 years ago

In 16297/josm:

see #18847, #19076: Changing value of curves.circlearc.angle-separation value in advanced preferences no longer appears to have any effect.

  • add curves.circlearc.angle-separation to the list of obsolete preference keys

comment:13 by skyper, 4 years ago

Found issues with a closed way, see #19188.

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.