Get rid of the transformation, and instead update geometry everywhere…

… that's necessary. #136

Introduces some big regressions from effectively detecting and short
roads earlier.

docs/how_it_works.md

@@ -49,6 +49,11 @@ For each intersection, this calculates vehicle movements (at the granularity of
 
 It first calculates trimmed intersection geometry for the two intersections. On each connected road (besides the short one being collapsed, of course), we store the trimming distance in `trim_roads_for_merging`, so that later the intersection geometry algorithm can follow a special case for the single merged intersection.
 
+### update_geometry
+
+This follows [this algorithm](https://a-b-street.github.io/docs/tech/map/geometry/index.html) (outdated!). Road center-lines get "trimmed" back from the intersection, and the intersection gets a polygon.
+
+This process trims every road on both ends. Sometimes the trims overlap and the road disappears entirely. In that case, we mark the road as `internal_junction_road` and remove it entirely with a later pass of `CollapseShortRoads`.
 
 ## Transformations
 
@@ -103,9 +108,3 @@ This is a hack to make dual carriageways drawn close together in OSM look half-r
 ### MergeDualCarriageways (experimental)
 
 TODO. Explain branches and bridges.
-
-### GenerateIntersectionGeometry
-
-Along with `ClassifyIntersections`, this is the most important transformation (and maybe should be expressed differently). For every intersection, it runs through [this algorithm](https://a-b-street.github.io/docs/tech/map/geometry/index.html). Road center-lines get "trimmed" back from the intersection, and the intersection gets a polygon.
-
-This process trims every road on both ends. Sometimes the trims overlap and the road disappears entirely. In that case, we mark the road as `internal_junction_road` and remove it entirely with a later pass of `CollapseShortRoads`.

osm2streets-js/src/lib.rs

@@ -1,4 +1,4 @@
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, BTreeSet};
 
 use abstutil::{Tags, Timer};
 use geom::{Distance, LonLat, PolyLine, Polygon};
@@ -70,24 +70,9 @@ impl JsStreetNetwork {
             transformations.push(Transformation::SnapCycleways);
             transformations.push(Transformation::TrimDeadendCycleways);
             transformations.push(Transformation::CollapseDegenerateIntersections);
-            // TODO Indeed it'd be much nicer to recalculate this as the above transformations
-            // modify things
-            transformations.push(Transformation::GenerateIntersectionGeometry);
         }
         if input.debug_each_step {
             street_network.apply_transformations_stepwise_debugging(transformations, &mut timer);
-
-            // For all but the last step, generate intersection geometry, so these
-            // intermediate states can be rendered.
-            // TODO Revisit this -- rendering should use untrimmed geometry, or an initial guess of
-            // trimmed geometry.
-            let mut steps = street_network.debug_steps.borrow_mut();
-            for i in 0..steps.len() - 1 {
-                steps[i].streets.apply_transformations(
-                    vec![Transformation::GenerateIntersectionGeometry],
-                    &mut timer,
-                );
-            }
         } else {
             street_network.apply_transformations(transformations, &mut timer);
         }
@@ -198,22 +183,23 @@ impl JsStreetNetwork {
         abstutil::to_json(&polygon.to_geojson(Some(&self.inner.gps_bounds)))
     }
 
-    /// Modifies all affected roads and only reruns `Transformation::GenerateIntersectionGeometry`.
+    /// Modifies all affected roads
     #[wasm_bindgen(js_name = overwriteOsmTagsForWay)]
     pub fn overwrite_osm_tags_for_way(&mut self, id: i64, tags: String) {
         let id = osm::WayID(id);
         let tags: Tags = abstutil::from_json(tags.as_bytes()).unwrap();
 
+        let mut intersections = BTreeSet::new();
         for road in self.inner.roads.values_mut() {
             if road.from_osm_way(id) {
                 // TODO This could panic, for example if the user removes the highway tag
                 road.lane_specs_ltr = osm2streets::get_lane_specs_ltr(&tags, &self.inner.config);
+                intersections.extend(road.endpoints());
             }
         }
-        self.inner.apply_transformations(
-            vec![Transformation::GenerateIntersectionGeometry],
-            &mut Timer::throwaway(),
-        );
+        for i in intersections {
+            self.inner.update_geometry(i);
+        }
 
         self.ways.get_mut(&id).unwrap().tags = tags;
     }

osm2streets/src/geometry/mod.rs

@@ -109,8 +109,6 @@ pub fn intersection_polygon(
         roads.insert(r.id, r);
     }
 
-    assert!(!roads.is_empty());
-
     let results = Results {
         intersection_id,
         intersection_polygon: Polygon::dummy(),
@@ -119,6 +117,14 @@ pub fn intersection_polygon(
         trim_starts: BTreeMap::new(),
         trim_ends: BTreeMap::new(),
     };
+
+    // TODO Hack! Transformation::CollapseDegenerateIntersections triggers this, because we try to
+    // update_geometry in the middle. We need to track changes and defer the recalculation.
+    if roads.is_empty() {
+        error!("Hack! intersection_polygon({intersection_id}) called with no roads");
+        return Ok(results);
+    }
+
     let mut untrimmed_roads = roads.clone();
 
     let mut results = if roads.len() == 1 {

osm2streets/src/intersection.rs

@@ -14,7 +14,7 @@ pub struct Intersection {
     /// intersection may have multiple OSM nodes (when the intersection is consolidated).
     pub osm_ids: Vec<osm::NodeID>,
 
-    /// This will be a placeholder until `Transformation::GenerateIntersectionGeometry` runs.
+    /// This will be a placeholder circle until `update_geometry` runs or if errors occur.
     ///
     /// TODO Consistently make this clockwise.
     pub polygon: Polygon,

osm2streets/src/lib.rs

@@ -99,7 +99,7 @@ impl StreetNetwork {
         for i in endpts {
             self.intersections.get_mut(&i).unwrap().roads.push(id);
             self.sort_roads(i);
-            // Recalculate movements and complexity.
+            self.update_geometry(i);
             self.update_movements(i);
         }
     }
@@ -112,6 +112,7 @@ impl StreetNetwork {
                 .roads
                 .retain(|r| *r != id);
             // Since the roads are already sorted, removing doesn't break the sort.
+            self.update_geometry(i);
             self.update_movements(i);
         }
         self.roads.remove(&id).unwrap()
@@ -145,9 +146,7 @@ impl StreetNetwork {
             .collect()
     }
 
-    /// This calculates a road's `trimmed_center_line` early, before
-    /// `Transformation::GenerateIntersectionGeometry` has run. Use sparingly.
-    // TODO Remove and maintain trim_start/end instead
+    /// This calculates a road's trimmed `center_line` early. TODO Remove entirely...
     pub(crate) fn estimate_trimmed_geometry(&self, road_id: RoadID) -> Option<PolyLine> {
         let orig_road = &self.roads[&road_id];
         let untrimmed = orig_road.get_untrimmed_center_line(self.config.driving_side);

osm2streets/src/operations/collapse_intersection.rs

@@ -29,6 +29,8 @@ impl StreetNetwork {
 
         // We could be more careful merging highway_type, layer, name, and other attributes, but in
         // practice, it doesn't matter for the short segments we're merging.
+        // TODO Here's an example where it'd be great to defer recalculating movements and
+        // geometry until insert_road() below.
         let mut keep_road = self.remove_road(keep_r);
         let destroy_road = self.remove_road(destroy_r);
         self.intersections.remove(&i).unwrap();

osm2streets/src/operations/collapse_short_road.rs

@@ -123,6 +123,7 @@ impl StreetNetwork {
 
         // We just connected a bunch of things to keep_i. Fix ordering and movements.
         self.sort_roads(keep_i);
+        self.update_geometry(keep_i);
         self.update_movements(keep_i);
 
         // TODO Fix up turn restrictions. Many cases:

osm2streets/src/operations/update_geometry.rs

@@ -35,9 +35,9 @@ impl StreetNetwork {
                 // Don't trim lines back at all
                 let road = &self.roads[&r];
                 let pt = if road.src_i == i.id {
-                    road.center_line.first_pt()
+                    road.reference_line.first_pt()
                 } else {
-                    road.center_line.last_pt()
+                    road.reference_line.last_pt()
                 };
                 self.intersections.get_mut(&id).unwrap().polygon =
                     Circle::new(pt, Distance::meters(3.0)).to_polygon();

osm2streets/src/road.rs

@@ -39,10 +39,9 @@ pub struct Road {
     /// tag and might be nonsense for the first/last segment.
     pub reference_line: PolyLine,
     pub reference_line_placement: Placement,
-    /// The physical center of all the lanes, including sidewalks (at RoadPosition::FullWidthCenter).
-    /// This will differ from reference_line and modified by transformations, notably it will be
-    /// offset based on reference_line_placement and trimmed by
-    /// `Transformation::GenerateIntersectionGeometry`.
+    /// The physical center of all the lanes, including sidewalks (at
+    /// RoadPosition::FullWidthCenter).  This will differ from `reference_line`, incorporating
+    /// `reference_line_placement`, `trim_start`, `trim_end`, etc.
     pub center_line: PolyLine,
     /// How much to trim from the start of `get_untrimmed_center_line`. Negative means to instead
     /// extend the first line.
@@ -391,9 +390,8 @@ impl Road {
         }
     }
 
-    /// Returns one PolyLine representing the center of each lane in this road. This must be called
-    /// after `Transformation::GenerateIntersectionGeometry` is run. The result also faces the same
-    /// direction as the road.
+    /// Returns one PolyLine representing the center of each lane in this road. The result also
+    /// faces the same direction as the road.
     pub(crate) fn get_lane_center_lines(&self) -> Vec<PolyLine> {
         let total_width = self.total_width();
 

osm2streets/src/transform/mod.rs

@@ -5,7 +5,6 @@ use crate::StreetNetwork;
 mod collapse_intersections;
 mod collapse_short_road;
 mod dual_carriageways;
-mod intersection_geometry;
 mod remove_disconnected;
 mod sausage_links;
 mod separate_cycletracks;
@@ -21,7 +20,6 @@ pub enum Transformation {
     CollapseSausageLinks,
     ShrinkOverlappingRoads,
     MergeDualCarriageways,
-    GenerateIntersectionGeometry,
 }
 
 impl Transformation {
@@ -33,10 +31,8 @@ impl Transformation {
             Transformation::CollapseShortRoads,
             Transformation::CollapseDegenerateIntersections,
             Transformation::ShrinkOverlappingRoads,
-            Transformation::GenerateIntersectionGeometry,
-            // The above may discover more roads to collapse
+            // Repeat this for now
             Transformation::CollapseShortRoads,
-            Transformation::GenerateIntersectionGeometry,
         ]
     }
 
@@ -75,7 +71,6 @@ impl Transformation {
             Transformation::CollapseSausageLinks => "collapse sausage links",
             Transformation::ShrinkOverlappingRoads => "shrink overlapping roads",
             Transformation::MergeDualCarriageways => "merge dual carriageways",
-            Transformation::GenerateIntersectionGeometry => "generate intersection geometry",
         }
     }
 
@@ -106,9 +101,6 @@ impl Transformation {
             Transformation::MergeDualCarriageways => {
                 dual_carriageways::merge(streets);
             }
-            Transformation::GenerateIntersectionGeometry => {
-                intersection_geometry::generate(streets, timer);
-            }
         }
         timer.stop(self.name());
     }

osm2streets/src/transform/sausage_links.rs

@@ -153,6 +153,7 @@ fn fix(streets: &mut StreetNetwork, id1: RoadID, id2: RoadID) {
     road1.update_center_line(streets.config.driving_side);
     let intersections = road1.endpoints();
     for i in intersections {
+        streets.update_geometry(i);
         streets.update_movements(i);
     }
 }

osm2streets/src/transform/shrink_roads.rs

@@ -1,4 +1,4 @@
-use std::collections::{HashMap, HashSet};
+use std::collections::{HashMap, HashSet, BTreeSet};
 
 use aabb_quadtree::QuadTree;
 use abstutil::Timer;
@@ -50,6 +50,7 @@ pub fn shrink(streets: &mut StreetNetwork, timer: &mut Timer) {
     }
 
     timer.start_iter("shrink overlapping roads", overlapping.len());
+    let mut intersections = BTreeSet::new();
     let mut shrunk = HashSet::new();
     for (r1, r2) in overlapping {
         timer.next();
@@ -65,9 +66,15 @@ pub fn shrink(streets: &mut StreetNetwork, timer: &mut Timer) {
             // Anything derived from lane_specs_ltr will need to be changed. When we store
             // untrimmed and trimmed center points instead of calculating them dynamically, that'll
             // have to happen here.
-            for spec in &mut streets.roads.get_mut(&id).unwrap().lane_specs_ltr {
+            let road = streets.roads.get_mut(&id).unwrap();
+            for spec in &mut road.lane_specs_ltr {
                 spec.width *= 0.5;
             }
+            intersections.extend(road.endpoints());
         }
     }
+
+    for i in intersections {
+        streets.update_geometry(i);
+    }
 }

streets_reader/src/split_ways.rs

@@ -220,13 +220,14 @@ pub fn split_up_roads(
     }
     timer.stop("match traffic signals to intersections");
 
-    timer.start("calculate intersection movements");
+    timer.start("calculate intersection geometry and movements");
     let intersection_ids: Vec<_> = streets.intersections.keys().cloned().collect();
     for i in intersection_ids {
         streets.sort_roads(i);
+        streets.update_geometry(i);
         streets.update_movements(i);
     }
-    timer.stop("calculate intersection movements");
+    timer.stop("calculate intersection geometry and movements");
 
     timer.stop("splitting up roads");
     pt_to_road