Index: src/org/openstreetmap/josm/plugins/utilsplugin2/curves/CircleArcMaker.java =================================================================== --- src/org/openstreetmap/josm/plugins/utilsplugin2/curves/CircleArcMaker.java (revision 35437) +++ src/org/openstreetmap/josm/plugins/utilsplugin2/curves/CircleArcMaker.java (working copy) @@ -1,6 +1,8 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.plugins.utilsplugin2.curves; +import static org.openstreetmap.josm.tools.I18n.tr; + import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -10,6 +12,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; import org.openstreetmap.josm.command.AddCommand; import org.openstreetmap.josm.command.ChangeCommand; @@ -27,6 +30,7 @@ import org.openstreetmap.josm.data.projection.ProjectionRegistry; import org.openstreetmap.josm.gui.MainApplication; import org.openstreetmap.josm.tools.Geometry; +import org.openstreetmap.josm.tools.JosmRuntimeException; /** * Create a circle arc @@ -107,6 +111,8 @@ LatLon ll2 = ProjectionRegistry.getProjection().eastNorth2latlon(center); double radiusInMeters = ll1.greatCircleDistance(ll2); + if (radiusInMeters < 0.01) + throw new JosmRuntimeException(tr("Radius too small")); int numberOfNodesInCircle = (int) Math.ceil(6.0 * Math.pow(radiusInMeters, 0.5)); // an odd number of nodes makes the distribution uneven @@ -126,90 +132,159 @@ final List nodes = new ArrayList<>(w.getNodes()); if (!selectedWays.isEmpty()) { + if (w.isClosed()) { + // see #19188 + nodes.clear(); + nodes.addAll(findShortestPart(w, anchorNodes)); + } // Fix #7341. sort nodes in ways nodes order List consideredNodes = Arrays.asList(n1, n2, n3); - Collections.sort(consideredNodes, (o1, o2) -> nodes.indexOf(o1) - nodes.indexOf(o2)); + consideredNodes.sort((o1, o2) -> nodes.indexOf(o1) - nodes.indexOf(o2)); n1 = consideredNodes.get(0); + n2 = consideredNodes.get(1); n3 = consideredNodes.get(2); + anchorNodes = Arrays.asList(n1, n2, n3); } - Set fixNodes = new HashSet<>(anchorNodes); - if (!selectedWays.isEmpty()) { - nodes.stream().filter( - n -> n.isTagged() || n.getParentWays().size() > 1 || n.referrers(Relation.class).count() > 0) - .forEach(fixNodes::add); + + List cwTest = new ArrayList<>(Arrays.asList(n1, n2, n3)); + if (cwTest.get(0) != cwTest.get(cwTest.size() - 1)) { + cwTest.add(cwTest.get(0)); } + boolean clockWise = Geometry.isClockwise(cwTest); + boolean needsUndo = false; if (!cmds.isEmpty()) { UndoRedoHandler.getInstance().add(new SequenceCommand("add nodes", cmds)); needsUndo = true; } + Set targetWays = new HashSet<>(); + anchorNodes.forEach(n -> targetWays.addAll(n.getParentWays())); int pos1 = nodes.indexOf(n1); int pos3 = nodes.indexOf(n3); - List toModify = new ArrayList<>(nodes.subList(pos1, pos3 + 1)); - cmds.addAll(worker(toModify, fixNodes, center, radius, maxAngle)); - if (toModify.size() > pos3 + 1 - pos1) { - List changed = new ArrayList<>(); - changed.addAll(nodes.subList(0, pos1)); - changed.addAll(toModify); - changed.addAll(nodes.subList(pos3 + 1, nodes.size())); - Way wNew = new Way(w); - wNew.setNodes(changed); - cmds.add(new ChangeCommand(w, wNew)); + Set fixNodes = new HashSet<>(anchorNodes); + if (!selectedWays.isEmpty()) { + for (int i = pos1 + 1; i < pos3; i++) { + Node n = nodes.get(i); + if (n.isTagged() || n.getParentWays().size() > 1 || n.referrers(Relation.class).count() > 0) + fixNodes.add(n); + } } - if (needsUndo) { - // make sure we don't add the new nodes twice - UndoRedoHandler.getInstance().undo(1); + + List orig = nodes.subList(pos1, pos3 + 1); + List arcNodes = new ArrayList<>(orig); + try { + cmds.addAll(worker(arcNodes, fixNodes, center, radius, maxAngle, clockWise)); + if (!arcNodes.equals(orig)) { + fuseArc(ds, arcNodes, targetWays, cmds); + } + } finally { + if (needsUndo) { + // make sure we don't add the new nodes twice + UndoRedoHandler.getInstance().undo(1); + } } + if (cmds.isEmpty()) { + throw new JosmRuntimeException(tr("Nothing to do")); + } return cmds; } + /** + * Try to find out which nodes should be moved when a closed way is modified. The positions of the anchor + * nodes might not give the right order. Rotate the nodes so that each of the selected nodes is first + * and check which variant produces the shortest part of the way. + * @param w the closed way to modify + * @param anchorNodes the (selected) anchor nodes + * @return the way nodes, possibly rotated + * @throws JosmRuntimeException if no usable rotation was found + */ + private static List findShortestPart(Way w, List anchorNodes) { + int bestRotate = 0; + double shortest = Double.MAX_VALUE; + final double wayLength = w.getLength(); + for (int i = 0; i < w.getNodesCount() - 1; i++) { + List nodes = rotate(w, i); + List positions = anchorNodes.stream().map(nodes::indexOf).sorted().collect(Collectors.toList()); + double lenghth = getLength(nodes, positions.get(0), positions.get(2)); + if (lenghth < shortest) { + bestRotate = i; + shortest = lenghth; + } + } + if (shortest >= wayLength / 2) + throw new JosmRuntimeException(tr("Don't know which part of closed way should be changed")); + return rotate(w, bestRotate); + } + + private static List rotate(Way w, int distance) { + List nodes = new ArrayList<>(w.getNodes()); + nodes.remove(nodes.size() - 1); // remove closing node + Collections.rotate(nodes, distance); + nodes.add(nodes.get(0)); + return nodes; + } + + private static double getLength(List nodes, int pos1, int pos3) { + Way tmp = new Way(); + tmp.setNodes(nodes.subList(pos1, pos3+1)); + return tmp.getLength(); + } + // code partly taken from AlignInCircleAction - private static List worker(List nodes, Set fixNodes, EastNorth center, double radius, double maxAngle) { + private static List worker(List nodes, Set origFixNodes, EastNorth center, double radius, + double maxAngle, boolean clockWise) { List 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(); + // Nodes that are not "fix" will be adjusted making regular arcs. - List cwTest = new ArrayList<>(nodes); - if (cwTest.get(0) != cwTest.get(cwTest.size() - 1)) { - cwTest.add(cwTest.get(0)); - } - boolean clockWise = Geometry.isClockwise(cwTest); + HashSet fixNodes = new HashSet<>(origFixNodes); double maxStep = Math.PI * 2 / (360.0 / maxAngle); - // Search first fixed node - int startPosition; - for (startPosition = 0; startPosition < nodeCount; startPosition++) { - if (fixNodes.contains(nodes.get(startPosition))) - break; - } - int i = startPosition; // Start position for current arc + double sumAbsDelta = 0; + int i = 0; // Start position for current arc int j; // End position for current arc - while (i < nodeCount) { - for (j = i + 1; j < nodeCount; j++) { - if (fixNodes.contains(nodes.get(j))) + while (i < nodes.size()) { + for (j = i + 1; j < nodes.size(); j++) { + if (fixNodes.contains(nodes.get(j))) { break; + } } Node first = nodes.get(i); + PolarCoor pcFirst = new PolarCoor(radius, PolarCoor.computeAngle(first.getEastNorth(), center), center); - PolarCoor pcFirst = new PolarCoor(radius, PolarCoor.computeAngle(first.getEastNorth(), center), center); - addMoveCommandIfNeeded(first, pcFirst, cmds); - if (j < nodeCount) { - double delta; - PolarCoor pcLast = new PolarCoor(nodes.get(j).getEastNorth(), center); - delta = pcLast.angle - pcFirst.angle; + if (j < nodes.size()) { + Node last = nodes.get(j); + PolarCoor pcLast = new PolarCoor(last.getEastNorth(), center); + double delta = pcLast.angle - pcFirst.angle; + if ((!clockWise && delta < 0 || clockWise && delta > 0) + && Math.signum(pcFirst.angle) == Math.signum(pcLast.angle)) { + // cannot project node onto circle arc, ignore that it is fixed + if (!last.isSelected() && fixNodes.remove(last)) { + continue; + } else { + throw new JosmRuntimeException(tr("Too many fixed nodes")); + } + } if (!clockWise && delta < 0) { delta += 2 * Math.PI; } else if (clockWise && delta > 0) { delta -= 2 * Math.PI; } + sumAbsDelta += Math.abs(delta); + if (sumAbsDelta > 2 * Math.PI) { + // something went wrong, we would add more than a full circle + throw new JosmRuntimeException(tr("Would produce a loop")); + } + // do we have enough nodes to produce a nice circle? int numToAdd = Math.max((int) Math.ceil(Math.abs(delta / maxStep)), j - i) - (j-i); double step = delta / (numToAdd + j - i); - for (int k = i + 1; k < j; k++) { + + // TODO: better mix new nodes and old nodes to reduce move distance + for (int k = i; k < j; k++) { PolarCoor p = new PolarCoor(radius, pcFirst.angle + (k - i) * step, center); addMoveCommandIfNeeded(nodes.get(k), p, cmds); } @@ -221,7 +296,6 @@ cmds.add(new AddCommand(nodes.get(0).getDataSet(), nNew)); } j += numToAdd; - nodeCount += numToAdd; } i = j; // Update start point for next iteration } @@ -237,4 +311,54 @@ } } + private static void fuseArc(DataSet ds, List arcNodes, Set targetWays, Collection cmds) { + // replace each segment of the target ways with the corresponding nodes of the new arc + for (Way originalTw : targetWays) { + Way tw = new Way(originalTw); + boolean twWasChanged = false; + for (int i = 0; i < arcNodes.size(); i++) { + Node arcNode1 = arcNodes.get(i); + // we don't want to match nodes which where added by worker + if (arcNode1.getDataSet() != ds || !arcNode1.getParentWays().contains(originalTw)) + continue; + + boolean changed = false; + for (int j = i + 1; j < arcNodes.size() && !changed; j++) { + Node arcNode2 = arcNodes.get(j); + if (arcNode2.getDataSet() != ds || !arcNode2.getParentWays().contains(originalTw)) + continue; + changed = tryAddArc(tw, i, j, arcNodes); + twWasChanged |= changed; + } + } + if (twWasChanged) { + cmds.add(new ChangeCommand(ds, originalTw, tw)); + } + } + } + + private static boolean tryAddArc(Way tw, int i, int j, List arcNodes) { + int pos1 = tw.getNodes().indexOf(arcNodes.get(i)); + int pos2 = tw.getNodes().indexOf(arcNodes.get(j)); + if (tw.isClosed()) { + if (pos1 - pos2 > 1 && pos2 == 0) { + pos2 = tw.getNodesCount() - 1; + } else if (pos2 - pos1 > 1 && pos1 == 0) { + pos1 = tw.getNodesCount() - 1; + } + } + if (pos2 + 1 == pos1) { + for (int k = i + 1; k < j; k++) { + tw.addNode(pos1, arcNodes.get(k)); + } + return true; + } else if (pos2 - 1 == pos1) { + for (int k = j - 1; k > i; k--) { + tw.addNode(pos2, arcNodes.get(k)); + } + return true; + } + return false; + } + } Index: src/org/openstreetmap/josm/plugins/utilsplugin2/curves/CurveAction.java =================================================================== --- src/org/openstreetmap/josm/plugins/utilsplugin2/curves/CurveAction.java (revision 35437) +++ src/org/openstreetmap/josm/plugins/utilsplugin2/curves/CurveAction.java (working copy) @@ -20,6 +20,7 @@ import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.gui.Notification; +import org.openstreetmap.josm.tools.JosmRuntimeException; import org.openstreetmap.josm.tools.Shortcut; // TODO: investigate splines @@ -46,12 +47,20 @@ List selectedNodes = new ArrayList<>(getLayerManager().getEditDataSet().getSelectedNodes()); List selectedWays = new ArrayList<>(getLayerManager().getEditDataSet().getSelectedWays()); - Collection cmds = CircleArcMaker.doCircleArc(selectedNodes, selectedWays); - if (cmds == null || cmds.isEmpty()) { - new Notification(tr("Could not use selection to create a curve")).setIcon(JOptionPane.WARNING_MESSAGE).show(); - } else { - UndoRedoHandler.getInstance().add(new SequenceCommand("Create a curve", cmds)); + String msg = null; + try { + Collection cmds = CircleArcMaker.doCircleArc(selectedNodes, selectedWays); + if (cmds == null || cmds.isEmpty()) { + msg = tr("Could not use selection to create a curve"); + } else { + UndoRedoHandler.getInstance().add(new SequenceCommand("Create a curve", cmds)); + } + } catch (JosmRuntimeException ex) { + msg = tr("Could not use selection to create a curve: {0}", ex.getMessage()); } + if (msg != null) { + new Notification(msg).setIcon(JOptionPane.WARNING_MESSAGE).show(); + } } @Override