Fix tests - backported changes need old namespace.

Conflicts:
	features/guidance/dedicated-turn-roads.feature
	include/extractor/guidance/intersection.hpp

CHANGELOG.md

@@ -1,3 +1,12 @@
+# 5.5.4
+  - Changes from 5.5.3
+    - Bugfixes
+      - PR #3561 - added missing backwards speeds for cycleways in bicycle profile
+      - PR #3515 - adjusted number of `nodes` in `annotation`
+      - Ticket #3430 - Fix possible division by zero by clamping latitude to +/- 85.05
+      - PR #3555 - Fix an error that occurs when a road forks immediately after exiting a ferry
+      - PR #3575 - Don't emit invalid turn types for obvious turns at sliproads and roundabouts.
+
 # 5.5.3
   - Changes from 5.5.2
     - Bugfixes:
@@ -396,5 +405,3 @@
         - `properties.use_turn_restrictions`
         - `properties.u_turn_penalty`
         - `properties.allow_u_turn_at_via`
-
-

CMakeLists.txt

@@ -53,7 +53,7 @@ endif()
 project(OSRM C CXX)
 set(OSRM_VERSION_MAJOR 5)
 set(OSRM_VERSION_MINOR 5)
-set(OSRM_VERSION_PATCH 3)
+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/guidance/dedicated-turn-roads.feature

@@ -782,3 +782,40 @@ Feature: Slipways and Dedicated Turn Lanes
        When I route I should get
             | waypoints | route                         | turns                              |
             | s,g       | main,sliproad,another,another | depart,turn right,turn left,arrive |
+
+    Scenario: Sliproad before a roundabout
+        Given the node map
+            """
+                      e
+            a - b - - c - d
+                    'f|l'
+                      m
+                      g
+                      |
+                     .h-_
+                k - i    |
+                     '.j.'
+
+            """
+
+        And the ways
+            | nodes | junction   | oneway | highway   | name |
+            | ab    |            | yes    | primary   | road |
+            | bc    |            | yes    | primary   | road |
+            | cd    |            | yes    | primary   | road |
+            | ec    |            | yes    | secondary |      |
+            | cm    |            | yes    | secondary |      |
+            | mg    |            | yes    | primary   |      |
+            | gh    |            | no     | primary   |      |
+            | hijh  | roundabout | yes    | primary   |      |
+            | ik    |            | yes    | primary   |      |
+            | bfm   |            | yes    | primary   |      |
+            | gld   |            | yes    | primary   |      |
+
+        And the relations
+            | type        | way:from | way:to | node:via | restriction   |
+            | restriction | bc       | cd     | c        | only_straight |
+
+        When I route I should get
+            | waypoints | route     | turns                                                     | locations |
+            | a,k       | road,,,   | depart,continue right,roundabout turn right exit-1,arrive | a,b,h,k   |

include/extractor/guidance/intersection.hpp

@@ -34,6 +34,14 @@ struct IntersectionShapeData
 };
 
 inline auto makeCompareShapeDataByBearing(const double base_bearing)
+{
+    return [base_bearing](const auto &lhs, const auto &rhs) {
+        return util::angularDeviation(base_bearing, lhs.bearing) <
+               util::angularDeviation(base_bearing, rhs.bearing);
+    };
+}
+
+inline auto makeCompareShapeDataAngleToBearing(const double base_bearing)
 {
     return [base_bearing](const auto &lhs, const auto &rhs) {
         return util::angleBetweenBearings(base_bearing, lhs.bearing) <

include/util/debug.hpp

@@ -29,6 +29,7 @@ inline void print(const engine::guidance::RouteStep &step)
               << " "
               << " Duration: " << step.duration << " Distance: " << step.distance
               << " Geometry: " << step.geometry_begin << " " << step.geometry_end
+              << " Exit: " << step.maneuver.exit << " Mode: " << (int)step.mode
               << "\n\tIntersections: " << step.intersections.size() << " [";
 
     for (const auto &intersection : step.intersections)

include/util/web_mercator.hpp

@@ -20,7 +20,7 @@ const constexpr double EARTH_RADIUS_WGS84 = 6378137.0;
 // earth circumference devided by 2
 const constexpr double MAXEXTENT = EARTH_RADIUS_WGS84 * boost::math::constants::pi<double>();
 // ^ math functions are not constexpr since they have side-effects (setting errno) :(
-const constexpr double MAX_LATITUDE = 85.;
+const constexpr double EPSG3857_MAX_LATITUDE = 85.051128779806592378; // 90(4*atan(exp(pi))/pi-1)
 const constexpr double MAX_LONGITUDE = 180.0;
 }
 
@@ -29,6 +29,18 @@ const constexpr double DEGREE_TO_PX = detail::MAXEXTENT / 180.0;
 // This is the global default tile size for all Mapbox Vector Tiles
 const constexpr double TILE_SIZE = 256.0;
 
+inline FloatLatitude clamp(const FloatLatitude lat)
+{
+    return std::max(std::min(lat, FloatLatitude{detail::EPSG3857_MAX_LATITUDE}),
+                    FloatLatitude{-detail::EPSG3857_MAX_LATITUDE});
+}
+
+inline FloatLongitude clamp(const FloatLongitude lon)
+{
+    return std::max(std::min(lon, FloatLongitude{detail::MAX_LONGITUDE}),
+                    FloatLongitude{-detail::MAX_LONGITUDE});
+}
+
 inline FloatLatitude yToLat(const double y)
 {
     const auto clamped_y = std::max(-180., std::min(180., y));
@@ -41,10 +53,9 @@ inline FloatLatitude yToLat(const double y)
 inline double latToY(const FloatLatitude latitude)
 {
     // apparently this is the (faster) version of the canonical log(tan()) version
-    const double f = std::sin(detail::DEGREE_TO_RAD * static_cast<double>(latitude));
-    const double y = detail::RAD_TO_DEGREE * 0.5 * std::log((1 + f) / (1 - f));
-    const auto clamped_y = std::max(-180., std::min(180., y));
-    return clamped_y;
+    const auto clamped_latitude = clamp(latitude);
+    const double f = std::sin(detail::DEGREE_TO_RAD * static_cast<double>(clamped_latitude));
+    return detail::RAD_TO_DEGREE * 0.5 * std::log((1 + f) / (1 - f));
 }
 
 template <typename T> constexpr double horner(double, T an) { return an; }
@@ -91,18 +102,6 @@ inline double latToYapprox(const FloatLatitude latitude)
                   -3.23083224835967391884404730e-28);
 }
 
-inline FloatLatitude clamp(const FloatLatitude lat)
-{
-    return std::max(std::min(lat, FloatLatitude{detail::MAX_LATITUDE}),
-                    FloatLatitude{-detail::MAX_LATITUDE});
-}
-
-inline FloatLongitude clamp(const FloatLongitude lon)
-{
-    return std::max(std::min(lon, FloatLongitude{detail::MAX_LONGITUDE}),
-                    FloatLongitude{-detail::MAX_LONGITUDE});
-}
-
 inline void pixelToDegree(const double shift, double &x, double &y)
 {
     const double b = shift / 2.0;
@@ -166,9 +165,9 @@ inline void xyzToMercator(
     xyzToWGS84(x, y, z, minx, miny, maxx, maxy);
 
     minx = static_cast<double>(clamp(util::FloatLongitude{minx})) * DEGREE_TO_PX;
-    miny = latToY(clamp(util::FloatLatitude{miny})) * DEGREE_TO_PX;
+    miny = latToY(util::FloatLatitude{miny}) * DEGREE_TO_PX;
     maxx = static_cast<double>(clamp(util::FloatLongitude{maxx})) * DEGREE_TO_PX;
-    maxy = latToY(clamp(util::FloatLatitude{maxy})) * DEGREE_TO_PX;
+    maxy = latToY(util::FloatLatitude{maxy}) * DEGREE_TO_PX;
 }
 }
 }

profiles/bicycle.lua

@@ -368,10 +368,13 @@ function way_function (way, result)
   -- cycleways
   if cycleway and cycleway_tags[cycleway] then
     result.forward_speed = bicycle_speeds["cycleway"]
+    result.backward_speed = bicycle_speeds["cycleway"]
   elseif cycleway_left and cycleway_tags[cycleway_left] then
     result.forward_speed = bicycle_speeds["cycleway"]
+    result.backward_speed = bicycle_speeds["cycleway"]
   elseif cycleway_right and cycleway_tags[cycleway_right] then
     result.forward_speed = bicycle_speeds["cycleway"]
+    result.backward_speed = bicycle_speeds["cycleway"]
   end
 
   -- dismount

src/engine/guidance/post_processing.cpp

@@ -1073,7 +1073,8 @@ std::vector<RouteStep> collapseTurns(std::vector<RouteStep> steps)
         if (one_back_step.maneuver.instruction.type == TurnType::Sliproad)
         {
             if (current_step.maneuver.instruction.type == TurnType::Suppressed &&
-                compatible(one_back_step, current_step))
+                compatible(one_back_step, current_step) && current_step.intersections.size() == 1 &&
+                current_step.intersections.front().entry.size() == 2)
             {
                 // Traffic light on the sliproad, the road itself will be handled in the next
                 // iteration, when one-back-index again points to the sliproad.
@@ -1441,6 +1442,7 @@ void trimShortSegments(std::vector<RouteStep> &steps, LegGeometry &geometry)
         // This can happen if the last coordinate snaps to a node in the unpacked geometry
         geometry.locations.pop_back();
         geometry.annotations.pop_back();
+        geometry.osm_node_ids.pop_back();
         geometry.segment_offsets.back()--;
         // since the last geometry includes the location of arrival, the arrival instruction
         // geometry overlaps with the previous segment

src/extractor/guidance/intersection_generator.cpp

@@ -135,8 +135,9 @@ IntersectionGenerator::ComputeIntersectionShape(const NodeID node_at_center_of_i
             }
             return util::reverseBearing(intersection.begin()->bearing);
         }();
-        std::sort(
-            intersection.begin(), intersection.end(), makeCompareShapeDataByBearing(base_bearing));
+        std::sort(intersection.begin(),
+                  intersection.end(),
+                  makeCompareShapeDataAngleToBearing(base_bearing));
     }
     return intersection;
 }

src/extractor/guidance/intersection_handler.cpp

@@ -167,6 +167,14 @@ void IntersectionHandler::assignFork(const EdgeID via_edge,
         node_based_graph.GetEdgeData(left.eid).road_classification.IsLowPriorityRoadClass();
     const bool low_priority_right =
         node_based_graph.GetEdgeData(right.eid).road_classification.IsLowPriorityRoadClass();
+    const auto same_mode_left =
+        in_data.travel_mode == node_based_graph.GetEdgeData(left.eid).travel_mode;
+    const auto same_mode_right =
+        in_data.travel_mode == node_based_graph.GetEdgeData(right.eid).travel_mode;
+    const auto suppressed_left_type =
+        same_mode_left ? TurnType::Suppressed : TurnType::Notification;
+    const auto suppressed_right_type =
+        same_mode_right ? TurnType::Suppressed : TurnType::Notification;
     if ((angularDeviation(left.angle, STRAIGHT_ANGLE) < MAXIMAL_ALLOWED_NO_TURN_DEVIATION &&
          angularDeviation(right.angle, STRAIGHT_ANGLE) > FUZZY_ANGLE_DIFFERENCE))
     {
@@ -198,7 +206,7 @@ void IntersectionHandler::assignFork(const EdgeID via_edge,
         }
         else
         {
-            left.instruction = {TurnType::Suppressed, DirectionModifier::Straight};
+            left.instruction = {suppressed_left_type, DirectionModifier::Straight};
             right.instruction = {findBasicTurnType(via_edge, right),
                                  DirectionModifier::SlightRight};
         }
@@ -237,7 +245,7 @@ void IntersectionHandler::assignFork(const EdgeID via_edge,
             }
             else
             {
-                right.instruction = {TurnType::Suppressed, DirectionModifier::Straight};
+                right.instruction = {suppressed_right_type, DirectionModifier::Straight};
                 left.instruction = {findBasicTurnType(via_edge, left),
                                     DirectionModifier::SlightLeft};
             }
@@ -245,7 +253,7 @@ void IntersectionHandler::assignFork(const EdgeID via_edge,
     }
     // left side of fork
     if (low_priority_right && !low_priority_left)
-        left.instruction = {TurnType::Suppressed, DirectionModifier::SlightLeft};
+        left.instruction = {suppressed_left_type, DirectionModifier::SlightLeft};
     else
     {
         if (low_priority_left && !low_priority_right)
@@ -256,7 +264,7 @@ void IntersectionHandler::assignFork(const EdgeID via_edge,
 
     // right side of fork
     if (low_priority_left && !low_priority_right)
-        right.instruction = {TurnType::Suppressed, DirectionModifier::SlightLeft};
+        right.instruction = {suppressed_right_type, DirectionModifier::SlightLeft};
     else
     {
         if (low_priority_right && !low_priority_left)
@@ -272,6 +280,12 @@ void IntersectionHandler::assignFork(const EdgeID via_edge,
                                      ConnectedRoad &right) const
 {
     // TODO handle low priority road classes in a reasonable way
+    const auto suppressed_type = [&](const ConnectedRoad &road) {
+        const auto in_mode = node_based_graph.GetEdgeData(via_edge).travel_mode;
+        const auto out_mode = node_based_graph.GetEdgeData(road.eid).travel_mode;
+        return in_mode == out_mode ? TurnType::Suppressed : TurnType::Notification;
+    };
+
     if (left.entry_allowed && center.entry_allowed && right.entry_allowed)
     {
         left.instruction = {TurnType::Fork, DirectionModifier::SlightLeft};
@@ -285,7 +299,7 @@ void IntersectionHandler::assignFork(const EdgeID via_edge,
             }
             else
             {
-                center.instruction = {TurnType::Suppressed, DirectionModifier::Straight};
+                center.instruction = {suppressed_type(center), DirectionModifier::Straight};
             }
         }
         else

src/extractor/guidance/turn_handler.cpp

@@ -566,6 +566,12 @@ std::pair<std::size_t, std::size_t> TurnHandler::findFork(const EdgeID via_edge,
             return true;
         }();
 
+        const auto has_compatible_modes = std::all_of(
+            intersection.begin() + right, intersection.begin() + left + 1, [&](const auto &road) {
+                return node_based_graph.GetEdgeData(road.eid).travel_mode ==
+                       node_based_graph.GetEdgeData(via_edge).travel_mode;
+            });
+
         // check if all entries in the fork range allow entry
         const bool only_valid_entries = [&]() {
             BOOST_ASSERT(right <= left && left < intersection.size());
@@ -585,7 +591,8 @@ std::pair<std::size_t, std::size_t> TurnHandler::findFork(const EdgeID via_edge,
 
         // TODO check whether 2*NARROW_TURN is too large
         if (valid_indices && separated_at_left_side && separated_at_right_side &&
-            not_more_than_three && !has_obvious && has_compatible_classes && only_valid_entries)
+            not_more_than_three && !has_obvious && has_compatible_classes && only_valid_entries &&
+            has_compatible_modes)
             return std::make_pair(right, left);
     }
     return std::make_pair(std::size_t{0}, std::size_t{0});

unit_tests/engine/guidance_assembly.cpp

@@ -3,6 +3,7 @@
 #include "engine/guidance/assemble_overview.hpp"
 #include "engine/guidance/assemble_route.hpp"
 #include "engine/guidance/assemble_steps.hpp"
+#include "engine/guidance/post_processing.hpp"
 
 #include <boost/test/test_case_template.hpp>
 #include <boost/test/unit_test.hpp>
@@ -17,4 +18,85 @@ BOOST_AUTO_TEST_CASE(rfc4648_test_vectors)
     // TODO(daniel-j-h):
 }
 
+BOOST_AUTO_TEST_CASE(trim_short_segments)
+{
+    using namespace osrm::extractor::guidance;
+    using namespace osrm::engine::guidance;
+    using namespace osrm::engine;
+    using namespace osrm::util;
+
+    Intersection intersection1{{FloatLongitude{-73.981154}, FloatLatitude{40.767762}},
+                                           {302},
+                                           {1},
+                                           Intersection::NO_INDEX,
+                                           0,
+                                           {0, 255},
+                                           {}};
+    Intersection intersection2{{FloatLongitude{-73.981495}, FloatLatitude{40.768275}},
+                                           {180},
+                                           {1},
+                                           0,
+                                           Intersection::NO_INDEX,
+                                           {0, 255},
+                                           {}};
+
+    // Check that duplicated coordinate in the end is removed
+    std::vector<RouteStep> steps = {{324,
+                                     "Central Park West",
+                                     "",
+                                     "",
+                                     "",
+                                     "",
+                                     "",
+                                     0.2,
+                                     1.9076601161280742,
+                                     TRAVEL_MODE_DRIVING,
+                                     {{FloatLongitude{-73.981492}, FloatLatitude{40.768258}},
+                                      329,
+                                      348,
+                                      {TurnType::ExitRotary, DirectionModifier::Straight},
+                                      WaypointType::Depart,
+                                      0},
+                                     0,
+                                     3,
+                                     {intersection1}},
+                                    {324,
+                                     "Central Park West",
+                                     "",
+                                     "",
+                                     "",
+                                     "",
+                                     "",
+                                     0,
+                                     0,
+                                     TRAVEL_MODE_DRIVING,
+                                     {{FloatLongitude{-73.981495}, FloatLatitude{40.768275}},
+                                      0,
+                                      0,
+                                      {TurnType::NoTurn, DirectionModifier::UTurn},
+                                      WaypointType::Arrive,
+                                      0},
+                                     2,
+                                     3,
+                                     {intersection2}}};
+
+    LegGeometry geometry;
+    geometry.locations = {{FloatLongitude{-73.981492}, FloatLatitude{40.768258}},
+                          {FloatLongitude{-73.981495}, FloatLatitude{40.768275}},
+                          {FloatLongitude{-73.981495}, FloatLatitude{40.768275}}};
+    geometry.segment_offsets = {0, 2};
+    geometry.segment_distances = {1.9076601161280742};
+    geometry.osm_node_ids = {OSMNodeID{0}, OSMNodeID{1}, OSMNodeID{2}};
+    geometry.annotations = {{1.9076601161280742, 0.2, 0}, {0, 0, 0}};
+
+    trimShortSegments(steps, geometry);
+
+    BOOST_CHECK_EQUAL(geometry.segment_distances.size(), 1);
+    BOOST_CHECK_EQUAL(geometry.segment_offsets.size(), 2);
+    BOOST_CHECK_EQUAL(geometry.segment_offsets.back(), 1);
+    BOOST_CHECK_EQUAL(geometry.annotations.size(), 1);
+    BOOST_CHECK_EQUAL(geometry.locations.size(), 2);
+    BOOST_CHECK_EQUAL(geometry.osm_node_ids.size(), 2);
+}
+
 BOOST_AUTO_TEST_SUITE_END()

unit_tests/util/coordinate_calculation.cpp

@@ -117,8 +117,7 @@ BOOST_AUTO_TEST_CASE(compute_angle)
     BOOST_CHECK_EQUAL(angle, 180);
 
     // Tiny changes below our calculation resolution
-    // This should be equivalent to having two points on the same
-    // spot.
+    // This should be equivalent to having two points on the same spot.
     first = Coordinate{FloatLongitude{0}, FloatLatitude{0}};
     middle = Coordinate{FloatLongitude{1}, FloatLatitude{0}};
     end = Coordinate{FloatLongitude{1 + std::numeric_limits<double>::epsilon()}, FloatLatitude{0}};
@@ -126,14 +125,12 @@ BOOST_AUTO_TEST_CASE(compute_angle)
     BOOST_CHECK_EQUAL(angle, 180);
 
     // Invalid values
-    /* TODO: Enable this when I figure out how to use BOOST_CHECK_THROW
-     *       and not have the whole test case fail...
-    first = Coordinate(FloatLongitude{0}, FloatLatitude{0});
-    middle = Coordinate(FloatLongitude{1}, FloatLatitude{0});
-    end = Coordinate(FloatLongitude(std::numeric_limits<double>::max()), FloatLatitude{0});
-    BOOST_CHECK_THROW( coordinate_calculation::computeAngle(first,middle,end),
-                       boost::numeric::positive_overflow);
-                       */
+    BOOST_CHECK_THROW(
+        coordinate_calculation::computeAngle(
+            Coordinate(FloatLongitude{0}, FloatLatitude{0}),
+            Coordinate(FloatLongitude{1}, FloatLatitude{0}),
+            Coordinate(FloatLongitude{std::numeric_limits<double>::max()}, FloatLatitude{0})),
+        boost::numeric::positive_overflow);
 }
 
 // Regression test for bug captured in #1347

unit_tests/util/web_mercator.cpp

@@ -73,12 +73,14 @@ BOOST_AUTO_TEST_CASE(xyz_to_mercator)
     double miny;
     double maxx;
     double maxy;
+
+    // http://tools.geofabrik.de/map/#13/85.0500/-175.5876&type=Geofabrik_Standard&grid=1
     web_mercator::xyzToMercator(100, 0, 13, minx, miny, maxx, maxy);
 
     BOOST_CHECK_CLOSE(minx, -19548311.361764118075, 0.0001);
-    BOOST_CHECK_CLOSE(miny, 19971868.8804085782, 0.0001);
+    BOOST_CHECK_CLOSE(miny, 20032616.372979045, 0.0001);
     BOOST_CHECK_CLOSE(maxx, -19543419.391953866929, 0.0001);
-    BOOST_CHECK_CLOSE(maxy, 19971868.880408578, 0.0001);
+    BOOST_CHECK_CLOSE(maxy, 20037508.342789277, 0.0001); // Mercator 6378137*pi, WGS 85.0511
 }
 
 BOOST_AUTO_TEST_SUITE_END()