[Patch] Improve move action (mouse drag of osm primitives) in MapMode by aligning movement if Ctrl modifier key is pressed

[cmuelle8]

The attached patch enhances moving the selection in MapMode.

If Ctrl modifier key is pressed then the last selected way within the selection is used as an alignment reference for the move operation.

The last selected way must consist of exactly two nodes for the mode option to work. It may be the only element in the selection.

Four "zones" (or quadrants) are recognized, each 90° wide. When the mouse is dragged, the angle between the vectors of the mouse move (relative to starting point of the move operation) and the reference way is continuously calculated.

• within range [0, 45] degrees or [0, pi/2] radians the selection is moved along the reference
• within range [45, 90] degrees or [pi/2, pi] radians it is moved perpendicular to it

The movement is only ever constrained to these axes if the modifier key "Ctrl" is pressed while a press+hold mouse operation is in progress. The modifier key may be toggled arbitrarily while the selection is moved to toggle the aligning.

It is neither intended to replace the angleSnap feature of the DrawAction, nor the AlignWays plugin that each serve slightly different purposes.

---

Another way to view the feature is as enhancement of the present keyboard commands that move the selection up/down/left/right (exactly) which aligned a selection move to the cardinal directions north/south/west/east respectively.

The code reuses functions present in class EastNorth to determine the difference between the headings of the vectors end points.

It has been tested to not interfer with the other use of Ctrl modifier key in mapmode, which is mergeNodes() feature:

• if Ctrl is held at release of the mouse button and a NodeToMerge was found, a Merge operation is carried out as expected from previous behavior
• if Ctrl is released before the mouse button is released, no node merge is performed
  • this is relevant only if a node to merge to is identified at the location the user wants to finish the drag operation
  • if the node merge is not desirable some care must be taken to not additionally move the mouse within the timespan Ctrl key and mouse button are released (in that order)

Although basic testing has been done, please test and improve the code changes before depending on them productively.

A specific use case for this is generating trapezoidal shapes or parallelograms (in combination with parallel ways tool, for instance).

[cmuelle8]

josm_SelectAction_-_movement_with_modifier_key_-_align_movement_to_vector_given_by_way_last_selected_in_selection

[skyper]

Well in micro-mapped areas, I need the Ctrl key to not change the selection when moving and not adding a new node in a segment instead. I have the preference Draw virtual nodes in select mode enabled. I hope this patch does not change this workflow.

[cmuelle8]

Can you please explain how Ctrl key currently helps you in not adding a new node?

Using unmodified josm-latest I just selected 2 way primitives, then pressed and held Ctrl and clicked on a virtual node. The result is a dropped selection, replaced by the draggable new node.

Do you refer to a different use case? Maybe dragging the map instead of moving the selected primitives?

The patch alters the move op that modifies the coordinates of primitives in return,
not map moves usually carried out by pressing and holding the right mouse button.

As Ctrl key has platform specific function on MacOS, it may help to know which system you work with, see source:/trunk/src/org/openstreetmap/josm/actions/mapmode/SelectAction.java#L494​ - I can't test MacOS specific features, but the logic to decide if the Map is in Mode.SELECT or Mode.MOVE states is not modified by the patch.

The patch looks at Ctrl after a user has taken all necessary actions to move the primitives in the selection.
As long as this mode is not entered, the logic to evaluate Ctrl key presses is unaltered.

[cmuelle8]

josm SelectAction - align movement to vector given by way last selected in selection on Ctrl modifier press (obsoletes previous patch, improves code readability, function unchanged)

[skyper]

I definitely move objects not the map (panning). I am on GNU Debian Linux with usually opendjk-17 and JOSM-latest package from this site

I'll try to come up with an example.

[skyper]

Ok, sorry, I was wrong about the cross-hair in the middle of the segments. It is the opposite. Ctrl prevents changes in selection if an unselected node or way has the focus and instead allows to move the current selection. In densely mapped areas this is useful if not needed as zooming in a lot often makes the bbox too smaller to properly move the objects.

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2022-02-18 08:34:05 +0100 (Fri, 18 Feb 2022)
Revision:18382
Build-Date:2022-02-19 02:31:08
URL:https://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (18382 en) Linux Debian GNU/Linux 11 (bullseye)
Java version: 17.0.2+8-Debian-1deb11u1, Debian, OpenJDK 64-Bit Server VM

[anonymous]

Did you actually try the patch? Or did you describe the problem hypothetically?

It should be a rather rare-case workflow. Actually one should zoom in if the mapview is too cluttered; notice that you can zoom out while moving objects with press+hold.

That said, I've reproduced your issue and while there is a small alternation from status quo, it should not be show-stopper. Adoption to the 'new' handling is rather quick and the benefit of the added functionality outweighs the issue, imho.

Current behavior (as described by your comment, reproducible with tested/latest rev):

• Ctrl moves the selection even if an unselected node or way has the focus (instead of selecting what is in focus).

Behavior with new patch:

• All the same, except that you have to release Ctrl key in order to move the selection freely if and only if the selection contains a way that the move can be aligned to.
• This precisely means that the last selected of all ways in the selection needs to consist of exactly two nodes. Only then the changed behavior will surface, in combination with the selection+moving pattern you have described.

So yes, there is a small change how you'd need to handle Ctrl, in the rare case that all below criteria meet:

• selection setting has a way detected as alignment vector
• user does not actually want to align the move operation to it
• ctrl was used to start the move operation (clicking unselected parts of the mapview)

The small change is that, as long as Ctrl is not released, the move operation will align.
Previous/current behavior does not require a user to release Ctrl in order to freely move the selection,
because (currently) toggling Ctrl does not effect how the move op is carried out.

The patch changes the no-op behavior of Ctrl while selection moval is in progress. It does not break the rare use case you've described, but you'd need to 'learn' to release Ctrl if and only if

• the situation / the setup outlined above is encountered
• you do not want to carry out an aligned move

The handling you've described is currently not documented at Help/Action/Select, so chances are we can change it and document the new feature at this place right away, should the patch be accepted. That said, I'm not a fan of changing behavior that is heavily used even if it's undocumented, but the arguments above do not indicate that this is critical handling in need to be preserved exactly and by all means.

[skyper]

No, I did not try your patch. I only feared that you did not notice the undocumented option. Zooming in and then out with keyboard shortcuts while moving does not work that well for me.

I think, I will have no problem in adjusting my workflow, if the only change is releasing Ctrl while moving.

Regarding documentation: It is a wiki. Do not expect it to be complete or up-to-date. Both, my special case and the new enhancement should be documented.

[anonymous]

Agreed. I will update the wiki with the relevant information (to my best knowledge) if the patch is applied.

[stoecker]

Milestone renamed

[taylor.smock]

A few comments:

• Can you add @since xxx javadocs to the new public methods (EastNorth#heading, DataSet#getLastSelectedOfSelectedWays)
• Can you check and see if platformMenuShortcutKeyMask from r18456 can replace ctrl in your patch. It should be able to (it still is ctrl on Linux/Windows, but is meta on Mac). On mac, the current code will not work
• The patch does not currently compile ("local variables referenced from a lambda expression must be final or effectively final").

[cmuelle8]

Replying to taylor.smock:

• The patch does not currently compile ("local variables referenced from a lambda expression must be final or effectively final").

The tc I use does not exhibit this error, but I presume it occurs for you, because currentEN is modified within the lambda (it is created non-final within mouseDragged(). I've updated the patch to first copy currentEN within the lambda and then work with that, because EastNorth is designed to be immutable once created.

3rd revision also writes platformMenuShortcutKeyMask instead of ctrl for mac users.

[cmuelle8]

josm_SelectAction_-_align_movement_to_vector_given_by_way_last_selected_in_selection_on_Ctrl_modifier_press_-_rev03

[cmuelle8]

I've seen people replacing xxx in @since javadoc by hand - which is why I keyed in a number manually.
Please change it accordingly if it is about to deviate from the revision the patch may be applied with..

[anonymous]

Replying to taylor.smock:

• Can you check and see if platformMenuShortcutKeyMask from r18456 can replace ctrl in your patch. It should be able to (it still is ctrl on Linux/Windows, but is meta on Mac). On mac, the current code will not work

Please make sure you add according branching to the documentation (i.e. wiki / source of the help system) as well, once you're finished moving ctrl to meta as for mac architecture (ignore this, if you have already done so..) - thanks. That help system makes lot of references to using ctrl key modifier and for all the places you modified the code, mac users may find the documentation to be invalid if it has not changed..

[taylor.smock]

Replying to cmuelle8:

I've seen people replacing xxx in @since javadoc by hand - which is why I keyed in a number manually.
Please change it accordingly if it is about to deviate from the revision the patch may be applied with..

We have a script​ for that. We just have to remember to run it.

Replying to anonymous:

Please make sure you add according branching to the documentation (i.e. wiki / source of the help system) as well, once you're finished moving ctrl to meta as for mac architecture (ignore this, if you have already done so..) - thanks. That help system makes lot of references to using ctrl key modifier and for all the places you modified the code, mac users may find the documentation to be invalid if it has not changed..

I'm going to start on that once the next stable is released. I've pre-written part of the changelog so that mac users are alerted to the change.

[anonymous]

Replying to taylor.smock:

I'm going to start on that once the next stable is released. I've pre-written part of the changelog so that mac users are alerted to the change.

Ok, I've just looked at the help for SelectAction and up to now it seems no architecture specific guidance has been included. Since there are so many places that refer to ctrl it may be cumbersome and worsen readability of the doc if each occurence is updated. If we can find a generic clause as to when ctrl is to be read as meta for mac, it may benefit the documentation to identify a central place where this is documented just once, but you probably have thought about this already..

Greetings

[taylor.smock]

I just modified wiki:Help/Action/Select . Yes it is a bit cumbersome. No, there isn't any way to detect OS and show the appropriate shortcut as far as I know.

TBH, the only reason I did decide to update that page is that it has been Ctrl (literally) for years instead of Cmd -- I believe most Mac users are trained to use Cmd instead of Ctrl when they see Ctrl in the keyboard shortcut.

[taylor.smock]

@cmuelle8: Is attachment:josm_SelectAction_-_align_movement_to_vector_given_by_way_last_selected_in_selection_on_Ctrl_modifier_press_-_rev03.patch​ the correct patch?

We haven't had the ds.beginUpdate() call since March 2020 (we are using ds.update(() -> ...)).

[anonymous]

Replying to taylor.smock:

@cmuelle8: Is attachment:josm_SelectAction_-_align_movement_to_vector_given_by_way_last_selected_in_selection_on_Ctrl_modifier_press_-_rev03.patch​ the correct patch?

Yes, it is the correct patch, I've 'backported' it before release - please simply adjust the patch line to reflect that of latest. It is not much of a difference wrt to the patched lines and should work. Let me know if you can't get it applied and I'll get you a new version with the context lines reflecting 'latest'..

For anyone interested: the first two versions of the patch should be usable with all josm versions after r15393 (as for the third patch version ctrl would need to be substituted, because platformMenuShortcutKeyMask variable is not available in the older source versions).

[cmuelle8]

josm_SelectAction_-_align_movement_to_vector_given_by_way_last_selected_in_selection_on_Ctrl_modifier_press_-_rev04.patch

[taylor.smock]

Replying to cmuelle8:

Yes, it is the correct patch, I've 'backported' it before release - please simply adjust the patch line to reflect that of latest. It is not much of a difference wrt to the patched lines and should work. Let me know if you can't get it applied and I'll get you a new version with the context lines reflecting 'latest'..

Erm. Why did you backport it? (As in, should there be something else I need to look at fixing?)

I'll go ahead and move this to 22.06 (I believe we are doing a release this weekend, and I'd rather do bug fixes only for a few days before/after a release).

[cmuelle8]

Replying to taylor.smock:

Erm. Why did you backport it? (As in, should there be something else I need to look at fixing?)

It'd be nice if you can include it in next milestone, it is proven to work.

_rev04 applies cleanly against latest

Sorry for the glitch, but at least people
looking to backport it to an older state
don't need to start from scratch..

The reason I've backported it has nothing to do with the patch itself (!)
I use some plugins that don't work properly with latest, hence the usage
with an older version.

Regards,
cmuelle8

[taylor.smock]

Replying to cmuelle8:

The reason I've backported it has nothing to do with the patch itself (!)
I use some plugins that don't work properly with latest, hence the usage
with an older version.

Which plugins? They should be fixed.

[cmuelle8]

Replying to taylor.smock:

I'll go ahead and move this to 22.06 (I believe we are doing a release this weekend, and I'd rather do bug fixes only for a few days before/after a release).

Also note, that this has been defered for three months already..

The print plugin namely, maybe others, I don't remember them
all and moreover I do not continuously test each and every new
release for state change on the issues I may have found with
an older version.

So while I remember issues with the print plugin, I've not
monitored if that has been fixed in the meantime.

I've also read about a lot of changes to the image plugin
(namely to support 360° panoramic viewing). This plugin had
a history of breakage back between the rev range 10300 ~ 10500,
which is why I may be overly careful in adopting changes to
that code (despite the fact that it may not have problems).

But please notice that we should not escalate on this _here_,
because this issue is clearly unrelated.

[taylor.smock]

Replying to cmuelle8:

Also note, that this has been defered for three months already..

This is a fair complaint. I'm trying to get patches applied, but I tend to prioritize fixing newly reported bugs (just due to how my RSS reader organizes the bugs). I've got a goal to get the open tickets down to < 100, but that is going to take some time.

The print plugin namely, maybe others, I don't remember them
[...snip...]
I've also read about a lot of changes to the image plugin
(namely to support 360° panoramic viewing).[...snip...]

No clue which plugin this is. But JOSM has equirectangular 360° support now.

But please notice that we should not escalate on this _here_,
because this issue is clearly unrelated.

Fair. Please ping/upvote the bugs that you encounter, if they have already been reported. Anything over a year old without new reports/duplicate reports/upvotes is something that is lower down on my priority list, just because they might be working now.

[anonymous]

Replying to taylor.smock:

No clue which plugin this is. But JOSM has equirectangular 360° support now.

Yes, this is not a plugin, but nevertheless
remembering issues with it on my side lead
to a more conservative update strategy..

.. but I'm comfortable with using different
versions of josm, and would not advocate
sticking to an older rev in general.

Upvoting stale bugs has not been a priority
of mine in the past, there was a time span
when this was frowned upon: Too many open
bugs + too few devs == frustration, but I'll
try to think of it.

[taylor.smock]

In 18468/josm:

Fix #21893: Align movement of objects when the last selected way has two nodes and the ctrl/meta modifier is pressed (patch by cmuelle8, modified)