From 7b749103f8694061f38ee9d269f3f88f03d248a4 Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Wed, 28 Feb 2018 10:00:32 -0500 Subject: [PATCH 01/32] Added post process logic to collapse segregated turn instructions --- include/engine/api/route_api.hpp | 4 + include/engine/guidance/collapse_turns.hpp | 10 ++- .../engine/guidance/collapsing_utility.hpp | 15 ++++ .../guidance/collapse_scenario_detection.cpp | 15 ---- src/engine/guidance/collapse_turns.cpp | 73 +++++++++++++++++++ 5 files changed, 100 insertions(+), 17 deletions(-) diff --git a/include/engine/api/route_api.hpp b/include/engine/api/route_api.hpp index 6b5242fc7f7..cfcbf12fdd3 100644 --- a/include/engine/api/route_api.hpp +++ b/include/engine/api/route_api.hpp @@ -148,6 +148,10 @@ class RouteAPI : public BaseAPI // processing is performed guidance::applyOverrides(BaseAPI::facade, steps, leg_geometry); + // Collapse segregated steps before others + // TODO: before or after overrides? + steps = guidance::collapseSegregatedTurnInstructions(std::move(steps)); + /* Perform step-based post-processing. * * Using post-processing on basis of route-steps for a single leg at a time diff --git a/include/engine/guidance/collapse_turns.hpp b/include/engine/guidance/collapse_turns.hpp index 5ad5f47e391..c0796457c1b 100644 --- a/include/engine/guidance/collapse_turns.hpp +++ b/include/engine/guidance/collapse_turns.hpp @@ -13,13 +13,19 @@ namespace engine namespace guidance { +// Multiple possible reasons can result in unnecessary/confusing instructions +// Collapsing such turns into a single turn instruction, we give a clearer +// set of instructions that is not cluttered by unnecessary turns/name changes. +OSRM_ATTR_WARN_UNUSED +std::vector collapseTurnInstructions(std::vector steps); + // Multiple possible reasons can result in unnecessary/confusing instructions // A prime example would be a segregated intersection. Turning around at this // intersection would result in two instructions to turn left. // Collapsing such turns into a single turn instruction, we give a clearer -// set of instructionst that is not cluttered by unnecessary turns/name changes. +// set of instructions that is not cluttered by unnecessary turns/name changes. OSRM_ATTR_WARN_UNUSED -std::vector collapseTurnInstructions(std::vector steps); +std::vector collapseSegregatedTurnInstructions(std::vector steps); // A combined turn is a set of two instructions that actually form a single turn, as far as we // perceive it. A u-turn consisting of two left turns is one such example. But there are also lots diff --git a/include/engine/guidance/collapsing_utility.hpp b/include/engine/guidance/collapsing_utility.hpp index b8760a7a8d8..b344e4f4e81 100644 --- a/include/engine/guidance/collapsing_utility.hpp +++ b/include/engine/guidance/collapsing_utility.hpp @@ -215,6 +215,21 @@ inline double totalTurnAngle(const RouteStep &entry_step, const RouteStep &exit_ return total_angle; } +// check bearings for u-turns. +// since bearings are wrapped around at 0 (we only support 0,360), we need to do some minor math to +// check if bearings `a` and `b` go in opposite directions. In general we accept some minor +// deviations for u-turns. +inline bool bearingsAreReversed(const double bearing_in, const double bearing_out) +{ + // Nearly perfectly reversed angles have a difference close to 180 degrees (straight) + const double left_turn_angle = [&]() { + if (0 <= bearing_out && bearing_out <= bearing_in) + return bearing_in - bearing_out; + return bearing_in + 360 - bearing_out; + }(); + return util::angularDeviation(left_turn_angle, 180) <= 35; +} + } /* namespace guidance */ } /* namespace engine */ } /* namespace osrm */ diff --git a/src/engine/guidance/collapse_scenario_detection.cpp b/src/engine/guidance/collapse_scenario_detection.cpp index 7a984cc1e22..27efe856d82 100644 --- a/src/engine/guidance/collapse_scenario_detection.cpp +++ b/src/engine/guidance/collapse_scenario_detection.cpp @@ -17,21 +17,6 @@ using namespace osrm::guidance; namespace { -// check bearings for u-turns. -// since bearings are wrapped around at 0 (we only support 0,360), we need to do some minor math to -// check if bearings `a` and `b` go in opposite directions. In general we accept some minor -// deviations for u-turns. -bool bearingsAreReversed(const double bearing_in, const double bearing_out) -{ - // Nearly perfectly reversed angles have a difference close to 180 degrees (straight) - const double left_turn_angle = [&]() { - if (0 <= bearing_out && bearing_out <= bearing_in) - return bearing_in - bearing_out; - return bearing_in + 360 - bearing_out; - }(); - return util::angularDeviation(left_turn_angle, 180) <= 35; -} - // to collapse steps, we focus on short segments that don't interact with other roads. To collapse // two instructions into one, we need to look at to instrutions immediately after each other. bool noIntermediaryIntersections(const RouteStep &step) diff --git a/src/engine/guidance/collapse_turns.cpp b/src/engine/guidance/collapse_turns.cpp index da60fd911a7..7833d92c490 100644 --- a/src/engine/guidance/collapse_turns.cpp +++ b/src/engine/guidance/collapse_turns.cpp @@ -477,6 +477,79 @@ RouteSteps collapseTurnInstructions(RouteSteps steps) return steps; } +// OTHER IMPLEMENTATIONS +OSRM_ATTR_WARN_UNUSED +RouteSteps collapseSegregatedTurnInstructions(RouteSteps steps) +{ + // make sure we can safely iterate over all steps (has depart/arrive with TurnType::NoTurn) + BOOST_ASSERT(!hasTurnType(steps.front()) && !hasTurnType(steps.back())); + BOOST_ASSERT(hasWaypointType(steps.front()) && hasWaypointType(steps.back())); + + if (steps.size() <= 2) + return steps; + + auto curr_step = steps.begin() + 1; + auto next_step = curr_step + 1; + + // Loop over steps to collapse the segregated intersections with the + while (next_step != steps.end()) + { + const auto prev_step = findPreviousTurn(curr_step); + + // if current step and next step are both segregated then combine the steps with no turn adjustment + if (curr_step->is_segregated && next_step->is_segregated) + { + suppressStep(*curr_step, *next_step); + ++next_step; + } + // else if the current step is segregated and the next step is not then combine with turn adjustment + else if (curr_step->is_segregated && !next_step->is_segregated) + { + // Determine if u-turn + if (bearingsAreReversed( + util::bearing::reverse( + curr_step->intersections.front().bearings[curr_step->intersections.front().in]), + next_step->intersections.front().bearings[next_step->intersections.front().out])) + { + // Collapse segregated u-turn + combineRouteSteps( + *curr_step, + *next_step, + SetFixedInstructionStrategy({TurnType::Continue, DirectionModifier::UTurn}), + TransferSignageStrategy(), + NoModificationStrategy()); + } + else + { + // Collapse segregated turn + combineRouteSteps( + *curr_step, + *next_step, + AdjustToCombinedTurnStrategy(*prev_step), + TransferSignageStrategy(), + NoModificationStrategy()); + } + + // Segregated step has been removed + curr_step->is_segregated = false; + ++next_step; + + } + // else next step + else + { + curr_step = next_step; + ++next_step; + } + + } + + // Clean up steps + steps = removeNoTurnInstructions(std::move(steps)); + + return steps; +} + } // namespace guidance } // namespace engine } // namespace osrm From 1d3eac56788f743076d6cb65c509350cc99c1b8b Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Wed, 28 Feb 2018 10:29:13 -0500 Subject: [PATCH 02/32] format updates --- src/engine/guidance/collapse_turns.cpp | 36 +++++++++++++------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/engine/guidance/collapse_turns.cpp b/src/engine/guidance/collapse_turns.cpp index 7833d92c490..44668ea73e0 100644 --- a/src/engine/guidance/collapse_turns.cpp +++ b/src/engine/guidance/collapse_turns.cpp @@ -496,44 +496,45 @@ RouteSteps collapseSegregatedTurnInstructions(RouteSteps steps) { const auto prev_step = findPreviousTurn(curr_step); - // if current step and next step are both segregated then combine the steps with no turn adjustment + // if current step and next step are both segregated then combine the steps with no turn + // adjustment if (curr_step->is_segregated && next_step->is_segregated) { suppressStep(*curr_step, *next_step); ++next_step; } - // else if the current step is segregated and the next step is not then combine with turn adjustment + // else if the current step is segregated and the next step is not then combine with turn + // adjustment else if (curr_step->is_segregated && !next_step->is_segregated) { // Determine if u-turn if (bearingsAreReversed( - util::bearing::reverse( - curr_step->intersections.front().bearings[curr_step->intersections.front().in]), - next_step->intersections.front().bearings[next_step->intersections.front().out])) + util::bearing::reverse(curr_step->intersections.front() + .bearings[curr_step->intersections.front().in]), + next_step->intersections.front() + .bearings[next_step->intersections.front().out])) { // Collapse segregated u-turn combineRouteSteps( - *curr_step, - *next_step, - SetFixedInstructionStrategy({TurnType::Continue, DirectionModifier::UTurn}), - TransferSignageStrategy(), - NoModificationStrategy()); + *curr_step, + *next_step, + SetFixedInstructionStrategy({TurnType::Continue, DirectionModifier::UTurn}), + TransferSignageStrategy(), + NoModificationStrategy()); } else { // Collapse segregated turn - combineRouteSteps( - *curr_step, - *next_step, - AdjustToCombinedTurnStrategy(*prev_step), - TransferSignageStrategy(), - NoModificationStrategy()); + combineRouteSteps(*curr_step, + *next_step, + AdjustToCombinedTurnStrategy(*prev_step), + TransferSignageStrategy(), + NoModificationStrategy()); } // Segregated step has been removed curr_step->is_segregated = false; ++next_step; - } // else next step else @@ -541,7 +542,6 @@ RouteSteps collapseSegregatedTurnInstructions(RouteSteps steps) curr_step = next_step; ++next_step; } - } // Clean up steps From eea551db350515ebcf660383f4d9c0229624a309 Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Wed, 28 Feb 2018 16:07:59 -0500 Subject: [PATCH 03/32] Fixed coordinates to reflect reality Updated left turn road name --- features/car/conditional_restrictions.feature | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/features/car/conditional_restrictions.feature b/features/car/conditional_restrictions.feature index bbc6fde88fe..9951b6145e2 100644 --- a/features/car/conditional_restrictions.feature +++ b/features/car/conditional_restrictions.feature @@ -691,14 +691,14 @@ Feature: Car - Turn restrictions # """ Given the node locations | node | lat | lon | - | a | 38.9113 | -77.0091 | - | b | 38.9108 | -77.0091 | - | c | 38.9104 | -77.0091 | - | d | 38.9110 | -77.0096 | - | e | 38.9106 | -77.0086 | - | f | 38.9105 | -77.0090 | - | g | 38.9108 | -77.0090 | - | h | 38.9113 | -77.0090 | + | a | 38.91124 | -77.00909 | + | b | 38.91080 | -77.00909 | + | c | 38.91038 | -77.00909 | + | d | 38.91105 | -77.00967 | + | e | 38.91063 | -77.00853 | + | f | 38.91036 | -77.00899 | + | g | 38.91076 | -77.00901 | + | h | 38.91124 | -77.00900 | And the ways | nodes | oneway | name | @@ -719,7 +719,7 @@ Feature: Car - Turn restrictions When I route I should get | from | to | route | turns | | a | e | cap south,florida nw,florida nw,florida ne | depart,turn right,continue uturn,arrive | - | f | d | cap north,florida,florida nw | depart,turn left,arrive | + | f | d | cap north,florida nw,florida nw | depart,turn left,arrive | | e | c | florida ne,florida nw,cap south,cap south | depart,continue uturn,turn right,arrive | @no_turning @conditionals From b56d6696bf26dfc845e7981c6a2416297d44451d Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Wed, 28 Feb 2018 16:43:29 -0500 Subject: [PATCH 04/32] fixed coordinates to fix test --- features/car/conditional_restrictions.feature | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/features/car/conditional_restrictions.feature b/features/car/conditional_restrictions.feature index 9951b6145e2..23a9e1efe26 100644 --- a/features/car/conditional_restrictions.feature +++ b/features/car/conditional_restrictions.feature @@ -695,7 +695,7 @@ Feature: Car - Turn restrictions | b | 38.91080 | -77.00909 | | c | 38.91038 | -77.00909 | | d | 38.91105 | -77.00967 | - | e | 38.91063 | -77.00853 | + | e | 38.91037 | -77.00807 | | f | 38.91036 | -77.00899 | | g | 38.91076 | -77.00901 | | h | 38.91124 | -77.00900 | @@ -738,14 +738,14 @@ Feature: Car - Turn restrictions # """ Given the node locations | node | lat | lon | - | a | 38.9113 | -77.0091 | - | b | 38.9108 | -77.0091 | - | c | 38.9104 | -77.0091 | - | d | 38.9110 | -77.0096 | - | e | 38.9106 | -77.0086 | - | f | 38.9105 | -77.0090 | - | g | 38.9108 | -77.0090 | - | h | 38.9113 | -77.0090 | + | a | 38.91124 | -77.00909 | + | b | 38.91080 | -77.00909 | + | c | 38.91038 | -77.00909 | + | d | 38.91105 | -77.00967 | + | e | 38.91037 | -77.00807 | + | f | 38.91036 | -77.00899 | + | g | 38.91076 | -77.00901 | + | h | 38.91124 | -77.00900 | And the ways | nodes | oneway | name | @@ -765,7 +765,7 @@ Feature: Car - Turn restrictions When I route I should get | from | to | route | turns | - | a | e | cap south,florida,florida ne | depart,turn left,arrive | + | a | e | cap south,florida ne,florida ne | depart,turn left,arrive | | f | d | cap north,florida ne,florida ne,florida nw | depart,turn sharp right,continue uturn,arrive | | e | c | florida ne,cap south,cap south | depart,turn left,arrive | From ea47f79d2470a7fd15d357dac0780240e28e0215 Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Thu, 1 Mar 2018 14:03:36 -0500 Subject: [PATCH 05/32] Skip last step when processing segregated steps --- src/engine/guidance/collapse_turns.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/engine/guidance/collapse_turns.cpp b/src/engine/guidance/collapse_turns.cpp index 44668ea73e0..8581eb67007 100644 --- a/src/engine/guidance/collapse_turns.cpp +++ b/src/engine/guidance/collapse_turns.cpp @@ -490,9 +490,10 @@ RouteSteps collapseSegregatedTurnInstructions(RouteSteps steps) auto curr_step = steps.begin() + 1; auto next_step = curr_step + 1; + const auto last_step = steps.end() - 1; - // Loop over steps to collapse the segregated intersections with the - while (next_step != steps.end()) + // Loop over steps to collapse the segregated intersections; ignore first and last step + while (next_step != last_step) { const auto prev_step = findPreviousTurn(curr_step); From 9a11432c94beffb41cfeedf19ad3eaea45283c77 Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Thu, 1 Mar 2018 14:41:33 -0500 Subject: [PATCH 06/32] updated segregated turn test --- features/guidance/collapse.feature | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/features/guidance/collapse.feature b/features/guidance/collapse.feature index a1723805a9f..8c65e1d47b2 100644 --- a/features/guidance/collapse.feature +++ b/features/guidance/collapse.feature @@ -35,20 +35,20 @@ Feature: Collapse | waypoints | route | turns | locations | | a,l | first,second,second | depart,turn right,arrive | a,b,l | | a,d | first,first | depart,arrive | a,d | - | a,j | first,second,second | depart,turn left,arrive | a,c,j | - | a,h | first,first,first | depart,continue uturn,arrive | a,c,h | + | a,j | first,second,second | depart,turn left,arrive | a,b,j | + | a,h | first,first,first | depart,continue uturn,arrive | a,b,h | | e,j | first,second,second | depart,turn right,arrive | e,f,j | | e,h | first,first | depart,arrive | e,h | - | e,l | first,second,second | depart,turn left,arrive | e,g,l | - | e,d | first,first,first | depart,continue uturn,arrive | e,g,d | + | e,l | first,second,second | depart,turn left,arrive | e,f,l | + | e,d | first,first,first | depart,continue uturn,arrive | e,f,d | | k,h | second,first,first | depart,turn right,arrive | k,g,h | | k,l | second,second | depart,arrive | k,l | - | k,d | second,first,first | depart,turn left,arrive | k,b,d | - | k,j | second,second,second | depart,continue uturn,arrive | k,b,j | + | k,d | second,first,first | depart,turn left,arrive | k,g,d | + | k,j | second,second,second | depart,continue uturn,arrive | k,g,j | | i,d | second,first,first | depart,turn right,arrive | i,c,d | | i,j | second,second | depart,arrive | i,j | - | i,h | second,first,first | depart,turn left,arrive | i,f,h | - | i,l | second,second,second | depart,continue uturn,arrive | i,f,l | + | i,h | second,first,first | depart,turn left,arrive | i,c,h | + | i,l | second,second,second | depart,continue uturn,arrive | i,c,l | Scenario: Segregated Intersection, Cross Belonging to Single Street Given the node map From 20f47e6194115e3e5cde2c6a7b36755c7f703a9d Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Thu, 1 Mar 2018 14:52:47 -0500 Subject: [PATCH 07/32] Updated segregated test --- features/guidance/collapse.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/guidance/collapse.feature b/features/guidance/collapse.feature index 8c65e1d47b2..eaa2fb92b14 100644 --- a/features/guidance/collapse.feature +++ b/features/guidance/collapse.feature @@ -74,7 +74,7 @@ Feature: Collapse When I route I should get | waypoints | route | turns | locations | - | a,i | first,second,third,third | depart,turn left,turn slight left,arrive | a,b,e,i | + | a,i | first,third,third | depart,turn sharp left,arrive | a,b,i | Scenario: Segregated Intersection, Cross Belonging to Correct Street Given the node map From 482529e74daec9cade43825d3c19e08f3dc8c47f Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Thu, 1 Mar 2018 15:26:57 -0500 Subject: [PATCH 08/32] Updated test: Segregated Intersection, Cross Belonging to Correct Street - features/guidance/collapse.feature:79 --- features/guidance/collapse.feature | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/features/guidance/collapse.feature b/features/guidance/collapse.feature index eaa2fb92b14..d7ec9d19ff8 100644 --- a/features/guidance/collapse.feature +++ b/features/guidance/collapse.feature @@ -106,20 +106,20 @@ Feature: Collapse | waypoints | route | turns | locations | | a,l | first,second,second | depart,turn right,arrive | a,b,l | | a,d | first,first | depart,arrive | a,d | - | a,j | first,second,second | depart,turn left,arrive | a,c,j | - | a,h | first,first,first | depart,continue uturn,arrive | a,c,h | + | a,j | first,second,second | depart,turn left,arrive | a,b,j | + | a,h | first,first,first | depart,continue uturn,arrive | a,b,h | | e,j | first,second,second | depart,turn right,arrive | e,f,j | | e,h | first,first | depart,arrive | e,h | - | e,l | first,second,second | depart,turn left,arrive | e,g,l | - | e,d | first,first,first | depart,continue uturn,arrive | e,g,d | + | e,l | first,second,second | depart,turn left,arrive | e,f,l | + | e,d | first,first,first | depart,continue uturn,arrive | e,f,d | | k,h | second,first,first | depart,turn right,arrive | k,g,h | | k,l | second,second | depart,arrive | k,l | - | k,d | second,first,first | depart,turn left,arrive | k,b,d | - | k,j | second,second,second | depart,continue uturn,arrive | k,b,j | + | k,d | second,first,first | depart,turn left,arrive | k,g,d | + | k,j | second,second,second | depart,continue uturn,arrive | k,g,j | | i,d | second,first,first | depart,turn right,arrive | i,c,d | | i,j | second,second | depart,arrive | i,j | - | i,h | second,first,first | depart,turn left,arrive | i,f,h | - | i,l | second,second,second | depart,continue uturn,arrive | i,f,l | + | i,h | second,first,first | depart,turn left,arrive | i,c,h | + | i,l | second,second,second | depart,continue uturn,arrive | i,c,l | Scenario: Segregated Intersection, Cross Belonging to Mixed Streets Given the node map From e7c7f3eb83edb18ede814fbbb8ea7dbd1a47c2ed Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Tue, 13 Mar 2018 15:20:39 -0400 Subject: [PATCH 09/32] Fixed all but one for features/guidance/collapse.feature:124 --- features/guidance/collapse.feature | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/features/guidance/collapse.feature b/features/guidance/collapse.feature index d7ec9d19ff8..fe7d8ca55b8 100644 --- a/features/guidance/collapse.feature +++ b/features/guidance/collapse.feature @@ -151,20 +151,20 @@ Feature: Collapse | waypoints | route | turns | locations | | a,l | first,second,second | depart,turn right,arrive | a,b,l | | a,d | first,first | depart,arrive | a,d | - | a,j | first,second,second | depart,turn left,arrive | a,c,j | - | a,h | first,first,first | depart,continue uturn,arrive | a,c,h | + | a,j | first,second,second | depart,turn left,arrive | a,b,j | + | a,h | first,first,first | depart,continue uturn,arrive | a,b,h | | e,j | first,second,second | depart,turn right,arrive | e,f,j | | e,h | first,first | depart,arrive | e,h | - | e,l | first,second,second | depart,turn left,arrive | e,g,l | - | e,d | first,first,first | depart,continue uturn,arrive | e,g,d | + | e,l | first,second,second | depart,turn left,arrive | e,f,l | + | e,d | first,first,first | depart,continue uturn,arrive | e,f,d | | k,h | second,first,first | depart,turn right,arrive | k,g,h | | k,l | second,second | depart,arrive | k,l | - | k,d | second,first,first | depart,turn left,arrive | k,b,d | - | k,j | second,second,second | depart,continue uturn,arrive | k,b,j | + | k,d | second,first,first | depart,turn left,arrive | k,g,d | + | k,j | second,second,second | depart,continue uturn,arrive | k,g,j | | i,d | second,first,first | depart,turn right,arrive | i,c,d | | i,j | second,second | depart,arrive | i,j | - | i,h | second,first,first | depart,turn left,arrive | i,f,h | - | i,l | second,second,second | depart,continue uturn,arrive | i,f,l | + | i,h | second,first,first | depart,turn left,arrive | i,c,h | + | i,l | second,second,second | depart,continue uturn,arrive | i,c,l | Scenario: Partly Segregated Intersection, Two Segregated Roads Given the node map From 9c81b7367c6816e6d2a6204016b200231a831ffe Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Tue, 13 Mar 2018 16:20:26 -0400 Subject: [PATCH 10/32] Fixed Scenario: Partly Segregated Intersection, Two Segregated Roads, Intersection belongs to Second - features/guidance/collapse.feature:219 --- features/guidance/collapse.feature | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/features/guidance/collapse.feature b/features/guidance/collapse.feature index fe7d8ca55b8..8d136c2fbfb 100644 --- a/features/guidance/collapse.feature +++ b/features/guidance/collapse.feature @@ -263,11 +263,11 @@ Feature: Collapse | d,c | first,first,first | depart,continue uturn,arrive | d,e,c | | g,c | second,first,first | depart,turn right,arrive | g,b,c | | g,j | second,second | depart,arrive | g,j | - | g,f | second,first,first | depart,turn left,arrive | g,e,f | + | g,f | second,first,first | depart,turn left,arrive | g,b,f | | g,h | second,second,second | depart,continue uturn,arrive | g,b,h | | i,f | second,first,first | depart,turn right,arrive | i,e,f | | i,h | second,second | depart,arrive | i,h | - | i,c | second,first,first | depart,turn left,arrive | i,b,c | + | i,c | second,first,first | depart,turn left,arrive | i,e,c | | i,j | second,second,second | depart,continue uturn,arrive | i,e,j | Scenario: Segregated Intersection, Cross Belonging to Mixed Streets - Slight Angles From b72b9dea521890baf740e1dc831ff833cc83c010 Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Tue, 13 Mar 2018 16:32:53 -0400 Subject: [PATCH 11/32] Fixed 7 of th 9 failures for Scenario: Partly Segregated Intersection, Two Segregated Roads, Intersection belongs to Second - features/guidance/collapse.feature:219 --- features/guidance/collapse.feature | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/features/guidance/collapse.feature b/features/guidance/collapse.feature index 8d136c2fbfb..a3fc1bd8e7d 100644 --- a/features/guidance/collapse.feature +++ b/features/guidance/collapse.feature @@ -300,20 +300,20 @@ Feature: Collapse | waypoints | route | turns | locations | | a,l | first,second,second | depart,turn right,arrive | a,b,l | | a,d | first,first | depart,arrive | a,d | - | a,j | first,second,second | depart,turn left,arrive | a,c,j | - | a,h | first,first,first | depart,continue uturn,arrive | a,c,h | + | a,j | first,second,second | depart,turn left,arrive | a,b,j | + | a,h | first,first,first | depart,continue uturn,arrive | a,b,h | | e,j | first,second,second | depart,turn right,arrive | e,f,j | | e,h | first,first | depart,arrive | e,h | - | e,l | first,second,second | depart,turn left,arrive | e,g,l | - | e,d | first,first,first | depart,continue uturn,arrive | e,g,d | + | e,l | first,second,second | depart,turn left,arrive | e,f,l | + | e,d | first,first,first | depart,continue uturn,arrive | e,f,d | | k,h | second,first,first | depart,turn right,arrive | k,g,h | | k,l | second,second | depart,arrive | k,l | - | k,d | second,first,first | depart,turn left,arrive | k,b,d | - | k,j | second,second,second | depart,continue uturn,arrive | k,b,j | + | k,d | second,first,first | depart,turn left,arrive | k,g,d | + | k,j | second,second,second | depart,continue uturn,arrive | k,g,j | | i,d | second,first,first | depart,turn right,arrive | i,c,d | | i,j | second,second | depart,arrive | i,j | - | i,h | second,first,first | depart,turn left,arrive | i,f,h | - | i,l | second,second,second | depart,continue uturn,arrive | i,f,l | + | i,h | second,first,first | depart,turn left,arrive | i,c,h | + | i,l | second,second,second | depart,continue uturn,arrive | i,c,l | Scenario: Segregated Intersection, Cross Belonging to Mixed Streets - Slight Angles (2) Given the node map From 0bf22f8d4e5b824cacbc8b3b1fb2d24b09616c0a Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Tue, 13 Mar 2018 16:48:25 -0400 Subject: [PATCH 12/32] Fixed 7 of the 9 failures for Scenario: Segregated Intersection, Cross Belonging to Mixed Streets - Slight Angles (2) - features/guidance/collapse.feature:318 --- features/guidance/collapse.feature | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/features/guidance/collapse.feature b/features/guidance/collapse.feature index a3fc1bd8e7d..49fefdbfc2a 100644 --- a/features/guidance/collapse.feature +++ b/features/guidance/collapse.feature @@ -345,20 +345,20 @@ Feature: Collapse | waypoints | route | turns | locations | | a,l | first,second,second | depart,turn right,arrive | a,b,l | | a,d | first,first | depart,arrive | a,d | - | a,j | first,second,second | depart,turn left,arrive | a,c,j | - | a,h | first,first,first | depart,continue uturn,arrive | a,c,h | + | a,j | first,second,second | depart,turn left,arrive | a,b,j | + | a,h | first,first,first | depart,continue uturn,arrive | a,b,h | | e,j | first,second,second | depart,turn right,arrive | e,f,j | | e,h | first,first | depart,arrive | e,h | - | e,l | first,second,second | depart,turn left,arrive | e,g,l | - | e,d | first,first,first | depart,continue uturn,arrive | e,g,d | + | e,l | first,second,second | depart,turn left,arrive | e,f,l | + | e,d | first,first,first | depart,continue uturn,arrive | e,f,d | | k,h | second,first,first | depart,turn right,arrive | k,g,h | | k,l | second,second | depart,arrive | k,l | - | k,d | second,first,first | depart,turn left,arrive | k,b,d | - | k,j | second,second,second | depart,continue uturn,arrive | k,b,j | + | k,d | second,first,first | depart,turn left,arrive | k,g,d | + | k,j | second,second,second | depart,continue uturn,arrive | k,g,j | | i,d | second,first,first | depart,turn right,arrive | i,c,d | | i,j | second,second | depart,arrive | i,j | - | i,h | second,first,first | depart,turn left,arrive | i,f,h | - | i,l | second,second,second | depart,continue uturn,arrive | i,f,l | + | i,h | second,first,first | depart,turn left,arrive | i,c,h | + | i,l | second,second,second | depart,continue uturn,arrive | i,c,l | Scenario: Entering a segregated road Given the node map From e9c61166ed37b955b7eef055d1fd0673c63c64b7 Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Tue, 13 Mar 2018 17:08:55 -0400 Subject: [PATCH 13/32] Fixed Scenario: Segregated Intersection into Slight Turn - features/guidance/collapse.feature:581 --- features/guidance/collapse.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/guidance/collapse.feature b/features/guidance/collapse.feature index 49fefdbfc2a..3cfe45ea266 100644 --- a/features/guidance/collapse.feature +++ b/features/guidance/collapse.feature @@ -603,7 +603,7 @@ Feature: Collapse When I route I should get | waypoints | route | turns | locations | - | i,h | in,road,road | depart,turn left,arrive | i,f,h | + | i,h | in,road,road | depart,turn slight left,arrive | i,c,h | | a,d | road,road | depart,arrive | a,d | | a,j | road,out,out | depart,turn slight right,arrive | a,b,j | From 7d5e9124597bb6171d7ffa8a33deda0e5ec59cd9 Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Thu, 15 Mar 2018 09:16:44 -0400 Subject: [PATCH 14/32] Updated Scenario: U-turn after a traffic light - features/guidance/turn-lanes.feature:1220 --- features/guidance/turn-lanes.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/guidance/turn-lanes.feature b/features/guidance/turn-lanes.feature index 54a8e3be32e..4f894e97a40 100644 --- a/features/guidance/turn-lanes.feature +++ b/features/guidance/turn-lanes.feature @@ -1243,4 +1243,4 @@ Feature: Turn Lane Guidance When I route I should get | waypoints | route | turns | lanes | locations | - | a,f | road1,road1,road1 | depart,continue uturn,arrive | ;left:false straight:true straight;right:false,left:true straight:false straight;right:false;;, | a,d,f | + | a,f | road1,road1,road1 | depart,continue uturn,arrive | ,;left:true straight:false straight;right:false;;, | a,c,f | From f956ddd0c9c321f88a6a28d8aeb31b07794eb0bb Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Thu, 15 Mar 2018 09:28:31 -0400 Subject: [PATCH 15/32] Updated how we combine segregated steps --- src/engine/guidance/collapse_turns.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/engine/guidance/collapse_turns.cpp b/src/engine/guidance/collapse_turns.cpp index 8581eb67007..888447c3a12 100644 --- a/src/engine/guidance/collapse_turns.cpp +++ b/src/engine/guidance/collapse_turns.cpp @@ -501,7 +501,12 @@ RouteSteps collapseSegregatedTurnInstructions(RouteSteps steps) // adjustment if (curr_step->is_segregated && next_step->is_segregated) { - suppressStep(*curr_step, *next_step); + // Combine segregated steps + combineRouteSteps(*curr_step, + *next_step, + NoModificationStrategy(), + TransferSignageStrategy(), + TransferLanesStrategy()); ++next_step; } // else if the current step is segregated and the next step is not then combine with turn From 4690c9386b23f9477a685891ea1eb41290a5cb6e Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Thu, 15 Mar 2018 11:30:13 -0400 Subject: [PATCH 16/32] Added test to Verify end of road left turn across divided roads --- features/guidance/divided-highways.feature | 25 ++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/features/guidance/divided-highways.feature b/features/guidance/divided-highways.feature index 6bfbca34d95..3b953a898ac 100644 --- a/features/guidance/divided-highways.feature +++ b/features/guidance/divided-highways.feature @@ -100,3 +100,28 @@ Feature: Divided road entry When I route I should get | waypoints | route | turns | | g,a | side st,main st,main st| depart,turn left,arrive | + + # Verify end of road left turn across divided roads + Scenario: Join on a divided road, named after the side street + Given the node map + """ + a-----h--b-----c + | | + d-----i--e-----f + | | + | | + j g + """ + + And the ways + | nodes | name | highway | oneway | + | ahbc | main st | residential | -1 | + | dief | main st | residential | yes | + | beg | side st | residential | -1 | + | hij | side st | residential | yes | + + When I route I should get + | waypoints | route | turns | + | g,a | side st,main st,main st| depart,end of road left,arrive | + + From 8385f6a2930b86a3b44d410fad82eed18e61990b Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Sat, 17 Mar 2018 14:37:44 -0400 Subject: [PATCH 17/32] Fixed divided highwat tests --- features/guidance/divided-highways.feature | 46 ++++++++++++++-------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/features/guidance/divided-highways.feature b/features/guidance/divided-highways.feature index 3b953a898ac..2824293bcd5 100644 --- a/features/guidance/divided-highways.feature +++ b/features/guidance/divided-highways.feature @@ -13,19 +13,23 @@ Feature: Divided road entry d-------e-----f | | - g + i---g---j + | + | + h """ And the ways - | nodes | name | highway | oneway | - | abc | main st | residential | -1 | - | def | main st | residential | yes | - | be | main st | residential | | - | eg | side st | residential | | + | nodes | name | highway | oneway | + | abc | main st | residential | -1 | + | def | main st | residential | yes | + | be | main st | residential | | + | egh | side st | residential | | + | igj | maple st | residential | | When I route I should get | waypoints | route | turns | - | g,a | side st,main st,main st| depart,end of road left,arrive | + | h,a | side st,main st,main st| depart,end of road left,arrive | # Similar to previous one, but the joining way is tagged with the side-street name @@ -37,18 +41,22 @@ Feature: Divided road entry d-------e-----f | | - g + i---g---j + | + | + h """ And the ways - | nodes | name | highway | oneway | - | abc | main st | residential | -1 | - | def | main st | residential | yes | - | beg | side st | residential | | + | nodes | name | highway | oneway | + | abc | main st | residential | -1 | + | def | main st | residential | yes | + | begh | side st | residential | | + | igj | maple st | residential | | When I route I should get | waypoints | route | turns | - | g,a | side st,main st,main st| depart,end of road left,arrive | + | h,a | side st,main st,main st| depart,end of road left,arrive | # Center join named after crossroad @@ -110,18 +118,22 @@ Feature: Divided road entry d-----i--e-----f | | | | - j g + m---j--g---n + | | + | | + k l """ And the ways | nodes | name | highway | oneway | | ahbc | main st | residential | -1 | | dief | main st | residential | yes | - | beg | side st | residential | -1 | - | hij | side st | residential | yes | + | begl | side st | residential | -1 | + | hijk | side st | residential | yes | + | mjgn | maple st| residential | no | When I route I should get | waypoints | route | turns | - | g,a | side st,main st,main st| depart,end of road left,arrive | + | l,a | side st,main st,main st| depart,end of road left,arrive | From c6b1d57034bd9ad38a32ab61e63aa938e9b8600f Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Sat, 17 Mar 2018 16:34:10 -0400 Subject: [PATCH 18/32] Fixed test failure --- features/guidance/collapse.feature | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/features/guidance/collapse.feature b/features/guidance/collapse.feature index 3cfe45ea266..f2590ceccee 100644 --- a/features/guidance/collapse.feature +++ b/features/guidance/collapse.feature @@ -209,11 +209,11 @@ Feature: Collapse | d,c | first,first,first | depart,continue uturn,arrive | d,e,c | | g,c | second,first,first | depart,turn right,arrive | g,b,c | | g,j | second,second | depart,arrive | g,j | - | g,f | second,first,first | depart,turn left,arrive | g,e,f | + | g,f | second,first,first | depart,turn left,arrive | g,b,f | | g,h | second,second,second | depart,continue uturn,arrive | g,b,h | | i,f | second,first,first | depart,turn right,arrive | i,e,f | | i,h | second,second | depart,arrive | i,h | - | i,c | second,first,first | depart,turn left,arrive | i,b,c | + | i,c | second,first,first | depart,turn left,arrive | i,e,c | | i,j | second,second,second | depart,continue uturn,arrive | i,e,j | Scenario: Partly Segregated Intersection, Two Segregated Roads, Intersection belongs to Second From 07a2a2d21c676dd19e0d99d6a3c9913e1377b0e8 Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Sun, 18 Mar 2018 16:37:45 -0400 Subject: [PATCH 19/32] fixed Scenario: Partitioned turn, Slight Curve - maxspeed - features/guidance/turn-lanes.feature:936 --- features/guidance/turn-lanes.feature | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/features/guidance/turn-lanes.feature b/features/guidance/turn-lanes.feature index 4f894e97a40..80cb9a54315 100644 --- a/features/guidance/turn-lanes.feature +++ b/features/guidance/turn-lanes.feature @@ -936,25 +936,27 @@ Feature: Turn Lane Guidance Scenario: Partitioned turn, Slight Curve - maxspeed Given the node map """ - f e - | | - | | - | c - a - b ' | - g d + f e + | | + i | | + | | c + h - a - b ' | + j g d """ And the ways | nodes | name | highway | oneway | turn:lanes:forward | maxspeed | + | ha | road | primary | yes | | 1 | | ab | road | primary | yes | left\|right | 1 | | bc | cross | primary | yes | | 1 | | fbg | cross | primary | yes | | 1 | | dce | cross | primary | yes | | 1 | + | iaj | kross | primary | no | | 1 | When I route I should get | waypoints | route | turns | lanes | locations | - | a,g | road,cross,cross | depart,turn right,arrive | ,left:false right:true, | a,b,g | - | a,e | road,cross,cross | depart,end of road left,arrive | ;left:true right:false,left:true right:false, | a,c,e | + | h,g | road,cross,cross | depart,turn right,arrive | ;,left:false right:true, | h,b,g | + | h,e | road,cross,cross | depart,end of road left,arrive | ;,left:true right:false;left:true right:false, | h,b,e | Scenario: Partitioned turn, Slight Curve Given the node map From 06d11a0a7bbd732b34af8e0e882c31afb48bb62c Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Sun, 18 Mar 2018 16:44:01 -0400 Subject: [PATCH 20/32] Fixed Scenario: Partitioned turn, Slight Curve - features/guidance/turn-lanes.feature:961 --- features/guidance/turn-lanes.feature | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/features/guidance/turn-lanes.feature b/features/guidance/turn-lanes.feature index 80cb9a54315..a079143e74e 100644 --- a/features/guidance/turn-lanes.feature +++ b/features/guidance/turn-lanes.feature @@ -961,25 +961,27 @@ Feature: Turn Lane Guidance Scenario: Partitioned turn, Slight Curve Given the node map """ - f e - | | - | | - | c - a - b ' | - g d + f e + | | + i | | + | | c + h - a - b ' | + j g d """ And the ways | nodes | name | highway | oneway | turn:lanes:forward | + | ha | road | primary | yes | | | ab | road | primary | yes | left\|right | | bc | cross | primary | yes | | | fbg | cross | primary | yes | | | dce | cross | primary | yes | | + | iaj | kross | primary | no | | When I route I should get - | waypoints | route | turns | lanes | locations | - | a,g | road,cross,cross | depart,turn right,arrive | ,left:false right:true, | a,b,g | - | a,e | road,cross,cross | depart,end of road left,arrive | ;left:true right:false,left:true right:false, | a,c,e | + | waypoints | route | turns | lanes | locations | + | h,g | road,cross,cross | depart,turn right,arrive | ;,left:false right:true, | h,b,g | + | h,e | road,cross,cross | depart,end of road left,arrive | ;,left:true right:false;left:true right:false, | h,b,e | Scenario: Lane Parsing Issue #2694 Given the node map From 832b52da47846b007dbf5a2aa3f42c3a739f53fc Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Tue, 20 Mar 2018 14:30:41 -0400 Subject: [PATCH 21/32] Added strategies to combine segrgated intersections --- include/engine/guidance/collapse_turns.hpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/include/engine/guidance/collapse_turns.hpp b/include/engine/guidance/collapse_turns.hpp index c0796457c1b..b8b5d319889 100644 --- a/include/engine/guidance/collapse_turns.hpp +++ b/include/engine/guidance/collapse_turns.hpp @@ -99,6 +99,22 @@ struct StaggeredTurnStrategy : CombineStrategy const RouteStep &step_prior_to_intersection; }; +// Handling of consecutive segregated steps +struct CombineSegregatedStepsStrategy : CombineStrategy +{ + void operator()(RouteStep &step_at_turn_location, const RouteStep &transfer_from_step) const; +}; + +// Handling of segregated intersections +struct SegregatedTurnStrategy : CombineStrategy +{ + SegregatedTurnStrategy(const RouteStep &step_prior_to_intersection); + + void operator()(RouteStep &step_at_turn_location, const RouteStep &transfer_from_step) const; + + const RouteStep &step_prior_to_intersection; +}; + // Signage Strategies // Transfer the signage from the next step onto this step From 99275c66ad1caea794d26da834f303e0da4bf2bf Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Tue, 20 Mar 2018 14:32:11 -0400 Subject: [PATCH 22/32] Added setModifier alias for readability --- include/engine/guidance/collapsing_utility.hpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/include/engine/guidance/collapsing_utility.hpp b/include/engine/guidance/collapsing_utility.hpp index b344e4f4e81..c772dd38578 100644 --- a/include/engine/guidance/collapsing_utility.hpp +++ b/include/engine/guidance/collapsing_utility.hpp @@ -104,6 +104,12 @@ inline void setInstructionType(RouteStep &step, const osrm::guidance::TurnType:: step.maneuver.instruction.type = type; } +// alias for readability +inline void setModifier(RouteStep &step, const osrm::guidance::DirectionModifier::Enum modifier) +{ + step.maneuver.instruction.direction_modifier = modifier; +} + // alias for readability inline bool haveSameMode(const RouteStep &lhs, const RouteStep &rhs) { From 3e0084a31b8bad78fff976c2bda6618f01b2ab76 Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Tue, 20 Mar 2018 14:43:27 -0400 Subject: [PATCH 23/32] Added strategies to combine segrgated intersections --- src/engine/guidance/collapse_turns.cpp | 100 ++++++++++++++++++++++++- 1 file changed, 98 insertions(+), 2 deletions(-) diff --git a/src/engine/guidance/collapse_turns.cpp b/src/engine/guidance/collapse_turns.cpp index 888447c3a12..c798dce7823 100644 --- a/src/engine/guidance/collapse_turns.cpp +++ b/src/engine/guidance/collapse_turns.cpp @@ -284,6 +284,101 @@ void StaggeredTurnStrategy::operator()(RouteStep &step_at_turn_location, : TurnType::NewName; } +void CombineSegregatedStepsStrategy::operator()(RouteStep &step_at_turn_location, + const RouteStep &transfer_from_step) const +{ + // TODO + if (hasTurnType(step_at_turn_location, TurnType::EndOfRoad) || + hasTurnType(transfer_from_step, TurnType::EndOfRoad)) + { + setInstructionType(step_at_turn_location, TurnType::EndOfRoad); + } + +} + +SegregatedTurnStrategy::SegregatedTurnStrategy(const RouteStep &step_prior_to_intersection) + : step_prior_to_intersection(step_prior_to_intersection) +{ +} + +void SegregatedTurnStrategy::operator()(RouteStep &step_at_turn_location, + const RouteStep &transfer_from_step) const +{ + const auto calculate_turn_direction = [](const RouteStep &entry_step, const RouteStep &exit_step) { + const double angle = + util::bearing::angleBetween(entry_step.maneuver.bearing_before, exit_step.maneuver.bearing_after); + + return getTurnDirection(angle); + }; + + // Calculate turn direction for segregated + const auto turn_direction = calculate_turn_direction(step_at_turn_location, transfer_from_step); + + const auto is_straight_step = [](const RouteStep &step) { + return ((hasTurnType(step, TurnType::NewName) || + hasTurnType(step, TurnType::Continue) || + hasTurnType(step, TurnType::Suppressed) || + hasTurnType(step, TurnType::Turn)) && + (hasModifier(step, DirectionModifier::Straight) || + hasModifier(step, DirectionModifier::SlightLeft) || + hasModifier(step, DirectionModifier::SlightRight))); + }; + + const auto is_turn_step = [](const RouteStep &step) { + return (hasTurnType(step, TurnType::Turn) || + hasTurnType(step, TurnType::Continue) || + hasTurnType(step, TurnType::NewName) || + hasTurnType(step, TurnType::Suppressed)); + }; + + // Process end of road step + if (hasTurnType(step_at_turn_location, TurnType::EndOfRoad) || + hasTurnType(transfer_from_step, TurnType::EndOfRoad)) + { + // Keep end of road + setInstructionType(step_at_turn_location, TurnType::EndOfRoad); + + // Set modifier based on turn angle + setModifier(step_at_turn_location, turn_direction); + } + // TODO maybe remove fork logic after updating identification logic + // Process fork step at turn + else if (hasTurnType(step_at_turn_location, TurnType::Fork)) + { + // Keep fork + setInstructionType(step_at_turn_location, TurnType::Fork); + } + // Process straight step + else if ((turn_direction == guidance::DirectionModifier::Straight) && + is_straight_step(transfer_from_step)) + { + // Determine if continue or new name + setInstructionType(step_at_turn_location, + (haveSameName(step_prior_to_intersection, transfer_from_step) + ? TurnType::Suppressed + : TurnType::NewName)); + } + // Process turn step + else if ((turn_direction != guidance::DirectionModifier::Straight) && + is_turn_step(transfer_from_step)) + { + // Mark as turn + setInstructionType(step_at_turn_location, TurnType::Turn); + } + // Process the others not covered above by using the transfer step turn type + else + { + // Set type from transfer step + setInstructionType(step_at_turn_location, transfer_from_step.maneuver.instruction.type); + } + + // If not fork then set modifier based on turn angle + if (!hasTurnType(step_at_turn_location, TurnType::Fork)) + { + setModifier(step_at_turn_location, turn_direction); + } +} + SetFixedInstructionStrategy::SetFixedInstructionStrategy(const TurnInstruction instruction) : instruction(instruction) { @@ -474,6 +569,7 @@ RouteSteps collapseTurnInstructions(RouteSteps steps) } } } + return steps; } @@ -504,7 +600,7 @@ RouteSteps collapseSegregatedTurnInstructions(RouteSteps steps) // Combine segregated steps combineRouteSteps(*curr_step, *next_step, - NoModificationStrategy(), + CombineSegregatedStepsStrategy(), TransferSignageStrategy(), TransferLanesStrategy()); ++next_step; @@ -533,7 +629,7 @@ RouteSteps collapseSegregatedTurnInstructions(RouteSteps steps) // Collapse segregated turn combineRouteSteps(*curr_step, *next_step, - AdjustToCombinedTurnStrategy(*prev_step), + SegregatedTurnStrategy(*prev_step), TransferSignageStrategy(), NoModificationStrategy()); } From 6ad4d27c1f1b756105bf070911cf220e0a3acae1 Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Tue, 20 Mar 2018 14:59:31 -0400 Subject: [PATCH 24/32] Format updates --- src/engine/guidance/collapse_turns.cpp | 40 ++++++++++++-------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/src/engine/guidance/collapse_turns.cpp b/src/engine/guidance/collapse_turns.cpp index c798dce7823..f42cfde654a 100644 --- a/src/engine/guidance/collapse_turns.cpp +++ b/src/engine/guidance/collapse_turns.cpp @@ -289,11 +289,10 @@ void CombineSegregatedStepsStrategy::operator()(RouteStep &step_at_turn_location { // TODO if (hasTurnType(step_at_turn_location, TurnType::EndOfRoad) || - hasTurnType(transfer_from_step, TurnType::EndOfRoad)) + hasTurnType(transfer_from_step, TurnType::EndOfRoad)) { setInstructionType(step_at_turn_location, TurnType::EndOfRoad); } - } SegregatedTurnStrategy::SegregatedTurnStrategy(const RouteStep &step_prior_to_intersection) @@ -302,11 +301,12 @@ SegregatedTurnStrategy::SegregatedTurnStrategy(const RouteStep &step_prior_to_in } void SegregatedTurnStrategy::operator()(RouteStep &step_at_turn_location, - const RouteStep &transfer_from_step) const + const RouteStep &transfer_from_step) const { - const auto calculate_turn_direction = [](const RouteStep &entry_step, const RouteStep &exit_step) { - const double angle = - util::bearing::angleBetween(entry_step.maneuver.bearing_before, exit_step.maneuver.bearing_after); + const auto calculate_turn_direction = [](const RouteStep &entry_step, + const RouteStep &exit_step) { + const double angle = util::bearing::angleBetween(entry_step.maneuver.bearing_before, + exit_step.maneuver.bearing_after); return getTurnDirection(angle); }; @@ -315,25 +315,21 @@ void SegregatedTurnStrategy::operator()(RouteStep &step_at_turn_location, const auto turn_direction = calculate_turn_direction(step_at_turn_location, transfer_from_step); const auto is_straight_step = [](const RouteStep &step) { - return ((hasTurnType(step, TurnType::NewName) || - hasTurnType(step, TurnType::Continue) || - hasTurnType(step, TurnType::Suppressed) || - hasTurnType(step, TurnType::Turn)) && + return ((hasTurnType(step, TurnType::NewName) || hasTurnType(step, TurnType::Continue) || + hasTurnType(step, TurnType::Suppressed) || hasTurnType(step, TurnType::Turn)) && (hasModifier(step, DirectionModifier::Straight) || - hasModifier(step, DirectionModifier::SlightLeft) || - hasModifier(step, DirectionModifier::SlightRight))); + hasModifier(step, DirectionModifier::SlightLeft) || + hasModifier(step, DirectionModifier::SlightRight))); }; const auto is_turn_step = [](const RouteStep &step) { - return (hasTurnType(step, TurnType::Turn) || - hasTurnType(step, TurnType::Continue) || - hasTurnType(step, TurnType::NewName) || - hasTurnType(step, TurnType::Suppressed)); + return (hasTurnType(step, TurnType::Turn) || hasTurnType(step, TurnType::Continue) || + hasTurnType(step, TurnType::NewName) || hasTurnType(step, TurnType::Suppressed)); }; // Process end of road step if (hasTurnType(step_at_turn_location, TurnType::EndOfRoad) || - hasTurnType(transfer_from_step, TurnType::EndOfRoad)) + hasTurnType(transfer_from_step, TurnType::EndOfRoad)) { // Keep end of road setInstructionType(step_at_turn_location, TurnType::EndOfRoad); @@ -350,17 +346,17 @@ void SegregatedTurnStrategy::operator()(RouteStep &step_at_turn_location, } // Process straight step else if ((turn_direction == guidance::DirectionModifier::Straight) && - is_straight_step(transfer_from_step)) + is_straight_step(transfer_from_step)) { // Determine if continue or new name setInstructionType(step_at_turn_location, - (haveSameName(step_prior_to_intersection, transfer_from_step) - ? TurnType::Suppressed - : TurnType::NewName)); + (haveSameName(step_prior_to_intersection, transfer_from_step) + ? TurnType::Suppressed + : TurnType::NewName)); } // Process turn step else if ((turn_direction != guidance::DirectionModifier::Straight) && - is_turn_step(transfer_from_step)) + is_turn_step(transfer_from_step)) { // Mark as turn setInstructionType(step_at_turn_location, TurnType::Turn); From 492e2fa835aadf9632198615f1a3fa6d20dfaff3 Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Wed, 21 Mar 2018 09:57:46 -0400 Subject: [PATCH 25/32] Fixes segregated indentification to not mark `circular` edge as segregated --- src/guidance/segregated_intersection_classification.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/guidance/segregated_intersection_classification.cpp b/src/guidance/segregated_intersection_classification.cpp index 50df992884c..08e8f97fcd9 100644 --- a/src/guidance/segregated_intersection_classification.cpp +++ b/src/guidance/segregated_intersection_classification.cpp @@ -118,8 +118,8 @@ std::unordered_set findSegregatedNodes(const extractor::NodeBasedGraphFa // Also they must be a road use (not footway, cycleway, etc.) // TODO - consider whether alleys, cul-de-sacs, and other road uses // are candidates to be marked as internal intersection edges. - // TODO adjust length as needed with lamda - if (edge_length > INTERNAL_LENGTH_MAX || current.flags.roundabout) + // TODO adjust length as needed with lambda + if (edge_length > INTERNAL_LENGTH_MAX || current.flags.roundabout || current.flags.circular) { return false; } From 6ed640c8da603032d71ebdbda6aaa323b7a7cd14 Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Mon, 26 Mar 2018 10:45:17 -0400 Subject: [PATCH 26/32] Added intersection prior to turn so we still call out end of road --- features/guidance/collapse.feature | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/features/guidance/collapse.feature b/features/guidance/collapse.feature index 81961898a2a..e7b4dad59ad 100644 --- a/features/guidance/collapse.feature +++ b/features/guidance/collapse.feature @@ -363,10 +363,11 @@ Feature: Collapse Scenario: Entering a segregated road Given the node map """ - a f g - | | . ' - b-e ' - / / + h + a f | g + | | i ' + b-e ' | + / / j / / c d """ @@ -376,7 +377,8 @@ Feature: Collapse | abc | primary | first | yes | | def | primary | first | yes | | be | primary | first | no | - | ge | primary | second | no | + | gie | primary | second | no | + | hij | primary | maple | no | When I route I should get | waypoints | route | turns | locations | @@ -385,7 +387,7 @@ Feature: Collapse | a,g | first,second,second | depart,turn left,arrive | a,b,g | | d,g | first,second,second | depart,turn right,arrive | d,e,g | | g,f | second,first,first | depart,turn right,arrive | g,e,f | - | g,c | second,first,first | depart,end of road left,arrive | g,b,c | + | g,c | second,first,first | depart,end of road left,arrive | g,e,c | Scenario: Do not collapse turning roads Given the node map From 7027cb193522e983a8b812bb83bb22a24992bb04 Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Mon, 26 Mar 2018 10:48:01 -0400 Subject: [PATCH 27/32] updated expectation to be turn instead of continue --- features/guidance/obvious-turn-discovery.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/guidance/obvious-turn-discovery.feature b/features/guidance/obvious-turn-discovery.feature index c510bc79042..414894e7cbb 100644 --- a/features/guidance/obvious-turn-discovery.feature +++ b/features/guidance/obvious-turn-discovery.feature @@ -1072,7 +1072,7 @@ Feature: Simple Turns When I route I should get | from | to | route | turns | | g | c | woll,brei,brei | depart,turn slight right,arrive | - | g | f | woll,scho,scho | depart,continue sharp left,arrive | + | g | f | woll,scho,scho | depart,turn sharp left,arrive | | a | c | scho,brei | depart,arrive | | d | f | brei,scho | depart,arrive | From acad9f125c7944c7382a481e9bc7a0397aa31cc6 Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Mon, 26 Mar 2018 11:00:46 -0400 Subject: [PATCH 28/32] Confirmed with @oxidase that the lane information if correct - updated the expectation --- features/guidance/turn-lanes.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/guidance/turn-lanes.feature b/features/guidance/turn-lanes.feature index b7d053b2c05..f34774e9e78 100644 --- a/features/guidance/turn-lanes.feature +++ b/features/guidance/turn-lanes.feature @@ -301,7 +301,7 @@ Feature: Turn Lane Guidance | e,l | road,cross,cross | depart,turn right,arrive | ,none:false straight:false straight;right:true, | | i,h | cross,road,road | depart,turn right,arrive | ,, | | i,j | cross,cross | depart,arrive | ;;left:false straight:true, | - | i,l | cross,cross,cross | depart,continue uturn,arrive | ;,left:true straight:false;left:true straight:false;left:false straight:true, | + | i,l | cross,cross,cross | depart,continue uturn,arrive | ,left:true straight:false;left:true straight:false;left:true straight:false;left:false straight:true, | @partition-lanes Scenario: Turn Lanes at Segregated Road From 468bbeca3f6b8d32dbbd8cdc662e9393432807c3 Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Tue, 27 Mar 2018 20:40:01 -0400 Subject: [PATCH 29/32] Added logic to handle wider straights Fixed tests --- .../engine/guidance/collapsing_utility.hpp | 66 +++++++++++++++++++ src/engine/guidance/collapse_turns.cpp | 50 +++++++++----- 2 files changed, 99 insertions(+), 17 deletions(-) diff --git a/include/engine/guidance/collapsing_utility.hpp b/include/engine/guidance/collapsing_utility.hpp index c772dd38578..51ed8999ba3 100644 --- a/include/engine/guidance/collapsing_utility.hpp +++ b/include/engine/guidance/collapsing_utility.hpp @@ -23,6 +23,7 @@ const constexpr std::size_t MIN_END_OF_ROAD_INTERSECTIONS = std::size_t{2}; const constexpr double MAX_COLLAPSE_DISTANCE = 30.0; // a bit larger than 100 to avoid oscillation in tests const constexpr double NAME_SEGMENT_CUTOFF_LENGTH = 105.0; +const double constexpr STRAIGHT_ANGLE = 180.; // check if a step is completely without turn type inline bool hasTurnType(const RouteStep &step) @@ -236,6 +237,71 @@ inline bool bearingsAreReversed(const double bearing_in, const double bearing_ou return util::angularDeviation(left_turn_angle, 180) <= 35; } +// Returns true if the specified step has only one intersection +inline bool hasSingleIntersection(const RouteStep &step) +{ + return (step.intersections.size() == 1); +} + +// Returns true if the specified angle is a wider straight turn +inline bool isWiderStraight(const double angle) { return (angle >= 125 && angle <= 235); } + +// Returns the straightest intersecting edge turn for the specified step +inline double getStraightestIntersectingEdgeTurn(const RouteStep &step) +{ + const auto &intersection = step.intersections.front(); + const double bearing_in = util::bearing::reverse(intersection.bearings[intersection.in]); + double staightest_turn = 360.; + double staightest_delta = 360.; + + for (std::size_t i = 0; i < intersection.bearings.size(); ++i) + { + // Skip the in, out, and non-traversable edges + if ((i == intersection.in) || (i == intersection.out) || !intersection.entry.at(i)) + continue; + + double intersecting_turn = + util::bearing::angleBetween(bearing_in, intersection.bearings.at(i)); + double straight_delta = util::angularDeviation(intersecting_turn, STRAIGHT_ANGLE); + + if (straight_delta < staightest_delta) + { + staightest_delta = straight_delta; + staightest_turn = intersecting_turn; + } + } + return staightest_turn; +} + +// Returns true if the specified step has the straightest turn as compared to +// the intersecting edges +inline bool hasStraightestTurn(const RouteStep &step) +{ + const auto &intersection = step.intersections.front(); + const double path_turn = + util::bearing::angleBetween(util::bearing::reverse(intersection.bearings[intersection.in]), + intersection.bearings[intersection.out]); + + // Path turn must be a wider straight + if (isWiderStraight(path_turn)) + { + const double straightest_intersecting_turn = getStraightestIntersectingEdgeTurn(step); + const double path_straight_delta = util::angularDeviation(path_turn, STRAIGHT_ANGLE); + const double intersecting_straight_delta = + util::angularDeviation(straightest_intersecting_turn, STRAIGHT_ANGLE); + const double path_intersecting_delta = + util::angularDeviation(path_turn, straightest_intersecting_turn); + + // Add some fuzz - the delta between the path and intersecting turn must be greater + // than 10 in order to consider using the intersecting turn as the straightest + return ((path_intersecting_delta > 10.) + ? (path_straight_delta <= intersecting_straight_delta) + : true); + } + + return false; +} + } /* namespace guidance */ } /* namespace engine */ } /* namespace osrm */ diff --git a/src/engine/guidance/collapse_turns.cpp b/src/engine/guidance/collapse_turns.cpp index 8ec1067213b..e7fffba70d6 100644 --- a/src/engine/guidance/collapse_turns.cpp +++ b/src/engine/guidance/collapse_turns.cpp @@ -221,12 +221,16 @@ void AdjustToCombinedTurnStrategy::operator()(RouteStep &step_at_turn_location, if (hasTurnType(step_at_turn_location, TurnType::Suppressed)) { if (new_modifier == DirectionModifier::Straight) + { setInstructionType(step_at_turn_location, TurnType::NewName); + } else + { step_at_turn_location.maneuver.instruction.type = haveSameName(step_prior_to_intersection, transfer_from_step) ? TurnType::Continue : TurnType::Turn; + } } else if (hasTurnType(step_at_turn_location, TurnType::NewName) && hasTurnType(transfer_from_step, TurnType::Suppressed) && @@ -288,7 +292,7 @@ void StaggeredTurnStrategy::operator()(RouteStep &step_at_turn_location, void CombineSegregatedStepsStrategy::operator()(RouteStep &step_at_turn_location, const RouteStep &transfer_from_step) const { - // TODO + // Handle end of road if (hasTurnType(step_at_turn_location, TurnType::EndOfRoad) || hasTurnType(transfer_from_step, TurnType::EndOfRoad)) { @@ -304,16 +308,17 @@ SegregatedTurnStrategy::SegregatedTurnStrategy(const RouteStep &step_prior_to_in void SegregatedTurnStrategy::operator()(RouteStep &step_at_turn_location, const RouteStep &transfer_from_step) const { - const auto calculate_turn_direction = [](const RouteStep &entry_step, - const RouteStep &exit_step) { - const double angle = util::bearing::angleBetween(entry_step.maneuver.bearing_before, - exit_step.maneuver.bearing_after); + // Used to control updating of the modifier based on turn direction + bool update_modifier_for_turn_direction = true; - return getTurnDirection(angle); + const auto calculate_turn_angle = [](const RouteStep &entry_step, const RouteStep &exit_step) { + return util::bearing::angleBetween(entry_step.maneuver.bearing_before, + exit_step.maneuver.bearing_after); }; - // Calculate turn direction for segregated - const auto turn_direction = calculate_turn_direction(step_at_turn_location, transfer_from_step); + // Calculate turn angle and direction for segregated + const auto turn_angle = calculate_turn_angle(step_at_turn_location, transfer_from_step); + const auto turn_direction = getTurnDirection(turn_angle); const auto is_straight_step = [](const RouteStep &step) { return ((hasTurnType(step, TurnType::NewName) || hasTurnType(step, TurnType::Continue) || @@ -334,16 +339,28 @@ void SegregatedTurnStrategy::operator()(RouteStep &step_at_turn_location, { // Keep end of road setInstructionType(step_at_turn_location, TurnType::EndOfRoad); - - // Set modifier based on turn angle - setModifier(step_at_turn_location, turn_direction); } - // TODO maybe remove fork logic after updating identification logic // Process fork step at turn else if (hasTurnType(step_at_turn_location, TurnType::Fork)) { - // Keep fork - setInstructionType(step_at_turn_location, TurnType::Fork); + // Do not update modifier based on turn direction + update_modifier_for_turn_direction = false; + } + // Process wider straight step + else if (isWiderStraight(turn_angle) && hasSingleIntersection(step_at_turn_location) && + hasStraightestTurn(step_at_turn_location) && hasStraightestTurn(transfer_from_step)) + { + // Determine if continue or new name + setInstructionType(step_at_turn_location, + (haveSameName(step_prior_to_intersection, transfer_from_step) + ? TurnType::Suppressed + : TurnType::NewName)); + + // Set modifier to straight + setModifier(step_at_turn_location, osrm::guidance::DirectionModifier::Straight); + + // Do not update modifier based on turn direction + update_modifier_for_turn_direction = false; } // Process straight step else if ((turn_direction == guidance::DirectionModifier::Straight) && @@ -369,8 +386,8 @@ void SegregatedTurnStrategy::operator()(RouteStep &step_at_turn_location, setInstructionType(step_at_turn_location, transfer_from_step.maneuver.instruction.type); } - // If not fork then set modifier based on turn angle - if (!hasTurnType(step_at_turn_location, TurnType::Fork)) + // Update modifier based on turn direction, if needed + if (update_modifier_for_turn_direction) { setModifier(step_at_turn_location, turn_direction); } @@ -566,7 +583,6 @@ RouteSteps collapseTurnInstructions(RouteSteps steps) } } } - return steps; } From 6749f8b09794556455147587ded7746a4dc86087 Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Tue, 27 Mar 2018 20:58:08 -0400 Subject: [PATCH 30/32] Update CHANGELOG.md Added #4925 --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ccfdd140fe9..d106905d429 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,12 +11,13 @@ - Guidance: - CHANGED #4929: Don't use obviousness for links bifurcations [#4929](https://github.com/Project-OSRM/osrm-backend/pull/4929) - FIXED #4929: Adjust Straight direction modifiers of side roads in driveway handler [#4929](https://github.com/Project-OSRM/osrm-backend/pull/4929) + - CHANGED #4925: Added post process logic to collapse segregated turn instructions [#4925](https://github.com/Project-OSRM/osrm-backend/pull/4925) - Tools: - `osrm-routed` accepts a new property `--memory_file` to store memory in a file on disk. - NodeJS: - `OSRM` object accepts a new option `memory_file` that stores the memory in a file on disk. - Internals - - CHANGED #4845 #4968: Updated segregated intersection identification + - CHANGED #4845 #4968: Updated segregated intersection identification [#4845](https://github.com/Project-OSRM/osrm-backend/pull/4845) [#4968](https://github.com/Project-OSRM/osrm-backend/pull/4968) - Documentation: - ADDED: Add documentation about OSM node ids in nearest service response [#4436](https://github.com/Project-OSRM/osrm-backend/pull/4436) - Performance From 37f74ca745c995dd7a65b659094afbf61305f846 Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Wed, 28 Mar 2018 09:07:55 -0400 Subject: [PATCH 31/32] Removed TODO --- include/engine/api/route_api.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/include/engine/api/route_api.hpp b/include/engine/api/route_api.hpp index 5eb925e9d26..1e6c9764743 100644 --- a/include/engine/api/route_api.hpp +++ b/include/engine/api/route_api.hpp @@ -149,7 +149,6 @@ class RouteAPI : public BaseAPI guidance::applyOverrides(BaseAPI::facade, steps, leg_geometry); // Collapse segregated steps before others - // TODO: before or after overrides? steps = guidance::collapseSegregatedTurnInstructions(std::move(steps)); /* Perform step-based post-processing. From 2f341b16057784ff0ba4bd09d403ef6eb22c8e1b Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Thu, 29 Mar 2018 21:22:50 -0400 Subject: [PATCH 32/32] Process straight step prior to wider straight step --- src/engine/guidance/collapse_turns.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/engine/guidance/collapse_turns.cpp b/src/engine/guidance/collapse_turns.cpp index e7fffba70d6..be3680827d8 100644 --- a/src/engine/guidance/collapse_turns.cpp +++ b/src/engine/guidance/collapse_turns.cpp @@ -346,6 +346,16 @@ void SegregatedTurnStrategy::operator()(RouteStep &step_at_turn_location, // Do not update modifier based on turn direction update_modifier_for_turn_direction = false; } + // Process straight step + else if ((turn_direction == guidance::DirectionModifier::Straight) && + is_straight_step(transfer_from_step)) + { + // Determine if continue or new name + setInstructionType(step_at_turn_location, + (haveSameName(step_prior_to_intersection, transfer_from_step) + ? TurnType::Suppressed + : TurnType::NewName)); + } // Process wider straight step else if (isWiderStraight(turn_angle) && hasSingleIntersection(step_at_turn_location) && hasStraightestTurn(step_at_turn_location) && hasStraightestTurn(transfer_from_step)) @@ -362,16 +372,6 @@ void SegregatedTurnStrategy::operator()(RouteStep &step_at_turn_location, // Do not update modifier based on turn direction update_modifier_for_turn_direction = false; } - // Process straight step - else if ((turn_direction == guidance::DirectionModifier::Straight) && - is_straight_step(transfer_from_step)) - { - // Determine if continue or new name - setInstructionType(step_at_turn_location, - (haveSameName(step_prior_to_intersection, transfer_from_step) - ? TurnType::Suppressed - : TurnType::NewName)); - } // Process turn step else if ((turn_direction != guidance::DirectionModifier::Straight) && is_turn_step(transfer_from_step))