From b5e289adc3d166c2c09e71a6aadfead20c0c7129 Mon Sep 17 00:00:00 2001 From: Moritz Kobitzsch Date: Tue, 24 Jan 2017 20:23:54 +0100 Subject: [PATCH] 3605 - fix error in treating end-of-road as choiceless (#3607) --- CHANGELOG.md | 1 + features/guidance/end-of-road.feature | 38 +++++++++++++++++++++++++ src/engine/guidance/post_processing.cpp | 28 ++++++++++++------ 3 files changed, 58 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a3cb9348..75413af8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ - Bugfixes - Fix #3475 removed an invalid `exit` field from the `arrive` maneuver - Fix #3515 adjusted number of `nodes` in `annotation` + - Fix #3605 Fixed a bug that could lead to turns at the end of the road to be suppressed - Infrastructure - Support building rpm packages. - Guidance diff --git a/features/guidance/end-of-road.feature b/features/guidance/end-of-road.feature index 834fa635f..d4d4e3ce5 100644 --- a/features/guidance/end-of-road.feature +++ b/features/guidance/end-of-road.feature @@ -24,6 +24,44 @@ Feature: End Of Road Instructions | a,c | aeb,cbd,cbd | depart,end of road left,arrive | | a,d | aeb,cbd,cbd | depart,end of road right,arrive | + @3605 + Scenario: End of Road with oneway through street + Given the node map + """ + c + a e b + f d + """ + + And the ways + | nodes | highway | oneway | + | aeb | primary | no | + | cbd | primary | yes | + | ef | primary | no | + + When I route I should get + | waypoints | route | turns | + | a,d | aeb,cbd,cbd | depart,end of road right,arrive | + + @3605 + Scenario: End of Road fromnameless onto through street + Given the node map + """ + c + a e b + f d + """ + + And the ways + | nodes | highway | oneway | name | + | aeb | primary | no | | + | cbd | primary | yes | cbd | + | ef | primary | no | ef | + + When I route I should get + | waypoints | route | turns | + | a,d | ,cbd,cbd | depart,end of road right,arrive | + Scenario: End of Road with three streets Given the node map """ diff --git a/src/engine/guidance/post_processing.cpp b/src/engine/guidance/post_processing.cpp index 463e3389d..ab245cf85 100644 --- a/src/engine/guidance/post_processing.cpp +++ b/src/engine/guidance/post_processing.cpp @@ -60,7 +60,7 @@ inline bool choiceless(const RouteStep &step, const RouteStep &previous) step.intersections.front().entry.end(), true); - return is_without_choice; + return is_without_choice && step.maneuver.instruction.type != TurnType::EndOfRoad; } // List of types that can be collapsed, if all other restrictions pass @@ -293,17 +293,20 @@ bool bearingsAreReversed(const double bearing_in, const double bearing_out) return angularDeviation(left_turn_angle, 180) <= 35; } -bool isLinkroad(const RouteStep &step) +bool isLinkroad(const RouteStep &pre_link_step, + const RouteStep &link_step, + const RouteStep &post_link_step) { const constexpr double MAX_LINK_ROAD_LENGTH = 60.0; - return step.distance <= MAX_LINK_ROAD_LENGTH && step.name_id == EMPTY_NAMEID; + return link_step.distance <= MAX_LINK_ROAD_LENGTH && link_step.name_id == EMPTY_NAMEID && + pre_link_step.name_id != EMPTY_NAMEID && post_link_step.name_id != EMPTY_NAMEID; } bool isUTurn(const RouteStep &in_step, const RouteStep &out_step, const RouteStep &pre_in_step) { const bool is_possible_candidate = in_step.distance <= MAX_COLLAPSE_DISTANCE || choiceless(out_step, in_step) || - (isLinkroad(in_step) && out_step.name_id != EMPTY_NAMEID && + (isLinkroad(pre_in_step, in_step, out_step) && out_step.name_id != EMPTY_NAMEID && pre_in_step.name_id != EMPTY_NAMEID && !isNoticeableNameChange(pre_in_step, out_step)); const bool takes_u_turn = bearingsAreReversed( util::bearing::reverse( @@ -459,7 +462,8 @@ void collapseTurnAt(std::vector &steps, // check if the actual turn we wan't to announce is delayed. This situation describes a turn // that is expressed by two turns, - const auto isDelayedTurn = [](const RouteStep &opening_turn, const RouteStep &finishing_turn) { + const auto isDelayedTurn = []( + const RouteStep &opening_turn, const RouteStep &finishing_turn, const RouteStep &pre_turn) { // only possible if both are compatible if (!compatible(opening_turn, finishing_turn)) return false; @@ -481,9 +485,12 @@ void collapseTurnAt(std::vector &steps, const auto is_delayed_turn_onto_a_ramp = opening_turn.distance <= 4 * MAX_COLLAPSE_DISTANCE && without_choice && hasRampType(finishing_turn.maneuver.instruction); + + const auto linkroad = isLinkroad(pre_turn, opening_turn, finishing_turn); + return !hasRampType(opening_turn.maneuver.instruction) && - (is_short_and_collapsable || is_not_too_long_and_choiceless || - isLinkroad(opening_turn) || is_delayed_turn_onto_a_ramp); + (is_short_and_collapsable || is_not_too_long_and_choiceless || linkroad || + is_delayed_turn_onto_a_ramp); } }; @@ -650,7 +657,8 @@ void collapseTurnAt(std::vector &steps, steps[one_back_index].Invalidate(); } // very short segment after turn, turn location remains at one_back_step - else if (isDelayedTurn(one_back_step, current_step)) // checks for compatibility + else if (isDelayedTurn( + one_back_step, current_step, steps[two_back_index])) // checks for compatibility { steps[one_back_index].ElongateBy(steps[step_index]); // TODO check for lanes (https://github.com/Project-OSRM/osrm-backend/issues/2553) @@ -1139,7 +1147,9 @@ std::vector collapseTurns(std::vector steps) } else if (one_back_index > 0 && (one_back_step.distance <= MAX_COLLAPSE_DISTANCE || - choiceless(current_step, one_back_step) || isLinkroad(one_back_step))) + choiceless(current_step, one_back_step) || + isLinkroad( + steps[getPreviousIndex(one_back_index, steps)], one_back_step, current_step))) { // check for one of the multiple collapse scenarios and, if possible, collapse the turn const auto two_back_index = getPreviousIndex(one_back_index, steps);