Align nodes in Circle creates Command which changes nothing

[GerdP]

What steps will reproduce the problem?

1. Load attached file with two overlapping ways
2. Select one way
3. Press O for "Align nodes in Circle"

What is the expected result?

Nodes arrange in circle or maybe a message that the selection is not OK

What happens instead?

Nothing obvious in the data, but command stack shows a new Sequence. This can be repeated.

Please provide any additional information below. Attach a screenshot if possible.

Build-Date:2020-11-06 08:56:27
Revision:17299
Is-Local-Build:true

Identification: JOSM/1.5 (17299 SVN en) Windows 10 64-Bit
OS Build number: Windows 10 Home 2004 (19041)
Memory Usage: 766 MB / 1753 MB (322 MB allocated, but free)
Java version: 1.8.0_272-b10, AdoptOpenJDK, OpenJDK 64-Bit Server VM
Look and Feel: com.sun.java.swing.plaf.windows.WindowsLookAndFeel
Screen: \Display0 1920x1080 (scaling 1.0x1.0)
Maximum Screen Size: 1920x1080
Best cursor sizes: 16x16 -> 32x32, 32x32 -> 32x32
Program arguments: [--debug]
Dataset consistency test: No problems found

Plugins:
+ OpeningHoursEditor (35579)
+ apache-commons (35524)
+ apache-http (35092)
+ buildings_tools (35579)
+ continuosDownload (91)
+ ejml (35313)
+ geotools (35169)
+ jaxb (35092)
+ jna (35092)
+ jts (35122)
+ o5m (35248)
+ opendata (35513)
+ pbf (35636)
+ poly (35248)
+ reltoolbox (35602)
+ reverter (35638)
+ undelete (35521)
+ utilsplugin2 (35624)

Validator rules:
+ c:\josm\core\resources\data\validator\geometry.mapcss

[GerdP]

Patch fixes the problem (it shows a notification and doesn't create a command) and simplifies the code. There is no unit test yet, so I'll code one.
Found a few more cases where also the patched code doesn't work well, e.g. with self-intersecting ways. Maybe I can fix that as well, and maybe I can find a fix that allows to align the nodes in my example, too.

[GerdP]

With 20041.2.patch​ the action refuses to work with

• a self-intersecting way
• old nodes outside of download area (the original code only shows an info and continues)

I'll add a unit test to improve coverage...

[GerdP]

While working on the unit test for this action I found out that I get different results where I didn't expect them. Please load alignCircleBefore.osm​ and duplicate the layer.

• In the first layer, select the roundabout way and press O to align in circle
• in the seceond layer, select the roundabout way, press Ctrl+Shift+N to select the nodes and press O to align in circle

Note the result is slightly different for the two layers. Is this intended or is it a bug?

[skyper]

I think these actions do, what they are supposed to do. Without a way the nodes are moved into a circle but not distributed. We need one action without distributing at least for nodes. Think about something like Stonehengh, where nodes are in circles but not evenly distributed.

In my eyes, I would not distribute outside downloaded area, as there might be unknown ways connected to any node.

By the way, I got an AIOOBE but cannot reproduce it. It looked like the CommandStack got stuck while undoing. Switching layers back and for got it working again but undo was not disabled after undoing the first action and another undo led to the exception.

2020-12-02 14:19:25.444 SEVERE: Handled by bug report queue: java.lang.ArrayIndexOutOfBoundsException: Index -1 out of bounds for length 10
java.lang.ArrayIndexOutOfBoundsException: Index -1 out of bounds for length 10
	at java.base/java.util.Vector.elementData(Vector.java:762)
	at java.base/java.util.Vector.elementAt(Vector.java:500)
	at java.desktop/javax.swing.tree.DefaultMutableTreeNode.getChildAt(DefaultMutableTreeNode.java:246)
	at org.openstreetmap.josm.gui.dialogs.CommandStackDialog.swapNode(CommandStackDialog.java:406)
	at org.openstreetmap.josm.gui.dialogs.CommandStackDialog.commandUndone(CommandStackDialog.java:393)
	at org.openstreetmap.josm.data.UndoRedoHandler$CommandUndoneEvent.fire(UndoRedoHandler.java:216)
	at java.base/java.lang.Iterable.forEach(Iterable.java:75)
	at org.openstreetmap.josm.data.UndoRedoHandler.fireEvent(UndoRedoHandler.java:454)
	at org.openstreetmap.josm.data.UndoRedoHandler.lambda$undo$1(UndoRedoHandler.java:398)
	at org.openstreetmap.josm.gui.util.GuiHelper.runInEDTAndWait(GuiHelper.java:224)
	at org.openstreetmap.josm.data.UndoRedoHandler.undo(UndoRedoHandler.java:382)
	at org.openstreetmap.josm.data.UndoRedoHandler.undo(UndoRedoHandler.java:372)
	at org.openstreetmap.josm.actions.UndoAction.actionPerformed(UndoAction.java:39)
	at java.desktop/javax.swing.SwingUtilities.notifyAction(SwingUtilities.java:1810)
	at java.desktop/javax.swing.JComponent.processKeyBinding(JComponent.java:2900)
	at java.desktop/javax.swing.KeyboardManager.fireBinding(KeyboardManager.java:311)
	at java.desktop/javax.swing.KeyboardManager.fireKeyboardAction(KeyboardManager.java:266)
	at java.desktop/javax.swing.JComponent.processKeyBindingsForAllComponents(JComponent.java:2993)
	at java.desktop/javax.swing.JComponent.processKeyBindings(JComponent.java:2985)
	at java.desktop/javax.swing.JComponent.processKeyEvent(JComponent.java:2862)
	at java.desktop/java.awt.Component.processEvent(Component.java:6412)
	at java.desktop/java.awt.Container.processEvent(Container.java:2263)
	at java.desktop/java.awt.Component.dispatchEventImpl(Component.java:5011)
	at java.desktop/java.awt.Container.dispatchEventImpl(Container.java:2321)
	at java.desktop/java.awt.Component.dispatchEvent(Component.java:4843)
	at java.desktop/java.awt.KeyboardFocusManager.redispatchEvent(KeyboardFocusManager.java:1950)
	at java.desktop/java.awt.DefaultKeyboardFocusManager.dispatchKeyEvent(DefaultKeyboardFocusManager.java:870)
	at java.desktop/java.awt.DefaultKeyboardFocusManager.preDispatchKeyEvent(DefaultKeyboardFocusManager.java:1139)
	at java.desktop/java.awt.DefaultKeyboardFocusManager.typeAheadAssertions(DefaultKeyboardFocusManager.java:1009)
	at java.desktop/java.awt.DefaultKeyboardFocusManager.dispatchEvent(DefaultKeyboardFocusManager.java:835)
	at java.desktop/java.awt.Component.dispatchEventImpl(Component.java:4892)
	at java.desktop/java.awt.Container.dispatchEventImpl(Container.java:2321)
	at java.desktop/java.awt.Window.dispatchEventImpl(Window.java:2772)
	at java.desktop/java.awt.Component.dispatchEvent(Component.java:4843)
	at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:772)
	at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:721)
	at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:715)
	at java.base/java.security.AccessController.doPrivileged(Native Method)
	at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:85)
	at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:95)
	at java.desktop/java.awt.EventQueue$5.run(EventQueue.java:745)
	at java.desktop/java.awt.EventQueue$5.run(EventQueue.java:743)
	at java.base/java.security.AccessController.doPrivileged(Native Method)
	at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:85)
	at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:742)
	at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203)
	at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124)
	at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113)
	at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109)
	at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
	at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90)

[GerdP]

The traceback is not related to this ticket, but may be important for #17196

[GerdP]

In 17386/josm:

fix #20041: Align nodes in Circle creates Command which changes nothing

• only create move command if node is visibly moved
• reject a self-intersecting way
• reject old nodes outside of download area (the original code only shows an info and continues)
• add some unit tests to improve coverage