refactor of post-processing

- moves collapse into a dedicated set of functions / files
 - make collapse scenarios distinct (slight performance cost)
 - reduce verbosity for short name segments (now actually working, was supposed to do so before)

include/engine/api/route_api.hpp

@@ -12,8 +12,10 @@
 #include "engine/guidance/assemble_overview.hpp"
 #include "engine/guidance/assemble_route.hpp"
 #include "engine/guidance/assemble_steps.hpp"
+#include "engine/guidance/collapse_turns.hpp"
 #include "engine/guidance/lane_processing.hpp"
 #include "engine/guidance/post_processing.hpp"
+#include "engine/guidance/verbosity_reduction.hpp"
 
 #include "engine/internal_route_result.hpp"
 
@@ -165,9 +167,10 @@ class RouteAPI : public BaseAPI
                  */
 
                 guidance::trimShortSegments(steps, leg_geometry);
-                leg.steps = guidance::postProcess(std::move(steps));
-                leg.steps = guidance::collapseTurns(std::move(leg.steps));
+                leg.steps = guidance::collapseTurnInstructions(std::move(steps));
+                leg.steps = guidance::postProcess(std::move(leg.steps));
                 leg.steps = guidance::buildIntersections(std::move(leg.steps));
+                leg.steps = guidance::suppressShortNameSegments(std::move(leg.steps));
                 leg.steps = guidance::assignRelativeLocations(std::move(leg.steps),
                                                               leg_geometry,
                                                               phantoms.source_phantom,

include/engine/guidance/collapse_scenario_detection.hpp

@@ -0,0 +1,94 @@
+#ifndef OSRM_ENGINE_GUIDANCE_COLLAPSE_SCENARIO_DETECTION_HPP_
+#define OSRM_ENGINE_GUIDANCE_COLLAPSE_SCENARIO_DETECTION_HPP_
+
+#include "engine/guidance/collapsing_utility.hpp"
+#include "engine/guidance/route_step.hpp"
+
+namespace osrm
+{
+namespace engine
+{
+namespace guidance
+{
+
+// check basic collapse preconditions (mode ok, no roundabout types);
+bool basicCollapsePreconditions(const RouteStepIterator first, const RouteStepIterator second);
+
+bool basicCollapsePreconditions(const RouteStepIterator first,
+                                const RouteStepIterator second,
+                                const RouteStepIterator third);
+
+// Staggered intersection are very short zig-zags of a few meters.
+// We do not want to announce these short left-rights or right-lefts:
+// 
+//      * -> b      a -> *
+//      |       or       |       becomes  a   ->   b
+// a -> *                * -> b
+bool isStaggeredIntersection(const RouteStepIterator step_prior_to_intersection,
+                             const RouteStepIterator step_entering_intersection,
+                             const RouteStepIterator step_leaving_intersection);
+
+// Two two turns following close after another, we can announce them as a U-Turn if both end up
+// involving the same (segregated) road.
+// 
+// b < - y
+//       |      will be represented by at x, turn around instead of turn left at x, turn left at y
+// a - > x
+bool isUTurn(const RouteStepIterator step_prior_to_intersection,
+             const RouteStepIterator step_entering_intersection,
+             const RouteStepIterator step_leaving_intersection);
+
+// 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
+bool isNameOszillation(const RouteStepIterator step_prior_to_intersection,
+                       const RouteStepIterator step_entering_intersection,
+                       const RouteStepIterator step_leaving_intersection);
+
+// Sometimes, segments names don't match the perceived turns. We try to detect these additional
+// name changes and issue a combined turn.
+// 
+//  |  e  |
+// a - b - c
+//         d
+// 
+// can have `a-b` as one name, `b-c-d` as a second. At `b` we would issue a new name, even though
+// the road turns right after. The offset would only be there due to the broad road at `e`
+bool maneuverPreceededByNameChange(const RouteStepIterator step_prior_to_intersection,
+                                   const RouteStepIterator step_entering_intersection,
+                                   const RouteStepIterator step_leaving_intersection);
+bool maneuverPreceededBySuppressedDirection(const RouteStepIterator step_entering_intersection,
+                                            const RouteStepIterator step_leaving_intersection);
+bool suppressedStraightBetweenTurns(const RouteStepIterator step_entering_intersection,
+                                    const RouteStepIterator step_at_center_of_intersection,
+                                    const RouteStepIterator step_leaving_intersection);
+
+bool maneuverSucceededByNameChange(const RouteStepIterator step_entering_intersection,
+                                   const RouteStepIterator step_leaving_intersection);
+bool maneuverSucceededBySuppressedDirection(const RouteStepIterator step_entering_intersection,
+                                            const RouteStepIterator step_leaving_intersection);
+bool nameChangeImmediatelyAfterSuppressed(const RouteStepIterator step_entering_intersection,
+                                          const RouteStepIterator step_leaving_intersection);
+bool closeChoicelessTurnAfterTurn(const RouteStepIterator step_entering_intersection,
+                                  const RouteStepIterator step_leaving_intersection);
+// if modelled turn roads meet in the center of a segregated intersection, we can end up with double
+// choiceless turns
+bool doubleChoiceless(const RouteStepIterator step_entering_intersection,
+                      const RouteStepIterator step_leaving_intersection);
+
+// Due to obvious detection, sometimes we can have straight turns followed by a different turn right
+// next to each other. We combine both turns into one, if the second turn is without choice
+// 
+//         e
+// a - b - c
+//       ' d
+// 
+// with a main road `abd`, the turn `continue straight` at `b` and `turn left at `c` will become a
+// `turn left` at `b`
+bool straightTurnFollowedByChoiceless(const RouteStepIterator step_entering_intersection,
+                                      const RouteStepIterator step_leaving_intersection);
+
+} /* namespace guidance */
+} /* namespace engine */
+} /* namespace osrm */
+
+#endif /* OSRM_ENGINE_GUIDANCE_COLLAPSE_SCENARIO_DETECTION_HPP_ */

include/engine/guidance/collapse_turns.hpp

@@ -0,0 +1,147 @@
+#ifndef OSRM_ENGINE_GUIDANCE_COLLAPSE_HPP
+
+#include "engine/guidance/route_step.hpp"
+#include "util/attributes.hpp"
+
+#include <type_traits>
+#include <vector>
+
+namespace osrm
+{
+namespace engine
+{
+namespace guidance
+{
+
+// Multiple possible reasons can result in unnecessary/confusing instructions
+// A prime example would be a segregated intersection. Turning around at this
+// intersection would result in two instructions to turn left.
+// 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<RouteStep> collapseTurnInstructions(std::vector<RouteStep> steps);
+
+// 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
+// of other items that influence how we combine turns. This function is an entry point, defining the
+// possibility to select one of multiple strategies when combining a turn with another one.
+template <typename CombinedTurnStrategy, typename SignageStrategy>
+RouteStep combineRouteSteps(const RouteStep &step_at_turn_location,
+                            const RouteStep &step_after_turn_location,
+                            const CombinedTurnStrategy combined_turn_stragey,
+                            const SignageStrategy signage_strategy);
+
+// TAGS
+// These tags are used to ensure correct strategy usage. Make sure your new strategy is derived from
+// (at least) one of these tags. It can only be used for the intended tags, to ensure we don't
+// accidently use a lane strategy to cover signage
+struct CombineStrategy
+{
+};
+
+struct SignageStrategy
+{
+};
+
+struct LaneStrategy
+{
+};
+
+// Return the step at the turn location, without modification
+struct NoModificationStrategy : CombineStrategy, SignageStrategy, LaneStrategy
+{
+    void operator()(RouteStep &step_at_turn_location, const RouteStep &transfer_from_step) const;
+};
+
+// transfer the turn type from the second step
+struct TransferTurnTypeStrategy : CombineStrategy
+{
+    void operator()(RouteStep &step_at_turn_location, const RouteStep &transfer_from_step) const;
+};
+
+// Combine both turn and turn angle to a common item
+struct AdjustToCombinedTurnAngleStrategy : CombineStrategy
+{
+    void operator()(RouteStep &step_at_turn_location, const RouteStep &transfer_from_step) const;
+};
+
+// Combine only the turn types
+struct AdjustToCombinedTurnStrategy : CombineStrategy
+{
+    AdjustToCombinedTurnStrategy(const RouteStep &step_prior_to_intersection);
+    void operator()(RouteStep &step_at_turn_location, const RouteStep &transfer_from_step) const;
+
+    const RouteStep &step_prior_to_intersection;
+};
+
+// Set a fixed instruction type
+struct SetFixedInstructionStrategy : CombineStrategy
+{
+    SetFixedInstructionStrategy(const extractor::guidance::TurnInstruction instruction);
+    void operator()(RouteStep &step_at_turn_location, const RouteStep &transfer_from_step) const;
+
+    const extractor::guidance::TurnInstruction instruction;
+};
+
+// Handling of staggered intersections
+struct StaggeredTurnStrategy : CombineStrategy
+{
+    StaggeredTurnStrategy(const RouteStep &step_prior_to_intersection);
+
+    void operator()(RouteStep &step_at_turn_location, const RouteStep &transfer_from_step) const;
+
+    const RouteStep &step_prior_to_intersection;
+};
+
+// Signage Strategies
+
+// Transfer the signage from the next step onto this step
+struct TransferSignageStrategy : SignageStrategy
+{
+    void operator()(RouteStep &step_at_turn_location, const RouteStep &transfer_from_step) const;
+};
+
+// Lane Strategies
+
+// Transfer the turn lanes from the intermediate step
+struct TransferLanesStrategy : LaneStrategy
+{
+    void operator()(RouteStep &step_at_turn_location, const RouteStep &transfer_from_step) const;
+};
+
+// Pattern to combine a route step using the predefined strategies
+template <typename CombineStrategyClass, typename SignageStrategyClass, typename LaneStrategyClass>
+void combineRouteSteps(RouteStep &step_at_turn_location,
+                       RouteStep &step_after_turn_location,
+                       CombineStrategyClass combined_turn_stragey,
+                       SignageStrategyClass signage_strategy,
+                       LaneStrategyClass lane_strategy)
+{
+    // assign the combined turn type
+    static_assert(std::is_base_of<CombineStrategy, CombineStrategyClass>::value,
+                  "Supplied Strategy isn't a combine strategy.");
+    combined_turn_stragey(step_at_turn_location, step_after_turn_location);
+
+    // assign the combind signage
+    static_assert(std::is_base_of<LaneStrategy, LaneStrategyClass>::value,
+                  "Supplied Strategy isn't a signage strategy.");
+    signage_strategy(step_at_turn_location, step_after_turn_location);
+
+    // assign the desired turn lanes
+    static_assert(std::is_base_of<LaneStrategy, LaneStrategyClass>::value,
+                  "Supplied Strategy isn't a lane strategy.");
+    lane_strategy(step_at_turn_location, step_after_turn_location);
+
+    // further stuff should happen here as well
+    step_at_turn_location.ElongateBy(step_after_turn_location);
+    step_after_turn_location.Invalidate();
+}
+
+// alias for suppressing a step, using CombineRouteStep with NoModificationStrategy only
+void suppressStep(RouteStep &step_at_turn_location, RouteStep &step_after_turn_location);
+
+} /* namespace guidance */
+} /* namespace osrm */
+} /* namespace osrm */
+
+#endif /* OSRM_ENGINE_GUIDANCE_COLLAPSE_HPP_ */

include/engine/guidance/collapsing_utility.hpp

@@ -0,0 +1,190 @@
+#ifndef OSRM_ENGINE_GUIDANCE_COLLAPSING_UTILITY_HPP_
+#define OSRM_ENGINE_GUIDANCE_COLLAPSING_UTILITY_HPP_
+
+#include "extractor/guidance/turn_instruction.hpp"
+#include "engine/guidance/route_step.hpp"
+#include "util/attributes.hpp"
+#include "util/guidance/name_announcements.hpp"
+
+#include <boost/range/algorithm_ext/erase.hpp>
+#include <cstddef>
+
+using osrm::extractor::guidance::TurnInstruction;
+using namespace osrm::extractor::guidance;
+
+namespace osrm
+{
+namespace engine
+{
+namespace guidance
+{
+
+using RouteSteps = std::vector<RouteStep>;
+using RouteStepIterator = typename RouteSteps::iterator;
+const constexpr std::size_t MIN_END_OF_ROAD_INTERSECTIONS = std::size_t{2};
+const constexpr double MAX_COLLAPSE_DISTANCE = 30.0;
+// a bit larger than 100 to avoid oscillation in tests
+const constexpr double NAME_SEGMENT_CUTOFF_LENGTH = 105.0;
+
+// check if a step is completely without turn type
+inline bool hasTurnType(const RouteStep &step)
+{
+    return step.maneuver.instruction.type != TurnType::NoTurn;
+}
+inline bool hasWaypointType(const RouteStep &step)
+{
+    return step.maneuver.waypoint_type != WaypointType::None;
+}
+
+// skip backwards through possibly disabled turns until we find a turn type (or the first step)
+inline RouteStepIterator findPreviousTurn(RouteStepIterator current_step)
+{
+    BOOST_ASSERT(!hasWaypointType(*current_step));
+    // find the first element preceeding the current step that has an actual turn type (not
+    // necessarily announced)
+    do
+    {
+        // safety to do this loop is asserted in collapseTurnInstructions
+        --current_step;
+    } while (!hasTurnType(*current_step) && !hasWaypointType(*current_step));
+    return current_step;
+}
+
+// skip forwards over possible NoTurn entries (e.g. ferries) until we find the next instruction with
+// a turn type
+inline RouteStepIterator findNextTurn(RouteStepIterator current_step)
+{
+    BOOST_ASSERT(!hasWaypointType(*current_step));
+    // find the first element preceeding the current step that has an actual turn type (not
+    // necessarily announced)
+    do
+    {
+        // safety to do this loop is asserted in collapseTurnInstructions
+        ++current_step;
+    } while (!hasTurnType(*current_step) && !hasWaypointType(*current_step));
+    return current_step;
+}
+
+// alias for comparisons
+inline bool hasTurnType(const RouteStep &step, const TurnType::Enum type)
+{
+    return type == step.maneuver.instruction.type;
+}
+// alias for comparisons
+inline bool hasModifier(const RouteStep &step, const DirectionModifier::Enum modifier)
+{
+    return modifier == step.maneuver.instruction.direction_modifier;
+}
+inline bool hasLanes(const RouteStep &step)
+{
+    return step.intersections.front().lanes.lanes_in_turn != 0;
+}
+
+// alias for detectors, gives the number of connected roads
+inline std::size_t numberOfAvailableTurns(const RouteStep &step)
+{
+    return step.intersections.front().entry.size();
+}
+// alias for detectors, counts only the allowed turns
+inline std::size_t numberOfAllowedTurns(const RouteStep &step)
+{
+    return std::count(
+        step.intersections.front().entry.begin(), step.intersections.front().entry.end(), true);
+}
+// traffic lights are very specifically modelled. Sometimes we need to skip them. All checks need to
+// fulfill:
+inline bool isTrafficLightStep(const RouteStep &step)
+{
+    return hasTurnType(step, TurnType::Suppressed) && numberOfAvailableTurns(step) == 2 &&
+           numberOfAllowedTurns(step) == 1;
+}
+
+// alias for readability
+inline void setInstructionType(RouteStep &step, const TurnType::Enum type)
+{
+    step.maneuver.instruction.type = type;
+}
+
+// alias for readability
+inline bool haveSameMode(const RouteStep &lhs, const RouteStep &rhs)
+{
+    return lhs.mode == rhs.mode;
+}
+
+// alias for readability
+inline bool haveSameMode(const RouteStep &first, const RouteStep &second, const RouteStep &third)
+{
+    return haveSameMode(first, second) && haveSameMode(second, third);
+}
+
+// alias for readability
+inline bool haveSameName(const RouteStep &lhs, const RouteStep &rhs)
+{
+    // make sure empty is not involved
+    if (lhs.name_id == EMPTY_NAMEID || rhs.name_id == EMPTY_NAMEID)
+        return false;
+
+    // easy check to not go over the strings if not necessary
+    else if (lhs.name_id == rhs.name_id)
+        return true;
+
+    // ok, bite the sour grape and check the strings already
+    else
+        return !util::guidance::requiresNameAnnounced(
+            lhs.name, lhs.ref, lhs.pronunciation, rhs.name, rhs.ref, rhs.pronunciation);
+}
+
+// alias for readability, both turn right | left
+inline bool areSameSide(const RouteStep &lhs, const RouteStep &rhs)
+{
+    const auto is_left = [](const RouteStep &step) {
+        return hasModifier(step, DirectionModifier::Straight) ||
+               hasLeftModifier(step.maneuver.instruction);
+    };
+
+    const auto is_right = [](const RouteStep &step) {
+        return hasModifier(step, DirectionModifier::Straight) ||
+               hasRightModifier(step.maneuver.instruction);
+    };
+
+    return (is_left(lhs) && is_left(rhs)) || (is_right(lhs) && is_right(rhs));
+}
+
+// do this after invalidating any steps to compress the step array again
+OSRM_ATTR_WARN_UNUSED
+inline std::vector<RouteStep> removeNoTurnInstructions(std::vector<RouteStep> steps)
+{
+    // finally clean up the post-processed instructions.
+    // Remove all invalid instructions from the set of instructions.
+    // An instruction is invalid, if its NO_TURN and has WaypointType::None.
+    // Two valid NO_TURNs exist in each leg in the form of Depart/Arrive
+
+    // keep valid instructions
+    const auto not_is_valid = [](const RouteStep &step) {
+        return step.maneuver.instruction == TurnInstruction::NO_TURN() &&
+               step.maneuver.waypoint_type == WaypointType::None;
+    };
+
+    boost::remove_erase_if(steps, not_is_valid);
+
+    // the steps should still include depart and arrive at least
+    BOOST_ASSERT(steps.size() >= 2);
+
+    BOOST_ASSERT(steps.front().intersections.size() >= 1);
+    BOOST_ASSERT(steps.front().intersections.front().bearings.size() == 1);
+    BOOST_ASSERT(steps.front().intersections.front().entry.size() == 1);
+    BOOST_ASSERT(steps.front().maneuver.waypoint_type == WaypointType::Depart);
+
+    BOOST_ASSERT(steps.back().intersections.size() == 1);
+    BOOST_ASSERT(steps.back().intersections.front().bearings.size() == 1);
+    BOOST_ASSERT(steps.back().intersections.front().entry.size() == 1);
+    BOOST_ASSERT(steps.back().maneuver.waypoint_type == WaypointType::Arrive);
+
+    return steps;
+}
+
+} /* namespace guidance */
+} /* namespace engine */
+} /* namespace osrm */
+
+#endif /* OSRM_ENGINE_GUIDANCE_COLLAPSING_UTILITY_HPP_ */

include/engine/guidance/post_processing.hpp

@@ -14,23 +14,11 @@ namespace engine
 {
 namespace guidance
 {
+
 // passed as none-reference to modify in-place and move out again
 OSRM_ATTR_WARN_UNUSED
 std::vector<RouteStep> postProcess(std::vector<RouteStep> steps);
 
-// Multiple possible reasons can result in unnecessary/confusing instructions
-// A prime example would be a segregated intersection. Turning around at this
-// intersection would result in two instructions to turn left.
-// 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<RouteStep> collapseTurns(std::vector<RouteStep> steps);
-
-// A check whether two instructions can be treated as one. This is only the case for very short
-// maneuvers that can, in some form, be seen as one. Lookahead of one step.
-// This is only a pre-check and does not necessarily allow collapsing turns!!!
-bool collapsable(const RouteStep &step, const RouteStep &next);
-
 // trim initial/final segment of very short length.
 // This function uses in/out parameter passing to modify both steps and geometry in place.
 // We use this method since both steps and geometry are closely coupled logically but
@@ -49,20 +37,6 @@ std::vector<RouteStep> assignRelativeLocations(std::vector<RouteStep> steps,
 OSRM_ATTR_WARN_UNUSED
 std::vector<RouteStep> buildIntersections(std::vector<RouteStep> steps);
 
-// remove steps invalidated by post-processing
-OSRM_ATTR_WARN_UNUSED
-std::vector<RouteStep> removeNoTurnInstructions(std::vector<RouteStep> steps);
-
-// remove use lane information that is not actually a turn. For post-processing, we need to
-// associate lanes with every turn. Some of these use-lane instructions are not required after lane
-// anticipation anymore. This function removes all use lane instructions that are not actually used
-// anymore since all lanes going straight are used anyhow.
-// FIXME this is currently only a heuristic. We need knowledge on which lanes actually might become
-// turn lanes. If a straight lane becomes a turn lane, this might be something to consider. Right
-// now we bet on lane-anticipation to catch this.
-OSRM_ATTR_WARN_UNUSED
-std::vector<RouteStep> collapseUseLane(std::vector<RouteStep> steps);
-
 // postProcess will break the connection between the leg geometry
 // for which a segment is supposed to represent exactly the coordinates
 // between routing maneuvers and the route steps itself.

include/engine/guidance/verbosity_reduction.hpp

@@ -0,0 +1,37 @@
+#ifndef OSRM_ENGINE_GUIDANCE_VERBOSITY_REDUCTION_HPP_
+#define OSRM_ENGINE_GUIDANCE_VERBOSITY_REDUCTION_HPP_
+
+#include "engine/guidance/route_step.hpp"
+#include "util/attributes.hpp"
+
+#include <vector>
+
+namespace osrm
+{
+namespace engine
+{
+namespace guidance
+{
+
+// Name changes on roads are posing relevant information. However if they are short, we don't want
+// to announce them. All these that are not collapsed into a single turn (think segregated
+// intersection) have to be checked for the length they are active in. If they are active for a
+// short distance only, we don't announce them
+OSRM_ATTR_WARN_UNUSED
+std::vector<RouteStep> suppressShortNameSegments(std::vector<RouteStep> steps);
+
+// remove use lane information that is not actually a turn. For post-processing, we need to
+// associate lanes with every turn. Some of these use-lane instructions are not required after lane
+// anticipation anymore. This function removes all use lane instructions that are not actually used
+// anymore since all lanes going straight are used anyhow.
+// FIXME this is currently only a heuristic. We need knowledge on which lanes actually might become
+// turn lanes. If a straight lane becomes a turn lane, this might be something to consider. Right
+// now we bet on lane-anticipation to catch this.
+OSRM_ATTR_WARN_UNUSED
+std::vector<RouteStep> collapseUseLane(std::vector<RouteStep> steps);
+
+} // namespace guidance
+} // namespace engine
+} // namespace osrm
+
+#endif /* OSRM_ENGINE_GUIDANCE_VERBOSITY_REDUCTION_HPP_ */