Modify

Opened 13 years ago

Closed 13 years ago

#3832 closed enhancement (fixed)

[PATCH] Extrude 'move along normal' and cleanups

Reported by: ris Owned by: team
Priority: normal Milestone:
Component: Core Version:
Keywords: extrude mapmode Cc: teemu.koskinen@…

Description

(copied from initial email)

Attached is a patch against SVN 2374 which adds a feature to extrude mode: hold ctrl while dragging to move an existing segment along its normal rather than creating a new segment.

This is needed to be able to easily edit buildings etc after they have been initially extruded.

In adding this, I touched most of the extrude mode code, removing some of the more glaring problems (like editing logic in the painting code) and tried to sanitize the mode's state machine. I also removed most of the remnants of the select mode it was copied from for clarity.

Please note I am a c++ programmer, not a java programmer, so comments are welcome.

robert.

Attachments (4)

josm-2374-extrude-movenormal-ris-v1.diff (15.5 KB) - added by ris 13 years ago.
josm-2374-extrude-movenormal-ris-v1-fixed.diff (15.5 KB) - added by Daeron 13 years ago.
Fixed patch
josm-2374-extrude-movenormal-ris-v2.diff (15.5 KB) - added by ris 13 years ago.
josm-2374-extrude-movenormal-ris-v3.diff (22.1 KB) - added by ris 13 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 13 years ago by Daeron

Cc: teemu.koskinen@… added

It doesn't build:

[javac] /home/daeron/osm/josm/core/src/org/openstreetmap/josm/actions/mapmode/ExtrudeAction.java:340: missing return statement

Even though those three values are the only valid possibilities for the mode, java doesn't know that, and it thinks that there might be a possibility that all the if's evaluate to false, and then it's in the end of the method where there's no return statement.

Attached is a fixed patch that compiles.

Changed 13 years ago by Daeron

Fixed patch

comment:2 Changed 13 years ago by ris

Balls.

I did actually fix this (this was just me testing whether java allowed no-return-value functions), but I exported the patch before I made the fix and clearly forgot to regenerate the patch and uploaded the old one.

I'm attaching the patch I meant to upload which also has a bunch of comment fixes.

comment:3 Changed 13 years ago by Daeron

If there's two overlapping ways (not necessarily sharing nodes, but aligned so that they are overlapping), and one of them is selected, extruding from the overlapping segments should always use the selected way.

Currently it doesn't always extrude the selected, but depending on position and zoom (etc.) selects either one of them.

comment:4 Changed 13 years ago by ris

I can't find any code relating to this in ExtrudeAction either before or after my patch. It seems this is all the task of MapView.getNearestWaySegment(). If yours wasn't working I suggest you might have been patching against a version that had regressions elsewhere, because it works for me.

comment:5 Changed 13 years ago by ris

Attaching a new patch, v3, again against SVN 2374, which adds some guideline painting, but more significantly, overhauls a lot (most) of the ExtrudeAction code.

The maths has all been moved to an AffineTransform to make it more reusable and sane. The huge majority of the maths is now done once, at mousePress, and all other functions just have to drop numbers into the generated AffineTransform.

A nice little guideline is shown in (ctrl-drag) 'move along normal' mode to make it more obvious what is going on. This uses a 'semi-infinite-line' method which I reckon would be useful in other areas and think should belong in MapView eventually.

A number of little quirks have been fixed such as the cursor changing being broken.

There are more features I want to add to extrude mode, but I would like to get this patch applied first before I worry about them.

comment:6 Changed 13 years ago by stoecker

Resolution: fixed
Status: newclosed

(In [2429]) applied #3832 - patch by ris - improve extrude action

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.