Bump RC7

passing some ramps or mering onto another motorway

CHANGELOG.md

@@ -1,10 +1,13 @@
-# 5.16.0 RC 5
+# 5.16.0 RC 7
   - Changes from 5.15.1:
     - 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
       - CHANGED #4835: MAXIMAL_ALLOWED_SEPARATION_WIDTH increased to 12 meters
       - CHANGED #4842: Lower priority links from a motorway now are used as motorway links [#4842](https://github.com/Project-OSRM/osrm-backend/pull/4842)
+      - CHANGED #4895: Use ramp bifurcations as fork intersections [#4895](https://github.com/Project-OSRM/osrm-backend/issues/4895)
+      - CHANGED #4893: Handle motorway forks with links as normal motorway intersections[#4893](https://github.com/Project-OSRM/osrm-backend/issues/4893)
+      - FIXED #4905: Check required tags of `maneuver` relations [#4905](https://github.com/Project-OSRM/osrm-backend/pull/4905)
     - Profile:
       - FIXED: `highway=service` will now be used for restricted access, `access=private` is still disabled for snapping.
       - ADDED #4775: Exposes more information to the turn function, now being able to set turn weights with highway and access information of the turn as well as other roads at the intersection [#4775](https://github.com/Project-OSRM/osrm-backend/issues/4775)

features/guidance/maneuver-tag.feature

@@ -30,7 +30,7 @@ Feature: Maneuver tag support
 
         When I route I should get
             | waypoints | route                               | turns                                    |
-        # Testing directly connected from/to 
+        # Testing directly connected from/to
             | a,j       | A Street,C Street,J Street,J Street | depart,turn sharp right,turn left,arrive |
             | b,g       | A Street,C Street,C Street          | depart,turn sharp right,arrive           |
         # Testing re-awakening suppressed turns
@@ -198,11 +198,26 @@ Feature: Maneuver tag support
             | pt    | 395     | no     | primary       |
 
         And the relations
-            | type     | way:from | node:via | way:via | way:to | maneuver  |
-            | maneuver | zy       | p        | yp      | pt     | suppress  |
+            | type     | way:from | node:via | way:via | way:to | maneuver | #                                                     |
+            | maneuver | zy       | p        | yp      | pt     | suppress | original: depart,on ramp left,fork slight left,arrive |
+
+        And the relations
+            | type     | way:from | way:via | way:to | maneuver | #                                  |
+            | maneuver | zy       | yp      | pb     | suppress | invalid relation: missing node:via |
+
+        And the relations
+            | type     | node:via | way:via | way:to | maneuver | #                                  |
+            | maneuver | p        | yp      | pb     | suppress | invalid relation: missing way:from |
+
+        And the relations
+            | type     | way:from | node:via | way:via | maneuver | #                                |
+            | maneuver | zy       | p        | yp      | suppress | invalid relation: missing way:to |
+
+        And the relations
+            | type     | way:from | node:via | way:via | way:to | maneuver | #                                   |
+            | maneuver | zy       | y, p     | yp      | pb     | suppress | invalid relation: multiple node:via |
 
         When I route I should get
-            | waypoints | route           | turns                      |
-            | z,t       | NY Ave,395,395  | depart,on ramp left,arrive |
-  #original | z,t       | NY Ave,,395,395 | depart,on ramp left,fork slight left,arrive |
-
+            | waypoints | route                 | turns                                        |
+            | z,t       | NY Ave,395,395        | depart,on ramp left,arrive                   |
+            | z,b       | NY Ave,,4th St,4th St | depart,on ramp left,fork slight right,arrive |

features/guidance/motorway.feature

@@ -299,3 +299,49 @@ Feature: Motorway Guidance
             | waypoints | route         | turns                               |
             | a,e       | abcde,abcde   | depart,arrive                       |
             | a,g       | abcde,bfg,bfg | depart,off ramp slight right,arrive |
+
+
+    # https://www.openstreetmap.org/node/67366428#map=18/33.64613/-84.44425
+    Scenario: Ramp Bifurcations should not be suppressed
+        Given the node map
+            """
+                 /-----------c      /-----------e
+            a---b------------------d------------f
+            """
+
+        And the ways
+            | nodes | highway       | name | destination                                         |
+            | ab    | motorway      |      |                                                     |
+            | bc    | motorway_link |      | City 17                                             |
+            | bd    | motorway_link |      |                                                     |
+            | de    | motorway_link |      | Domestic Terminal;Camp Creek Parkway;Riverdale Road |
+            | df    | motorway_link |      | Montgomery                                          |
+
+
+       When I route I should get
+            | waypoints | route | turns                                            |
+            | a,c       | ,,    | depart,fork slight left,arrive                   |
+            | a,e       | ,,,   | depart,fork slight right,fork slight left,arrive  |
+            | a,f       | ,,,   | depart,fork slight right,fork slight right,arrive |
+
+
+    # https://www.openstreetmap.org/#map=19/53.46186/-2.24509
+    Scenario: Highway Fork with a Link
+        Given the node map
+            """
+                 /-----------d
+            a-b-c------------e
+                 \-----------f
+            """
+
+        And the ways
+            | nodes | highway       |
+            | abce  | motorway      |
+            | cf    | motorway      |
+            | cd    | motorway_link |
+
+       When I route I should get
+            | waypoints | route      | turns                              |
+            | a,d       | abce,cd,cd | depart,off ramp slight left,arrive |
+            | a,e       | abce,abce  | depart,arrive                      |
+            | a,f       | abce,cf,cf | depart,turn slight right,arrive |

include/extractor/files.hpp

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

include/extractor/serialization.hpp

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

include/partitioner/partitioner_config.hpp

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

include/partitioner/renumber.hpp

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

package.json

@@ -1,6 +1,6 @@
 {
   "name": "osrm",
-  "version": "5.16.0-rc.5",
+  "version": "5.16.0-rc.7",
   "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/extractor/edge_based_graph_factory.cpp

@@ -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.";

src/extractor/maneuver_override_relation_parser.cpp

@@ -52,8 +52,10 @@ ManeuverOverrideRelationParser::TryParse(const osmium::Relation &relation) const
     maneuver_override.maneuver = relation.tags().get_value_by_key("maneuver", "");
     maneuver_override.direction = relation.tags().get_value_by_key("direction", "");
 
-    boost::optional<std::uint64_t> from = boost::none, via = boost::none, to = boost::none;
-    std::vector<std::uint64_t> via_ways;
+    bool valid_relation = true;
+    OSMNodeID via_node = SPECIAL_OSM_NODEID;
+    OSMWayID from = SPECIAL_OSM_WAYID, to = SPECIAL_OSM_WAYID;
+    std::vector<OSMWayID> via_ways;
 
     for (const auto &member : relation.members())
     {
@@ -74,8 +76,9 @@ ManeuverOverrideRelationParser::TryParse(const osmium::Relation &relation) const
                 continue;
             }
             BOOST_ASSERT(0 == strcmp("via", role));
-            via = static_cast<std::uint64_t>(member.ref());
             // set via node id
+            valid_relation &= via_node == SPECIAL_OSM_NODEID;
+            via_node = OSMNodeID{static_cast<std::uint64_t>(member.ref())};
             break;
         }
         case osmium::item_type::way:
@@ -83,15 +86,17 @@ ManeuverOverrideRelationParser::TryParse(const osmium::Relation &relation) const
                          0 == strcmp("via", role));
             if (0 == strcmp("from", role))
             {
-                from = static_cast<std::uint64_t>(member.ref());
+                valid_relation &= from == SPECIAL_OSM_WAYID;
+                from = OSMWayID{static_cast<std::uint64_t>(member.ref())};
             }
             else if (0 == strcmp("to", role))
             {
-                to = static_cast<std::uint64_t>(member.ref());
+                valid_relation &= to == SPECIAL_OSM_WAYID;
+                to = OSMWayID{static_cast<std::uint64_t>(member.ref())};
             }
             else if (0 == strcmp("via", role))
             {
-                via_ways.push_back(static_cast<std::uint64_t>(member.ref()));
+                via_ways.push_back(OSMWayID{static_cast<std::uint64_t>(member.ref())});
             }
             break;
         case osmium::item_type::relation:
@@ -103,18 +108,17 @@ ManeuverOverrideRelationParser::TryParse(const osmium::Relation &relation) const
         }
     }
 
-    if (from && (via || via_ways.size() > 0) && to)
+    // Check required roles
+    valid_relation &= from != SPECIAL_OSM_WAYID;
+    valid_relation &= to != SPECIAL_OSM_WAYID;
+    valid_relation &= via_node != SPECIAL_OSM_NODEID;
+
+    if (valid_relation)
     {
-        via_ways.insert(via_ways.begin(), *from);
-        via_ways.push_back(*to);
-        if (via)
-        {
-            maneuver_override.via_node = {*via};
-        }
-        for (const auto &n : via_ways)
-        {
-            maneuver_override.via_ways.push_back(OSMWayID{n});
-        }
+        maneuver_override.via_ways.push_back(from);
+        std::copy(via_ways.begin(), via_ways.end(), std::back_inserter(maneuver_override.via_ways));
+        maneuver_override.via_ways.push_back(to);
+        maneuver_override.via_node = via_node;
     }
     else
     {

src/guidance/motorway_handler.cpp

@@ -118,16 +118,6 @@ Intersection MotorwayHandler::fromMotorway(const EdgeID via_eid, Intersection in
         node_data_container.GetAnnotation(node_based_graph.GetEdgeData(via_eid).annotation_data);
     BOOST_ASSERT(isMotorwayClass(via_eid, node_based_graph));
 
-    const auto countExitingMotorways = [this](const Intersection &intersection) {
-        unsigned count = 0;
-        for (const auto &road : intersection)
-        {
-            if (road.entry_allowed && isMotorwayClass(road.eid, node_based_graph))
-                ++count;
-        }
-        return count;
-    };
-
     // find the angle that continues on our current highway
     const auto getContinueAngle = [this, in_data](const Intersection &intersection) {
         for (const auto &road : intersection)
@@ -219,7 +209,13 @@ Intersection MotorwayHandler::fromMotorway(const EdgeID via_eid, Intersection in
     }
     else
     {
-        const unsigned exiting_motorways = countExitingMotorways(intersection);
+        const auto valid_exits = std::count_if(intersection.begin(),
+                                               intersection.end(),
+                                               [](const auto &road) { return road.entry_allowed; });
+        const auto exiting_motorways =
+            std::count_if(intersection.begin(), intersection.end(), [this](const auto &road) {
+                return road.entry_allowed && isMotorwayClass(road.eid, node_based_graph);
+            });
 
         if (exiting_motorways == 0)
         {
@@ -233,7 +229,7 @@ Intersection MotorwayHandler::fromMotorway(const EdgeID via_eid, Intersection in
                 }
             }
         }
-        else if (exiting_motorways == 1)
+        else if (exiting_motorways == 1 || exiting_motorways != valid_exits)
         {
             // normal motorway passing some ramps or mering onto another motorway
             if (intersection.size() == 2)

src/guidance/turn_handler.cpp

@@ -249,8 +249,13 @@ Intersection TurnHandler::handleThreeWayTurn(const EdgeID via_edge, Intersection
                OOOOOOO
      */
 
+    const auto all_ramps =
+        std::all_of(intersection.begin(), intersection.end(), [this](const auto &road) {
+            return node_based_graph.GetEdgeData(road.eid).flags.road_classification.IsRampClass();
+        });
+
     auto fork = findFork(via_edge, intersection);
-    if (fork && obvious_index == 0)
+    if (fork && (all_ramps || obvious_index == 0))
     {
         assignFork(via_edge, fork->getLeft(), fork->getRight());
     }

src/partitioner/partitioner.cpp

@@ -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 "

src/storage/storage.cpp

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