diff --git a/CHANGELOG.md b/CHANGELOG.md index 169710d4e..68a57036c 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/features/guidance/roundabout.feature b/features/guidance/roundabout.feature index 62145aa75..bfb33cc57 100644 --- a/features/guidance/roundabout.feature +++ b/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 diff --git a/include/engine/api/route_api.hpp b/include/engine/api/route_api.hpp index f36390d75..d987da120 100644 --- a/include/engine/api/route_api.hpp +++ b/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); diff --git a/include/engine/guidance/lane_processing.hpp b/include/engine/guidance/lane_processing.hpp index 9d5c3ca80..f2f8fb383 100644 --- a/include/engine/guidance/lane_processing.hpp +++ b/include/engine/guidance/lane_processing.hpp @@ -22,10 +22,6 @@ OSRM_ATTR_WARN_UNUSED std::vector anticipateLaneChange(std::vector steps, const double min_duration_needed_for_lane_change = 15); -// Remove all lane information from roundabouts. See #2626. -OSRM_ATTR_WARN_UNUSED -std::vector removeLanesFromRoundabouts(std::vector steps); - } // namespace guidance } // namespace engine } // namespace osrm diff --git a/src/engine/guidance/lane_processing.cpp b/src/engine/guidance/lane_processing.cpp index 33dd43bb5..372555e2a 100644 --- a/src/engine/guidance/lane_processing.cpp +++ b/src/engine/guidance/lane_processing.cpp @@ -180,29 +180,6 @@ std::vector anticipateLaneChange(std::vector steps, return steps; } -std::vector removeLanesFromRoundabouts(std::vector 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 diff --git a/src/engine/guidance/post_processing.cpp b/src/engine/guidance/post_processing.cpp index 17ec68c96..5db7bc7a0 100644 --- a/src/engine/guidance/post_processing.cpp +++ b/src/engine/guidance/post_processing.cpp @@ -1053,9 +1053,13 @@ std::vector collapseTurns(std::vector steps) const auto ¤t_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; diff --git a/src/extractor/guidance/coordinate_extractor.cpp b/src/extractor/guidance/coordinate_extractor.cpp index 03016fea5..f393c3589 100644 --- a/src/extractor/guidance/coordinate_extractor.cpp +++ b/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); diff --git a/src/extractor/guidance/intersection_handler.cpp b/src/extractor/guidance/intersection_handler.cpp index c3b10008a..7c61029be 100644 --- a/src/extractor/guidance/intersection_handler.cpp +++ b/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" diff --git a/src/extractor/guidance/roundabout_handler.cpp b/src/extractor/guidance/roundabout_handler.cpp index c6117678f..fb1c7e805 100644 --- a/src/extractor/guidance/roundabout_handler.cpp +++ b/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 #include diff --git a/src/extractor/guidance/turn_lane_handler.cpp b/src/extractor/guidance/turn_lane_handler.cpp index 971bb8135..95684415f 100644 --- a/src/extractor/guidance/turn_lane_handler.cpp +++ b/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 @@ -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;