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,