autorouter: if band removal fails, propagate error instead of panicking

src/autorouter/autorouter.rs

@@ -4,7 +4,11 @@ use thiserror::Error;
 
 use crate::{
     board::{mesadata::AccessMesadata, Board},
-    drawing::{dot::FixedDotIndex, Infringement},
+    drawing::{
+        band::{BandTermsegIndex, BandUid},
+        dot::FixedDotIndex,
+        Infringement,
+    },
     layout::via::ViaWeight,
     router::{navmesh::NavmeshError, RouterError},
     triangulation::GetTrianvertexNodeIndex,
@@ -30,6 +34,8 @@ pub enum AutorouterError {
     Router(#[from] RouterError),
     #[error("could not place via")]
     CouldNotPlaceVia(#[from] Infringement),
+    #[error("could not remove band")]
+    CouldNotRemoveBand(BandTermsegIndex),
     #[error("need exactly two ratlines")]
     NeedExactlyTwoRatlines,
 }
@@ -56,11 +62,14 @@ impl<M: AccessMesadata> Autorouter<M> {
         Autoroute::new(self, ratlines)
     }
 
-    pub fn undo_autoroute(&mut self, selection: &PinSelection) {
+    pub fn undo_autoroute(&mut self, selection: &PinSelection) -> Result<(), AutorouterError> {
         self.undo_autoroute_ratlines(self.selected_ratlines(selection))
     }
 
-    pub(super) fn undo_autoroute_ratlines(&mut self, ratlines: Vec<EdgeIndex<usize>>) {
+    pub(super) fn undo_autoroute_ratlines(
+        &mut self,
+        ratlines: Vec<EdgeIndex<usize>>,
+    ) -> Result<(), AutorouterError> {
         for ratline in ratlines.iter() {
             let band = self
                 .ratsnest
@@ -69,8 +78,13 @@ impl<M: AccessMesadata> Autorouter<M> {
                 .unwrap()
                 .band_termseg
                 .unwrap();
-            self.board.layout_mut().remove_band(band);
+            self.board
+                .layout_mut()
+                .remove_band(band)
+                .map_err(|_| AutorouterError::CouldNotRemoveBand(band))?;
         }
+
+        Ok(())
     }
 
     pub fn place_via(&self, weight: ViaWeight) -> Result<PlaceVia, AutorouterError> {

src/autorouter/invoker.rs

@@ -289,11 +289,17 @@ impl<M: AccessMesadata> Invoker<M> {
         let command = self.history.last_done()?;
 
         match command {
-            Command::Autoroute(ref selection) => self.autorouter.undo_autoroute(selection),
+            Command::Autoroute(ref selection) => {
-            Command::PlaceVia(weight) => self.autorouter.undo_place_via(*weight),
+                self.autorouter.undo_autoroute(selection);
-            Command::RemoveBands(ref selection) => self.autorouter.undo_remove_bands(selection),
+            }
-            Command::CompareDetours(..) => (),
+            Command::PlaceVia(weight) => {
-            Command::MeasureLength(..) => (),
+                self.autorouter.undo_place_via(*weight);
+            }
+            Command::RemoveBands(ref selection) => {
+                self.autorouter.undo_remove_bands(selection);
+            }
+            Command::CompareDetours(..) => {}
+            Command::MeasureLength(..) => {}
         }
 
         Ok::<(), InvokerError>(self.history.undo()?)

src/bin/topola-egui/app.rs

@@ -22,7 +22,7 @@ use topola::{
         graph::{MakePrimitive, PrimitiveIndex},
         primitive::MakePrimitiveShape,
         rules::AccessRules,
-        Drawing, Infringement, LayoutException,
+        Drawing, DrawingException, Infringement,
     },
     geometry::{
         compound::ManageCompounds,
@@ -152,8 +152,6 @@ impl App {
                 invoker.replay(serde_json::from_reader(bufread).unwrap())
             }
 
-            // TODO: Make Save History work too.
-
             if let Some(ref mut execute) = self.maybe_execute {
                 let status = match execute.step(invoker) {
                     Ok(status) => status,

src/drawing/drawing.rs

@@ -42,7 +42,7 @@ use super::head::{Head, HeadRef};
 
 #[enum_dispatch]
 #[derive(Error, Debug, Clone, Copy)]
-pub enum LayoutException {
+pub enum DrawingException {
     #[error(transparent)]
     NoTangents(#[from] NoTangents),
     #[error(transparent)]
@@ -91,7 +91,7 @@ impl<CW: Copy, R: AccessRules> Drawing<CW, R> {
         }
     }
 
-    pub fn remove_band(&mut self, band: BandTermsegIndex) {
+    pub fn remove_band(&mut self, band: BandTermsegIndex) -> Result<(), DrawingException> {
         match band {
             BandTermsegIndex::Straight(seg) => {
                 self.geometry_with_rtree.remove_seg(seg.into());
@@ -148,10 +148,12 @@ impl<CW: Copy, R: AccessRules> Drawing<CW, R> {
                 }
 
                 for outer in outers {
-                    self.update_this_and_outward_bows(outer).unwrap(); // Must never fail.
+                    self.update_this_and_outward_bows(outer)?;
                 }
             }
         }
+
+        Ok(())
     }
 
     #[debug_ensures(ret.is_ok() -> self.geometry_with_rtree.graph().node_count() == old(self.geometry_with_rtree.graph().node_count() + 1))]
@@ -272,7 +274,7 @@ impl<CW: Copy, R: AccessRules> Drawing<CW, R> {
         around: GearIndex,
         weight: LooseBendWeight,
         infringables: Option<&[PrimitiveIndex]>,
-    ) -> Result<LooseBendIndex, LayoutException> {
+    ) -> Result<LooseBendIndex, DrawingException> {
         // It makes no sense to wrap something around or under one of its connectables.
         //
         if let Some(net) = weight.maybe_net {
@@ -399,7 +401,7 @@ impl<CW: Copy, R: AccessRules> Drawing<CW, R> {
         seg_weight: SeqLooseSegWeight,
         bend_weight: LooseBendWeight,
         cw: bool,
-    ) -> Result<Cane, LayoutException> {
+    ) -> Result<Cane, DrawingException> {
         let maybe_next_gear = around.ref_(self).next_gear();
         let cane = self.add_cane_with_infringables(
             from,
@@ -430,7 +432,7 @@ impl<CW: Copy, R: AccessRules> Drawing<CW, R> {
             return Err(collision.into());
         }
 
-        Ok::<Cane, LayoutException>(cane)
+        Ok::<Cane, DrawingException>(cane)
     }
 
     #[debug_ensures(self.geometry_with_rtree.graph().node_count() == old(self.geometry_with_rtree.graph().node_count()))]
@@ -438,7 +440,7 @@ impl<CW: Copy, R: AccessRules> Drawing<CW, R> {
     fn update_this_and_outward_bows(
         &mut self,
         around: LooseBendIndex,
-    ) -> Result<(), LayoutException> {
+    ) -> Result<(), DrawingException> {
         // FIXME: Fail gracefully on infringement.
         let mut maybe_rail = Some(around);
 
@@ -536,7 +538,7 @@ impl<CW: Copy, R: AccessRules> Drawing<CW, R> {
             maybe_rail = self.primitive(rail).outer();
         }
 
-        Ok::<(), LayoutException>(())
+        Ok::<(), DrawingException>(())
     }
 
     #[debug_ensures(ret.is_ok() -> self.geometry_with_rtree.graph().node_count() == old(self.geometry_with_rtree.graph().node_count() + 4))]
@@ -551,7 +553,7 @@ impl<CW: Copy, R: AccessRules> Drawing<CW, R> {
         seg_weight: SeqLooseSegWeight,
         bend_weight: LooseBendWeight,
         cw: bool,
-    ) -> Result<Cane, LayoutException> {
+    ) -> Result<Cane, DrawingException> {
         self.add_cane_with_infringables(
             from,
             around,
@@ -576,7 +578,7 @@ impl<CW: Copy, R: AccessRules> Drawing<CW, R> {
         bend_weight: LooseBendWeight,
         cw: bool,
         infringables: Option<&[PrimitiveIndex]>,
-    ) -> Result<Cane, LayoutException> {
+    ) -> Result<Cane, DrawingException> {
         let seg_to = self.add_dot_with_infringables(dot_weight, infringables)?;
         let seg = self
             .add_seg_with_infringables(from, seg_to.into(), seg_weight, infringables)
@@ -604,7 +606,7 @@ impl<CW: Copy, R: AccessRules> Drawing<CW, R> {
                 err
             })?;
 
-        Ok::<Cane, LayoutException>(Cane {
+        Ok::<Cane, DrawingException>(Cane {
             seg,
             dot: seg_to,
             bend,

src/layout/layout.rs

@@ -17,7 +17,7 @@ use crate::{
             FixedSegIndex, FixedSegWeight, LoneLooseSegIndex, LoneLooseSegWeight, SeqLooseSegIndex,
             SeqLooseSegWeight,
         },
-        Drawing, Infringement, LayoutException,
+        Drawing, DrawingException, Infringement,
     },
     geometry::{compound::ManageCompounds, shape::MeasureLength, GenericNode},
     graph::{GenericIndex, GetPetgraphIndex},
@@ -54,7 +54,7 @@ impl<R: AccessRules> Layout<R> {
         seg_weight: SeqLooseSegWeight,
         bend_weight: LooseBendWeight,
         cw: bool,
-    ) -> Result<Cane, LayoutException> {
+    ) -> Result<Cane, DrawingException> {
         self.drawing
             .insert_cane(from, around, dot_weight, seg_weight, bend_weight, cw)
     }
@@ -204,8 +204,8 @@ impl<R: AccessRules> Layout<R> {
         )
     }
 
-    pub fn remove_band(&mut self, band: BandTermsegIndex) {
+    pub fn remove_band(&mut self, band: BandTermsegIndex) -> Result<(), DrawingException> {
-        self.drawing.remove_band(band);
+        self.drawing.remove_band(band)
     }
 
     pub fn polys<W: 'static>(

src/router/draw.rs

@@ -14,7 +14,7 @@ use crate::{
         primitive::GetOtherJoint,
         rules::AccessRules,
         seg::{LoneLooseSegWeight, SeqLooseSegWeight},
-        Infringement, LayoutException,
+        DrawingException, Infringement,
     },
     layout::Layout,
     math::{Circle, NoTangents},
@@ -26,9 +26,9 @@ pub enum DrawException {
     NoTangents(#[from] NoTangents),
     // TODO add real error messages + these should eventually use Display
     #[error("cannot finish in {0:?}")]
-    CannotFinishIn(FixedDotIndex, #[source] LayoutException),
+    CannotFinishIn(FixedDotIndex, #[source] DrawingException),
     #[error("cannot wrap around {0:?}")]
-    CannotWrapAround(GearIndex, #[source] LayoutException),
+    CannotWrapAround(GearIndex, #[source] DrawingException),
 }
 
 pub struct Draw<'a, R: AccessRules> {
@@ -158,7 +158,7 @@ impl<'a, R: AccessRules> Draw<'a, R> {
         cw: bool,
         width: f64,
         offset: f64,
-    ) -> Result<CaneHead, LayoutException> {
+    ) -> Result<CaneHead, DrawingException> {
         let head = self.extend_head(head, from)?;
         self.cane(head, around, to, cw, width, offset)
     }
@@ -183,7 +183,7 @@ impl<'a, R: AccessRules> Draw<'a, R> {
         cw: bool,
         width: f64,
         offset: f64,
-    ) -> Result<CaneHead, LayoutException> {
+    ) -> Result<CaneHead, DrawingException> {
         let layer = head.face().primitive(self.layout.drawing()).layer();
         let maybe_net = head.face().primitive(self.layout.drawing()).maybe_net();
         let cane = self.layout.insert_cane(
@@ -210,7 +210,7 @@ impl<'a, R: AccessRules> Draw<'a, R> {
             },
             cw,
         )?;
-        Ok::<CaneHead, LayoutException>(CaneHead {
+        Ok::<CaneHead, DrawingException>(CaneHead {
             face: self
                 .layout
                 .drawing()

src/router/router.rs

@@ -12,7 +12,7 @@ use crate::{
         head::GetFace,
         primitive::MakePrimitiveShape,
         rules::AccessRules,
-        Collision, Infringement, LayoutException,
+        Collision, DrawingException, Infringement,
     },
     geometry::{
         primitive::{AccessPrimitiveShape, PrimitiveShape},
@@ -140,12 +140,14 @@ impl<'a, R: AccessRules> AstarStrategy<Navmesh, f64, BandTermsegIndex>
                     };
 
                     let (ghost, obstacle) = match layout_err {
-                        LayoutException::NoTangents(..) => return None,
+                        DrawingException::NoTangents(..) => return None,
-                        LayoutException::Infringement(Infringement(ghost, obstacle)) => {
+                        DrawingException::Infringement(Infringement(ghost, obstacle)) => {
                             (ghost, obstacle)
                         }
-                        LayoutException::Collision(Collision(ghost, obstacle)) => (ghost, obstacle),
+                        DrawingException::Collision(Collision(ghost, obstacle)) => {
-                        LayoutException::AlreadyConnected(..) => return None,
+                            (ghost, obstacle)
+                        }
+                        DrawingException::AlreadyConnected(..) => return None,
                     };
 
                     self.probe_ghosts = vec![ghost];