Correctly handle compressed traffic signals (#6724)

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). Co-authored-by: Michael Bell <[email protected]>
Project-OSRM · Mar 17, 2024 · 6e77d53 · 6e77d53
1 parent 99875b4
commit 6e77d53
Show file tree

Hide file tree

Showing 8 changed files with 204 additions and 64 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -35,6 +35,7 @@
  - FIXED: Bicycle and foot profiles now don't route on proposed ways [#6615](https:/Project-OSRM/osrm-backend/pull/6615)
  - Routing:
  - FIXED: Fix adding traffic signal penalties during compression [#6419](https:/Project-OSRM/osrm-backend/pull/6419)
+ - FIXED: Correctly handle compressed traffic signals. [#6724](https:/Project-OSRM/osrm-backend/pull/6724)
 # 5.27.1
  - Changes from 5.27.0
  - Misc:

diff --git a/features/car/traffic_light_penalties.feature b/features/car/traffic_light_penalties.feature
@@ -32,14 +32,14 @@ Feature: Car - Handle traffic lights
  | l | traffic_signals |
 
  When I route I should get
- | from | to | time | # |
+ | from | to | time | #  |
  | 1 | 2 | 11.1s | no turn with no traffic light |
  | 3 | 4 | 13.1s | no turn with traffic light |
  | g | j | 18.7s | turn with no traffic light |
  | k | n | 20.7s | turn with traffic light |
 
 
- Scenario: Car - Traffic signal direction
+ Scenario: Car - Traffic signal direction straight
  Given the node map
  """
  a-1-b-2-c
@@ -112,14 +112,14 @@ Feature: Car - Handle traffic lights
 
 
 
- Scenario: Car - Encounters a traffic light
+ Scenario: Car - Encounters a traffic light direction
  Given the node map
  """
- a f k
- | | |
- b-c-d h-g-i l-m-n
- | | |
- e j o
+ a f k p
+ | | | |
+ b-c-d h-g-i l-m-n q-r-s
+ | | | |
+ e j o t
 
  """
 
@@ -131,53 +131,70 @@ Feature: Car - Handle traffic lights
  | fgj | primary |
  | lmn | primary |
  | kmo | primary |
+ | qrs | primary |
+ | prt | primary |
 
  And the nodes
  | node | highway | traffic_signals:direction |
- | g | traffic_signals | forward |
- | m | traffic_signals | backward |
+ | g | traffic_signals | |
+ | m | traffic_signals | forward |
+ | r | traffic_signals | backward |
 
 
  When I route I should get
+ # Base case
  | from | to | time | # |
- | a | d | 21.9s | no turn with no traffic light |
- | a | e | 22.2s | no turn with traffic light |
  | a | b | 18.7s | turn with no traffic light |
- | e | b | 21.9s | no turn with no traffic light |
- | e | a | 22.2s | no turn with traffic light |
+ | a | e | 22.2s | no turn with no traffic light |
+ | a | d | 21.9s | turn with no traffic light |
+ | e | b | 21.9s | turn with no traffic light |
+ | e | a | 22.2s | no turn with no traffic light |
  | e | d | 18.7s | turn with no traffic light |
- | d | e | 21.9s | no turn with no traffic light |
- | d | b | 11s | no turn with traffic light  |
+ | d | e | 21.9s | turn with no traffic light  |
+ | d | b | 11s | no turn with no traffic light |
  | d | a | 18.7s | turn with no traffic light |
- | b | a | 21.9s | no turn with no traffic light |
- | b | d | 11s | no turn with traffic light  |
+ | b | a | 21.9s | turn with no traffic light  |
+ | b | d | 11s | no turn with no traffic light |
  | b | e | 18.7s | turn with no traffic light |
-
- | f | i | 23.9s | no turn with no traffic light |
+ # All have traffic lights - 2s penalty
+ | f | h | 20.7s | turn with traffic light  |
  | f | j | 24.2s | no turn with traffic light |
- | f | h | 20.7s | turn with no traffic light |
- | j | h | 21.9s | no turn with no traffic light |
- | j | f | 22.2s | no turn with traffic light |
- | j | i | 18.7s | turn with no traffic light |
- | i | j | 21.9s | no turn with no traffic light |
- | i | h | 11s | no turn with traffic light |
- | i | f | 18.7s | turn with no traffic light |
- | h | f | 23.9s | no turn with no traffic light |
+ | f | i | 23.9s | turn with traffic light  |
+ | j | h | 23.9s | turn with traffic light  |
+ | j | f | 24.2s | no turn with traffic light |
+ | j | i | 20.7s | turn with traffic light  |
+ | i | j | 23.9s | turn with traffic light  |
+ | i | h | 13s | no turn with traffic light |
+ | i | f | 20.7s | turn with traffic light  |
+ | h | f | 23.9s | turn with traffic light  |
  | h | i | 13s | no turn with traffic light |
- | h | j | 20.7s | turn with no traffic light |
-
- | k | n | 21.9s | no turn with no traffic light |
- | k | o | 22.2s | no turn with traffic light |
- | k | l | 18.7s | turn with no traffic light |
- | o | l | 23.9s | no turn with no traffic light |
- | o | k | 24.2s | no turn with traffic light |
- | o | n | 20.7s | turn with no traffic light |
- | n | o | 23.9s | no turn with no traffic light |
- | n | l | 13s | no turn with traffic light |
- | n | k | 20.7s | turn with no traffic light |
- | l | k | 21.9s | no turn with no traffic light |
- | l | n | 11s | no turn with traffic light |
- | l | o | 18.7s | turn with no traffic light |
+ | h | j | 20.7s | turn with traffic light |
+ # Front direction have traffic lights - 2s penalty
+ | k | l | 20.7s | turn with traffic light |
+ | k | o | 24.2s | no turn with traffic light |
+ | k | n | 23.9s | turn with traffic light |
+ | o | l | 21.9s | turn with no traffic light |
+ | o | k | 22.2s | no turn with no traffic light |
+ | o | n | 18.7s | turn with no traffic light |
+ | n | o | 21.9s | turn with no traffic light |
+ | n | l | 11s | no turn with no traffic light |
+ | n | k | 18.7s | turn with no traffic light |
+ | l | k | 23.9s | turn with traffic light |
+ | l | n | 13s | no turn with traffic light |
+ | l | o | 20.7s | turn with traffic light |
+ # Reverse direction have traffic lights - 2s penalty
+ | p | q | 18.7s | turn with no traffic light |
+ | p | t | 22.2s | no turn with no traffic light |
+ | p | s | 21.9s | turn with no traffic light |
+ | t | q | 23.9s | turn with traffic light |
+ | t | p | 24.2s | no turn with traffic light |
+ | t | s | 20.7s | turn with traffic light |
+ | s | t | 23.9s | turn with traffic light |
+ | s | q | 13s | no turn with traffic light |
+ | s | p | 20.7s | turn with traffic light |
+ | q | p | 21.9s | turn with no traffic light |
+ | q | s | 11s | no turn with no traffic light |
+ | q | t | 18.7s | turn with no traffic light |
 
 
  Scenario: Traffic Signal Geometry
@@ -343,3 +360,106 @@ Feature: Car - Handle traffic lights
  | from | to | route | speed | weights | time | distances | a:datasources | a:nodes | a:speed | a:duration | a:weight |
  | 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 |
  | 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 |
+
+
+ Scenario: Car - Traffic signal straight direction with edge compression
+ Given the node map
+ """
+ a-1-b - c - d-2-e
+
+ """
+
+ And the ways
+ | nodes | highway |
+ | abcde | primary |
+
+ And the nodes
+ | node | highway | traffic_signals:direction |
+ | c | traffic_signals | forward |
+
+ When I route I should get
+ | from | to | time | weight | # |
+ | 1 | 2 | 35.3s | 35.3 | no turn with traffic light |
+ | 2 | 1 | 33.3s | 33.3 | no turn with no traffic light |
+
+
+ Scenario: Car - Traffic signal turn direction with edge compression
+ Given the node map
+ """
+ d
+ |
+ 2
+ |
+ a-1-b - c - f
+ |
+ e
+
+ j
+ |
+ 4
+ |
+ g-3-h - i - k
+ |
+ l
+
+ """
+
+ And the ways
+ | nodes | highway |
+ | abc | primary |
+ | cf | primary |
+ | fd | primary |
+ | fe | primary |
+ | ghi | primary |
+ | ik | primary |
+ | kj | primary |
+ | kl | primary |
+
+ And the nodes
+ | node | highway | traffic_signals:direction |
+ | k | traffic_signals | forward |
+
+ When I route I should get
+ | from | to | time | weight | # |
+ | 1 | 2 | 44.2s | 44.2 | turn with no traffic light |
+ | 2 | 1 | 41s | 41 | turn with no traffic light |
+ | 3 | 4 | 46.2s | 46.2 | turn with traffic light |
+ | 4 | 3 | 41s | 41 | turn with no traffic light |
+
+
+ Scenario: Car - Traffic signal turn direction with turn restriction
+ Given the node map
+ """
+ d
+ |
+ 2
+ |
+ a-1-b - c - f
+ |
+ e
+
+ """
+
+ And the ways
+ | nodes | highway |
+ | abc | primary |
+ | cf | primary |
+ | fd | primary |
+ | fe | primary |
+
+ And the nodes
+ | node | highway | traffic_signals:direction |
+ | f | traffic_signals | forward |
+
+ And the relations
+ | type | way:from | way:to | way:via | restriction |
+ | restriction | abc | fe | cf | no_right_turn |
+
+ And the relations
+ | type | way:from | way:to | node:via | restriction |
+ | restriction | df | fc | f | right_turn_only |
+
+ When I route I should get
+ | from | to | time | weight | # |
+ | 1 | 2 | 46.2s | 46.2 | turn with traffic light |
+ | 2 | 1 | 41s | 41 | turn with no traffic light |
diff --git a/include/extractor/graph_compressor.hpp b/include/extractor/graph_compressor.hpp
@@ -24,7 +24,7 @@ class GraphCompressor
 
  public:
  void Compress(const std::unordered_set<NodeID> &barrier_nodes,
- const TrafficSignals &traffic_signals,
+ TrafficSignals &traffic_signals,
  ScriptingEnvironment &scripting_environment,
  std::vector<TurnRestriction> &turn_restrictions,
  std::vector<UnresolvedManeuverOverride> &maneuver_overrides,

diff --git a/include/extractor/node_based_graph_factory.hpp b/include/extractor/node_based_graph_factory.hpp
@@ -39,7 +39,7 @@ class NodeBasedGraphFactory
  NodeBasedGraphFactory(ScriptingEnvironment &scripting_environment,
  std::vector<TurnRestriction> &turn_restrictions,
  std::vector<UnresolvedManeuverOverride> &maneuver_overrides,
- const TrafficSignals &traffic_signals,
+ TrafficSignals &traffic_signals,
  std::unordered_set<NodeID> &&barriers,
  std::vector<util::Coordinate> &&coordinates,
  extractor::PackedOSMIDs &&osm_node_ids,
@@ -71,7 +71,7 @@ class NodeBasedGraphFactory
  void Compress(ScriptingEnvironment &scripting_environment,
  std::vector<TurnRestriction> &turn_restrictions,
  std::vector<UnresolvedManeuverOverride> &maneuver_overrides,
- const TrafficSignals &traffic_signals);
+ TrafficSignals &traffic_signals);
 
  // Most ways are bidirectional, making the geometry in forward and backward direction the same,
  // except for reversal. We make use of this fact by keeping only one representation of the

diff --git a/include/extractor/traffic_signals.hpp b/include/extractor/traffic_signals.hpp
@@ -19,6 +19,21 @@ struct TrafficSignals
  {
  return bidirectional_nodes.count(to) > 0 || unidirectional_segments.count({from, to}) > 0;
  }
+
+ void Compress(NodeID from, NodeID via, NodeID to)
+ {
+ bidirectional_nodes.erase(via);
+ if (unidirectional_segments.count({via, to}))
+ {
+ unidirectional_segments.erase({via, to});
+ unidirectional_segments.insert({from, to});
+ }
+ if (unidirectional_segments.count({via, from}))
+ {
+ unidirectional_segments.erase({via, from});
+ unidirectional_segments.insert({to, from});
+ }
+ }
 };
 } // namespace osrm::extractor
 

diff --git a/src/extractor/edge_based_graph_factory.cpp b/src/extractor/edge_based_graph_factory.cpp
@@ -608,26 +608,12 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges(
  BOOST_ASSERT(!edge_data1.reversed);
  BOOST_ASSERT(!edge_data2.reversed);
 
- // We write out the mapping between the edge-expanded edges and the original nodes.
- // Since each edge represents a possible maneuver, external programs can use this to
- // quickly perform updates to edge weights in order to penalize certain turns.
-
- // If this edge is 'trivial' -- where the compressed edge corresponds exactly to an
- // original OSM segment -- we can pull the turn's preceding node ID directly with
- // `node_along_road_entering`;
- // otherwise, we need to look up the node immediately preceding the turn from the
- // compressed edge container.
- const bool isTrivial = m_compressed_edge_container.IsTrivial(node_based_edge_from);
-
- const auto &from_node =
- isTrivial ? node_along_road_entering
- : m_compressed_edge_container.GetLastEdgeSourceID(node_based_edge_from);
-
  // compute weight and duration penalties
  // In theory we shouldn't get a directed traffic light on a turn, as it indicates that
  // the traffic signal direction was potentially ambiguously annotated on the junction
  // node But we'll check anyway.
- const auto is_traffic_light = m_traffic_signals.HasSignal(from_node, intersection_node);
+ const auto is_traffic_light =
+ m_traffic_signals.HasSignal(node_along_road_entering, intersection_node);
  const auto is_uturn =
  guidance::getTurnDirection(turn_angle) == guidance::DirectionModifier::UTurn;
 
@@ -694,6 +680,21 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges(
  true,
  false};
 
+ // We write out the mapping between the edge-expanded edges and the original nodes.
+ // Since each edge represents a possible maneuver, external programs can use this to
+ // quickly perform updates to edge weights in order to penalize certain turns.
+
+ // If this edge is 'trivial' -- where the compressed edge corresponds exactly to an
+ // original OSM segment -- we can pull the turn's preceding node ID directly with
+ // `node_along_road_entering`;
+ // otherwise, we need to look up the node immediately preceding the turn from the
+ // compressed edge container.
+ const bool isTrivial = m_compressed_edge_container.IsTrivial(node_based_edge_from);
+
+ const auto &from_node =
+ isTrivial ? node_along_road_entering
+ : m_compressed_edge_container.GetLastEdgeSourceID(node_based_edge_from);
+
  const auto &to_node =
  m_compressed_edge_container.GetFirstEdgeTargetID(node_based_edge_to);
 

diff --git a/src/extractor/graph_compressor.cpp b/src/extractor/graph_compressor.cpp
@@ -20,7 +20,7 @@ namespace osrm::extractor
 static constexpr int SECOND_TO_DECISECOND = 10;
 
 void GraphCompressor::Compress(const std::unordered_set<NodeID> &barrier_nodes,
- const TrafficSignals &traffic_signals,
+ TrafficSignals &traffic_signals,
  ScriptingEnvironment &scripting_environment,
  std::vector<TurnRestriction> &turn_restrictions,
  std::vector<UnresolvedManeuverOverride> &maneuver_overrides,
@@ -338,6 +338,9 @@ void GraphCompressor::Compress(const std::unordered_set<NodeID> &barrier_nodes,
  // update any involved turn relations
  turn_path_compressor.Compress(node_u, node_v, node_w);
 
+ // Update traffic signal paths containing compressed node.
+ traffic_signals.Compress(node_u, node_v, node_w);
+
  // Forward and reversed compressed edge lengths need to match.
  // Set a dummy empty penalty weight if opposite value exists.
  auto set_dummy_penalty = [](EdgeWeight &weight_penalty,

diff --git a/src/extractor/node_based_graph_factory.cpp b/src/extractor/node_based_graph_factory.cpp
@@ -17,7 +17,7 @@ NodeBasedGraphFactory::NodeBasedGraphFactory(
  ScriptingEnvironment &scripting_environment,
  std::vector<TurnRestriction> &turn_restrictions,
  std::vector<UnresolvedManeuverOverride> &maneuver_overrides,
- const TrafficSignals &traffic_signals,
+ TrafficSignals &traffic_signals,
  std::unordered_set<NodeID> &&barriers,
  std::vector<util::Coordinate> &&coordinates,
  extractor::PackedOSMIDs &&osm_node_ids,
@@ -72,7 +72,7 @@ void NodeBasedGraphFactory::BuildCompressedOutputGraph(const std::vector<NodeBas
 void NodeBasedGraphFactory::Compress(ScriptingEnvironment &scripting_environment,
  std::vector<TurnRestriction> &turn_restrictions,
  std::vector<UnresolvedManeuverOverride> &maneuver_overrides,
- const TrafficSignals &traffic_signals)
+ TrafficSignals &traffic_signals)
 {
  GraphCompressor graph_compressor;
  graph_compressor.Compress(barriers,