Bump version to 5.16.4

Fix annotation values for annotations on edges where phantom nodes are snapped.

CHANGELOG.md

@@ -1,3 +1,19 @@
+# 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

CMakeLists.txt

@@ -61,7 +61,7 @@ endif()
 project(OSRM C CXX)
 set(OSRM_VERSION_MAJOR 5)
 set(OSRM_VERSION_MINOR 16)
-set(OSRM_VERSION_PATCH 1)
+set(OSRM_VERSION_PATCH 4)
 set(OSRM_VERSION "${OSRM_VERSION_MAJOR}.${OSRM_VERSION_MINOR}.${OSRM_VERSION_PATCH}")
 
 add_definitions(-DOSRM_PROJECT_DIR="${CMAKE_CURRENT_SOURCE_DIR}")

features/step_definitions/matching.js

@@ -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')) {

features/testbot/annotations.feature

@@ -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 |
+            | from | to | route    | a:speed     | a:weight | a:nodes |
-            | h    | j  | hk,jk,jk | 6.7:6.7     | 15:15    |
+            | 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    |
+            | 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 |
+            | 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 |
+            | 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   |
+            | a    | c  | abc,abc  | 10:10   | 249.998641:299.931643 | 25:30      | 1:2:3   |

features/testbot/matching.feature

@@ -684,3 +684,112 @@ Feature: Basic Map Matching
         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      |

features/testbot/traffic_speeds.feature

@@ -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           |
+          | b    | c  | bc,bc       | 27 km/h | 741.5,0              | 1             |
-          | a    | d  | ad,ad       | 27 km/h | 1275.7,0             | 1:0           |
+          | 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           |
+          | b    | c  | bc,bc       | 27 km/h | 741.5,0              | 1             |
-          | a    | d  | ad,ad       | 27 km/h | 1275.7,0             | 1:0           |
+          | 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    | 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:0         |
+          | 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:0           |
+          | 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 |
+          | from | to | route    | speed   | weights       | a:datasources | a:speed | a:nodes|
-          | a    | b  | fb,fb    | 36 km/h | 329.4,0       | 0             |
+          | 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:0         |
+          | 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:0           |
+          | 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:0         |
+          | 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             |
+          | 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             |
+          | 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             |
+          | 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

features/testbot/weight.feature

@@ -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     |
+            | 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           | 3.1        | 3.1      | 6.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  |
 

include/engine/api/route_api.hpp

@@ -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(),

include/engine/guidance/assemble_geometry.hpp

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

include/engine/internal_route_result.hpp

@@ -145,11 +145,32 @@ inline InternalRouteResult CollapseInternalRouteResult(const InternalRouteResult
             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(),
+            if (!leggy_result.unpacked_path_segments[i].empty())
-                                leggy_result.unpacked_path_segments[i].begin(),
+            {
-                                leggy_result.unpacked_path_segments[i].end());
+                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;
 }
 }

package.json

@@ -1,6 +1,6 @@
 {
   "name": "osrm",
-  "version": "5.16.1",
+  "version": "5.16.4",
   "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": {

src/engine/guidance/post_processing.cpp

@@ -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);

src/engine/plugins/match.cpp

@@ -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;
             }

unit_tests/library/route.cpp

@@ -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);
     }
 }