When we can help it, don't shift road polylines twice. #860

apps/game/src/edit/roads.rs

@@ -1026,14 +1026,11 @@ fn draw_drop_position(app: &App, r: RoadID, from: usize, to: usize) -> GeomBatch
     if from == to {
         return batch;
     }
-    let mut width = Distance::ZERO;
     let map = &app.primary.map;
     let road = map.get_r(r);
     let take_num = if from < to { to + 1 } else { to };
-    for l in road.lanes.iter().take(take_num) {
-        width += l.width;
-    }
-    if let Ok(pl) = road.get_left_side().shift_right(width) {
+    let width = road.lanes.iter().take(take_num).map(|x| x.width).sum();
+    if let Ok(pl) = road.shift_from_left_side(width) {
         batch.push(app.cs.selected, pl.make_polygons(OUTLINE_THICKNESS));
     }
     batch

geom/src/polyline.rs

@@ -417,6 +417,22 @@ impl PolyLine {
         self.shift_with_corrections(width)
     }
 
+    /// `self` represents some center, with `total_width`. Logically this shifts left by
+    /// `total_width / 2`, then right by `width_from_left_side`, but without exasperating sharp
+    /// bends.
+    pub fn shift_from_center(
+        &self,
+        total_width: Distance,
+        width_from_left_side: Distance,
+    ) -> Result<PolyLine> {
+        let half_width = total_width / 2.0;
+        if width_from_left_side < half_width {
+            self.shift_left(half_width - width_from_left_side)
+        } else {
+            self.shift_right(width_from_left_side - half_width)
+        }
+    }
+
     // Things to remember about shifting polylines:
     // - the length before and after probably don't match up
     // - the number of points may not match

map_gui/src/render/road.rs

@@ -47,7 +47,7 @@ impl DrawRoad {
                 && pair[0].lane_type.is_for_moving_vehicles()
                 && pair[1].lane_type.is_for_moving_vehicles()
             {
-                let pl = r.get_left_side().must_shift_right(width);
+                let pl = r.shift_from_left_side(width).unwrap();
                 batch.extend(
                     center_line_color,
                     pl.dashed_lines(

map_model/src/objects/movement.rs

@@ -137,8 +137,8 @@ impl Movement {
         }
 
         let mut pl = r
-            .get_left_side()
-            .must_shift_right((leftmost + rightmost) / 2.0);
+            .shift_from_left_side((leftmost + rightmost) / 2.0)
+            .unwrap();
         if self.id.from.dir == Direction::Back {
             pl = pl.reversed();
         }

map_model/src/objects/road.rs

@@ -190,9 +190,9 @@ impl Road {
             .collect()
     }
 
-    /// Gets the left PolyLine of the road
-    pub fn get_left_side(&self) -> PolyLine {
-        self.center_pts.must_shift_left(self.get_half_width())
+    pub fn shift_from_left_side(&self, width_from_left_side: Distance) -> Result<PolyLine> {
+        self.center_pts
+            .shift_from_center(self.get_width(), width_from_left_side)
     }
 
     /// lane must belong to this road. Offset 0 is the centermost lane on each side of a road, then
@@ -516,16 +516,7 @@ impl Road {
     pub(crate) fn recreate_lanes(&mut self, lane_specs_ltr: Vec<LaneSpec>) {
         self.lanes.clear();
 
-        let mut total_width = Distance::ZERO;
-        for lane in &lane_specs_ltr {
-            total_width += lane.width;
-        }
-        // TODO Maybe easier to use the road's "yellow center line" and shift left/right from
-        // there.
-        let road_left_pts = self
-            .center_pts
-            .shift_left(total_width / 2.0)
-            .unwrap_or_else(|_| self.center_pts.clone());
+        let total_width = lane_specs_ltr.iter().map(|x| x.width).sum();
 
         let mut width_so_far = Distance::ZERO;
         for lane in lane_specs_ltr {
@@ -540,18 +531,18 @@ impl Road {
                 (self.dst_i, self.src_i)
             };
 
-            let pl = if let Ok(pl) = road_left_pts.shift_right(width_so_far + (lane.width / 2.0)) {
-                pl
-            } else {
-                error!("{} geometry broken; lane not shifted!", id);
-                road_left_pts.clone()
-            };
+            width_so_far += lane.width / 2.0;
+            let pl = self
+                .center_pts
+                .shift_from_center(total_width, width_so_far)
+                .unwrap_or_else(|_| self.center_pts.clone());
+            width_so_far += lane.width / 2.0;
+
             let lane_center_pts = if lane.dir == Direction::Fwd {
                 pl
             } else {
                 pl.reversed()
             };
-            width_so_far += lane.width;
 
             self.lanes.push(Lane {
                 id,

tests/goldenfiles/prebaked_summaries.json

@@ -4,27 +4,27 @@
     "scenario": "weekday",
     "finished_trips": 76640,
     "cancelled_trips": 0,
-    "total_trip_duration_seconds": 43735836.876801066
+    "total_trip_duration_seconds": 43816297.68860026
   },
   {
     "map": "montlake (in seattle (us))",
     "scenario": "weekday",
-    "finished_trips": 36713,
-    "cancelled_trips": 83,
-    "total_trip_duration_seconds": 18485539.44410009
+    "finished_trips": 36715,
+    "cancelled_trips": 81,
+    "total_trip_duration_seconds": 18514844.62500008
   },
   {
     "map": "parliament (in tehran (ir))",
     "scenario": "random people going to and from work",
     "finished_trips": 29350,
     "cancelled_trips": 16734,
-    "total_trip_duration_seconds": 41822176.78990006
+    "total_trip_duration_seconds": 41860049.65870012
   },
   {
     "map": "sao_miguel_paulista (in sao_paulo (br))",
     "scenario": "Full",
     "finished_trips": 122948,
     "cancelled_trips": 33043,
-    "total_trip_duration_seconds": 103803144.64800021
+    "total_trip_duration_seconds": 103775524.31170104
   }
 ]