Filter out edges that have any speed=0 segments.

They become non-snappable and non-routable.  Note that a single segment
of speed=0 will eliminate the entire edge.

features/car/traffic_speeds.feature

@@ -23,8 +23,8 @@ Feature: Traffic - speeds
             | fb    | primary |
         And the speed file
         """
-        1,2,27
+        1,2,0
-        2,1,27
+        2,1,0
         2,3,27
         3,2,27
         1,4,27
@@ -37,11 +37,57 @@ Feature: Traffic - speeds
         Given the contract extra arguments "--segment-speed-file speeds.csv"
         And I route I should get
             | from | to | route          | speed   |
-            | a    | b  | ab,ab    | 27 km/h |
+            | a    | b  | ad,de,eb,eb    | 30 km/h |
-            | a    | c  | ab,bc,bc | 27 km/h |
+            | a    | c  | ad,dc,dc       | 31 km/h |
             | b    | c  | bc,bc          | 27 km/h |
             | a    | d  | ad,ad          | 27 km/h |
             | d    | c  | dc,dc          | 36 km/h |
-            | g    | b  | ab,ab    | 27 km/h |
+            | g    | b  | fb,fb          | 36 km/h |
-            | a    | g  | ab,ab    | 27 km/h |
+            | a    | g  | ad,df,fb,fb    | 30 km/h |
 
+
+    Scenario: Speeds that isolate a single node (a)
+        Given the profile "testbot"
+        Given the extract extra arguments "--generate-edge-lookup"
+        Given the contract extra arguments "--segment-speed-file speeds.csv"
+        Given the speed file
+        """
+        1,2,0
+        2,1,0
+        2,3,27
+        3,2,27
+        1,4,0
+        4,1,0
+        """
+        And I route I should get
+            | from | to | route          | speed   |
+            | a    | b  | fb,fb          | 36 km/h |
+            | a    | c  | fb,bc,bc       | 30 km/h |
+            | b    | c  | bc,bc          | 27 km/h |
+            | a    | d  | fb,df,df       | 36 km/h |
+            | d    | c  | dc,dc          | 36 km/h |
+            | g    | b  | fb,fb          | 36 km/h |
+            | a    | g  | fb,fb          | 36 km/h |
+
+    Scenario: Verify that negative values are treated like 0
+        Given the profile "testbot"
+        Given the extract extra arguments "--generate-edge-lookup"
+        Given the contract extra arguments "--segment-speed-file speeds.csv"
+        Given the speed file
+        """
+        1,2,-10
+        2,1,-20
+        2,3,27
+        3,2,27
+        1,4,-3
+        4,1,-5
+        """
+        And I route I should get
+            | from | to | route          | speed   |
+            | a    | b  | fb,fb          | 36 km/h |
+            | a    | c  | fb,bc,bc       | 30 km/h |
+            | b    | c  | bc,bc          | 27 km/h |
+            | a    | d  | fb,df,df       | 36 km/h |
+            | d    | c  | dc,dc          | 36 km/h |
+            | g    | b  | fb,fb          | 36 km/h |
+            | a    | g  | fb,fb          | 36 km/h |

include/engine/geospatial_query.hpp

@@ -20,6 +20,11 @@ namespace osrm
 namespace engine
 {
 
+inline std::pair<bool, bool> boolPairAnd(const std::pair<bool, bool> &A, const std::pair<bool, bool> &B)
+{
+    return std::make_pair(A.first && B.first, A.second && B.second);
+}
+
 // Implements complex queries on top of an RTree and builds PhantomNodes from it.
 //
 // Only holds a weak reference on the RTree and coordinates!
@@ -48,7 +53,7 @@ template <typename RTreeT, typename DataFacadeT> class GeospatialQuery
     {
         auto results =
             rtree.Nearest(input_coordinate,
-                          [](const CandidateSegment &) { return std::make_pair(true, true); },
+                          [this](const CandidateSegment &segment) { return HasValidEdge(segment); },
                           [this, max_distance, input_coordinate](const std::size_t,
                                                                  const CandidateSegment &segment) {
                               return CheckSegmentDistance(input_coordinate, segment, max_distance);
@@ -68,7 +73,7 @@ template <typename RTreeT, typename DataFacadeT> class GeospatialQuery
         auto results = rtree.Nearest(
             input_coordinate,
             [this, bearing, bearing_range, max_distance](const CandidateSegment &segment) {
-                return CheckSegmentBearing(segment, bearing, bearing_range);
+                return boolPairAnd(CheckSegmentBearing(segment, bearing, bearing_range),HasValidEdge(segment));
             },
             [this, max_distance, input_coordinate](const std::size_t,
                                                    const CandidateSegment &segment) {
@@ -89,7 +94,7 @@ template <typename RTreeT, typename DataFacadeT> class GeospatialQuery
         auto results =
             rtree.Nearest(input_coordinate,
                           [this, bearing, bearing_range](const CandidateSegment &segment) {
-                              return CheckSegmentBearing(segment, bearing, bearing_range);
+                              return boolPairAnd(CheckSegmentBearing(segment, bearing, bearing_range), HasValidEdge(segment));
                           },
                           [max_results](const std::size_t num_results, const CandidateSegment &) {
                               return num_results >= max_results;
@@ -111,7 +116,7 @@ template <typename RTreeT, typename DataFacadeT> class GeospatialQuery
         auto results =
             rtree.Nearest(input_coordinate,
                           [this, bearing, bearing_range](const CandidateSegment &segment) {
-                              return CheckSegmentBearing(segment, bearing, bearing_range);
+                              return boolPairAnd(CheckSegmentBearing(segment, bearing, bearing_range), HasValidEdge(segment));
                           },
                           [this, max_distance, max_results, input_coordinate](
                               const std::size_t num_results, const CandidateSegment &segment) {
@@ -129,7 +134,7 @@ template <typename RTreeT, typename DataFacadeT> class GeospatialQuery
     {
         auto results =
             rtree.Nearest(input_coordinate,
-                          [](const CandidateSegment &) { return std::make_pair(true, true); },
+                          [this](const CandidateSegment &segment) { return HasValidEdge(segment); },
                           [max_results](const std::size_t num_results, const CandidateSegment &) {
                               return num_results >= max_results;
                           });
@@ -146,7 +151,7 @@ template <typename RTreeT, typename DataFacadeT> class GeospatialQuery
     {
         auto results =
             rtree.Nearest(input_coordinate,
-                          [](const CandidateSegment &) { return std::make_pair(true, true); },
+                          [this](const CandidateSegment &segment) { return HasValidEdge(segment); },
                           [this, max_distance, max_results, input_coordinate](
                               const std::size_t num_results, const CandidateSegment &segment) {
                               return num_results >= max_results ||
@@ -166,14 +171,18 @@ template <typename RTreeT, typename DataFacadeT> class GeospatialQuery
         bool has_big_component = false;
         auto results = rtree.Nearest(
             input_coordinate,
-            [&has_big_component, &has_small_component](const CandidateSegment &segment) {
+            [this, &has_big_component, &has_small_component](const CandidateSegment &segment) {
                 auto use_segment = (!has_small_component ||
                                     (!has_big_component && !segment.data.component.is_tiny));
                 auto use_directions = std::make_pair(use_segment, use_segment);
+                const auto valid_edges = HasValidEdge(segment);
 
+                if (valid_edges.first || valid_edges.second)
+                {
                     has_big_component = has_big_component || !segment.data.component.is_tiny;
                     has_small_component = has_small_component || segment.data.component.is_tiny;
-
+                }
+                use_directions = boolPairAnd(use_directions, valid_edges);
                 return use_directions;
             },
             [this, &has_big_component, max_distance, input_coordinate](
@@ -201,14 +210,21 @@ template <typename RTreeT, typename DataFacadeT> class GeospatialQuery
         bool has_big_component = false;
         auto results = rtree.Nearest(
             input_coordinate,
-            [&has_big_component, &has_small_component](const CandidateSegment &segment) {
+            [this, &has_big_component, &has_small_component](const CandidateSegment &segment) {
                 auto use_segment = (!has_small_component ||
                                     (!has_big_component && !segment.data.component.is_tiny));
                 auto use_directions = std::make_pair(use_segment, use_segment);
+                if (!use_directions.first && !use_directions.second) return use_directions;
+                const auto valid_edges = HasValidEdge(segment);
+
+                if (valid_edges.first || valid_edges.second)
+                {
 
                     has_big_component = has_big_component || !segment.data.component.is_tiny;
                     has_small_component = has_small_component || segment.data.component.is_tiny;
+                }
 
+                use_directions = boolPairAnd(use_directions, valid_edges);
                 return use_directions;
             },
             [&has_big_component](const std::size_t num_results, const CandidateSegment &) {
@@ -239,10 +255,11 @@ template <typename RTreeT, typename DataFacadeT> class GeospatialQuery
                 auto use_segment = (!has_small_component ||
                                     (!has_big_component && !segment.data.component.is_tiny));
                 auto use_directions = std::make_pair(use_segment, use_segment);
+                use_directions = boolPairAnd(use_directions, HasValidEdge(segment));
 
                 if (use_segment)
                 {
-                    use_directions = CheckSegmentBearing(segment, bearing, bearing_range);
+                    use_directions = boolPairAnd(CheckSegmentBearing(segment, bearing, bearing_range), HasValidEdge(segment));
                     if (use_directions.first || use_directions.second)
                     {
                         has_big_component = has_big_component || !segment.data.component.is_tiny;
@@ -283,10 +300,11 @@ template <typename RTreeT, typename DataFacadeT> class GeospatialQuery
                 auto use_segment = (!has_small_component ||
                                     (!has_big_component && !segment.data.component.is_tiny));
                 auto use_directions = std::make_pair(use_segment, use_segment);
+                use_directions = boolPairAnd(use_directions, HasValidEdge(segment));
 
                 if (use_segment)
                 {
-                    use_directions = CheckSegmentBearing(segment, bearing, bearing_range);
+                    use_directions = boolPairAnd(CheckSegmentBearing(segment, bearing, bearing_range), HasValidEdge(segment));
                     if (use_directions.first || use_directions.second)
                     {
                         has_big_component = has_big_component || !segment.data.component.is_tiny;
@@ -440,6 +458,47 @@ template <typename RTreeT, typename DataFacadeT> class GeospatialQuery
         return std::make_pair(forward_bearing_valid, backward_bearing_valid);
     }
 
+    /**
+     * Checks to see if the edge weights are valid.  We might have an edge,
+     * but a traffic update might set the speed to 0 (weight == INVALID_EDGE_WEIGHT).
+     * which means that this edge is not currently traversible.  If this is the case,
+     * then we shouldn't snap to this edge.
+     */
+    std::pair<bool, bool> HasValidEdge(const CandidateSegment &segment) const
+    {
+
+        bool forward_edge_valid = false;
+        bool reverse_edge_valid = false;
+
+        if (segment.data.forward_packed_geometry_id != SPECIAL_EDGEID)
+        {
+            std::vector<EdgeWeight> forward_weight_vector;
+            datafacade.GetUncompressedWeights(segment.data.forward_packed_geometry_id,
+                                              forward_weight_vector);
+
+            if (forward_weight_vector[segment.data.fwd_segment_position] != INVALID_EDGE_WEIGHT)
+            {
+                forward_edge_valid = segment.data.forward_segment_id.enabled;
+            }
+        }
+
+        if (segment.data.reverse_packed_geometry_id != SPECIAL_EDGEID)
+        {
+            std::vector<EdgeWeight> reverse_weight_vector;
+            datafacade.GetUncompressedWeights(segment.data.reverse_packed_geometry_id,
+                                              reverse_weight_vector);
+
+            BOOST_ASSERT(segment.data.fwd_segment_position < reverse_weight_vector.size());
+
+            if (reverse_weight_vector[reverse_weight_vector.size() - segment.data.fwd_segment_position - 1] != INVALID_EDGE_WEIGHT)
+            {
+                reverse_edge_valid = segment.data.reverse_segment_id.enabled;
+            }
+        }
+
+        return std::make_pair(forward_edge_valid, reverse_edge_valid);
+    }
+
     const RTreeT &rtree;
     const CoordinateList &coordinates;
     DataFacadeT &datafacade;

src/contractor/contractor.cpp

@@ -758,6 +758,7 @@ EdgeID Contractor::LoadEdgeExpandedGraph(
 
         if (update_edge_weights || update_turn_penalties)
         {
+            bool skip_this_edge = false;
             auto header = reinterpret_cast<const extractor::lookup::SegmentHeaderBlock *>(
                 edge_segment_byte_ptr);
             edge_segment_byte_ptr += sizeof(extractor::lookup::SegmentHeaderBlock);
@@ -787,8 +788,11 @@ EdgeID Contractor::LoadEdgeExpandedGraph(
                     }
                     else
                     {
-                        // This edge is blocked, we don't need to continue updating
+                        // If we hit a 0-speed edge, then it's effectively not traversible.
-                        new_weight = INVALID_EDGE_WEIGHT;
+                        // We don't want to include it in the edge_based_edge_list, so
+                        // we set a flag and `continue` the parent loop as soon as we can.
+                        // This would be a perfect place to use `goto`, but Patrick vetoed it.
+                        skip_this_edge = true;
                         break;
                     }
                 }
@@ -801,6 +805,13 @@ EdgeID Contractor::LoadEdgeExpandedGraph(
                 previous_osm_node_id = segmentblocks[i].this_osm_node_id;
             }
 
+            // We found a zero-speed edge, so we'll skip this whole edge-based-edge which
+            // effectively removes it from the routing network.
+            if (skip_this_edge) {
+                penaltyblock++;
+                continue;
+            }
+
             const auto turn_iter = turn_penalty_lookup.find(
                 std::make_tuple(penaltyblock->from_id, penaltyblock->via_id, penaltyblock->to_id));
             if (turn_iter != turn_penalty_lookup.end())

unit_tests/mocks/mock_datafacade.hpp

@@ -62,8 +62,10 @@ class MockDataFacade final : public engine::datafacade::BaseDataFacade
     {
     }
     void GetUncompressedWeights(const EdgeID /* id */,
-                                std::vector<EdgeWeight> & /* result_weights */) const override
+                                std::vector<EdgeWeight> &result_weights) const override
     {
+        result_weights.resize(1);
+        result_weights[0] = 1;
     }
     void GetUncompressedDatasources(const EdgeID /*id*/,
                                     std::vector<uint8_t> & /*data_sources*/) const override

unit_tests/util/static_rtree.cpp

@@ -165,6 +165,9 @@ struct GraphFixture
             // to examine during tests.
             d.forward_segment_id = {pair.second, true};
             d.reverse_segment_id = {pair.first, true};
+            d.fwd_segment_position = 0;
+            d.forward_packed_geometry_id = 0;
+            d.reverse_packed_geometry_id = 0;
             edges.emplace_back(d);
         }
     }