From bdb02e60c2955099a267a4e9791ba3fae3bfa5f9 Mon Sep 17 00:00:00 2001 From: =?utf-8?q?Ilpo=20J=C3=A4rvinen?= Date: Sun, 18 Dec 2011 02:58:07 +0200 Subject: [PATCH 2/2] Make followline to handle wraps and stop at ambiguities When the toFollow way is closed and wraps, the followline does not correctly continue following because the prev and last indexes don't have abs(delta)==1, yet the nodes are still adjacent. I first came across very complicated fix using the existing code, but then decided to make the code logic followable to mortals too by removing the index complexity which would have gotten even more complicated if the wrap-arounds would have been considered. While recoding, also another issue (not a bug but more an undesirable feature) was easy to fix, namely, the followline previously picked just pseudo-randomly one of the possible extensions. If there are follow "candidate ways" diverging from the current node which share the path from prev, followline just seemingly picks one "randomly". Instead, make it to stop when first ambiguity is encountered forcing the user to point to which direction the way should be extended. There are two cases to handle: two separate ways diverging and a single way with a complex junction. The new approach simply find the neighbouring nodes and removes the shared one. The remaining neighbour is the node where followline should continue. If there is not exactly one remaining neighbour, there's an ambiguous junction or end of the way (in case no other neighbour exists). Two diverging candidate ways case handled by checking that all possible follow-ups point to the same node. I'd think that the getNeighbours method I added to the Way is useful in general so I put that there instead of as private into followline. As the actual extension is moved outside of the inner loop, the diff looks very messy due to indentation change. diff -b gives much more comprehendable view to the (much smaller) real changes. --- .../josm/actions/FollowLineAction.java | 62 +++++++++----------- src/org/openstreetmap/josm/data/osm/Way.java | 25 ++++++++ 2 files changed, 53 insertions(+), 34 deletions(-) diff --git a/src/org/openstreetmap/josm/actions/FollowLineAction.java b/src/org/openstreetmap/josm/actions/FollowLineAction.java index 58fe454..6fbffec 100644 --- a/src/org/openstreetmap/josm/actions/FollowLineAction.java +++ b/src/org/openstreetmap/josm/actions/FollowLineAction.java @@ -8,6 +8,7 @@ import java.awt.event.KeyEvent; import java.util.Collection; import java.util.Iterator; import java.util.List; +import java.util.Set; import org.openstreetmap.josm.Main; import org.openstreetmap.josm.actions.mapmode.DrawAction; @@ -82,6 +83,7 @@ public class FollowLineAction extends JosmAction { List referrers = last.getReferrers(); if (referrers.size() < 2) return; // There's nothing to follow + Node newPoint = null; Iterator i = referrers.iterator(); while (i.hasNext()) { OsmPrimitive referrer = i.next(); @@ -92,43 +94,35 @@ public class FollowLineAction extends JosmAction { if (toFollow.equals(follower)) { continue; } - List points = toFollow.getNodes(); - int indexOfLast = points.indexOf(last); - int indexOfPrev = points.indexOf(prev); - if ((indexOfLast == -1) || (indexOfPrev == -1)) { // Both points must belong to both lines + Set points = toFollow.getNeighbours(last); + if (!points.remove(prev) || (points.size() == 0)) continue; - } - if (Math.abs(indexOfPrev - indexOfLast) != 1) { // ...and be consecutive - continue; - } - Node newPoint = null; - if (indexOfPrev == (indexOfLast - 1)) { - if ((indexOfLast + 1) < points.size()) { - newPoint = points.get(indexOfLast + 1); - } - } else { - if ((indexOfLast - 1) >= 0) { - newPoint = points.get(indexOfLast - 1); - } - } - if (newPoint != null) { - Way newFollower = new Way(follower); - if (reversed) { - newFollower.addNode(0, newPoint); - } else { - newFollower.addNode(newPoint); - } - Main.main.undoRedo.add(new ChangeCommand(follower, newFollower)); - osmLayer.data.clearSelection(); - osmLayer.data.addSelected(newFollower); - osmLayer.data.addSelected(newPoint); - // "viewport following" mode for tracing long features - // from aerial imagery or GPS tracks. - if (Main.map.mapView.viewportFollowing) { - Main.map.mapView.smoothScrollTo(newPoint.getEastNorth()); - }; + if (points.size() > 1) // Ambiguous junction? return; + + Node newPointCandidate = points.iterator().next(); + + if ((newPoint != null) && (newPoint != newPointCandidate)) + return; // Ambiguous junction, force to select next + + newPoint = newPointCandidate; + } + if (newPoint != null) { + Way newFollower = new Way(follower); + if (reversed) { + newFollower.addNode(0, newPoint); + } else { + newFollower.addNode(newPoint); } + Main.main.undoRedo.add(new ChangeCommand(follower, newFollower)); + osmLayer.data.clearSelection(); + osmLayer.data.addSelected(newFollower); + osmLayer.data.addSelected(newPoint); + // "viewport following" mode for tracing long features + // from aerial imagery or GPS tracks. + if (Main.map.mapView.viewportFollowing) { + Main.map.mapView.smoothScrollTo(newPoint.getEastNorth()); + }; } } } diff --git a/src/org/openstreetmap/josm/data/osm/Way.java b/src/org/openstreetmap/josm/data/osm/Way.java index 8336bc4..137fbc4 100644 --- a/src/org/openstreetmap/josm/data/osm/Way.java +++ b/src/org/openstreetmap/josm/data/osm/Way.java @@ -7,6 +7,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Set; +import java.util.HashSet; import org.openstreetmap.josm.Main; import org.openstreetmap.josm.data.coor.LatLon; @@ -138,6 +139,30 @@ public final class Way extends OsmPrimitive implements IWay { return false; } + /** + * Return nodes adjacent to node + * + * @param node the node. May be null. + * @return Set of nodes adjacent to node + * @since 4666 + */ + public Set getNeighbours(Node node) { + HashSet neigh = new HashSet(); + + if (node == null) return neigh; + + Node[] nodes = this.nodes; + for (int i=0; i 0) + neigh.add(nodes[i-1]); + if (i < nodes.length-1) + neigh.add(nodes[i+1]); + } + } + return neigh; + } + public List> getNodePairs(boolean sort) { List> chunkSet = new ArrayList>(); if (isIncomplete()) return chunkSet; -- 1.7.2.5