Changeset 15428 in josm

Timestamp:

2019-10-06T11:41:29+02:00 (5 years ago)

Author:

GerdP

Message:

fix #18189 (18189-v3.patch)

Location:

trunk

Files:

• src/org/openstreetmap/josm/actions/JoinNodeWayAction.java (modified) (6 diffs)
• test/data/regress/11508 (added)
• test/data/regress/11508/11508_example.osm (added)
• test/data/regress/18189 (added)
• test/data/regress/18189/data.osm (added)
• test/data/regress/18189/moveontocrossing.osm (added)
• test/data/regress/18189/moveontoway.osm (added)
• test/unit/org/openstreetmap/josm/actions/JoinNodeWayActionTest.java (added)

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

@@ -12 +12 @@
 import java.util.Comparator;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.SortedSet;
-import java.util.TreeSet;
+
+import javax.swing.JOptionPane;
 
 import org.openstreetmap.josm.command.ChangeCommand;
@@ -33 +34 @@
 import org.openstreetmap.josm.gui.MainApplication;
 import org.openstreetmap.josm.gui.MapView;
+import org.openstreetmap.josm.gui.Notification;
 import org.openstreetmap.josm.tools.Geometry;
 import org.openstreetmap.josm.tools.MultiMap;
@@ -98 +100 @@
         for (Node node : selectedNodes) {
             List<WaySegment> wss = mapView.getNearestWaySegments(mapView.getPoint(node), OsmPrimitive::isSelectable);
-            MultiMap<Way, Integer> insertPoints = new MultiMap<>();
+            Set<Way> seenWays = new HashSet<>();
             for (WaySegment ws : wss) {
                 // Maybe cleaner to pass a "isSelected" predicate to getNearestWaySegments, but this is less invasive.
@@ -104 +106 @@
                     continue;
                 }
-
-                if (!ws.getFirstNode().equals(node) && !ws.getSecondNode().equals(node)) {
-                    insertPoints.put(ws.way, ws.lowerIndex);
+                // 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);
+                    }
+                    innerMap.put(ws.lowerIndex, node);
+                    seenWays.add(ws.way);
                 }
             }
-            for (Map.Entry<Way, Set<Integer>> entry : insertPoints.entrySet()) {
-                final Way w = entry.getKey();
-                final Set<Integer> insertPointsForWay = entry.getValue();
-                for (int i : pruneSuccs(insertPointsForWay)) {
-                    MultiMap<Integer, Node> innerMap;
-                    if (!data.containsKey(w)) {
-                        innerMap = new MultiMap<>();
-                    } else {
-                        innerMap = data.get(w);
-                    }
-                    innerMap.put(i, node);
-                    data.put(w, innerMap);
-                }
-            }
         }
 
         // Execute phase: traverse the structure "data" and finally put the nodes into place
+        Map<Node, EastNorth> movedNodes = new HashMap<>();
         for (Map.Entry<Way, MultiMap<Integer, Node>> entry : data.entrySet()) {
             final Way w = entry.getKey();
@@ -143 +139 @@
                                 w.getNode(segmentIndex+1).getEastNorth(),
                                 node.getEastNorth());
-                        MoveCommand c = new MoveCommand(
-                                node, ProjectionRegistry.getProjection().eastNorth2latlon(newPosition));
-                        // Avoid moving a given node several times at the same position in case of overlapping ways
-                        if (!cmds.contains(c)) {
-                            cmds.add(c);
+                        EastNorth prevMove = movedNodes.get(node);
+                        if (prevMove != null) {
+                            if (!prevMove.equalsEpsilon(newPosition, 1e-4)) {
+                                new Notification(tr("Multiple target ways, no common point found. Nothing was changed."))
+                                        .setIcon(JOptionPane.INFORMATION_MESSAGE)
+                                        .show();
+                                return;
+                            }
+                            continue;
                         }
+                        MoveCommand c = new MoveCommand(node,
+                                ProjectionRegistry.getProjection().eastNorth2latlon(newPosition));
+                        cmds.add(c);
+                        movedNodes.put(node, newPosition);
                     }
                 }
@@ -166 +170 @@
     }
 
-    private static SortedSet<Integer> pruneSuccs(Collection<Integer> is) {
-        SortedSet<Integer> is2 = new TreeSet<>();
-        for (int i : is) {
-            if (!is2.contains(i - 1) && !is2.contains(i + 1)) {
-                is2.add(i);
-            }
-        }
-        return is2;
-    }
-
     /**
      * Sorts collinear nodes by their distance to a common reference node.