Changeset 8247 in josm for trunk/src/org

Timestamp:

2015-04-22T22:21:50+02:00 (10 years ago)

Author:

Balaitous

Message:

fix #11302 - "distribute nodes" acts weirdly if the way is not reasonably straight already

When one way (no self-crossing) is selected with at most two of its nodes, distibute node keeping nodes order along the way. Selected nodes are kept in place.
In any other case, no changes are made to the algorithm.

File:

• trunk/src/org/openstreetmap/josm/actions/DistributeAction.java (modified) (6 diffs)

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

@@ -11 +11 @@
 import java.util.Iterator;
 import java.util.LinkedList;
+import java.util.List;
 import java.util.Set;
 
@@ -32 +33 @@
 public final class DistributeAction extends JosmAction {
 
+    private static final String ACTION_SHORT_NAME = "Distribute Nodes";
+
     /**
      * Constructs a new {@code DistributeAction}.
      */
     public DistributeAction() {
-        super(tr("Distribute Nodes"), "distribute", tr("Distribute the selected nodes to equal distances along a line."),
-                Shortcut.registerShortcut("tools:distribute", tr("Tool: {0}", tr("Distribute Nodes")), KeyEvent.VK_B,
-                Shortcut.SHIFT), true);
+        super(tr(ACTION_SHORT_NAME), "distribute",
+              tr("Distribute the selected nodes to equal distances along a line."),
+              Shortcut.registerShortcut("tools:distribute",
+                                        tr("Tool: {0}", tr(ACTION_SHORT_NAME)),
+                                        KeyEvent.VK_B, Shortcut.SHIFT),
+              true);
         putValue("help", ht("/Action/DistributeNodes"));
     }
 
     /**
-     * The general algorithm here is to find the two selected nodes
-     * that are furthest apart, and then to distribute all other selected
-     * nodes along the straight line between these nodes.
+     * Perform action.
+     * Select method according to user selection.
+     * Case 1: One Way (no self-crossing) and at most 2 nodes contains by this way:
+     *     Distribute nodes keeping order along the way
+     * Case 2: Other
+     *     Distribute nodes
      */
     @Override
@@ -51 +60 @@
         if (!isEnabled())
             return;
-        Collection<OsmPrimitive> sel = getCurrentDataSet().getSelected();
-        Collection<Node> nodes = new LinkedList<>();
-        Collection<Node> itnodes = new LinkedList<>();
-        for (OsmPrimitive osm : sel)
-            if (osm instanceof Node) {
-                nodes.add((Node)osm);
-                itnodes.add((Node)osm);
-            }
-        // special case if no single nodes are selected and exactly one way is:
-        // then use the way's nodes
-        if (nodes.isEmpty() && (sel.size() == 1)) {
-            for (OsmPrimitive osm : sel)
-                if (osm instanceof Way) {
-                    nodes.addAll(((Way)osm).getNodes());
-                    itnodes.addAll(((Way)osm).getNodes());
-                }
+
+        // Collect user selected objects
+        Collection<OsmPrimitive> selected = getCurrentDataSet().getSelected();
+        Collection<Way> ways = new LinkedList<>();
+        Collection<Node> nodes = new HashSet<>();
+        for(OsmPrimitive osm : selected) {
+            if(osm instanceof Node) {
+                nodes.add((Node) osm);
+            } else if(osm instanceof Way) {
+                ways.add((Way) osm);
+            }
         }
 
         Set<Node> ignoredNodes = removeNodesWithoutCoordinates(nodes);
-        ignoredNodes.addAll(removeNodesWithoutCoordinates(itnodes));
         if (!ignoredNodes.isEmpty()) {
             Main.warn(tr("Ignoring {0} nodes with null coordinates", ignoredNodes.size()));
@@ -76 +79 @@
         }
 
-        if (nodes.size() < 3) {
+        // Switch between algorithms
+        Collection<Command> cmds;
+        if(checkDistributeWay(ways, nodes)) {
+            cmds = distributeWay(ways, nodes);
+        } else if(checkDistributeNodes(ways, nodes)) {
+            cmds = distributeNodes(nodes);
+        } else {
             new Notification(
-                    tr("Please select at least three nodes."))
-                    .setIcon(JOptionPane.INFORMATION_MESSAGE)
-                    .setDuration(Notification.TIME_SHORT)
-                    .show();
+                             tr("Please select :\n" +
+                                "* One no self-crossing way with at most two of its nodes;\n" +
+                                "* Three nodes."))
+                .setIcon(JOptionPane.INFORMATION_MESSAGE)
+                .setDuration(Notification.TIME_SHORT)
+                .show();
             return;
         }
 
+        if(cmds.isEmpty()) {
+            return;
+        }
+
+        // Do it!
+        Main.main.undoRedo.add(new SequenceCommand(tr(ACTION_SHORT_NAME), cmds));
+        Main.map.repaint();
+    }
+
+    /**
+     * Test if one way, no self-crossing, is selected with at most two of its nodes.
+     * @param ways Selected ways
+     * @param nodes Selected nodes
+     * @return true in this case
+     */
+    private Boolean checkDistributeWay(Collection<Way> ways, Collection<Node> nodes) {
+        if(ways.size() == 1 && nodes.size() <= 2) {
+            Way w = ways.iterator().next();
+            Set<Node> unduplicated = new HashSet<>(w.getNodes());
+            if(unduplicated.size() != w.getNodesCount()) {
+                // No self crossing way
+                return false;
+            }
+            for(Node node: nodes) {
+                if(!w.containsNode(node)) {
+                    return false;
+                }
+            }
+            return true;
+        }
+        return false;
+    }
+
+    /**
+     * Distribute nodes contained by a way, keeping nodes order.
+     * If one or two nodes are selected, keep these nodes in place.
+     * @param ways Selected ways, must be collection of size 1.
+     * @param nodes Selected nodes, at most two nodes.
+     * @return Collection of command to be executed.
+     */
+    private Collection<Command> distributeWay(Collection<Way> ways,
+                                              Collection<Node> nodes) {
+        Way w = ways.iterator().next();
+        Collection<Command> cmds = new LinkedList<>();
+
+        if(w.getNodesCount() == nodes.size() || w.getNodesCount() <= 2) {
+            // Nothing to do
+            return cmds;
+        }
+
+        double xa, ya; // Start point
+        double dx, dy; // Segment increment
+        if(nodes.isEmpty()) {
+            Node na = w.firstNode();
+            nodes.add(na);
+            Node nb = w.lastNode();
+            nodes.add(nb);
+            xa = na.getEastNorth().east();
+            ya = na.getEastNorth().north();
+            dx = (nb.getEastNorth().east() - xa) / (w.getNodesCount() - 1);
+            dy = (nb.getEastNorth().north() - ya) / (w.getNodesCount() - 1);
+        } else if(nodes.size() == 1) {
+            Node n = nodes.iterator().next();
+            int nIdx = w.getNodes().indexOf(n);
+            Node na = w.firstNode();
+            Node nb = w.lastNode();
+            dx = (nb.getEastNorth().east() - na.getEastNorth().east()) /
+                (w.getNodesCount() - 1);
+            dy = (nb.getEastNorth().north() - na.getEastNorth().north()) /
+                (w.getNodesCount() - 1);
+            xa = n.getEastNorth().east() - dx * nIdx;
+            ya = n.getEastNorth().north() - dy * nIdx;
+        } else {
+            Iterator<Node> it = nodes.iterator();
+            Node na = it.next();
+            Node nb = it.next();
+            List<Node> wayNodes = w.getNodes();
+            int naIdx = wayNodes.indexOf(na);
+            int nbIdx = wayNodes.indexOf(nb);
+            dx = (nb.getEastNorth().east() - na.getEastNorth().east()) / (nbIdx - naIdx);
+            dy = (nb.getEastNorth().north() - na.getEastNorth().north()) / (nbIdx - naIdx);
+            xa = na.getEastNorth().east() - dx * naIdx;
+            ya = na.getEastNorth().north() - dy * naIdx;
+        }
+
+        for(int i = 0; i < w.getNodesCount(); i++) {
+            Node n = w.getNode(i);
+            if (!n.isLatLonKnown() || nodes.contains(n)) {
+                continue;
+            }
+            double x = xa + i * dx;
+            double y = ya + i * dy;
+            cmds.add(new MoveCommand(n, x - n.getEastNorth().east(),
+                                     y - n.getEastNorth().north()));
+        }
+        return cmds;
+    }
+
+    /**
+     * Test if nodes oriented algorithm applies to the selection.
+     * @param ways Selected ways
+     * @param nodes Selected nodes
+     * @return true in this case
+     */
+    private Boolean checkDistributeNodes(Collection<Way> ways, Collection<Node> nodes) {
+        return ways.isEmpty() && nodes.size() >= 3;
+    }
+
+    /**
+     * Distribute nodes when only nodes are selected.
+     * The general algorithm here is to find the two selected nodes
+     * that are furthest apart, and then to distribute all other selected
+     * nodes along the straight line between these nodes.
+     * @return Commands to execute to perform action
+     */
+    private Collection<Command> distributeNodes(Collection<Node> nodes) {
         // Find from the selected nodes two that are the furthest apart.
         // Let's call them A and B.
@@ -92 +219 @@
         Node nodeb = null;
 
+        Collection<Node> itnodes = new LinkedList<>(nodes);
         for (Node n : nodes) {
             itnodes.remove(n);
@@ -146 +274 @@
         }
 
-        // Do it!
-        Main.main.undoRedo.add(new SequenceCommand(tr("Distribute Nodes"), cmds));
-        Main.map.repaint();
-    }
-
+        return cmds;
+    }
+
+    /**
+     * Remove nodes without knowned coordinates from a collection.
+     * @param col Collection of nodes to check
+     * @return Set of nodes without coordinates
+     */
     private Set<Node> removeNodesWithoutCoordinates(Collection<Node> col) {
         Set<Node> result = new HashSet<>();