Avoid using signed integers for edge IDs

Bump OSRM version to 5.16.5
Simplify toEdges and make it more robust against accidental memory allocations
2018-04-12 10:10:37 +02:00 · 2018-04-09 11:10:52 +00:00 · 2018-04-09 11:09:00 +00:00 · 2018-03-13 11:35:42 -07:00 · 2018-03-13 11:34:22 -07:00 · 2018-03-09 16:02:11 +01:00
24 changed files with 437 additions and 98 deletions
@@ -1,5 +1,36 @@
-# 5.16.0 RC 6
-  - Changes from 5.15.1:
+# 5.16.6
+  - Changes from 5.16.5:
+    - Bugfixes:
+      - FIXED: integer overflow in `DynamicGraph::Renumber` [#5021](https://github.com/Project-OSRM/osrm-backend/pull/5021)
+
+# 5.16.5
+  - Changes from 5.16.4:
+    - Debug:
+      - ADDED: Made toEdges allocation more robust and add better memory usage debugging.
+
+# 5.16.4
+  - Changes from 5.16.3:
+    - Bugfixes:
+      - FIXED: Properly calculate annotations for speeds, durations and distances when waypoints are used with mapmatching [#4949](https://github.com/Project-OSRM/osrm-backend/pull/4949)
+
+# 5.16.3
+  - Changes from 5.16.2:
+      - FIXED: Remove the last short annotation segment in `trimShortSegments` [#4946](https://github.com/Project-OSRM/osrm-backend/pull/4946)
+    - Performance
+      - FIXED: Speed up response time when lots of legs exist and geojson is used with `steps=true` [#4936](https://github.com/Project-OSRM/osrm-backend/pull/4936)
+
+# 5.16.2
+  - Changes from 5.16.1:
+    - Bugfixes:
+      - FIXED #4920: Use smaller range for U-turn angles in map-matching [#4920](https://github.com/Project-OSRM/osrm-backend/pull/4920)
+
+# 5.16.1
+  - Changes from 5.16.0:
+    - Bugfixes
+      - FIXED #4909: deduplication of route steps when waypoints are used [#4909](https://github.com/Project-OSRM/osrm-backend/issues/4909)
+
+# 5.16.0
+  - Changes from 5.15.2:
    - Guidance
      - ADDED #4676: Support for maneuver override relation, allowing data-driven overrides for turn-by-turn instructions [#4676](https://github.com/Project-OSRM/osrm-backend/pull/4676)
      - CHANGED #4830: Announce reference change if names are empty
@@ -14,6 +45,13 @@
      - FIXED #4763: Add support for non-numerical units in car profile for maxheight [#4763](https://github.com/Project-OSRM/osrm-backend/issues/4763)
      - ADDED #4872: Handling of `barrier=height_restrictor` nodes [#4872](https://github.com/Project-OSRM/osrm-backend/pull/4872)

+# 5.15.2
+  - Changes from 5.15.1:
+    - Features:
+        - ADDED: Exposed the waypoints parameter in the node bindings interface
+    - Bugfixes:
+        - FIXED: Segfault causing bug in leg collapsing map matching when traversing edges in reverse
+
 # 5.15.1
  - Changes from 5.15.0:
    - Bugfixes:
@@ -61,7 +61,7 @@ endif()
 project(OSRM C CXX)
 set(OSRM_VERSION_MAJOR 5)
 set(OSRM_VERSION_MINOR 16)
-set(OSRM_VERSION_PATCH 0)
+set(OSRM_VERSION_PATCH 6)
 set(OSRM_VERSION "${OSRM_VERSION_MAJOR}.${OSRM_VERSION_MINOR}.${OSRM_VERSION_PATCH}")

 add_definitions(-DOSRM_PROJECT_DIR="${CMAKE_CURRENT_SOURCE_DIR}")
@@ -74,7 +74,7 @@ module.exports = function () {

                        if (headers.has('turns')) {
                            if (json.matchings.length != 1) throw new Error('*** Checking turns only supported for matchings with one subtrace');
-                            turns = this.turnList(json.matchings[0].instructions);
+                            turns = this.turnList(json.matchings[0]);
                        }

                        if (headers.has('route')) {
@@ -11,7 +11,7 @@ Feature: Annotations
            """

        And the query options
-          | annotations | duration,speed,weight |
+          | annotations | duration,speed,weight,nodes |

        And the ways
            | nodes | highway     |
@@ -22,10 +22,10 @@ Feature: Annotations
            | lm    | residential |

        When I route I should get
-            | from | to | route    | a:speed     | a:weight |
-            | h    | j  | hk,jk,jk | 6.7:6.7     | 15:15    |
-            | i    | m  | il,lm,lm | 6.7:6.7     | 15:15    |
-            | j    | m  | jk,lm    | 6.7:6.7:6.7 | 15:15:15 |
+            | from | to | route    | a:speed     | a:weight | a:nodes |
+            | h    | j  | hk,jk,jk | 6.7:6.7     | 15:15    | 1:4:3   |
+            | i    | m  | il,lm,lm | 6.7:6.7     | 15:15    | 2:5:6   |
+            | j    | m  | jk,lm    | 6.7:6.7:6.7 | 15:15:15 | 3:4:5:6 |


    Scenario: There should be different forward/reverse datasources
@@ -81,5 +81,5 @@ Feature: Annotations
          | bearings    | 90,5;180,5                    |

        When I route I should get
-            | from | to | route    | a:speed   | a:distance              | a:duration | a:nodes |
-            | a    | c  | abc,abc  | 10:10:10  | 249.998641:299.931643:0 | 25:30:0    | 1:2:3   |
+            | from | to | route    | a:speed | a:distance            | a:duration | a:nodes |
+            | a    | c  | abc,abc  | 10:10   | 249.998641:299.931643 | 25:30      | 1:2:3   |
@@ -626,3 +626,170 @@ Feature: Basic Map Matching
        | trace    | timestamps                                   | matchings  | code |
        | abbecd   | 10 11 27 1516914902 1516914913 1516914952    | ab,ecd     | Ok   |

+    Scenario: Regression test - waypoints trimming too much geometry
+        # fixes bug in map matching collapsing that was dropping path geometries
+        # after segments that had 0 distance in internal route results
+        Given the node map
+            """
+            ad
+            |
+            |
+            |
+            |
+            |e   g
+            b--------------c
+            f              h
+            """
+
+        And the ways
+            | nodes |
+            | ab    |
+            | bc    |
+
+        Given the query options
+            | waypoints | 0;4   |
+            | overview  | full  |
+
+        When I match I should get
+            | trace    | geometry                           | code |
+            | defgh    | 1,1,1,0.999461,1.000674,0.999461   | Ok   |
+
+    @match @testbot
+    Scenario: Regression test - waypoints trimming too much geometry
+        Given the profile "testbot"
+        Given a grid size of 10 meters
+        Given the query options
+          | geometries | geojson |
+        Given the node map
+          """
+            bh
+             |
+             |
+             |
+             c
+             g\
+               \k
+                \
+                 \
+                  \
+                 j f
+          """
+        And the ways
+          | nodes |
+          | hc    |
+          | cf    |
+        Given the query options
+          | waypoints | 0;3  |
+          | overview  | full |
+        When I match I should get
+          | trace | geometry                                      | code |
+          | bgkj  | 1.000135,1,1.000135,0.99964,1.000387,0.999137 | Ok   |
+
+
+    @match @testbot
+    # Regression test for issue #4919
+    Scenario: Regression test - non-uturn maneuver preferred over uturn
+        Given the profile "testbot"
+        Given a grid size of 10 meters
+        Given the query options
+          | geometries | geojson |
+        Given the node map
+          """
+                e
+                ;
+                ;
+          a----hb-----c
+                ;
+                ;
+                d
+          """
+        And the ways
+          | nodes |
+          | abc   |
+          | dbe   |
+        Given the query options
+          | waypoints | 0;2  |
+          | overview  | full |
+          | steps     | true |
+        When I match I should get
+          | trace | geometry                                   | turns                    | code |
+          | abc   | 1,0.99973,1.00027,0.99973,1.000539,0.99973 | depart,arrive            | Ok   |
+          | abd   | 1,0.99973,1.00027,0.99973,1.00027,0.999461 | depart,turn right,arrive | Ok   |
+          | abe   | 1,0.99973,1.00027,0.99973,1.00027,1        | depart,turn left,arrive  | Ok   |
+          | ahd   | 1,0.99973,1.00027,0.99973,1.00027,0.999461 | depart,turn right,arrive | Ok   |
+          | ahe   | 1,0.99973,1.00027,0.99973,1.00027,1        | depart,turn left,arrive  | Ok   |
+
+    @match @testbot
+    Scenario: Regression test - add source phantoms properly (one phantom on one edge)
+        Given the profile "testbot"
+        Given a grid size of 10 meters
+        Given the node map
+          """
+          a--1-b2-cd3--e
+          """
+        And the ways
+          | nodes |
+          | ab    |
+          | bcd   |
+          | de    |
+        Given the query options
+          | geometries     | geojson         |
+          | overview       | full            |
+          | steps          | true            |
+          | waypoints      | 0;2             |
+          | annotations    | duration,weight |
+          | generate_hints | false           |
+        When I match I should get
+          | trace | geometry                                             | a:duration    | a:weight      | duration |
+          | 123   | 1.000135,1,1.000225,1,1.00036,1,1.000405,1,1.00045,1 | 1:1.5:0.5:0.5 | 1:1.5:0.5:0.5 | 3.5      |
+          | 321   | 1.00045,1,1.000405,1,1.00036,1,1.000225,1,1.000135,1 | 0.5:0.5:1.5:1 | 0.5:0.5:1.5:1 | 3.5      |
+
+    @match @testbot
+    Scenario: Regression test - add source phantom properly (two phantoms on one edge)
+        Given the profile "testbot"
+        Given a grid size of 10 meters
+        Given the node map
+          """
+          a--1-b23-c4--d
+          """
+        And the ways
+          | nodes |
+          | ab    |
+          | bc    |
+          | cd    |
+        Given the query options
+          | geometries     | geojson         |
+          | overview       | full            |
+          | steps          | true            |
+          | waypoints      | 0;3             |
+          | annotations    | duration,weight |
+          | generate_hints | false           |
+        When I match I should get
+          | trace | geometry                                   | a:duration | a:weight | duration |
+          | 1234  | 1.000135,1,1.000225,1,1.000405,1,1.00045,1 | 1:2:0.5    | 1:2:0.5  | 3.5      |
+          | 4321  | 1.00045,1,1.000405,1,1.000225,1,1.000135,1 | 0.5:2:1    | 0.5:2:1  | 3.5      |
+
+    @match @testbot
+    Scenario: Regression test - add source phantom properly (two phantoms on one edge)
+        Given the profile "testbot"
+        Given a grid size of 10 meters
+        Given the node map
+          """
+          a--12345-b
+          """
+        And the ways
+          | nodes |
+          | ab    |
+        Given the query options
+          | geometries     | geojson                  |
+          | overview       | full                     |
+          | steps          | true                     |
+          | waypoints      | 0;3                      |
+          | annotations    | duration,weight,distance |
+          | generate_hints | false                    |
+
+        # These should have the same weights/duration in either direction
+        When I match I should get
+          | trace | geometry             | a:distance | a:duration | a:weight | duration |
+          | 2345  | 1.00018,1,1.000315,1 | 15.013264  | 1.5        | 1.5      | 1.5      |
+          | 4321  | 1.00027,1,1.000135,1 | 15.013264  | 1.5        | 1.5      | 1.5      |
@@ -48,10 +48,10 @@ Feature: Traffic - speeds

        When I route I should get
          | from | to | route       | speed   | weights              | a:datasources |
-          | a    | b  | ad,de,eb,eb | 30 km/h | 1275.7,400.4,378.2,0 | 1:0:0:0       |
+          | a    | b  | ad,de,eb,eb | 30 km/h | 1275.7,400.4,378.2,0 | 1:0:0         |
          | a    | c  | ad,dc,dc    | 31 km/h | 1275.7,956.8,0       | 1:0           |
-          | b    | c  | bc,bc       | 27 km/h | 741.5,0              | 1:0           |
-          | a    | d  | ad,ad       | 27 km/h | 1275.7,0             | 1:0           |
+          | b    | c  | bc,bc       | 27 km/h | 741.5,0              | 1             |
+          | a    | d  | ad,ad       | 27 km/h | 1275.7,0             | 1             |
          | d    | c  | dc,dc       | 36 km/h | 956.8,0              | 0             |
          | g    | b  | fb,fb       | 36 km/h | 164.7,0              | 0             |
          | a    | g  | ad,df,fb,fb | 30 km/h | 1275.7,487.5,304.7,0 | 1:0:0         |
@@ -74,12 +74,12 @@ Feature: Traffic - speeds

        When I route I should get
          | from | to | route       | speed   | weights              | a:datasources |
-          | a    | b  | ad,de,eb,eb | 30 km/h | 1275.7,400.4,378.2,0 | 1:0:0:0       |
+          | a    | b  | ad,de,eb,eb | 30 km/h | 1275.7,400.4,378.2,0 | 1:0:0         |
          | a    | c  | ad,dc,dc    | 31 km/h | 1275.7,956.8,0       | 1:0           |
-          | b    | c  | bc,bc       | 27 km/h | 741.5,0              | 1:0           |
-          | a    | d  | ad,ad       | 27 km/h | 1275.7,0             | 1:0           |
+          | b    | c  | bc,bc       | 27 km/h | 741.5,0              | 1             |
+          | a    | d  | ad,ad       | 27 km/h | 1275.7,0             | 1             |
          | d    | c  | dc,dc       | 36 km/h | 956.8,0              | 0             |
-          | g    | b  | ab,ab       | 1 km/h  | 10010.4,0            | 1:0           |
+          | g    | b  | ab,ab       | 1 km/h  | 10010.4,0            | 1             |
          | a    | g  | ab,ab       | 1 km/h  | 10010.3,0            | 1             |


@@ -106,14 +106,14 @@ Feature: Traffic - speeds

        When I route I should get
          | from | to | route       | speed   | weights                  | a:datasources |
-          | a    | b  | ab,ab       | 1 km/h  | 20020.73,0               | 1:0           |
-          | a    | c  | ab,bc,bc    | 2 km/h  | 20020.73,741.51,0        | 1:1:0         |
-          | b    | c  | bc,bc       | 27 km/h | 741.51,0                 | 1:0           |
+          | a    | b  | ab,ab       | 1 km/h  | 20020.73,0               | 1             |
+          | a    | c  | ab,bc,bc    | 2 km/h  | 20020.73,741.51,0        | 1:1           |
+          | b    | c  | bc,bc       | 27 km/h | 741.51,0                 | 1             |
          | a    | d  | ab,eb,de,de | 2 km/h  | 20020.73,378.17,400.41,0 | 1:0:0         |
          | d    | c  | dc,dc       | 36 km/h | 956.8,0                  | 0             |
-          | g    | b  | ab,ab       | 1 km/h  | 10010.37,0               | 1:0           |
+          | g    | b  | ab,ab       | 1 km/h  | 10010.37,0               | 1             |
          | a    | g  | ab,ab       | 1 km/h  | 10010.36,0               | 1             |
-          | g    | a  | ab,ab       | 1 km/h  | 10010.36,0               | 1:1           |
+          | g    | a  | ab,ab       | 1 km/h  | 10010.36,0               | 1             |


    Scenario: Speeds that isolate a single node (a)
@@ -135,14 +135,14 @@ Feature: Traffic - speeds
          | annotations | true |

        When I route I should get
-          | from | to | route    | speed   | weights       | a:datasources |
-          | a    | b  | fb,fb    | 36 km/h | 329.4,0       | 0             |
-          | a    | c  | fb,bc,bc | 30 km/h | 329.4,741.5,0 | 0:1:0         |
-          | b    | c  | bc,bc    | 27 km/h | 741.5,0       | 1:0           |
-          | a    | d  | fb,df,df | 36 km/h | 140,487.5,0   | 0:0:0         |
-          | d    | c  | dc,dc    | 36 km/h | 956.8,0       | 0             |
-          | g    | b  | fb,fb    | 36 km/h | 164.7,0       | 0             |
-          | a    | g  | fb,fb    | 36 km/h | 164.7,0       | 0             |
+          | from | to | route    | speed   | weights       | a:datasources | a:speed | a:nodes|
+          | a    | b  | fb,fb    | 36 km/h | 329.4,0       | 0             | 10      | 6:2    |
+          | a    | c  | fb,bc,bc | 30 km/h | 329.4,741.5,0 | 0:1           | 10:7.5  | 6:2:3  |
+          | b    | c  | bc,bc    | 27 km/h | 741.5,0       | 1             | 7.5     | 2:3    |
+          | a    | d  | fb,df,df | 36 km/h | 140,487.5,0   | 0:0           | 10:10   | 2:6:4  |
+          | d    | c  | dc,dc    | 36 km/h | 956.8,0       | 0             | 10      | 4:3    |
+          | g    | b  | fb,fb    | 36 km/h | 164.7,0       | 0             | 10      | 6:2    |
+          | a    | g  | fb,fb    | 36 km/h | 164.7,0       | 0             | 10      | 6:2    |


    Scenario: Verify that negative values cause an error, they're not valid at all
@@ -53,8 +53,8 @@ Feature: Weight tests

        When I route I should get
            | waypoints | route   | distances | weights | times   | a:distance          | a:duration | a:weight | a:speed |
-            | s,t       | abc,abc | 20m,0m    | 2.1,0   | 2.1s,0s | 20.017685           | 3          | 3        | 6.7     |
-            | t,s       | abc,abc | 20m,0m    | 2.1,0   | 2.1s,0s | 20.017685           | 3.1        | 3.1      | 6.5     |
+            | s,t       | abc,abc | 20m,0m    | 2.1,0   | 2.1s,0s | 20.017685           | 2.1        | 2.1      | 9.5     |
+            | t,s       | abc,abc | 20m,0m    | 2.1,0   | 2.1s,0s | 20.017685           | 2.1        | 2.1      | 9.5     |
            | s,e       | abc,abc | 40m,0m    | 4.1,0   | 4.1s,0s | 30.026527:10.008842 | 3.1:1      | 3.1:1    | 9.7:10  |
            | e,s       | abc,abc | 40m,0m    | 4.1,0   | 4.1s,0s | 10.008842:30.026527 | 1:3.1      | 1:3.1    | 10:9.7  |

@@ -128,23 +128,24 @@ ContractorGraph toContractorGraph(NodeID number_of_nodes, InputEdgeContainer inp

 template <class Edge, typename GraphT> inline std::vector<Edge> toEdges(GraphT graph)
 {
-    std::vector<Edge> edges;
-    edges.reserve(graph.GetNumberOfEdges());
+    util::Log() << "Converting contracted graph with " << graph.GetNumberOfEdges()
+                << " to edge list (" << (graph.GetNumberOfEdges() * sizeof(Edge)) << " bytes)";
+    std::vector<Edge> edges(graph.GetNumberOfEdges());

-    util::UnbufferedLog log;
-    log << "Getting edges of minimized graph ";
-    util::Percent p(log, graph.GetNumberOfNodes());
-    const NodeID number_of_nodes = graph.GetNumberOfNodes();
-    if (graph.GetNumberOfNodes())
    {
-        Edge new_edge;
+        util::UnbufferedLog log;
+        log << "Getting edges of minimized graph ";
+        util::Percent p(log, graph.GetNumberOfNodes());
+        const NodeID number_of_nodes = graph.GetNumberOfNodes();
+        std::size_t edge_index = 0;
        for (const auto node : util::irange(0u, number_of_nodes))
        {
            p.PrintStatus(node);
            for (auto edge : graph.GetAdjacentEdgeRange(node))
            {
                const NodeID target = graph.GetTarget(edge);
-                const ContractorGraph::EdgeData &data = graph.GetEdgeData(edge);
+                const auto &data = graph.GetEdgeData(edge);
+                auto &new_edge = edges[edge_index++];
                new_edge.source = node;
                new_edge.target = target;
                BOOST_ASSERT_MSG(SPECIAL_NODEID != new_edge.target, "Target id invalid");
@@ -156,16 +157,12 @@ template <class Edge, typename GraphT> inline std::vector<Edge> toEdges(GraphT g
                                 "edge id invalid");
                new_edge.data.forward = data.forward;
                new_edge.data.backward = data.backward;
-                edges.push_back(new_edge);
            }
        }
+        BOOST_ASSERT(edge_index == edges.size());
    }

-    // sort and remove duplicates
    tbb::parallel_sort(edges.begin(), edges.end());
-    auto new_end = std::unique(edges.begin(), edges.end());
-    edges.resize(new_end - edges.begin());
-    edges.shrink_to_fit();

    return edges;
 }
@@ -211,12 +211,16 @@ class RouteAPI : public BaseAPI
        }

        std::vector<util::json::Value> step_geometries;
+        const auto total_step_count =
+            std::accumulate(legs.begin(), legs.end(), 0, [](const auto &v, const auto &leg) {
+                return v + leg.steps.size();
+            });
+        step_geometries.reserve(total_step_count);
+
        for (const auto idx : util::irange<std::size_t>(0UL, legs.size()))
        {
            auto &leg_geometry = leg_geometries[idx];

-            step_geometries.reserve(step_geometries.size() + legs[idx].steps.size());
-
            std::transform(
                legs[idx].steps.begin(),
                legs[idx].steps.end(),
@@ -12,6 +12,7 @@
 #include "util/coordinate_calculation.hpp"

 #include <algorithm>
+#include <cmath>
 #include <utility>
 #include <vector>

@@ -80,7 +81,9 @@ inline LegGeometry assembleGeometry(const datafacade::BaseDataFacade &facade,
        prev_coordinate = coordinate;

        const auto osm_node_id = facade.GetOSMNodeIDOfNode(path_point.turn_via_node);
-        if (osm_node_id != geometry.osm_node_ids.back())
+
+        if (osm_node_id != geometry.osm_node_ids.back() ||
+            path_point.turn_instruction.type != osrm::guidance::TurnType::NoTurn)
        {
            geometry.annotations.emplace_back(LegGeometry::Annotation{
                current_distance,
@@ -111,15 +114,39 @@ inline LegGeometry assembleGeometry(const datafacade::BaseDataFacade &facade,
    const std::vector<DatasourceID> forward_datasources =
        facade.GetUncompressedForwardDatasources(target_geometry_id);

-    // FIXME if source and target phantoms are on the same segment then duration and weight
-    // will be from one projected point till end of segment
-    // testbot/weight.feature:Start and target on the same and adjacent edge
-    geometry.annotations.emplace_back(LegGeometry::Annotation{
-        current_distance,
-        (reversed_target ? target_node.reverse_duration : target_node.forward_duration) / 10.,
-        (reversed_target ? target_node.reverse_weight : target_node.forward_weight) /
-            facade.GetWeightMultiplier(),
-        forward_datasources[target_node.fwd_segment_position]});
+    // This happens when the source/target are on the same edge-based-node
+    // There will be no entries in the unpacked path, thus no annotations.
+    // We will need to calculate the lone annotation by looking at the position
+    // of the source/target nodes, and calculating their differences.
+    if (geometry.annotations.empty())
+    {
+        auto duration =
+            std::abs(
+                (reversed_target ? target_node.reverse_duration : target_node.forward_duration) -
+                (reversed_source ? source_node.reverse_duration : source_node.forward_duration)) /
+            10.;
+        BOOST_ASSERT(duration >= 0);
+        auto weight =
+            std::abs((reversed_target ? target_node.reverse_weight : target_node.forward_weight) -
+                     (reversed_source ? source_node.reverse_weight : source_node.forward_weight)) /
+            facade.GetWeightMultiplier();
+        BOOST_ASSERT(weight >= 0);
+
+        geometry.annotations.emplace_back(
+            LegGeometry::Annotation{current_distance,
+                                    duration,
+                                    weight,
+                                    forward_datasources[target_node.fwd_segment_position]});
+    }
+    else
+    {
+        geometry.annotations.emplace_back(LegGeometry::Annotation{
+            current_distance,
+            (reversed_target ? target_node.reverse_duration : target_node.forward_duration) / 10.,
+            (reversed_target ? target_node.reverse_weight : target_node.forward_weight) /
+                facade.GetWeightMultiplier(),
+            forward_datasources[target_node.fwd_segment_position]});
+    }

    geometry.segment_offsets.push_back(geometry.locations.size());
    geometry.locations.push_back(target_node.location);
@@ -139,22 +139,38 @@ inline InternalRouteResult CollapseInternalRouteResult(const InternalRouteResult
        {
            BOOST_ASSERT(!collapsed.unpacked_path_segments.empty());
            auto &last_segment = collapsed.unpacked_path_segments.back();
-            // deduplicate last segment (needs to be checked for empty for the same node query edge
-            // case)
-            if (!last_segment.empty())
-                last_segment.pop_back();
-            // update target phantom node of leg
            BOOST_ASSERT(!collapsed.segment_end_coordinates.empty());
            collapsed.segment_end_coordinates.back().target_phantom =
                leggy_result.segment_end_coordinates[i].target_phantom;
            collapsed.target_traversed_in_reverse.back() =
                leggy_result.target_traversed_in_reverse[i];
            // copy path segments into current leg
-            last_segment.insert(last_segment.end(),
-                                leggy_result.unpacked_path_segments[i].begin(),
-                                leggy_result.unpacked_path_segments[i].end());
+            if (!leggy_result.unpacked_path_segments[i].empty())
+            {
+                auto old_size = last_segment.size();
+                last_segment.insert(last_segment.end(),
+                                    leggy_result.unpacked_path_segments[i].begin(),
+                                    leggy_result.unpacked_path_segments[i].end());
+
+                // The first segment of the unpacked path is missing the weight of the
+                // source phantom.  We need to add those values back so that the total
+                // edge weight is correct
+                last_segment[old_size].weight_until_turn +=
+
+                    leggy_result.source_traversed_in_reverse[i]
+                        ? leggy_result.segment_end_coordinates[i].source_phantom.reverse_weight
+                        : leggy_result.segment_end_coordinates[i].source_phantom.forward_weight;
+
+                last_segment[old_size].duration_until_turn +=
+                    leggy_result.source_traversed_in_reverse[i]
+                        ? leggy_result.segment_end_coordinates[i].source_phantom.reverse_duration
+                        : leggy_result.segment_end_coordinates[i].source_phantom.forward_duration;
+            }
        }
    }
+
+    BOOST_ASSERT(collapsed.segment_end_coordinates.size() ==
+                 collapsed.unpacked_path_segments.size());
    return collapsed;
 }
 }
@@ -261,6 +261,29 @@ inline void writeTurnLaneDescriptions(const boost::filesystem::path &path,
    storage::serialization::write(writer, turn_offsets);
    storage::serialization::write(writer, turn_masks);
 }
+
+// reads .osrm.maneuver_overrides
+template <typename StorageManeuverOverrideT, typename NodeSequencesT>
+inline void readManeuverOverrides(const boost::filesystem::path &path,
+                                  StorageManeuverOverrideT &maneuver_overrides,
+                                  NodeSequencesT &node_sequences)
+{
+    const auto fingerprint = storage::io::FileReader::VerifyFingerprint;
+    storage::io::FileReader reader{path, fingerprint};
+
+    serialization::read(reader, maneuver_overrides, node_sequences);
+}
+
+// writes .osrm.maneuver_overrides
+inline void writeManeuverOverrides(const boost::filesystem::path &path,
+                                   const std::vector<StorageManeuverOverride> &maneuver_overrides,
+                                   const std::vector<NodeID> &node_sequences)
+{
+    const auto fingerprint = storage::io::FileWriter::GenerateFingerprint;
+    storage::io::FileWriter writer{path, fingerprint};
+
+    serialization::write(writer, maneuver_overrides, node_sequences);
+}
 }
 }
 }
@@ -304,10 +304,17 @@ inline void write(storage::io::FileWriter &writer,
                  const std::vector<StorageManeuverOverride> &maneuver_overrides,
                  const std::vector<NodeID> &node_sequences)
 {
-    writer.WriteElementCount64(maneuver_overrides.size());
-    writer.WriteElementCount64(node_sequences.size());
-    writer.WriteFrom(maneuver_overrides);
-    writer.WriteFrom(node_sequences);
+    storage::serialization::write(writer, maneuver_overrides);
+    storage::serialization::write(writer, node_sequences);
+}
+
+template <typename ManeuverOverridesT, typename NodeSequenceT>
+inline void read(storage::io::FileReader &reader,
+                 ManeuverOverridesT &maneuver_overrides,
+                 NodeSequenceT &node_sequences)
+{
+    storage::serialization::read(reader, maneuver_overrides);
+    storage::serialization::read(reader, node_sequences);
 }
 }
 }
@@ -16,10 +16,14 @@ namespace partitioner
 struct PartitionerConfig final : storage::IOConfig
 {
    PartitionerConfig()
-        : IOConfig(
-              {".osrm", ".osrm.fileIndex", ".osrm.ebg_nodes"},
-              {".osrm.hsgr", ".osrm.cnbg"},
-              {".osrm.ebg", ".osrm.cnbg", ".osrm.cnbg_to_ebg", ".osrm.partition", ".osrm.cells"}),
+        : IOConfig({".osrm", ".osrm.fileIndex", ".osrm.ebg_nodes"},
+                   {".osrm.hsgr", ".osrm.cnbg"},
+                   {".osrm.ebg",
+                    ".osrm.cnbg",
+                    ".osrm.cnbg_to_ebg",
+                    ".osrm.partition",
+                    ".osrm.cells",
+                    ".osrm.maneuver_overrides"}),
          requested_num_threads(0), balance(1.2), boundary_factor(0.25), num_optimizing_cuts(10),
          small_component_size(1000),
          max_cell_sizes({128, 128 * 32, 128 * 32 * 16, 128 * 32 * 16 * 32})
@@ -2,6 +2,7 @@
 #define OSRM_PARTITIONER_RENUMBER_HPP

 #include "extractor/edge_based_node_segment.hpp"
+#include "extractor/maneuver_override.hpp"
 #include "extractor/nbg_to_ebg.hpp"
 #include "extractor/node_data_container.hpp"

@@ -73,6 +74,25 @@ inline void renumber(std::vector<extractor::NBGToEBG> &mapping,
    }
 }

+inline void renumber(std::vector<NodeID> &node_ids, const std::vector<std::uint32_t> &permutation)
+{
+    for (auto &node_id : node_ids)
+    {
+        if (node_id != SPECIAL_NODEID)
+            node_id = permutation[node_id];
+    }
+}
+
+inline void renumber(std::vector<extractor::StorageManeuverOverride> &maneuver_overrides,
+                     const std::vector<std::uint32_t> &permutation)
+{
+    for (auto &maneuver_override : maneuver_overrides)
+    {
+        if (maneuver_override.start_node != SPECIAL_NODEID)
+            maneuver_override.start_node = permutation[maneuver_override.start_node];
+    }
+}
+
 } // namespace partitioner
 } // namespace osrm

@@ -2,6 +2,8 @@
 #define DYNAMICGRAPH_HPP

 #include "util/deallocating_vector.hpp"
+#include "util/exception.hpp"
+#include "util/exception_utils.hpp"
 #include "util/integer_range.hpp"
 #include "util/permutation.hpp"
 #include "util/typedefs.hpp"
@@ -424,7 +426,7 @@ template <typename EdgeDataT> class DynamicGraph
        util::inplacePermutation(node_array.begin(), node_array.end(), old_to_new_node);

        // Build up edge permutation
-        auto new_edge_index = 0;
+        EdgeID new_edge_index = 0;
        std::vector<EdgeID> old_to_new_edge(edge_list.size(), SPECIAL_EDGEID);
        for (auto node : util::irange<NodeID>(0, number_of_nodes))
        {
@@ -432,6 +434,11 @@ template <typename EdgeDataT> class DynamicGraph
            // move all filled edges
            for (auto edge : GetAdjacentEdgeRange(node))
            {
+                if (new_edge_index == std::numeric_limits<EdgeID>::max())
+                {
+                    throw util::exception("There are too many edges, OSRM only supports 2^32" +
+                                          SOURCE_REF);
+                }
                edge_list[edge].target = old_to_new_node[edge_list[edge].target];
                BOOST_ASSERT(edge_list[edge].target != SPECIAL_NODEID);
                old_to_new_edge[edge] = new_edge_index++;
@@ -1,6 +1,6 @@
 {
  "name": "osrm",
-  "version": "5.16.0-rc.6",
+  "version": "5.16.6",
  "private": false,
  "description": "The Open Source Routing Machine is a high performance routing engine written in C++14 designed to run on OpenStreetMap data.",
  "dependencies": {
@@ -370,7 +370,7 @@ void trimShortSegments(std::vector<RouteStep> &steps, LegGeometry &geometry)
        geometry.segment_offsets.pop_back();
        // remove all the last coordinates from the geometry
        geometry.locations.resize(geometry.segment_offsets.back() + 1);
-        geometry.annotations.resize(geometry.segment_offsets.back() + 1);
+        geometry.annotations.resize(geometry.segment_offsets.back());
        geometry.osm_node_ids.resize(geometry.segment_offsets.back() + 1);

        BOOST_ASSERT(geometry.segment_distances.back() <= 1);
@@ -429,6 +429,10 @@ void trimShortSegments(std::vector<RouteStep> &steps, LegGeometry &geometry)
        last_step.intersections.front().bearings.front() = util::bearing::reverse(bearing);
    }

+    BOOST_ASSERT(geometry.segment_offsets.back() + 1 == geometry.locations.size());
+    BOOST_ASSERT(geometry.segment_offsets.back() + 1 == geometry.osm_node_ids.size());
+    BOOST_ASSERT(geometry.segment_offsets.back() == geometry.annotations.size());
+
    BOOST_ASSERT(steps.back().geometry_end == geometry.locations.size());

    BOOST_ASSERT(steps.front().intersections.size() >= 1);
@@ -44,7 +44,7 @@ void filterCandidates(const std::vector<util::Coordinate> &coordinates,
                                                           coordinates[current_coordinate + 1]);

            // sharp turns indicate a possible uturn
-            if (turn_angle <= 90.0 || turn_angle >= 270.0)
+            if (turn_angle <= 45.0 || turn_angle >= 315.0)
            {
                allow_uturn = true;
            }
@@ -1106,11 +1106,9 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges(
        std::sort(storage_maneuver_overrides.begin(),
                  storage_maneuver_overrides.end(),
                  [](const auto &a, const auto &b) { return a.start_node < b.start_node; });
-        // write conditional turn penalties into the restrictions file
-        storage::io::FileWriter writer(maneuver_overrides_filename,
-                                       storage::io::FileWriter::GenerateFingerprint);
-        extractor::serialization::write(
-            writer, storage_maneuver_overrides, maneuver_override_sequences);
+
+        files::writeManeuverOverrides(
+            maneuver_overrides_filename, storage_maneuver_overrides, maneuver_override_sequences);
    }

    util::Log() << "done.";
@@ -145,6 +145,15 @@ int Partitioner::Run(const PartitionerConfig &config)
        renumber(node_data, permutation);
        extractor::files::writeNodeData(config.GetPath(".osrm.ebg_nodes"), node_data);
    }
+    {
+        const auto &filename = config.GetPath(".osrm.maneuver_overrides");
+        std::vector<extractor::StorageManeuverOverride> maneuver_overrides;
+        std::vector<NodeID> node_sequences;
+        extractor::files::readManeuverOverrides(filename, maneuver_overrides, node_sequences);
+        renumber(maneuver_overrides, permutation);
+        renumber(node_sequences, permutation);
+        extractor::files::writeManeuverOverrides(filename, maneuver_overrides, node_sequences);
+    }
    if (boost::filesystem::exists(config.GetPath(".osrm.hsgr")))
    {
        util::Log(logWARNING) << "Found existing .osrm.hsgr file, removing. You need to re-run "
@@ -445,10 +445,11 @@ void Storage::PopulateLayout(DataLayout &layout)
    {
        io::FileReader maneuver_overrides_file(config.GetPath(".osrm.maneuver_overrides"),
                                               io::FileReader::VerifyFingerprint);
-        const auto number_of_overrides = maneuver_overrides_file.ReadElementCount64();
+        const auto number_of_overrides =
+            maneuver_overrides_file.ReadVectorSize<extractor::StorageManeuverOverride>();
        layout.SetBlockSize<extractor::StorageManeuverOverride>(DataLayout::MANEUVER_OVERRIDES,
                                                                number_of_overrides);
-        const auto number_of_nodes = maneuver_overrides_file.ReadElementCount64();
+        const auto number_of_nodes = maneuver_overrides_file.ReadVectorSize<NodeID>();
        layout.SetBlockSize<NodeID>(DataLayout::MANEUVER_OVERRIDE_NODE_SEQUENCES, number_of_nodes);
    }

@@ -1089,18 +1090,21 @@ void Storage::PopulateData(const DataLayout &layout, char *memory_ptr)

        // load maneuver overrides
        {
-            io::FileReader maneuver_overrides_file(config.GetPath(".osrm.maneuver_overrides"),
-                                                   io::FileReader::VerifyFingerprint);
-            const auto number_of_overrides = maneuver_overrides_file.ReadElementCount64();
-            const auto number_of_nodes = maneuver_overrides_file.ReadElementCount64();
            const auto maneuver_overrides_ptr =
                layout.GetBlockPtr<extractor::StorageManeuverOverride, true>(
                    memory_ptr, DataLayout::MANEUVER_OVERRIDES);
-            maneuver_overrides_file.ReadInto(maneuver_overrides_ptr, number_of_overrides);
-
            const auto maneuver_override_node_sequences_ptr = layout.GetBlockPtr<NodeID, true>(
                memory_ptr, DataLayout::MANEUVER_OVERRIDE_NODE_SEQUENCES);
-            maneuver_overrides_file.ReadInto(maneuver_override_node_sequences_ptr, number_of_nodes);
+
+            util::vector_view<extractor::StorageManeuverOverride> maneuver_overrides(
+                maneuver_overrides_ptr, layout.num_entries[DataLayout::MANEUVER_OVERRIDES]);
+            util::vector_view<NodeID> maneuver_override_node_sequences(
+                maneuver_override_node_sequences_ptr,
+                layout.num_entries[DataLayout::MANEUVER_OVERRIDE_NODE_SEQUENCES]);
+
+            extractor::files::readManeuverOverrides(config.GetPath(".osrm.maneuver_overrides"),
+                                                    maneuver_overrides,
+                                                    maneuver_override_node_sequences);
        }
    }
 }
@@ -38,6 +38,10 @@ BOOST_AUTO_TEST_CASE(unchanged_collapse_route_result)

 BOOST_AUTO_TEST_CASE(two_legs_to_one_leg)
 {
+    // from_edge_based_node, turn_via_node, name_id, is_segregated, weight_until_turn,
+    // weight_of_turn,
+    // duration_until_turn, duration_of_turn, turn_instruction, lane_data, travel_mode, classes,
+    // entry_class, datasource_id, pre_turn_bearing, post_turn_bearing, left_hand
    PathData pathy{0, 2, 17, false, 2, 3, 4, 5, 0, {}, 4, 2, {}, 2, {1.0}, {1.0}, false};
    PathData kathy{0, 1, 16, false, 1, 2, 3, 4, 1, {}, 3, 1, {}, 1, {2.0}, {3.0}, false};
    PathData cathy{0, 3, 16, false, 1, 2, 3, 4, 1, {}, 3, 1, {}, 1, {2.0}, {3.0}, false};
@@ -61,10 +65,11 @@ BOOST_AUTO_TEST_CASE(two_legs_to_one_leg)
    BOOST_CHECK_EQUAL(collapsed.segment_end_coordinates[0].target_phantom.forward_segment_id.id,
                      12);
    BOOST_CHECK_EQUAL(collapsed.segment_end_coordinates[0].source_phantom.forward_segment_id.id, 1);
-    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[0].size(), 3);
+    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[0].size(), 4);
    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[0][0].turn_via_node, 2);
    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[0][1].turn_via_node, 1);
-    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[0][2].turn_via_node, 3);
+    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[0][2].turn_via_node, 1);
+    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[0][3].turn_via_node, 3);
 }

 BOOST_AUTO_TEST_CASE(three_legs_to_two_legs)
@@ -101,13 +106,14 @@ BOOST_AUTO_TEST_CASE(three_legs_to_two_legs)
    BOOST_CHECK_EQUAL(collapsed.segment_end_coordinates[1].target_phantom.forward_segment_id.id,
                      18);
    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[0].size(), 2);
-    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[1].size(), 4);
+    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[1].size(), 5);
    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[0][0].turn_via_node, 2);
    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[0][1].turn_via_node, 1);
    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[1][0].turn_via_node, 1);
    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[1][1].turn_via_node, 5);
    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[1][2].turn_via_node, 3);
-    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[1][3].turn_via_node, 4);
+    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[1][3].turn_via_node, 3);
+    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[1][4].turn_via_node, 4);
 }

 BOOST_AUTO_TEST_CASE(two_legs_to_two_legs)
@@ -407,12 +407,20 @@ BOOST_AUTO_TEST_CASE(speed_annotation_matches_duration_and_distance)
    const auto &durations = annotation.values.at("duration").get<json::Array>().values;
    const auto &distances = annotation.values.at("distance").get<json::Array>().values;
    int length = speeds.size();
+
+    BOOST_CHECK_EQUAL(length, 1);
    for (int i = 0; i < length; i++)
    {
        auto speed = speeds[i].get<json::Number>().value;
        auto duration = durations[i].get<json::Number>().value;
        auto distance = distances[i].get<json::Number>().value;
-        BOOST_CHECK_EQUAL(speed, std::round(distance / duration * 10.) / 10.);
+        auto calc = std::round(distance / duration * 10.) / 10.;
+        BOOST_CHECK_EQUAL(speed, std::isnan(calc) ? 0 : calc);
+
+        // Because we route from/to the same location, all annotations should be 0;
+        BOOST_CHECK_EQUAL(speed, 0);
+        BOOST_CHECK_EQUAL(distance, 0);
+        BOOST_CHECK_EQUAL(duration, 0);
    }
 }
Author	SHA1	Message	Date
Michael Krasnyk	ba095c8566	Avoid using signed integers for edge IDs	2018-04-12 10:10:37 +02:00
Patrick Niklaus	c1fd02bb8d	Bump OSRM version to 5.16.5	2018-04-09 11:10:52 +00:00
Patrick Niklaus	54d02fac15	Simplify toEdges and make it more robust against accidental memory allocations	2018-04-09 11:09:00 +00:00
Daniel Patterson	0a1418c766	Bump version to 5.16.4	2018-03-13 11:35:42 -07:00
Daniel Patterson	1d367ccc0d	Add source phantom weight to first segment when merging legs (#4949 ) Fix annotation values for annotations on edges where phantom nodes are snapped.	2018-03-13 11:34:22 -07:00
Patrick Niklaus	860c452b83	Bump version to 5.16.3	2018-03-09 16:02:11 +01:00
Michael Krasnyk	3b096963a0	Remove correctly the last segment in annotation	2018-03-09 16:02:11 +01:00
Daniel Patterson	7154ed9d59	Avoid copying json::Value objects a lot when reallocating vector	2018-03-08 15:10:07 +00:00
Michael Krasnyk	8f633e61bf	Bump version 5.16.2	2018-02-28 15:54:44 +01:00
karenzshea	86dd319d29	avoid testing self-intersecting way	2018-02-28 15:52:56 +01:00
Michael Krasnyk	89d32eca94	Use smaller range for U-turn angles in map-matching	2018-02-28 15:52:45 +01:00
Karen Shea	f5120d1cec	Remove deduplication of unpacked_path_segments in MM collapsing (#4911 )	2018-02-24 21:37:36 -08:00
Huyen Chau Nguyen	d30d28cf4a	final 5.16 version	2018-02-23 13:23:34 +01:00
Michael Krasnyk	87de37168b	Bump RC7	2018-02-22 11:44:01 +01:00
Michael Krasnyk	658b8ef738	Renumber node IDs in .osrm.maneuver_overrides	2018-02-22 11:43:34 +01:00