Refactor unfavoring of virtual edges (graphhopper#885)

Also needed for graphhopper/map-matching#70

CONTRIBUTORS.md

@@ -42,7 +42,8 @@ Most of the contributors are mentioned at Github as [Members](https://github.com
  * rajanski, script to do routing via PostGIS
  * rodneyodonnell, improved dead end removal and fords
  * rodo, more descriptions
- * seeebiii, motorcycle improvements 
+ * seeebiii, motorcycle improvements
+ * stefanholder, Stefan Holder, BMW AG, refactored unfavoring of virtual edges #885
  * Svantulden, improved documentation and nearest API
  * thehereward, code cleanups like #620
  * vvikas, ideas for many to many improvements and #616

core/src/main/java/com/graphhopper/routing/QueryGraph.java

@@ -141,7 +141,10 @@ public double getEle(int nodeId) {
             return getElevation(nodeId);
         }
     };
-    private List<VirtualEdgeIteratorState> modifiedEdges = new ArrayList<VirtualEdgeIteratorState>(5);
+
+    // Use LinkedHashSet for predictable iteration order.
+    private final Set<VirtualEdgeIteratorState> unfavoredEdges = new LinkedHashSet<>(5);
+
     private boolean useEdgeExplorerCache = false;
 
     public QueryGraph(Graph graph) {
@@ -407,7 +410,7 @@ private void createEdges(int origTraversalKey, int origRevTraversalKey,
 
     /**
      * Set those edges at the virtual node (nodeId) to 'unfavored' that require at least a turn of
-     * 100° from favoredHeading
+     * 100° from favoredHeading.
      * <p>
      *
      * @param nodeId         VirtualNode at which edges get unfavored
@@ -451,11 +454,11 @@ public boolean enforceHeading(int nodeId, double favoredHeading, boolean incomin
             if (Math.abs(delta) > 1.74) // penalize if a turn of more than 100°
             {
                 edge.setUnfavored(true);
-                modifiedEdges.add(edge);
+                unfavoredEdges.add(edge);
                 //also apply to opposite edge for reverse routing
                 VirtualEdgeIteratorState reverseEdge = virtualEdges.get(virtNodeIDintern * 4 + getPosOfReverseEdge(edgePos));
                 reverseEdge.setUnfavored(true);
-                modifiedEdges.add(reverseEdge);
+                unfavoredEdges.add(reverseEdge);
                 enforcementOccurred = true;
             }
 
@@ -464,35 +467,49 @@ public boolean enforceHeading(int nodeId, double favoredHeading, boolean incomin
     }
 
     /**
-     * Set one specific edge at the virtual node with nodeId to 'unfavored' to enforce routing along
-     * other edges
+     * Sets the virtual edge with virtualEdgeId and its reverse edge to 'unfavored', which
+     * effectively penalizes both virtual edges towards an adjacent node of virtualNodeId.
+     * This makes it more likely (but does not guarantee) that the router chooses a route towards
+     * the other adjacent node of virtualNodeId.
      * <p>
      *
-     * @param nodeId   VirtualNode at which edges get unfavored
-     * @param edgeId   edge to become unfavored
-     * @param incoming if true, incoming edge is unfavored, else outgoing edge
-     * @return boolean indicating if enforcement took place
+     * @param virtualNodeId  virtual node at which edges get unfavored
+     * @param virtualEdgeId  this edge and the reverse virtual edge become unfavored
      */
-    public boolean enforceHeadingByEdgeId(int nodeId, int edgeId, boolean incoming) {
-        if (!isVirtualNode(nodeId))
-            return false;
+    public void unfavorVirtualEdgePair(int virtualNodeId, int virtualEdgeId) {
+        if (!isVirtualNode(virtualNodeId)) {
+            throw new IllegalArgumentException("Node id " + virtualNodeId
+                    + " must be a virtual node.");
+        }
 
-        VirtualEdgeIteratorState incomingEdge = (VirtualEdgeIteratorState) getEdgeIteratorState(edgeId, nodeId);
-        VirtualEdgeIteratorState reverseEdge = (VirtualEdgeIteratorState) getEdgeIteratorState(edgeId, incomingEdge.getBaseNode());
+        VirtualEdgeIteratorState incomingEdge =
+                (VirtualEdgeIteratorState) getEdgeIteratorState(virtualEdgeId, virtualNodeId);
+        VirtualEdgeIteratorState reverseEdge = (VirtualEdgeIteratorState) getEdgeIteratorState(
+                virtualEdgeId, incomingEdge.getBaseNode());
         incomingEdge.setUnfavored(true);
-        modifiedEdges.add(incomingEdge);
+        unfavoredEdges.add(incomingEdge);
         reverseEdge.setUnfavored(true);
-        modifiedEdges.add(reverseEdge);
-        return true;
+        unfavoredEdges.add(reverseEdge);
+    }
+
+    /**
+     * Returns all virtual edges that have been unfavored via
+     * {@link #enforceHeading(int, double, boolean)} or {@link #unfavorVirtualEdgePair(int, int)}.
+     */
+    public Set<EdgeIteratorState> getUnfavoredVirtualEdges() {
+        // Need to create a new set to convert Set<VirtualEdgeIteratorState> to
+        // Set<EdgeIteratorState>.
+        return new LinkedHashSet<EdgeIteratorState>(unfavoredEdges);
     }
 
     /**
      * Removes the 'unfavored' status of all virtual edges.
      */
     public void clearUnfavoredStatus() {
-        for (VirtualEdgeIteratorState edge : modifiedEdges) {
+        for (VirtualEdgeIteratorState edge : unfavoredEdges) {
             edge.setUnfavored(false);
         }
+        unfavoredEdges.clear();
     }
 
     @Override

core/src/main/java/com/graphhopper/routing/template/ViaRoutingTemplate.java

@@ -98,7 +98,7 @@ public List<Path> calcPaths(QueryGraph queryGraph, RoutingAlgorithmFactory algoF
                 Path prevRoute = pathList.get(placeIndex - 2);
                 if (prevRoute.getEdgeCount() > 0) {
                     EdgeIteratorState incomingVirtualEdge = prevRoute.getFinalEdge();
-                    queryGraph.enforceHeadingByEdgeId(fromQResult.getClosestNode(), incomingVirtualEdge.getEdge(), false);
+                    queryGraph.unfavorVirtualEdgePair(fromQResult.getClosestNode(), incomingVirtualEdge.getEdge());
                 }
             }
 

core/src/test/java/com/graphhopper/routing/QueryGraphTest.java

@@ -30,6 +30,8 @@
 import org.junit.Test;
 
 import java.util.Arrays;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
 
 import static com.graphhopper.storage.index.QueryResult.Position.*;
 import static org.junit.Assert.*;
@@ -602,7 +604,7 @@ public void testEnforceHeading() {
     }
 
     @Test
-    public void testEnforceHeadingByEdgeIdUnfavorIncoming() {
+    public void testunfavorVirtualEdgePair() {
 
         initHorseshoeGraph(g);
         EdgeIteratorState edge = GHUtility.getEdge(g, 0, 1);
@@ -613,47 +615,21 @@ public void testEnforceHeadingByEdgeIdUnfavorIncoming() {
         queryGraph.lookup(Arrays.asList(qr));
 
         // enforce coming in north
-        queryGraph.enforceHeadingByEdgeId(2, 1, true);
+        queryGraph.unfavorVirtualEdgePair(2, 1);
         // test penalized south
         VirtualEdgeIteratorState incomingEdge = (VirtualEdgeIteratorState) queryGraph.getEdgeIteratorState(1, 2);
         VirtualEdgeIteratorState incomingEdgeReverse = (VirtualEdgeIteratorState) queryGraph.getEdgeIteratorState(1, incomingEdge.getBaseNode());
-        boolean expect = true;  // expect incoming edge to be avoided
+        boolean expect = true;  // expect incoming and reverse incoming edge to be avoided
         assertEquals(expect, incomingEdge.getBool(EdgeIteratorState.K_UNFAVORED_EDGE, !expect));
-        expect = false;         // expect reverse incoming edge not to be avoided
         assertEquals(expect, incomingEdgeReverse.getBool(EdgeIteratorState.K_UNFAVORED_EDGE, !expect));
+        assertEquals(new LinkedHashSet<>(Arrays.asList(incomingEdge, incomingEdgeReverse)),
+                queryGraph.getUnfavoredVirtualEdges());
 
         queryGraph.clearUnfavoredStatus();
         expect = false; // expect incoming and reverse incoming edge not to be avoided
         assertEquals(expect, incomingEdge.getBool(EdgeIteratorState.K_UNFAVORED_EDGE, !expect));
         assertEquals(expect, incomingEdgeReverse.getBool(EdgeIteratorState.K_UNFAVORED_EDGE, !expect));
-    }
-
-    @Test
-    public void testEnforceHeadingByEdgeIdUnfavorOutgoing() {
-
-        initHorseshoeGraph(g);
-        EdgeIteratorState edge = GHUtility.getEdge(g, 0, 1);
-
-        // query result on first vertical part of way (upward)
-        QueryResult qr = fakeEdgeQueryResult(edge, 1.5, 0, 0);
-        QueryGraph queryGraph = new QueryGraph(g);
-        queryGraph.lookup(Arrays.asList(qr));
-
-        // enforce coming in north
-        queryGraph.enforceHeadingByEdgeId(2, 1, false);
-        // test penalized south
-        VirtualEdgeIteratorState incomingEdge = (VirtualEdgeIteratorState) queryGraph.getEdgeIteratorState(1, 2);
-        VirtualEdgeIteratorState incomingEdgeReverse = (VirtualEdgeIteratorState) queryGraph.getEdgeIteratorState(1, incomingEdge.getBaseNode());
-        boolean expect = false;  // expect incoming edge not to be avoided
-        assertEquals(expect, incomingEdge.getBool(EdgeIteratorState.K_UNFAVORED_EDGE, !expect));
-        expect = true;           // expect reverse incoming edge to be avoided
-        assertEquals(expect, incomingEdgeReverse.getBool(EdgeIteratorState.K_UNFAVORED_EDGE, !expect));
-
-        queryGraph.clearUnfavoredStatus();
-        expect = false;          // expect incoming and reverse incoming edge not to be avoided
-        assertEquals(expect, incomingEdge.getBool(EdgeIteratorState.K_UNFAVORED_EDGE, !expect));
-
-        assertEquals(expect, incomingEdgeReverse.getBool(EdgeIteratorState.K_UNFAVORED_EDGE, !expect));
+        assertEquals(new LinkedHashSet<>(), queryGraph.getUnfavoredVirtualEdges());
     }
 
     @Test