Ticket #20041: 20041.2.patch

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

@@ -27 +27 @@
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.Way;
+import org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon;
+import org.openstreetmap.josm.data.validation.tests.SelfIntersectingWay;
 import org.openstreetmap.josm.gui.Notification;
 import org.openstreetmap.josm.tools.Geometry;
 import org.openstreetmap.josm.tools.Shortcut;
@@ -54 +56 @@
     }
 
     /**
-     * Create a {@link MoveCommand} to move a node to a PolarCoor.
+     * Add a {@link MoveCommand} to move a node to a PolarCoor if there is a significant move.
      * @param n Node to move
      * @param coor polar coordinate where to move the node
-     * @return new MoveCommand
-     * @since 13107
+     * @param cmds list of commands
+     * @since xxx
      */
-    public static MoveCommand createMoveCommand(Node n, PolarCoor coor) {
+    public static void addMoveCommandIfNeeded(Node n, PolarCoor coor, List<Command> cmds) {
         EastNorth en = coor.toEastNorth();
-        return new MoveCommand(n, en.east() - n.getEastNorth().east(), en.north() - n.getEastNorth().north());
+        double deltaEast = en.east() - n.getEastNorth().east();
+        double deltaNorth = en.north() - n.getEastNorth().north();
+        if (Math.abs(deltaEast) > 5e-6 || Math.abs(deltaNorth) > 5e-6) {
+            cmds.add(new MoveCommand(n, deltaEast, deltaNorth));
+        }
     }
 
     /**
@@ -112 +118 @@
         if (ways.size() == 1 && !ways.get(0).isClosed()) {
             // Case 1
             Way w = ways.get(0);
+            SelfIntersectingWay test = new SelfIntersectingWay();
+            test.startTest(null);
+            test.visit(w);
+            test.endTest();
+            if (!test.getErrors().isEmpty()) {
+                showInvalidSelection(tr("Self-intersecting way"));
+                return;
+            }
+
             fixNodes.add(w.firstNode());
             fixNodes.add(w.lastNode());
             fixNodes.addAll(nodes);
@@ -151 +166 @@
             fixNodes.addAll(collectNodesWithExternReferrers(ways));
             nodes = collectNodesAnticlockwise(ways);
             if (nodes.size() < 4) {
-                new Notification(
-                        tr("Not enough nodes in selected ways."))
-                .setIcon(JOptionPane.INFORMATION_MESSAGE)
-                .setDuration(Notification.TIME_SHORT)
-                .show();
+                showInvalidSelection(tr("Not enough nodes in selected ways."));
                 return;
             }
         } else if (ways.isEmpty() && nodes.size() > 3) {
@@ -163 +174 @@
             fixNodes.addAll(nodes);
             // No need to reorder nodes since all are fix
         } else {
-            // Invalid action
-            new Notification(
-                    tr("Please select at least four nodes."))
-                    .setIcon(JOptionPane.INFORMATION_MESSAGE)
-                    .setDuration(Notification.TIME_SHORT)
-                    .show();
+            showInvalidSelection(null);
             return;
         }
 
+        if (!actionAllowed(nodes)) return;
+
         if (center == null) {
             // Compute the center of nodes
             center = Geometry.getCenter(nodes);
             if (center == null) {
-                new Notification(tr("Cannot determine center of selected nodes."))
-                    .setIcon(JOptionPane.INFORMATION_MESSAGE)
-                    .setDuration(Notification.TIME_SHORT)
-                    .show();
+                showInvalidSelection(tr("Cannot determine center of selected nodes."));
                 return;
             }
         }
@@ -194 +199 @@
             radius = radius / nodes.size();
         }
 
-        if (!actionAllowed(nodes)) return;
+        List<Command> cmds = new LinkedList<>();
 
-        Collection<Command> cmds = new LinkedList<>();
-
         // Move each node to that distance from the center.
         // Nodes that are not "fix" will be adjust making regular arcs.
         int nodeCount = nodes.size();
@@ -216 +219 @@
             }
             Node first = nodes.get(i % nodeCount);
             PolarCoor pcFirst = new PolarCoor(radius, PolarCoor.computeAngle(first.getEastNorth(), center), center);
-            cmds.add(createMoveCommand(first, pcFirst));
+            addMoveCommandIfNeeded(first, pcFirst, cmds);
             if (j > i + 1) {
                 double delta;
                 if (j == i + nodeCount) {
@@ -230 +233 @@
                 }
                 for (int k = i+1; k < j; k++) {
                     PolarCoor p = new PolarCoor(radius, pcFirst.angle + (k-i)*delta, center);
-                    cmds.add(createMoveCommand(nodes.get(k % nodeCount), p));
+                    addMoveCommandIfNeeded(nodes.get(k % nodeCount), p, cmds);
                 }
             }
             i = j; // Update start point for next iteration
         }
+        if (cmds.isEmpty()) {
+            showInvalidSelection(null);
+        } else {
+            UndoRedoHandler.getInstance().add(new SequenceCommand(tr("Align Nodes in Circle"), cmds));
+        }
+    }
 
-        UndoRedoHandler.getInstance().add(new SequenceCommand(tr("Align Nodes in Circle"), cmds));
+    private static void showInvalidSelection(String message) {
+        if (message == null)
+            message = tr("Selection could not be used to align in circle.");
+        // Invalid action
+        new Notification(message)
+                .setIcon(JOptionPane.INFORMATION_MESSAGE)
+                .setDuration(Notification.TIME_SHORT)
+                .show();
     }
 
     /**
@@ -254 +270 @@
      * @return Nodes anticlockwise ordered
      */
     private static List<Node> collectNodesAnticlockwise(List<Way> ways) {
-        List<Node> nodes = new ArrayList<>();
-        Node firstNode = ways.get(0).firstNode();
-        Node lastNode = null;
-        Way lastWay = null;
-        while (firstNode != lastNode) {
-            if (lastNode == null) lastNode = firstNode;
-            for (Way way: ways) {
-                if (way == lastWay) continue;
-                if (way.firstNode() == lastNode) {
-                    List<Node> wayNodes = way.getNodes();
-                    for (int i = 0; i < wayNodes.size() - 1; i++) {
-                        nodes.add(wayNodes.get(i));
-                    }
-                    lastNode = way.lastNode();
-                    lastWay = way;
-                    break;
-                }
-                if (way.lastNode() == lastNode) {
-                    List<Node> wayNodes = way.getNodes();
-                    for (int i = wayNodes.size() - 1; i > 0; i--) {
-                        nodes.add(wayNodes.get(i));
-                    }
-                    lastNode = way.firstNode();
-                    lastWay = way;
-                    break;
-                }
-            }
-        }
-        // Check if nodes are in anticlockwise order
-        int nc = nodes.size();
-        double area = 0;
-        for (int i = 0; i < nc; i++) {
-            EastNorth p1 = nodes.get(i).getEastNorth();
-            EastNorth p2 = nodes.get((i+1) % nc).getEastNorth();
-            area += p1.east()*p2.north() - p2.east()*p1.north();
-        }
-        if (area < 0)
+        List<Node> nodes = new ArrayList<>(Multipolygon.joinWays(ways).iterator().next().getNodes());
+        if (Geometry.isClockwise(nodes))
             Collections.reverse(nodes);
+        nodes.remove(nodes.size() - 1);
         return nodes;
     }
 
@@ -308 +290 @@
                     .setIcon(JOptionPane.WARNING_MESSAGE)
                     .setDuration(Notification.TIME_SHORT)
                     .show();
-        return true;
+        return !outside;
     }
 
     @Override
@@ -323 +305 @@
     }
 
     /**
-     * Determines if ways can be joined into a polygon.
+     * Determines if ways can be joined into a single polygon.
      * @param ways The ways collection to check
-     * @return true if all ways can be joined into a polygon
+     * @return true if all ways can be joined into a single polygon
      */
     private static boolean checkWaysArePolygon(Collection<Way> ways) {
+        if (Multipolygon.joinWays(ways).size() != 1)
+            return false;
         // For each way, nodes strictly between first and last should't be reference by an other way
         for (Way way: ways) {
             for (Node node: way.getNodes()) {
@@ -337 +321 @@
                 }
             }
         }
-        // Test if ways can be joined
-        Way currentWay = null;
-        Node startNode = null, endNode = null;
-        int used = 0;
-        while (true) {
-            Way nextWay = null;
-            for (Way w: ways) {
-                if (w.isClosed()) return ways.size() == 1;
-                if (w == currentWay) continue;
-                if (currentWay == null) {
-                    nextWay = w;
-                    startNode = w.firstNode();
-                    endNode = w.lastNode();
-                    break;
-                }
-                if (w.firstNode() == endNode) {
-                    nextWay = w;
-                    endNode = w.lastNode();
-                    break;
-                }
-                if (w.lastNode() == endNode) {
-                    nextWay = w;
-                    endNode = w.firstNode();
-                    break;
-                }
-            }
-            if (nextWay == null) return false;
-            used += 1;
-            currentWay = nextWay;
-            if (endNode == startNode) return used == ways.size();
-        }
+        return true;
     }
+
 }