restructure for review remarks

CHANGELOG.md

@@ -14,6 +14,8 @@
       - Pass functions instead of strings to `WayHandlers.run()`, so it's possible to mix in your own functions.
       - Reorders arguments to `WayHandlers` functions to match `process_way()`.
       - Profiles must return a hash of profile functions. This makes it easier for profiles to include each other.
+      - BREAKING: the file format requires re-processing due to the changes on via-ways
+      - Added support for via-way restrictions
 
 # 5.9.1
     - Infrastructure

features/car/restrictions.feature

@@ -639,6 +639,8 @@ Feature: Car - Turn restrictions
                 |   | \                                     /
                 i - d - e - - - - - - - - - - - - - - - - -
             """
+        # The long distances here are required to make other turns undesriable in comparison to the restricted turns.
+        # Otherwise they might just be picked without the actual turns being restricted
 
         And the ways
             | nodes | oneway |

include/extractor/edge_based_graph_factory.hpp

@@ -92,7 +92,7 @@ class EdgeBasedGraphFactory
              const std::string &turn_duration_penalties_filename,
              const std::string &turn_penalties_index_filename,
              const std::string &cnbg_ebg_mapping_path,
-             const RestrictionMap &restriction_map,
+             const RestrictionMap &node_restriction_map,
              const WayRestrictionMap &way_restriction_map);
 
     // The following get access functions destroy the content in the factory
@@ -137,9 +137,11 @@ class EdgeBasedGraphFactory
     EdgeBasedNodeDataContainer m_edge_based_node_container;
     util::DeallocatingVector<EdgeBasedEdge> m_edge_based_edge_list;
 
-    // the number of edge-based nodes is mostly made up out of the edges in the node-based graph.
+    // The number of edge-based nodes is mostly made up out of the edges in the node-based graph.
     // Any edge in the node-based graph represents a node in the edge-based graph. In addition, we
     // add a set of artificial edge-based nodes into the mix to model via-way turn restrictions.
+    // See https://github.com/Project-OSRM/osrm-backend/issues/2681#issuecomment-313080353 for
+    // reference
     std::uint64_t m_number_of_edge_based_nodes;
 
     const std::vector<util::Coordinate> &m_coordinates;
@@ -157,15 +159,21 @@ class EdgeBasedGraphFactory
 
     unsigned RenumberEdges();
 
+    // During the generation of the edge-expanded nodes, we need to also generate duplicates that
+    // represent state during via-way restrictions (see
+    // https://github.com/Project-OSRM/osrm-backend/issues/2681#issuecomment-313080353). Access to
+    // the information on what to duplicate and how is provided via the way_restriction_map
     std::vector<NBGToEBG> GenerateEdgeExpandedNodes(const WayRestrictionMap &way_restriction_map);
 
+    // Edge-expanded edges are generate for all valid turns. The validity can be checked via the
+    // restriction maps
     void GenerateEdgeExpandedEdges(ScriptingEnvironment &scripting_environment,
                                    const std::string &original_edge_data_filename,
                                    const std::string &turn_lane_data_filename,
                                    const std::string &turn_weight_penalties_filename,
                                    const std::string &turn_duration_penalties_filename,
                                    const std::string &turn_penalties_index_filename,
-                                   const RestrictionMap &restriction_map,
+                                   const RestrictionMap &node_restriction_map,
                                    const WayRestrictionMap &way_restriction_map);
 
     NBGToEBG InsertEdgeBasedNode(const NodeID u, const NodeID v);

include/extractor/extraction_containers.hpp

@@ -51,7 +51,10 @@ class ExtractionContainers
 
     unsigned max_internal_node_id;
 
-    // list of restrictions before we transform them into the output types
+    // list of restrictions before we transform them into the output types. Input containers
+    // reference OSMNodeIDs. We can only transform them to the correct internal IDs after we've read
+    // everything. Without a multi-parse approach, we have to remember the output restrictions
+    // before converting them to the internal formats
     std::vector<InputConditionalTurnRestriction> restrictions_list;
 
     // turn restrictions split into conditional and unconditional turn restrictions

include/extractor/restriction.hpp

@@ -5,7 +5,7 @@
 #include "util/opening_hours.hpp"
 #include "util/typedefs.hpp"
 
-#include <boost/variant.hpp>
+#include "mapbox/variant.hpp"
 #include <limits>
 
 namespace osrm
@@ -55,21 +55,21 @@ enum RestrictionType
 struct InputTurnRestriction
 {
     // keep in the same order as the turn restrictions below
-    boost::variant<InputNodeRestriction, InputWayRestriction> node_or_way;
+    mapbox::util::variant<InputNodeRestriction, InputWayRestriction> node_or_way;
     bool is_only;
 
     OSMWayID From() const
     {
         return node_or_way.which() == RestrictionType::NODE_RESTRICTION
-                   ? boost::get<InputNodeRestriction>(node_or_way).from
-                   : boost::get<InputWayRestriction>(node_or_way).from;
+                   ? mapbox::util::get<InputNodeRestriction>(node_or_way).from
+                   : mapbox::util::get<InputWayRestriction>(node_or_way).from;
     }
 
     OSMWayID To() const
     {
         return node_or_way.which() == RestrictionType::NODE_RESTRICTION
-                   ? boost::get<InputNodeRestriction>(node_or_way).to
-                   : boost::get<InputWayRestriction>(node_or_way).to;
+                   ? mapbox::util::get<InputNodeRestriction>(node_or_way).to
+                   : mapbox::util::get<InputWayRestriction>(node_or_way).to;
     }
 
     RestrictionType Type() const
@@ -81,25 +81,25 @@ struct InputTurnRestriction
     InputWayRestriction &AsWayRestriction()
     {
         BOOST_ASSERT(node_or_way.which() == RestrictionType::WAY_RESTRICTION);
-        return boost::get<InputWayRestriction>(node_or_way);
+        return mapbox::util::get<InputWayRestriction>(node_or_way);
     }
 
     const InputWayRestriction &AsWayRestriction() const
     {
         BOOST_ASSERT(node_or_way.which() == RestrictionType::WAY_RESTRICTION);
-        return boost::get<InputWayRestriction>(node_or_way);
+        return mapbox::util::get<InputWayRestriction>(node_or_way);
     }
 
     InputNodeRestriction &AsNodeRestriction()
     {
         BOOST_ASSERT(node_or_way.which() == RestrictionType::NODE_RESTRICTION);
-        return boost::get<InputNodeRestriction>(node_or_way);
+        return mapbox::util::get<InputNodeRestriction>(node_or_way);
     }
 
     const InputNodeRestriction &AsNodeRestriction() const
     {
         BOOST_ASSERT(node_or_way.which() == RestrictionType::NODE_RESTRICTION);
-        return boost::get<InputNodeRestriction>(node_or_way);
+        return mapbox::util::get<InputNodeRestriction>(node_or_way);
     }
 };
 struct InputConditionalTurnRestriction : InputTurnRestriction
@@ -121,12 +121,6 @@ struct NodeRestriction
         return from != SPECIAL_NODEID && to != SPECIAL_NODEID && via != SPECIAL_NODEID;
     };
 
-    std::string ToString() const
-    {
-        return "From " + std::to_string(from) + " via " + std::to_string(via) + " to " +
-               std::to_string(to);
-    }
-
     bool operator==(const NodeRestriction &other) const
     {
         return std::tie(from, via, to) == std::tie(other.from, other.via, other.to);
@@ -168,7 +162,7 @@ struct WayRestriction
 struct TurnRestriction
 {
     // keep in the same order as the turn restrictions above
-    boost::variant<NodeRestriction, WayRestriction> node_or_way;
+    mapbox::util::variant<NodeRestriction, WayRestriction> node_or_way;
     bool is_only;
 
     // construction for NodeRestrictions
@@ -191,25 +185,25 @@ struct TurnRestriction
     WayRestriction &AsWayRestriction()
     {
         BOOST_ASSERT(node_or_way.which() == RestrictionType::WAY_RESTRICTION);
-        return boost::get<WayRestriction>(node_or_way);
+        return mapbox::util::get<WayRestriction>(node_or_way);
     }
 
     const WayRestriction &AsWayRestriction() const
     {
         BOOST_ASSERT(node_or_way.which() == RestrictionType::WAY_RESTRICTION);
-        return boost::get<WayRestriction>(node_or_way);
+        return mapbox::util::get<WayRestriction>(node_or_way);
     }
 
     NodeRestriction &AsNodeRestriction()
     {
         BOOST_ASSERT(node_or_way.which() == RestrictionType::NODE_RESTRICTION);
-        return boost::get<NodeRestriction>(node_or_way);
+        return mapbox::util::get<NodeRestriction>(node_or_way);
     }
 
     const NodeRestriction &AsNodeRestriction() const
     {
         BOOST_ASSERT(node_or_way.which() == RestrictionType::NODE_RESTRICTION);
-        return boost::get<NodeRestriction>(node_or_way);
+        return mapbox::util::get<NodeRestriction>(node_or_way);
     }
 
     RestrictionType Type() const
@@ -250,24 +244,6 @@ struct TurnRestriction
             return AsNodeRestriction() == other.AsNodeRestriction();
         }
     }
-
-    std::string ToString() const
-    {
-        std::string representation;
-        if (node_or_way.which() == RestrictionType::WAY_RESTRICTION)
-        {
-            auto const &way = AsWayRestriction();
-            representation =
-                "In: " + way.in_restriction.ToString() + " Out: " + way.out_restriction.ToString();
-        }
-        else
-        {
-            auto const &node = AsNodeRestriction();
-            representation = node.ToString();
-        }
-        representation += " is_only: " + std::to_string(is_only);
-        return representation;
-    }
 };
 
 struct ConditionalTurnRestriction : TurnRestriction

include/extractor/restriction_compressor.hpp

@@ -32,12 +32,12 @@ class RestrictionCompressor
     void Compress(const NodeID from, const NodeID via, const NodeID to);
 
   private:
-    // a turn restriction is given as `from head via node to tail`. Edges ending at `head` being
+    // a turn restriction is given as `from star via node to end`. Edges ending at `head` being
     // contracted move the head pointer to their respective head. Edges starting at tail move the
     // tail values to their respective tails. Way turn restrictions are represented by two
     // node-restrictions, so we can focus on them alone
-    boost::unordered_multimap<NodeID, NodeRestriction *> heads;
-    boost::unordered_multimap<NodeID, NodeRestriction *> tails;
+    boost::unordered_multimap<NodeID, NodeRestriction *> starts;
+    boost::unordered_multimap<NodeID, NodeRestriction *> ends;
 };
 
 } // namespace extractor

include/extractor/serialization.hpp

@@ -184,18 +184,18 @@ inline void write(storage::io::FileWriter &writer, const TurnRestriction &restri
     writer.WriteOne(restriction.is_only);
     if (restriction.Type() == RestrictionType::WAY_RESTRICTION)
     {
-        write(writer, boost::get<WayRestriction>(restriction.node_or_way));
+        write(writer, mapbox::util::get<WayRestriction>(restriction.node_or_way));
     }
     else
     {
         BOOST_ASSERT(restriction.Type() == RestrictionType::NODE_RESTRICTION);
-        write(writer, boost::get<NodeRestriction>(restriction.node_or_way));
+        write(writer, mapbox::util::get<NodeRestriction>(restriction.node_or_way));
     }
 }
 
 inline void write(storage::io::FileWriter &writer, const ConditionalTurnRestriction &restriction)
 {
-    write(writer, static_cast<TurnRestriction>(restriction));
+    write(writer, static_cast<const TurnRestriction &>(restriction));
     writer.WriteElementCount64(restriction.condition.size());
     for (const auto &c : restriction.condition)
     {
@@ -208,9 +208,7 @@ inline void write(storage::io::FileWriter &writer, const ConditionalTurnRestrict
 
 inline void read(storage::io::FileReader &reader, ConditionalTurnRestriction &restriction)
 {
-    TurnRestriction base;
-    read(reader, base);
-    reinterpret_cast<TurnRestriction &>(restriction) = std::move(base);
+    read(reader, static_cast<TurnRestriction &>(restriction));
     const auto num_conditions = reader.ReadElementCount64();
     restriction.condition.resize(num_conditions);
     for (uint64_t i = 0; i < num_conditions; i++)

include/extractor/way_restriction_map.hpp

@@ -23,30 +23,30 @@ class WayRestrictionMap
   public:
     struct ViaWay
     {
-        std::size_t id;
         NodeID from;
         NodeID to;
     };
     WayRestrictionMap(const std::vector<TurnRestriction> &turn_restrictions);
 
-    // check if an edge between two nodes is a restricted turn. The check needs to be performed
+    // Check if an edge between two nodes is a restricted turn. The check needs to be performed to
+    // find duplicated nodes during the creation of edge-based-edges
     bool IsViaWay(const NodeID from, const NodeID to) const;
 
-    // number of duplicated nodes
+    // Every via-way results in a duplicated node that is required in the edge-based-graph. This
+    // count is essentially the same as the number of valid via-way restrictions (except for
+    // non-only restrictions that share the same in/via combination)
     std::size_t NumberOfDuplicatedNodes() const;
 
-    // returns a representative for the duplicated way, consisting of the representative ID (first
+    // Returns a representative for each duplicated node, consisting of the representative ID (first
     // ID of the nodes restrictions) and the from/to vertices of the via-way
     // This is used to construct edge based nodes that act as intermediate nodes.
     std::vector<ViaWay> DuplicatedNodeRepresentatives() const;
 
     // Access all duplicated NodeIDs for a set of nodes indicating a via way
-    util::range<std::size_t> DuplicatedNodeIDs(const NodeID from, const NodeID to) const;
+    util::range<DuplicatedNodeID> DuplicatedNodeIDs(const NodeID from, const NodeID to) const;
 
     // check whether a turn onto a given node is restricted, when coming from a duplicated node
-    bool IsRestricted(std::size_t duplicated_node, const NodeID to) const;
-
-    TurnRestriction const &GetRestriction(std::size_t) const;
+    bool IsRestricted(DuplicatedNodeID duplicated_node, const NodeID to) const;
 
     // changes edge_based_node to the correct duplicated_node_id in case node_based_from,
     // node_based_via, node_based_to can be identified with a restriction group
@@ -57,7 +57,7 @@ class WayRestrictionMap
                              const NodeID number_of_edge_based_nodes) const;
 
   private:
-    std::size_t AsDuplicatedNodeID(const std::size_t restriction_id) const;
+    DuplicatedNodeID AsDuplicatedNodeID(const RestrictionID restriction_id) const;
 
     // access all restrictions that have the same starting way and via way. Any duplicated node
     // represents the same in-way + via-way combination. This vector contains data about all
@@ -75,10 +75,10 @@ class WayRestrictionMap
     //
     //                    EBN: 0 . | 2 | 3 | 4 ...
     // duplicated node groups: ... | 5 | 7 | ...
-    std::vector<std::size_t> duplicated_node_groups;
+    std::vector<DuplicatedNodeID> duplicated_node_groups;
 
-    boost::unordered_multimap<std::pair<NodeID, NodeID>, std::size_t> restriction_starts;
-    boost::unordered_multimap<std::pair<NodeID, NodeID>, std::size_t> restriction_ends;
+    boost::unordered_multimap<std::pair<NodeID, NodeID>, RestrictionID> restriction_starts;
+    boost::unordered_multimap<std::pair<NodeID, NodeID>, RestrictionID> restriction_ends;
 
     std::vector<TurnRestriction> restriction_data;
 };

include/util/typedefs.hpp

@@ -45,12 +45,18 @@ struct osm_node_id
 struct osm_way_id
 {
 };
+struct duplicated_node
+{
+};
 }
 using OSMNodeID = osrm::Alias<std::uint64_t, tag::osm_node_id>;
 static_assert(std::is_pod<OSMNodeID>(), "OSMNodeID is not a valid alias");
 using OSMWayID = osrm::Alias<std::uint64_t, tag::osm_way_id>;
 static_assert(std::is_pod<OSMWayID>(), "OSMWayID is not a valid alias");
 
+using DuplicatedNodeID = std::uint64_t;
+using RestrictionID = std::uint64_t;
+
 static const OSMNodeID SPECIAL_OSM_NODEID =
     OSMNodeID{std::numeric_limits<OSMNodeID::value_type>::max()};
 static const OSMWayID SPECIAL_OSM_WAYID =