Ticket #7164: 0002-Make-followline-to-handle-wraps-and-stop-at-ambiguit.2.patch

File 0002-Make-followline-to-handle-wraps-and-stop-at-ambiguit.2.patch, 7.5 KB (added by ij, 15 years ago)

v2 2/2

  • src/org/openstreetmap/josm/actions/FollowLineAction.java 

    From bdb02e60c2955099a267a4e9791ba3fae3bfa5f9 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@helsinki.fi>
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 b import java.awt.event.KeyEvent;  
    88import java.util.Collection;
    99import java.util.Iterator;
    1010import java.util.List;
     11import java.util.Set;
    1112
    1213import org.openstreetmap.josm.Main;
    1314import org.openstreetmap.josm.actions.mapmode.DrawAction;
    … … public class FollowLineAction extends JosmAction {  
    8283        List<OsmPrimitive> referrers = last.getReferrers();
    8384        if (referrers.size() < 2) return; // There's nothing to follow
    8485       
     86        Node newPoint = null;       
    8587        Iterator<OsmPrimitive> i = referrers.iterator();
    8688        while (i.hasNext()) {
    8789            OsmPrimitive referrer = i.next();
    … … public class FollowLineAction extends JosmAction {  
    9294            if (toFollow.equals(follower)) {
    9395                continue;
    9496            }
    95             List<Node> points = toFollow.getNodes();
    96             int indexOfLast = points.indexOf(last);
    97             int indexOfPrev = points.indexOf(prev);
    98             if ((indexOfLast == -1) || (indexOfPrev == -1)) { // Both points must belong to both lines
     97            Set<Node> points = toFollow.getNeighbours(last);
     98            if (!points.remove(prev) || (points.size() == 0))
    9999                continue;
    100             }
    101             if (Math.abs(indexOfPrev - indexOfLast) != 1) {  // ...and be consecutive
    102                 continue;
    103             }
    104             Node newPoint = null;
    105             if (indexOfPrev == (indexOfLast - 1)) {
    106                 if ((indexOfLast + 1) < points.size()) {
    107                     newPoint = points.get(indexOfLast + 1);
    108                 }
    109             } else {
    110                 if ((indexOfLast - 1) >= 0) {
    111                     newPoint = points.get(indexOfLast - 1);
    112                 }
    113             }
    114             if (newPoint != null) {
    115                 Way newFollower = new Way(follower);
    116                 if (reversed) {
    117                     newFollower.addNode(0, newPoint);
    118                 } else {
    119                     newFollower.addNode(newPoint);
    120                 }
    121                 Main.main.undoRedo.add(new ChangeCommand(follower, newFollower));
    122                 osmLayer.data.clearSelection();
    123                 osmLayer.data.addSelected(newFollower);
    124                 osmLayer.data.addSelected(newPoint);
    125                 // "viewport following" mode for tracing long features
    126                 // from aerial imagery or GPS tracks.
    127                 if (Main.map.mapView.viewportFollowing) {
    128                     Main.map.mapView.smoothScrollTo(newPoint.getEastNorth());
    129                 };
     100            if (points.size() > 1)    // Ambiguous junction?
    130101                return;
     102
     103            Node newPointCandidate = points.iterator().next();
     104
     105            if ((newPoint != null) && (newPoint != newPointCandidate))
     106                return;         // Ambiguous junction, force to select next
     107
     108            newPoint = newPointCandidate;
     109        }
     110        if (newPoint != null) {
     111            Way newFollower = new Way(follower);
     112            if (reversed) {
     113                newFollower.addNode(0, newPoint);
     114            } else {
     115                newFollower.addNode(newPoint);
    131116            }
     117            Main.main.undoRedo.add(new ChangeCommand(follower, newFollower));
     118            osmLayer.data.clearSelection();
     119            osmLayer.data.addSelected(newFollower);
     120            osmLayer.data.addSelected(newPoint);
     121            // "viewport following" mode for tracing long features
     122            // from aerial imagery or GPS tracks.
     123            if (Main.map.mapView.viewportFollowing) {
     124                Main.map.mapView.smoothScrollTo(newPoint.getEastNorth());
     125            };
    132126        }
    133127    }
    134128}

  • src/org/openstreetmap/josm/data/osm/Way.java 

    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 b import java.util.ArrayList;  
    77import java.util.Arrays;
    88import java.util.List;
    99import java.util.Set;
     10import java.util.HashSet;
    1011
    1112import org.openstreetmap.josm.Main;
    1213import org.openstreetmap.josm.data.coor.LatLon;
    … … public final class Way extends OsmPrimitive implements IWay {  
    138139        return false;
    139140    }
    140141
     142    /**
     143     * Return nodes adjacent to <code>node</code>
     144     *
     145     * @param node the node. May be null.
     146     * @return Set of nodes adjacent to <code>node</code>
     147     * @since 4666
     148     */
     149    public Set<Node> getNeighbours(Node node) {
     150        HashSet<Node> neigh = new HashSet<Node>();
     151
     152        if (node == null) return neigh;
     153
     154        Node[] nodes = this.nodes;
     155        for (int i=0; i<nodes.length; i++) {
     156            if (nodes[i].equals(node)) {
     157                if (i > 0)
     158                    neigh.add(nodes[i-1]);
     159                if (i < nodes.length-1)
     160                    neigh.add(nodes[i+1]);
     161            }
     162        }
     163        return neigh;
     164    }
     165
    141166    public List<Pair<Node,Node>> getNodePairs(boolean sort) {
    142167        List<Pair<Node,Node>> chunkSet = new ArrayList<Pair<Node,Node>>();
    143168        if (isIncomplete()) return chunkSet;