From ad9ef2c6bcf5118a4cd3a0aec31fe2df3b0d2a5b Mon Sep 17 00:00:00 2001 From: Michael Bell Date: Sat, 16 Mar 2024 19:29:36 +0000 Subject: [PATCH] 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). --- CHANGELOG.md | 1 + features/car/traffic_light_penalties.feature | 204 ++++++++++++++---- include/extractor/graph_compressor.hpp | 2 +- .../extractor/node_based_graph_factory.hpp | 4 +- include/extractor/traffic_signals.hpp | 15 ++ src/extractor/edge_based_graph_factory.cpp | 33 +-- src/extractor/graph_compressor.cpp | 5 +- src/extractor/node_based_graph_factory.cpp | 4 +- 8 files changed, 204 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c4f43265349..aec4f0d6616 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ - FIXED: Bicycle and foot profiles now don't route on proposed ways [#6615](https://github.com/Project-OSRM/osrm-backend/pull/6615) - Routing: - FIXED: Fix adding traffic signal penalties during compression [#6419](https://github.com/Project-OSRM/osrm-backend/pull/6419) + - FIXED: Correctly handle compressed traffic signals. [#6724](https://github.com/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 index 048c25f4fa7..2a5ded5f02e 100644 --- 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 index ba518e210b9..960bd64ff22 100644 --- 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 &barrier_nodes, - const TrafficSignals &traffic_signals, + TrafficSignals &traffic_signals, ScriptingEnvironment &scripting_environment, std::vector &turn_restrictions, std::vector &maneuver_overrides, diff --git a/include/extractor/node_based_graph_factory.hpp b/include/extractor/node_based_graph_factory.hpp index 29cc25ac42d..c69842d3c63 100644 --- 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 &turn_restrictions, std::vector &maneuver_overrides, - const TrafficSignals &traffic_signals, + TrafficSignals &traffic_signals, std::unordered_set &&barriers, std::vector &&coordinates, extractor::PackedOSMIDs &&osm_node_ids, @@ -71,7 +71,7 @@ class NodeBasedGraphFactory void Compress(ScriptingEnvironment &scripting_environment, std::vector &turn_restrictions, std::vector &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 index a70d67d5491..739b57bcb7b 100644 --- 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 index 0e44276f20a..cfb29e6c103 100644 --- 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 index 9c933ef4f01..cee5afe929a 100644 --- 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 &barrier_nodes, - const TrafficSignals &traffic_signals, + TrafficSignals &traffic_signals, ScriptingEnvironment &scripting_environment, std::vector &turn_restrictions, std::vector &maneuver_overrides, @@ -338,6 +338,9 @@ void GraphCompressor::Compress(const std::unordered_set &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 index 9819a977898..045e495436b 100644 --- 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 &turn_restrictions, std::vector &maneuver_overrides, - const TrafficSignals &traffic_signals, + TrafficSignals &traffic_signals, std::unordered_set &&barriers, std::vector &&coordinates, extractor::PackedOSMIDs &&osm_node_ids, @@ -72,7 +72,7 @@ void NodeBasedGraphFactory::BuildCompressedOutputGraph(const std::vector &turn_restrictions, std::vector &maneuver_overrides, - const TrafficSignals &traffic_signals) + TrafficSignals &traffic_signals) { GraphCompressor graph_compressor; graph_compressor.Compress(barriers,