reduce numbers of intersections in findNextIntersection, don't normalise for turn lanes

Author: Moritz Kobitzsch

Diff for: features/guidance/collapse.feature

@@ -338,10 +338,10 @@ Feature: Collapse
         Given the node map
             """
               a f       g
-
-              b e
-
-
+              | |   . '
+              b-e '
+              / /
+             / /
             c d
             """
 
@@ -353,13 +353,13 @@ Feature: Collapse
             | ge    | primary | second | no     |
 
         When I route I should get
-            | waypoints | route               | turns                          |
-            | d,c       | first,first,first   | depart,continue uturn,arrive   |
-            | a,f       | first,first,first   | depart,continue uturn,arrive   |
-            | a,g       | first,second,second | depart,turn left,arrive        |
-            | d,g       | first,second,second | depart,turn right,arrive       |
-            | g,f       | second,first,first  | depart,turn right,arrive       |
-            | g,c       | second,first,first  | depart,end of road left,arrive |
+            | waypoints | route               | turns                        |
+            | d,c       | first,first,first   | depart,continue uturn,arrive |
+            | a,f       | first,first,first   | depart,continue uturn,arrive |
+            | a,g       | first,second,second | depart,turn left,arrive      |
+            | d,g       | first,second,second | depart,turn right,arrive     |
+            | g,f       | second,first,first  | depart,turn right,arrive     |
+            | g,c       | second,first,first  | depart,turn left,arrive      |
 
     Scenario: Do not collapse turning roads
         Given the node map

Diff for: include/engine/datafacade/process_memory_datafacade.hpp

@@ -3,8 +3,8 @@
 
 // implements all data storage when shared memory is _NOT_ used
 
-#include "engine/datafacade/contiguous_internalmem_datafacade_base.hpp"
 #include "storage/storage.hpp"
+#include "engine/datafacade/contiguous_internalmem_datafacade_base.hpp"
 
 namespace osrm
 {

Diff for: include/extractor/extraction_way.hpp

@@ -15,19 +15,18 @@ namespace extractor
 {
 namespace detail
 {
-    inline void maybeSetString(std::string &str, const char* value)
+inline void maybeSetString(std::string &str, const char *value)
+{
+    if (value == nullptr)
+    {
+        str.clear();
+    }
+    else
     {
-        if (value == nullptr)
-        {
-            str.clear();
-        }
-        else
-        {
-            str = std::string(value);
-        }
+        str = std::string(value);
     }
 }
-
+}
 
 /**
  * This struct is the direct result of the call to ```way_function```
@@ -67,18 +66,24 @@ struct ExtractionWay
     TravelMode get_backward_mode() const { return backward_travel_mode; }
 
     // wrappers to allow assigning nil (nullptr) to string values
-    void SetName(const char* value) { detail::maybeSetString(name, value); }
-    const char* GetName() const { return name.c_str(); }
-    void SetRef(const char* value) { detail::maybeSetString(ref, value); }
-    const char* GetRef() const { return ref.c_str(); }
-    void SetDestinations(const char* value) { detail::maybeSetString(destinations, value); }
-    const char* GetDestinations() const { return destinations.c_str(); }
-    void SetPronunciation(const char* value) { detail::maybeSetString(pronunciation, value); }
-    const char* GetPronunciation() const { return pronunciation.c_str(); }
-    void SetTurnLanesForward(const char* value) { detail::maybeSetString(turn_lanes_forward, value); }
-    const char* GetTurnLanesForward() const { return turn_lanes_forward.c_str(); }
-    void SetTurnLanesBackward(const char* value) { detail::maybeSetString(turn_lanes_backward, value); }
-    const char* GetTurnLanesBackward() const { return turn_lanes_backward.c_str(); }
+    void SetName(const char *value) { detail::maybeSetString(name, value); }
+    const char *GetName() const { return name.c_str(); }
+    void SetRef(const char *value) { detail::maybeSetString(ref, value); }
+    const char *GetRef() const { return ref.c_str(); }
+    void SetDestinations(const char *value) { detail::maybeSetString(destinations, value); }
+    const char *GetDestinations() const { return destinations.c_str(); }
+    void SetPronunciation(const char *value) { detail::maybeSetString(pronunciation, value); }
+    const char *GetPronunciation() const { return pronunciation.c_str(); }
+    void SetTurnLanesForward(const char *value)
+    {
+        detail::maybeSetString(turn_lanes_forward, value);
+    }
+    const char *GetTurnLanesForward() const { return turn_lanes_forward.c_str(); }
+    void SetTurnLanesBackward(const char *value)
+    {
+        detail::maybeSetString(turn_lanes_backward, value);
+    }
+    const char *GetTurnLanesBackward() const { return turn_lanes_backward.c_str(); }
 
     double forward_speed;
     double backward_speed;

Diff for: include/extractor/guidance/coordinate_extractor.hpp

@@ -1,6 +1,7 @@
 #ifndef OSRM_EXTRACTOR_COORDINATE_EXTRACTOR_HPP_
 #define OSRM_EXTRACTOR_COORDINATE_EXTRACTOR_HPP_
 
+#include <utility>
 #include <vector>
 
 #include "extractor/compressed_edge_container.hpp"
@@ -28,6 +29,7 @@ class CoordinateExtractor
     /* Find a interpolated coordinate a long the compressed geometries. The desired coordinate
      * should be in a certain distance. This method is dedicated to find representative coordinates
      * at turns.
+     * Since we are computing the length of the segment anyhow, we also return it.
      */
     OSRM_ATTR_WARN_UNUSED
     util::Coordinate GetCoordinateAlongRoad(const NodeID intersection_node,
@@ -36,12 +38,23 @@ class CoordinateExtractor
                                             const NodeID to_node,
                                             const std::uint8_t number_of_in_lanes) const;
 
-    // instead of finding only a single coordinate, we can also list all coordinates along a road.
+    // same as above, only with precomputed coordinate vector (move it in)
     OSRM_ATTR_WARN_UNUSED
-    std::vector<util::Coordinate> GetCoordinatesAlongRoad(const NodeID intersection_node,
-                                                          const EdgeID turn_edge,
-                                                          const bool traversed_in_reverse,
-                                                          const NodeID to_node) const;
+    util::Coordinate
+    ExtractRepresentativeCoordinate(const NodeID intersection_node,
+                                    const EdgeID turn_edge,
+                                    const bool traversed_in_reverse,
+                                    const NodeID to_node,
+                                    const std::uint8_t intersection_lanes,
+                                    std::vector<util::Coordinate> coordinates) const;
+
+    // instead of finding only a single coordinate, we can also list all coordinates along a
+    // road.
+    OSRM_ATTR_WARN_UNUSED std::vector<util::Coordinate>
+    GetCoordinatesAlongRoad(const NodeID intersection_node,
+                            const EdgeID turn_edge,
+                            const bool traversed_in_reverse,
+                            const NodeID to_node) const;
 
     // wrapper in case of normal forward edges (traversed_in_reverse = false, to_node =
     // node_based_graph.GetTarget(turn_edge)

Diff for: include/extractor/guidance/intersection.hpp

@@ -9,6 +9,8 @@
 #include "util/node_based_graph.hpp"
 #include "util/typedefs.hpp" // EdgeID
 
+#include <boost/optional.hpp>
+
 namespace osrm
 {
 namespace extractor
@@ -53,10 +55,13 @@ struct ConnectedRoad final : public TurnOperation
 {
     using Base = TurnOperation;
 
-    ConnectedRoad(const TurnOperation turn, const bool entry_allowed = false);
+    ConnectedRoad(const TurnOperation turn,
+                  const bool entry_allowed = false,
+                  const boost::optional<double> segment_length = {});
 
     // a turn may be relevant to good instructions, even if we cannot enter the road
     bool entry_allowed;
+    boost::optional<double> segment_length;
 
     // used to sort the set of connected roads (we require sorting throughout turn handling)
     bool compareByAngle(const ConnectedRoad &other) const;

Diff for: include/extractor/guidance/turn_analysis.hpp

@@ -68,6 +68,9 @@ class TurnAnalysis
     std::vector<TurnOperation>
     transformIntersectionIntoTurns(const Intersection &intersection) const;
 
+    Intersection
+    assignTurnTypes(const NodeID from_node, const EdgeID via_eid, Intersection intersection) const;
+
     const IntersectionGenerator &GetIntersectionGenerator() const;
 
   private:
@@ -79,9 +82,6 @@ class TurnAnalysis
     const TurnHandler turn_handler;
     const SliproadHandler sliproad_handler;
 
-    Intersection
-    assignTurnTypes(const NodeID from_node, const EdgeID via_eid, Intersection intersection) const;
-
     // Utility function, setting basic turn types. Prepares for normal turn handling.
     Intersection
     setTurnTypes(const NodeID from, const EdgeID via_edge, Intersection intersection) const;

Diff for: include/extractor/guidance/turn_discovery.hpp

@@ -22,7 +22,7 @@ namespace lanes
 bool findPreviousIntersection(
     const NodeID node,
     const EdgeID via_edge,
-    const Intersection intersection,
+    const Intersection &intersection,
     const IntersectionGenerator &intersection_generator,
     const util::NodeBasedDynamicGraph &node_based_graph, // query edge data
     // output parameters, will be in an arbitrary state on failure

Diff for: include/storage/shared_datatype.hpp

@@ -134,11 +134,11 @@ struct DataLayout
     // Interface Similar to [ptr.align] but omits space computation.
     // The method can be removed and changed directly to an std::align
     // function call after dropping gcc < 5 support.
-    inline void* align(std::size_t align, std::size_t , void*& ptr) const noexcept
+    inline void *align(std::size_t align, std::size_t, void *&ptr) const noexcept
     {
         const auto intptr = reinterpret_cast<uintptr_t>(ptr);
         const auto aligned = (intptr - 1u + align) & -align;
-        return ptr = reinterpret_cast<void*>(aligned);
+        return ptr = reinterpret_cast<void *>(aligned);
     }
 
     inline void *GetAlignedBlockPtr(void *ptr, BlockID bid) const

Diff for: src/engine/plugins/match.cpp

@@ -19,7 +19,8 @@
 #include <string>
 #include <vector>
 
-static double search_radius_for_gps_radius(double gps_radius) {
+static double search_radius_for_gps_radius(double gps_radius)
+{
     // For a given GPS radius, determine the radius we need to search for candidate street segments
     // to have a 99.9% chance of finding the correct segment.
     // For more detail, see the analysis at https://github.com/Project-OSRM/osrm-backend/pull/3184

Diff for: src/extractor/guidance/coordinate_extractor.cpp

@@ -56,13 +56,29 @@ CoordinateExtractor::GetCoordinateAlongRoad(const NodeID intersection_node,
                                             const NodeID to_node,
                                             const std::uint8_t intersection_lanes) const
 {
-    const auto considered_lanes =
-        (intersection_lanes == 0) ? ASSUMED_LANE_COUNT : intersection_lanes;
-
     // we first extract all coordinates from the road
     auto coordinates =
         GetCoordinatesAlongRoad(intersection_node, turn_edge, traversed_in_reverse, to_node);
 
+    return ExtractRepresentativeCoordinate(intersection_node,
+                                           turn_edge,
+                                           traversed_in_reverse,
+                                           to_node,
+                                           intersection_lanes,
+                                           std::move(coordinates));
+}
+
+util::Coordinate CoordinateExtractor::ExtractRepresentativeCoordinate(
+    const NodeID intersection_node,
+    const EdgeID turn_edge,
+    const bool traversed_in_reverse,
+    const NodeID to_node,
+    const std::uint8_t intersection_lanes,
+    std::vector<util::Coordinate> coordinates) const
+{
+    const auto considered_lanes =
+        (intersection_lanes == 0) ? ASSUMED_LANE_COUNT : intersection_lanes;
+
     /* if we are looking at a straight line, we don't care where exactly the coordinate
      * is. Simply return the final coordinate. Turn angles/turn vectors are the same no matter which
      * coordinate we look at.
@@ -406,10 +422,12 @@ CoordinateExtractor::GetCoordinatesAlongRoad(const NodeID intersection_node,
                            geometry.rend(),
                            std::back_inserter(result),
                            compressedGeometryToCoordinate);
+            BOOST_ASSERT(intersection_node < node_coordinates.size());
             result.push_back(node_coordinates[intersection_node]);
         }
         else
         {
+            BOOST_ASSERT(intersection_node < node_coordinates.size());
             result.push_back(node_coordinates[intersection_node]);
             std::transform(geometry.begin(),
                            geometry.end(),

Diff for: src/extractor/guidance/intersection.cpp

@@ -17,8 +17,10 @@ namespace extractor
 namespace guidance
 {
 
-ConnectedRoad::ConnectedRoad(const TurnOperation turn, const bool entry_allowed)
-    : TurnOperation(turn), entry_allowed(entry_allowed)
+ConnectedRoad::ConnectedRoad(const TurnOperation turn,
+                             const bool entry_allowed,
+                             boost::optional<double> segment_length)
+    : TurnOperation(turn), entry_allowed(entry_allowed), segment_length(segment_length)
 {
 }