From 0d36d472c92bfcbab692d2e5a98a5dcf3ffa36c3 Mon Sep 17 00:00:00 2001 From: Moritz Kobitzsch Date: Thu, 14 Jul 2016 15:05:46 +0200 Subject: [PATCH] change paradigm of merge to only emit on motorway-like roads --- CHANGELOG.md | 1 + features/guidance/anticipate-lanes.feature | 6 +- features/guidance/collapse.feature | 53 +++++++++++++ .../guidance/dedicated-turn-roads.feature | 4 +- features/guidance/fork.feature | 6 +- features/guidance/merge.feature | 78 ++++++++++++++----- features/guidance/turn-lanes.feature | 6 +- features/testbot/bearing_param.feature | 18 ++--- include/extractor/guidance/constants.hpp | 2 +- include/util/debug.hpp | 15 ++++ src/engine/guidance/post_processing.cpp | 15 ++++ .../guidance/intersection_handler.cpp | 41 +++++++++- src/extractor/guidance/turn_analysis.cpp | 1 - 13 files changed, 202 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 18f989b35..102702438 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Handle Access tags for lanes, only considering valid lanes in lane-guidance (think car | car | bike | car) - API: - `annotations=true` now returns the data source id for each segment as `datasources` + - Reduced semantic of merge to refer only to merges from a lane onto a motorway-like road # 5.3.0 Changes from 5.3.0-rc.3 diff --git a/features/guidance/anticipate-lanes.feature b/features/guidance/anticipate-lanes.feature index 19d58194d..ac6f12530 100644 --- a/features/guidance/anticipate-lanes.feature +++ b/features/guidance/anticipate-lanes.feature @@ -135,9 +135,9 @@ Feature: Turn Lane Guidance | cj | | 1 | motorway_link | yes | xbcj | When I route I should get - | waypoints | route | turns | lanes | - | a,i | ab,xbcj,ci,ci | depart,merge slight left,turn slight right,arrive | ,,none:false slight right:true, | - | a,j | ab,xbcj,xbcj | depart,merge slight left,arrive | ,, | + | waypoints | route | turns | lanes | + | a,i | ab,ci,ci | depart,turn slight right,arrive | ,none:false slight right:true, | + | a,j | ab,xbcj | depart,arrive | , | @anticipate diff --git a/features/guidance/collapse.feature b/features/guidance/collapse.feature index 709cf7cc8..a9ac1271c 100644 --- a/features/guidance/collapse.feature +++ b/features/guidance/collapse.feature @@ -506,6 +506,36 @@ Feature: Collapse | icf | secondary | in | yes | | gbj | secondary | out | yes | + When I route I should get + | waypoints | route | turns | + | i,h | in,road,road | depart,turn left,arrive | + | a,d | road,road | depart,arrive | + | a,j | road,out,out | depart,turn slight right,arrive | + + Scenario: Segregated Intersection into Very Slight Turn + Given the node map + | h | | | | | | | + | a | | | | | | | + | | | | | | | | + | | | | | | | | + | | | g | | | | | + | | | b | | | | | + | | | | f | | | | + | | | | c | | | | + | | | | | | | | + | | | | | | | | + | | | | | | | | + | | | | | | | e | + | | | | | | | d | + | | | j | i | | | | + + And the ways + | nodes | highway | name | oneway | + | abcd | primary | road | yes | + | efgh | primary | road | yes | + | icf | secondary | in | yes | + | gbj | secondary | out | yes | + When I route I should get | waypoints | route | turns | | i,h | in,road,road | depart,turn slight left,arrive | @@ -630,3 +660,26 @@ Feature: Collapse | f,d | on,Hwy,Hwy | depart,merge slight right,arrive | | f,e | on,Hwy,off,off | depart,merge slight right,off ramp right,arrive | | a,e | Hwy,off,off | depart,off ramp right,arrive | + + @negative @straight + Scenario: Don't collapse going straight if actual turn + Given the node map + | | c | e | | | + | | | d | | f | + | | | | | | + | | | b | | | + | | | | | | + | | | | | | + | | | a | | | + + And the ways + | nodes | name | highway | + | abc | main | primary | + | bde | straight | residential | + | df | right | residential | + + When I route I should get + | waypoints | route | turns | + | a,c | main,main | depart,arrive | + | a,e | main,straight,straight | depart,turn straight,arrive | + | a,f | main,straight,right,right | depart,turn straight,turn right,arrive | diff --git a/features/guidance/dedicated-turn-roads.feature b/features/guidance/dedicated-turn-roads.feature index 974ad95a3..1f0f1a1cb 100644 --- a/features/guidance/dedicated-turn-roads.feature +++ b/features/guidance/dedicated-turn-roads.feature @@ -52,8 +52,8 @@ Feature: Slipways and Dedicated Turn Lanes | efg | primary | second | When I route I should get - | waypoints | route | turns | - | a,g | first,,second,second | depart,off ramp slight right,merge slight left,arrive | + | waypoints | route | turns | + | a,g | first,,second,second | depart,off ramp slight right,turn straight,arrive | Scenario: Inner city expressway with on road Given the node map diff --git a/features/guidance/fork.feature b/features/guidance/fork.feature index 2c8d59217..047d897bd 100644 --- a/features/guidance/fork.feature +++ b/features/guidance/fork.feature @@ -311,6 +311,6 @@ Feature: Fork Instructions | ab | on | motorway_link | When I route I should get - | waypoints | route | turns | - | a,j | on,xbcj,xbcj | depart,merge slight left,arrive | - | a,i | on,xbcj,off,off | depart,merge slight left,turn slight right,arrive | + | waypoints | route | turns | + | a,j | on,xbcj | depart,arrive | + | a,i | on,off,off | depart,turn slight right,arrive | diff --git a/features/guidance/merge.feature b/features/guidance/merge.feature index 0190c8b89..2b52ba75d 100644 --- a/features/guidance/merge.feature +++ b/features/guidance/merge.feature @@ -3,13 +3,14 @@ Feature: Merging Background: Given the profile "car" - Given a grid size of 10 meters + And a grid size of 10 meters + @merge Scenario: Merge on Four Way Intersection Given the node map - | d | | | - | a | b | c | - | e | | | + | d | | | | | | | | | | + | a | | b | | | | | | | c | + | e | | | | | | | | | | And the ways | nodes | highway | @@ -18,14 +19,15 @@ Feature: Merging | eb | primary | When I route I should get - | waypoints | route | turns | - | d,c | db,abc,abc | depart,merge slight right,arrive | - | e,c | eb,abc,abc | depart,merge slight left,arrive | + | waypoints | route | turns | + | d,c | db,abc,abc | depart,turn straight,arrive | + | e,c | eb,abc,abc | depart,turn straight,arrive | + @merge Scenario: Merge on Three Way Intersection Right Given the node map - | d | | | - | a | b | c | + | d | | | | | | | | | | + | a | | b | | | | | | | c | And the ways | nodes | highway | @@ -33,13 +35,14 @@ Feature: Merging | db | primary | When I route I should get - | waypoints | route | turns | - | d,c | db,abc,abc | depart,merge slight right,arrive | + | waypoints | route | turns | + | d,c | db,abc,abc | depart,turn straight,arrive | - Scenario: Merge on Three Way Intersection Right + @merge @negative + Scenario: Don't Merge on Short-Three Way Intersection Right Given the node map - | a | b | c | - | d | | | + | d | | | | | | | | + | a | | b | | | | | c | And the ways | nodes | highway | @@ -47,15 +50,37 @@ Feature: Merging | db | primary | When I route I should get - | waypoints | route | turns | - | d,c | db,abc,abc | depart,merge slight left,arrive | + | waypoints | route | turns | + | d,c | db,abc,abc | depart,turn slight left,arrive | + + @merge + Scenario: Merge on Three Way Intersection Right + Given the node map + | a | | b | | | | | | | c | + | d | | | | | | | | | | + + And the ways + | nodes | highway | + | abc | primary | + | db | primary | + + When I route I should get + | waypoints | route | turns | + | d,c | db,abc,abc | depart,turn straight,arrive | + + @merge Scenario: Merge onto a turning road Given the node map | | | | | | | e | | | | | | | | | | | | | | | | | | | | | | | | | + | | | | | | | | + | | | | | | | | + | | | | | | | | + | | | | | | | | + | | | | | | | | | | | | | | d | | | | | | | | | | | | | | | | | | @@ -71,6 +96,21 @@ Feature: Merging | fd | residential | in | When I route I should get - | waypoints | turns | route | - | f,e | depart,merge slight left,arrive | in,road,road | - | f,a | depart,turn sharp left,arrive | in,road,road | + | waypoints | turns | route | + | f,e | depart,turn straight,arrive | in,road,road | + | f,a | depart,turn sharp left,arrive | in,road,road | + + @merge + Scenario: Merge onto a motorway + Given the node map + | d | | | | | | | | | | + | a | | | b | | | | | | c | + + And the ways + | nodes | name | highway | oneway | + | abc | A100 | motorway | yes | + | db | | motorway_link | yes | + + When I route I should get + | waypoints | route | turns | + | d,c | ,A100,A100 | depart,merge slight right,arrive | diff --git a/features/guidance/turn-lanes.feature b/features/guidance/turn-lanes.feature index afb8fff3d..f6ea81d3a 100644 --- a/features/guidance/turn-lanes.feature +++ b/features/guidance/turn-lanes.feature @@ -696,9 +696,9 @@ Feature: Turn Lane Guidance | ab | on | motorway_link | | When I route I should get - | waypoints | route | turns | lanes | - | a,j | on,xbcj,xbcj | depart,merge slight left,arrive | ,, | - | a,i | on,xbcj,off,off | depart,merge slight left,turn slight right,arrive | ,,none:false slight right:true, | + | waypoints | route | turns | lanes | + | a,j | on,xbcj | depart,arrive | , | + | a,i | on,off,off | depart,turn slight right,arrive | ,none:false slight right:true, | #http://www.openstreetmap.org/#map=17/52.47414/13.35712 @todo @ramp @2645 diff --git a/features/testbot/bearing_param.feature b/features/testbot/bearing_param.feature index 8650398c4..7693cdb8a 100644 --- a/features/testbot/bearing_param.feature +++ b/features/testbot/bearing_param.feature @@ -104,12 +104,12 @@ Feature: Bearing parameter | ha | yes | ring | When I route I should get - | from | to | bearings | route | bearing | - | 0 | q | 0 90 | ia,ring,ring,ring,ring,ring | 0->0,0->90,180->270,270->0,0->90,90->0 | - | 0 | a | 45 90 | jb,ring,ring,ring,ring,ring | 0->45,45->180,180->270,270->0,0->90,90->0 | - | 0 | q | 90 90 | kc,ring,ring,ring,ring | 0->90,90->180,270->0,0->90,90->0 | - | 0 | a | 135 90 | ld,ring,ring,ring,ring | 0->135,135->270,270->0,0->90,90->0 | - | 0 | a | 180 90 | me,ring,ring,ring | 0->180,180->270,0->90,90->0 | - | 0 | a | 225 90 | nf,ring,ring,ring | 0->225,225->0,0->90,90->0 | - | 0 | a | 270 90 | og,ring,ring | 0->270,270->0,90->0 | - | 0 | a | 315 90 | ph,ring,ring | 0->315,315->90,90->0 | + | from | to | bearings | route | bearing | + | 0 | q | 0 90 | ia,ring,ring | 0->0,0->90,90->0 | + | 0 | a | 45 90 | jb,ring,ring | 0->45,45->180,90->0 | + | 0 | q | 90 90 | kc,ring,ring | 0->90,90->180,90->0 | + | 0 | a | 135 90 | ld,ring,ring | 0->135,135->270,90->0 | + | 0 | a | 180 90 | me,ring,ring | 0->180,180->270,90->0 | + | 0 | a | 225 90 | nf,ring,ring | 0->225,225->0,90->0 | + | 0 | a | 270 90 | og,ring,ring | 0->270,270->0,90->0 | + | 0 | a | 315 90 | ph,ring,ring | 0->315,315->90,90->0 | diff --git a/include/extractor/guidance/constants.hpp b/include/extractor/guidance/constants.hpp index 54729ba7e..f6a0f4309 100644 --- a/include/extractor/guidance/constants.hpp +++ b/include/extractor/guidance/constants.hpp @@ -18,7 +18,7 @@ const double constexpr MAXIMAL_ALLOWED_NO_TURN_DEVIATION = 3.; const double constexpr NARROW_TURN_ANGLE = 40.; const double constexpr GROUP_ANGLE = 60; // angle difference that can be classified as straight, if its the only narrow turn -const double constexpr FUZZY_ANGLE_DIFFERENCE = 20.; +const double constexpr FUZZY_ANGLE_DIFFERENCE = 25.; const double constexpr DISTINCTION_RATIO = 2; const double constexpr MAX_ROUNDABOUT_INTERSECTION_RADIUS = 5; diff --git a/include/util/debug.hpp b/include/util/debug.hpp index 89d3565e4..843ec28fd 100644 --- a/include/util/debug.hpp +++ b/include/util/debug.hpp @@ -5,6 +5,7 @@ #include "extractor/guidance/turn_lane_data.hpp" #include "extractor/query_node.hpp" #include "engine/guidance/route_step.hpp" +#include "util/node_based_graph.hpp" #include "util/typedefs.hpp" #include @@ -62,6 +63,20 @@ inline void print(const extractor::guidance::Intersection &intersection) std::cout << std::flush; } +inline void print(const NodeBasedDynamicGraph &node_based_graph, + const extractor::guidance::Intersection &intersection) +{ + std::cout << " Intersection:\n"; + for (const auto &road : intersection) + { + std::cout << "\t" << toString(road) << "\n"; + std::cout << "\t\t" + << node_based_graph.GetEdgeData(road.turn.eid).road_classification.ToString() + << "\n"; + } + std::cout << std::flush; +} + inline void print(const extractor::guidance::lanes::LaneDataVector &turn_lane_data) { std::cout << " Tags:\n"; diff --git a/src/engine/guidance/post_processing.cpp b/src/engine/guidance/post_processing.cpp index 05752669b..6a8dfd823 100644 --- a/src/engine/guidance/post_processing.cpp +++ b/src/engine/guidance/post_processing.cpp @@ -37,6 +37,13 @@ namespace const constexpr std::size_t MIN_END_OF_ROAD_INTERSECTIONS = std::size_t{2}; const constexpr double MAX_COLLAPSE_DISTANCE = 30; +// check if at least one of the turns is actually a maneuver +inline bool hasManeuver(const RouteStep &first, const RouteStep &second) +{ + return first.maneuver.instruction.type != TurnType::Suppressed || + second.maneuver.instruction.type != TurnType::Suppressed; +} + inline bool choiceless(const RouteStep &step, const RouteStep &previous) { // if the next turn is choiceless, we consider longer turn roads collapsable than usually @@ -375,6 +382,10 @@ void collapseTurnAt(std::vector &steps, }; BOOST_ASSERT(!one_back_step.intersections.empty() && !current_step.intersections.empty()); + + if (!hasManeuver(one_back_step, current_step)) + return; + // Very Short New Name if (((collapsable(one_back_step) || (isCollapsableInstruction(one_back_step.maneuver.instruction) && @@ -661,6 +672,10 @@ std::vector collapseTurns(std::vector steps) BOOST_ASSERT(one_back_index < steps.size()); const auto &one_back_step = steps[one_back_index]; + + if (!hasManeuver(one_back_step, current_step)) + continue; + // how long has a name change to be so that we announce it, even as a bridge? const constexpr auto name_segment_cutoff_length = 100; const auto isBasicNameChange = [](const RouteStep &step) { diff --git a/src/extractor/guidance/intersection_handler.cpp b/src/extractor/guidance/intersection_handler.cpp index 4a821665c..5674a6531 100644 --- a/src/extractor/guidance/intersection_handler.cpp +++ b/src/extractor/guidance/intersection_handler.cpp @@ -2,6 +2,7 @@ #include "extractor/guidance/constants.hpp" #include "extractor/guidance/toolkit.hpp" +#include "util/coordinate_calculation.hpp" #include "util/guidance/toolkit.hpp" #include "util/simple_logger.hpp" @@ -93,9 +94,43 @@ TurnInstruction IntersectionHandler::getInstructionForObvious(const std::size_t // obvious turn onto a through street is a merge if (through_street) { - return {TurnType::Merge, - road.turn.angle > STRAIGHT_ANGLE ? DirectionModifier::SlightRight - : DirectionModifier::SlightLeft}; + // We reserve merges for motorway types. All others are considered for simply going + // straight onto a road. This avoids confusion about merge directions on streets + // that could potentially also offer different choices + if (out_data.road_classification.IsMotorwayClass()) + return {TurnType::Merge, + road.turn.angle > STRAIGHT_ANGLE ? DirectionModifier::SlightRight + : DirectionModifier::SlightLeft}; + else if (in_data.road_classification.IsRampClass() && + out_data.road_classification.IsRampClass()) + { + if (in_mode == out_mode) + return {TurnType::Suppressed, getTurnDirection(road.turn.angle)}; + else + return {TurnType::Notification, getTurnDirection(road.turn.angle)}; + } + else + { + const double constexpr MAX_COLLAPSE_DISTANCE = 30; + // in normal road condidtions, we check if the turn is nearly straight. + // Doing so, we widen the angle that a turn is considered straight, but since it + // is obvious, the choice is arguably better. + + // FIXME this requires https://github.com/Project-OSRM/osrm-backend/pull/2399, + // since `distance` does not refer to an actual distance but rather to the + // duration/weight of the traversal. We can only approximate the distance here + // or actually follow the full road. When 2399 lands, we can exchange here for a + // precalculated distance value. + const auto distance = util::coordinate_calculation::haversineDistance( + node_info_list[node_based_graph.GetTarget(via_edge)], + node_info_list[node_based_graph.GetTarget(road.turn.eid)]); + return {TurnType::Turn, + (angularDeviation(road.turn.angle, STRAIGHT_ANGLE) < + FUZZY_ANGLE_DIFFERENCE || + distance > 2 * MAX_COLLAPSE_DISTANCE) + ? DirectionModifier::Straight + : getTurnDirection(road.turn.angle)}; + } } else { diff --git a/src/extractor/guidance/turn_analysis.cpp b/src/extractor/guidance/turn_analysis.cpp index 69d989f32..92c5e6f66 100644 --- a/src/extractor/guidance/turn_analysis.cpp +++ b/src/extractor/guidance/turn_analysis.cpp @@ -93,7 +93,6 @@ Intersection TurnAnalysis::assignTurnTypes(const NodeID from_nid, road.turn.instruction.type = TurnType::OffRamp; }); } - return intersection; }