Changeset 15610 in josm for trunk/src

Timestamp:

2019-12-24T11:01:03+01:00 (5 years ago)

Author:

GerdP

Message:

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)

File:

• trunk/src/org/openstreetmap/josm/actions/JoinNodeWayAction.java (modified) (5 diffs)

trunk/src/org/openstreetmap/josm/actions/JoinNodeWayAction.java

@@ -13 +13 @@
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.LinkedHashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
+import java.util.TreeMap;
 
 import javax.swing.JOptionPane;
@@ -91 +94 @@
         Collection<Node> selectedNodes = ds.getSelectedNodes();
         Collection<Command> cmds = new LinkedList<>();
-        Map<Way, MultiMap<Integer, Node>> data = new HashMap<>();
+        Map<Way, MultiMap<Integer, Node>> data = new LinkedHashMap<>();
 
         // If the user has selected some ways, only join the node to these.
@@ -100 +103 @@
         for (Node node : selectedNodes) {
             List<WaySegment> wss = mapView.getNearestWaySegments(mapView.getPoint(node), OsmPrimitive::isSelectable);
-            Set<Way> seenWays = new HashSet<>();
+            // we cannot trust the order of elements in wss because it was calculated based on rounded position value of node
+            TreeMap<Double, List<WaySegment>> nearestMap = new TreeMap<>();
+            EastNorth en = node.getEastNorth();
             for (WaySegment ws : wss) {
                 // Maybe cleaner to pass a "isSelected" predicate to getNearestWaySegments, but this is less invasive.
@@ -106 +111 @@
                     continue;
                 }
-                // only use the closest WaySegment of each way and ignore those that already contain the node
-                if (!ws.getFirstNode().equals(node) && !ws.getSecondNode().equals(node)
-                        && !seenWays.contains(ws.way)) {
-                    MultiMap<Integer, Node> innerMap = data.get(ws.way);
-                    if (innerMap == null) {
-                        innerMap = new MultiMap<>();
-                        data.put(ws.way, innerMap);
+                /* perpendicular distance squared
+                 * loose some precision to account for possible deviations in the calculation above
+                 * e.g. if identical (A and B) come about reversed in another way, values may differ
+                 * -- zero out least significant 32 dual digits of mantissa..
+                 */
+                double distSq = en.distanceSq(Geometry.closestPointToSegment(ws.getFirstNode().getEastNorth(),
+                        ws.getSecondNode().getEastNorth(), en));
+                // resolution in numbers with large exponent not needed here..
+                distSq = Double.longBitsToDouble(Double.doubleToLongBits(distSq) >> 32 << 32);
+                List<WaySegment> wslist = nearestMap.computeIfAbsent(distSq, k -> new LinkedList<>());
+                wslist.add(ws);
+            }
+            Set<Way> seenWays = new HashSet<>();
+            Double usedDist = null;
+            while (!nearestMap.isEmpty()) {
+                Entry<Double, List<WaySegment>> entry = nearestMap.pollFirstEntry();
+                if (usedDist != null) {
+                    double delta = entry.getKey() - usedDist;
+                    if (delta > 1e-4)
+                        break;
+                }
+                for (WaySegment ws : entry.getValue()) {
+                    // only use the closest WaySegment of each way and ignore those that already contain the node
+                    if (!ws.getFirstNode().equals(node) && !ws.getSecondNode().equals(node)
+                            && !seenWays.contains(ws.way)) {
+                        if (usedDist == null)
+                            usedDist = entry.getKey();
+                        MultiMap<Integer, Node> innerMap = data.get(ws.way);
+                        if (innerMap == null) {
+                            innerMap = new MultiMap<>();
+                            data.put(ws.way, innerMap);
+                        }
+                        innerMap.put(ws.lowerIndex, node);
                     }
-                    innerMap.put(ws.lowerIndex, node);
                     seenWays.add(ws.way);
                 }
@@ -142 +172 @@
                         if (prevMove != null) {
                             if (!prevMove.equalsEpsilon(newPosition, 1e-4)) {
+                                // very unlikely: node has same distance to multiple ways which are not nearly overlapping
                                 new Notification(tr("Multiple target ways, no common point found. Nothing was changed."))
                                         .setIcon(JOptionPane.INFORMATION_MESSAGE)