Index: /trunk/src/org/openstreetmap/josm/actions/AlignInLineAction.java
===================================================================
--- /trunk/src/org/openstreetmap/josm/actions/AlignInLineAction.java (revision 7849)
+++ /trunk/src/org/openstreetmap/josm/actions/AlignInLineAction.java (revision 7850)
@@ -12,4 +12,6 @@
import java.util.HashSet;
import java.util.List;
+import java.util.Map;
+import java.util.Set;
import javax.swing.JOptionPane;
@@ -27,12 +29,18 @@
/**
- * Aligns all selected nodes into a straight line (useful for
- * roads that should be straight, but have side roads and
+ * Aligns all selected nodes into a straight line (useful for roads that should be straight, but have side roads and
* therefore need multiple nodes)
*
- * Case 1: Only ways selected, align each ways taking care of intersection.
- * Case 2: Single node selected, align this node relative to the surrounding nodes.
- * Case 3: Single node and ways selected, align this node relative to the surrounding nodes only parts of selected ways.
- * Case 4: Only nodes selected, align these nodes respect to the line passing through the most distant nodes.
+ *
+ * Case 1: 1 or 2 ways selected and no nodes selected: align nodes of ways taking care of intersection.
+ * Case 2: Single node selected and no ways selected: align this node relative to all referrer ways (2 at most).
+ * Case 3: Single node and ways selected: align this node relative to selected ways.
+ * Case 4.1: Only nodes selected, part of a non-closed way: align these nodes on the line passing through the
+ * extremity nodes (most distant in the way sequence). See https://josm.openstreetmap.de/ticket/9605#comment:3
+ * Case 4.2: Only nodes selected, part of a closed way: align these nodes on the line passing through the most distant
+ * nodes.
+ * Case 4.3: Only nodes selected, part of multiple ways: align these nodes on the line passing through the most distant
+ * nodes.
+ *
*
* @author Matthew Newton
@@ -71,19 +79,17 @@
/**
- * Compute 2 anchor points to align a set of nodes.
- * If all nodes are part of a same way anchor points are choose farthest relative to this way,
- * else choose farthest nodes.
- * @param nodes Nodes to be aligned
- * @param resultOut Array of size >= 2
- */
- private void nodePairFurthestApart(List nodes, Node[] resultOut) {
- if(resultOut.length < 2)
- throw new IllegalArgumentException();
-
+ * Return 2 nodes making up the line along which provided nodes must be aligned.
+ *
+ * @param nodes Nodes to be aligned.
+ * @return A array of two nodes.
+ */
+ private Node[] nodePairFurthestApart(List nodes) {
Node nodea = null;
Node nodeb = null;
- // Intersection of all ways referred by each node
- HashSet waysRef = null;
+ // Detect if selected nodes are on the same way.
+
+ // Get ways passing though all selected nodes.
+ Set waysRef = null;
for(Node n: nodes) {
Collection ref = OsmPrimitive.getFilteredList(n.getReferrers(), Way.class);
@@ -93,36 +99,61 @@
waysRef.retainAll(ref);
}
- if(waysRef.size() == 1) {
- // All nodes are part of the same way. See #9605
- HashSet remainNodes = new HashSet<>(nodes);
- Way way = waysRef.iterator().next();
- for(Node n: way.getNodes()) {
- if(!remainNodes.contains(n)) continue;
- if(nodea == null) nodea = n;
- if(remainNodes.size() == 1) {
- nodeb = remainNodes.iterator().next();
- break;
+
+ // Nodes belongs to multiple ways, return most distant nodes.
+ if (waysRef.size() != 1)
+ return nodeFurthestAppart(nodes);
+
+ // All nodes are part of the same way. See #9605.
+ Way way = waysRef.iterator().next();
+
+ if (way.isClosed()) {
+ // Align these nodes on the line passing through the most distant nodes.
+ return nodeFurthestAppart(nodes);
+ }
+
+ // The way is open, align nodes on the line passing through the extremity nodes (most distant in the way
+ // sequence). See #9605#comment:3.
+ Set remainNodes = new HashSet<>(nodes);
+ for (Node n : way.getNodes()) {
+ if (!remainNodes.contains(n))
+ continue;
+ if (nodea == null)
+ nodea = n;
+ if (remainNodes.size() == 1) {
+ nodeb = remainNodes.iterator().next();
+ break;
+ }
+ remainNodes.remove(n);
+ }
+
+ return new Node[] { nodea, nodeb };
+ }
+
+ /**
+ * Return the two nodes the most distant from the provided list.
+ *
+ * @param nodes List of nodes to analyze.
+ * @return An array containing the two most distant nodes.
+ */
+ private Node[] nodeFurthestAppart(List nodes) {
+ Node node1 = null, node2 = null;
+ double minSqDistance = 0;
+ int nb;
+
+ nb = nodes.size();
+ for (int i = 0; i < nb - 1; i++) {
+ Node n = nodes.get(i);
+ for (int j = i + 1; j < nb; j++) {
+ Node m = nodes.get(j);
+ double sqDist = n.getEastNorth().distanceSq(m.getEastNorth());
+ if (sqDist > minSqDistance) {
+ node1 = n;
+ node2 = m;
+ minSqDistance = sqDist;
}
- remainNodes.remove(n);
- }
- } else {
- // Find from the selected nodes two that are the furthest apart.
- // Let's call them A and B.
- double distance = 0;
- 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;
- nodeb = m;
- distance = dist;
- }
- }
- }
- }
- resultOut[0] = nodea;
- resultOut[1] = nodeb;
+ }
+ }
+
+ return new Node[] { node1, node2 };
}
@@ -161,5 +192,5 @@
cmd = alignSingleNode(selectedNodes.get(0), lines);
}
- /// More than 3 nodes selected -> align those nodes
+ // More than 3 nodes and way(s) selected -> align selected nodes. Don't care of way(s).
else if(selectedNodes.size() >= 3) {
cmd = alignOnlyNodes(selectedNodes);
@@ -182,18 +213,13 @@
/**
- * Align nodes in case that only nodes are selected
+ * Align nodes in case 3 or more nodes are selected.
*
- * 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.
-
- * @param nodes Nodes to be aligned
- * @return Command that perform action
- * @throws InvalidSelection
+ * @param nodes Nodes to be aligned.
+ * @return Command that perform action.
+ * @throws InvalidSelection If the nodes have same coordinates.
*/
private Command alignOnlyNodes(List nodes) throws InvalidSelection {
- Node[] anchors = new Node[2]; // oh, java I love you so much..
- // use the nodes furthest apart as anchors
- nodePairFurthestApart(nodes, anchors);
+ // Choose nodes used as anchor points for projection.
+ Node[] anchors = nodePairFurthestApart(nodes);
Collection cmds = new ArrayList<>(nodes.size());
Line line = new Line(anchors[0], anchors[1]);
@@ -212,6 +238,6 @@
private Command alignMultiWay(Collection ways) throws InvalidSelection {
// Collect all nodes and compute line equation
- HashSet nodes = new HashSet<>();
- HashMap lines = new HashMap<>();
+ Set nodes = new HashSet<>();
+ Map lines = new HashMap<>();
for(Way w: ways) {
if(w.firstNode() == w.lastNode())
@@ -250,6 +276,6 @@
*/
private List getInvolvedLines(Node node, List refWays) throws InvalidSelection {
- ArrayList lines = new ArrayList<>();
- ArrayList neighbors = new ArrayList<>();
+ List lines = new ArrayList<>();
+ List neighbors = new ArrayList<>();
for(Way way: refWays) {
List nodes = way.getNodes();
Index: /trunk/test/unit/org/openstreetmap/josm/actions/AlignInLineActionTest.java
===================================================================
--- /trunk/test/unit/org/openstreetmap/josm/actions/AlignInLineActionTest.java (revision 7850)
+++ /trunk/test/unit/org/openstreetmap/josm/actions/AlignInLineActionTest.java (revision 7850)
@@ -0,0 +1,210 @@
+// License: GPL. For details, see LICENSE file.
+package org.openstreetmap.josm.actions;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.openstreetmap.josm.JOSMFixture;
+import org.openstreetmap.josm.Main;
+import org.openstreetmap.josm.data.coor.EastNorth;
+import org.openstreetmap.josm.data.coor.LatLon;
+import org.openstreetmap.josm.data.osm.DataSet;
+import org.openstreetmap.josm.data.osm.Node;
+import org.openstreetmap.josm.data.osm.Way;
+import org.openstreetmap.josm.gui.layer.OsmDataLayer;
+import org.openstreetmap.josm.io.OnlineResource;
+
+/**
+ * Unit tests for class AlignInLineAction.
+ */
+public final class AlignInLineActionTest {
+
+ /** Class under test. */
+ private static AlignInLineAction action;
+
+ /**
+ * Setup test.
+ */
+ @BeforeClass
+ public static void setUp() {
+ JOSMFixture.createUnitTestFixture().init(true);
+
+ // Enable "Align in line" feature.
+ action = Main.main.menu.alignInLine;
+ action.setEnabled(true);
+
+ Main.setOffline(OnlineResource.ALL);
+ }
+
+ /**
+ * Test case: only nodes selected, part of an open way: align these nodes on the line passing through the extremity
+ * nodes (the most distant in the way sequence, not the most euclidean-distant). See
+ * https://josm.openstreetmap.de/ticket/9605#comment:3. Note that in this test, after alignment, way is overlapping
+ * itself.
+ */
+ @Test
+ public void nodesOpenWay() {
+ DataSet dataSet;
+ OsmDataLayer layer;
+ Node point1, point2, point3;
+
+ dataSet = new DataSet();
+ layer = new OsmDataLayer(dataSet, OsmDataLayer.createNewName(), null);
+
+ // Create test points, lower left is (0,0).
+ //
+ // 1 - - -
+ // - 3 - 2
+ // - - - -
+ point1 = new Node(new EastNorth(0, 2));
+ point2 = new Node(new EastNorth(3, 1));
+ point3 = new Node(new EastNorth(1, 1));
+
+ try {
+ Main.main.addLayer(layer);
+
+ // Create an open way.
+ createWay(dataSet, point1, point2, point3);
+
+ // Select nodes to align.
+ dataSet.addSelected(point1, point2, point3);
+
+ action.actionPerformed(null);
+ } finally {
+ // Ensure we clean the place before leaving, even if test fails.
+ Main.map.mapView.removeLayer(layer);
+ }
+
+ // Points 1 and 3 are the extremities and must not have moved. Only point 2 must have moved.
+ assertCoordEq(point1, 0, 2);
+ assertCoordEq(point2, 2, 0);
+ assertCoordEq(point3, 1, 1);
+ }
+
+ /**
+ * Test case: only nodes selected, part of a closed way: align these nodes on the line passing through the most
+ * distant nodes.
+ */
+ @Test
+ public void nodesClosedWay() {
+ DataSet dataSet;
+ OsmDataLayer layer;
+ Node point1, point2, point3, point4;
+
+ dataSet = new DataSet();
+ layer = new OsmDataLayer(dataSet, OsmDataLayer.createNewName(), null);
+
+ // Create test points, lower left is (0,0).
+ //
+ // 4 - 3
+ // - - -
+ // 1 - 2
+ point1 = new Node(new EastNorth(0, 0));
+ point2 = new Node(new EastNorth(2, 0));
+ point3 = new Node(new EastNorth(2, 2));
+ point4 = new Node(new EastNorth(0, 2));
+
+ try {
+ Main.main.addLayer(layer);
+
+ // Create a closed way.
+ createWay(dataSet, point1, point2, point3, point4, point1);
+ // Select nodes to align (point1 must be in the second position to exhibit the bug).
+ dataSet.addSelected(point4, point1, point2);
+
+ action.actionPerformed(null);
+ } finally {
+ // Ensure we clean the place before leaving, even if test fails.
+ Main.map.mapView.removeLayer(layer);
+ }
+
+ // Only point 1 must have moved.
+ assertCoordEq(point1, 1, 1);
+ assertCoordEq(point2, 2, 0);
+ assertCoordEq(point3, 2, 2);
+ assertCoordEq(point4, 0, 2);
+ }
+
+ /**
+ * Test case: only nodes selected, part of multiple ways: align these nodes on the line passing through the most
+ * distant nodes.
+ */
+ @Test
+ public void nodesOpenWays() {
+ DataSet dataSet;
+ OsmDataLayer layer;
+ Node point1, point2, point3, point4;
+
+ dataSet = new DataSet();
+ layer = new OsmDataLayer(dataSet, OsmDataLayer.createNewName(), null);
+
+ // Create test points, lower left is (0,0).
+ //
+ // 1 - -
+ // 3 - 2
+ // - - 4
+ point1 = new Node(new EastNorth(0, 2));
+ point2 = new Node(new EastNorth(2, 1));
+ point3 = new Node(new EastNorth(0, 1));
+ point4 = new Node(new EastNorth(2, 0));
+
+ try {
+ Main.main.addLayer(layer);
+
+ // Create 2 ways.
+ createWay(dataSet, point1, point2);
+ createWay(dataSet, point3, point4);
+
+ // Select nodes to align.
+ dataSet.addSelected(point1, point2, point3, point4);
+
+ // Points must align between points 1 and 4.
+ action.actionPerformed(null);
+ } finally {
+ // Ensure we clean the place before leaving, even if test fails.
+ Main.map.mapView.removeLayer(layer);
+ }
+
+ assertCoordEq(point1, 0, 2);
+ assertCoordEq(point2, 1.5, 0.5);
+ assertCoordEq(point3, 0.5, 1.5);
+ assertCoordEq(point4, 2, 0);
+ }
+
+ /**
+ * Create a way made of the provided nodes and select nodes.
+ *
+ * @param dataSet Dataset in which adding nodes.
+ * @param nodes List of nodes to add to dataset.
+ */
+ private void createWay(DataSet dataSet, Node... nodes) {
+ Way way;
+
+ way = new Way();
+ dataSet.addPrimitive(way);
+
+ for (Node node : nodes) {
+ // Add primitive to dataset only if not already included.
+ if (dataSet.getPrimitiveById(node) == null)
+ dataSet.addPrimitive(node);
+
+ way.addNode(node);
+ }
+ }
+
+ /**
+ * Assert that the provided node has the specified coordinates. If not fail the test.
+ *
+ * @param node Node to test.
+ * @param x X coordinate.
+ * @param y Y coordinate.
+ */
+ private void assertCoordEq(Node node, double x, double y) {
+ EastNorth coordinate;
+
+ coordinate = node.getEastNorth();
+ assertEquals("Wrong x coordinate.", x, coordinate.getX(), LatLon.MAX_SERVER_PRECISION);
+ assertEquals("Wrong y coordinate.", y, coordinate.getY(), LatLon.MAX_SERVER_PRECISION);
+ }
+}