diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 66462ef7f6a..317909fbd61 100644 --- 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 index 99f5337a21e..8c98e8e6244 100644 --- a/core/src/main/java/com/graphhopper/routing/QueryGraph.java +++ b/core/src/main/java/com/graphhopper/routing/QueryGraph.java @@ -141,7 +141,10 @@ public double getEle(int nodeId) { return getElevation(nodeId); } }; - private List modifiedEdges = new ArrayList(5); + + // Use LinkedHashSet for predictable iteration order. + private final Set 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. *

* * @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. *

* - * @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 getUnfavoredVirtualEdges() { + // Need to create a new set to convert Set to + // Set. + return new LinkedHashSet(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 index 5cb6d25d954..cfa22fb9372 100644 --- 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 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 index bbbeb92a401..4e6164fdf7a 100644 --- a/core/src/test/java/com/graphhopper/routing/QueryGraphTest.java +++ b/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