Modify

Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#8431 closed defect (fixed)

"Create circle" changes way direction

Reported by: naoliv Owned by: team
Priority: normal Milestone: 14.03
Component: Core Version:
Keywords: create circle Cc: richlv

Description (last modified by Don-vip)

While selecting one node + it's way to create a circle (via the O shortcut) with the nodes properly aligned I saw that Josm is reverting the way direction.

See the attached video and the attached .osm file.

The example roundabout (from the video) is tagged with a FIXME=josm in the osm file.
Note how the way has it's direction reversed.

JOSM is:

Repository Root: http://josm.openstreetmap.de/svn
Build-Date: 2013-02-13 02:31:45
Last Changed Author: Don-vip
Revision: 5713
Repository UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
URL: http://josm.openstreetmap.de/svn/trunk
Last Changed Date: 2013-02-12 23:31:18 +0100 (Tue, 12 Feb 2013)
Last Changed Rev: 5713

Attachments (7)

josm-circle.ogv (438.8 KB ) - added by naoliv 11 years ago.
Video of the problem
circle-error.osm.bz2 (188.9 KB ) - added by naoliv 11 years ago.
Example OSM file. Search for the roudabout tagged with FIXME=josm
bug8431.patch (11.9 KB ) - added by Balaitous 10 years ago.
patch from r6892 Order nodes anticlockwise before align in circle, cleanup old code for regular polygon (replaced by regular arc since r6892)
circle_testcase.osm (2.5 KB ) - added by richlv 10 years ago.
bug8431_2.patch (11.1 KB ) - added by Balaitous 10 years ago.
since r6920
use_case_of_half_circle.jpg (80.1 KB ) - added by Balaitous 10 years ago.
circle-uneven.png (3.3 KB ) - added by naoliv 10 years ago.

Download all attachments as: .zip

Change History (44)

by naoliv, 11 years ago

Attachment: josm-circle.ogv added

Video of the problem

by naoliv, 11 years ago

Attachment: circle-error.osm.bz2 added

Example OSM file. Search for the roudabout tagged with FIXME=josm

comment:1 by skyper, 11 years ago

Component: PluginCore

comment:2 by Don-vip, 11 years ago

In 5719/josm:

see #8431 - Align in circle: Do not allow "regular polygon" mode for way nodes being referred by another primitives. The initial problem (way being reverted) remains.

comment:3 by Don-vip, 11 years ago

Priority: minornormal

comment:4 by Don-vip, 11 years ago

see also #8425

comment:5 by Don-vip, 10 years ago

Ticket #9290 has been marked as a duplicate of this ticket.

comment:6 by Don-vip, 10 years ago

Cc: richlv added

r5719 has introduced a problem: it should be possible to allow regular polygon in that case, see dataset attached to #9290.

comment:7 by Don-vip, 10 years ago

Description: modified (diff)

comment:8 by Don-vip, 10 years ago

Milestone: 14.02

comment:9 by richlv, 10 years ago

for the record, regression reported in #9290 still there in 6767

comment:10 by Don-vip, 10 years ago

Milestone: 14.0214.03

by Balaitous, 10 years ago

Attachment: bug8431.patch added

patch from r6892 Order nodes anticlockwise before align in circle, cleanup old code for regular polygon (replaced by regular arc since r6892)

comment:11 by Balaitous, 10 years ago

Summary: "Create circle" changes way direction[patch] "Create circle" changes way direction

comment:12 by skyper, 10 years ago

Keywords: create circle added

comment:13 by skyper, 10 years ago

see also #7421

comment:14 by Don-vip, 10 years ago

Resolution: fixed
Status: newclosed

In 6919/josm:

fix #8431, fix #9839 - fix issues with "align nodes in circle" action (patch by Balaitous)

comment:15 by richlv, 10 years ago

Resolution: fixed
Status: closedreopened

hmm. thanks, but now it does not seem to be consistent - and it's different from the original behaviour :)

a) single closed way
previously :
select way only - circular, but node distribution is not changed
select way + 1 node - circular, nodes evenly distributed
now :
select way only - circular, nodes evenly distributed
select way + 1 node - circular, nodes evenly distributed

b) closed way + one attached way
previously :
select way only - circular, but node distribution is not changed
select way + 1 node - circular, nodes evenly distributed
now :
select way only - circular, nodes evenly distributed
select way + 1 node - circular, nodes... not sure quite what happens here, but it seems to do this :

from the selected node on each side nodes are distributed evenly up to the connected way - so the circle gets uneven distribution. this only seems to happen if some other way attaches at the beginning of segment 1.

see attached testcase. select the rightmost node, press 'o'.

note that selecting two nodes keeps them in place and makes an arc of the remaining, that seems to work properly.


as for the change in behaviour where selecting just the way did not change node distribution - if that was intended, probably documentation should be updated ?

Last edited 10 years ago by richlv (previous) (diff)

by richlv, 10 years ago

Attachment: circle_testcase.osm added

comment:16 by naoliv, 10 years ago

I like to have the nodes evenly distributed.
Just in case this feature gets reverted/removed, it could at least be made as an option (to have or not an even distribution)

Last edited 10 years ago by naoliv (previous) (diff)

comment:17 by richlv, 10 years ago

note that previously nodes were not evenly distributed if you selected only the way and evenly distributed if you selected way and one node on it. i do agree that there must be a way to distribute the nodes evenly :)

comment:18 by Balaitous, 10 years ago

As JOSM user, I think it is better to have evenly distributed nodes as often as possible.
So I propose to define the cases where angle of a node (from center) doesn't have to change, and allow change for others.

Nodes that should have fix angle are:

  • Nodes connected to other ways (that's the case in this version)
  • Nodes used by a relation (that's the case in this version)
  • Nodes with tags (what tag ? all including source tag ?) (that's NOT the case in this version)

And nodes between fix angle nodes are evenly distributed.
And after to update documentation.

But there is a more important bug: If you select 2 disjoint ways and >= 4 nodes, it does anything ! I will send a patch tomorrow (this bug was existing before my patch in r6814)

comment:19 by Balaitous, 10 years ago

I propose this:

  • by default nodes are evenly distributed
  • to prevent evenly distribution of a node, user have to explicitly select it
    /**
     * Perform AlignInCircle action.
     * 
     * A fixed node is a node for which it is forbidden to change the angle relative to center of the circle.
     * All other nodes are uniformly distributed.
     * 
     * Case 1: One unclosed way.
     * --> allow action, and align selected way nodes
     * If nodes contained by this way are selected, there are fix.
     * If nodes outside from the way are selected there are ignored.
     * 
     * Case 2: One or more ways are selected and can be joined into a polygon
     * --> allow action, and align selected ways nodes
     * If 1 node outside of way is selected, it became center
     * If 1 node outside and 1 node inside are selected there define center and radius
     * If no outside node and 2 inside nodes are selected those 2 nodes define diameter
     * In all other cases outside nodes are ignored
     * In all cases, selected nodes are fix, nodes with more than one referrers are fix
     * (first referrer is the selected way)
     * 
     * Case 3: Only nodes are selected
     * --> Align these nodes, all are fix
     */

This is implemented in bug8431_2.patch

by Balaitous, 10 years ago

Attachment: bug8431_2.patch added

since r6920

in reply to:  19 comment:20 by skyper, 10 years ago

Replying to Balaitous:

I do not really understand Case 1 and 3:

about 1.:

  • How should this work ? What does it have to do with circle
  • In code you use an temporally closed way but I can not think of many use-cases where an unclosed way should be a circle.
    • I have use-cases for align nodes in curve and the curve could be a circle but this is out of scope of "align nodes in circle".
    • I believe that start and end node need to be fixed to make some sense as you would be able to draw/align parts of a circle.

about 3.:

  • Can be dropped as all selected nodes are fixed anyway. We need at least one way or above is also true for this case but without start/end node.
  • I have several cases where I need to have some nodes of a way to form a circle but leave the rest in place or even keep the rest of the geometry in shape. So far I use some temporary construction ways to get to the result. Could this be done with case 3 ?
Last edited 10 years ago by skyper (previous) (diff)

comment:21 by Balaitous, 10 years ago

About 3:

  • It's exactly what JOSM already does (for example r6767)

About 1:

  • Temporally closed way is just a code convenience for the call to the collectNodesAnticlockwise function
  • first and last node are fix
                fixNodes.add(w.firstNode());
                fixNodes.add(w.lastNode());
    
  • It can produce nice half circle, but to be fully useful, need also #7423
  • There are use case for this, see attach file.

by Balaitous, 10 years ago

Attachment: use_case_of_half_circle.jpg added

in reply to:  21 comment:22 by skyper, 10 years ago

Replying to Balaitous:

About 3:

  • It's exactly what JOSM already does (for example r6767)

Thanks, I know why I always use an additional way: You are able have some fixed nodes.

About 1:

  • Temporally closed way is just a code convenience for the call to the collectNodesAnticlockwise function
  • first and last node are fix
                fixNodes.add(w.firstNode());
                fixNodes.add(w.lastNode());
    
  • It can produce nice half circle, but to be fully useful, need also #7423
  • There are use case for this, see attach file.

Ok, I have overread that part. Thanks again.

comment:23 by skyper, 10 years ago

Oh forgot. For case 1:

  • Why not allow to have several connected ways as long as the are in on row ?

comment:24 by skyper, 10 years ago

By the way, do you know the curve function from utilsplugin2 ?. I like it a lot but it is buggy and little orphaned.

Version 0, edited 10 years ago by skyper (next)

comment:25 by Don-vip, 10 years ago

Resolution: fixed
Status: reopenedclosed

In 6933/josm:

fix #8431 - improve "align in circle" action (patch by Balaitous):

  • by default nodes are evenly distributed
  • to prevent evenly distribution of a node, user have to explicitly select it

comment:26 by Don-vip, 10 years ago

thanks for the patch! this is really great :)

by naoliv, 10 years ago

Attachment: circle-uneven.png added

comment:27 by naoliv, 10 years ago

(if I need to open a new bug or if I am doing something wrong, please, tell me)

I have createcircle.nodecount=16 set in JOSM and with this new patch, creating an open triangle with 3 nodes (one way with 2 segments) and then pressing Shift+O I get a circle with 19 nodes that isn't evenly distributed:


Last edited 10 years ago by naoliv (previous) (diff)

comment:28 by stoecker, 10 years ago

Resolution: fixed
Status: closedreopened

in reply to:  27 comment:29 by skyper, 10 years ago

Replying to naoliv:

(if I need to open a new bug or if I am doing something wrong, please, tell me)

I have createcircle.nodecount=16 set in JOSM and with this new patch, creating an open triangle with 3 nodes (one way with 2 segments) and then pressing Shift+O I get a circle with 19 nodes that isn't evenly distributed:

This is an rather old problem and has nothing to do with the last changes. See #5922.

in reply to:  23 comment:30 by Balaitous, 10 years ago

Replying to skyper:

  • Why not allow to have several connected ways as long as the are in on row ?

It should be a good improvement. In France we have a lot of roundabout (the most important density around the word! ) some of them are broken into small arc to describe bus line.

By the way, do you know the circle arc function from utilsplugin2 ?. I like it a lot but it is buggy and little orphaned.

I don't know. I will have a look.

in reply to:  26 ; comment:31 by Balaitous, 10 years ago

Replying to Don-vip:

thanks for the patch! this is really great :)

If you think by contribution are useful, take a look at #7423. I correct a problem with circle center.

in reply to:  31 ; comment:32 by Don-vip, 10 years ago

Replying to Balaitous:

If you think by contribution are useful, take a look at #7423. I correct a problem with circle center.

Thanks, I did miss this one. I am considering it for inclusion in this release, see my remarks there.
Can you also take a look to naoliv and skyper comments about the createcircle.nodecount stuff? :)

comment:33 by skyper, 10 years ago

Summary: [patch] "Create circle" changes way direction"Create circle" changes way direction

comment:34 by Don-vip, 10 years ago

Resolution: fixed
Status: reopenedclosed

This problem is fixed, please continue discussion on #5922.

in reply to:  32 ; comment:35 by Balaitous, 10 years ago

Replying to Don-vip:

Can you also take a look to naoliv and skyper comments about the createcircle.nodecount stuff? :)

This is done.

comment:36 by Don-vip, 10 years ago

In 6942/josm:

fix #5922, see #8431 - value of createcircle.nodecount not always applied (patch by Balaitous)

in reply to:  35 comment:37 by Don-vip, 10 years ago

Replying to Balaitous:

This is done.

Thank you so much! You're doing a great job :)

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. 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.