Refactor PathRequest creators that go between two buildings. #176 (#404)

fifteen_min/src/isochrone.rs

@@ -4,7 +4,7 @@ use abstutil::MultiMap;
 use geom::{Duration, Polygon};
 use map_gui::tools::{amenity_type, Grid};
 use map_gui::SimpleApp;
-use map_model::{connectivity, BuildingID, Map, PathConstraints, PathRequest};
+use map_model::{connectivity, BuildingID, Map, Path, PathConstraints, PathRequest};
 use widgetry::{Color, Drawable, EventCtx, GeomBatch};
 
 /// Represents the area reachable from a single building.
@@ -51,26 +51,9 @@ impl Isochrone {
         }
     }
 
-    pub fn path_to(&self, map: &Map, destination_id: BuildingID) -> Option<map_model::Path> {
-        let (start, end) = match self.constraints {
-            PathConstraints::Pedestrian => (
-                map.get_b(self.start).sidewalk_pos,
-                map.get_b(destination_id).sidewalk_pos,
-            ),
-            PathConstraints::Bike => (
-                map.get_b(self.start).biking_connection(map)?.0,
-                map.get_b(destination_id).biking_connection(map)?.0,
-            ),
-            _ => unimplemented!("unhandled constraints: {:?}", self.constraints),
-        };
-
-        let path_request = PathRequest {
-            start,
-            end,
-            constraints: self.constraints,
-        };
-
-        map.pathfind(path_request)
+    pub fn path_to(&self, map: &Map, to: BuildingID) -> Option<Path> {
+        let req = PathRequest::between_buildings(map, self.start, to, self.constraints)?;
+        map.pathfind(req)
     }
 }
 

importer/src/soundcast/trips.rs

@@ -56,76 +56,25 @@ fn endpoints(
     // TODO It'd be nice to fix depart_at, trip_time, and trip_dist. Assume constant speed
     // through the trip. But when I last tried this, the distance was way off. :\
 
-    // If this isn't huge_seattle, use the large map to find the real path somebody might take,
-    // then try to match that to a border in the smaller map.
-    let maybe_other_border = if let Some((huge_map, huge_osm_id_to_bldg)) = maybe_huge_map {
-        let maybe_b1 = from
-            .osm_building
-            .and_then(|id| huge_osm_id_to_bldg.get(&id))
-            .cloned();
-        let maybe_b2 = to
-            .osm_building
-            .and_then(|id| huge_osm_id_to_bldg.get(&id))
-            .cloned();
-        if let (Some(b1), Some(b2)) = (maybe_b1, maybe_b2) {
-            // TODO Super rough...
-            let start = if constraints == PathConstraints::Pedestrian {
-                Some(huge_map.get_b(b1).sidewalk_pos)
-            } else {
-                huge_map
-                    .get_b(b1)
-                    .driving_connection(huge_map)
-                    .map(|(pos, _)| pos)
-            };
-            let end = if constraints == PathConstraints::Pedestrian {
-                Some(huge_map.get_b(b2).sidewalk_pos)
-            } else {
-                huge_map
-                    .get_b(b2)
-                    .driving_connection(huge_map)
-                    .map(|(pos, _)| pos)
-            };
-            if let Some(path) = start.and_then(|start| {
-                end.and_then(|end| {
-                    huge_map.pathfind(PathRequest {
-                        start,
-                        end,
-                        constraints,
-                    })
-                })
-            }) {
-                // Do any of the usable borders match the path?
-                // TODO Calculate this once
-                let mut node_id_to_border = HashMap::new();
-                for (i, _) in usable_borders {
-                    node_id_to_border.insert(map.get_i(*i).orig_id, *i);
-                }
-                let mut found_border = None;
-                for step in path.get_steps() {
-                    if let PathStep::Turn(t) = step {
-                        if let Some(i) = node_id_to_border.get(&huge_map.get_i(t.parent).orig_id) {
-                            found_border = Some(*i);
-                            break;
-                        }
-                    }
-                }
-                found_border
-            } else {
-                None
-            }
-        } else {
-            None
-        }
-    } else {
-        None
-    };
-    // Fallback to finding the nearest border with straight-line distance
-    let border_i = maybe_other_border.or_else(|| {
-        usable_borders
-            .iter()
-            .min_by_key(|(_, pt)| pt.fast_dist(border_endpt.pos))
-            .map(|(id, _)| *id)
-    })?;
+    let border_i = maybe_huge_map
+        .and_then(|(huge_map, huge_osm_id_to_bldg)| {
+            other_border(
+                from,
+                to,
+                constraints,
+                huge_map,
+                huge_osm_id_to_bldg,
+                map,
+                usable_borders,
+            )
+        })
+        .or_else(|| {
+            // Fallback to finding the nearest border with straight-line distance
+            usable_borders
+                .iter()
+                .min_by_key(|(_, pt)| pt.fast_dist(border_endpt.pos))
+                .map(|(id, _)| *id)
+        })?;
     let border = TripEndpoint::Border(border_i);
     if let Some(b) = from_bldg {
         Some((TripEndpoint::Bldg(b), border))
@@ -134,6 +83,42 @@ fn endpoints(
     }
 }
 
+// Use the large map to find the real path somebody might take, then try to match that to a border
+// in the smaller map.
+fn other_border(
+    from: &Endpoint,
+    to: &Endpoint,
+    constraints: PathConstraints,
+    huge_map: &Map,
+    huge_osm_id_to_bldg: &HashMap<osm::OsmID, BuildingID>,
+    map: &Map,
+    usable_borders: &Vec<(IntersectionID, LonLat)>,
+) -> Option<IntersectionID> {
+    let b1 = *from
+        .osm_building
+        .and_then(|id| huge_osm_id_to_bldg.get(&id))?;
+    let b2 = *to
+        .osm_building
+        .and_then(|id| huge_osm_id_to_bldg.get(&id))?;
+    let req = PathRequest::between_buildings(huge_map, b1, b2, constraints)?;
+    let path = huge_map.pathfind(req)?;
+
+    // Do any of the usable borders match the path?
+    // TODO Calculate this once
+    let mut node_id_to_border = HashMap::new();
+    for (i, _) in usable_borders {
+        node_id_to_border.insert(map.get_i(*i).orig_id, *i);
+    }
+    for step in path.get_steps() {
+        if let PathStep::Turn(t) = step {
+            if let Some(i) = node_id_to_border.get(&huge_map.get_i(t.parent).orig_id) {
+                return Some(*i);
+            }
+        }
+    }
+    None
+}
+
 fn clip_trips(map: &Map, popdat: &PopDat, huge_map: &Map, timer: &mut Timer) -> Vec<Trip> {
     let maybe_huge_map = if map.get_name().map == "huge_seattle" {
         None

map_model/src/pathfind/mod.rs

@@ -14,8 +14,8 @@ pub use self::dijkstra::{build_graph_for_pedestrians, build_graph_for_vehicles};
 pub use self::driving::driving_cost;
 pub use self::walking::{walking_cost, WalkingNode};
 use crate::{
-    osm, BusRouteID, BusStopID, Lane, LaneID, LaneType, Map, Position, Traversable, TurnID,
-    UberTurn,
+    osm, BuildingID, BusRouteID, BusStopID, Lane, LaneID, LaneType, Map, Position, Traversable,
+    TurnID, UberTurn,
 };
 
 mod ch;
@@ -524,6 +524,38 @@ impl fmt::Display for PathRequest {
     }
 }
 
+impl PathRequest {
+    /// Determines the start and end position to travel between two buildings for a certain mode.
+    /// The path won't cover modality transfers -- if somebody has to walk between the building and
+    /// a parking spot or bikeable position, that won't be captured here.
+    pub fn between_buildings(
+        map: &Map,
+        from: BuildingID,
+        to: BuildingID,
+        constraints: PathConstraints,
+    ) -> Option<PathRequest> {
+        let from = map.get_b(from);
+        let to = map.get_b(to);
+        let (start, end) = match constraints {
+            PathConstraints::Pedestrian => (from.sidewalk_pos, to.sidewalk_pos),
+            PathConstraints::Bike => (from.biking_connection(map)?.0, to.biking_connection(map)?.0),
+            PathConstraints::Car => (
+                from.driving_connection(map)?.0,
+                to.driving_connection(map)?.0,
+            ),
+            // These two aren't useful here. A pedestrian using transit would pass in
+            // PathConstraints::Pedestrian. There's no reason yet to find a route for a bus or
+            // train to travel between buildings.
+            PathConstraints::Bus | PathConstraints::Train => unimplemented!(),
+        };
+        Some(PathRequest {
+            start,
+            end,
+            constraints,
+        })
+    }
+}
+
 fn validate_continuity(map: &Map, steps: &Vec<PathStep>) {
     if steps.is_empty() {
         panic!("Empty path");

sim/src/make/activity_model.rs

@@ -219,15 +219,15 @@ fn create_prole(
         (TripEndpoint::Bldg(home_bldg), TripEndpoint::Bldg(work_bldg)) => {
             // Decide mode based on walking distance. If the buildings aren't connected,
             // probably a bug in importing; just skip this person.
-            let dist = if let Some(dist) = map
-                .pathfind(PathRequest {
-                    start: map.get_b(*home_bldg).sidewalk_pos,
-                    end: map.get_b(*work_bldg).sidewalk_pos,
-                    constraints: PathConstraints::Pedestrian,
-                })
-                .map(|p| p.total_length())
+            let dist = if let Some(path) = PathRequest::between_buildings(
+                map,
+                *home_bldg,
+                *work_bldg,
+                PathConstraints::Pedestrian,
+            )
+            .and_then(|req| map.pathfind(req))
             {
-                dist
+                path.total_length()
             } else {
                 return Err("no path found".into());
             };