Modify

Ticket #5427 (closed enhancement: fixed)

Opened 17 months ago

Last modified 11 months ago

Extrude leaves unnecessary nodes and 'antennae' segments when extruding again in a direction

Reported by: Ldp Owned by: team
Priority: normal Component: Core
Version: latest Keywords:
Cc:

Description

As outlined in  http://lists.openstreetmap.org/pipermail/merkaartor/2010-September/002637.html.

1) Extruding in a direction that you already extruded in works fine, but the nodes of the earlier position remain. As the edge they're in is straight, these usually don't need to remain in the way.

2) When you extrude too far and want to pull back a bit, two nodes will remain at the original location, still connected to the way being extruded. These currently have to be manually cleaned up, but it would be nice if Extrude could take care of that.

Attachments

extrude-patch-1.diff Download (28.9 KB) - added by extropy 17 months ago.
Do not add nodes when segments colinear, more directions to extrude to
extrude-patch-2.diff Download (34.6 KB) - added by extropy 17 months ago.
Improved patch, cal extrude in additional directions when moving, always adds nodes if SHIFT is pressed
extrude.jpg Download (46.1 KB) - added by michael@… 12 months ago.

Change History

comment:1 Changed 17 months ago by extropy

I will take a look into this.

Changed 17 months ago by extropy

Do not add nodes when segments colinear, more directions to extrude to

comment:2 Changed 17 months ago by extropy

  • Summary changed from Extrude leaves unnecessary nodes and 'antennae' segments when extruding again in a direction to [PATCH] Extrude leaves unnecessary nodes and 'antennae' segments when extruding again in a direction

Here is a fix, that improves the extrude function.

Can somebody review this and possibly commit?

comment:3 Changed 17 months ago by stoecker

When you do such long patches a bit larger description is helpful, as reading the code is not so easy.

comment:4 Changed 17 months ago by extropy

Sorry for the long patch. The original code was quite messy, so I could not resist reordering methods and renaming variables.

Changed:

  • replace undo stack manipulation hack on mouseDragged with something a bit saner.
  • improved mouseUp to not introduce new nodes, when segments are colinear (the main point of the patch).
  • added mechanism to define several extrusion angles with automatic angle matching, depending on drag direction - see possibleMoveDirections list and it's uses.
  • made three possible extrusion directions, one perpendicual to segment and two parallel to adjacent segments.
  • moved geometry code out in separate methods (hopefully to be moved out into some tools class).

Also trivial improvement to WaySegment class to make code more readable.

comment:5 Changed 17 months ago by bastiK

Could you add Shift or Alt modifier such that it retains the normally superfluous nodes? I can imagine some use cases for this, e.g. drawing a cross shaped building:

  • draw single segment
  • extrude to rectangle
  • extend 2 times
  • you have a rectangle with 2 nodes on the long sides -> extrude these middle segments

(If we would introduce extrude today, it wouldn't matter, but there may be users who are accustomed to the old behaviour. Then we can tell them to hold Shift / Alt modifier. :) )

Otherwise the improvements are great, can you add the additional angles to the "move-segment" mode (Ctrl), as well?

Maybe one could also snap to angles of 45 deg. But I'm not sure it is a good idea.

Changed 17 months ago by extropy

Improved patch, cal extrude in additional directions when moving, always adds nodes if SHIFT is pressed

comment:6 Changed 17 months ago by bastiK

  • Status changed from new to closed
  • Resolution set to fixed

(In [3557]) applied #5427 (patch by extropy) - Extrude leaves unnecessary nodes and 'antennae' segments when extruding again in a direction

comment:7 Changed 16 months ago by bastiK

see #5560

comment:8 Changed 12 months ago by Andygol

  • Status changed from closed to reopened
  • Version set to latest
  • Resolution fixed deleted

Changed 12 months ago by michael@…

comment:9 Changed 12 months ago by Ldp

Seconded.

comment:10 Changed 12 months ago by stoecker

  • Summary changed from [PATCH] Extrude leaves unnecessary nodes and 'antennae' segments when extruding again in a direction to Extrude leaves unnecessary nodes and 'antennae' segments when extruding again in a direction

comment:11 Changed 12 months ago by anonymous

Looks like the problem is inside org.openstreetmap.josm.tools.Geometry, method segmentsParralel(). There is used constant 1e-13 to compare distance, but this is not working anymore because of change in Mercator projection.

comment:12 Changed 12 months ago by Ldp

While someone is fixing that, can he/she change it to the correct 'parallel' ?

comment:13 Changed 11 months ago by stoecker

  • Status changed from reopened to closed
  • Resolution set to fixed

In [3939/josm]:

hopefully fix #5427

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'reopened'
Author


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

 
Note: See TracTickets for help on using tickets.