Skip to content

Commit

Permalink
Improve turn instructions when leaving the roads (#1882)
Browse files Browse the repository at this point in the history 
* Improve turn instructions when leaving the roads

* Rename nrOf to get following default Java naming.

Co-authored-by: Peter <graphhopper@gmx.de>
  • Loading branch information
@boldtrn @karussell
boldtrn and karussell committed Feb 5, 2020
1 parent 04cfcd0 commit a03a938
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 48 deletions.
6 changes: 3 additions & 3 deletions core/src/main/java/com/graphhopper/routing/InstructionsFromEdges.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -391,12 +391,12 @@ private int getTurn(EdgeIteratorState edge, int baseNode, int prevNode, int adjN
forceInstruction = true;
}

InstructionsOutgoingEdges outgoingEdges = new InstructionsOutgoingEdges(prevEdge, edge, encoder, maxSpeedEnc, crossingExplorer, nodeAccess, prevNode, baseNode, adjNode);
int nrOfPossibleTurns = outgoingEdges.nrOfAllowedOutgoingEdges();
InstructionsOutgoingEdges outgoingEdges = new InstructionsOutgoingEdges(prevEdge, edge, encoder, maxSpeedEnc, roadClassEnc, roadClassLinkEnc, crossingExplorer, nodeAccess, prevNode, baseNode, adjNode);
int nrOfPossibleTurns = outgoingEdges.getAllowedTurns();

// there is no other turn possible
if (nrOfPossibleTurns <= 1) {
if (Math.abs(sign) > 1 && outgoingEdges.nrOfAllOutgoingEdges() > 1) {
if (Math.abs(sign) > 1 && outgoingEdges.getVisibleTurns() > 1) {
// This is an actual turn because |sign| > 1
// There could be some confusion, if we would not create a turn instruction, even though it is the only
// possible turn, also see #1048
Expand Down
65 changes: 34 additions & 31 deletions core/src/main/java/com/graphhopper/routing/InstructionsOutgoingEdges.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@

import com.graphhopper.routing.profiles.BooleanEncodedValue;
import com.graphhopper.routing.profiles.DecimalEncodedValue;
import com.graphhopper.routing.profiles.EnumEncodedValue;
import com.graphhopper.routing.profiles.RoadClass;
import com.graphhopper.routing.util.FlagEncoder;
import com.graphhopper.storage.IntsRef;
import com.graphhopper.storage.NodeAccess;
import com.graphhopper.util.EdgeExplorer;
import com.graphhopper.util.EdgeIterator;
Expand All @@ -35,8 +36,8 @@
* There are different sets of edges.
* The previous edge is the edge we are coming from.
* The current edge is the edge we turn on.
* The allowedOutgoingEdges contains all edges that the current vehicle is allowed(*) to turn on to, excluding the prev edge and the current edge.
* The allOutgoingEdges contains all edges surrounding this turn instruction, without the prev edge and the current edge.
* The allowedAlternativeTurns contains all edges that the current vehicle is allowed(*) to turn on to, excluding the prev edge and the current edge.
* The visibleAlternativeTurns contains all edges surrounding this turn instruction, without the prev edge and the current edge.
* (*): This might not consider turn restrictions, but only simple access values.
* Here is an example:
* <pre>
Expand All @@ -48,8 +49,8 @@
* For the route from A->B->C and baseNode=B, adjacentNode=C:
* - the previous edge is A->B
* - the current edge is B->C
* - the allowedOutgoingEdges are B->C => return value of {@link #nrOfAllowedOutgoingEdges()} is 1
* - the allOutgoingEdges are B->X and B->C => return values of {@link #nrOfAllOutgoingEdges()} is 2
* - the allowedAlternativeTurns are B->C => return value of {@link #getAllowedTurns()} is 1
* - the visibleAlternativeTurns are B->X and B->C => return values of {@link #getVisibleTurns()} is 2
*
* @author Robin Boldt
*/
Expand All @@ -58,17 +59,21 @@ class InstructionsOutgoingEdges {
private final EdgeIteratorState prevEdge;
private final EdgeIteratorState currentEdge;
// Outgoing edges that we would be allowed to turn on
private final List<EdgeIteratorState> filteredOutgoingEdges;
private final List<EdgeIteratorState> allowedAlternativeTurns;
// All outgoing edges, including oneways in the wrong direction
private final List<EdgeIteratorState> filteredEdges;
private final List<EdgeIteratorState> visibleAlternativeTurns;
private final DecimalEncodedValue maxSpeedEnc;
private final DecimalEncodedValue avgSpeedEnc;
private final EnumEncodedValue<RoadClass> roadClassEnc;
private final BooleanEncodedValue roadClassLinkEnc;
private final NodeAccess nodeAccess;

public InstructionsOutgoingEdges(EdgeIteratorState prevEdge,
EdgeIteratorState currentEdge,
FlagEncoder encoder,
DecimalEncodedValue maxSpeedEnc,
EnumEncodedValue<RoadClass> roadClassEnc,
BooleanEncodedValue roadClassLinkEnc,
EdgeExplorer crossingExplorer,
NodeAccess nodeAccess,
int prevNode,
Expand All @@ -79,19 +84,21 @@ public InstructionsOutgoingEdges(EdgeIteratorState prevEdge,
BooleanEncodedValue accessEnc = encoder.getAccessEnc();
this.maxSpeedEnc = maxSpeedEnc;
this.avgSpeedEnc = encoder.getAverageSpeedEnc();
this.roadClassEnc = roadClassEnc;
this.roadClassLinkEnc = roadClassLinkEnc;
this.nodeAccess = nodeAccess;

EdgeIteratorState tmpEdge;

filteredEdges = new ArrayList<>();
filteredOutgoingEdges = new ArrayList<>();
visibleAlternativeTurns = new ArrayList<>();
allowedAlternativeTurns = new ArrayList<>();
EdgeIterator edgeIter = crossingExplorer.setBaseNode(baseNode);
while (edgeIter.next()) {
if (edgeIter.getAdjNode() != prevNode && edgeIter.getAdjNode() != adjNode) {
tmpEdge = edgeIter.detach(false);
filteredEdges.add(tmpEdge);
visibleAlternativeTurns.add(tmpEdge);
if (tmpEdge.get(accessEnc)) {
filteredOutgoingEdges.add(tmpEdge);
allowedAlternativeTurns.add(tmpEdge);
}
}
}
Expand All @@ -101,16 +108,16 @@ public InstructionsOutgoingEdges(EdgeIteratorState prevEdge,
* This method calculates the number of allowed outgoing edges, which could be considered the number of possible
* roads one might take at the intersection. This excludes the road you are coming from and inaccessible roads.
*/
public int nrOfAllowedOutgoingEdges() {
return 1 + filteredOutgoingEdges.size();
public int getAllowedTurns() {
return 1 + allowedAlternativeTurns.size();
}

/**
* This method calculates the number of all outgoing edges, which could be considered the number of roads you see
* at the intersection. This excludes the road your are coming from.
*/
public int nrOfAllOutgoingEdges() {
return 1 + filteredEdges.size();
public int getVisibleTurns() {
return 1 + visibleAlternativeTurns.size();
}


Expand All @@ -129,7 +136,7 @@ public boolean outgoingEdgesAreSlowerByFactor(double factor) {

double maxSurroundingSpeed = -1;

for (EdgeIteratorState edge : filteredEdges) {
for (EdgeIteratorState edge : visibleAlternativeTurns) {
tmpSpeed = getSpeed(edge);
if (tmpSpeed < 1) {
// This might happen for the DataFlagEncoder, might create unnecessary turn instructions
Expand Down Expand Up @@ -162,7 +169,7 @@ private double getSpeed(EdgeIteratorState edge) {
*/
public EdgeIteratorState getOtherContinue(double prevLat, double prevLon, double prevOrientation) {
int tmpSign;
for (EdgeIteratorState edge : filteredOutgoingEdges) {
for (EdgeIteratorState edge : allowedAlternativeTurns) {
GHPoint point = InstructionsHelper.getPointForOrientationCalculation(edge, nodeAccess);
tmpSign = InstructionsHelper.calculateSign(prevLat, prevLon, point.getLat(), point.getLon(), prevOrientation);
if (Math.abs(tmpSign) <= 1) {
Expand All @@ -182,27 +189,23 @@ public boolean isLeavingCurrentStreet(String prevName, String name) {
return false;
}

// If flags are changing, there might be a chance we find these flags on a different edge
boolean checkFlag = currentEdge.getFlags() != prevEdge.getFlags();
for (EdgeIteratorState edge : filteredOutgoingEdges) {
boolean roadClassOrLinkChange = !isTheSameRoadClassAndLink(prevEdge, currentEdge);
for (EdgeIteratorState edge : allowedAlternativeTurns) {
String edgeName = edge.getName();
IntsRef edgeFlag = edge.getFlags();
// leave the current street || enter a different street
if (isTheSameStreet(prevName, prevEdge.getFlags(), edgeName, edgeFlag, checkFlag)
|| isTheSameStreet(name, currentEdge.getFlags(), edgeName, edgeFlag, checkFlag)) {
// leave the current street
if (InstructionsHelper.isNameSimilar(prevName, edgeName) || (roadClassOrLinkChange && isTheSameRoadClassAndLink(prevEdge, edge))) {
return true;
}
}
return false;
}

private boolean isTheSameStreet(String name1, IntsRef flags1, String name2, IntsRef flags2, boolean checkFlag) {
if (InstructionsHelper.isNameSimilar(name1, name2)) {
if (!checkFlag || flags1.equals(flags2)) {
// enter a different street
if (InstructionsHelper.isNameSimilar(name, edgeName) || (roadClassOrLinkChange && isTheSameRoadClassAndLink(currentEdge, edge))) {
return true;
}
}
return false;
}

private boolean isTheSameRoadClassAndLink(EdgeIteratorState edge1, EdgeIteratorState edge2) {
return edge1.get(roadClassEnc) == edge2.get(roadClassEnc) && edge1.get(roadClassLinkEnc) == edge2.get(roadClassLinkEnc);
}

}
43 changes: 42 additions & 1 deletion core/src/test/java/com/graphhopper/routing/PathTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* additional information regarding copyright ownership.
*
* GraphHopper GmbH licenses this file to you under the Apache License,
* Version 2.0 (the "License"); you may not use this file except in
* Version 2.0 (the "License"); you may not use this file except in
* compliance with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
Expand Down Expand Up @@ -716,6 +716,47 @@ public void testCalcInstructionsOntoOneway() {
assertEquals(2, wayList.get(1).getSign());
}

@Test
public void testCalcInstructionIssue1047() {
final Graph g = new GraphBuilder(carManager).create();
final NodeAccess na = g.getNodeAccess();

// Actual example: point=51.367105%2C14.491246&point=51.369048%2C14.483092
// 1-2 & 2-3 is a road that is turning right, 2-4 is a that is branching off.
// When driving 1-2-4, we should create an instruction notifying the user to continue straight instead of turning and following the road
// When driving 1-2-3, we should create an instruction as well
//
// 1 ---- 2 ---- 4
// |
// 3
na.setNode(1, 51.367544, 14.488209);
na.setNode(2, 51.368046, 14.486525);
na.setNode(3, 51.36875, 14.487019);
na.setNode(4, 51.368428, 14.485173);

EnumEncodedValue<RoadClass> roadClassEnc = carManager.getEnumEncodedValue(RoadClass.KEY, RoadClass.class);
BooleanEncodedValue roadClassLinkEnc = carManager.getBooleanEncodedValue(RoadClassLink.KEY);

g.edge(1, 2, 5, true).setName("B 156").set(roadClassEnc, RoadClass.PRIMARY).set(roadClassLinkEnc, false);
g.edge(2, 4, 5, true).setName("S 108").set(roadClassEnc, RoadClass.SECONDARY).set(roadClassLinkEnc, false);
g.edge(2, 3, 5, true).setName("B 156").set(roadClassEnc, RoadClass.PRIMARY).set(roadClassLinkEnc, false);

ShortestWeighting weighting = new ShortestWeighting(encoder);
Path p = new Dijkstra(g, weighting, TraversalMode.NODE_BASED)
.calcPath(1, 4);
assertTrue(p.isFound());
InstructionList wayList = InstructionsFromEdges.calcInstructions(p, p.graph, weighting, carManager, tr);

assertEquals(3, wayList.size());

p = new Dijkstra(g, weighting, TraversalMode.NODE_BASED)
.calcPath(1, 3);
assertTrue(p.isFound());
wayList = InstructionsFromEdges.calcInstructions(p, p.graph, weighting, carManager, tr);

assertEquals(3, wayList.size());
}

@Test
public void testCalcInstructionContinueLeavingStreet() {
final Graph g = new GraphBuilder(carManager).create();
Expand Down
26 changes: 13 additions & 13 deletions reader-osm/src/test/java/com/graphhopper/GraphHopperIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public void testMonacoWithInstructions() {
assertEquals(43.7495432, arsp.getWaypoints().getLat(1), 1e-7);

InstructionList il = arsp.getInstructions();
assertEquals(15, il.size());
assertEquals(16, il.size());

// TODO roundabout fine tuning -> enter + leave roundabout (+ two roundabouts -> is it necessary if we do not leave the street?)
Translation tr = hopper.getTranslationMap().getWithFallBack(Locale.US);
Expand Down Expand Up @@ -515,22 +515,22 @@ public void testMonacoVia() {
assertEquals(171, arsp.getPoints().getSize());

InstructionList il = arsp.getInstructions();
assertEquals(29, il.size());
assertEquals(30, il.size());
assertEquals("continue onto Avenue des Guelfes", il.get(0).getTurnDescription(tr));
assertEquals("continue onto Avenue des Papalins", il.get(1).getTurnDescription(tr));
assertEquals("turn sharp right onto Quai Jean-Charles Rey", il.get(4).getTurnDescription(tr));
assertEquals("turn left", il.get(5).getTurnDescription(tr));
assertEquals("turn right onto Avenue Albert II", il.get(6).getTurnDescription(tr));

assertEquals("waypoint 1", il.get(14).getTurnDescription(tr));
assertEquals(Instruction.U_TURN_UNKNOWN, il.get(15).getSign());
assertEquals("waypoint 1", il.get(15).getTurnDescription(tr));
assertEquals(Instruction.U_TURN_UNKNOWN, il.get(16).getSign());

assertEquals("continue onto Avenue Albert II", il.get(22).getTurnDescription(tr));
assertEquals("turn left", il.get(23).getTurnDescription(tr));
assertEquals("turn right onto Quai Jean-Charles Rey", il.get(24).getTurnDescription(tr));
assertEquals("turn sharp left onto Avenue des Papalins", il.get(25).getTurnDescription(tr));
assertEquals("continue onto Avenue des Guelfes", il.get(27).getTurnDescription(tr));
assertEquals("arrive at destination", il.get(28).getTurnDescription(tr));
assertEquals("continue onto Avenue Albert II", il.get(23).getTurnDescription(tr));
assertEquals("turn left", il.get(24).getTurnDescription(tr));
assertEquals("turn right onto Quai Jean-Charles Rey", il.get(25).getTurnDescription(tr));
assertEquals("turn sharp left onto Avenue des Papalins", il.get(26).getTurnDescription(tr));
assertEquals("continue onto Avenue des Guelfes", il.get(28).getTurnDescription(tr));
assertEquals("arrive at destination", il.get(29).getTurnDescription(tr));

assertEquals(11, il.get(0).getDistance(), 1);
assertEquals(97, il.get(1).getDistance(), 1);
Expand Down Expand Up @@ -725,11 +725,11 @@ public void testSRTMWithInstructions() {

PathWrapper arsp = rsp.getBest();
assertEquals(1625.4, arsp.getDistance(), .1);
assertEquals(54, arsp.getPoints().getSize());
assertEquals(55, arsp.getPoints().getSize());
assertTrue(arsp.getPoints().is3D());

InstructionList il = arsp.getInstructions();
assertEquals(12, il.size());
assertEquals(13, il.size());
assertTrue(il.get(0).getPoints().is3D());

String str = arsp.getPoints().toString();
Expand All @@ -750,7 +750,7 @@ public void testSRTMWithInstructions() {
assertEquals(99, arsp.getAscend(), 1e-1);
assertEquals(150, arsp.getDescend(), 1e-1);

assertEquals(54, arsp.getPoints().size());
assertEquals(55, arsp.getPoints().size());
assertEquals(new GHPoint3D(43.73068455771767, 7.421283689825812, 62.0), arsp.getPoints().get(0));
assertEquals(new GHPoint3D(43.727680946587874, 7.4191987684222065, 11.0), arsp.getPoints().get(arsp.getPoints().size() - 1));

Expand Down

0 comments on commit a03a938

Please sign in to comment.