diff --git a/include/engine/api/route_api.hpp b/include/engine/api/route_api.hpp index 3f278b9b6..51248a15d 100644 --- a/include/engine/api/route_api.hpp +++ b/include/engine/api/route_api.hpp @@ -173,7 +173,7 @@ class RouteAPI : public BaseAPI guidance::trimShortSegments(steps, leg_geometry); leg.steps = guidance::handleRoundabouts(std::move(steps)); - leg.steps = guidance::collapseTurnInstructions(std::move(leg.steps)); + leg.steps = guidance::collapseTurnInstructions(std::move(leg.steps), facade); leg.steps = guidance::anticipateLaneChange(std::move(leg.steps)); leg.steps = guidance::buildIntersections(std::move(leg.steps)); leg.steps = guidance::suppressShortNameSegments(std::move(leg.steps)); diff --git a/include/engine/guidance/collapse_scenario_detection.hpp b/include/engine/guidance/collapse_scenario_detection.hpp index b2f302339..2aa359317 100644 --- a/include/engine/guidance/collapse_scenario_detection.hpp +++ b/include/engine/guidance/collapse_scenario_detection.hpp @@ -36,7 +36,10 @@ bool isStaggeredIntersection(const RouteStepIterator step_prior_to_intersection, // a - > x bool isUTurn(const RouteStepIterator step_prior_to_intersection, const RouteStepIterator step_entering_intersection, - const RouteStepIterator step_leaving_intersection); + const RouteStepIterator step_leaving_intersection, + const std::string &step_prior_name, + const std::string &step_entering_name, + const std::string &step_leaving_name); // detect oscillating names where a name switch A->B->A occurs. This is often the case due to // bridges or tunnels. Any such oszillation is not supposed to show up diff --git a/include/engine/guidance/collapse_turns.hpp b/include/engine/guidance/collapse_turns.hpp index bcdca07d7..2518038b3 100644 --- a/include/engine/guidance/collapse_turns.hpp +++ b/include/engine/guidance/collapse_turns.hpp @@ -1,5 +1,6 @@ #ifndef OSRM_ENGINE_GUIDANCE_COLLAPSE_HPP +#include "engine/datafacade/datafacade_base.hpp" #include "engine/guidance/route_step.hpp" #include "util/attributes.hpp" @@ -19,7 +20,8 @@ namespace guidance // Collapsing such turns into a single turn instruction, we give a clearer // set of instructionst that is not cluttered by unnecessary turns/name changes. OSRM_ATTR_WARN_UNUSED -std::vector collapseTurnInstructions(std::vector steps); +std::vector collapseTurnInstructions(std::vector steps, + const datafacade::BaseDataFacade &facade); // 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/src/engine/guidance/collapse_scenario_detection.cpp b/src/engine/guidance/collapse_scenario_detection.cpp index d8ab058dc..2aed5cff9 100644 --- a/src/engine/guidance/collapse_scenario_detection.cpp +++ b/src/engine/guidance/collapse_scenario_detection.cpp @@ -41,15 +41,15 @@ bool noIntermediaryIntersections(const RouteStep &step) } // Link roads, as far as we are concerned, are short unnamed segments between to named segments. -bool isLinkroad(const RouteStep &pre_link_step, - const RouteStep &link_step, - const RouteStep &post_link_step) +bool isLinkRoad(const RouteStep &link_step, + const std::string &pre_link_step_name, + const std::string &link_step_name, + const std::string &post_link_step_name) { const constexpr double MAX_LINK_ROAD_LENGTH = 2 * MAX_COLLAPSE_DISTANCE; const auto is_short = link_step.distance <= MAX_LINK_ROAD_LENGTH; - const auto unnamed = link_step.name_id == EMPTY_NAMEID; - const auto between_named = - (pre_link_step.name_id != EMPTY_NAMEID) && (post_link_step.name_id != EMPTY_NAMEID); + const auto unnamed = link_step_name.empty(); + const auto between_named = !pre_link_step_name.empty() && !post_link_step_name.empty(); return is_short && unnamed && between_named && noIntermediaryIntersections(link_step); } @@ -163,7 +163,10 @@ bool isStaggeredIntersection(const RouteStepIterator step_prior_to_intersection, bool isUTurn(const RouteStepIterator step_prior_to_intersection, const RouteStepIterator step_entering_intersection, - const RouteStepIterator step_leaving_intersection) + const RouteStepIterator step_leaving_intersection, + const std::string &step_prior_name, + const std::string &step_entering_name, + const std::string &step_leaving_name) { if (!basicCollapsePreconditions( step_prior_to_intersection, step_entering_intersection, step_leaving_intersection)) @@ -196,9 +199,10 @@ bool isUTurn(const RouteStepIterator step_prior_to_intersection, const auto only_allowed_turn = (numberOfAllowedTurns(*step_leaving_intersection) == 1) && noIntermediaryIntersections(*step_entering_intersection); - return collapsable || isLinkroad(*step_prior_to_intersection, - *step_entering_intersection, - *step_leaving_intersection) || + return collapsable || isLinkRoad(*step_entering_intersection, + step_prior_name, + step_entering_name, + step_leaving_name) || only_allowed_turn; } diff --git a/src/engine/guidance/collapse_turns.cpp b/src/engine/guidance/collapse_turns.cpp index 96eeb6470..6587ed7e3 100644 --- a/src/engine/guidance/collapse_turns.cpp +++ b/src/engine/guidance/collapse_turns.cpp @@ -326,7 +326,7 @@ void suppressStep(RouteStep &step_at_turn_location, RouteStep &step_after_turn_l // OTHER IMPLEMENTATIONS OSRM_ATTR_WARN_UNUSED -RouteSteps collapseTurnInstructions(RouteSteps steps) +RouteSteps collapseTurnInstructions(RouteSteps steps, const datafacade::BaseDataFacade &facade) { // make sure we can safely iterate over all steps (has depart/arrive with TurnType::NoTurn) BOOST_ASSERT(!hasTurnType(steps.front()) && !hasTurnType(steps.back())); @@ -371,6 +371,9 @@ RouteSteps collapseTurnInstructions(RouteSteps steps) break; const auto previous_step = findPreviousTurn(current_step); + const auto previous_step_name = facade.GetNameForID(previous_step->name_id).to_string(); + const auto current_step_name = facade.GetNameForID(current_step->name_id).to_string(); + const auto next_step_name = facade.GetNameForID(next_step->name_id).to_string(); // don't collapse anything that does change modes if (current_step->mode != next_step->mode) @@ -387,7 +390,12 @@ RouteSteps collapseTurnInstructions(RouteSteps steps) TransferSignageStrategy(), NoModificationStrategy()); } - else if (isUTurn(previous_step, current_step, next_step)) + else if (isUTurn(previous_step, + current_step, + next_step, + previous_step_name, + current_step_name, + next_step_name)) { combineRouteSteps( *current_step, @@ -464,9 +472,15 @@ RouteSteps collapseTurnInstructions(RouteSteps steps) if (!hasWaypointType(*previous_step)) { const auto far_back_step = findPreviousTurn(previous_step); + const auto far_back_step_name = facade.GetNameForID(far_back_step->name_id).to_string(); // due to name changes, we can find u-turns a bit late. Thats why we check far back as // well - if (isUTurn(far_back_step, previous_step, current_step)) + if (isUTurn(far_back_step, + previous_step, + current_step, + far_back_step_name, + previous_step_name, + current_step_name)) { combineRouteSteps( *previous_step,