Tag smoothness=impassable breaks pedestrian routing (valhalla#5023)

CHANGELOG.md

@@ -8,6 +8,7 @@
    * FIXED: update CircleCI runners to Ubuntu 24.04 [#5002](https://github.com/valhalla/valhalla/pull/5002)
    * FIXED: Fixed a typo in the (previously undocumented) matrix-APIs responses `algorithm` field: `timedistancbssematrix` is now `timedistancebssmatrix` [#5000](https://github.com/valhalla/valhalla/pull/5000).
    * FIXED: More trivial cases in `CostMatrix` [#5001](https://github.com/valhalla/valhalla/pull/5001)
+   * FIXED: Tag smoothness=impassable breaks pedestrian routing [#5023](https://github.com/valhalla/valhalla/pull/5023)
    * FIXED: Make isochrone geotiff serialization use "north up" geotransform [#5019](https://github.com/valhalla/valhalla/pull/5019)
 * **Enhancement**
    * ADDED: Consider smoothness in all profiles that use surface [#4949](https://github.com/valhalla/valhalla/pull/4949)

lua/graph.lua

@@ -907,7 +907,7 @@ function filter_tags_generic(kv)
       kv[k] = v
     end
 
-    if kv["impassable"] == "yes" or kv["smoothness"] == "impassable" or access == "false" or (kv["access"] == "private" and (kv["emergency"] == "yes" or kv["service"] == "emergency_access")) then
+    if kv["impassable"] == "yes" or access == "false" or (kv["access"] == "private" and (kv["emergency"] == "yes" or kv["service"] == "emergency_access")) then
 
       kv["auto_forward"] = "false"
       kv["truck_forward"] = "false"
@@ -926,7 +926,7 @@ function filter_tags_generic(kv)
       kv["motorcycle_backward"] = "false"
       kv["pedestrian_backward"] = "false"
       kv["bike_backward"] = "false"
-    elseif kv["vehicle"] == "no" then --don't change ped access.
+    elseif kv["smoothness"] == "impassable" or kv["vehicle"] == "no" then --don't change ped access.
       kv["auto_forward"] = "false"
       kv["truck_forward"] = "false"
       kv["bus_forward"] = "false"
@@ -996,7 +996,7 @@ function filter_tags_generic(kv)
       default_val = tostring(rail)
     end
 
-    if ((ferry == false and rail == false) or kv["impassable"] == "yes" or kv["smoothness"] == "impassable" or access == "false" or (kv["access"] == "private" and (kv["emergency"] == "yes" or kv["service"] == "emergency_access"))) then
+    if ((ferry == false and rail == false) or kv["impassable"] == "yes" or access == "false" or (kv["access"] == "private" and (kv["emergency"] == "yes" or kv["service"] == "emergency_access"))) then
 
       kv["auto_forward"] = "false"
       kv["truck_forward"] = "false"
@@ -1018,7 +1018,7 @@ function filter_tags_generic(kv)
 
     else
       local ped_val = default_val
-      if kv["vehicle"] == "no" then --don't change ped access.
+      if kv["smoothness"] == "impassable" or kv["vehicle"] == "no" then --don't change ped access.
         default_val = "false"
       end
       --check for auto_forward overrides
@@ -1931,7 +1931,7 @@ function nodes_proc (kv, nokeys)
   local initial_access = access[kv["access"]]
   local access = initial_access or "true"
 
-  if (kv["impassable"] == "yes" or kv["smoothness"] == "impassable" or (kv["access"] == "private" and (kv["emergency"] == "yes" or kv["service"] == "emergency_access"))) then
+  if (kv["impassable"] == "yes" or (kv["access"] == "private" and (kv["emergency"] == "yes" or kv["service"] == "emergency_access"))) then
     access = "false"
   end
 
@@ -2029,7 +2029,7 @@ function nodes_proc (kv, nokeys)
   local motorcycle = motorcycle_tag or 1024
 
   --if access = false use tag if exists, otherwise no access for that mode.
-  if (access == "false" or kv["vehicle"] == "no" or kv["hov"] == "designated") then
+  if (access == "false" or kv["vehicle"] == "no" or kv["smoothness"] == "impassable" or kv["hov"] == "designated") then
     auto = auto_tag or 0
     truck = truck_tag or 0
     bus = bus_tag or 0

src/mjolnir/directededgebuilder.cc

@@ -153,7 +153,8 @@ DirectedEdgeBuilder::DirectedEdgeBuilder(const OSMWay& way,
   if ((way.pedestrian_forward() && !forward) || (way.pedestrian_backward() && forward)) {
     reverse_access |= kPedestrianAccess;
   }
-  if (way.use() != Use::kSteps && way.use() != Use::kConstruction) {
+  if (way.use() != Use::kSteps && way.use() != Use::kConstruction &&
+      way.surface() != Surface::kImpassable) {
     if (way.wheelchair_tag() && way.wheelchair()) {
       forward_access |= kWheelchairAccess;
       reverse_access |= kWheelchairAccess;

src/mjolnir/pbfgraphparser.cc

@@ -1498,6 +1498,8 @@ struct graph_callback : public OSMPBF::Callback {
           way_.set_surface(Surface::kGravel);
         } else if (tag_.second == "very_horrible") {
           way_.set_surface(Surface::kPath);
+        } else if (tag_.second == "impassable") {
+          way_.set_surface(Surface::kImpassable);
         } else {
           has_surface_ = false;
         }

test/gurka/test_exclude_unpaved_smoothness.cc

@@ -179,11 +179,16 @@ TEST(Standalone, SmoothnessAccess) {
     const DirectedEdge* edge_2 = nullptr;
 
     std::tie(edge_id_1, edge_1, edge_id_2, edge_2) = findEdge(*graph_reader, map.nodes, "AB", "B");
-    // no access due to smoothness = impassable and are therefore tossed.
+    // ped only access due to smoothness = impassable.
     // edge_1 = AB
     // edge_2 = BA
-    EXPECT_EQ(edge_1, nullptr);
-    EXPECT_EQ(edge_2, nullptr);
+    EXPECT_EQ(edge_1->forwardaccess(), baldr::kPedestrianAccess);
+    EXPECT_EQ(edge_1->reverseaccess(), baldr::kPedestrianAccess);
+    EXPECT_EQ(edge_2->forwardaccess(), baldr::kPedestrianAccess);
+    EXPECT_EQ(edge_2->reverseaccess(), baldr::kPedestrianAccess);
+
+    EXPECT_EQ(edge_1->surface(), baldr::Surface::kImpassable);
+    EXPECT_EQ(edge_2->surface(), baldr::Surface::kImpassable);
 
     std::tie(edge_id_1, edge_1, edge_id_2, edge_2) = findEdge(*graph_reader, map.nodes, "B1C", "C");
     // edge_1 = B1C
@@ -194,10 +199,13 @@ TEST(Standalone, SmoothnessAccess) {
     EXPECT_NE(edge_2->forwardaccess(), 0);
     EXPECT_NE(edge_2->reverseaccess(), 0);
 
+    EXPECT_EQ(edge_1->surface(), baldr::Surface::kPavedSmooth);
+    EXPECT_EQ(edge_2->surface(), baldr::Surface::kPavedSmooth);
+
     auto node_id = gurka::findNode(*graph_reader, map.nodes, "1");
     const auto* node = graph_reader->nodeinfo(node_id);
-    // no access due to smoothness = impassable
-    EXPECT_EQ(node->access(), 0);
+    // ped only access due to smoothness = impassable
+    EXPECT_EQ(node->access(), baldr::kPedestrianAccess);
   }
 }