Index: trunk/src/org/openstreetmap/josm/actions/AlignInCircleAction.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/AlignInCircleAction.java (revision 17392) +++ trunk/src/org/openstreetmap/josm/actions/AlignInCircleAction.java (revision 17393) @@ -14,4 +14,6 @@ import java.util.List; import java.util.Set; +import java.util.SortedMap; +import java.util.TreeMap; import java.util.stream.Collectors; @@ -30,4 +32,5 @@ import org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon; import org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon.JoinedWay; +import org.openstreetmap.josm.data.validation.tests.CrossingWays; import org.openstreetmap.josm.data.validation.tests.SelfIntersectingWay; import org.openstreetmap.josm.gui.Notification; @@ -43,4 +46,5 @@ * @author Teemu Koskinen * @author Alain Delplanque + * @author Gerd Petermann * * @since 146 @@ -107,5 +111,13 @@ try { Command cmd = buildCommand(getLayerManager().getEditDataSet()); - UndoRedoHandler.getInstance().add(cmd); + if (cmd != null) + UndoRedoHandler.getInstance().add(cmd); + else { + new Notification(tr("nothing changed")) + .setIcon(JOptionPane.INFORMATION_MESSAGE) + .setDuration(Notification.TIME_SHORT) + .show(); + + } } catch (InvalidSelection except) { Logging.debug(except); @@ -115,5 +127,4 @@ .show(); } - } @@ -140,5 +151,5 @@ * --> Align these nodes, all are fix * @param ds data set in which the command operates - * @return the resulting command to execute to perform action + * @return the resulting command to execute to perform action, or null if nothing was changed * @throws InvalidSelection if selection cannot be used * @since 17386 @@ -147,5 +158,5 @@ public static Command buildCommand(DataSet ds) throws InvalidSelection { Collection sel = ds.getSelected(); - List nodes = new LinkedList<>(); + List selectedNodes = new LinkedList<>(); // fixNodes: All nodes for which the angle relative to center should not be modified Set fixNodes = new HashSet<>(); @@ -156,5 +167,5 @@ for (OsmPrimitive osm : sel) { if (osm instanceof Node) { - nodes.add((Node) osm); + selectedNodes.add((Node) osm); } else if (osm instanceof Way) { ways.add((Way) osm); @@ -162,61 +173,62 @@ } - if (ways.size() == 1 && !ways.get(0).isClosed()) { + // nodes on selected ways + List onWay = new ArrayList<>(); + if (!ways.isEmpty()) { + List potentialCenter = new ArrayList<>(); + for (Node n : selectedNodes) { + if (ways.stream().anyMatch(w -> w.containsNode(n))) { + onWay.add(n); + } else { + potentialCenter.add(n); + } + } + if (potentialCenter.size() == 1) { + // center is given + center = potentialCenter.get(0).getEastNorth(); + if (onWay.size() == 1) { + radius = center.distance(onWay.get(0).getEastNorth()); + } + } else if (potentialCenter.size() > 1) { + throw new InvalidSelection(tr("Please select only one node as center.")); + } + + } + + final List nodes; + if (ways.isEmpty()) { + nodes = sortByAngle(selectedNodes); + fixNodes.addAll(nodes); + } else if (ways.size() == 1 && !ways.get(0).isClosed()) { // Case 1 Way w = ways.get(0); - if (SelfIntersectingWay.isSelfIntersecting(w)) { - throw new InvalidSelection(tr("Self-intersecting way")); - } - fixNodes.add(w.firstNode()); fixNodes.add(w.lastNode()); - fixNodes.addAll(nodes); - fixNodes.addAll(collectNodesWithExternReferrers(ways)); + fixNodes.addAll(onWay); // Temporary closed way used to reorder nodes Way closedWay = new Way(w); - closedWay.addNode(w.firstNode()); - nodes = collectNodesAnticlockwise(Collections.singletonList(closedWay)); - closedWay.setNodes(null); // see #19885 - } else if (!ways.isEmpty() && checkWaysArePolygon(ways)) { - // Case 2 - List inside = new ArrayList<>(); - List outside = new ArrayList<>(); - - for (Node n: nodes) { - boolean isInside = ways.stream().anyMatch(w -> w.getNodes().contains(n)); - if (isInside) - inside.add(n); - else - outside.add(n); - } - - if (outside.size() == 1 && inside.isEmpty()) { - center = outside.get(0).getEastNorth(); - } else if (outside.size() == 1 && inside.size() == 1) { - center = outside.get(0).getEastNorth(); - radius = center.distance(inside.get(0).getEastNorth()); - } else if (inside.size() == 2 && outside.isEmpty()) { - // 2 nodes inside, define diameter - EastNorth en0 = inside.get(0).getEastNorth(); - EastNorth en1 = inside.get(1).getEastNorth(); - center = new EastNorth((en0.east() + en1.east()) / 2, (en0.north() + en1.north()) / 2); + try { + closedWay.addNode(w.firstNode()); + nodes = collectNodesAnticlockwise(Collections.singletonList(closedWay)); + } finally { + closedWay.setNodes(null); // see #19885 + } + } else if (Multipolygon.joinWays(ways).size() == 1) { + // Case 2: + if (onWay.size() == 2) { + // 2 way nodes define diameter + EastNorth en0 = onWay.get(0).getEastNorth(); + EastNorth en1 = onWay.get(1).getEastNorth(); radius = en0.distance(en1) / 2; - } - - fixNodes.addAll(inside); - fixNodes.addAll(collectNodesWithExternReferrers(ways)); + if (center == null) { + center = en0.getCenter(en1); + } + } + fixNodes.addAll(onWay); nodes = collectNodesAnticlockwise(ways); - if (nodes.size() < 4) { - throw new InvalidSelection(tr("Not enough nodes in selected ways.")); - } - } else if (ways.isEmpty() && nodes.size() > 3) { - // Case 3 - fixNodes.addAll(nodes); - // No need to reorder nodes since all are fix } else { - if (ways.isEmpty() && nodes.size() <= 3) - throw new InvalidSelection(tr("Please select at least four nodes.")); throw new InvalidSelection(); } + fixNodes.addAll(collectNodesWithExternReferrers(ways)); // Check if one or more nodes are outside of download area @@ -224,9 +236,15 @@ throw new InvalidSelection(tr("One or more nodes involved in this action is outside of the downloaded area.")); + if (center == null) { - // Compute the center of nodes - center = Geometry.getCenter(nodes); + if (nodes.size() < 4) { + throw new InvalidSelection(tr("Not enough nodes to calculate center.")); + } + if (validateGeometry(nodes)) { + // Compute the center of nodes + center = Geometry.getCenter(nodes); + } if (center == null) { - throw new InvalidSelection(tr("Cannot determine center of selected nodes.")); + throw new InvalidSelection(tr("Cannot determine center of circle for this geometry.")); } } @@ -282,6 +300,38 @@ } if (cmds.isEmpty()) - throw new InvalidSelection(tr("nothing changed")); + return null; return new SequenceCommand(tr("Align Nodes in Circle"), cmds); + } + + private static List sortByAngle(final List nodes) { + EastNorth sum = new EastNorth(0, 0); + for (Node n : nodes) { + EastNorth en = n.getEastNorth(); + sum = sum.add(en.east(), en.north()); + } + final EastNorth simpleCenter = new EastNorth(sum.east()/nodes.size(), sum.north()/nodes.size()); + + SortedMap> orderedMap = new TreeMap<>(); + for (Node n : nodes) { + double angle = new PolarCoor(n.getEastNorth(), simpleCenter).angle; + orderedMap.computeIfAbsent(angle, k-> new ArrayList<>()).add(n); + } + return orderedMap.values().stream().flatMap(List::stream).collect(Collectors.toList()); + } + + private static boolean validateGeometry(List nodes) { + Way test = new Way(); + test.setNodes(nodes); + if (!test.isClosed()) { + test.addNode(test.firstNode()); + } + + try { + if (CrossingWays.isSelfCrossing(test)) + return false; + return !SelfIntersectingWay.isSelfIntersecting(test); + } finally { + test.setNodes(null); // see #19855 + } } @@ -304,7 +354,7 @@ Collection rings = Multipolygon.joinWays(ways); if (rings.size() != 1) - throw new InvalidSelection(); + throw new InvalidSelection(); // we should never get here List nodes = new ArrayList<>(rings.iterator().next().getNodes()); - if (nodes.get(0) != nodes.get(nodes.size()-1)) + if (nodes.get(0) != nodes.get(nodes.size() - 1)) throw new InvalidSelection(); if (Geometry.isClockwise(nodes)) @@ -324,24 +374,3 @@ updateEnabledStateOnModifiableSelection(selection); } - - /** - * 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 single polygon - */ - private static boolean checkWaysArePolygon(Collection 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()) { - if (way.isFirstLastNode(node)) continue; - if (ways.stream().filter(wayOther -> way != wayOther).anyMatch(wayOther -> node.getReferrers().contains(wayOther))) { - return false; - } - } - } - return true; - } - } Index: trunk/src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java (revision 17392) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java (revision 17393) @@ -27,4 +27,5 @@ import org.openstreetmap.josm.data.validation.util.ValUtil; import org.openstreetmap.josm.gui.progress.ProgressMonitor; +import org.openstreetmap.josm.tools.CheckParameterUtil; import org.openstreetmap.josm.tools.Logging; @@ -469,3 +470,16 @@ } + /** + * Check if the given way is self crossing + * @param way the way to check + * @return {@code true} if one or more segments of the way are crossing + * @see SelfIntersectingWay + * @since xxx + */ + public static boolean isSelfCrossing(Way way) { + CheckParameterUtil.ensureParameterNotNull(way, "way"); + SelfCrossing test = new SelfCrossing(); + test.visit(way); + return !test.getErrors().isEmpty(); + } }