fixes issue #1019:

- fix ignored turn restriction on chains of degree-2 nodes - add a cucumber test to test for potential regressions
Project-OSRM · May 20, 2014 · d028a30 · d028a30
1 parent 75a2d4d
commit d028a30
Show file tree

Hide file tree

Showing 4 changed files with 87 additions and 6 deletions.
diff --git a/Contractor/EdgeBasedGraphFactory.cpp b/Contractor/EdgeBasedGraphFactory.cpp
@@ -305,6 +305,12 @@ void EdgeBasedGraphFactory::CompressGeometry()
             continue;
         }
 
+        // check if v is a via node for a turn restriction, i.e. a 'directed' barrier node
+        if (m_restriction_map->IsNodeAViaNode(v))
+        {
+            continue;
+        }
+
         const bool reverse_edge_order =
             !(m_node_based_graph->GetEdgeData(m_node_based_graph->BeginEdges(v)).forward);
         const EdgeID forward_e2 = m_node_based_graph->BeginEdges(v) + reverse_edge_order;

diff --git a/DataStructures/RestrictionMap.cpp b/DataStructures/RestrictionMap.cpp
@@ -1,6 +1,40 @@
+/*
+
+Copyright (c) 2013, Project OSRM, Dennis Luxen, others
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without modification,
+are permitted provided that the following conditions are met:
+
+Redistributions of source code must retain the above copyright notice, this list
+of conditions and the following disclaimer.
+Redistributions in binary form must reproduce the above copyright notice, this
+list of conditions and the following disclaimer in the documentation and/or
+other materials provided with the distribution.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR
+ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+*/
+
 #include "RestrictionMap.h"
 #include "NodeBasedGraph.h"
 
+#include "../Util/SimpleLogger.h"
+
+bool RestrictionMap::IsNodeAViaNode(const NodeID node) const
+{
+    return m_no_turn_via_node_set.find(node) != m_no_turn_via_node_set.end();
+}
+
 RestrictionMap::RestrictionMap(const std::shared_ptr<NodeBasedDynamicGraph> &graph,
                                const std::vector<TurnRestriction> &input_restrictions_list)
     : m_count(0), m_graph(graph)
@@ -9,8 +43,14 @@ RestrictionMap::RestrictionMap(const std::shared_ptr<NodeBasedDynamicGraph> &gra
     // and all end-nodes
     for (auto &restriction : input_restrictions_list)
     {
+
+        m_no_turn_via_node_set.insert(restriction.viaNode);
+
         std::pair<NodeID, NodeID> restriction_source =
             std::make_pair(restriction.fromNode, restriction.viaNode);
+
+        SimpleLogger().Write(logDEBUG) << "restr from: " << restriction.fromNode << ", v: " << restriction.viaNode << ", to: " << restriction.toNode << ", is_only_: " << (restriction.flags.isOnly ? "y" : "n");
+
         unsigned index;
         auto restriction_iter = m_restriction_map.find(restriction_source);
         if (restriction_iter == m_restriction_map.end())
@@ -70,8 +110,11 @@ void RestrictionMap::FixupArrivingTurnRestriction(const NodeID u, const NodeID v
         const std::pair<NodeID, NodeID> restr_start = std::make_pair(x, u);
         auto restriction_iterator = m_restriction_map.find(restr_start);
         if (restriction_iterator == m_restriction_map.end())
+        {
             continue;
+        }
 
+        SimpleLogger().Write(logDEBUG) << "fixing arriving instruction at turn <" << u << "," << v << "," << w << ">";
         const unsigned index = restriction_iterator->second;
         auto &bucket = m_restriction_bucket_list.at(index);
         for (RestrictionTarget &restriction_target : bucket)
@@ -98,6 +141,7 @@ void RestrictionMap::FixupStartingTurnRestriction(const NodeID u, const NodeID v
     auto restriction_iterator = m_restriction_map.find(old_start);
     if (restriction_iterator != m_restriction_map.end())
     {
+        SimpleLogger().Write(logDEBUG) << "fixing restr start at turn <" << u << "," << v << "," << w << ">";
         const unsigned index = restriction_iterator->second;
         // remove old restriction start (v,w)
         m_restriction_map.erase(restriction_iterator);
@@ -141,6 +185,7 @@ NodeID RestrictionMap::CheckForEmanatingIsOnlyTurn(const NodeID u, const NodeID
  */
 bool RestrictionMap::CheckIfTurnIsRestricted(const NodeID u, const NodeID v, const NodeID w) const
 {
+    // SimpleLogger().Write(logDEBUG) << "checking turn <" << u << "," << v << "," << w << ">";
     BOOST_ASSERT(u != std::numeric_limits<unsigned>::max());
     BOOST_ASSERT(v != std::numeric_limits<unsigned>::max());
     BOOST_ASSERT(w != std::numeric_limits<unsigned>::max());

diff --git a/DataStructures/RestrictionMap.h b/DataStructures/RestrictionMap.h
@@ -36,11 +36,11 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include "NodeBasedGraph.h"
 
 #include <boost/unordered_map.hpp>
+#include <unordered_set>
 
-/*!
- * Makee it efficent to look up if an edge is the start + via node of a TurnRestriction.
- * Is needed by EdgeBasedGraphFactory.
- */
+
+// Make it efficent to look up if an edge is the start + via node of a TurnRestriction
+// EdgeBasedEdgeFactory decides by it if edges are inserted or geometry is compressed
 class RestrictionMap
 {
   public:
@@ -51,7 +51,7 @@ class RestrictionMap
     void FixupStartingTurnRestriction(const NodeID u, const NodeID v, const NodeID w);
     NodeID CheckForEmanatingIsOnlyTurn(const NodeID u, const NodeID v) const;
     bool CheckIfTurnIsRestricted(const NodeID u, const NodeID v, const NodeID w) const;
-
+    bool IsNodeAViaNode(const NodeID node) const;
     unsigned size() { return m_count; }
 
   private:
@@ -66,6 +66,7 @@ class RestrictionMap
     std::vector<EmanatingRestrictionsVector> m_restriction_bucket_list;
     //! maps (start, via) -> bucket index
     boost::unordered_map<RestrictionSource, unsigned> m_restriction_map;
+    std::unordered_set<NodeID> m_no_turn_via_node_set;
 };
 
 #endif
diff --git a/features/car/restrictions.feature b/features/car/restrictions.feature
@@ -30,6 +30,35 @@ Feature: Car - Turn restrictions
             | s    | n  | sj,nj |
             | s    | e  | sj,ej |
 
+    @no_turning
+    Scenario: Car - No straight on
+        Given the node map
+            | a | b | j | d | e |
+            | v |   |   |   | z |
+            |   | w | x | y |   |
+
+        And the ways
+            | nodes | oneway |
+            | ab    | no     |
+            | bj    | no     |
+            | jd    | no     |
+            | de    | no     |
+            | ej    | no     |
+            | av    | yes    |
+            | vw    | yes    |
+            | wx    | yes    |
+            | xy    | yes    |
+            | yz    | yes    |
+            | ze    | yes    |
+
+        And the relations
+            | type        | way:from | way:to | node:via | restriction    |
+            | restriction | bj       | jd     | j        | no_straight_on |
+
+        When I route I should get
+            | from | to | route             |
+            | a    | e  | av,vw,wx,xy,yz,ze |
+
     @no_turning
     Scenario: Car - No right turn
         Given the node map
@@ -248,4 +277,4 @@ Feature: Car - Turn restrictions
         When I route I should get
             | from | to | route |
             | s    | a  | sj,aj |
-            | s    | b  | sj,bj |
+            | s    | b  | sj,bj |