Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a bug caused by support OSM traffic signal directions #6724

Merged
merged 1 commit into from
Mar 17, 2024
+204 −64
Merged

Fix a bug caused by support OSM traffic signal directions #6724

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
204 changes: 162 additions & 42 deletions features/car/traffic_light_penalties.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

"""

Expand All @@ -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
Expand Down Expand Up @@ -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 |
2 changes: 1 addition & 1 deletion include/extractor/graph_compressor.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions include/extractor/node_based_graph_factory.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions include/extractor/traffic_signals.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
33 changes: 17 additions & 16 deletions src/extractor/edge_based_graph_factory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);

Expand Down
5 changes: 4 additions & 1 deletion src/extractor/graph_compressor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions src/extractor/node_based_graph_factory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Loading