Opened 15 years ago
Closed 15 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)
Change History (10)
by , 15 years ago
Attachment: | josm-2374-extrude-movenormal-ris-v1.diff added |
---|
comment:1 by , 15 years ago
Cc: | added |
---|
comment:2 by , 15 years ago
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.
by , 15 years ago
Attachment: | josm-2374-extrude-movenormal-ris-v2.diff added |
---|
comment:3 by , 15 years ago
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 by , 15 years ago
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 by , 15 years ago
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.
by , 15 years ago
Attachment: | josm-2374-extrude-movenormal-ris-v3.diff added |
---|
comment:6 by , 15 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
It doesn't build:
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.