Shift logic from the select boundary UI to the partitioning structure.

That was the original intention, but it wound up being easier to
prototype otherwise.

Should be no behavioral changes.

ltn/src/browse.rs

@@ -162,7 +162,7 @@ impl State<App> for BrowseNeighborhoods {
 fn make_world(ctx: &mut EventCtx, app: &App, timer: &mut Timer) -> World<NeighborhoodID> {
     let mut world = World::bounded(app.map.get_bounds());
     let map = &app.map;
-    for (id, (block, color)) in &app.session.partitioning.neighborhoods {
+    for (id, (block, color)) in app.session.partitioning.all_neighborhoods() {
         match app.session.draw_neighborhood_style {
             Style::SimpleColoring => {
                 world
@@ -215,7 +215,7 @@ fn draw_boundary_roads(ctx: &EventCtx, app: &App) -> ToggleZoomed {
     let mut seen_roads = HashSet::new();
     let mut seen_borders = HashSet::new();
     let mut batch = ToggleZoomed::builder();
-    for (block, _) in app.session.partitioning.neighborhoods.values() {
+    for (block, _) in app.session.partitioning.all_neighborhoods().values() {
         for id in &block.perimeter.roads {
             let r = id.road;
             if seen_roads.contains(&r) {

ltn/src/export.rs

@@ -53,7 +53,7 @@ fn geojson_string(ctx: &EventCtx, app: &App) -> Result<String> {
     let mut features = Vec::new();
 
     // All neighborhood boundaries
-    for (id, (block, color)) in &app.session.partitioning.neighborhoods {
+    for (id, (block, color)) in app.session.partitioning.all_neighborhoods() {
         let mut feature = Feature {
             bbox: None,
             geometry: Some(block.polygon.to_geojson(None)),

ltn/src/impact/mod.rs

@@ -219,7 +219,7 @@ impl Results {
 // Just add the base layer of non-clickable neighborhoods
 fn make_world(ctx: &mut EventCtx, app: &App) -> World<Obj> {
     let mut world = World::bounded(app.map.get_bounds());
-    for (id, (block, color)) in &app.session.partitioning.neighborhoods {
+    for (id, (block, color)) in app.session.partitioning.all_neighborhoods() {
         world
             .add(Obj::Neighborhood(*id))
             .hitbox(block.polygon.clone())

ltn/src/neighborhood.rs

@@ -57,8 +57,10 @@ pub struct DistanceInterval {
 impl Neighborhood {
     pub fn new(ctx: &EventCtx, app: &App, id: NeighborhoodID) -> Neighborhood {
         let map = &app.map;
-        let orig_perimeter = app.session.partitioning.neighborhoods[&id]
-            .0
+        let orig_perimeter = app
+            .session
+            .partitioning
+            .neighborhood_block(id)
             .perimeter
             .clone();
 

ltn/src/partition.rs

@@ -1,9 +1,11 @@
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, BTreeSet};
+
+use anyhow::Result;
 
 use abstio::MapName;
 use abstutil::Timer;
 use map_model::osm::RoadRank;
-use map_model::{Block, Perimeter};
+use map_model::{Block, Map, Perimeter, RoadID, RoadSideID};
 use widgetry::Color;
 
 use crate::App;
@@ -20,17 +22,26 @@ const COLORS: [Color; 6] = [
 /// An opaque ID, won't be contiguous as we adjust boundaries
 #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct NeighborhoodID(usize);
+
+/// Identifies a single / unmerged block, which never changes
+#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub struct BlockID(usize);
+
 // Some states want this
 impl widgetry::mapspace::ObjectID for NeighborhoodID {}
+impl widgetry::mapspace::ObjectID for BlockID {}
 
 #[derive(Clone)]
 pub struct Partitioning {
     pub map: MapName,
-    pub neighborhoods: BTreeMap<NeighborhoodID, (Block, Color)>,
+    neighborhoods: BTreeMap<NeighborhoodID, (Block, Color)>,
     // The single / unmerged blocks never change
-    pub single_blocks: Vec<Block>,
+    single_blocks: Vec<Block>,
 
-    id_counter: usize,
+    neighborhood_id_counter: usize,
+
+    // Invariant: This is a bijection, every block belongs to exactly one neighborhood
+    block_to_neighborhood: BTreeMap<BlockID, NeighborhoodID>,
 }
 
 impl Partitioning {
@@ -41,7 +52,9 @@ impl Partitioning {
             neighborhoods: BTreeMap::new(),
             single_blocks: Vec::new(),
 
-            id_counter: 0,
+            neighborhood_id_counter: 0,
+
+            block_to_neighborhood: BTreeMap::new(),
         }
     }
 
@@ -89,14 +102,29 @@ impl Partitioning {
         for block in blocks {
             neighborhoods.insert(NeighborhoodID(neighborhoods.len()), (block, Color::RED));
         }
-        let id_counter = neighborhoods.len();
+        let neighborhood_id_counter = neighborhoods.len();
         let mut p = Partitioning {
             map: map.get_name().clone(),
             neighborhoods,
             single_blocks,
 
-            id_counter,
+            neighborhood_id_counter,
+            block_to_neighborhood: BTreeMap::new(),
         };
+
+        // TODO We could probably build this up as we go
+        for id in p.all_block_ids() {
+            if let Some(neighborhood) = p.neighborhood_containing(id) {
+                p.block_to_neighborhood.insert(id, neighborhood);
+            } else {
+                // TODO What happened? This will break everything downstream.
+                error!(
+                    "Block doesn't belong to any neighborhood?! {:?}",
+                    p.get_block(id).perimeter
+                );
+            }
+        }
+
         p.recalculate_coloring();
         p
     }
@@ -118,8 +146,130 @@ impl Partitioning {
         orig_coloring != new_coloring
     }
 
-    pub fn neighborhood_containing(&self, find_block: &Block) -> Option<NeighborhoodID> {
+    // TODO Explain return value
+    pub fn transfer_block(
+        &mut self,
+        map: &Map,
+        id: BlockID,
+        old_owner: NeighborhoodID,
+        new_owner: NeighborhoodID,
+    ) -> Result<Option<NeighborhoodID>> {
+        assert_ne!(old_owner, new_owner);
+
+        // Is the newly expanded neighborhood a valid perimeter?
+        let new_owner_blocks: Vec<BlockID> = self
+            .block_to_neighborhood
+            .iter()
+            .filter_map(|(block, neighborhood)| {
+                if *neighborhood == new_owner || *block == id {
+                    Some(*block)
+                } else {
+                    None
+                }
+            })
+            .collect();
+        let new_neighborhood_block = self.make_merged_block(map, new_owner_blocks)?;
+
+        // Is the old neighborhood, minus this block, still valid?
+        // TODO refactor Neighborhood to BlockIDs?
+        let old_owner_blocks: Vec<BlockID> = self
+            .block_to_neighborhood
+            .iter()
+            .filter_map(|(block, neighborhood)| {
+                if *neighborhood == old_owner && *block != id {
+                    Some(*block)
+                } else {
+                    None
+                }
+            })
+            .collect();
+        if old_owner_blocks.is_empty() {
+            // We're deleting the old neighborhood!
+            self.neighborhoods.get_mut(&new_owner).unwrap().0 = new_neighborhood_block;
+            self.neighborhoods.remove(&old_owner).unwrap();
+            self.block_to_neighborhood.insert(id, new_owner);
+            // Tell the caller to recreate this SelectBoundary state, switching to the neighborhood
+            // we just donated to, since the old is now gone
+            return Ok(Some(new_owner));
+        }
+
+        let old_neighborhood_block = self.make_merged_block(map, old_owner_blocks)?;
+        // Great! Do the transfer.
+        self.neighborhoods.get_mut(&old_owner).unwrap().0 = old_neighborhood_block;
+        self.neighborhoods.get_mut(&new_owner).unwrap().0 = new_neighborhood_block;
+
+        self.block_to_neighborhood.insert(id, new_owner);
+        Ok(None)
+    }
+
+    /// Needs to find an existing neighborhood to take the block, or make a new one
+    pub fn remove_block_from_neighborhood(
+        &mut self,
+        map: &Map,
+        id: BlockID,
+        old_owner: NeighborhoodID,
+    ) -> Result<Option<NeighborhoodID>> {
+        // Find all RoadSideIDs in the block matching the current neighborhood perimeter. Look for
+        // the first one that borders another neighborhood, and transfer the block there.
+        // TODO This can get unintuitive -- if we remove a block bordering two other
+        // neighborhoods, which one should we donate to?
+        let current_perim_set: BTreeSet<RoadSideID> = self.neighborhoods[&old_owner]
+            .0
+            .perimeter
+            .roads
+            .iter()
+            .cloned()
+            .collect();
+        for road_side in &self.get_block(id).perimeter.roads {
+            if !current_perim_set.contains(road_side) {
+                continue;
+            }
+            // Is there another neighborhood that has the other side of this road on its perimeter?
+            // TODO We could map road -> BlockID then use block_to_neighborhood
+            let other_side = road_side.other_side();
+            if let Some((new_owner, _)) = self
+                .neighborhoods
+                .iter()
+                .find(|(_, (block, _))| block.perimeter.roads.contains(&other_side))
+            {
+                let new_owner = *new_owner;
+                return self.transfer_block(map, id, old_owner, new_owner);
+            }
+        }
+
+        // We didn't find any match, so we're jettisoning a block near the edge of the map (or a
+        // buggy area missing blocks). Create a new neighborhood with just this block.
+        let new_owner = NeighborhoodID(self.neighborhood_id_counter);
+        self.neighborhood_id_counter += 1;
+        // Temporary color
+        self.neighborhoods
+            .insert(new_owner, (self.get_block(id).clone(), Color::RED));
+        let result = self.transfer_block(map, id, old_owner, new_owner);
+        if result.is_err() {
+            // Revert the change above!
+            self.neighborhoods.remove(&new_owner).unwrap();
+        }
+        result
+    }
+}
+
+// Read-only
+impl Partitioning {
+    pub fn neighborhood_block(&self, id: NeighborhoodID) -> &Block {
+        &self.neighborhoods[&id].0
+    }
+
+    pub fn neighborhood_color(&self, id: NeighborhoodID) -> Color {
+        self.neighborhoods[&id].1
+    }
+
+    pub fn all_neighborhoods(&self) -> &BTreeMap<NeighborhoodID, (Block, Color)> {
+        &self.neighborhoods
+    }
+
+    pub fn neighborhood_containing(&self, find_block: BlockID) -> Option<NeighborhoodID> {
         // TODO We could probably build this mapping up when we do Perimeter::merge_all
+        let find_block = self.get_block(find_block);
         for (id, (block, _)) in &self.neighborhoods {
             if block.perimeter.contains(&find_block.perimeter) {
                 return Some(*id);
@@ -128,19 +278,57 @@ impl Partitioning {
         None
     }
 
-    /// Starts a new neighborhood containing a single block. This will temporarily leave the
-    /// Partitioning in an invalid state, with one block being part of two neighborhoods. The
-    /// caller must keep rearranging things.
-    pub fn create_new_neighborhood(&mut self, block: Block) -> NeighborhoodID {
-        let id = NeighborhoodID(self.id_counter);
-        self.id_counter += 1;
-        // Temporary color
-        self.neighborhoods.insert(id, (block, Color::RED));
-        id
+    pub fn all_single_blocks(&self) -> Vec<(BlockID, &Block)> {
+        self.single_blocks
+            .iter()
+            .enumerate()
+            .map(|(idx, block)| (BlockID(idx), block))
+            .collect()
     }
 
-    /// Undo the above. Lots of trust on the caller.
-    pub fn remove_new_neighborhood(&mut self, id: NeighborhoodID) {
-        self.neighborhoods.remove(&id).unwrap();
+    pub fn all_block_ids(&self) -> Vec<BlockID> {
+        (0..self.single_blocks.len()).map(BlockID).collect()
+    }
+
+    pub fn get_block(&self, id: BlockID) -> &Block {
+        &self.single_blocks[id.0]
+    }
+
+    // Will crash if the original matching failed
+    pub fn block_to_neighborhood(&self, id: BlockID) -> NeighborhoodID {
+        self.block_to_neighborhood[&id]
+    }
+
+    /// Blocks on the "frontier" are adjacent to the perimeter, either just inside or outside.
+    pub fn calculate_frontier(&self, perim: &Perimeter) -> BTreeSet<BlockID> {
+        let perim_roads: BTreeSet<RoadID> = perim.roads.iter().map(|id| id.road).collect();
+
+        let mut frontier = BTreeSet::new();
+        for (block_id, block) in self.all_single_blocks() {
+            for road_side_id in &block.perimeter.roads {
+                // If the perimeter has this RoadSideID on the same side, we're just inside. If it has
+                // the other side, just on the outside. Either way, on the frontier.
+                if perim_roads.contains(&road_side_id.road) {
+                    frontier.insert(block_id);
+                    break;
+                }
+            }
+        }
+        frontier
+    }
+
+    fn make_merged_block(&self, map: &Map, input: Vec<BlockID>) -> Result<Block> {
+        let mut perimeters = Vec::new();
+        for id in input {
+            perimeters.push(self.get_block(id).perimeter.clone());
+        }
+        let mut merged = Perimeter::merge_all(perimeters, false);
+        if merged.len() != 1 {
+            bail!(format!(
+                "Splitting this neighborhood into {} pieces is currently unsupported",
+                merged.len()
+            ));
+        }
+        merged.pop().unwrap().to_block(map)
     }
 }

ltn/src/select_boundary.rs

@@ -1,16 +1,17 @@
-use std::collections::{BTreeMap, BTreeSet};
+use std::collections::BTreeSet;
 
 use anyhow::Result;
 
 use geom::Distance;
-use map_model::{Block, Perimeter, RoadID, RoadSideID};
+use map_model::Block;
 use widgetry::mapspace::ToggleZoomed;
-use widgetry::mapspace::{ObjectID, World, WorldOutcome};
+use widgetry::mapspace::{World, WorldOutcome};
 use widgetry::{
     Color, EventCtx, GfxCtx, HorizontalAlignment, Key, Line, Outcome, Panel, State, Text, TextExt,
     VerticalAlignment, Widget,
 };
 
+use crate::partition::BlockID;
 use crate::{App, NeighborhoodID, Partitioning, Transition};
 
 const SELECTED: Color = Color::CYAN;
@@ -18,66 +19,46 @@ const SELECTED: Color = Color::CYAN;
 pub struct SelectBoundary {
     panel: Panel,
     id: NeighborhoodID,
-    // These are always single, unmerged blocks. Thus, these blocks never change -- only their
-    // color and assignment to a neighborhood.
-    blocks: BTreeMap<BlockID, Block>,
     world: World<BlockID>,
+    // TODO Redundant
     selected: BTreeSet<BlockID>,
     draw_outline: ToggleZoomed,
-    block_to_neighborhood: BTreeMap<BlockID, NeighborhoodID>,
     frontier: BTreeSet<BlockID>,
 
     orig_partitioning: Partitioning,
 }
 
-#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
-struct BlockID(usize);
-impl ObjectID for BlockID {}
-
 impl SelectBoundary {
     pub fn new_state(ctx: &mut EventCtx, app: &App, id: NeighborhoodID) -> Box<dyn State<App>> {
-        let initial_boundary = app.session.partitioning.neighborhoods[&id]
-            .0
-            .perimeter
-            .clone();
+        let initial_boundary = app.session.partitioning.neighborhood_block(id);
 
         let mut state = SelectBoundary {
             panel: make_panel(ctx, app),
             id,
-            blocks: BTreeMap::new(),
             world: World::bounded(app.map.get_bounds()),
             selected: BTreeSet::new(),
             draw_outline: ToggleZoomed::empty(ctx),
-            block_to_neighborhood: BTreeMap::new(),
             frontier: BTreeSet::new(),
 
             orig_partitioning: app.session.partitioning.clone(),
         };
 
-        for (idx, block) in app.session.partitioning.single_blocks.iter().enumerate() {
-            let id = BlockID(idx);
-            if let Some(neighborhood) = app.session.partitioning.neighborhood_containing(block) {
-                state.block_to_neighborhood.insert(id, neighborhood);
-            } else {
-                // TODO What happened?
-                error!(
-                    "Block doesn't belong to any neighborhood?! {:?}",
-                    block.perimeter
-                );
-            }
-            if initial_boundary.contains(&block.perimeter) {
+        for (id, block) in app.session.partitioning.all_single_blocks() {
+            if initial_boundary.perimeter.contains(&block.perimeter) {
                 state.selected.insert(id);
             }
-            state.blocks.insert(id, block.clone());
         }
-        state.frontier = calculate_frontier(&initial_boundary, &state.blocks);
+        state.frontier = app
+            .session
+            .partitioning
+            .calculate_frontier(&initial_boundary.perimeter);
 
         // Fill out the world initially
-        for id in state.blocks.keys().cloned().collect::<Vec<_>>() {
+        for id in app.session.partitioning.all_block_ids() {
             state.add_block(ctx, app, id);
         }
 
-        state.redraw_outline(ctx, app, initial_boundary);
+        state.redraw_outline(ctx, initial_boundary);
         state.world.initialize_hover(ctx);
         Box::new(state)
     }
@@ -85,20 +66,18 @@ impl SelectBoundary {
     fn add_block(&mut self, ctx: &mut EventCtx, app: &App, id: BlockID) {
         let color = if self.selected.contains(&id) {
             SELECTED
-        } else if let Some(neighborhood) = self.block_to_neighborhood.get(&id) {
+        } else {
+            let neighborhood = app.session.partitioning.block_to_neighborhood(id);
             // Use the original color. This assumes the partitioning has been updated, of
             // course
-            app.session.partitioning.neighborhoods[neighborhood].1
-        } else {
-            // TODO A broken case, block has no neighborhood
-            Color::RED
+            app.session.partitioning.neighborhood_color(neighborhood)
         };
 
         if self.frontier.contains(&id) {
             let mut obj = self
                 .world
                 .add(id)
-                .hitbox(self.blocks[&id].polygon.clone())
+                .hitbox(app.session.partitioning.get_block(id).polygon.clone())
                 .draw_color(color.alpha(0.5))
                 .hover_alpha(0.8)
                 .clickable();
@@ -117,24 +96,21 @@ impl SelectBoundary {
             // it
             self.world
                 .add(id)
-                .hitbox(self.blocks[&id].polygon.clone())
+                .hitbox(app.session.partitioning.get_block(id).polygon.clone())
                 .draw_color(color.alpha(0.3))
                 .build(ctx);
         }
     }
 
-    fn redraw_outline(&mut self, ctx: &mut EventCtx, app: &App, perimeter: Perimeter) {
+    fn redraw_outline(&mut self, ctx: &mut EventCtx, block: &Block) {
         // Draw the outline of the current blocks
         let mut batch = ToggleZoomed::builder();
-        if let Ok(block) = perimeter.to_block(&app.map) {
-            if let Ok(outline) = block.polygon.to_outline(Distance::meters(10.0)) {
-                batch.unzoomed.push(Color::RED, outline);
-            }
-            if let Ok(outline) = block.polygon.to_outline(Distance::meters(5.0)) {
-                batch.zoomed.push(Color::RED.alpha(0.5), outline);
-            }
+        if let Ok(outline) = block.polygon.to_outline(Distance::meters(10.0)) {
+            batch.unzoomed.push(Color::RED, outline);
+        }
+        if let Ok(outline) = block.polygon.to_outline(Distance::meters(5.0)) {
+            batch.zoomed.push(Color::RED.alpha(0.5), outline);
         }
-        // TODO If this fails, maybe also revert
         self.draw_outline = batch.build(ctx);
     }
 
@@ -147,11 +123,12 @@ impl SelectBoundary {
             }
             Ok(None) => {
                 let old_frontier = std::mem::take(&mut self.frontier);
-                let new_perimeter = app.session.partitioning.neighborhoods[&self.id]
-                    .0
-                    .perimeter
-                    .clone();
-                self.frontier = calculate_frontier(&new_perimeter, &self.blocks);
+                self.frontier = app.session.partitioning.calculate_frontier(
+                    &app.session
+                        .partitioning
+                        .neighborhood_block(self.id)
+                        .perimeter,
+                );
 
                 // Redraw all of the blocks that changed
                 let mut changed_blocks: Vec<BlockID> = old_frontier
@@ -165,7 +142,7 @@ impl SelectBoundary {
                     // The coloring of neighborhoods changed; this could possibly have impact far
                     // away. Just redraw all blocks.
                     changed_blocks.clear();
-                    changed_blocks.extend(self.blocks.keys().cloned());
+                    changed_blocks.extend(app.session.partitioning.all_block_ids());
                 }
 
                 for changed in changed_blocks {
@@ -173,8 +150,7 @@ impl SelectBoundary {
                     self.add_block(ctx, app, changed);
                 }
 
-                // TODO Pass in the Block
-                self.redraw_outline(ctx, app, new_perimeter);
+                self.redraw_outline(ctx, app.session.partitioning.neighborhood_block(self.id));
                 self.panel = make_panel(ctx, app);
             }
             Err(err) => {
@@ -191,170 +167,25 @@ impl SelectBoundary {
         Transition::Keep
     }
 
-    fn make_merged_block(&self, app: &App, input: Vec<BlockID>) -> Result<Block> {
-        let mut perimeters = Vec::new();
-        for id in input {
-            perimeters.push(self.blocks[&id].perimeter.clone());
-        }
-        let mut merged = Perimeter::merge_all(perimeters, false);
-        if merged.len() != 1 {
-            bail!(format!(
-                "Splitting this neighborhood into {} pieces is currently unsupported",
-                merged.len()
-            ));
-        }
-        merged.pop().unwrap().to_block(&app.map)
-    }
-
     // Ok(Some(x)) means the current neighborhood was destroyed, and the caller should switch to
     // focusing on a different neigbhorhood
     fn try_block_changed(&mut self, app: &mut App, id: BlockID) -> Result<Option<NeighborhoodID>> {
         if self.selected.contains(&id) {
-            self.add_block_to_current(app, id)
-        } else {
-            self.remove_block_from_current(app, id)
-        }
-    }
-
-    fn add_block_to_current(
-        &mut self,
-        app: &mut App,
-        id: BlockID,
-    ) -> Result<Option<NeighborhoodID>> {
-        let old_owner = app
-            .session
-            .partitioning
-            .neighborhood_containing(&self.blocks[&id])
-            .unwrap();
-        // Ignore the return value if the old neighborhood is deleted
-        self.transfer_block(app, id, old_owner, self.id)?;
-        Ok(None)
-    }
-
-    fn remove_block_from_current(
-        &mut self,
-        app: &mut App,
-        id: BlockID,
-    ) -> Result<Option<NeighborhoodID>> {
-        // Find all RoadSideIDs in the block matching the current neighborhood perimeter. Look for
-        // the first one that borders another neighborhood, and transfer the block there.
-        // TODO This can get unintuitive -- if we remove a block bordering two other
-        // neighborhoods, which one should we donate to?
-        let current_perim_set: BTreeSet<RoadSideID> = app.session.partitioning.neighborhoods
-            [&self.id]
-            .0
-            .perimeter
-            .roads
-            .iter()
-            .cloned()
-            .collect();
-        for road_side in &self.blocks[&id].perimeter.roads {
-            if !current_perim_set.contains(road_side) {
-                continue;
-            }
-            // Is there another neighborhood that has the other side of this road on its perimeter?
-            // TODO We could map road -> BlockID then use block_to_neighborhood
-            let other_side = road_side.other_side();
-            if let Some((new_owner, _)) = app
+            let old_owner = app
                 .session
                 .partitioning
-                .neighborhoods
-                .iter()
-                .find(|(_, (block, _))| block.perimeter.roads.contains(&other_side))
-            {
-                let new_owner = *new_owner;
-                return self.transfer_block(app, id, self.id, new_owner);
-            }
-        }
-
-        // We didn't find any match, so we're jettisoning a block near the edge of the map (or a
-        // buggy area missing blocks). Create a new neighborhood with just this block.
-        let new_owner = app
-            .session
-            .partitioning
-            .create_new_neighborhood(self.blocks[&id].clone());
-        let result = self.transfer_block(app, id, self.id, new_owner);
-        if result.is_err() {
-            // Revert the change above!
-            app.session.partitioning.remove_new_neighborhood(new_owner);
-        }
-        result
-    }
-
-    // This doesn't use self.selected; it's agnostic to what the current block is
-    // TODO Move it to Partitioning
-    fn transfer_block(
-        &mut self,
-        app: &mut App,
-        id: BlockID,
-        old_owner: NeighborhoodID,
-        new_owner: NeighborhoodID,
-    ) -> Result<Option<NeighborhoodID>> {
-        assert_ne!(old_owner, new_owner);
-
-        // Is the newly expanded neighborhood a valid perimeter?
-        let new_owner_blocks: Vec<BlockID> = self
-            .block_to_neighborhood
-            .iter()
-            .filter_map(|(block, neighborhood)| {
-                if *neighborhood == new_owner || *block == id {
-                    Some(*block)
-                } else {
-                    None
-                }
-            })
-            .collect();
-        let new_neighborhood_block = self.make_merged_block(app, new_owner_blocks)?;
-
-        // Is the old neighborhood, minus this block, still valid?
-        // TODO refactor Neighborhood to BlockIDs?
-        let old_owner_blocks: Vec<BlockID> = self
-            .block_to_neighborhood
-            .iter()
-            .filter_map(|(block, neighborhood)| {
-                if *neighborhood == old_owner && *block != id {
-                    Some(*block)
-                } else {
-                    None
-                }
-            })
-            .collect();
-        if old_owner_blocks.is_empty() {
-            // We're deleting the old neighborhood!
-            app.session
-                .partitioning
-                .neighborhoods
-                .get_mut(&new_owner)
-                .unwrap()
-                .0 = new_neighborhood_block;
-            app.session
-                .partitioning
-                .neighborhoods
-                .remove(&old_owner)
+                .neighborhood_containing(id)
                 .unwrap();
-            self.block_to_neighborhood.insert(id, new_owner);
-            // Tell the caller to recreate this SelectBoundary state, switching to the neighborhood
-            // we just donated to, since the old is now gone
-            return Ok(Some(new_owner));
+            // Ignore the return value if the old neighborhood is deleted
+            app.session
+                .partitioning
+                .transfer_block(&app.map, id, old_owner, self.id)?;
+            Ok(None)
+        } else {
+            app.session
+                .partitioning
+                .remove_block_from_neighborhood(&app.map, id, self.id)
         }
-
-        let old_neighborhood_block = self.make_merged_block(app, old_owner_blocks)?;
-        // Great! Do the transfer.
-        app.session
-            .partitioning
-            .neighborhoods
-            .get_mut(&old_owner)
-            .unwrap()
-            .0 = old_neighborhood_block;
-        app.session
-            .partitioning
-            .neighborhoods
-            .get_mut(&new_owner)
-            .unwrap()
-            .0 = new_neighborhood_block;
-
-        self.block_to_neighborhood.insert(id, new_owner);
-        Ok(None)
     }
 }
 
@@ -466,21 +297,3 @@ fn make_panel(ctx: &mut EventCtx, app: &App) -> Panel {
     .aligned(HorizontalAlignment::Left, VerticalAlignment::Top)
     .build(ctx)
 }
-
-// Blocks on the "frontier" are adjacent to the perimeter, either just inside or outside.
-fn calculate_frontier(perim: &Perimeter, blocks: &BTreeMap<BlockID, Block>) -> BTreeSet<BlockID> {
-    let perim_roads: BTreeSet<RoadID> = perim.roads.iter().map(|id| id.road).collect();
-
-    let mut frontier = BTreeSet::new();
-    for (block_id, block) in blocks {
-        for road_side_id in &block.perimeter.roads {
-            // If the perimeter has this RoadSideID on the same side, we're just inside. If it has
-            // the other side, just on the outside. Either way, on the frontier.
-            if perim_roads.contains(&road_side_id.road) {
-                frontier.insert(*block_id);
-                break;
-            }
-        }
-    }
-    frontier
-}