Modify

Opened 7 years ago

Closed 6 years ago

Last modified 2 years ago

#8798 closed defect (fixed)

[PATCH] eXtruder misbehaviour

Reported by: skyper Owned by: team
Priority: normal Milestone: 13.11
Component: Core Version: latest
Keywords: extruder Cc:

Description (last modified by skyper)

There was a bug reported on the German mailing list and I just found another misbehaviour. All I use is the extruder mode (I gonna attach the example .osm file):

screenshot

Misbehaviour (upper samples)

  1. Draw way a
  2. Add node A
  3. Draw way b (ALT)
  4. Move northern wall of way b

Instead of moving node A a new node is created which is only part of way b.

Bug (lower sample)

  1. Draw way a
  2. Draw way b (ALT)
  3. Move northern and/or southern wall of way a

Nodes A and B should not be part of way a anymore but they still are. And the new corner nodes of way a are not added to way b.

Repository Root: http://josm.openstreetmap.de/svn
Build-Date: 2013-06-13 01:35:38
Last Changed Author: Don-vip
Revision: 6005
Repository UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
URL: http://josm.openstreetmap.de/svn/trunk
Last Changed Date: 2013-06-13 02:16:00 +0200 (Thu, 13 Jun 2013)
Last Changed Rev: 6005

Identification: JOSM/1.5 (6005 de) Linux Debian GNU/Linux 7.0 (wheezy)
Memory Usage: 225 MB / 672 MB (130 MB allocated, but free)
Java version: 1.6.0_27, Sun Microsystems Inc., OpenJDK 64-Bit Server VM
Program arguments: [extruder.osm]
Dataset consistency test: No problems found

Attachments (8)

extruder_sample.png (18.3 KB) - added by skyper 7 years ago.
screenshot
extruder.osm (7.2 KB) - added by skyper 7 years ago.
osm sample file
IgnoreSharedNodes1-OverlappingSegmentOnly.patch (3.8 KB) - added by AlfonZ 7 years ago.
Should extrusion create overlapping segments, which can't be simplified by moving the node because it's shared with another way, the node is removed from extruded way and replaced by new one. The behavior is controlled by advanced property extrude.ignore-shared-nodes (disabled by default).
IgnoreSharedNodes2-BothDirections.patch (2.4 KB) - added by AlfonZ 7 years ago.
Should extrusion create overlapping segments or segments in straight line, which can't be simplified by moving the node because it's shared with another way, the node is removed from extruded way and replaced by new one. The behavior is controlled by advanced property extrude.ignore-shared-nodes (disabled by default).
CurrentBehavior.png (18.3 KB) - added by AlfonZ 7 years ago.
IgnoreSharedNodes1-OverlappingSegmentOnly.png (18.2 KB) - added by AlfonZ 7 years ago.
IgnoreSharedNodes1-OverlappingSegmentOnly.patch in action Upper row: 1. original, 2. extrude from 1 downwards, 3. extrude from 1 upwards Lower row: 1. original, 2. extrude from 1 outwards, 3. extrude from 1 inwards
IgnoreSharedNodes2-BothDirections.png (17.4 KB) - added by AlfonZ 7 years ago.
IgnoreSharedNodes2-BothDirections.patch in action Upper row: 1. original, 2. extrude from 1 downwards, 3. extrude from 1 upwards Lower row: 1. original, 2. extrude from 1 outwards, 3. extrude from 1 inwards
IgnoreSharedNodes1-Nonsymmetry.png (26.0 KB) - added by AlfonZ 7 years ago.
Upper row: 1. original, 2. extrude from 1 downwards, 3. extrude from 2 upwards, 4. extrude from 1 upwards Lower row: 1. original, 2. extrude from 1 upwards, 3. extrude from 2 downwards, 4. extrude from 1 downwards

Download all attachments as: .zip

Change History (19)

Changed 7 years ago by skyper

Attachment: extruder_sample.png added

screenshot

Changed 7 years ago by skyper

Attachment: extruder.osm added

osm sample file

comment:1 Changed 7 years ago by skyper

Description: modified (diff)

comment:2 Changed 7 years ago by AlfonZ

My guess is that this is correct behavior. Extrude affects only one way, the one being extruded.
If a node (A in the upper example, A and B in the lower example) is part of other way it is not moved as to not change the other way b (see #5560).

It is always possible to clean up nodes afterwards manually. In the upper example by merging node A into B. In the lower example by joining node A into way b, ungluing node A from way b and deleting the one that is part of way a.

comment:3 in reply to:  2 Changed 7 years ago by skyper

Replying to AlfonZ:

My guess is that this is correct behavior. Extrude affects only one way, the one being extruded.
If a node (A in the upper example, A and B in the lower example) is part of other way it is not moved as to not change the other way b (see #5560).

Ok, I can understand why nodes are not moved still think it could be smarter but the lower example is a bug, as way a should not contain the former corner nodes.

I can always clean up afterwards but that is not the nicest way to progress, time consuming and error-prone.

Changed 7 years ago by AlfonZ

Should extrusion create overlapping segments, which can't be simplified by moving the node because it's shared with another way, the node is removed from extruded way and replaced by new one. The behavior is controlled by advanced property extrude.ignore-shared-nodes (disabled by default).

Changed 7 years ago by AlfonZ

Should extrusion create overlapping segments or segments in straight line, which can't be simplified by moving the node because it's shared with another way, the node is removed from extruded way and replaced by new one. The behavior is controlled by advanced property extrude.ignore-shared-nodes (disabled by default).

Changed 7 years ago by AlfonZ

Attachment: CurrentBehavior.png added

Changed 7 years ago by AlfonZ

IgnoreSharedNodes1-OverlappingSegmentOnly.patch in action
Upper row: 1. original, 2. extrude from 1 downwards, 3. extrude from 1 upwards
Lower row: 1. original, 2. extrude from 1 outwards, 3. extrude from 1 inwards

Changed 7 years ago by AlfonZ

IgnoreSharedNodes2-BothDirections.patch in action
Upper row: 1. original, 2. extrude from 1 downwards, 3. extrude from 1 upwards
Lower row: 1. original, 2. extrude from 1 outwards, 3. extrude from 1 inwards

Changed 7 years ago by AlfonZ

Upper row: 1. original, 2. extrude from 1 downwards, 3. extrude from 2 upwards, 4. extrude from 1 upwards
Lower row: 1. original, 2. extrude from 1 upwards, 3. extrude from 2 downwards, 4. extrude from 1 downwards

comment:4 Changed 7 years ago by AlfonZ

I have attached two variants of a patch. The new behavior is controlled by property extrude.ignore-shared-nodes (disabled by default).

For reference, current behavior :


IgnoreSharedNodes1-OverlappingSegmentOnly.patch in action:

IgnoreSharedNodes1-OverlappingSegmentOnly.patch in action [[br]] Upper row: 1. original, 2. extrude from 1 downwards, 3. extrude from 1 upwards [[br]] Lower row: 1. original, 2. extrude from 1 outwards, 3. extrude from 1 inwards

Note that shared node A is part of way when extruding outwards and not part when extruding inwards. This creates nonsymmetry.
Consider following: First extrude downwards, then upwards. The node A is part of the way. Extrude only upwards, node A is not part of the way. Similar for reverse order of actions.

Upper row: 1. original, 2. extrude from 1 downwards, 3. extrude from 2 upwards, 4. extrude from 1 upwards [[br]] Lower row: 1. original, 2. extrude from 1 upwards, 3. extrude from 2 downwards, 4. extrude from 1 downwards

IgnoreSharedNodes2-BothDirections.patch in action:

IgnoreSharedNodes2-BothDirections.patch in action [[br]] Upper row: 1. original, 2. extrude from 1 downwards, 3. extrude from 1 upwards [[br]] Lower row: 1. original, 2. extrude from 1 outwards, 3. extrude from 1 inwards

comment:5 Changed 7 years ago by stoecker

Summary: eXtruder misbehaviour[PATCH] eXtruder misbehaviour

comment:6 Changed 7 years ago by skyper

Thanks a lot.

  • I like the first patch much better.
  • You can always use undo for the cases where you first went to the wrong direction.
  • Still I think that when extruding inwards the new nodes should be part of both ways.

By the way #7991 is a really annoying bug, too. Well if someone finds the time ....

Last edited 7 years ago by skyper (previous) (diff)

comment:7 Changed 7 years ago by AlfonZ

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

comment:8 Changed 6 years ago by skyper

Any news ?

comment:9 Changed 6 years ago by stoecker

Resolution: fixed
Status: newclosed

In 6252/josm:

fix #8798 - patch by AlfonZ - fix extruder

comment:10 Changed 6 years ago by Don-vip

Milestone: 13.11 (6383)

comment:11 Changed 2 years ago by stoecker

Milestone: 13.11 (6383)13.11

Milestone renamed

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.