From e7acc9df7649e87e6035770900537ea9046720b8 Mon Sep 17 00:00:00 2001 From: themylogin Date: Wed, 4 Jul 2018 22:14:58 +0300 Subject: [PATCH 1/3] Fix turn.roads_on_the_left and turn.roads_on_the right for two-way roads #5128 --- CHANGELOG.md | 1 + .../options/extract/turn_function.feature | 27 +++++++ src/extractor/edge_based_graph_factory.cpp | 78 +++++++++++++------ 3 files changed, 81 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ff20e4f62f..ffdab7cf06d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - FIXED: Reduce copying in API parameter constructors [#5925](https://github.com/Project-OSRM/osrm-backend/pull/5925) - Misc: - CHANGED: Unify `.osrm.turn_penalites_index` dump processing same with `.osrm.turn_weight_penalties` and `.osrm.turn_duration_penalties` [#5868](https://github.com/Project-OSRM/osrm-backend/pull/5868) + - FIXED: turn.roads_on_the_left not containing incoming roads and turn.roads_on_the_right not containing outgoing roads on two-way roads [#5128](https://github.com/Project-OSRM/osrm-backend/issues/5128) - Profile: - ADDED: Profile debug script which fetches a way from OSM then outputs the result of the profile. [#5908](https://github.com/Project-OSRM/osrm-backend/pull/5908) - Infrastructure diff --git a/features/options/extract/turn_function.feature b/features/options/extract/turn_function.feature index a1db9339f92..eecb2960492 100644 --- a/features/options/extract/turn_function.feature +++ b/features/options/extract/turn_function.feature @@ -180,3 +180,30 @@ Feature: Turn Function Information And stdout should contain /roads_on_the_right \[1\] speed: [0-9]+, is_incoming: true, is_outgoing: false, highway_turn_classification: 3, access_turn_classification: 0/ # turning abc, give information about about db And stdout should contain /roads_on_the_left \[1\] speed: [0-9]+, is_incoming: true, is_outgoing: false, highway_turn_classification: 0, access_turn_classification: 1/ + + Scenario: Turns should have correct information of two-way roads at intersection + Given the node map + """ + b + | + a-c-d + | + e + """ + And the ways + | nodes | highway | oneway | + | ac | motorway | yes | + | cd | motorway_link | yes | + | bc | trunk | yes | + | cb | trunk_link | yes | + | ce | primary | yes | + | ec | primary_link | yes | + And the data has been saved to disk + + When I run "osrm-extract --profile {profile_file} {osm_file}" + Then it should exit successfully + # Turn acd + # on the left there should be cb (and bc) + And stdout should contain /roads_on_the_left \[1\] speed: [0-9]+, is_incoming: true, is_outgoing: true, highway_turn_classification: [0-9]+, access_turn_classification: 0, priority_class: 3/ + # on the right there should be ce and ec + And stdout should contain /roads_on_the_right \[1\] speed: [0-9]+, is_incoming: true, is_outgoing: true, highway_turn_classification: [0-9]+, access_turn_classification: 0, priority_class: 4/ diff --git a/src/extractor/edge_based_graph_factory.cpp b/src/extractor/edge_based_graph_factory.cpp index ba9b1e85bf2..4a73d1d3df3 100644 --- a/src/extractor/edge_based_graph_factory.cpp +++ b/src/extractor/edge_based_graph_factory.cpp @@ -714,16 +714,15 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges( { ++node_based_edge_counter; - const auto intersection_view = - convertToIntersectionView(m_node_based_graph, - m_edge_based_node_container, - unconditional_node_restriction_map, - m_barrier_nodes, - edge_geometries, - turn_lanes_data, - incoming_edge, - outgoing_edges, - merged_edge_ids); + const auto connected_roads = + extractor::intersection::getConnectedRoads(m_node_based_graph, + m_edge_based_node_container, + m_coordinates, + m_compressed_edge_container, + unconditional_node_restriction_map, + m_barrier_nodes, + turn_lanes_data, + incoming_edge); // check if this edge is part of a restriction via-way const auto is_restriction_via_edge = @@ -746,12 +745,12 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges( continue; const auto turn = - std::find_if(intersection_view.begin(), - intersection_view.end(), + std::find_if(connected_roads.begin(), + connected_roads.end(), [edge = outgoing_edge.edge](const auto &road) { return road.eid == edge; }); - OSRM_ASSERT(turn != intersection_view.end(), + OSRM_ASSERT(turn != connected_roads.end(), m_coordinates[intersection_node]); std::vector road_legs_on_the_right; @@ -760,6 +759,37 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges( auto get_connected_road_info = [&](const auto &connected_edge) { const auto &edge_data = m_node_based_graph.GetEdgeData(connected_edge.eid); + + bool is_incoming, is_outgoing; + if (edge_data.reversed) + { + // If getConnectedRoads adds reversed edge it means + // this edge is incoming-only + is_incoming = true; + is_outgoing = false; + } + else + { + // It does not add incoming edge if there is outgoing so we should + // find it ourselves + is_incoming = false; + auto reversed_edge = m_node_based_graph.FindEdge( + m_node_based_graph.GetTarget(connected_edge.eid), + intersection_node); + if (reversed_edge != SPECIAL_EDGEID) + { + const auto &reversed_edge_data = + m_node_based_graph.GetEdgeData(reversed_edge); + + if (!reversed_edge_data.reversed) + { + is_incoming = true; + } + } + + is_outgoing = true; + } + return ExtractionTurnLeg( edge_data.flags.restricted, edge_data.flags.road_classification.IsMotorwayClass(), @@ -772,10 +802,8 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges( edge_data.duration) * 36, edge_data.flags.road_classification.GetPriority(), - !connected_edge.entry_allowed || - (edge_data.flags.forward && - edge_data.flags.backward), // is incoming - connected_edge.entry_allowed); + is_incoming, + is_outgoing); }; // all connected roads on the right of a u turn @@ -783,35 +811,35 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges( guidance::DirectionModifier::UTurn; if (is_uturn) { - if (turn != intersection_view.begin()) + if (turn != connected_roads.begin()) { - std::transform(intersection_view.begin() + 1, + std::transform(connected_roads.begin() + 1, turn, std::back_inserter(road_legs_on_the_right), get_connected_road_info); } std::transform(turn + 1, - intersection_view.end(), + connected_roads.end(), std::back_inserter(road_legs_on_the_right), get_connected_road_info); } else { - if (intersection_view.begin() != turn) + if (connected_roads.begin() != turn) { - std::transform(intersection_view.begin() + 1, + std::transform(connected_roads.begin() + 1, turn, std::back_inserter(road_legs_on_the_right), get_connected_road_info); } std::transform(turn + 1, - intersection_view.end(), + connected_roads.end(), std::back_inserter(road_legs_on_the_left), get_connected_road_info); } - if (is_uturn && turn != intersection_view.begin()) + if (is_uturn && turn != connected_roads.begin()) { util::Log(logWARNING) << "Turn is a u turn but not turning to the first connected " @@ -819,7 +847,7 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges( << intersection_node << ", OSM link: " << toOSMLink(m_coordinates[intersection_node]); } - else if (turn == intersection_view.begin() && !is_uturn) + else if (turn == connected_roads.begin() && !is_uturn) { util::Log(logWARNING) << "Turn is a u turn but not classified as a u turn. Node ID: " From d9d873903feacf4c96d057f04e7be71bcb163fd3 Mon Sep 17 00:00:00 2001 From: themylogin Date: Sun, 16 Dec 2018 17:49:13 +0100 Subject: [PATCH 2/3] Account merged edges when processing turn legs --- .../intersection/intersection_analysis.hpp | 9 ++++ src/extractor/edge_based_graph_factory.cpp | 16 +++---- .../intersection/intersection_analysis.cpp | 45 ++++++++++++++----- 3 files changed, 51 insertions(+), 19 deletions(-) diff --git a/include/extractor/intersection/intersection_analysis.hpp b/include/extractor/intersection/intersection_analysis.hpp index 6dafcb7bc26..df5f51992ee 100644 --- a/include/extractor/intersection/intersection_analysis.hpp +++ b/include/extractor/intersection/intersection_analysis.hpp @@ -70,6 +70,15 @@ IntersectionView getConnectedRoads(const util::NodeBasedDynamicGraph &graph, const TurnLanesIndexedArray &turn_lanes_data, const IntersectionEdge &incoming_edge); +IntersectionView getConnectedRoadsForEdgeGeometries(const util::NodeBasedDynamicGraph &graph, + const EdgeBasedNodeDataContainer &node_data_container, + const RestrictionMap &node_restriction_map, + const std::unordered_set &barrier_nodes, + const TurnLanesIndexedArray &turn_lanes_data, + const IntersectionEdge &incoming_edge, + const IntersectionEdgeGeometries &edge_geometries, + const std::unordered_set &merged_edge_ids); + // Graph Compression cannot compress every setting. For example any barrier/traffic light cannot // be compressed. As a result, a simple road of the form `a ----- b` might end up as having an // intermediate intersection, if there is a traffic light in between. If we want to look farther diff --git a/src/extractor/edge_based_graph_factory.cpp b/src/extractor/edge_based_graph_factory.cpp index 4a73d1d3df3..06a1ce15f7d 100644 --- a/src/extractor/edge_based_graph_factory.cpp +++ b/src/extractor/edge_based_graph_factory.cpp @@ -715,14 +715,14 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges( ++node_based_edge_counter; const auto connected_roads = - extractor::intersection::getConnectedRoads(m_node_based_graph, - m_edge_based_node_container, - m_coordinates, - m_compressed_edge_container, - unconditional_node_restriction_map, - m_barrier_nodes, - turn_lanes_data, - incoming_edge); + extractor::intersection::getConnectedRoadsForEdgeGeometries(m_node_based_graph, + m_edge_based_node_container, + unconditional_node_restriction_map, + m_barrier_nodes, + turn_lanes_data, + incoming_edge, + edge_geometries, + merged_edge_ids); // check if this edge is part of a restriction via-way const auto is_restriction_via_edge = diff --git a/src/extractor/intersection/intersection_analysis.cpp b/src/extractor/intersection/intersection_analysis.cpp index e96a814fb14..ded594ac6f7 100644 --- a/src/extractor/intersection/intersection_analysis.cpp +++ b/src/extractor/intersection/intersection_analysis.cpp @@ -741,36 +741,59 @@ IntersectionView getConnectedRoads(const util::NodeBasedDynamicGraph &graph, const IntersectionEdge &incoming_edge) { const auto intersection_node = graph.GetTarget(incoming_edge.edge); - const auto &outgoing_edges = intersection::getOutgoingEdges(graph, intersection_node); auto edge_geometries = getIntersectionOutgoingGeometries( graph, compressed_geometries, node_coordinates, intersection_node); + auto merged_edge_ids = std::unordered_set(); + + return getConnectedRoadsForEdgeGeometries(graph, + node_data_container, + node_restriction_map, + barrier_nodes, + turn_lanes_data, + incoming_edge, + edge_geometries, + merged_edge_ids); +} + +IntersectionView getConnectedRoadsForEdgeGeometries(const util::NodeBasedDynamicGraph &graph, + const EdgeBasedNodeDataContainer &node_data_container, + const RestrictionMap &node_restriction_map, + const std::unordered_set &barrier_nodes, + const TurnLanesIndexedArray &turn_lanes_data, + const IntersectionEdge &incoming_edge, + const IntersectionEdgeGeometries &edge_geometries, + const std::unordered_set &merged_edge_ids) +{ + const auto intersection_node = graph.GetTarget(incoming_edge.edge); + const auto &outgoing_edges = intersection::getOutgoingEdges(graph, intersection_node); // Add incoming edges with reversed bearings - const auto edges_number = edge_geometries.size(); - edge_geometries.resize(2 * edges_number); + auto processed_edge_geometries = IntersectionEdgeGeometries(edge_geometries); + const auto edges_number = processed_edge_geometries.size(); + processed_edge_geometries.resize(2 * edges_number); for (std::size_t index = 0; index < edges_number; ++index) { - const auto &geometry = edge_geometries[index]; + const auto &geometry = processed_edge_geometries[index]; const auto remote_node = graph.GetTarget(geometry.eid); const auto incoming_edge = graph.FindEdge(remote_node, intersection_node); - edge_geometries[edges_number + index] = {incoming_edge, - util::bearing::reverse(geometry.initial_bearing), - util::bearing::reverse(geometry.perceived_bearing), - geometry.segment_length}; + processed_edge_geometries[edges_number + index] = {incoming_edge, + util::bearing::reverse(geometry.initial_bearing), + util::bearing::reverse(geometry.perceived_bearing), + geometry.segment_length}; } // Enforce ordering of edges by IDs - std::sort(edge_geometries.begin(), edge_geometries.end()); + std::sort(processed_edge_geometries.begin(), processed_edge_geometries.end()); return convertToIntersectionView(graph, node_data_container, node_restriction_map, barrier_nodes, - edge_geometries, + processed_edge_geometries, turn_lanes_data, incoming_edge, outgoing_edges, - std::unordered_set()); + merged_edge_ids); } template IntersectionView From b0b8069ab030590545ff34e22f06592e64019122 Mon Sep 17 00:00:00 2001 From: themylogin Date: Wed, 27 Jan 2021 13:11:54 +0100 Subject: [PATCH 3/3] clang-format --- .../intersection/intersection_analysis.hpp | 17 ++++++------ src/extractor/edge_based_graph_factory.cpp | 21 ++++++++------- .../intersection/intersection_analysis.cpp | 26 ++++++++++--------- 3 files changed, 34 insertions(+), 30 deletions(-) diff --git a/include/extractor/intersection/intersection_analysis.hpp b/include/extractor/intersection/intersection_analysis.hpp index df5f51992ee..aa4a1e939de 100644 --- a/include/extractor/intersection/intersection_analysis.hpp +++ b/include/extractor/intersection/intersection_analysis.hpp @@ -70,14 +70,15 @@ IntersectionView getConnectedRoads(const util::NodeBasedDynamicGraph &graph, const TurnLanesIndexedArray &turn_lanes_data, const IntersectionEdge &incoming_edge); -IntersectionView getConnectedRoadsForEdgeGeometries(const util::NodeBasedDynamicGraph &graph, - const EdgeBasedNodeDataContainer &node_data_container, - const RestrictionMap &node_restriction_map, - const std::unordered_set &barrier_nodes, - const TurnLanesIndexedArray &turn_lanes_data, - const IntersectionEdge &incoming_edge, - const IntersectionEdgeGeometries &edge_geometries, - const std::unordered_set &merged_edge_ids); +IntersectionView +getConnectedRoadsForEdgeGeometries(const util::NodeBasedDynamicGraph &graph, + const EdgeBasedNodeDataContainer &node_data_container, + const RestrictionMap &node_restriction_map, + const std::unordered_set &barrier_nodes, + const TurnLanesIndexedArray &turn_lanes_data, + const IntersectionEdge &incoming_edge, + const IntersectionEdgeGeometries &edge_geometries, + const std::unordered_set &merged_edge_ids); // Graph Compression cannot compress every setting. For example any barrier/traffic light cannot // be compressed. As a result, a simple road of the form `a ----- b` might end up as having an diff --git a/src/extractor/edge_based_graph_factory.cpp b/src/extractor/edge_based_graph_factory.cpp index 06a1ce15f7d..7bd359de1bb 100644 --- a/src/extractor/edge_based_graph_factory.cpp +++ b/src/extractor/edge_based_graph_factory.cpp @@ -715,14 +715,15 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges( ++node_based_edge_counter; const auto connected_roads = - extractor::intersection::getConnectedRoadsForEdgeGeometries(m_node_based_graph, - m_edge_based_node_container, - unconditional_node_restriction_map, - m_barrier_nodes, - turn_lanes_data, - incoming_edge, - edge_geometries, - merged_edge_ids); + extractor::intersection::getConnectedRoadsForEdgeGeometries( + m_node_based_graph, + m_edge_based_node_container, + unconditional_node_restriction_map, + m_barrier_nodes, + turn_lanes_data, + incoming_edge, + edge_geometries, + merged_edge_ids); // check if this edge is part of a restriction via-way const auto is_restriction_via_edge = @@ -770,8 +771,8 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges( } else { - // It does not add incoming edge if there is outgoing so we should - // find it ourselves + // It does not add incoming edge if there is outgoing so we + // should find it ourselves is_incoming = false; auto reversed_edge = m_node_based_graph.FindEdge( m_node_based_graph.GetTarget(connected_edge.eid), diff --git a/src/extractor/intersection/intersection_analysis.cpp b/src/extractor/intersection/intersection_analysis.cpp index ded594ac6f7..d29d38d0a6f 100644 --- a/src/extractor/intersection/intersection_analysis.cpp +++ b/src/extractor/intersection/intersection_analysis.cpp @@ -755,14 +755,15 @@ IntersectionView getConnectedRoads(const util::NodeBasedDynamicGraph &graph, merged_edge_ids); } -IntersectionView getConnectedRoadsForEdgeGeometries(const util::NodeBasedDynamicGraph &graph, - const EdgeBasedNodeDataContainer &node_data_container, - const RestrictionMap &node_restriction_map, - const std::unordered_set &barrier_nodes, - const TurnLanesIndexedArray &turn_lanes_data, - const IntersectionEdge &incoming_edge, - const IntersectionEdgeGeometries &edge_geometries, - const std::unordered_set &merged_edge_ids) +IntersectionView +getConnectedRoadsForEdgeGeometries(const util::NodeBasedDynamicGraph &graph, + const EdgeBasedNodeDataContainer &node_data_container, + const RestrictionMap &node_restriction_map, + const std::unordered_set &barrier_nodes, + const TurnLanesIndexedArray &turn_lanes_data, + const IntersectionEdge &incoming_edge, + const IntersectionEdgeGeometries &edge_geometries, + const std::unordered_set &merged_edge_ids) { const auto intersection_node = graph.GetTarget(incoming_edge.edge); const auto &outgoing_edges = intersection::getOutgoingEdges(graph, intersection_node); @@ -776,10 +777,11 @@ IntersectionView getConnectedRoadsForEdgeGeometries(const util::NodeBasedDynamic const auto &geometry = processed_edge_geometries[index]; const auto remote_node = graph.GetTarget(geometry.eid); const auto incoming_edge = graph.FindEdge(remote_node, intersection_node); - processed_edge_geometries[edges_number + index] = {incoming_edge, - util::bearing::reverse(geometry.initial_bearing), - util::bearing::reverse(geometry.perceived_bearing), - geometry.segment_length}; + processed_edge_geometries[edges_number + index] = { + incoming_edge, + util::bearing::reverse(geometry.initial_bearing), + util::bearing::reverse(geometry.perceived_bearing), + geometry.segment_length}; } // Enforce ordering of edges by IDs