From d00e7f7bca88efef17360a6be3a2a5da20aed379 Mon Sep 17 00:00:00 2001 From: Dustin Carlino Date: Wed, 16 Dec 2020 16:09:11 -0800 Subject: [PATCH] Simplify path trace API -- nobody needs dist_ahead. I think that was originally meant to be a performance trick to only draw the next little bit of a route --- fifteen_min/src/viewer.rs | 2 +- game/src/debug/mod.rs | 2 +- game/src/info/building.rs | 2 +- game/src/info/trip.rs | 2 +- .../sandbox/dashboards/generic_trip_table.rs | 4 +- game/src/sandbox/gameplay/freeform.rs | 4 +- game/src/sandbox/misc_tools.rs | 3 +- map_model/src/pathfind/mod.rs | 97 ++++++------------- sim/src/mechanics/driving.rs | 10 +- sim/src/mechanics/walking.rs | 10 +- sim/src/sim/queries.rs | 11 +-- 11 files changed, 47 insertions(+), 100 deletions(-) diff --git a/fifteen_min/src/viewer.rs b/fifteen_min/src/viewer.rs index a39188e3a4..ff294661e9 100644 --- a/fifteen_min/src/viewer.rs +++ b/fifteen_min/src/viewer.rs @@ -267,7 +267,7 @@ impl HoverOnBuilding { let mut batch = GeomBatch::new(); if let Some(polyline) = isochrone .path_to(&app.map, hover_id) - .and_then(|path| path.trace(&app.map, Distance::ZERO, None)) + .and_then(|path| path.trace(&app.map, Distance::ZERO)) { let dashed_lines = polyline.dashed_lines( Distance::meters(0.75 * scale_factor), diff --git a/game/src/debug/mod.rs b/game/src/debug/mod.rs index 5adc4b20aa..38cb8591d3 100644 --- a/game/src/debug/mod.rs +++ b/game/src/debug/mod.rs @@ -430,7 +430,7 @@ fn calc_all_routes(ctx: &EventCtx, app: &mut App) -> (usize, Drawable) { Parallelism::Fastest, agents, |id| { - sim.trace_route(id, map, None) + sim.trace_route(id, map) .map(|trace| trace.make_polygons(NORMAL_LANE_THICKNESS)) }, ) { diff --git a/game/src/info/building.rs b/game/src/info/building.rs index 75429b668a..08110d27bf 100644 --- a/game/src/info/building.rs +++ b/game/src/info/building.rs @@ -74,7 +74,7 @@ pub fn info(ctx: &mut EventCtx, app: &App, details: &mut Details, id: BuildingID .primary .sim .walking_path_to_nearest_parking_spot(&app.primary.map, id) - .and_then(|path| path.trace(&app.primary.map, path.get_req().start.dist_along(), None)) + .and_then(|path| path.trace(&app.primary.map, path.get_req().start.dist_along())) { let color = app.cs.parking_trip; // TODO But this color doesn't show up well against the info panel... diff --git a/game/src/info/trip.rs b/game/src/info/trip.rs index d3f1d985f2..1f6c3935c9 100644 --- a/game/src/info/trip.rs +++ b/game/src/info/trip.rs @@ -746,7 +746,7 @@ fn make_trip_details( // This is expensive, so cache please if idx == open_trip.cached_routes.len() { if let Some(trace) = - path.trace(map_for_pathfinding, path.get_req().start.dist_along(), None) + path.trace(map_for_pathfinding, path.get_req().start.dist_along()) { open_trip.cached_routes.push(Some(( trace.make_polygons(Distance::meters(10.0)), diff --git a/game/src/sandbox/dashboards/generic_trip_table.rs b/game/src/sandbox/dashboards/generic_trip_table.rs index 68178010b3..a842dd90a5 100644 --- a/game/src/sandbox/dashboards/generic_trip_table.rs +++ b/game/src/sandbox/dashboards/generic_trip_table.rs @@ -147,9 +147,7 @@ fn preview_route(g: &mut GfxCtx, app: &App, id: TripID) -> GeomBatch { .get_trip_phases(id, &app.primary.map) { if let Some(path) = &p.path { - if let Some(trace) = - path.trace(&app.primary.map, path.get_req().start.dist_along(), None) - { + if let Some(trace) = path.trace(&app.primary.map, path.get_req().start.dist_along()) { batch.push( color_for_trip_phase(app, p.phase_type), trace.make_polygons(Distance::meters(20.0)), diff --git a/game/src/sandbox/gameplay/freeform.rs b/game/src/sandbox/gameplay/freeform.rs index ba32420ef4..710647be65 100644 --- a/game/src/sandbox/gameplay/freeform.rs +++ b/game/src/sandbox/gameplay/freeform.rs @@ -322,7 +322,7 @@ impl State for AgentSpawner { { self.goal = Some(( to, - path.trace(&app.primary.map, path.get_req().start.dist_along(), None) + path.trace(&app.primary.map, path.get_req().start.dist_along()) .map(|pl| pl.make_polygons(NORMAL_LANE_THICKNESS)), )); } else { @@ -387,7 +387,7 @@ impl State for AgentSpawner { { self.goal = Some(( hovering, - path.trace(&app.primary.map, path.get_req().start.dist_along(), None) + path.trace(&app.primary.map, path.get_req().start.dist_along()) .map(|pl| pl.make_polygons(NORMAL_LANE_THICKNESS)), )); } else { diff --git a/game/src/sandbox/misc_tools.rs b/game/src/sandbox/misc_tools.rs index c78aebb452..4d1b313ed4 100644 --- a/game/src/sandbox/misc_tools.rs +++ b/game/src/sandbox/misc_tools.rs @@ -44,8 +44,7 @@ impl RoutePreview { // Only draw the preview when zoomed in. If we wanted to do this unzoomed, we'd // want a different style; the dashed lines don't show up well. if zoomed { - if let Some(trace) = app.primary.sim.trace_route(agent, &app.primary.map, None) - { + if let Some(trace) = app.primary.sim.trace_route(agent, &app.primary.map) { batch.extend( app.cs.route, trace.dashed_lines( diff --git a/map_model/src/pathfind/mod.rs b/map_model/src/pathfind/mod.rs index a54599d546..d3eeb5db33 100644 --- a/map_model/src/pathfind/mod.rs +++ b/map_model/src/pathfind/mod.rs @@ -52,14 +52,14 @@ impl PathStep { self.as_traversable().as_turn() } - // Returns dist_remaining. start is relative to the start of the actual geometry -- so from the - // lane's real start for ContraflowLane. - fn slice( + // start is relative to the start of the actual geometry -- so from the lane's real start for + // ContraflowLane. + fn exact_slice( &self, map: &Map, start: Distance, dist_ahead: Option, - ) -> Result<(PolyLine, Distance), String> { + ) -> Result { if let Some(d) = dist_ahead { if d < Distance::ZERO { panic!("Negative dist_ahead?! {}", d); @@ -73,26 +73,26 @@ impl PathStep { PathStep::Lane(id) => { let pts = &map.get_l(*id).lane_center_pts; if let Some(d) = dist_ahead { - pts.slice(start, start + d) + pts.maybe_exact_slice(start, start + d) } else { - pts.slice(start, pts.length()) + pts.maybe_exact_slice(start, pts.length()) } } PathStep::ContraflowLane(id) => { let pts = map.get_l(*id).lane_center_pts.reversed(); let reversed_start = pts.length() - start; if let Some(d) = dist_ahead { - pts.slice(reversed_start, reversed_start + d) + pts.maybe_exact_slice(reversed_start, reversed_start + d) } else { - pts.slice(reversed_start, pts.length()) + pts.maybe_exact_slice(reversed_start, pts.length()) } } PathStep::Turn(id) => { let pts = &map.get_t(*id).geom; if let Some(d) = dist_ahead { - pts.slice(start, start + d) + pts.maybe_exact_slice(start, start + d) } else { - pts.slice(start, pts.length()) + pts.maybe_exact_slice(start, pts.length()) } } } @@ -321,73 +321,45 @@ impl Path { self.steps[self.steps.len() - 1] } - /// dist_ahead is unlimited when None. Note this starts at the beginning (or end, for some - /// walking paths) of the first lane, not accounting for the original request's start distance. - pub fn trace( - &self, - map: &Map, - start_dist: Distance, - dist_ahead: Option, - ) -> Option { - let mut pts_so_far: Option = None; - let mut dist_remaining = dist_ahead; + /// Traces along the path from a specified distance along the first step until the end. + pub fn trace(&self, map: &Map, start_dist: Distance) -> Option { let orig_end_dist = self.orig_req.end.dist_along(); if self.steps.len() == 1 { - let dist = if start_dist < orig_end_dist { + let dist_ahead = if start_dist < orig_end_dist { orig_end_dist - start_dist } else { start_dist - orig_end_dist }; - if let Some(d) = dist_remaining { - if dist < d { - dist_remaining = Some(dist); - } - } else { - dist_remaining = Some(dist); - } + + // Why might this fail? It's possible there are paths on their last step that're + // effectively empty, because they're a 0-length turn, or something like a pedestrian + // crossing a front path and immediately getting on a bike. + return self.steps[0] + .exact_slice(map, start_dist, Some(dist_ahead)) + .ok(); } - // Special case the first step. - if let Ok((pts, dist)) = self.steps[0].slice(map, start_dist, dist_remaining) { + let mut pts_so_far: Option = None; + + // Special case the first step with start_dist. + if let Ok(pts) = self.steps[0].exact_slice(map, start_dist, None) { pts_so_far = Some(pts); - if dist_remaining.is_some() { - dist_remaining = Some(dist); - } - } - - if self.steps.len() == 1 { - // It's possible there are paths on their last step that're effectively empty, because - // they're a 0-length turn, or something like a pedestrian crossing a front path and - // immediately getting on a bike. - return pts_so_far; } // Crunch through the intermediate steps, as long as we can. for i in 1..self.steps.len() { - if let Some(d) = dist_remaining { - if d <= Distance::ZERO { - // We know there's at least some geometry if we made it here, so unwrap to - // verify that understanding. - return Some(pts_so_far.unwrap()); - } - } - // If we made it to the last step, maybe use the end_dist. - if i == self.steps.len() - 1 { - let end_dist = match self.steps[i] { + // Restrict the last step's slice + let dist_ahead = if i == self.steps.len() - 1 { + Some(match self.steps[i] { PathStep::ContraflowLane(l) => { map.get_l(l).lane_center_pts.reversed().length() - orig_end_dist } _ => orig_end_dist, - }; - if let Some(d) = dist_remaining { - if end_dist < d { - dist_remaining = Some(end_dist); - } - } else { - dist_remaining = Some(end_dist); - } - } + }) + } else { + None + }; let start_dist_this_step = match self.steps[i] { // TODO Length of a PolyLine can slightly change when points are reversed! That @@ -395,9 +367,7 @@ impl Path { PathStep::ContraflowLane(l) => map.get_l(l).lane_center_pts.reversed().length(), _ => Distance::ZERO, }; - if let Ok((new_pts, dist)) = - self.steps[i].slice(map, start_dist_this_step, dist_remaining) - { + if let Ok(new_pts) = self.steps[i].exact_slice(map, start_dist_this_step, dist_ahead) { if pts_so_far.is_some() { match pts_so_far.unwrap().extend(new_pts) { Ok(new) => { @@ -411,9 +381,6 @@ impl Path { } else { pts_so_far = Some(new_pts); } - if dist_remaining.is_some() { - dist_remaining = Some(dist); - } } } diff --git a/sim/src/mechanics/driving.rs b/sim/src/mechanics/driving.rs index 29b8dd5f09..0afe57e6d4 100644 --- a/sim/src/mechanics/driving.rs +++ b/sim/src/mechanics/driving.rs @@ -1047,16 +1047,10 @@ impl DrivingSimState { .collect() } - pub fn trace_route( - &self, - now: Time, - id: CarID, - map: &Map, - dist_ahead: Option, - ) -> Option { + pub fn trace_route(&self, now: Time, id: CarID, map: &Map) -> Option { let car = self.cars.get(&id)?; let front = self.get_car_front(now, car); - car.router.get_path().trace(map, front, dist_ahead) + car.router.get_path().trace(map, front) } pub fn percent_along_route(&self, id: CarID) -> f64 { diff --git a/sim/src/mechanics/walking.rs b/sim/src/mechanics/walking.rs index 78a0c4587a..cb80175ed1 100644 --- a/sim/src/mechanics/walking.rs +++ b/sim/src/mechanics/walking.rs @@ -398,18 +398,12 @@ impl WalkingSimState { } } - pub fn trace_route( - &self, - now: Time, - id: PedestrianID, - map: &Map, - dist_ahead: Option, - ) -> Option { + pub fn trace_route(&self, now: Time, id: PedestrianID, map: &Map) -> Option { let p = self.peds.get(&id)?; let body_radius = SIDEWALK_THICKNESS / 4.0; let dist = (p.get_dist_along(now, map) + body_radius) .min(p.path.current_step().as_traversable().length(map)); - p.path.trace(map, dist, dist_ahead) + p.path.trace(map, dist) } pub fn get_path(&self, id: PedestrianID) -> Option<&Path> { diff --git a/sim/src/sim/queries.rs b/sim/src/sim/queries.rs index d61284ed48..3727ab2fc3 100644 --- a/sim/src/sim/queries.rs +++ b/sim/src/sim/queries.rs @@ -215,15 +215,10 @@ impl Sim { self.driving.get_all_driving_paths() } - pub fn trace_route( - &self, - id: AgentID, - map: &Map, - dist_ahead: Option, - ) -> Option { + pub fn trace_route(&self, id: AgentID, map: &Map) -> Option { match id { - AgentID::Car(car) => self.driving.trace_route(self.time, car, map, dist_ahead), - AgentID::Pedestrian(ped) => self.walking.trace_route(self.time, ped, map, dist_ahead), + AgentID::Car(car) => self.driving.trace_route(self.time, car, map), + AgentID::Pedestrian(ped) => self.walking.trace_route(self.time, ped, map), AgentID::BusPassenger(_, _) => None, } }