improve segregated road detection

features/guidance/perception.feature

@@ -124,23 +124,3 @@ Feature: Simple Turns
             | a,f       | depart,continue left,continue right,arrive        | place,place,place,place    |
             | d,f       | depart,turn right,continue right,arrive           | bottom,place,place,place   |
             | d,h       | depart,turn right,continue left,turn right,arrive | bottom,place,place,top,top |
-
-    Scenario: Don't Collapse Places:
-        Given the node map
-            |   |   |   |   |   |   | h |   |   |   |   |   |   |
-            |   |   |   |   |   |   | g |   |   |   |   |   |   |
-            |   |   |   |   |   |   |   |   |   |   |   |   |   |
-            |   |   |   |   |   |   |   |   |   |   |   |   |   |
-            |   |   |   |   |   |   |   |   |   |   |   |   |   |
-            |   |   |   |   |   |   |   |   |   |   |   |   |   |
-            |   | b |   |   |   |   |   |   |   |   |   | e | f |
-
-        And the ways
-            | nodes | name   | oneway |
-            | fe    | place  | yes     |
-            | gh    | top    | yes     |
-            | egb | place  | yes    |
-
-        When I route I should get
-            | waypoints | turns                                             | route                      |
-            | f,h       | depart,turn right,arrive | place,top,top |

include/extractor/guidance/intersection_generator.hpp

@@ -62,7 +62,8 @@ class IntersectionGenerator
     Intersection GetConnectedRoads(const NodeID from_node, const EdgeID via_eid) const;
 
     // check if two indices in an intersection can be seen as a single road in the perceived
-    // intersection representation
+    // intersection representation See below for an example. Utility function for
+    // MergeSegregatedRoads
     bool CanMerge(const NodeID intersection_node,
                   const Intersection &intersection,
                   std::size_t first_index,

include/extractor/guidance/intersection_handler.hpp

@@ -55,10 +55,16 @@ class IntersectionHandler
     // Decide on a basic turn types
     TurnType::Enum findBasicTurnType(const EdgeID via_edge, const ConnectedRoad &candidate) const;
 
-    // Find the most obvious turn to follow
+    // Find the most obvious turn to follow. The function returns an index into the intersection
+    // determining whether there is a road that can be seen as obvious turn in the presence of many
+    // other possible turns. The function will consider road categories and other inputs like the
+    // turn angles.
     std::size_t findObviousTurn(const EdgeID via_edge, const Intersection &intersection) const;
 
-    // Get the Instruction for an obvious turn
+    // Obvious turns can still take multiple forms. This function looks at the turn onto a road
+    // candidate when coming from a via_edge and determines the best instruction to emit.
+    // `through_street` indicates if the street turned onto is a through sreet (think mergees and
+    // similar)
     TurnInstruction getInstructionForObvious(const std::size_t number_of_candidates,
                                              const EdgeID via_edge,
                                              const bool through_street,

include/util/node_based_graph.hpp

@@ -51,7 +51,7 @@ struct NodeBasedEdgeData
     LaneDescriptionID lane_description_id;
     extractor::guidance::RoadClassification road_classification;
 
-    bool IsCompatibleToExceptForName(const NodeBasedEdgeData &other) const
+    bool IsCompatibleTo(const NodeBasedEdgeData &other) const
     {
         return (reversed == other.reversed) &&
                (roundabout == other.roundabout) && (startpoint == other.startpoint) &&
@@ -60,9 +60,9 @@ struct NodeBasedEdgeData
                (road_classification == other.road_classification);
     }
 
-    bool IsCompatibleTo(const NodeBasedEdgeData &other) const
+    bool CanCombineWith(const NodeBasedEdgeData &other) const
     {
-        return (name_id == other.name_id) && IsCompatibleToExceptForName(other);
+        return (name_id == other.name_id) && IsCompatibleTo(other);
     }
 };
 

src/engine/guidance/post_processing.cpp

@@ -485,13 +485,14 @@ void collapseTurnAt(std::vector<RouteStep> &steps,
         const bool direct_u_turn = steps[two_back_index].name == current_step.name;
 
         // however, we might also deal with a dual-collapse scenario in which we have to
-        // additionall collapse a name-change as well
+        // additionall collapse a name-change as welll
+        const auto next_step_index = step_index + 1;
         const bool continues_with_name_change =
-            (step_index + 1 < steps.size()) &&
+            (next_step_index < steps.size()) &&
-            (steps[step_index + 1].maneuver.instruction.type == TurnType::UseLane ||
+            (steps[next_step_index].maneuver.instruction.type == TurnType::UseLane ||
-             isCollapsableInstruction(steps[step_index + 1].maneuver.instruction));
+             isCollapsableInstruction(steps[next_step_index].maneuver.instruction));
         const bool u_turn_with_name_change =
-            continues_with_name_change && steps[step_index + 1].name == steps[two_back_index].name;
+            continues_with_name_change && steps[next_step_index].name == steps[two_back_index].name;
 
         if (direct_u_turn || u_turn_with_name_change)
         {
@@ -500,8 +501,8 @@ void collapseTurnAt(std::vector<RouteStep> &steps,
             if (u_turn_with_name_change)
             {
                 steps[one_back_index] =
-                    elongate(std::move(steps[one_back_index]), steps[step_index + 1]);
+                    elongate(std::move(steps[one_back_index]), steps[next_step_index]);
-                invalidateStep(steps[step_index + 1]); // will be skipped due to the
+                invalidateStep(steps[next_step_index]); // will be skipped due to the
                                                        // continue statement at the
                                                        // beginning of this function
             }
@@ -599,22 +600,23 @@ std::vector<RouteStep> postProcess(std::vector<RouteStep> steps)
     // In this case, exits are numbered from the start of the leg.
     for (std::size_t step_index = 0; step_index < steps.size(); ++step_index)
     {
+        const auto next_step_index = step_index + 1;
         auto &step = steps[step_index];
         const auto instruction = step.maneuver.instruction;
         if (entersRoundabout(instruction))
         {
             has_entered_roundabout = setUpRoundabout(step);
 
-            if (has_entered_roundabout && step_index + 1 < steps.size())
+            if (has_entered_roundabout && next_step_index < steps.size())
-                steps[step_index + 1].maneuver.exit = step.maneuver.exit;
+                steps[next_step_index].maneuver.exit = step.maneuver.exit;
         }
         else if (instruction.type == TurnType::StayOnRoundabout)
         {
             on_roundabout = true;
             // increase the exit number we require passing the exit
             step.maneuver.exit += 1;
-            if (step_index + 1 < steps.size())
+            if (next_step_index < steps.size())
-                steps[step_index + 1].maneuver.exit = step.maneuver.exit;
+                steps[next_step_index].maneuver.exit = step.maneuver.exit;
         }
         else if (leavesRoundabout(instruction))
         {
@@ -626,9 +628,9 @@ std::vector<RouteStep> postProcess(std::vector<RouteStep> steps)
             has_entered_roundabout = false;
             on_roundabout = false;
         }
-        else if (on_roundabout && step_index + 1 < steps.size())
+        else if (on_roundabout && next_step_index < steps.size())
         {
-            steps[step_index + 1].maneuver.exit = step.maneuver.exit;
+            steps[next_step_index].maneuver.exit = step.maneuver.exit;
         }
     }
 
@@ -697,6 +699,7 @@ std::vector<RouteStep> collapseTurns(std::vector<RouteStep> steps)
     for (std::size_t step_index = 1; step_index + 1 < steps.size(); ++step_index)
     {
         const auto &current_step = steps[step_index];
+        const auto next_step_index = step_index + 1;
         if (current_step.maneuver.instruction.type == TurnType::NoTurn)
             continue;
         const auto one_back_index = getPreviousIndex(step_index);
@@ -765,7 +768,7 @@ std::vector<RouteStep> collapseTurns(std::vector<RouteStep> steps)
         else if (isCollapsableInstruction(current_step.maneuver.instruction) &&
                  current_step.maneuver.instruction.type != TurnType::Suppressed &&
                  steps[getPreviousNameIndex(step_index)].name == current_step.name &&
-                 canCollapseAll(getPreviousNameIndex(step_index) + 1, step_index + 1))
+                 canCollapseAll(getPreviousNameIndex(step_index) + 1, next_step_index))
         {
             BOOST_ASSERT(step_index > 0);
             const std::size_t last_available_name_index = getPreviousNameIndex(step_index);
@@ -817,17 +820,17 @@ std::vector<RouteStep> collapseTurns(std::vector<RouteStep> steps)
             }
             else if (step_index + 2 < steps.size() &&
                      current_step.maneuver.instruction.type == TurnType::NewName &&
-                     steps[step_index + 1].maneuver.instruction.type == TurnType::NewName &&
+                     steps[next_step_index].maneuver.instruction.type == TurnType::NewName &&
-                     one_back_step.name == steps[step_index + 1].name)
+                     one_back_step.name == steps[next_step_index].name)
             {
                 // if we are crossing an intersection and go immediately after into a name change,
                 // we don't wan't to collapse the initial intersection.
                 // a - b ---BRIDGE -- c
                 steps[one_back_index] =
                     elongate(std::move(steps[one_back_index]),
-                             elongate(std::move(steps[step_index]), steps[step_index + 1]));
+                             elongate(std::move(steps[step_index]), steps[next_step_index]));
                 invalidateStep(steps[step_index]);
-                invalidateStep(steps[step_index + 1]);
+                invalidateStep(steps[next_step_index]);
             }
             else if (choiceless(current_step, one_back_step) ||
                      one_back_step.distance <= MAX_COLLAPSE_DISTANCE)
@@ -1155,6 +1158,7 @@ std::vector<RouteStep> buildIntersections(std::vector<RouteStep> steps)
     std::size_t last_valid_instruction = 0;
     for (std::size_t step_index = 0; step_index < steps.size(); ++step_index)
     {
+        const auto next_step_index = step_index + 1;
         auto &step = steps[step_index];
         const auto instruction = step.maneuver.instruction;
         if (instruction.type == TurnType::Suppressed)
@@ -1176,7 +1180,7 @@ std::vector<RouteStep> buildIntersections(std::vector<RouteStep> steps)
             // previous instruction.
             if (instruction.type == TurnType::EndOfRoad)
             {
-                BOOST_ASSERT(step_index > 0 && step_index + 1 < steps.size());
+                BOOST_ASSERT(step_index > 0 && next_step_index < steps.size());
                 const auto &previous_step = steps[last_valid_instruction];
                 if (previous_step.intersections.size() < MIN_END_OF_ROAD_INTERSECTIONS)
                     step.maneuver.instruction.type = TurnType::Turn;

src/extractor/graph_compressor.cpp

@@ -99,8 +99,8 @@ void GraphCompressor::Compress(const std::unordered_set<NodeID> &barrier_nodes,
             continue;
         }
 
-        if (fwd_edge_data1.IsCompatibleTo(fwd_edge_data2) &&
+        if (fwd_edge_data1.CanCombineWith(fwd_edge_data2) &&
-            rev_edge_data1.IsCompatibleTo(rev_edge_data2))
+            rev_edge_data1.CanCombineWith(rev_edge_data2))
         {
             BOOST_ASSERT(graph.GetEdgeData(forward_e1).name_id ==
                          graph.GetEdgeData(reverse_e1).name_id);
@@ -108,7 +108,7 @@ void GraphCompressor::Compress(const std::unordered_set<NodeID> &barrier_nodes,
                          graph.GetEdgeData(reverse_e2).name_id);
 
             // Do not compress edge if it crosses a traffic signal.
-            // This can't be done in IsCompatibleTo, becase we only store the
+            // This can't be done in CanCombineWith, becase we only store the
             // traffic signals in the `traffic_lights` list, which EdgeData
             // doesn't have access to.
             const bool has_node_penalty = traffic_lights.find(node_v) != traffic_lights.end();

src/extractor/guidance/intersection_generator.cpp

@@ -165,6 +165,8 @@ Intersection IntersectionGenerator::GetConnectedRoads(const NodeID from_node,
     return intersection;
 }
 
+// Checks for mergability of two ways that represent the same intersection. For further information
+// see interface documentation in header.
 bool IntersectionGenerator::CanMerge(const NodeID node_at_intersection,
                                      const Intersection &intersection,
                                      std::size_t first_index,
@@ -227,6 +229,8 @@ bool IntersectionGenerator::CanMerge(const NodeID node_at_intersection,
 
         const auto target_id = GetActualTarget(index);
         const auto other_target_id = GetActualTarget(other_index);
+        if (target_id == node_at_intersection || other_target_id == node_at_intersection)
+            return false;
 
         const auto coordinate_at_target = node_info_list[target_id];
         const auto coordinate_at_other_target = node_info_list[other_target_id];
@@ -258,13 +262,14 @@ bool IntersectionGenerator::CanMerge(const NodeID node_at_intersection,
     if (angle_between < 60)
         return true;
 
-    // return false;
     // Finally, we also allow merging if all streets offer the same name, it is only three roads and
     // the angle is not fully extreme:
     if (intersection.size() != 3)
         return false;
 
-    const std::size_t missing_index = [first_index, second_index]() {
+    // since we have an intersection of size three now, there is only one index we are not looking
+    // at right now. The final index in the intersection is calculated next:
+    const std::size_t third_index = [first_index, second_index]() {
         if (first_index == 0)
             return second_index == 2 ? 1 : 2;
         else if (first_index == 1)
@@ -274,22 +279,23 @@ bool IntersectionGenerator::CanMerge(const NodeID node_at_intersection,
     }();
 
     // needs to be same road coming in
-    if (node_based_graph.GetEdgeData(intersection[missing_index].turn.eid).name_id !=
+    if (node_based_graph.GetEdgeData(intersection[third_index].turn.eid).name_id !=
         first_data.name_id)
         return false;
 
-    // we only allow collapsing of a Y like fork. So the angle to the missing index has to be
+    // we only allow collapsing of a Y like fork. So the angle to the third index has to be
     // roughly equal:
     const auto y_angle_difference =
-        angularDeviation(angularDeviation(intersection[missing_index].turn.angle,
+        angularDeviation(angularDeviation(intersection[third_index].turn.angle,
                                           intersection[first_index].turn.angle),
-                         angularDeviation(intersection[missing_index].turn.angle,
+                         angularDeviation(intersection[third_index].turn.angle,
                                           intersection[second_index].turn.angle));
 
     // Allow larger angles if its three roads only of the same name
-    const bool could_be_valid_y_intersection =
+    // This is a heuristic and might need to be revised.
+    const bool assume_y_intersection =
         angle_between < 100 && y_angle_difference < FUZZY_ANGLE_DIFFERENCE;
-    return could_be_valid_y_intersection;
+    return assume_y_intersection;
 }
 
 /*
@@ -458,7 +464,8 @@ Intersection IntersectionGenerator::MergeSegregatedRoads(const NodeID intersecti
 
 // OSM can have some very steep angles for joining roads. Considering the following intersection:
 //        x
-//        |__________c
+//        |
+//        v __________c
 //       /
 // a ---d
 //       \ __________b
@@ -467,13 +474,14 @@ Intersection IntersectionGenerator::MergeSegregatedRoads(const NodeID intersecti
 // and d->b as a oneway, the turn von x->d is actually a turn from x->a. So when looking at the
 // intersection coming from x, we want to interpret the situation as
 //           x
-// a __ d __ |__________c
+//           |
+// a __ d __ v__________c
 //      |
 //      |_______________b
 //
 // Where we see the turn to `d` as a right turn, rather than going straight.
 // We do this by adjusting the local turn angle at `x` to turn onto `d` to be reflective of this
-// situation.
+// situation, where `v` would be the node at the intersection.
 Intersection IntersectionGenerator::AdjustForJoiningRoads(const NodeID node_at_intersection,
                                                           Intersection intersection) const
 {
@@ -481,10 +489,11 @@ Intersection IntersectionGenerator::AdjustForJoiningRoads(const NodeID node_at_i
     if (intersection.size() <= 1)
         return intersection;
 
-    // We can't adjust the very first angle, because the u-turn should always be 0
+    const util::Coordinate coordinate_at_intersection = node_info_list[node_at_intersection];
-    for (std::size_t road_index = 1; road_index < intersection.size(); ++road_index)
+    // never adjust u-turns
+    for (std::size_t index = 1; index < intersection.size(); ++index)
     {
-        auto &road = intersection[road_index];
+        auto &road = intersection[index];
         // to find out about the above situation, we need to look at the next intersection (at d in
         // the example). If the initial road can be merged to the left/right, we are about to adjust
         // the angle.
@@ -493,6 +502,13 @@ Intersection IntersectionGenerator::AdjustForJoiningRoads(const NodeID node_at_i
         if (next_intersection_along_road.size() <= 1)
             continue;
 
+        const auto node_at_next_intersection = node_based_graph.GetTarget(road.turn.eid);
+        const util::Coordinate coordinate_at_next_intersection =
+            node_info_list[node_at_next_intersection];
+        if (util::coordinate_calculation::haversineDistance(coordinate_at_intersection,
+                                                            coordinate_at_next_intersection) > 30)
+            continue;
+
         const auto adjustAngle = [](double angle, double offset) {
             angle += offset;
             if (angle > 360)
@@ -502,7 +518,10 @@ Intersection IntersectionGenerator::AdjustForJoiningRoads(const NodeID node_at_i
             return angle;
         };
 
-        if (CanMerge(node_based_graph.GetTarget(road.turn.eid), next_intersection_along_road, 0, 1))
+        // check if the u-turn edge at the next intersection could be merged to the left/right. If
+        // this is the case and the road is not far away (see previous distance check), if
+        // influences the perceived angle.
+        if (CanMerge(node_at_next_intersection, next_intersection_along_road, 0, 1))
         {
             const auto offset = 0.5 * angularDeviation(next_intersection_along_road[0].turn.angle,
                                                        next_intersection_along_road[1].turn.angle);
@@ -510,7 +529,7 @@ Intersection IntersectionGenerator::AdjustForJoiningRoads(const NodeID node_at_i
             // angle to the left
             road.turn.angle = adjustAngle(road.turn.angle, offset);
         }
-        else if (CanMerge(node_based_graph.GetTarget(road.turn.eid),
+        else if (CanMerge(node_at_next_intersection,
                           next_intersection_along_road,
                           0,
                           next_intersection_along_road.size() - 1))
@@ -536,19 +555,19 @@ IntersectionGenerator::GetActualNextIntersection(const NodeID starting_node,
 {
     // This function skips over traffic lights/graph compression issues and similar to find the next
     // actual intersection
-    Intersection potential_result = GetConnectedRoads(starting_node, via_edge);
+    Intersection result = GetConnectedRoads(starting_node, via_edge);
 
     // Skip over stuff that has not been compressed due to barriers/parallel edges
     NodeID node_at_intersection = starting_node;
     EdgeID incoming_edge = via_edge;
 
-    while (potential_result.size() == 2 &&
+    while (result.size() == 2 &&
-           node_based_graph.GetEdgeData(via_edge).IsCompatibleToExceptForName(
+           node_based_graph.GetEdgeData(via_edge).IsCompatibleTo(
-               node_based_graph.GetEdgeData(potential_result[1].turn.eid)))
+               node_based_graph.GetEdgeData(result[1].turn.eid)))
     {
         node_at_intersection = node_based_graph.GetTarget(incoming_edge);
-        incoming_edge = potential_result[1].turn.eid;
+        incoming_edge = result[1].turn.eid;
-        potential_result = GetConnectedRoads(node_at_intersection, incoming_edge);
+        result = GetConnectedRoads(node_at_intersection, incoming_edge);
     }
 
     // return output if requested
@@ -557,7 +576,7 @@ IntersectionGenerator::GetActualNextIntersection(const NodeID starting_node,
     if (resulting_via_edge)
         *resulting_via_edge = incoming_edge;
 
-    return potential_result;
+    return result;
 }
 
 } // namespace guidance

src/extractor/guidance/intersection_handler.cpp

@@ -23,7 +23,7 @@ namespace detail
 {
 inline bool requiresAnnouncement(const EdgeData &from, const EdgeData &to)
 {
-    return !from.IsCompatibleTo(to);
+    return !from.CanCombineWith(to);
 }
 }
 
@@ -610,12 +610,13 @@ std::size_t IntersectionHandler::findObviousTurn(const EdgeID via_edge,
             {
                 // since we look at the intersection in the wrong direction, a similar angle
                 // actually represents a near 180 degree different in bearings between the two
-                // roads.
+                // roads. So if there is a road that is enterable in the opposite direction just
+                // prior, a turn is not obvious
+                const auto &turn_data = node_based_graph.GetEdgeData(comparison_road.turn.eid);
                 if (angularDeviation(comparison_road.turn.angle, STRAIGHT_ANGLE) > GROUP_ANGLE &&
                     angularDeviation(comparison_road.turn.angle, continue_road.turn.angle) <
                         FUZZY_ANGLE_DIFFERENCE &&
-                    continue_data.IsCompatibleTo(
+                    !turn_data.reversed && continue_data.CanCombineWith(turn_data))
-                        node_based_graph.GetEdgeData(comparison_road.turn.eid)))
                     return 0;
             }
         }