Refactor unfavoring of virtual edges (graphhopper#885)

Also needed for graphhopper/map-matching#70
stefanholder · Dec 10, 2016 · d6bde93 · d6bde93
1 parent 8e9d571
commit d6bde93
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 22 deletions.
diff --git a/CONTRIBUTORS.md b/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

diff --git a/core/src/main/java/com/graphhopper/routing/QueryGraph.java b/core/src/main/java/com/graphhopper/routing/QueryGraph.java
@@ -141,7 +141,7 @@ public double getEle(int nodeId) {
             return getElevation(nodeId);
         }
     };
-    private List<VirtualEdgeIteratorState> modifiedEdges = new ArrayList<VirtualEdgeIteratorState>(5);
+    private final List<VirtualEdgeIteratorState> unfavoredEdges = new ArrayList<>(5);
     private boolean useEdgeExplorerCache = false;
 
     public QueryGraph(Graph graph) {
@@ -407,7 +407,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 +451,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 +464,43 @@ 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);
+    }
+
+    public List<VirtualEdgeIteratorState> getUnfavoredVirtualEdges() {
+        return 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

diff --git a/core/src/main/java/com/graphhopper/routing/template/ViaRoutingTemplate.java b/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());
                 }
             }
 

diff --git a/core/src/test/java/com/graphhopper/routing/QueryGraphTest.java b/core/src/test/java/com/graphhopper/routing/QueryGraphTest.java
@@ -613,7 +613,7 @@ public void testEnforceHeadingByEdgeId() {
         queryGraph.lookup(Arrays.asList(qr));
 
         // enforce coming in north
-        queryGraph.enforceHeadingByEdgeId(2, 1, false);
+        queryGraph.unfavorVirtualEdgePair(2, 1);
         // test penalized south
         boolean expect = true;
         VirtualEdgeIteratorState incomingEdge = (VirtualEdgeIteratorState) queryGraph.getEdgeIteratorState(1, 2);