Skip to content

Commit 91b77f6

Browse files
boldtrnkarussell
boldtrn
authored and
karussell
committed
Show instructions for turns onto oneway streets (graphhopper#1281)
* Show instructions for turns onto oneway streets * Added another test * Fixed test * Improve comments * Reverted Alternative Route Test Change * Improved naming of surrouding/outgoing edges
1 parent d140953 commit 91b77f6

File tree

3 files changed

+169
-50
lines changed

3 files changed

+169
-50
lines changed

‎core/src/main/java/com/graphhopper/routing/InstructionsFromEdges.java

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -313,11 +313,18 @@ private int getTurn(EdgeIteratorState edge, int baseNode, int prevNode, int adjN
313313
forceInstruction = true;
314314
}
315315

316-
InstructionsSurroundingEdges surroundingEdges = new InstructionsSurroundingEdges(prevEdge, edge, encoder, crossingExplorer, nodeAccess, prevNode, baseNode, adjNode);
317-
int nrOfPossibleTurns = surroundingEdges.nrOfPossibleTurns();
316+
InstructionsOutgoingEdges outgoingEdges = new InstructionsOutgoingEdges(prevEdge, edge, encoder, crossingExplorer, nodeAccess, prevNode, baseNode, adjNode);
317+
int nrOfPossibleTurns = outgoingEdges.nrOfAllowedOutgoingEdges();
318318

319319
// there is no other turn possible
320320
if (nrOfPossibleTurns <= 1) {
321+
if (Math.abs(sign) > 1 && outgoingEdges.nrOfAllOutgoingEdges() > 1) {
322+
// This is an actual turn because |sign| > 1
323+
// There could be some confusion, if we would not create a turn instruction, even though it is the only
324+
// possible turn, also see #1048
325+
// TODO if we see issue with this approach we could consider checking if the edge is a oneway
326+
return sign;
327+
}
321328
return returnForcedInstructionOrIgnore(forceInstruction, sign);
322329
}
323330

@@ -327,7 +334,7 @@ private int getTurn(EdgeIteratorState edge, int baseNode, int prevNode, int adjN
327334
* Don't show an instruction if the user is following a street, even though the street is
328335
* bending. We should only do this, if following the street is the obvious choice.
329336
*/
330-
if (InstructionsHelper.isNameSimilar(name, prevName) && surroundingEdges.surroundingStreetsAreSlowerByFactor(2)) {
337+
if (InstructionsHelper.isNameSimilar(name, prevName) && outgoingEdges.outgoingEdgesAreSlowerByFactor(2)) {
331338
return returnForcedInstructionOrIgnore(forceInstruction, sign);
332339
}
333340

@@ -348,12 +355,12 @@ private int getTurn(EdgeIteratorState edge, int baseNode, int prevNode, int adjN
348355
long flag = edge.getFlags();
349356
long prevFlag = prevEdge.getFlags();
350357

351-
boolean surroundingStreetsAreSlower = surroundingEdges.surroundingStreetsAreSlowerByFactor(1);
358+
boolean outgoingEdgesAreSlower = outgoingEdges.outgoingEdgesAreSlowerByFactor(1);
352359

353360
// There is at least one other possibility to turn, and we are almost going straight
354361
// Check the other turns if one of them is also going almost straight
355362
// If not, we don't need a turn instruction
356-
EdgeIteratorState otherContinue = surroundingEdges.getOtherContinue(prevLat, prevLon, prevOrientation);
363+
EdgeIteratorState otherContinue = outgoingEdges.getOtherContinue(prevLat, prevLon, prevOrientation);
357364

358365
// Signs provide too less detail, so we use the delta for a precise comparision
359366
double delta = InstructionsHelper.calculateOrientationDelta(prevLat, prevLon, lat, lon, prevOrientation);
@@ -367,10 +374,11 @@ private int getTurn(EdgeIteratorState edge, int baseNode, int prevNode, int adjN
367374
|| InstructionsHelper.isNameSimilar(otherContinue.getName(), prevName)
368375
|| prevFlag != flag
369376
|| prevFlag == otherContinue.getFlags()
370-
|| !surroundingStreetsAreSlower) {
377+
|| !outgoingEdgesAreSlower) {
371378
GHPoint tmpPoint = InstructionsHelper.getPointForOrientationCalculation(otherContinue, nodeAccess);
372379
double otherDelta = InstructionsHelper.calculateOrientationDelta(prevLat, prevLon, tmpPoint.getLat(), tmpPoint.getLon(), prevOrientation);
373380

381+
// This is required to avoid keep left/right on the motorway at off-ramps/motorway_links
374382
if (Math.abs(delta) < .1 && Math.abs(otherDelta) > .15 && InstructionsHelper.isNameSimilar(name, prevName)) {
375383
return Instruction.CONTINUE_ON_STREET;
376384
}
@@ -385,9 +393,9 @@ private int getTurn(EdgeIteratorState edge, int baseNode, int prevNode, int adjN
385393
}
386394
}
387395

388-
if (!surroundingStreetsAreSlower) {
396+
if (!outgoingEdgesAreSlower) {
389397
if (Math.abs(delta) > .4
390-
|| surroundingEdges.isLeavingCurrentStreet(prevName, name)) {
398+
|| outgoingEdges.isLeavingCurrentStreet(prevName, name)) {
391399
// Leave the current road -> create instruction
392400
return sign;
393401

‎core/src/main/java/com/graphhopper/routing/InstructionsSurroundingEdges.java renamed to ‎core/src/main/java/com/graphhopper/routing/InstructionsOutgoingEdges.java

Lines changed: 61 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -20,80 +20,105 @@
2020
import com.graphhopper.routing.util.DataFlagEncoder;
2121
import com.graphhopper.routing.util.FlagEncoder;
2222
import com.graphhopper.storage.NodeAccess;
23-
import com.graphhopper.util.*;
23+
import com.graphhopper.util.EdgeExplorer;
24+
import com.graphhopper.util.EdgeIterator;
25+
import com.graphhopper.util.EdgeIteratorState;
2426
import com.graphhopper.util.shapes.GHPoint;
2527

2628
import java.util.ArrayList;
2729
import java.util.List;
2830

2931
/**
30-
* This class maintains the surrounding edges for a single turn instruction.
31-
* <p>
32-
* There a different sets of edges.
33-
* The previous edge is the edge we are comming from.
32+
* This class handles the outgoing edges for a single turn instruction.
33+
*
34+
* There are different sets of edges.
35+
* The previous edge is the edge we are coming from.
3436
* The current edge is the edge we turn on.
35-
* The reachable edges are all edges we could turn on, without the prev edge and the current edge.
36-
* The surrounding edges are all edges surrounding the turn, without the prev edge and the current edge.
37+
* The allowedOutgoingEdges contains all edges that the current vehicle is allowed(*) to turn on to, excluding the prev edge and the current edge.
38+
* The allOutgoingEdges contains all edges surrounding this turn instruction, without the prev edge and the current edge.
39+
*
40+
* (*): This might not consider turn restrictions, but only simple access values.
41+
*
42+
* Here is an example:
43+
*
44+
* A --> B --> C
45+
* ^
46+
* |
47+
* X
48+
*
49+
* For the route from A->B->C and baseNode=B, adjacentNode=C:
50+
* - the previous edge is A->B
51+
* - the current edge is B->C
52+
* - the allowedOutgoingEdges are B->C => return value of {@link #nrOfAllowedOutgoingEdges()} is 1
53+
* - the allOutgoingEdges are B->X and B->C => return values of {@link #nrOfAllOutgoingEdges()} is 2
3754
*
3855
* @author Robin Boldt
3956
*/
40-
class InstructionsSurroundingEdges {
57+
class InstructionsOutgoingEdges {
4158

4259
final EdgeIteratorState prevEdge;
4360
final EdgeIteratorState currentEdge;
4461

45-
// Streets that are alternative turns, excluding oneways in the wrong direction
46-
final List<EdgeIteratorState> reachableEdges;
62+
// Outgoing edges that we would be allowed to turn on
63+
final List<EdgeIteratorState> allowedOutgoingEdges;
4764

48-
// All Streets surrounding the turn, including oneways in the wrong direction
49-
final List<EdgeIteratorState> surroundingEdges;
65+
// All outgoing edges, including oneways in the wrong direction
66+
final List<EdgeIteratorState> allOutgoingEdges;
5067

5168
final FlagEncoder encoder;
5269
final NodeAccess nodeAccess;
5370

54-
public InstructionsSurroundingEdges(EdgeIteratorState prevEdge,
55-
EdgeIteratorState currentEdge,
56-
FlagEncoder encoder,
57-
EdgeExplorer crossingExplorer,
58-
NodeAccess nodeAccess,
59-
int prevNode,
60-
int baseNode,
61-
int adjNode) {
71+
public InstructionsOutgoingEdges(EdgeIteratorState prevEdge,
72+
EdgeIteratorState currentEdge,
73+
FlagEncoder encoder,
74+
EdgeExplorer crossingExplorer,
75+
NodeAccess nodeAccess,
76+
int prevNode,
77+
int baseNode,
78+
int adjNode) {
6279
this.prevEdge = prevEdge;
6380
this.currentEdge = currentEdge;
6481
this.encoder = encoder;
6582
this.nodeAccess = nodeAccess;
6683

6784
EdgeIteratorState tmpEdge;
6885

69-
surroundingEdges = new ArrayList<>();
70-
reachableEdges = new ArrayList<>();
86+
allOutgoingEdges = new ArrayList<>();
87+
allowedOutgoingEdges = new ArrayList<>();
7188
EdgeIterator edgeIter = crossingExplorer.setBaseNode(baseNode);
7289
while (edgeIter.next()) {
7390
if (edgeIter.getAdjNode() != prevNode && edgeIter.getAdjNode() != adjNode) {
7491
tmpEdge = edgeIter.detach(false);
75-
surroundingEdges.add(tmpEdge);
92+
allOutgoingEdges.add(tmpEdge);
7693
if (encoder.isForward(tmpEdge.getFlags())) {
77-
reachableEdges.add(tmpEdge);
94+
allowedOutgoingEdges.add(tmpEdge);
7895
}
7996
}
8097
}
8198
}
8299

83100
/**
84-
* Calculates the Number of possible turns, including the current turn.
85-
* If there is only one turn possible, e.g. continue straight on the road is a turn,
86-
* the method will return 1.
101+
* This method calculates the number of allowed outgoing edges, which could be considered the number of possible
102+
* roads one might take at the intersection. This excludes the road you are coming from and inaccessible roads.
87103
*/
88-
public int nrOfPossibleTurns() {
89-
return 1 + reachableEdges.size();
104+
public int nrOfAllowedOutgoingEdges() {
105+
return 1 + allowedOutgoingEdges.size();
90106
}
91107

92108
/**
93-
* Checks if the surrounding streets are slower. If they are, this indicates, that we are staying
109+
* This method calculates the number of all outgoing edges, which could be considered the number of roads you see
110+
* at the intersection. This excludes the road your are coming from.
111+
*/
112+
public int nrOfAllOutgoingEdges() {
113+
return 1 + allOutgoingEdges.size();
114+
}
115+
116+
117+
/**
118+
* Checks if the outgoing edges are slower by the provided factor. If they are, this indicates, that we are staying
94119
* on the prominent street that one would follow anyway.
95120
*/
96-
public boolean surroundingStreetsAreSlowerByFactor(double factor) {
121+
public boolean outgoingEdgesAreSlowerByFactor(double factor) {
97122
double tmpSpeed = getSpeed(currentEdge);
98123
double pathSpeed = getSpeed(prevEdge);
99124

@@ -104,7 +129,7 @@ public boolean surroundingStreetsAreSlowerByFactor(double factor) {
104129

105130
double maxSurroundingSpeed = -1;
106131

107-
for (EdgeIteratorState edge : surroundingEdges) {
132+
for (EdgeIteratorState edge : allOutgoingEdges) {
108133
tmpSpeed = getSpeed(edge);
109134
if (tmpSpeed < 1) {
110135
// This might happen for the DataFlagEncoder, might create unnecessary turn instructions
@@ -128,12 +153,13 @@ private double getSpeed(EdgeIteratorState edge) {
128153
}
129154

130155
/**
131-
* Returns an edge that is going into more or less straight compared to the prevEdge.
156+
* Returns an edge that has more or less in the same orientation as the prevEdge, but is not the currentEdge.
157+
* If there is one, this indicates that we might need an instruction to help finding the correct edge out of the different choices.
132158
* If there is none, return null.
133159
*/
134160
public EdgeIteratorState getOtherContinue(double prevLat, double prevLon, double prevOrientation) {
135161
int tmpSign;
136-
for (EdgeIteratorState edge : reachableEdges) {
162+
for (EdgeIteratorState edge : allowedOutgoingEdges) {
137163
GHPoint point = InstructionsHelper.getPointForOrientationCalculation(edge, nodeAccess);
138164
tmpSign = InstructionsHelper.calculateSign(prevLat, prevLon, point.getLat(), point.getLon(), prevOrientation);
139165
if (Math.abs(tmpSign) <= 1) {
@@ -155,7 +181,7 @@ public boolean isLeavingCurrentStreet(String prevName, String name) {
155181

156182
// If flags are changing, there might be a chance we find these flags on a different edge
157183
boolean checkFlag = currentEdge.getFlags() != prevEdge.getFlags();
158-
for (EdgeIteratorState edge : reachableEdges) {
184+
for (EdgeIteratorState edge : allowedOutgoingEdges) {
159185
String edgeName = edge.getName();
160186
long edgeFlag = edge.getFlags();
161187
// leave the current street || enter a different street

‎core/src/test/java/com/graphhopper/routing/PathTest.java

Lines changed: 92 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,91 @@ public void testCalcInstructionForForkWithSameName() {
541541
assertEquals(-7, wayList.get(1).getSign());
542542
}
543543

544+
@Test
545+
public void testCalcInstructionsEnterMotoway() {
546+
final Graph g = new GraphBuilder(carManager).create();
547+
final NodeAccess na = g.getNodeAccess();
548+
549+
// Actual example: point=48.630533%2C9.459416&point=48.630544%2C9.459829
550+
// 1 -2 -3 is a motorway and tagged as oneway
551+
// 1 ->- 2 ->- 3
552+
// /
553+
// 4
554+
na.setNode(1, 48.630647, 9.459041);
555+
na.setNode(2, 48.630586, 9.459604);
556+
na.setNode(3, 48.630558, 9.459851);
557+
na.setNode(4, 48.63054, 9.459406);
558+
559+
g.edge(1, 2, 5, false).setName("A 8");
560+
g.edge(2, 3, 5, false).setName("A 8");
561+
g.edge(4, 2, 5, false).setName("A 8");
562+
563+
Path p = new Dijkstra(g, new ShortestWeighting(encoder), TraversalMode.NODE_BASED)
564+
.calcPath(4, 3);
565+
assertTrue(p.isFound());
566+
InstructionList wayList = p.calcInstructions(tr);
567+
568+
// no turn instruction for entering the highway
569+
assertEquals(2, wayList.size());
570+
}
571+
572+
@Test
573+
public void testCalcInstructionsMotowayJunction() {
574+
final Graph g = new GraphBuilder(carManager).create();
575+
final NodeAccess na = g.getNodeAccess();
576+
577+
// Actual example: point=48.70672%2C9.164266&point=48.706805%2C9.162995
578+
// A typical motorway junction, when following 1-2-3, there should be a keep right at 2
579+
// -- 4
580+
// /
581+
// 1 -- 2 -- 3
582+
na.setNode(1, 48.70672, 9.164266);
583+
na.setNode(2, 48.706741, 9.163719);
584+
na.setNode(3, 48.706805, 9.162995);
585+
na.setNode(4, 48.706705, 9.16329);
586+
587+
g.edge(1, 2, 5, false).setName("A 8");
588+
g.edge(2, 3, 5, false).setName("A 8");
589+
g.edge(2, 4, 5, false).setName("A 8");
590+
591+
Path p = new Dijkstra(g, new ShortestWeighting(encoder), TraversalMode.NODE_BASED)
592+
.calcPath(1, 3);
593+
assertTrue(p.isFound());
594+
InstructionList wayList = p.calcInstructions(tr);
595+
596+
assertEquals(3, wayList.size());
597+
// TODO this should be a keep_right
598+
assertEquals(0, wayList.get(1).getSign());
599+
}
600+
601+
@Test
602+
public void testCalcInstructionsOntoOneway() {
603+
final Graph g = new GraphBuilder(carManager).create();
604+
final NodeAccess na = g.getNodeAccess();
605+
606+
// Actual example: point=-33.824566%2C151.187834&point=-33.82441%2C151.188231
607+
// 1 -2 -3 is a oneway
608+
// 1 ->- 2 ->- 3
609+
// |
610+
// 4
611+
na.setNode(1, -33.824245, 151.187866);
612+
na.setNode(2, -33.824335, 151.188017);
613+
na.setNode(3, -33.824415, 151.188177);
614+
na.setNode(4, -33.824437, 151.187925);
615+
616+
g.edge(1, 2, 5, false).setName("Pacific Highway");
617+
g.edge(2, 3, 5, false).setName("Pacific Highway");
618+
g.edge(4, 2, 5, true).setName("Greenwich Road");
619+
620+
Path p = new Dijkstra(g, new ShortestWeighting(encoder), TraversalMode.NODE_BASED)
621+
.calcPath(4, 3);
622+
assertTrue(p.isFound());
623+
InstructionList wayList = p.calcInstructions(tr);
624+
625+
assertEquals(3, wayList.size());
626+
assertEquals(2, wayList.get(1).getSign());
627+
}
628+
544629
@Test
545630
public void testCalcInstructionContinueLeavingStreet() {
546631
final Graph g = new GraphBuilder(carManager).create();
@@ -643,13 +728,13 @@ public void testUTurnRight() {
643728
// 4----5----6
644729
// |
645730
// 3----2----1
646-
na.setNode(1, -33.885758,151.181472);
647-
na.setNode(2, -33.885852,151.180968);
648-
na.setNode(3, -33.885968,151.180501);
649-
na.setNode(4, -33.885883,151.180442);
650-
na.setNode(5, -33.885772,151.180941);
651-
na.setNode(6, -33.885692,151.181445);
652-
na.setNode(7, -33.885692,151.181445);
731+
na.setNode(1, -33.885758, 151.181472);
732+
na.setNode(2, -33.885852, 151.180968);
733+
na.setNode(3, -33.885968, 151.180501);
734+
na.setNode(4, -33.885883, 151.180442);
735+
na.setNode(5, -33.885772, 151.180941);
736+
na.setNode(6, -33.885692, 151.181445);
737+
na.setNode(7, -33.885692, 151.181445);
653738

654739
g.edge(1, 2, 5, false).setName("Parramatta Road");
655740
g.edge(2, 3, 5, false).setName("Parramatta Road");

0 commit comments

Comments
 (0)