Modify

Opened 19 months ago

Closed 18 months ago

Last modified 17 months ago

#20041 closed defect (fixed)

Align nodes in Circle creates Command which changes nothing

Reported by: GerdP Owned by: GerdP
Priority: normal Milestone: 20.12
Component: Core Version: latest
Keywords: template_report Cc: Klumbumbus, skyper

Description

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

Attachments (4)

circle-action.osm (2.9 KB) - added by GerdP 19 months ago.
20041.patch (7.9 KB) - added by GerdP 19 months ago.
20041.2.patch (9.9 KB) - added by GerdP 18 months ago.
alignCircleBefore.osm (7.0 KB) - added by GerdP 18 months ago.

Download all attachments as: .zip

Change History (12)

Changed 19 months ago by GerdP

Attachment: circle-action.osm added

comment:1 Changed 19 months ago by GerdP

Owner: changed from team to GerdP
Status: newassigned

Changed 19 months ago by GerdP

Attachment: 20041.patch added

comment:2 Changed 19 months ago by 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.

Changed 18 months ago by GerdP

Attachment: 20041.2.patch added

comment:3 Changed 18 months ago by 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...

Changed 18 months ago by GerdP

Attachment: alignCircleBefore.osm added

comment:4 Changed 18 months ago by GerdP

Cc: Klumbumbus skyper added

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?

comment:5 Changed 18 months ago by 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)

comment:6 Changed 18 months ago by GerdP

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

comment:7 Changed 18 months ago by GerdP

Resolution: fixed
Status: assignedclosed

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

comment:8 Changed 17 months ago by GerdP

Milestone: 20.12

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain GerdP.
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.