From a0ed70f0a241c97c51fb54c9dbb8ec215e12200f Mon Sep 17 00:00:00 2001 From: Moritz Kobitzsch Date: Fri, 2 Sep 2016 14:06:38 +0200 Subject: [PATCH] don't detect sliproads at wrong locations, don't emit invalid instructions --- CHANGELOG.md | 2 + .../guidance/dedicated-turn-roads.feature | 76 +++++++++++++++++-- src/engine/guidance/post_processing.cpp | 4 +- src/extractor/guidance/sliproad_handler.cpp | 31 +++++--- 4 files changed, 95 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6eafc1fcd..c19d4e351 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ - Fixed a bug that could result in an infinite loop when finding information about an upcoming intersection - Fixed a bug that led to not discovering if a road simply looses a considered prefix - BREAKING: Fixed a bug that could crash postprocessing of instructions on invalid roundabout taggings. This change requires reprocessing datasets with osrm-extract and osrm-contract + - Fixed an issue that could emit `invalid` as instruction when ending on a sliproad after a traffic-light + - Fixed an issue that would detect turning circles as sliproads # 5.3.0 Changes from 5.3.0-rc.3 diff --git a/features/guidance/dedicated-turn-roads.feature b/features/guidance/dedicated-turn-roads.feature index 1f0f1a1cb..ad0e2d68b 100644 --- a/features/guidance/dedicated-turn-roads.feature +++ b/features/guidance/dedicated-turn-roads.feature @@ -176,7 +176,7 @@ Feature: Slipways and Dedicated Turn Lanes | | | | | | | | | | | | | | | | | e | g | | | | | | | | | | | | - | | | | | | | | | | + | | | | | | | 1 | | | | | | | | | | | h | | | | | | | | | | | | | | | | | | | | f | | @@ -194,7 +194,73 @@ Feature: Slipways and Dedicated Turn Lanes | jcghf | primary | Brauerstrasse | yes | When I route I should get - | waypoints | route | turns | - | a,i | Ebertstrasse,Ebertstrasse | depart,arrive | - | a,l | Ebertstrasse,Ebertstrasse | depart,arrive | - | a,f | Ebertstrasse,Brauerstrasse,Brauerstrasse | depart,turn right,arrive | + | waypoints | route | turns | + | a,i | Ebertstrasse,Ebertstrasse | depart,arrive | + | a,l | Ebertstrasse,Ebertstrasse | depart,arrive | + | a,f | Ebertstrasse,Brauerstrasse,Brauerstrasse | depart,turn right,arrive | + | a,1 | Ebertstrasse,, | depart,turn slight right,arrive | + + #2839 + Scenario: Self-Loop + Given the node map + # 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 + | | | | | | | | | | | | | | | | | | | | | | l | | | k | | | | | | | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | j | | | | + | | | | | | | | | | | | | | | | | | | | m | | | | | | | | | | | | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | i | | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | h | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | + | | | | | | | | | | | | | | | | | | | n | | | | | | | | | | | | | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | g | + | | | | | | | | | | | | | | | | | | | o | | | | | | | | | | | | | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | f | | + | | | | | | | | | | | | | | | | | | p | | | | | | | | | | | | | | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | e | | | | + | a | | | | | b | | | | | | | | | c | | | | | | | | | | d | | | | | | | + + And the ways + | nodes | name | oneway | highway | lanes | + | abc | circled | no | residential | 1 | + | cdefghijklmnopc | circled | yes | residential | 1 | + + When I route I should get + | waypoints | bearings | route | turns | + | b,a | 90,10 270,10 | circled,circled | depart,arrive | + + @todo + #due to the current turn function, the left turn bcp is exactly the same cost as pcb, a right turn. The right turn should be way faster,though + #for that reason we cannot distinguish between driving clockwise through the circle or counter-clockwise. Until this is improved, this case here + #has to remain as todo (see #https://github.com/Project-OSRM/osrm-backend/pull/2849) + Scenario: Self-Loop - Bidirectional + Given the node map + # 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 + | | | | | | | | | | | | | | | | | | | | | | l | | | k | | | | | | | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | j | | | | + | | | | | | | | | | | | | | | | | | | | m | | | | | | | | | | | | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | i | | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | h | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | + | | | | | | | | | | | | | | | | | | | n | | | | | | | | | | | | | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | g | + | | | | | | | | | | | | | | | | | | | o | | | | | | | | | | | | | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | f | | + | | | | | | | | | | | | | | | | | | p | | | | | | | | | | | | | | + | | | | | | | | | | | | | | | | | | | | | | | | | | | | e | | | | + | a | | | | | b | | | | | | | | | c | | | | | | | | | | d | | | | | | | + + And the ways + | nodes | name | oneway | highway | lanes | + | abc | circled | no | residential | 1 | + | cdefghijklmnopc | circled | no | residential | 1 | + + When I route I should get + | waypoints | bearings | route | turns | + | b,a | 90,10 270,10 | circled,circled | depart,arrive | diff --git a/src/engine/guidance/post_processing.cpp b/src/engine/guidance/post_processing.cpp index 8ba06e6ba..519b970d2 100644 --- a/src/engine/guidance/post_processing.cpp +++ b/src/engine/guidance/post_processing.cpp @@ -854,9 +854,9 @@ std::vector collapseTurns(std::vector steps) // handle final sliproad if (steps.size() >= 3 && - steps[steps.size() - 2].maneuver.instruction.type == TurnType::Sliproad) + steps[getPreviousIndex(steps.size() - 1)].maneuver.instruction.type == TurnType::Sliproad) { - steps[steps.size() - 2].maneuver.instruction.type = TurnType::Turn; + steps[getPreviousIndex(steps.size() - 1)].maneuver.instruction.type = TurnType::Turn; } BOOST_ASSERT(steps.front().intersections.size() >= 1); diff --git a/src/extractor/guidance/sliproad_handler.cpp b/src/extractor/guidance/sliproad_handler.cpp index b071a4fe1..0cb27805d 100644 --- a/src/extractor/guidance/sliproad_handler.cpp +++ b/src/extractor/guidance/sliproad_handler.cpp @@ -52,16 +52,15 @@ operator()(const NodeID, const EdgeID source_edge_id, Intersection intersection) return intersection; const auto findNextIntersectionForRoad = - [&](const NodeID at_node, const ConnectedRoad &road, NodeID *output_node) { + [&](const NodeID at_node, const ConnectedRoad &road, NodeID &output_node) { auto intersection = intersection_generator(at_node, road.turn.eid); auto in_edge = road.turn.eid; // skip over traffic lights - if (intersection.size() == 2) + while (intersection.size() == 2) { const auto node = node_based_graph.GetTarget(in_edge); in_edge = intersection[1].turn.eid; - if (output_node) - *output_node = node_based_graph.GetTarget(in_edge); + output_node = node_based_graph.GetTarget(in_edge); intersection = intersection_generator(node, in_edge); } return intersection; @@ -78,10 +77,14 @@ operator()(const NodeID, const EdgeID source_edge_id, Intersection intersection) // a one-sided sliproad, however, the non-sliproad can be considered `obvious`. Here we // assume that this could be the case and check for a potential sliproad/non-sliproad // situation. - const auto intersection_following_index_one = - findNextIntersectionForRoad(intersection_node_id, intersection[1], NULL); - const auto intersection_following_index_two = - findNextIntersectionForRoad(intersection_node_id, intersection[2], NULL); + NodeID intersection_node_one, intersection_node_two; + const auto intersection_following_index_one = findNextIntersectionForRoad( + intersection_node_id, intersection[1], intersection_node_one); + const auto intersection_following_index_two = findNextIntersectionForRoad( + intersection_node_id, intersection[2], intersection_node_two); + + // In case of loops at the end of the road, we will arrive back at the intersection + // itself. If that is the case, the road is obviously not a sliproad. // a sliproad has to enter a road without choice const auto couldBeSliproad = [](const Intersection &intersection) { @@ -93,9 +96,11 @@ operator()(const NodeID, const EdgeID source_edge_id, Intersection intersection) return true; }; - if (couldBeSliproad(intersection_following_index_one)) + if (couldBeSliproad(intersection_following_index_one) && + intersection_node_id != intersection_node_two) return 2; - else if (couldBeSliproad(intersection_following_index_two)) + else if (couldBeSliproad(intersection_following_index_two) && + intersection_node_id != intersection_node_one) return 1; else return 0; @@ -149,7 +154,11 @@ operator()(const NodeID, const EdgeID source_edge_id, Intersection intersection) auto next_intersection_node = node_based_graph.GetTarget(next_road.turn.eid); const auto next_road_next_intersection = - findNextIntersectionForRoad(intersection_node_id, next_road, &next_intersection_node); + findNextIntersectionForRoad(intersection_node_id, next_road, next_intersection_node); + + // If we are at a traffic loop at the end of a road, don't consider it a sliproad + if (intersection_node_id == next_intersection_node) + return intersection; std::unordered_set target_road_names;