Modify

Opened 2 months ago

Closed 7 weeks ago

Last modified 3 weeks 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 2 months ago.
20041.patch (7.9 KB) - added by GerdP 2 months ago.
20041.2.patch (9.9 KB) - added by GerdP 7 weeks ago.
alignCircleBefore.osm (7.0 KB) - added by GerdP 7 weeks ago.

Download all attachments as: .zip

Change History (12)

Changed 2 months ago by GerdP

Attachment: circle-action.osm added

comment:1 Changed 2 months ago by GerdP

Owner: changed from team to GerdP
Status: newassigned

Changed 2 months ago by GerdP

Attachment: 20041.patch added

comment:2 Changed 2 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 7 weeks ago by GerdP

Attachment: 20041.2.patch added

comment:3 Changed 7 weeks 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 7 weeks ago by GerdP

Attachment: alignCircleBefore.osm added

comment:4 Changed 7 weeks 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 7 weeks 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 7 weeks ago by GerdP

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

comment:7 Changed 7 weeks 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 3 weeks 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.