Use LaneSpec in map edits, letting width of each lane be modified too. …

…#597

game/src/edit/lanes.rs

@@ -246,6 +246,6 @@ fn reverse_lane(map: &Map, l: LaneID) -> EditCmd {
     let r = map.get_parent(l);
     let idx = r.offset(l);
     map.edit_road_cmd(r.id, |new| {
-        new.lanes_ltr[idx].1 = new.lanes_ltr[idx].1.opposite();
+        new.lanes_ltr[idx].dir = new.lanes_ltr[idx].dir.opposite();
     })
 }

game/src/edit/roads.rs

@@ -1,5 +1,5 @@
 use map_gui::tools::PromptInput;
-use map_model::{Direction, LaneType, Road};
+use map_model::{Direction, LaneSpec, LaneType, Road};
 use widgetry::{EventCtx, State};
 
 use crate::app::{App, Transition};
@@ -47,7 +47,7 @@ fn lanes_to_string(road: &Road) -> String {
     string
 }
 
-fn string_to_lanes(string: String) -> Vec<(LaneType, Direction)> {
+fn string_to_lanes(string: String) -> Vec<LaneSpec> {
     let mut lanes = Vec::new();
     let mut dir = Direction::Back;
     for x in string.chars() {
@@ -60,7 +60,11 @@ fn string_to_lanes(string: String) -> Vec<(LaneType, Direction)> {
             .find(|(_, code)| *code == x)
             .unwrap()
             .0;
-        lanes.push((lt, dir));
+        lanes.push(LaneSpec {
+            lt,
+            dir,
+            width: map_model::NORMAL_LANE_THICKNESS,
+        });
     }
     lanes
 }

game/src/edit/validate.rs

@@ -121,7 +121,7 @@ pub fn try_change_lt(
     let cmd = {
         let r = map.get_l(l).parent;
         map.edit_road_cmd(r, |new| {
-            new.lanes_ltr[map.get_r(r).offset(l)].0 = new_lt;
+            new.lanes_ltr[map.get_r(r).offset(l)].lt = new_lt;
         })
     };
     edits.commands.push(cmd.clone());

map_gui/src/render/lane.rs

@@ -26,10 +26,6 @@ impl DrawLane {
         }
     }
 
-    pub fn clear_rendering(&mut self) {
-        *self.draw_default.borrow_mut() = None;
-    }
-
     pub fn render<P: AsRef<Prerender>>(&self, prerender: &P, app: &dyn AppLike) -> GeomBatch {
         let map = app.map();
         let lane = map.get_l(self.id);

map_model/src/edits/compat.rs

@@ -9,7 +9,8 @@ use geom::Speed;
 
 use crate::raw::OriginalRoad;
 use crate::{
-    osm, AccessRestrictions, Direction, EditCmd, EditRoad, LaneType, Map, PermanentMapEdits, RoadID,
+    osm, AccessRestrictions, Direction, EditCmd, EditRoad, LaneSpec, LaneType, Map,
+    PermanentMapEdits, RoadID,
 };
 
 /// When the PermanentMapEdits format changes, add a transformation here to automatically convert
@@ -89,6 +90,13 @@ pub fn upgrade(mut value: Value, map: &Map) -> Result<PermanentMapEdits> {
             .unwrap()
             .insert("version".to_string(), Value::Number(8.into()));
     }
+    if value["version"] == Value::Number(8.into()) {
+        fix_lane_widths(&mut value, map)?;
+        value
+            .as_object_mut()
+            .unwrap()
+            .insert("version".to_string(), Value::Number(9.into()));
+    }
 
     abstutil::from_json(&value.to_string().into_bytes())
 }
@@ -190,10 +198,10 @@ fn fix_old_lane_cmds(value: &mut Value, map: &Map) -> Result<()> {
             let obj: ChangeLaneType = serde_json::from_value(obj).unwrap();
             let (r, idx) = obj.id.lookup(map)?;
             let road = modified.entry(r).or_insert_with(|| map.get_r_edit(r));
-            if road.lanes_ltr[idx].0 != obj.orig_lt {
+            if road.lanes_ltr[idx].lt != obj.orig_lt {
                 bail!("{:?} lane type has changed", obj);
             }
-            road.lanes_ltr[idx].0 = obj.lt;
+            road.lanes_ltr[idx].lt = obj.lt;
         } else if let Some(obj) = cmd.remove("ReverseLane") {
             let obj: ReverseLane = serde_json::from_value(obj).unwrap();
             let (r, idx) = obj.l.lookup(map)?;
@@ -206,10 +214,10 @@ fn fix_old_lane_cmds(value: &mut Value, map: &Map) -> Result<()> {
             } else {
                 bail!("{:?}'s road doesn't point to dst_i at all", obj);
             };
-            if road.lanes_ltr[idx].1 == edits_dir {
+            if road.lanes_ltr[idx].dir == edits_dir {
                 bail!("{:?}'s road already points to dst_i", obj);
             }
-            road.lanes_ltr[idx].1 = edits_dir;
+            road.lanes_ltr[idx].dir = edits_dir;
         } else if let Some(obj) = cmd.remove("ChangeSpeedLimit") {
             let obj: ChangeSpeedLimit = serde_json::from_value(obj).unwrap();
             let r = map.find_r_by_osm_id(obj.id)?;
@@ -342,6 +350,44 @@ fn fix_city_name(value: &mut Value) {
     }
 }
 
+// 0e09c1decfece370eff72d3433ae00b4aff3332f added lane width to EditRoad.
+fn fix_lane_widths(value: &mut Value, map: &Map) -> Result<()> {
+    for orig in value.as_object_mut().unwrap()["commands"]
+        .as_array_mut()
+        .unwrap()
+    {
+        let cmd = orig.as_object_mut().unwrap();
+        if let Some(cmd) = cmd.get_mut("ChangeRoad") {
+            let road_id: OriginalRoad = serde_json::from_value(cmd["r"].clone()).unwrap();
+            let road = map.get_r(map.find_r_by_osm_id(road_id)?);
+            let cmd = cmd.as_object_mut().unwrap();
+
+            for key in vec!["old", "new"] {
+                let mut lanes_ltr = Vec::new();
+                for (idx, mut pair) in cmd[key]["lanes_ltr"]
+                    .as_array_mut()
+                    .unwrap()
+                    .drain(..)
+                    .enumerate()
+                {
+                    let pair = pair.as_array_mut().unwrap();
+                    let lt: LaneType = serde_json::from_value(pair[0].clone()).unwrap();
+                    let dir: Direction = serde_json::from_value(pair[1].clone()).unwrap();
+                    lanes_ltr.push(LaneSpec {
+                        lt,
+                        dir,
+                        // Before this commit, lane widths weren't modifiable, so this lookup works
+                        // for both "old" and "new".
+                        width: map.get_l(road.lanes_ltr()[idx].0).width,
+                    });
+                }
+                cmd[key]["lanes_ltr"] = serde_json::to_value(lanes_ltr).unwrap();
+            }
+        }
+    }
+    Ok(())
+}
+
 // These're old structs used in fix_old_lane_cmds.
 #[derive(Debug, Deserialize)]
 struct OriginalLane {

map_model/src/edits/mod.rs

@@ -12,10 +12,9 @@ use geom::{Speed, Time};
 
 pub use self::perma::PermanentMapEdits;
 use crate::make::initial::lane_specs::get_lane_specs_ltr;
-use crate::make::initial::LaneSpec;
 use crate::{
-    connectivity, AccessRestrictions, BusRouteID, ControlStopSign, ControlTrafficSignal, Direction,
-    IntersectionID, IntersectionType, LaneID, LaneType, Map, MapConfig, PathConstraints,
+    connectivity, AccessRestrictions, BusRouteID, ControlStopSign, ControlTrafficSignal,
+    IntersectionID, IntersectionType, LaneID, LaneSpec, Map, MapConfig, PathConstraints,
     Pathfinder, Road, RoadID, TurnID, Zone,
 };
 
@@ -57,18 +56,15 @@ pub enum EditIntersection {
 
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
 pub struct EditRoad {
-    pub lanes_ltr: Vec<(LaneType, Direction)>,
+    pub lanes_ltr: Vec<LaneSpec>,
     pub speed_limit: Speed,
     pub access_restrictions: AccessRestrictions,
 }
 
 impl EditRoad {
     pub fn get_orig_from_osm(r: &Road, cfg: &MapConfig) -> EditRoad {
         EditRoad {
-            lanes_ltr: get_lane_specs_ltr(&r.osm_tags, cfg)
-                .into_iter()
-                .map(|spec| (spec.lt, spec.dir))
-                .collect(),
+            lanes_ltr: get_lane_specs_ltr(&r.osm_tags, cfg),
             speed_limit: r.speed_limit_from_osm(),
             access_restrictions: r.access_restrictions_from_osm(),
         }
@@ -77,13 +73,17 @@ impl EditRoad {
     fn diff(&self, other: &EditRoad) -> Vec<String> {
         let mut lt = 0;
         let mut dir = 0;
-        for ((lt1, dir1), (lt2, dir2)) in self.lanes_ltr.iter().zip(other.lanes_ltr.iter()) {
-            if lt1 != lt2 {
+        let mut width = 0;
+        for (spec1, spec2) in self.lanes_ltr.iter().zip(other.lanes_ltr.iter()) {
+            if spec1.lt != spec2.lt {
                 lt += 1;
             }
-            if dir1 != dir2 {
+            if spec1.dir != spec2.dir {
                 dir += 1;
             }
+            if spec1.width != spec2.width {
+                width += 1;
+            }
         }
 
         let mut changes = Vec::new();
@@ -95,7 +95,12 @@ impl EditRoad {
         if dir == 1 {
             changes.push(format!("1 lane reversal"));
         } else if dir > 1 {
-            changes.push(format!("{} lane reversal", dir));
+            changes.push(format!("{} lane reversals", dir));
+        }
+        if width == 1 {
+            changes.push(format!("1 lane width"));
+        } else {
+            changes.push(format!("{} lane widths", width));
         }
         if self.speed_limit != other.speed_limit {
             changes.push(format!("speed limit"));
@@ -254,9 +259,9 @@ impl MapEdits {
                     // unclear -- just mark the entire road.
                     roads.insert(r.id);
                 } else {
-                    for (idx, (lt, dir)) in orig.lanes_ltr.into_iter().enumerate() {
-                        if lanes_ltr[idx].1 != dir || lanes_ltr[idx].2 != lt {
-                            lanes.insert(lanes_ltr[idx].0);
+                    for ((l, dir, lt), spec) in lanes_ltr.into_iter().zip(orig.lanes_ltr.iter()) {
+                        if dir != spec.dir || lt != spec.lt || map.get_l(l).width != spec.width {
+                            lanes.insert(l);
                         }
                     }
                 }
@@ -451,29 +456,27 @@ fn recalculate_turns(id: IntersectionID, map: &mut Map, effects: &mut EditEffect
     }
 }
 
-fn modify_lanes(
-    map: &mut Map,
-    r: RoadID,
-    lanes_ltr: Vec<(LaneType, Direction)>,
-    effects: &mut EditEffects,
-) {
+fn modify_lanes(map: &mut Map, r: RoadID, lanes_ltr: Vec<LaneSpec>, effects: &mut EditEffects) {
     let road = &mut map.roads[r.0];
 
     // TODO Widening roads is still experimental. If we're just modifying lane types, preserve
     // LaneIDs.
     if road.lanes_ltr.len() == lanes_ltr.len() {
-        for (idx, (lt, dir)) in lanes_ltr.into_iter().enumerate() {
+        for (idx, spec) in lanes_ltr.into_iter().enumerate() {
             let lane = map.lanes.get_mut(&road.lanes_ltr[idx].0).unwrap();
-            road.lanes_ltr[idx].2 = lt;
-            lane.lane_type = lt;
+            road.lanes_ltr[idx].2 = spec.lt;
+            lane.lane_type = spec.lt;
 
             // Direction change?
-            if road.lanes_ltr[idx].1 != dir {
-                road.lanes_ltr[idx].1 = dir;
+            if road.lanes_ltr[idx].1 != spec.dir {
+                road.lanes_ltr[idx].1 = spec.dir;
                 std::mem::swap(&mut lane.src_i, &mut lane.dst_i);
                 lane.lane_center_pts = lane.lane_center_pts.reversed();
-                lane.dir = dir;
+                lane.dir = spec.dir;
             }
+
+            // TODO If width is changing and the number of lanes isn't, we'll ignore the width
+            // change. Don't use this old code-path for that!
         }
         return;
     }
@@ -494,17 +497,7 @@ fn modify_lanes(
     // This approach creates gaps in the lane ID space, since it deletes the contiguous block of a
     // road's lanes, then creates it again at the end. If this winds up mattering, we could use
     // different approaches for "filling in the gaps."
-    let mut lane_specs_ltr = Vec::new();
-    for (lt, dir) in lanes_ltr {
-        lane_specs_ltr.push(LaneSpec {
-            lt,
-            dir,
-            // TODO Plumb in width (an entire LaneSpec, in fact) through edits directly. Then we
-            // can preserve existing lane width and/or allow modifying width.
-            width: crate::NORMAL_LANE_THICKNESS,
-        });
-    }
-    let new_lanes = road.create_lanes(lane_specs_ltr, &mut map.lane_id_counter);
+    let new_lanes = road.create_lanes(lanes_ltr, &mut map.lane_id_counter);
     for lane in &new_lanes {
         road.lanes_ltr.push((lane.id, lane.dir, lane.lane_type));
     }
@@ -548,7 +541,11 @@ impl Map {
             lanes_ltr: r
                 .lanes_ltr()
                 .into_iter()
-                .map(|(_, dir, lt)| (lt, dir))
+                .map(|(l, dir, lt)| LaneSpec {
+                    lt,
+                    dir,
+                    width: self.get_l(l).width,
+                })
                 .collect(),
             speed_limit: r.speed_limit,
             access_restrictions: r.access_restrictions.clone(),

map_model/src/edits/perma.rs

@@ -138,7 +138,7 @@ impl MapEdits {
             map_name: map.get_name().clone(),
             edits_name: self.edits_name.clone(),
             // Increase this every time there's a schema change
-            version: 8,
+            version: 9,
             proposal_description: self.proposal_description.clone(),
             proposal_link: self.proposal_link.clone(),
             commands: self.commands.iter().map(|cmd| cmd.to_perma(map)).collect(),

map_model/src/lib.rs

@@ -45,7 +45,7 @@ pub use crate::objects::building::{
 };
 pub use crate::objects::bus_stop::{BusRoute, BusRouteID, BusStop, BusStopID};
 pub use crate::objects::intersection::{Intersection, IntersectionID, IntersectionType};
-pub use crate::objects::lane::{Lane, LaneID, LaneType, PARKING_LOT_SPOT_LENGTH};
+pub use crate::objects::lane::{Lane, LaneID, LaneSpec, LaneType, PARKING_LOT_SPOT_LENGTH};
 pub use crate::objects::parking_lot::{ParkingLot, ParkingLotID};
 pub use crate::objects::road::{DirectedRoadID, Direction, Road, RoadID};
 pub use crate::objects::stop_signs::{ControlStopSign, RoadWithStopSign};

map_model/src/make/initial/lane_specs.rs

@@ -2,20 +2,12 @@
 use std::iter;
 
 use abstutil::Tags;
-use geom::Distance;
 
 use crate::{
-    osm, Direction, DrivingSide, LaneType, MapConfig, NORMAL_LANE_THICKNESS,
+    osm, Direction, DrivingSide, LaneSpec, LaneType, MapConfig, NORMAL_LANE_THICKNESS,
     SERVICE_ROAD_LANE_THICKNESS, SHOULDER_THICKNESS, SIDEWALK_THICKNESS,
 };
 
-#[derive(PartialEq)]
-pub struct LaneSpec {
-    pub lt: LaneType,
-    pub dir: Direction,
-    pub width: Distance,
-}
-
 fn fwd(lt: LaneType) -> LaneSpec {
     LaneSpec {
         lt,

map_model/src/make/initial/mod.rs

@@ -9,9 +9,8 @@ use abstutil::{Tags, Timer};
 use geom::{Bounds, Circle, Distance, PolyLine, Polygon, Pt2D};
 
 pub use self::geometry::intersection_polygon;
-pub use crate::make::initial::lane_specs::LaneSpec;
 use crate::raw::{OriginalRoad, RawMap, RawRoad};
-use crate::{osm, IntersectionType, MapConfig};
+use crate::{osm, IntersectionType, LaneSpec, MapConfig};
 
 mod geometry;
 pub mod lane_specs;

map_model/src/objects/lane.rs

@@ -125,6 +125,13 @@ pub struct Lane {
     pub biking_blackhole: bool,
 }
 
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+pub struct LaneSpec {
+    pub lt: LaneType,
+    pub dir: Direction,
+    pub width: Distance,
+}
+
 impl Lane {
     // TODO most of these are wrappers; stop doing this?
     pub fn first_pt(&self) -> Pt2D {

map_model/src/objects/road.rs

@@ -8,11 +8,10 @@ use serde::{Deserialize, Serialize};
 use abstutil::{deserialize_usize, serialize_usize, Tags};
 use geom::{Distance, PolyLine, Polygon, Speed};
 
-use crate::make::initial::LaneSpec;
 use crate::raw::{OriginalRoad, RestrictionType};
 use crate::{
-    osm, AccessRestrictions, BusStopID, DrivingSide, IntersectionID, Lane, LaneID, LaneType, Map,
-    PathConstraints, Zone,
+    osm, AccessRestrictions, BusStopID, DrivingSide, IntersectionID, Lane, LaneID, LaneSpec,
+    LaneType, Map, PathConstraints, Zone,
 };
 
 #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, PartialOrd, Ord, Serialize, Deserialize)]