Updated segregated intersection identification (#4845)

* Initial internal intersection updates
paired with @oxidase and @kdiluca
TODO fix tests and add in new ones

* Added Internal Intersection Model

* removed debug info

* updates per PR 4845

* fixing build errors

* fixing all compile errors

* fixed EdgeID param

* Added is_internal_straight lambda
Added/Updated constexpr names and values

* added rejection case turn degree logic

* debug logging

* added turn angle logic to reject if there are incoming edges that have opposite turn degrees than outgoing edges or if the outgoing edges have opposing turn degrees; also merged with master v5.16

* fixed formatting

* fix to decrease tile size based on latest turn angle internal intersection updates

* Removed breaks

Breaks in code were a mistake and caused a change in the internal intersection identification.

* Update segregated_intersection_classification.cpp

* Update CHANGELOG.md

Added CHANGED #4845: Updated segregated intersection identification to Unreleased

CHANGELOG.md

@@ -5,6 +5,8 @@
       - `osrm-routed` accepts a new property `--memory_file` to store memory in a file on disk.
     - NodeJS:
       - `OSRM` object accepts a new option `memory_file` that stores the memory in a file on disk.
+    - Internals 
+      - CHANGED #4845: Updated segregated intersection identification  
 
 
 # 5.16.0

features/guidance/internal-intersections.feature

@@ -0,0 +1,46 @@
+@guidance
+Feature: Internal Intersection Model
+
+    Background:
+        Given the profile "car"
+        Given a grid size of 10 meters
+
+    Scenario: Dual-carriage way intersection
+        Given the node map
+          """
+             a  b
+             |  |
+          c--d--e--f
+             |  |
+          g--h--i--j
+             |  |
+             k  l
+          """
+
+        And the ways
+          | nodes | oneway | name                  |
+          | adhk  | yes    | Broken Land Parkway   |
+          | lieb  | yes    | Broken Land Parkway   |
+          | fed   | yes    | Snowden River Parkway |
+          | dc    | yes    | Patuxent Woods Drive  |
+          | gh    | yes    | Patuxent Woods Drive  |
+          | hij   | yes    | Snowden River Parkway |
+
+        When I route I should get
+          | waypoints | route                                                              | turns                        | # |
+          | a,k       | Broken Land Parkway,Broken Land Parkway                            | depart,arrive                ||
+          | l,b       | Broken Land Parkway,Broken Land Parkway                            | depart,arrive                ||
+#         | g,j       | Patuxent Woods Drive,Snowden River Parkway,Snowden River Parkway   | depart,continue,arrive       | did not work as expected - might be another issue to handle in post process? |
+#         | f,c       | Snowden River Parkway,Patuxent Woods Drive,Patuxent Woods Drive    | depart,continue,arrive       | did not work as expected - might be another issue to handle in post process? |
+          | a,c       | Broken Land Parkway,Patuxent Woods Drive,Patuxent Woods Drive      | depart,turn right,arrive     ||
+          | g,k       | Patuxent Woods Drive,Broken Land Parkway,Broken Land Parkway       | depart,turn right,arrive     ||
+          | l,j       | Broken Land Parkway,Snowden River Parkway,Snowden River Parkway    | depart,turn right,arrive     ||
+          | f,b       | Snowden River Parkway,Broken Land Parkway,Broken Land Parkway      | depart,turn right,arrive     ||
+          | a,j       | Broken Land Parkway,Snowden River Parkway,Snowden River Parkway    | depart,turn left,arrive      ||
+          | g,b       | Patuxent Woods Drive,Broken Land Parkway,Broken Land Parkway       | depart,turn left,arrive      ||
+          | l,c       | Broken Land Parkway,Patuxent Woods Drive,Patuxent Woods Drive      | depart,turn left,arrive      ||
+          | f,k       | Snowden River Parkway,Broken Land Parkway,Broken Land Parkway      | depart,turn left,arrive      ||
+          | a,b       | Broken Land Parkway,Broken Land Parkway,Broken Land Parkway        | depart,continue uturn,arrive ||
+          | g,c       | Patuxent Woods Drive,Patuxent Woods Drive,Patuxent Woods Drive     | depart,continue uturn,arrive ||
+          | l,k       | Broken Land Parkway,Broken Land Parkway,Broken Land Parkway        | depart,continue uturn,arrive ||
+          | f,j       | Snowden River Parkway,Snowden River Parkway,Snowden River Parkway  | depart,continue uturn,arrive ||

src/guidance/segregated_intersection_classification.cpp

@@ -1,29 +1,39 @@
 #include "guidance/segregated_intersection_classification.hpp"
 #include "extractor/intersection/coordinate_extractor.hpp"
 #include "extractor/node_based_graph_factory.hpp"
+#include "guidance/turn_instruction.hpp"
 
 #include "util/coordinate_calculation.hpp"
 #include "util/name_table.hpp"
+#include <set>
+
+using osrm::guidance::getTurnDirection;
 
 namespace osrm
 {
 namespace guidance
 {
 
-namespace RoadPriorityClass = extractor::RoadPriorityClass;
+// Maximum length in meters of an internal intersection edge
+constexpr auto INTERNAL_LENGTH_MAX = 32.0f;
+
+// The lower and upper bound internal straight values
+constexpr auto INTERNAL_STRAIGHT_LOWER_BOUND = 150.0;
+constexpr auto INTERNAL_STRAIGHT_UPPER_BOUND = 210.0;
 
 struct EdgeInfo
 {
+    EdgeID edge;
+
     NodeID node;
 
     util::StringView name;
 
-    // 0 - outgoing (forward), 1 - incoming (reverse), 2 - both outgoing and incoming
+    bool reversed;
-    int direction;
 
     extractor::ClassData road_class;
 
-    RoadPriorityClass::Enum road_priority_class;
+    extractor::NodeBasedEdgeClassification flags;
 
     struct LessName
     {
@@ -31,112 +41,19 @@ struct EdgeInfo
     };
 };
 
-bool IsSegregated(std::vector<EdgeInfo> v1,
-                  std::vector<EdgeInfo> v2,
-                  EdgeInfo const &current,
-                  double edgeLength)
-{
-    if (v1.size() < 2 || v2.size() < 2)
-        return false;
-
-    auto const sort_by_name_fn = [](std::vector<EdgeInfo> &v) {
-        std::sort(v.begin(), v.end(), EdgeInfo::LessName());
-    };
-
-    sort_by_name_fn(v1);
-    sort_by_name_fn(v2);
-
-    // Internal edge with the name should be connected with any other neibour edge with the same
-    // name, e.g. isolated edge with unique name is not segregated.
-    //              b - 'b' road continues here
-    //              |
-    //      - - a - |
-    //              b - segregated edge
-    //      - - a - |
-    if (!current.name.empty())
-    {
-        auto const findNameFn = [&current](std::vector<EdgeInfo> const &v) {
-            return std::binary_search(v.begin(), v.end(), current, EdgeInfo::LessName());
-        };
-
-        if (!findNameFn(v1) && !findNameFn(v2))
-            return false;
-    }
-
-    // set_intersection like routine to get equal result pairs
-    std::vector<std::pair<EdgeInfo const *, EdgeInfo const *>> commons;
-
-    auto i1 = v1.begin();
-    auto i2 = v2.begin();
-
-    while (i1 != v1.end() && i2 != v2.end())
-    {
-        if (i1->name == i2->name)
-        {
-            if (!i1->name.empty())
-                commons.push_back(std::make_pair(&(*i1), &(*i2)));
-
-            ++i1;
-            ++i2;
-        }
-        else if (i1->name < i2->name)
-            ++i1;
-        else
-            ++i2;
-    }
-
-    if (commons.size() < 2)
-        return false;
-
-    auto const check_equal_class = [](std::pair<EdgeInfo const *, EdgeInfo const *> const &e) {
-        // Or (e.first->road_class & e.second->road_class != 0)
-        return e.first->road_class == e.second->road_class;
-    };
-
-    size_t equal_class_count = 0;
-    for (auto const &e : commons)
-        if (check_equal_class(e))
-            ++equal_class_count;
-
-    if (equal_class_count < 2)
-        return false;
-
-    auto const get_length_threshold = [](EdgeInfo const *e) {
-        switch (e->road_priority_class)
-        {
-        case RoadPriorityClass::MOTORWAY:
-        case RoadPriorityClass::TRUNK:
-            return 30.0;
-        case RoadPriorityClass::PRIMARY:
-            return 20.0;
-        case RoadPriorityClass::SECONDARY:
-        case RoadPriorityClass::TERTIARY:
-            return 10.0;
-        default:
-            return 5.0;
-        }
-    };
-
-    double threshold = std::numeric_limits<double>::max();
-    for (auto const &e : commons)
-        threshold =
-            std::min(threshold, get_length_threshold(e.first) + get_length_threshold(e.second));
-
-    return edgeLength <= threshold;
-}
-
 std::unordered_set<EdgeID> findSegregatedNodes(const extractor::NodeBasedGraphFactory &factory,
                                                const util::NameTable &names)
 {
-
     auto const &graph = factory.GetGraph();
     auto const &annotation = factory.GetAnnotationData();
+    auto const &coordinates = factory.GetCoordinates();
 
     extractor::intersection::CoordinateExtractor coordExtractor(
-        graph, factory.GetCompressedEdges(), factory.GetCoordinates());
+        graph, factory.GetCompressedEdges(), coordinates);
 
-    auto const get_edge_length = [&](NodeID from_node, EdgeID edgeID, NodeID to_node) {
+    auto const get_edge_length = [&](NodeID from_node, EdgeID edge_id, NodeID to_node) {
-        auto const geom = coordExtractor.GetCoordinatesAlongRoad(from_node, edgeID, false, to_node);
+        auto const geom =
+            coordExtractor.GetCoordinatesAlongRoad(from_node, edge_id, false, to_node);
         double length = 0.0;
         for (size_t i = 1; i < geom.size(); ++i)
         {
@@ -145,18 +62,166 @@ std::unordered_set<EdgeID> findSegregatedNodes(const extractor::NodeBasedGraphFa
         return length;
     };
 
-    auto const get_edge_info = [&](NodeID node, auto const &edgeData) -> EdgeInfo {
+    // Returns an angle between edges from from_edge_id to to_edge_id
+    auto const get_angle = [&](NodeID from_node, EdgeID from_edge_id, EdgeID to_edge_id) {
+        auto intersection_node = graph.GetTarget(from_edge_id);
+        auto from_edge_id_outgoing = graph.FindEdge(intersection_node, from_node);
+        auto to_node = graph.GetTarget(to_edge_id);
+        auto const node_to =
+            coordExtractor.GetCoordinateCloseToTurn(intersection_node, to_edge_id, false, to_node);
+        auto const node_from = coordExtractor.GetCoordinateCloseToTurn(
+            intersection_node, from_edge_id_outgoing, false, from_node);
+        return util::coordinate_calculation::computeAngle(
+            node_from, coordinates[intersection_node], node_to);
+    };
+
+    auto const get_edge_info = [&](EdgeID edge_id, NodeID node, auto const &edge_data) -> EdgeInfo {
         /// @todo Make string normalization/lowercase/trim for comparison ...
 
-        auto const id = annotation[edgeData.annotation_data].name_id;
+        auto const id = annotation[edge_data.annotation_data].name_id;
         BOOST_ASSERT(id != INVALID_NAMEID);
         auto const name = names.GetNameForID(id);
-
+        return {edge_id,
-        return {node,
+                node,
                 name,
-                edgeData.reversed ? 1 : 0,
+                edge_data.reversed,
-                annotation[edgeData.annotation_data].classes,
+                annotation[edge_data.annotation_data].classes,
-                edgeData.flags.road_classification.GetClass()};
+                edge_data.flags};
+    };
+
+    auto is_bidirectional = [](auto flags) {
+        return flags.is_split || (!flags.is_split && flags.forward && flags.backward);
+    };
+
+    auto is_internal_straight = [](auto const turn_degree) {
+        return (turn_degree > INTERNAL_STRAIGHT_LOWER_BOUND &&
+                turn_degree < INTERNAL_STRAIGHT_UPPER_BOUND);
+    };
+
+    // Lambda to check if the turn set includes a right turn type
+    const auto has_turn_right = [](std::set<guidance::DirectionModifier::Enum> &turn_types) {
+        return turn_types.find(guidance::DirectionModifier::Right) != turn_types.end() ||
+               turn_types.find(guidance::DirectionModifier::SharpRight) != turn_types.end();
+    };
+    // Lambda to check if the turn set includes a left turn type
+    const auto has_turn_left = [](std::set<guidance::DirectionModifier::Enum> &turn_types) {
+        return turn_types.find(guidance::DirectionModifier::Left) != turn_types.end() ||
+               turn_types.find(guidance::DirectionModifier::SharpLeft) != turn_types.end();
+    };
+
+    auto isSegregated = [&](NodeID node1,
+                            std::vector<EdgeInfo> v1,
+                            std::vector<EdgeInfo> v2,
+                            EdgeInfo const &current,
+                            double edge_length) {
+        // Internal intersection edges must be short and cannot be a roundabout.
+        // Also they must be a road use (not footway, cycleway, etc.)
+        // TODO - consider whether alleys, cul-de-sacs, and other road uses
+        // are candidates to be marked as internal intersection edges.
+        // TODO adjust length as needed with lamda
+        if (edge_length > INTERNAL_LENGTH_MAX || current.flags.roundabout)
+        {
+            return false;
+        }
+        // Iterate through inbound edges and get turn degrees from driveable inbound
+        // edges onto the candidate edge.
+        bool oneway_inbound = false;
+        std::set<guidance::DirectionModifier::Enum> incoming_turn_type;
+        for (auto const &edge_from : v1)
+        {
+            // Get the inbound edge and edge data
+            auto edge_inbound = graph.FindEdge(edge_from.node, node1);
+            auto const &edge_inbound_data = graph.GetEdgeData(edge_inbound);
+            if (!edge_inbound_data.reversed)
+            {
+                // Store the turn type of incoming driveable edges.
+                incoming_turn_type.insert(guidance::getTurnDirection(
+                    get_angle(edge_from.node, edge_inbound, current.edge)));
+
+                // Skip any inbound edges not oneway (i.e. skip bidirectional)
+                // and link edge
+                // and not a road
+                if (is_bidirectional(edge_inbound_data.flags) ||
+                    edge_inbound_data.flags.road_classification.IsLinkClass() ||
+                    (edge_inbound_data.flags.road_classification.GetClass() >
+                     extractor::RoadPriorityClass::SIDE_RESIDENTIAL))
+                {
+                    continue;
+                }
+                // Get the turn degree from the inbound edge to the current edge
+                // Skip if the inbound edge is not somewhat perpendicular to the current edge
+                if (is_internal_straight(get_angle(edge_from.node, edge_inbound, current.edge)))
+                {
+                    continue;
+                }
+                // If we are here the edge is a candidate oneway inbound
+                oneway_inbound = true;
+            }
+        }
+
+        // Must have an inbound oneway, excluding edges that are nearly straight
+        // turn type onto the directed edge.
+        if (!oneway_inbound)
+        {
+            return false;
+        }
+
+        // Iterate through outbound edges and get turn degrees from the candidate
+        // edge onto outbound driveable edges.
+        bool oneway_outbound = false;
+        std::set<guidance::DirectionModifier::Enum> outgoing_turn_type;
+        for (auto const &edge_to : v2)
+        {
+            if (!edge_to.reversed)
+            {
+                // Store outgoing turn type for any driveable edges
+                outgoing_turn_type.insert(
+                    guidance::getTurnDirection(get_angle(node1, current.edge, edge_to.edge)));
+
+                // Skip any outbound edges not oneway (i.e. skip bidirectional)
+                // and link edge
+                // and not a road
+                if (is_bidirectional(edge_to.flags) ||
+                    edge_to.flags.road_classification.IsLinkClass() ||
+                    (edge_to.flags.road_classification.GetClass() >
+                     extractor::RoadPriorityClass::SIDE_RESIDENTIAL))
+                {
+                    continue;
+                }
+
+                // Get the turn degree from the current edge to the outbound edge
+                // Skip if the outbound edge is not somewhat perpendicular to the current edge
+                if (is_internal_straight(get_angle(node1, current.edge, edge_to.edge)))
+                {
+                    continue;
+                }
+
+                // If we are here the edge is a candidate oneway outbound
+                oneway_outbound = true;
+            }
+        }
+        // Must have outbound oneway at end node (exclude edges that are nearly
+        // straight turn from directed edge
+        if (!oneway_outbound)
+        {
+            return false;
+        }
+
+        // A further rejection case is if there are incoming edges that
+        // have "opposite" turn degrees than outgoing edges or if the outgoing
+        // edges have opposing turn degrees.
+        if ((has_turn_left(incoming_turn_type) && has_turn_right(outgoing_turn_type)) ||
+            (has_turn_right(incoming_turn_type) && has_turn_left(outgoing_turn_type)) ||
+            (has_turn_left(outgoing_turn_type) && has_turn_right(outgoing_turn_type)))
+        {
+            return false;
+        }
+
+        // TODO - determine if we need to add name checks or need to check headings
+        // of the inbound and outbound oneway edges
+
+        // Assume this is an intersection internal edge
+        return true;
     };
 
     auto const collect_edge_info_fn = [&](auto const &edges1, NodeID node2) {
@@ -168,7 +233,7 @@ std::unordered_set<EdgeID> findSegregatedNodes(const extractor::NodeBasedGraphFa
             if (target == node2)
                 continue;
 
-            info.push_back(get_edge_info(target, graph.GetEdgeData(e)));
+            info.push_back(get_edge_info(e, target, graph.GetEdgeData(e)));
         }
 
         if (info.empty())
@@ -178,22 +243,6 @@ std::unordered_set<EdgeID> findSegregatedNodes(const extractor::NodeBasedGraphFa
             return e1.node < e2.node;
         });
 
-        // Merge equal infos with correct direction.
-        auto curr = info.begin();
-        auto next = curr;
-        while (++next != info.end())
-        {
-            if (curr->node == next->node)
-            {
-                BOOST_ASSERT(curr->name == next->name);
-                BOOST_ASSERT(curr->road_class == next->road_class);
-                BOOST_ASSERT(curr->direction != next->direction);
-                curr->direction = 2;
-            }
-            else
-                curr = next;
-        }
-
         info.erase(
             std::unique(info.begin(),
                         info.end(),
@@ -203,36 +252,39 @@ std::unordered_set<EdgeID> findSegregatedNodes(const extractor::NodeBasedGraphFa
         return info;
     };
 
-    auto const isSegregatedFn = [&](auto const &edgeData,
+    auto const isSegregatedFn = [&](EdgeID edge_id,
+                                    auto const &edge_data,
                                     auto const &edges1,
                                     NodeID node1,
                                     auto const &edges2,
                                     NodeID node2,
-                                    double edgeLength) {
+                                    double edge_length) {
-        return IsSegregated(collect_edge_info_fn(edges1, node2),
+        return isSegregated(node1,
+                            collect_edge_info_fn(edges1, node2),
                             collect_edge_info_fn(edges2, node1),
-                            get_edge_info(node1, edgeData),
+                            get_edge_info(edge_id, node1, edge_data),
-                            edgeLength);
+                            edge_length);
     };
 
     std::unordered_set<EdgeID> segregated_edges;
 
-    for (NodeID sourceID = 0; sourceID < graph.GetNumberOfNodes(); ++sourceID)
+    for (NodeID source_id = 0; source_id < graph.GetNumberOfNodes(); ++source_id)
     {
-        auto const sourceEdges = graph.GetAdjacentEdgeRange(sourceID);
+        auto const source_edges = graph.GetAdjacentEdgeRange(source_id);
-        for (EdgeID edgeID : sourceEdges)
+        for (EdgeID edge_id : source_edges)
         {
-            auto const &edgeData = graph.GetEdgeData(edgeID);
+            auto const &edgeData = graph.GetEdgeData(edge_id);
 
             if (edgeData.reversed)
                 continue;
 
-            NodeID const targetID = graph.GetTarget(edgeID);
+            NodeID const target_id = graph.GetTarget(edge_id);
-            auto const targetEdges = graph.GetAdjacentEdgeRange(targetID);
+            auto const targetEdges = graph.GetAdjacentEdgeRange(target_id);
 
-            double const length = get_edge_length(sourceID, edgeID, targetID);
+            double const length = get_edge_length(source_id, edge_id, target_id);
-            if (isSegregatedFn(edgeData, sourceEdges, sourceID, targetEdges, targetID, length))
+            if (isSegregatedFn(
-                segregated_edges.insert(edgeID);
+                    edge_id, edgeData, source_edges, source_id, targetEdges, target_id, length))
+                segregated_edges.insert(edge_id);
         }
     }
 

test/nodejs/constants.js

@@ -10,7 +10,7 @@ exports.three_test_coordinates = [[7.41337, 43.72956],
 
 exports.two_test_coordinates = exports.three_test_coordinates.slice(0, 2)
 
-exports.test_tile = {'at': [17059, 11948, 15], 'size': 169387};
+exports.test_tile = {'at': [17059, 11948, 15], 'size': 168612};
 
 // Test files generated by the routing engine; check test/data
 if (process.env.OSRM_DATA_PATH !== undefined) {