Ticket #6966: ticket-6966-improve-align-nodes-in-line-v1.patch

src/org/openstreetmap/josm/actions/AlignInLineAction.java

@@ -1 +1 @@
 // License: GPL. Copyright 2007 by Immanuel Scholz and others
 package org.openstreetmap.josm.actions;
 
-import static org.openstreetmap.josm.tools.I18n.tr;
 import static org.openstreetmap.josm.gui.help.HelpUtil.ht;
+import static org.openstreetmap.josm.tools.I18n.tr;
 
 import java.awt.event.ActionEvent;
 import java.awt.event.KeyEvent;
+import java.util.ArrayList;
 import java.util.Collection;
-import java.util.LinkedList;
+import java.util.List;
 
 import javax.swing.JOptionPane;
 
@@ -35 +36 @@
         putValue("help", ht("/Action/AlignInLine"));
     }
 
-    /**
-     * The general algorithm here is to find the two selected nodes
-     * that are furthest apart, and then to align all other selected
-     * nodes onto the straight line between these nodes.
-     */
-    public void actionPerformed(ActionEvent e) {
-        if (!isEnabled())
-            return;
-        Collection<OsmPrimitive> sel = getCurrentDataSet().getSelected();
-        Collection<Node> nodes = new LinkedList<Node>();
-        Collection<Node> itnodes = new LinkedList<Node>();
-        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.size() == 0) && (sel.size() == 1)) {
-            for (OsmPrimitive osm : sel)
-                if (osm instanceof Way) {
-                    nodes.addAll(((Way)osm).getNodes());
-                    itnodes.addAll(((Way)osm).getNodes());
-                }
-        }
-        if (nodes.size() < 3) {
-            JOptionPane.showMessageDialog(
-                    Main.parent,
-                    tr("Please select at least three nodes."),
-                    tr("Information"),
-                    JOptionPane.INFORMATION_MESSAGE
-            );
-            return;
-        }
-
+    // the joy of single return values only...
+    private void nodePairFurthestApart(ArrayList<Node> nodes, Node[] resultOut) {
+        if(resultOut.length < 2)
+            throw new IllegalArgumentException();
         // Find from the selected nodes two that are the furthest apart.
         // Let's call them A and B.
         double distance = 0;
@@ -77 +47 @@
         Node nodea = null;
         Node nodeb = null;
 
-        for (Node n : nodes) {
-            itnodes.remove(n);
-            for (Node m : itnodes) {
+        for (int i = 0; i < nodes.size()-1; i++) {
+            Node n = nodes.get(i);
+            for (int j = i+1; j < nodes.size(); j++) {
+                Node m = nodes.get(j);
                 double dist = Math.sqrt(n.getEastNorth().distance(m.getEastNorth()));
                 if (dist > distance) {
                     nodea = n;
@@ -88 +59 @@
                 }
             }
         }
+        resultOut[0] = nodea;
+        resultOut[1] = nodeb;
+    }
+
+    private void showWarning() {
+        JOptionPane.showMessageDialog(
+                Main.parent,
+                tr("Please select at least three nodes."),
+                tr("Information"),
+                JOptionPane.INFORMATION_MESSAGE
+        );
+        return;
+    }
 
-        // Remove the nodes A and B from the list of nodes to move
+    private static int indexWrap(int size, int i) {
+        i = i % size; // -2 % 5 = -2, -7 % 5 = -2, -5 % 5 = 0
+        if (i < 0) {
+            i = size + i;
+        }
+        return i;
+    }
+    // get the node in w at index i relative to refI
+    private static Node getNodeRelative(Way w, int refI, int i) {
+        int absI = indexWrap(w.getNodesCount(), refI + i);
+        if(w.isClosed() && refI + i < 0) {
+            absI--;  // node duplicated in closed ways
+        }
+        return w.getNode(absI);
+    }
+
+    /**
+     * The general algorithm here is to find the two selected nodes
+     * that are furthest apart, and then to align all other selected
+     * nodes onto the straight line between these nodes.
+     */
+
+
+    /**
+     * Operation depends on the selected objects:
+     */
+    public void actionPerformed(ActionEvent e) {
+        if (!isEnabled())
+            return;
+
+        Node[] anchors = new Node[2]; // oh, java I love you so much..
+
+        List<Node> selectedNodes = new ArrayList<Node>(getCurrentDataSet().getSelectedNodes());
+        Collection<Way> selectedWays = getCurrentDataSet().getSelectedWays();
+        ArrayList<Node> nodes = new ArrayList<Node>();
+
+        //// Decide what to align based on selection:
+
+        /// Only ways selected -> Align their nodes.
+        if ((selectedNodes.size() == 0) && (selectedWays.size() == 1)) { // TODO: handle multiple ways
+            for (Way way : selectedWays) {
+                nodes.addAll(way.getNodes());
+            }
+            // use the nodes furthest apart as anchors
+            nodePairFurthestApart(nodes, anchors);
+        }
+        /// More than 3 nodes selected -> align those nodes
+        else if(selectedNodes.size() >= 3) {
+            nodes.addAll(selectedNodes);
+            // use the nodes furthest apart as anchors
+            nodePairFurthestApart(nodes, anchors);
+        }
+        /// One node selected -> align that node to the relevant neighbors
+        else if (selectedNodes.size() == 1) {
+            Node n = selectedNodes.iterator().next();
+
+            Way w = null;
+            if(selectedWays.size() == 1) {
+                w = selectedWays.iterator().next();
+                if(w.containsNode(n) == false)
+                    // warning
+                    return;
+            } else if(n.isReferredByWays(2) == false) { // node used in only one way
+                w = OsmPrimitive.getFilteredList(n.getReferrers(), Way.class).iterator().next();
+            }
+            if (w == null || w.getNodesCount() < 3)
+                // warning, need at least 3 nodes
+                return;
+
+            // Find anchors
+            int nodeI = w.getNodes().indexOf(n);
+            // End-node in non-circular way selected: align this node with the two neighbors.
+            if ((nodeI == 0 || nodeI == w.getNodesCount()-1) && !w.isClosed()) {
+                int direction = nodeI == 0 ? 1 : -1;
+                anchors[0] = w.getNode(nodeI + direction);
+                anchors[1] = w.getNode(nodeI + direction*2);
+            } else {
+                // o---O---o
+                anchors[0] = getNodeRelative(w, nodeI, 1);
+                anchors[1] = getNodeRelative(w, nodeI, -1);
+            }
+            nodes.add(n);
+        }
+
+        if (anchors[0] == null || anchors[1] == null) {
+            showWarning();
+            return;
+        }
+
+
+        Collection<Command> cmds = new ArrayList<Command>(nodes.size());
+
+        createAlignNodesCommands(anchors, nodes, cmds);
+
+        // Do it!
+        Main.main.undoRedo.add(new SequenceCommand(tr("Align Nodes in Line"), cmds));
+        Main.map.repaint();
+    }
+
+    private void createAlignNodesCommands(Node[] anchors, Collection<Node> nodes, Collection<Command> cmds) {
+        Node nodea = anchors[0];
+        Node nodeb = anchors[1];
+
+        // The anchors are aligned per definition
         nodes.remove(nodea);
         nodes.remove(nodeb);
 
@@ -99 +186 @@
         double bx = nodeb.getEastNorth().east();
         double by = nodeb.getEastNorth().north();
 
-        // A list of commands to do
-        Collection<Command> cmds = new LinkedList<Command>();
-
         // OK, for each node to move, work out where to move it!
         for (Node n : nodes) {
             // Get existing co-ords of node to move
@@ -124 +208 @@
                 nx = (c2 - c1) / (m1 - m2);
                 ny = (m1 * nx) + c1;
             }
-
+            double newX = nx - n.getEastNorth().east();
+            double newY = ny - n.getEastNorth().north();
             // Add the command to move the node to its new position.
-            cmds.add(new MoveCommand(n, nx - n.getEastNorth().east(), ny - n.getEastNorth().north() ));
+            cmds.add(new MoveCommand(n, newX, newY));
         }
-
-        // Do it!
-        Main.main.undoRedo.add(new SequenceCommand(tr("Align Nodes in Line"), cmds));
-        Main.map.repaint();
     }
 
     @Override

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

@@ -35 +35 @@
     public void actionPerformed(ActionEvent e) {
         if (!isEnabled())
             return;
-        Collection<OsmPrimitive> sel = getCurrentDataSet().getSelected();
-        if (sel.size() < 1) return;
+        Collection<Node> selectedNodes = getCurrentDataSet().getSelectedNodes();
+        // Allow multiple selected nodes too?
+        if (selectedNodes.size() != 1) return;
+
+        Node node = selectedNodes.iterator().next();
 
         Collection<Command> cmds = new LinkedList<Command>();
 
-        for (OsmPrimitive osm : sel) {
-            if (!(osm instanceof Node)) {
-                continue;
-            }
-            Node node = (Node) osm;
+        // If the user has selected some ways, only join the node to these.
+        boolean restrictToSelectedWays =
+            getCurrentDataSet().getSelectedWays().size() > 0;
 
             List<WaySegment> wss = Main.map.mapView.getNearestWaySegments(
                     Main.map.mapView.getPoint(node), OsmPrimitive.isSelectablePredicate);
             HashMap<Way, List<Integer>> insertPoints = new HashMap<Way, List<Integer>>();
             for (WaySegment ws : wss) {
+                // Maybe cleaner to pass a "isSelected" predicate to getNearestWaySegements, but this is atm. less invasive.
+                if(restrictToSelectedWays && !ws.way.isSelected()) {
+                    continue;
+                }
+
                 List<Integer> is;
                 if (insertPoints.containsKey(ws.way)) {
                     is = insertPoints.get(ws.way);
@@ -80 +86 @@
                 wnew.setNodes(nodesToAdd);
                 cmds.add(new ChangeCommand(w, wnew));
             }
-        }
-        if (cmds.size() == 0) return;
-        Main.main.undoRedo.add(new SequenceCommand(tr("Join Node and Line"), cmds));
-        Main.map.repaint();
+            if (cmds.size() == 0) return;
+            Main.main.undoRedo.add(new SequenceCommand(tr("Join Node and Line"), cmds));
+            Main.map.repaint();
     }
 
     private static void pruneSuccsAndReverse(List<Integer> is) {