Reorganizing pathfinder code to prepare for sharing the start/stop-in-a-zone logic between the CH and Dijkstra implementation. #411

map_model/src/pathfind/ch.rs

@@ -63,6 +63,63 @@ impl ContractionHierarchyPathfinder {
         }
     }
 
+    // Doesn't handle zones
+    pub fn simple_pathfind(&self, req: &PathRequest, map: &Map) -> Option<Path> {
+        match req.constraints {
+            PathConstraints::Pedestrian => {
+                let steps = walking_path_to_steps(self.walking_graph.pathfind(req, map)?, map);
+                Some(Path::new(map, steps, req.end.dist_along(), Vec::new()))
+            }
+            PathConstraints::Car => self.car_graph.pathfind(req, map).map(|(p, _)| p),
+            PathConstraints::Bike => self.bike_graph.pathfind(req, map).map(|(p, _)| p),
+            PathConstraints::Bus => self.bus_graph.pathfind(req, map).map(|(p, _)| p),
+            PathConstraints::Train => self.train_graph.pathfind(req, map).map(|(p, _)| p),
+        }
+    }
+
+    pub fn simple_walking_path(&self, req: &PathRequest, map: &Map) -> Option<Vec<WalkingNode>> {
+        self.walking_graph.pathfind(req, map)
+    }
+
+    pub fn should_use_transit(
+        &self,
+        map: &Map,
+        start: Position,
+        end: Position,
+    ) -> Option<(BusStopID, Option<BusStopID>, BusRouteID)> {
+        self.walking_with_transit_graph
+            .should_use_transit(map, start, end)
+    }
+
+    pub fn apply_edits(&mut self, map: &Map, timer: &mut Timer) {
+        timer.start("apply edits to car pathfinding");
+        self.car_graph.apply_edits(map);
+        timer.stop("apply edits to car pathfinding");
+
+        timer.start("apply edits to bike pathfinding");
+        self.bike_graph.apply_edits(map);
+        timer.stop("apply edits to bike pathfinding");
+
+        timer.start("apply edits to bus pathfinding");
+        self.bus_graph.apply_edits(map);
+        timer.stop("apply edits to bus pathfinding");
+
+        // Can't edit anything related to trains
+
+        timer.start("apply edits to pedestrian pathfinding");
+        self.walking_graph
+            .apply_edits(map, &self.bus_graph, &self.train_graph);
+        timer.stop("apply edits to pedestrian pathfinding");
+
+        timer.start("apply edits to pedestrian using transit pathfinding");
+        self.walking_with_transit_graph
+            .apply_edits(map, &self.bus_graph, &self.train_graph);
+        timer.stop("apply edits to pedestrian using transit pathfinding");
+    }
+}
+
+// TODO Lift this out of here
+impl ContractionHierarchyPathfinder {
     pub fn pathfind(&self, req: PathRequest, map: &Map) -> Option<Path> {
         if req.start.lane() == req.end.lane() && req.constraints == PathConstraints::Pedestrian {
             return Some(one_step_walking_path(&req, map));
@@ -135,16 +192,7 @@ impl ContractionHierarchyPathfinder {
             }
             (None, None) => {}
         }
-        match req.constraints {
-            PathConstraints::Pedestrian => {
-                let steps = walking_path_to_steps(self.walking_graph.pathfind(&req, map)?, map);
-                Some(Path::new(map, steps, req.end.dist_along(), Vec::new()))
-            }
-            PathConstraints::Car => self.car_graph.pathfind(&req, map).map(|(p, _)| p),
-            PathConstraints::Bike => self.bike_graph.pathfind(&req, map).map(|(p, _)| p),
-            PathConstraints::Bus => self.bus_graph.pathfind(&req, map).map(|(p, _)| p),
-            PathConstraints::Train => self.train_graph.pathfind(&req, map).map(|(p, _)| p),
-        }
+        self.simple_pathfind(&req, map)
     }
 
     // TODO Alright, reconsider refactoring pieces of this again. :)
@@ -213,7 +261,7 @@ impl ContractionHierarchyPathfinder {
                 one_step.dedup();
                 one_step
             } else {
-                self.walking_graph.pathfind(&req, map)?
+                self.simple_walking_path(&req, map)?
             };
             interior_path.extend(main_path);
             let steps = walking_path_to_steps(interior_path, map);
@@ -221,13 +269,7 @@ impl ContractionHierarchyPathfinder {
         }
 
         let mut interior_path = zone.pathfind(interior_req, map)?;
-        let main_path = match req.constraints {
-            PathConstraints::Pedestrian => unreachable!(),
-            PathConstraints::Car => self.car_graph.pathfind(&req, map).map(|(p, _)| p),
-            PathConstraints::Bike => self.bike_graph.pathfind(&req, map).map(|(p, _)| p),
-            PathConstraints::Bus => self.bus_graph.pathfind(&req, map).map(|(p, _)| p),
-            PathConstraints::Train => self.train_graph.pathfind(&req, map).map(|(p, _)| p),
-        }?;
+        let main_path = self.simple_pathfind(&req, map)?;
         interior_path.append(main_path, map);
         Some(interior_path)
     }
@@ -298,7 +340,7 @@ impl ContractionHierarchyPathfinder {
                 one_step.dedup();
                 one_step
             } else {
-                self.walking_graph.pathfind(&req, map)?
+                self.simple_walking_path(&req, map)?
             };
 
             main_path.extend(interior_path);
@@ -307,51 +349,9 @@ impl ContractionHierarchyPathfinder {
         }
 
         let interior_path = zone.pathfind(interior_req, map)?;
-        let mut main_path = match req.constraints {
-            PathConstraints::Pedestrian => unreachable!(),
-            PathConstraints::Car => self.car_graph.pathfind(&req, map).map(|(p, _)| p),
-            PathConstraints::Bike => self.bike_graph.pathfind(&req, map).map(|(p, _)| p),
-            PathConstraints::Bus => self.bus_graph.pathfind(&req, map).map(|(p, _)| p),
-            PathConstraints::Train => self.train_graph.pathfind(&req, map).map(|(p, _)| p),
-        }?;
+        let mut main_path = self.simple_pathfind(&req, map)?;
         main_path.append(interior_path, map);
         main_path.end_dist = orig_end_dist;
         Some(main_path)
     }
-
-    pub fn should_use_transit(
-        &self,
-        map: &Map,
-        start: Position,
-        end: Position,
-    ) -> Option<(BusStopID, Option<BusStopID>, BusRouteID)> {
-        self.walking_with_transit_graph
-            .should_use_transit(map, start, end)
-    }
-
-    pub fn apply_edits(&mut self, map: &Map, timer: &mut Timer) {
-        timer.start("apply edits to car pathfinding");
-        self.car_graph.apply_edits(map);
-        timer.stop("apply edits to car pathfinding");
-
-        timer.start("apply edits to bike pathfinding");
-        self.bike_graph.apply_edits(map);
-        timer.stop("apply edits to bike pathfinding");
-
-        timer.start("apply edits to bus pathfinding");
-        self.bus_graph.apply_edits(map);
-        timer.stop("apply edits to bus pathfinding");
-
-        // Can't edit anything related to trains
-
-        timer.start("apply edits to pedestrian pathfinding");
-        self.walking_graph
-            .apply_edits(map, &self.bus_graph, &self.train_graph);
-        timer.stop("apply edits to pedestrian pathfinding");
-
-        timer.start("apply edits to pedestrian using transit pathfinding");
-        self.walking_with_transit_graph
-            .apply_edits(map, &self.bus_graph, &self.train_graph);
-        timer.stop("apply edits to pedestrian using transit pathfinding");
-    }
 }

map_model/src/pathfind/dijkstra.rs

@@ -12,12 +12,13 @@ use crate::{LaneID, Map, Path, PathConstraints, PathRequest, PathStep, TurnID};
 
 // TODO These should maybe keep the DiGraphMaps as state. It's cheap to recalculate it for edits.
 
-pub fn pathfind(req: PathRequest, map: &Map) -> Option<Path> {
+// Doesn't handle zones
+pub fn simple_pathfind(req: &PathRequest, map: &Map) -> Option<Path> {
     if req.constraints == PathConstraints::Pedestrian {
         if req.start.lane() == req.end.lane() {
-            return Some(one_step_walking_path(&req, map));
+            return Some(one_step_walking_path(req, map));
         }
-        let steps = walking_path_to_steps(pathfind_walking(req.clone(), map)?, map);
+        let steps = walking_path_to_steps(simple_walking_path(req, map)?, map);
         return Some(Path::new(map, steps, req.end.dist_along(), Vec::new()));
     }
 
@@ -56,10 +57,10 @@ pub fn pathfind_avoiding_lanes(
         }
     }
 
-    calc_path(graph, req, map)
+    calc_path(graph, &req, map)
 }
 
-fn calc_path(graph: DiGraphMap<LaneID, TurnID>, req: PathRequest, map: &Map) -> Option<Path> {
+fn calc_path(graph: DiGraphMap<LaneID, TurnID>, req: &PathRequest, map: &Map) -> Option<Path> {
     let (_, path) = petgraph::algo::astar(
         &graph,
         req.start.lane(),
@@ -109,7 +110,7 @@ pub fn build_graph_for_pedestrians(map: &Map) -> DiGraphMap<WalkingNode, usize>
     graph
 }
 
-fn pathfind_walking(req: PathRequest, map: &Map) -> Option<Vec<WalkingNode>> {
+pub fn simple_walking_path(req: &PathRequest, map: &Map) -> Option<Vec<WalkingNode>> {
     let graph = build_graph_for_pedestrians(map);
 
     let closest_start = WalkingNode::closest(req.start, map);

map_model/src/pathfind/mod.rs

@@ -1,27 +1,27 @@
 //! Everything related to pathfinding through a map for different types of agents.
 
-use std::collections::{BTreeSet, VecDeque};
+use std::collections::VecDeque;
 use std::fmt;
 
 use enumset::EnumSetType;
 use serde::{Deserialize, Serialize};
 
-use abstutil::Timer;
 use geom::{Distance, PolyLine, EPSILON_DIST};
 
 pub use self::ch::ContractionHierarchyPathfinder;
 pub use self::dijkstra::{build_graph_for_pedestrians, build_graph_for_vehicles};
 pub use self::driving::driving_cost;
+pub use self::pathfinder::Pathfinder;
 pub use self::walking::{walking_cost, WalkingNode};
 use crate::{
-    osm, BuildingID, BusRouteID, BusStopID, Lane, LaneID, LaneType, Map, Position, Traversable,
-    TurnID, UberTurn,
+    osm, BuildingID, Lane, LaneID, LaneType, Map, Position, Traversable, TurnID, UberTurn,
 };
 
 mod ch;
 mod dijkstra;
 mod driving;
 mod node_map;
+mod pathfinder;
 // TODO tmp
 pub mod uber_turns;
 mod walking;
@@ -604,50 +604,3 @@ fn validate_restrictions(map: &Map, steps: &Vec<PathStep>) {
         }
     }
 }
-
-/// Most of the time, prefer using the faster contraction hierarchies. But sometimes, callers can
-/// explicitly opt into a slower (but preparation-free) pathfinder that just uses Dijkstra's
-/// maneuever.
-#[derive(Serialize, Deserialize)]
-pub enum Pathfinder {
-    Dijkstra,
-    CH(ContractionHierarchyPathfinder),
-}
-
-impl Pathfinder {
-    pub fn pathfind(&self, req: PathRequest, map: &Map) -> Option<Path> {
-        match self {
-            Pathfinder::Dijkstra => dijkstra::pathfind(req, map),
-            Pathfinder::CH(ref p) => p.pathfind(req, map),
-        }
-    }
-    pub fn pathfind_avoiding_lanes(
-        &self,
-        req: PathRequest,
-        avoid: BTreeSet<LaneID>,
-        map: &Map,
-    ) -> Option<Path> {
-        dijkstra::pathfind_avoiding_lanes(req, avoid, map)
-    }
-
-    // TODO Consider returning the walking-only path in the failure case, to avoid wasting work
-    pub fn should_use_transit(
-        &self,
-        map: &Map,
-        start: Position,
-        end: Position,
-    ) -> Option<(BusStopID, Option<BusStopID>, BusRouteID)> {
-        match self {
-            // TODO Implement this
-            Pathfinder::Dijkstra => None,
-            Pathfinder::CH(ref p) => p.should_use_transit(map, start, end),
-        }
-    }
-
-    pub fn apply_edits(&mut self, map: &Map, timer: &mut Timer) {
-        match self {
-            Pathfinder::Dijkstra => {}
-            Pathfinder::CH(ref mut p) => p.apply_edits(map, timer),
-        }
-    }
-}

map_model/src/pathfind/pathfinder.rs

@@ -0,0 +1,57 @@
+use std::collections::BTreeSet;
+
+use serde::{Deserialize, Serialize};
+
+use abstutil::Timer;
+
+use crate::pathfind::ch::ContractionHierarchyPathfinder;
+use crate::pathfind::dijkstra;
+use crate::{BusRouteID, BusStopID, LaneID, Map, Path, PathRequest, Position};
+
+/// Most of the time, prefer using the faster contraction hierarchies. But sometimes, callers can
+/// explicitly opt into a slower (but preparation-free) pathfinder that just uses Dijkstra's
+/// maneuever.
+#[derive(Serialize, Deserialize)]
+pub enum Pathfinder {
+    Dijkstra,
+    CH(ContractionHierarchyPathfinder),
+}
+
+impl Pathfinder {
+    pub fn pathfind(&self, req: PathRequest, map: &Map) -> Option<Path> {
+        match self {
+            Pathfinder::Dijkstra => dijkstra::simple_pathfind(&req, map),
+            Pathfinder::CH(ref p) => p.pathfind(req, map),
+        }
+    }
+
+    pub fn pathfind_avoiding_lanes(
+        &self,
+        req: PathRequest,
+        avoid: BTreeSet<LaneID>,
+        map: &Map,
+    ) -> Option<Path> {
+        dijkstra::pathfind_avoiding_lanes(req, avoid, map)
+    }
+
+    // TODO Consider returning the walking-only path in the failure case, to avoid wasting work
+    pub fn should_use_transit(
+        &self,
+        map: &Map,
+        start: Position,
+        end: Position,
+    ) -> Option<(BusStopID, Option<BusStopID>, BusRouteID)> {
+        match self {
+            // TODO Implement this
+            Pathfinder::Dijkstra => None,
+            Pathfinder::CH(ref p) => p.should_use_transit(map, start, end),
+        }
+    }
+
+    pub fn apply_edits(&mut self, map: &Map, timer: &mut Timer) {
+        match self {
+            Pathfinder::Dijkstra => {}
+            Pathfinder::CH(ref mut p) => p.apply_edits(map, timer),
+        }
+    }
+}