fix roundabout handling with lanes

instead of artificially removing lanes from a roundabout, we don't assing them in the first place.
this also prevents a problem where we would end up collapsing turns with lanes in a roundabout

CHANGELOG.md

@@ -2,6 +2,7 @@
   - Changes from 5.5.0
     - Bugfixes
       - Fix #3418 and ensure we only return bearings in the range 0-359 in API responses
+      - Fixed a bug that could lead to emitting false instructions for staying on a roundabout
 
 # 5.5.0
   - Changes from 5.4.0

features/guidance/roundabout.feature

@@ -564,16 +564,16 @@ Feature: Basic Roundabout
             | kl    | trunk       | yes    |            | Europastrasse |                                        |
             | km    | trunk       | yes    | roundabout | Europaplatz   |                                        |
             | nm    | trunk       | yes    |            | Europastrasse |                                        |
-            | mo    | trunk       | yes    | rounadbout | Europaplatz   |                                        |
+            | mo    | trunk       | yes    | roundabout | Europaplatz   |                                        |
             | op    | trunk_link  | yes    |            |               |                                        |
             | ob    | trunk       | yes    | roundabout | Europaplatz   |                                        |
 
        When I route I should get
-           | waypoints | route                                    | turns                           | lanes |
-           | a,d       | ,Europaplatz,Europastrasse,Europastrasse | depart,roundabout-exit-1,arrive | ,,    |
-           | a,h       | ,Europaplatz,Allee Cite,Allee Cite       | depart,roundabout-exit-2,arrive | ,,    |
-           | a,l       | ,Europaplatz,Europastrasse,Europastrasse | depart,roundabout-exit-3,arrive | ,,    |
-           | a,p       | ,Europaplatz,,                           | depart,roundabout-exit-4,arrive | ,,    |
+           | waypoints | route                        | turns                            | lanes |
+           | a,d       | ,Europastrasse,Europastrasse | depart,Europaplatz-exit-1,arrive | ,,    |
+           | a,h       | ,Allee Cite,Allee Cite       | depart,Europaplatz-exit-2,arrive | ,,    |
+           | a,l       | ,Europastrasse,Europastrasse | depart,Europaplatz-exit-3,arrive | ,,    |
+           | a,p       | ,,                           | depart,Europaplatz-exit-4,arrive | ,,    |
 
     @turboroundabout
     # http://www.openstreetmap.org/?mlat=50.180039&mlon=8.474939&zoom=16#map=19/50.17999/8.47506

include/engine/api/route_api.hpp

@@ -159,7 +159,6 @@ class RouteAPI : public BaseAPI
                                                               leg_geometry,
                                                               phantoms.source_phantom,
                                                               phantoms.target_phantom);
-                leg.steps = guidance::removeLanesFromRoundabouts(std::move(leg.steps));
                 leg.steps = guidance::anticipateLaneChange(std::move(leg.steps));
                 leg.steps = guidance::collapseUseLane(std::move(leg.steps));
                 leg_geometry = guidance::resyncGeometry(std::move(leg_geometry), leg.steps);

include/engine/guidance/lane_processing.hpp

@@ -22,10 +22,6 @@ OSRM_ATTR_WARN_UNUSED
 std::vector<RouteStep> anticipateLaneChange(std::vector<RouteStep> steps,
                                             const double min_duration_needed_for_lane_change = 15);
 
-// Remove all lane information from roundabouts. See #2626.
-OSRM_ATTR_WARN_UNUSED
-std::vector<RouteStep> removeLanesFromRoundabouts(std::vector<RouteStep> steps);
-
 } // namespace guidance
 } // namespace engine
 } // namespace osrm

src/engine/guidance/lane_processing.cpp

@@ -180,29 +180,6 @@ std::vector<RouteStep> anticipateLaneChange(std::vector<RouteStep> steps,
     return steps;
 }
 
-std::vector<RouteStep> removeLanesFromRoundabouts(std::vector<RouteStep> steps)
-{
-    using namespace util::guidance;
-
-    const auto removeLanes = [](RouteStep &step) {
-        for (auto &intersection : step.intersections)
-        {
-            intersection.lane_description = {};
-            intersection.lanes = {};
-        }
-    };
-
-    for (auto &step : steps)
-    {
-        const auto inst = step.maneuver.instruction;
-
-        if (entersRoundabout(inst) || staysOnRoundabout(inst) || leavesRoundabout(inst))
-            removeLanes(step);
-    }
-
-    return steps;
-}
-
 } // namespace guidance
 } // namespace engine
 } // namespace osrm

src/engine/guidance/post_processing.cpp

@@ -1053,9 +1053,13 @@ std::vector<RouteStep> collapseTurns(std::vector<RouteStep> steps)
         const auto &current_step = steps[step_index];
         const auto next_step_index = step_index + 1;
         const auto one_back_index = getPreviousIndex(step_index, steps);
+
         BOOST_ASSERT(one_back_index < steps.size());
 
         const auto &one_back_step = steps[one_back_index];
+        if (hasRoundaboutType(current_step.maneuver.instruction) ||
+            hasRoundaboutType(one_back_step.maneuver.instruction))
+            continue;
 
         if (!hasManeuver(one_back_step, current_step))
             continue;

src/extractor/guidance/coordinate_extractor.cpp

@@ -146,8 +146,7 @@ util::Coordinate CoordinateExtractor::ExtractRepresentativeCoordinate(
     // roundabouts, check early to avoid other costly checks
     if (turn_edge_data.roundabout || turn_edge_data.circular)
     {
-        const auto result = ExtractCoordinateAtLength(
-            skipping_inaccuracies_distance, coordinates);
+        const auto result = ExtractCoordinateAtLength(skipping_inaccuracies_distance, coordinates);
         BOOST_ASSERT(is_valid_result(result));
         return result;
     }
@@ -234,8 +233,7 @@ util::Coordinate CoordinateExtractor::ExtractRepresentativeCoordinate(
         std::accumulate(segment_distances.begin(), segment_distances.end(), 0.);
 
     // if we are now left with two, well than we don't have to worry, or the segment is very small
-    if (coordinates.size() == 2 ||
-        total_distance <= skipping_inaccuracies_distance)
+    if (coordinates.size() == 2 || total_distance <= skipping_inaccuracies_distance)
     {
         BOOST_ASSERT(is_valid_result(coordinates.back()));
         return coordinates.back();
@@ -252,8 +250,8 @@ util::Coordinate CoordinateExtractor::ExtractRepresentativeCoordinate(
         // As a back-up, we have to check for this case
         if (coordinates.front() == coordinates.back())
         {
-            const auto result = ExtractCoordinateAtLength(
-                skipping_inaccuracies_distance, coordinates);
+            const auto result =
+                ExtractCoordinateAtLength(skipping_inaccuracies_distance, coordinates);
             BOOST_ASSERT(is_valid_result(result));
             return result;
         }
@@ -373,18 +371,15 @@ util::Coordinate CoordinateExtractor::ExtractRepresentativeCoordinate(
          * We distinguish between turns that simply model the initial way of getting onto the
          * destination lanes and the ones that performa a larger turn.
          */
-        coordinates =
-            TrimCoordinatesToLength(std::move(coordinates),
-                                    2 * skipping_inaccuracies_distance,
-                                    segment_distances);
+        coordinates = TrimCoordinatesToLength(
+            std::move(coordinates), 2 * skipping_inaccuracies_distance, segment_distances);
         BOOST_ASSERT(coordinates.size() >= 2);
         segment_distances.resize(coordinates.size());
         segment_distances.back() = util::coordinate_calculation::haversineDistance(
             *(coordinates.end() - 2), coordinates.back());
         const auto vector_head = coordinates.back();
-        coordinates = TrimCoordinatesToLength(std::move(coordinates),
-                                              skipping_inaccuracies_distance,
-                                              segment_distances);
+        coordinates = TrimCoordinatesToLength(
+            std::move(coordinates), skipping_inaccuracies_distance, segment_distances);
         BOOST_ASSERT(coordinates.size() >= 2);
         const auto result =
             GetCorrectedCoordinate(turn_coordinate, coordinates.back(), vector_head);

src/extractor/guidance/intersection_handler.cpp

@@ -1,6 +1,7 @@
 #include "extractor/guidance/intersection_handler.hpp"
 #include "extractor/guidance/constants.hpp"
 
+#include "util/coordinate_calculation.hpp"
 #include "util/guidance/name_announcements.hpp"
 #include "util/log.hpp"
 

src/extractor/guidance/roundabout_handler.cpp

@@ -3,8 +3,8 @@
 
 #include "util/bearing.hpp"
 #include "util/coordinate_calculation.hpp"
-#include "util/log.hpp"
 #include "util/guidance/name_announcements.hpp"
+#include "util/log.hpp"
 
 #include <algorithm>
 #include <cmath>

src/extractor/guidance/turn_lane_handler.cpp

@@ -3,8 +3,8 @@
 #include "extractor/guidance/turn_discovery.hpp"
 #include "extractor/guidance/turn_lane_augmentation.hpp"
 #include "extractor/guidance/turn_lane_matcher.hpp"
-#include "util/log.hpp"
 #include "util/bearing.hpp"
+#include "util/log.hpp"
 #include "util/typedefs.hpp"
 
 #include <cstddef>
@@ -149,6 +149,17 @@ TurnLaneScenario TurnLaneHandler::deduceScenario(const NodeID at,
                                                  LaneDataVector &previous_lane_data,
                                                  LaneDescriptionID &previous_description_id)
 {
+    // as long as we don't want to emit lanes on roundabout, don't assign them
+    if (node_based_graph.GetEdgeData(via_edge).roundabout)
+        return TurnLaneScenario::NONE;
+
+    // really don't touch roundabouts (#2626)
+    if (intersection.end() !=
+        std::find_if(intersection.begin(), intersection.end(), [](const auto &road) {
+            return hasRoundaboutType(road.instruction);
+        }))
+        return TurnLaneScenario::NONE;
+
     // if only a uturn exists, there is nothing we can do
     if (intersection.size() == 1)
         return TurnLaneScenario::NONE;