Merge pull request #3324 from Project-OSRM/constrain-staggering

Constrain staggered intersection detection by mode change and intermediary intersections

features/guidance/bike-staggered-intersections.feature

@@ -0,0 +1,139 @@
+@routing  @guidance @staggered-intersections
+Feature: Staggered Intersections
+
+    Background:
+        Given the profile "bicycle"
+        Given a grid size of 1 meters
+        # Note the one meter grid size: staggered intersections make zig-zags of a couple of meters only
+
+    Scenario: Staggered Intersection - pushing in the middle
+        Given the node map
+            """
+                j
+            a b c
+                d
+                e f g
+                h
+                i
+            """
+
+        And the ways
+            | nodes  | highway     | name     | oneway |
+            | abc    | residential | Oak St   |        |
+            | efg    | residential | Oak St   |        |
+            | ihedcj | residential | Cedar Dr | yes    |
+
+        When I route I should get
+            | waypoints | route                         | turns                              | modes                                |
+            | a,g       | Oak St,Cedar Dr,Oak St,Oak St | depart,turn right,turn left,arrive | cycling,pushing bike,cycling,cycling |
+            | g,a       | Oak St,Oak St                 | depart,arrive                      | cycling,cycling                      |
+
+    Scenario: Staggered Intersection - pushing at start
+        Given the node map
+            """
+                j
+            a b c
+                d
+                e f g
+                h
+                i
+            """
+
+        And the ways
+            | nodes  | highway     | name     | oneway |
+            | cba    | residential | Oak St   | yes    |
+            | efg    | residential | Oak St   |        |
+            | ihedcj | residential | Cedar Dr |        |
+
+        When I route I should get
+            | waypoints | route                         | turns                              | modes                                |
+            | a,g       | Oak St,Cedar Dr,Oak St,Oak St | depart,turn right,turn left,arrive | pushing bike,cycling,cycling,cycling |
+            | g,a       | Oak St,Oak St                 | depart,arrive                      | cycling,cycling                      |
+
+    Scenario: Staggered Intersection - pushing at end
+        Given the node map
+            """
+                j
+            a b c
+                d
+                e f g
+                h
+                i
+            """
+
+        And the ways
+            | nodes  | highway     | name     | oneway |
+            | abc    | residential | Oak St   |        |
+            | gfe    | residential | Oak St   | yes    |
+            | ihedcj | residential | Cedar Dr |        |
+
+        When I route I should get
+            | waypoints | route         | turns                                              | modes                                     |
+            | a,g       | Oak St,Cedar Dr,Oak St,Oak St | depart,turn right,turn left,arrive | cycling,cycling,pushing bike,pushing bike |
+            | g,a       | Oak St,Oak St | depart,arrive                                      | cycling,cycling                           |
+
+    Scenario: Staggered Intersection - pushing at start and end
+        Given the node map
+            """
+                j
+            a b c
+                d
+                e f g
+                h
+                i
+            """
+
+        And the ways
+            | nodes  | highway     | name     | oneway |
+            | cba    | residential | Oak St   | yes    |
+            | gfe    | residential | Oak St   | yes    |
+            | ihedcj | residential | Cedar Dr |        |
+
+        When I route I should get
+            | waypoints | route                         | turns                              | modes                                          |
+            | a,g       | Oak St,Cedar Dr,Oak St,Oak St | depart,turn right,turn left,arrive | pushing bike,cycling,pushing bike,pushing bike |
+            | g,a       | Oak St,Oak St                 | depart,arrive                      | cycling,cycling                                |
+
+    Scenario: Staggered Intersection - pushing at start and end
+        Given the node map
+            """
+                j
+            a b c
+                d
+                e f g
+                h
+                i
+            """
+
+        And the ways
+            | nodes  | highway     | name     |
+            | cba    | pedestrian  | Oak St   |
+            | gfe    | pedestrian  | Oak St   |
+            | ihedcj | residential | Cedar Dr |
+
+        When I route I should get
+            | waypoints | route                         | turns                              | modes                                          |
+            | a,g       | Oak St,Cedar Dr,Oak St,Oak St | depart,turn right,turn left,arrive | pushing bike,cycling,pushing bike,pushing bike |
+            | g,a       | Oak St,Cedar Dr,Oak St,Oak St | depart,turn right,turn left,arrive | pushing bike,cycling,pushing bike,pushing bike |
+
+    Scenario: Staggered Intersection - control, all cycling on staggered intersection
+        Given the node map
+            """
+                j
+            a b c
+                d
+                e f g
+                h
+                i
+            """
+
+        And the ways
+            | nodes  | highway     | name     |
+            | cba    | residential | Oak St   |
+            | gfe    | residential | Oak St   |
+            | ihedcj | residential | Cedar Dr |
+
+        When I route I should get
+            | waypoints | route         | turns         | modes           |
+            | a,g       | Oak St,Oak St | depart,arrive | cycling,cycling |
+            | g,a       | Oak St,Oak St | depart,arrive | cycling,cycling |

features/guidance/staggered-intersections.feature

@@ -98,3 +98,48 @@ Feature: Staggered Intersections
             | waypoints | route                         | turns                              |
             | a,g       | Oak St,Cedar Dr,Elm St,Elm St | depart,turn right,turn left,arrive |
             | g,a       | Elm St,Cedar Dr,Oak St,Oak St | depart,turn right,turn left,arrive |
+
+    Scenario: Staggered Intersection: do not collapse if a mode change is involved
+        Given the node map
+            """
+                j
+            a b c
+                d
+                e  f g
+                h
+            """
+
+        And the ways
+            | nodes  | highway | name     | route |
+            | abc    | primary | to_sea   |       |
+            | ef     |         | to_sea   | ferry |
+            | fg     | primary | road     |       |
+            | jcdeh  | primary | road     |       |
+
+        When I route I should get
+            | waypoints | route                          | turns                                                    | modes                                 |
+            | a,g       | to_sea,road,to_sea,road,road   | depart,turn right,turn left,notification straight,arrive | driving,driving,ferry,driving,driving |
+            | g,a       | road,to_sea,road,to_sea,to_sea | depart,notification straight,turn right,turn left,arrive | driving,ferry,driving,driving,driving |
+
+    Scenario: Staggered Intersection: do not collapse intermediary intersections
+        Given the node map
+            """
+                j
+            a b c
+                e f g
+                d
+                k l m
+                i
+            """
+
+        And the ways
+            | nodes   | highway     | name     |
+            | abc     | primary     | Oak St   |
+            | efg     | residential | Elm St   |
+            | klm     | residential | Oak St   |
+            | jcedki  | residential | Cedar Dr |
+
+        When I route I should get
+            | waypoints | route         | turns         |
+            | a,m       | Oak St,Cedar Dr,Oak St,Oak St | depart,turn right,turn left,arrive |
+            | m,a       | Oak St,Cedar Dr,Oak St,Oak St | depart,turn right,turn left,arrive |

src/engine/guidance/post_processing.cpp

@@ -1,6 +1,6 @@
-#include "engine/guidance/post_processing.hpp"
 #include "extractor/guidance/constants.hpp"
 #include "extractor/guidance/turn_instruction.hpp"
+#include "engine/guidance/post_processing.hpp"
 #include "engine/guidance/toolkit.hpp"
 
 #include "engine/guidance/assemble_steps.hpp"
@@ -443,7 +443,7 @@ std::size_t getPreviousIndex(std::size_t index, const std::vector<RouteStep> &st
         --index;
 
     return index;
-};
+}
 
 void collapseUTurn(std::vector<RouteStep> &steps,
                    const std::size_t two_back_index,
@@ -528,32 +528,31 @@ void collapseTurnAt(std::vector<RouteStep> &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 &openining_turn,
+    const auto isDelayedTurn = [](const RouteStep &opening_turn, const RouteStep &finishing_turn) {
-                                  const RouteStep &finishing_turn) {
         // only possible if both are compatible
-        if (!compatible(openining_turn, finishing_turn))
+        if (!compatible(opening_turn, finishing_turn))
             return false;
         else
         {
             const auto is_short_and_collapsable =
-                openining_turn.distance <= MAX_COLLAPSE_DISTANCE &&
+                opening_turn.distance <= MAX_COLLAPSE_DISTANCE &&
                 isCollapsableInstruction(finishing_turn.maneuver.instruction);
 
-            const auto without_choice = choiceless(finishing_turn, openining_turn);
+            const auto without_choice = choiceless(finishing_turn, opening_turn);
 
             const auto is_not_too_long_and_choiceless =
-                openining_turn.distance <= 2 * MAX_COLLAPSE_DISTANCE && without_choice;
+                opening_turn.distance <= 2 * MAX_COLLAPSE_DISTANCE && without_choice;
 
             // for ramps we allow longer stretches, since they are often on some major brides/large
             // roads. A combined distance of of 4 intersections would be to long for a normal
             // collapse. In case of a ramp though, we also account for situations that have the ramp
             // tagged late
             const auto is_delayed_turn_onto_a_ramp =
-                openining_turn.distance <= 4 * MAX_COLLAPSE_DISTANCE && without_choice &&
+                opening_turn.distance <= 4 * MAX_COLLAPSE_DISTANCE && without_choice &&
                 util::guidance::hasRampType(finishing_turn.maneuver.instruction);
-            return !util::guidance::hasRampType(openining_turn.maneuver.instruction) &&
+            return !util::guidance::hasRampType(opening_turn.maneuver.instruction) &&
                    (is_short_and_collapsable || is_not_too_long_and_choiceless ||
-                    isLinkroad(openining_turn) || is_delayed_turn_onto_a_ramp);
+                    isLinkroad(opening_turn) || is_delayed_turn_onto_a_ramp);
         }
     };
 
@@ -816,13 +815,17 @@ void collapseTurnAt(std::vector<RouteStep> &steps,
 //      |       or       |       becomes  a   ->   b
 // a -> *                * -> b
 //
-bool isStaggeredIntersection(const RouteStep &previous, const RouteStep &current)
+bool isStaggeredIntersection(const std::vector<RouteStep> &steps,
+                             const std::size_t &current_index,
+                             const std::size_t &previous_index)
 {
+    const RouteStep previous = steps[previous_index];
+    const RouteStep current = steps[current_index];
+
     // don't touch roundabouts
     if (entersRoundabout(previous.maneuver.instruction) ||
         entersRoundabout(current.maneuver.instruction))
         return false;
-
     // Base decision on distance since the zig-zag is a visual clue.
     // If adjusted, make sure to check validity of the is_right/is_left classification below
     const constexpr auto MAX_STAGGERED_DISTANCE = 3; // debatable, but keep short to be on safe side
@@ -853,7 +856,21 @@ bool isStaggeredIntersection(const RouteStep &previous, const RouteStep &current
     // We are only interested in the distance between the first and the second.
     const auto is_short = previous.distance < MAX_STAGGERED_DISTANCE;
 
-    return is_short && (left_right || right_left);
+    auto intermediary_mode_change = false;
+    if (current_index > 1)
+    {
+        const auto &two_back_index = getPreviousIndex(previous_index, steps);
+        const auto two_back_step = steps[two_back_index];
+        intermediary_mode_change =
+            two_back_step.mode == current.mode && previous.mode != current.mode;
+    }
+
+    // previous step maneuver intersections should be length 1 to indicate that
+    // there are no intersections between the two potentially collapsible turns
+    const auto no_intermediary_intersections = previous.intersections.size() == 1;
+
+    return is_short && (left_right || right_left) && !intermediary_mode_change &&
+           no_intermediary_intersections;
 }
 
 } // namespace
@@ -1162,13 +1179,13 @@ std::vector<RouteStep> collapseTurns(std::vector<RouteStep> steps)
                 invalidateStep(steps[index]);
             }
         }
-        // If we look at two consecutive name changes, we can check for a name oszillation.
+        // If we look at two consecutive name changes, we can check for a name oscillation.
-        // A name oszillation changes from name A shortly to name B and back to A.
+        // A name oscillation changes from name A shortly to name B and back to A.
         // In these cases, the name change will be suppressed.
         else if (one_back_index > 0 && compatible(current_step, one_back_step) &&
                  ((isCollapsableInstruction(current_step.maneuver.instruction) &&
                    isCollapsableInstruction(one_back_step.maneuver.instruction)) ||
-                  isStaggeredIntersection(one_back_step, current_step)))
+                  isStaggeredIntersection(steps, step_index, one_back_index)))
         {
             const auto two_back_index = getPreviousIndex(one_back_index, steps);
             BOOST_ASSERT(two_back_index < steps.size());