Skip to content

Commit ad9ef2c

Browse files
mjjbellmjjbell
committed
Correctly handle compressed traffic signals
Unidirectional traffic signal segments are currently not compressed. This means traffic signals which are not on turns can be missed and not applied the correct penalty. This commit changes this behaviour to correctly handle the graph compression. Additional tests are added to ensure there is no regression for other cases (turns, restrictions).
1 parent 99875b4 commit ad9ef2c

File tree

8 files changed

+204
-64
lines changed

8 files changed

+204
-64
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
- FIXED: Bicycle and foot profiles now don't route on proposed ways [#6615](https://github.com/Project-OSRM/osrm-backend/pull/6615)
3636
- Routing:
3737
- FIXED: Fix adding traffic signal penalties during compression [#6419](https://github.com/Project-OSRM/osrm-backend/pull/6419)
38+
- FIXED: Correctly handle compressed traffic signals. [#6724](https://github.com/Project-OSRM/osrm-backend/pull/6724)
3839
# 5.27.1
3940
- Changes from 5.27.0
4041
- Misc:

‎features/car/traffic_light_penalties.feature‎

Lines changed: 162 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,14 @@ Feature: Car - Handle traffic lights
3232
| l | traffic_signals |
3333

3434
When I route I should get
35-
| from | to | time | # |
35+
| from | to | time | # |
3636
| 1 | 2 | 11.1s | no turn with no traffic light |
3737
| 3 | 4 | 13.1s | no turn with traffic light |
3838
| g | j | 18.7s | turn with no traffic light |
3939
| k | n | 20.7s | turn with traffic light |
4040

4141

42-
Scenario: Car - Traffic signal direction
42+
Scenario: Car - Traffic signal direction straight
4343
Given the node map
4444
"""
4545
a-1-b-2-c
@@ -112,14 +112,14 @@ Feature: Car - Handle traffic lights
112112

113113

114114

115-
Scenario: Car - Encounters a traffic light
115+
Scenario: Car - Encounters a traffic light direction
116116
Given the node map
117117
"""
118-
a f k
119-
| | |
120-
b-c-d h-g-i l-m-n
121-
| | |
122-
e j o
118+
a f k p
119+
| | | |
120+
b-c-d h-g-i l-m-n q-r-s
121+
| | | |
122+
e j o t
123123
124124
"""
125125

@@ -131,53 +131,70 @@ Feature: Car - Handle traffic lights
131131
| fgj | primary |
132132
| lmn | primary |
133133
| kmo | primary |
134+
| qrs | primary |
135+
| prt | primary |
134136

135137
And the nodes
136138
| node | highway | traffic_signals:direction |
137-
| g | traffic_signals | forward |
138-
| m | traffic_signals | backward |
139+
| g | traffic_signals | |
140+
| m | traffic_signals | forward |
141+
| r | traffic_signals | backward |
139142

140143

141144
When I route I should get
145+
# Base case
142146
| from | to | time | # |
143-
| a | d | 21.9s | no turn with no traffic light |
144-
| a | e | 22.2s | no turn with traffic light |
145147
| a | b | 18.7s | turn with no traffic light |
146-
| e | b | 21.9s | no turn with no traffic light |
147-
| e | a | 22.2s | no turn with traffic light |
148+
| a | e | 22.2s | no turn with no traffic light |
149+
| a | d | 21.9s | turn with no traffic light |
150+
| e | b | 21.9s | turn with no traffic light |
151+
| e | a | 22.2s | no turn with no traffic light |
148152
| e | d | 18.7s | turn with no traffic light |
149-
| d | e | 21.9s | no turn with no traffic light |
150-
| d | b | 11s | no turn with traffic light |
153+
| d | e | 21.9s | turn with no traffic light |
154+
| d | b | 11s | no turn with no traffic light |
151155
| d | a | 18.7s | turn with no traffic light |
152-
| b | a | 21.9s | no turn with no traffic light |
153-
| b | d | 11s | no turn with traffic light |
156+
| b | a | 21.9s | turn with no traffic light |
157+
| b | d | 11s | no turn with no traffic light |
154158
| b | e | 18.7s | turn with no traffic light |
155-
156-
| f | i | 23.9s | no turn with no traffic light |
159+
# All have traffic lights - 2s penalty
160+
| f | h | 20.7s | turn with traffic light |
157161
| f | j | 24.2s | no turn with traffic light |
158-
| f | h | 20.7s | turn with no traffic light |
159-
| j | h | 21.9s | no turn with no traffic light |
160-
| j | f | 22.2s | no turn with traffic light |
161-
| j | i | 18.7s | turn with no traffic light |
162-
| i | j | 21.9s | no turn with no traffic light |
163-
| i | h | 11s | no turn with traffic light |
164-
| i | f | 18.7s | turn with no traffic light |
165-
| h | f | 23.9s | no turn with no traffic light |
162+
| f | i | 23.9s | turn with traffic light |
163+
| j | h | 23.9s | turn with traffic light |
164+
| j | f | 24.2s | no turn with traffic light |
165+
| j | i | 20.7s | turn with traffic light |
166+
| i | j | 23.9s | turn with traffic light |
167+
| i | h | 13s | no turn with traffic light |
168+
| i | f | 20.7s | turn with traffic light |
169+
| h | f | 23.9s | turn with traffic light |
166170
| h | i | 13s | no turn with traffic light |
167-
| h | j | 20.7s | turn with no traffic light |
168-
169-
| k | n | 21.9s | no turn with no traffic light |
170-
| k | o | 22.2s | no turn with traffic light |
171-
| k | l | 18.7s | turn with no traffic light |
172-
| o | l | 23.9s | no turn with no traffic light |
173-
| o | k | 24.2s | no turn with traffic light |
174-
| o | n | 20.7s | turn with no traffic light |
175-
| n | o | 23.9s | no turn with no traffic light |
176-
| n | l | 13s | no turn with traffic light |
177-
| n | k | 20.7s | turn with no traffic light |
178-
| l | k | 21.9s | no turn with no traffic light |
179-
| l | n | 11s | no turn with traffic light |
180-
| l | o | 18.7s | turn with no traffic light |
171+
| h | j | 20.7s | turn with traffic light |
172+
# Front direction have traffic lights - 2s penalty
173+
| k | l | 20.7s | turn with traffic light |
174+
| k | o | 24.2s | no turn with traffic light |
175+
| k | n | 23.9s | turn with traffic light |
176+
| o | l | 21.9s | turn with no traffic light |
177+
| o | k | 22.2s | no turn with no traffic light |
178+
| o | n | 18.7s | turn with no traffic light |
179+
| n | o | 21.9s | turn with no traffic light |
180+
| n | l | 11s | no turn with no traffic light |
181+
| n | k | 18.7s | turn with no traffic light |
182+
| l | k | 23.9s | turn with traffic light |
183+
| l | n | 13s | no turn with traffic light |
184+
| l | o | 20.7s | turn with traffic light |
185+
# Reverse direction have traffic lights - 2s penalty
186+
| p | q | 18.7s | turn with no traffic light |
187+
| p | t | 22.2s | no turn with no traffic light |
188+
| p | s | 21.9s | turn with no traffic light |
189+
| t | q | 23.9s | turn with traffic light |
190+
| t | p | 24.2s | no turn with traffic light |
191+
| t | s | 20.7s | turn with traffic light |
192+
| s | t | 23.9s | turn with traffic light |
193+
| s | q | 13s | no turn with traffic light |
194+
| s | p | 20.7s | turn with traffic light |
195+
| q | p | 21.9s | turn with no traffic light |
196+
| q | s | 11s | no turn with no traffic light |
197+
| q | t | 18.7s | turn with no traffic light |
181198

182199

183200
Scenario: Traffic Signal Geometry
@@ -343,3 +360,106 @@ Feature: Car - Handle traffic lights
343360
| from | to | route | speed | weights | time | distances | a:datasources | a:nodes | a:speed | a:duration | a:weight |
344361
| a | c | abc,abc | 65 km/h | 22.2,0 | 22.2s | 400m,0m | 1:0 | 1:2:3 | 18:18 | 11.1:11.1 | 11.1:11.1 |
345362
| c | a | abc,abc | 60 km/h | 24.2,0 | 24.2s | 400m,0m | 0:1 | 3:2:1 | 18:18 | 11.1:11.1 | 11.1:11.1 |
363+
364+
365+
Scenario: Car - Traffic signal straight direction with edge compression
366+
Given the node map
367+
"""
368+
a-1-b - c - d-2-e
369+
370+
"""
371+
372+
And the ways
373+
| nodes | highway |
374+
| abcde | primary |
375+
376+
And the nodes
377+
| node | highway | traffic_signals:direction |
378+
| c | traffic_signals | forward |
379+
380+
When I route I should get
381+
| from | to | time | weight | # |
382+
| 1 | 2 | 35.3s | 35.3 | no turn with traffic light |
383+
| 2 | 1 | 33.3s | 33.3 | no turn with no traffic light |
384+
385+
386+
Scenario: Car - Traffic signal turn direction with edge compression
387+
Given the node map
388+
"""
389+
d
390+
|
391+
2
392+
|
393+
a-1-b - c - f
394+
|
395+
e
396+
397+
j
398+
|
399+
4
400+
|
401+
g-3-h - i - k
402+
|
403+
l
404+
405+
"""
406+
407+
And the ways
408+
| nodes | highway |
409+
| abc | primary |
410+
| cf | primary |
411+
| fd | primary |
412+
| fe | primary |
413+
| ghi | primary |
414+
| ik | primary |
415+
| kj | primary |
416+
| kl | primary |
417+
418+
And the nodes
419+
| node | highway | traffic_signals:direction |
420+
| k | traffic_signals | forward |
421+
422+
When I route I should get
423+
| from | to | time | weight | # |
424+
| 1 | 2 | 44.2s | 44.2 | turn with no traffic light |
425+
| 2 | 1 | 41s | 41 | turn with no traffic light |
426+
| 3 | 4 | 46.2s | 46.2 | turn with traffic light |
427+
| 4 | 3 | 41s | 41 | turn with no traffic light |
428+
429+
430+
Scenario: Car - Traffic signal turn direction with turn restriction
431+
Given the node map
432+
"""
433+
d
434+
|
435+
2
436+
|
437+
a-1-b - c - f
438+
|
439+
e
440+
441+
"""
442+
443+
And the ways
444+
| nodes | highway |
445+
| abc | primary |
446+
| cf | primary |
447+
| fd | primary |
448+
| fe | primary |
449+
450+
And the nodes
451+
| node | highway | traffic_signals:direction |
452+
| f | traffic_signals | forward |
453+
454+
And the relations
455+
| type | way:from | way:to | way:via | restriction |
456+
| restriction | abc | fe | cf | no_right_turn |
457+
458+
And the relations
459+
| type | way:from | way:to | node:via | restriction |
460+
| restriction | df | fc | f | right_turn_only |
461+
462+
When I route I should get
463+
| from | to | time | weight | # |
464+
| 1 | 2 | 46.2s | 46.2 | turn with traffic light |
465+
| 2 | 1 | 41s | 41 | turn with no traffic light |

‎include/extractor/graph_compressor.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ class GraphCompressor
2424

2525
public:
2626
void Compress(const std::unordered_set<NodeID> &barrier_nodes,
27-
const TrafficSignals &traffic_signals,
27+
TrafficSignals &traffic_signals,
2828
ScriptingEnvironment &scripting_environment,
2929
std::vector<TurnRestriction> &turn_restrictions,
3030
std::vector<UnresolvedManeuverOverride> &maneuver_overrides,

‎include/extractor/node_based_graph_factory.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class NodeBasedGraphFactory
3939
NodeBasedGraphFactory(ScriptingEnvironment &scripting_environment,
4040
std::vector<TurnRestriction> &turn_restrictions,
4141
std::vector<UnresolvedManeuverOverride> &maneuver_overrides,
42-
const TrafficSignals &traffic_signals,
42+
TrafficSignals &traffic_signals,
4343
std::unordered_set<NodeID> &&barriers,
4444
std::vector<util::Coordinate> &&coordinates,
4545
extractor::PackedOSMIDs &&osm_node_ids,
@@ -71,7 +71,7 @@ class NodeBasedGraphFactory
7171
void Compress(ScriptingEnvironment &scripting_environment,
7272
std::vector<TurnRestriction> &turn_restrictions,
7373
std::vector<UnresolvedManeuverOverride> &maneuver_overrides,
74-
const TrafficSignals &traffic_signals);
74+
TrafficSignals &traffic_signals);
7575

7676
// Most ways are bidirectional, making the geometry in forward and backward direction the same,
7777
// except for reversal. We make use of this fact by keeping only one representation of the

‎include/extractor/traffic_signals.hpp‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,21 @@ struct TrafficSignals
1919
{
2020
return bidirectional_nodes.count(to) > 0 || unidirectional_segments.count({from, to}) > 0;
2121
}
22+
23+
void Compress(NodeID from, NodeID via, NodeID to)
24+
{
25+
bidirectional_nodes.erase(via);
26+
if (unidirectional_segments.count({via, to}))
27+
{
28+
unidirectional_segments.erase({via, to});
29+
unidirectional_segments.insert({from, to});
30+
}
31+
if (unidirectional_segments.count({via, from}))
32+
{
33+
unidirectional_segments.erase({via, from});
34+
unidirectional_segments.insert({to, from});
35+
}
36+
}
2237
};
2338
} // namespace osrm::extractor
2439

‎src/extractor/edge_based_graph_factory.cpp‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -608,26 +608,12 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges(
608608
BOOST_ASSERT(!edge_data1.reversed);
609609
BOOST_ASSERT(!edge_data2.reversed);
610610

611-
// We write out the mapping between the edge-expanded edges and the original nodes.
612-
// Since each edge represents a possible maneuver, external programs can use this to
613-
// quickly perform updates to edge weights in order to penalize certain turns.
614-
615-
// If this edge is 'trivial' -- where the compressed edge corresponds exactly to an
616-
// original OSM segment -- we can pull the turn's preceding node ID directly with
617-
// `node_along_road_entering`;
618-
// otherwise, we need to look up the node immediately preceding the turn from the
619-
// compressed edge container.
620-
const bool isTrivial = m_compressed_edge_container.IsTrivial(node_based_edge_from);
621-
622-
const auto &from_node =
623-
isTrivial ? node_along_road_entering
624-
: m_compressed_edge_container.GetLastEdgeSourceID(node_based_edge_from);
625-
626611
// compute weight and duration penalties
627612
// In theory we shouldn't get a directed traffic light on a turn, as it indicates that
628613
// the traffic signal direction was potentially ambiguously annotated on the junction
629614
// node But we'll check anyway.
630-
const auto is_traffic_light = m_traffic_signals.HasSignal(from_node, intersection_node);
615+
const auto is_traffic_light =
616+
m_traffic_signals.HasSignal(node_along_road_entering, intersection_node);
631617
const auto is_uturn =
632618
guidance::getTurnDirection(turn_angle) == guidance::DirectionModifier::UTurn;
633619

@@ -694,6 +680,21 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges(
694680
true,
695681
false};
696682

683+
// We write out the mapping between the edge-expanded edges and the original nodes.
684+
// Since each edge represents a possible maneuver, external programs can use this to
685+
// quickly perform updates to edge weights in order to penalize certain turns.
686+
687+
// If this edge is 'trivial' -- where the compressed edge corresponds exactly to an
688+
// original OSM segment -- we can pull the turn's preceding node ID directly with
689+
// `node_along_road_entering`;
690+
// otherwise, we need to look up the node immediately preceding the turn from the
691+
// compressed edge container.
692+
const bool isTrivial = m_compressed_edge_container.IsTrivial(node_based_edge_from);
693+
694+
const auto &from_node =
695+
isTrivial ? node_along_road_entering
696+
: m_compressed_edge_container.GetLastEdgeSourceID(node_based_edge_from);
697+
697698
const auto &to_node =
698699
m_compressed_edge_container.GetFirstEdgeTargetID(node_based_edge_to);
699700

‎src/extractor/graph_compressor.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ namespace osrm::extractor
2020
static constexpr int SECOND_TO_DECISECOND = 10;
2121

2222
void GraphCompressor::Compress(const std::unordered_set<NodeID> &barrier_nodes,
23-
const TrafficSignals &traffic_signals,
23+
TrafficSignals &traffic_signals,
2424
ScriptingEnvironment &scripting_environment,
2525
std::vector<TurnRestriction> &turn_restrictions,
2626
std::vector<UnresolvedManeuverOverride> &maneuver_overrides,
@@ -338,6 +338,9 @@ void GraphCompressor::Compress(const std::unordered_set<NodeID> &barrier_nodes,
338338
// update any involved turn relations
339339
turn_path_compressor.Compress(node_u, node_v, node_w);
340340

341+
// Update traffic signal paths containing compressed node.
342+
traffic_signals.Compress(node_u, node_v, node_w);
343+
341344
// Forward and reversed compressed edge lengths need to match.
342345
// Set a dummy empty penalty weight if opposite value exists.
343346
auto set_dummy_penalty = [](EdgeWeight &weight_penalty,

‎src/extractor/node_based_graph_factory.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ NodeBasedGraphFactory::NodeBasedGraphFactory(
1717
ScriptingEnvironment &scripting_environment,
1818
std::vector<TurnRestriction> &turn_restrictions,
1919
std::vector<UnresolvedManeuverOverride> &maneuver_overrides,
20-
const TrafficSignals &traffic_signals,
20+
TrafficSignals &traffic_signals,
2121
std::unordered_set<NodeID> &&barriers,
2222
std::vector<util::Coordinate> &&coordinates,
2323
extractor::PackedOSMIDs &&osm_node_ids,
@@ -72,7 +72,7 @@ void NodeBasedGraphFactory::BuildCompressedOutputGraph(const std::vector<NodeBas
7272
void NodeBasedGraphFactory::Compress(ScriptingEnvironment &scripting_environment,
7373
std::vector<TurnRestriction> &turn_restrictions,
7474
std::vector<UnresolvedManeuverOverride> &maneuver_overrides,
75-
const TrafficSignals &traffic_signals)
75+
TrafficSignals &traffic_signals)
7676
{
7777
GraphCompressor graph_compressor;
7878
graph_compressor.Compress(barriers,

0 commit comments

Comments
 (0)