Move node onto ways not working anymore

[sharcrash]

What steps will reproduce the problem?

1. pressing n shortcut while having selected one node near several close ways

What is the expected result?

connect

What happens instead?

nothing

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

It used to work

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2019-12-01 23:10:15 +0100 (Sun, 01 Dec 2019)
Build-Date:2019-12-02 02:30:57
Revision:15553
Relative:URL: ^/trunk

Identification: JOSM/1.5 (15553 en) Windows 7 64-Bit
OS Build number: Windows 7 Professional (7601)
Memory Usage: 1228 MB / 1820 MB (354 MB allocated, but free)
Java version: 1.8.0_231-b11, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Screen: \Display0 1280x1024, \Display1 1280x1024
Maximum Screen Size: 1280x1024
Dataset consistency test: No problems found

Plugins:
+ SimplifyArea (34977)
+ areaselector (359)
+ austriaaddresshelper (57)
+ buildings_tools (35171)
+ continuosDownload (82)
+ contourmerge (v0.1.5)
+ ejml (35122)
+ geochat (35163)
+ imagery_offset_db (34908)
+ log4j (34908)
+ reltoolbox (35196)
+ reverter (35226)
+ turnrestrictions (34977)
+ undelete (34977)
+ utilsplugin2 (35238)
+ wikipedia (1.1.3)

Map paint styles:
- https://josm.openstreetmap.de/josmfile?page=Styles/Fixme&zip=1
- https://github.com/bastik/mapcss-tools/raw/osm/mapnik2mapcss/osm-results/mapnik.zip
- https://josm.openstreetmap.de/josmfile?page=Styles/Surface&zip=1
+ https://github.com/gmgeo/osmic-josm-style/archive/master.zip
- https://josm.openstreetmap.de/josmfile?page=Styles/Enhanced_Lane_and_Road_Attributes&zip=1

Last errors/warnings:
- W: java.net.SocketTimeoutException: Read timed out
- W: java.net.SocketTimeoutException: Read timed out
- W: java.net.SocketTimeoutException: Read timed out
- W: java.net.SocketTimeoutException: Read timed out
- W: java.net.SocketTimeoutException: Read timed out
- W: java.net.SocketTimeoutException: Read timed out
- W: java.net.SocketTimeoutException: Read timed out
- E: Could not get any sources:
- E: Could not get any sources:
- E: Could not get any sources:

[Don-vip]

Can you please attach a sample .osm file showing the problem?

[Don-vip]

Could it be a regression of r15428?

[GerdP]

Replying to Don-vip:

Could it be a regression of r15428?

Hard to say without an example. The results depend on the distance and the zoom and the resolution of the screen and a preference setting.
Load attached example and select the single node. On my PC (1920x1080 screen resolution) I see this
Case 1 Press 3 (zoom to selection) followed by n: Nothing happens
Case 2: Press - (zoom out) followed by n: Node is joined to the closest building
Case 3: undo, 3, zoom out 2 or 3 times, n : Node is joined to the closest building
Case 4: undo, 3, zoom out 4 or more times, n : Popup saying that there are multiple target ways

I think Case 1 is the problem here. Reason for the behaviour is that the action selects way segments within a fixed range of screen pixels using NavigatableComponent.getNearestWaySegmentsImpl(). The range value is stored in preference mappaint.segment.snap-distance, default is 10, for screens with a high resoltion this might be too small.

[GerdP]

case 4 old version

[GerdP]

case 4 old version zoomed out several times

[GerdP]

Before r15428 the behaviour was different for Case 4: The action connected two or more building in a rather unpredictable way:

If n is pressed when zoomed out far the result was complete weird:

[sharcrash]

Hello! Sorry for the late reply, i totally had forgotten.

Here is a sample with a road centred between a forest area and a grass area. Some nodes are not connected and would be useful to smoothen the road direction according real situation. If i press n to attached the node to both areas at once, it doesn't do it and we get the message: multiple target ways, no common point found, nothing happened. It used to work though in a previous JOSM version. Not sure when/which sorry, because sometimes i don't update directly.

[sharcrash]

Sample situation

[GerdP]

Ah, yes, I think this might be caused by the changes in r15428, it is a variant of case 4 described above.
Not sure if the old code worked well in this case, but I'll try to change the code so that it selects the closest of multiple segments in "snap distance".
As a work around you can select the way to which the node should be moved.

[GerdP]

I am not sure where to fix this problem. I've learned that the results returned from NavigatableComponent.getNearestWaySegmentsImpl() are not reliable when zoomed out far. It should return the nearest way segments ordered by perpendiculer distance but it doesn't. It returns the wanted segments, but the order is not always correct. The problem is that the position of the selected node is rounded to int values BEFORE doing the distance calculations. The more you zoom out the larger the rounding error.
The current algortihm in JoinNodeWayAction relies on the correct order.
So, I could either implement the sorting in JoinNodeWayAction again using the precise positions or we could change the interface to one of the oldest routines in JOSM to use Point2D instead of Point.

[GerdP]

Clarification: The code in NavigatableComponent.getNearestWaySegmentsImpl() might be correct, it just doesn't make much sense to do the sorting based on a Point rounded to int.
Anyway, there is also a problem in the JoinNodeWayAction code, so I'll try to fix that first using a locally sorted list.

[GerdP]

The algorithm is quite complex because it allows multiple nodes and target ways. Goal is to find the closest way segment and to move the node onto that segment, but also onto other segments which are overlapping or almost overlapping at the place where the node is moved. I hope I got it right now.
I found a possible reason for the random unit test failures. I changed field data from HashMap to LinkedHashMap because we iterate over the elements and that should not be done in a random order. I'll try again to code stable tests including the problems in this ticket...

[GerdP]

In 15610/josm:

fix #18420: Move node onto ways not working anymore

• sort result from NavigatableComponent again using higher precision
• improve handling of multiple target ways with similar distance so that the node is added to the closest segment and also to segments with almost the same position but NOT to segments further away or with a similar distance but different position.
• fix possible error when multiple nodes are selected
• use LinkedHashMap for field data to avoid random results from iterator (hope this fixes the unit test problems)

[sharcrash]

That should help us working faster and if needed to connect a node to multiple ways, pressing again n should do the trick now. Thanks!