fix(drawing/band): BandTermsegIndex computation should be fallible

Fixes #221.

src/autorouter/autoroute.rs

@@ -125,7 +125,8 @@ impl<M: AccessMesadata> Step<Autorouter<M>, Option<LayoutEdit>, AutorouteContinu
                 .board
                 .layout()
                 .drawing()
-                .loose_band_uid(band_termseg.into());
+                .loose_band_uid(band_termseg.into())
+                .expect("a completely routed band should've Seg's as ends");
 
             autorouter
                 .ratsnest

src/autorouter/selection.rs

@@ -125,7 +125,7 @@ impl BandSelector {
             _ => return None,
         };
 
-        Self::try_from_uid(board, &board.layout().drawing().loose_band_uid(loose))
+        Self::try_from_uid(board, &board.layout().drawing().loose_band_uid(loose).ok()?)
     }
 
     pub fn try_from_uid(

src/board/mod.rs

@@ -60,8 +60,10 @@ impl<'a> ResolvedSelector<'a> {
         if let Some(pin_name) = board.node_pinname(&node) {
             Some(ResolvedSelector::Pin { pin_name, layer })
         } else {
-            loose.map(|loose| ResolvedSelector::Band {
+            loose.and_then(|loose| {
-                band_uid: board.layout().drawing().loose_band_uid(loose),
+                Some(ResolvedSelector::Band {
+                    band_uid: board.layout().drawing().loose_band_uid(loose).ok()?,
+                })
             })
         }
     }

src/drawing/band.rs

@@ -2,12 +2,9 @@
 //
 // SPDX-License-Identifier: MIT
 
-use enum_dispatch::enum_dispatch;
-use petgraph::stable_graph::NodeIndex;
-
 use crate::{
     geometry::{shape::MeasureLength, GetLayer},
-    graph::{GetPetgraphIndex, MakeRef},
+    graph::MakeRef,
 };
 
 use super::{
@@ -15,36 +12,12 @@ use super::{
     loose::{GetPrevNextLoose, LooseIndex},
     primitive::MakePrimitiveShape,
     rules::AccessRules,
-    seg::{LoneLooseSegIndex, SegIndex, SeqLooseSegIndex},
+    seg::LooseSegIndex,
     Drawing,
 };
 
 pub type BandUid = planar_incr_embed::navmesh::OrderedPair<BandTermsegIndex>;
-
+pub type BandTermsegIndex = LooseSegIndex;
-#[enum_dispatch(GetPetgraphIndex)]
-#[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd, Hash)]
-pub enum BandTermsegIndex {
-    Straight(LoneLooseSegIndex),
-    Bended(SeqLooseSegIndex),
-}
-
-impl From<BandTermsegIndex> for LooseIndex {
-    fn from(termseg: BandTermsegIndex) -> Self {
-        match termseg {
-            BandTermsegIndex::Straight(seg) => LooseIndex::LoneSeg(seg),
-            BandTermsegIndex::Bended(seg) => LooseIndex::SeqSeg(seg),
-        }
-    }
-}
-
-impl From<BandTermsegIndex> for SegIndex {
-    fn from(termseg: BandTermsegIndex) -> Self {
-        match termseg {
-            BandTermsegIndex::Straight(seg) => SegIndex::LoneLoose(seg),
-            BandTermsegIndex::Bended(seg) => SegIndex::SeqLoose(seg),
-        }
-    }
-}
 
 impl<'a, CW: 'a, Cel: 'a, R: 'a> MakeRef<'a, Drawing<CW, Cel, R>> for BandTermsegIndex {
     type Output = BandRef<'a, CW, Cel, R>;
@@ -69,35 +42,30 @@ impl<'a, CW: 'a, Cel: 'a, R: 'a> BandRef<'a, CW, Cel, R> {
 
 impl<CW: Clone, Cel: Copy, R: AccessRules> GetLayer for BandRef<'_, CW, Cel, R> {
     fn layer(&self) -> usize {
-        match self.first_seg {
+        self.first_seg.primitive(self.drawing).layer()
-            BandTermsegIndex::Straight(seg) => seg.primitive(self.drawing),
-            BandTermsegIndex::Bended(seg) => seg.primitive(self.drawing),
-        }
-        .layer()
     }
 }
 
 impl<CW: Clone, Cel: Copy, R: AccessRules> MeasureLength for BandRef<'_, CW, Cel, R> {
     fn length(&self) -> f64 {
-        match self.first_seg {
+        let mut maybe_loose: Option<LooseIndex> = Some(match self.first_seg {
-            BandTermsegIndex::Straight(seg) => {
+            BandTermsegIndex::Lone(seg) => {
-                self.drawing.geometry().seg_shape(seg.into()).length()
+                return self.drawing.geometry().seg_shape(seg.into()).length();
             }
-            BandTermsegIndex::Bended(first_loose_seg) => {
+            BandTermsegIndex::Seq(first_loose_seg) => first_loose_seg.into(),
-                let mut maybe_loose: Option<LooseIndex> = Some(first_loose_seg.into());
+        });
-                let mut prev = None;
-                let mut length = 0.0;
 
-                while let Some(loose) = maybe_loose {
+        let mut prev = None;
-                    length += loose.primitive(self.drawing).shape().length();
+        let mut length = 0.0;
 
-                    let prev_prev = prev;
+        while let Some(loose) = maybe_loose {
-                    prev = maybe_loose;
+            length += loose.primitive(self.drawing).shape().length();
-                    maybe_loose = self.drawing.loose(loose).next_loose(prev_prev);
-                }
 
-                length
+            let prev_prev = prev;
-            }
+            prev = maybe_loose;
+            maybe_loose = self.drawing.loose(loose).next_loose(prev_prev);
         }
+
+        length
     }
 }

src/drawing/collect.rs

@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: MIT
 
-use crate::graph::{GenericIndex, GetPetgraphIndex, MakeRef};
+use crate::graph::MakeRef;
 
 use super::{
     band::{BandTermsegIndex, BandUid},
@@ -15,8 +15,14 @@ use super::{
     Drawing,
 };
 
+#[derive(Clone, Debug, thiserror::Error)]
+#[error("unable to resolve Loose to BandUid")]
+pub struct BandUidError {
+    pub maybe_end: Option<BandTermsegIndex>,
+}
+
 pub trait Collect {
-    fn loose_band_uid(&self, start_loose: LooseIndex) -> BandUid;
+    fn loose_band_uid(&self, start_loose: LooseIndex) -> Result<BandUid, BandUidError>;
 
     fn bend_bow(&self, bend: LooseBendIndex) -> Vec<PrimitiveIndex>;
 
@@ -26,11 +32,15 @@ pub trait Collect {
 }
 
 impl<CW: Clone, Cel: Copy, R: AccessRules> Collect for Drawing<CW, Cel, R> {
-    fn loose_band_uid(&self, start_loose: LooseIndex) -> BandUid {
+    fn loose_band_uid(&self, start_loose: LooseIndex) -> Result<BandUid, BandUidError> {
-        BandUid::from((
+        match (
             self.loose_band_first_seg(start_loose),
             self.loose_band_last_seg(start_loose),
-        ))
+        ) {
+            (Some(first), Some(last)) => Ok(BandUid::from((first, last))),
+            (Some(x), None) | (None, Some(x)) => Err(BandUidError { maybe_end: Some(x) }),
+            (None, None) => Err(BandUidError { maybe_end: None }),
+        }
     }
 
     fn bend_bow(&self, bend: LooseBendIndex) -> Vec<PrimitiveIndex> {
@@ -88,14 +98,14 @@ impl<CW: Clone, Cel: Copy, R: AccessRules> Collect for Drawing<CW, Cel, R> {
 }
 
 trait CollectPrivate {
-    fn loose_band_first_seg(&self, start_loose: LooseIndex) -> BandTermsegIndex;
+    fn loose_band_first_seg(&self, start_loose: LooseIndex) -> Option<BandTermsegIndex>;
-    fn loose_band_last_seg(&self, start_loose: LooseIndex) -> BandTermsegIndex;
+    fn loose_band_last_seg(&self, start_loose: LooseIndex) -> Option<BandTermsegIndex>;
 }
 
 impl<CW: Clone, Cel: Copy, R: AccessRules> CollectPrivate for Drawing<CW, Cel, R> {
-    fn loose_band_first_seg(&self, start_loose: LooseIndex) -> BandTermsegIndex {
+    fn loose_band_first_seg(&self, start_loose: LooseIndex) -> Option<BandTermsegIndex> {
         if let LooseIndex::LoneSeg(seg) = start_loose {
-            return BandTermsegIndex::Straight(seg);
+            return Some(BandTermsegIndex::Lone(seg));
         }
 
         let mut loose = start_loose;
@@ -106,14 +116,14 @@ impl<CW: Clone, Cel: Copy, R: AccessRules> CollectPrivate for Drawing<CW, Cel, R
                 prev = Some(loose);
                 loose = next_loose;
             } else {
-                return BandTermsegIndex::Bended(GenericIndex::new(loose.petgraph_index()));
+                return loose.try_into().ok();
             }
         }
     }
 
-    fn loose_band_last_seg(&self, start_loose: LooseIndex) -> BandTermsegIndex {
+    fn loose_band_last_seg(&self, start_loose: LooseIndex) -> Option<BandTermsegIndex> {
         if let LooseIndex::LoneSeg(seg) = start_loose {
-            return BandTermsegIndex::Straight(seg);
+            return Some(BandTermsegIndex::Lone(seg));
         }
 
         let mut loose = start_loose;
@@ -124,7 +134,7 @@ impl<CW: Clone, Cel: Copy, R: AccessRules> CollectPrivate for Drawing<CW, Cel, R
                 next = Some(loose);
                 loose = prev_loose;
             } else {
-                return BandTermsegIndex::Bended(GenericIndex::new(loose.petgraph_index()));
+                return loose.try_into().ok();
             }
         }
     }

src/drawing/drawing.rs

@@ -34,7 +34,6 @@ use crate::{
         edit::{ApplyGeometryEdit, GeometryEdit},
         primitive::{AccessPrimitiveShape, PrimitiveShape},
         recording_with_rtree::RecordingGeometryWithRtree,
-        shape::MeasureLength,
         with_rtree::BboxedIndex,
         AccessBendWeight, AccessDotWeight, AccessSegWeight, GenericNode, Geometry, GeometryLabel,
         GetLayer, GetOffset, GetSetPos, GetWidth,
@@ -214,72 +213,70 @@ impl<CW: Clone, Cel: Copy, R: AccessRules> Drawing<CW, Cel, R> {
         recorder: &mut DrawingEdit<CW, Cel>,
         band: BandTermsegIndex,
     ) -> Result<(), DrawingException> {
-        match band {
+        let mut maybe_loose: Option<LooseIndex> = Some(match band {
-            BandTermsegIndex::Straight(seg) => {
+            BandTermsegIndex::Lone(seg) => {
                 self.recording_geometry_with_rtree
                     .remove_seg(recorder, seg.into());
+                return Ok(());
             }
-            BandTermsegIndex::Bended(first_loose_seg) => {
+            BandTermsegIndex::Seq(first_loose_seg) => first_loose_seg.into(),
-                let mut dots = vec![];
+        });
-                let mut segs = vec![];
-                let mut bends = vec![];
-                let mut outers = vec![];
 
-                let mut maybe_loose = Some(first_loose_seg.into());
+        let mut dots = vec![];
-                let mut prev = None;
+        let mut segs = vec![];
+        let mut bends = vec![];
+        let mut outers = vec![];
+        let mut prev = None;
 
-                while let Some(loose) = maybe_loose {
+        while let Some(loose) = maybe_loose {
-                    match loose {
+            match loose {
-                        LooseIndex::Dot(dot) => {
+                LooseIndex::Dot(dot) => {
-                            dots.push(dot);
+                    dots.push(dot);
-                        }
-                        LooseIndex::LoneSeg(seg) => {
-                            self.recording_geometry_with_rtree
-                                .remove_seg(recorder, seg.into());
-                            break;
-                        }
-                        LooseIndex::SeqSeg(seg) => {
-                            segs.push(seg);
-                        }
-                        LooseIndex::Bend(bend) => {
-                            bends.push(bend);
-
-                            if let Some(outer) = self.primitive(bend).outer() {
-                                outers.push(outer);
-                                self.reattach_bend(recorder, outer, self.primitive(bend).inner());
-                            }
-                        }
-                    }
-
-                    let prev_prev = prev;
-                    prev = maybe_loose;
-                    maybe_loose = self.loose(loose).next_loose(prev_prev);
                 }
-
+                LooseIndex::LoneSeg(seg) => {
-                for bend in bends {
-                    self.recording_geometry_with_rtree
-                        .remove_bend(recorder, bend.into());
-                }
-
-                for seg in segs {
                     self.recording_geometry_with_rtree
                         .remove_seg(recorder, seg.into());
+                    break;
                 }
-
+                LooseIndex::SeqSeg(seg) => {
-                // We must remove the dots only after the segs and bends because we need dots to calculate
+                    segs.push(seg);
-                // the shapes, which we first need unchanged to remove the segs and bends from the R-tree.
-
-                for dot in dots {
-                    self.recording_geometry_with_rtree
-                        .remove_dot(recorder, dot.into());
                 }
+                LooseIndex::Bend(bend) => {
+                    bends.push(bend);
 
-                for outer in outers {
+                    if let Some(outer) = self.primitive(bend).outer() {
-                    self.update_this_and_outward_bows(recorder, outer)?;
+                        outers.push(outer);
+                        self.reattach_bend(recorder, outer, self.primitive(bend).inner());
+                    }
                 }
             }
+
+            let prev_prev = prev;
+            prev = maybe_loose;
+            maybe_loose = self.loose(loose).next_loose(prev_prev);
         }
 
+        for bend in bends {
+            self.recording_geometry_with_rtree
+                .remove_bend(recorder, bend.into());
+        }
+
+        for seg in segs {
+            self.recording_geometry_with_rtree
+                .remove_seg(recorder, seg.into());
+        }
+
+        // We must remove the dots only after the segs and bends because we need dots to calculate
+        // the shapes, which we first need unchanged to remove the segs and bends from the R-tree.
+
+        for dot in dots {
+            self.recording_geometry_with_rtree
+                .remove_dot(recorder, dot.into());
+        }
+
+        for outer in outers {
+            self.update_this_and_outward_bows(recorder, outer)?;
+        }
         Ok(())
     }
 
@@ -414,7 +411,7 @@ impl<CW: Clone, Cel: Copy, R: AccessRules> Drawing<CW, Cel, R> {
         termseg: BandTermsegIndex,
     ) {
         self.recording_geometry_with_rtree
-            .remove_seg(recorder, termseg.into());
+            .remove_seg(recorder, termseg.into())
     }
 
     #[debug_ensures(ret.is_ok() -> self.recording_geometry_with_rtree.graph().node_count() == old(self.recording_geometry_with_rtree.graph().node_count() + 1))]
@@ -866,6 +863,8 @@ impl<CW: Clone, Cel: Copy, R: AccessRules> Drawing<CW, Cel, R> {
                     .remove_dot(recorder, seg_to.into());
             })?;
 
+        #[cfg(debug_assertions)]
+        use crate::geometry::shape::MeasureLength;
         #[cfg(debug_assertions)]
         approx::assert_abs_diff_eq!(bend.primitive(self).shape().length(), 0.0);
 

src/drawing/seg.rs

@@ -7,6 +7,7 @@ use enum_dispatch::enum_dispatch;
 use crate::{
     drawing::{
         graph::{GetMaybeNet, MakePrimitive, PrimitiveIndex, PrimitiveWeight},
+        loose::LooseIndex,
         primitive::{GenericPrimitive, Primitive},
         rules::AccessRules,
         Drawing,
@@ -18,13 +19,62 @@ use crate::{
 use petgraph::stable_graph::NodeIndex;
 
 #[enum_dispatch(GetPetgraphIndex, MakePrimitive)]
-#[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd)]
+#[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd, Hash)]
 pub enum SegIndex {
     Fixed(FixedSegIndex),
     LoneLoose(LoneLooseSegIndex),
     SeqLoose(SeqLooseSegIndex),
 }
 
+#[enum_dispatch(GetPetgraphIndex, MakePrimitive)]
+#[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd, Hash)]
+pub enum LooseSegIndex {
+    Lone(LoneLooseSegIndex),
+    Seq(SeqLooseSegIndex),
+}
+
+impl From<LooseSegIndex> for SegIndex {
+    fn from(seg: LooseSegIndex) -> Self {
+        match seg {
+            LooseSegIndex::Lone(seg) => SegIndex::LoneLoose(seg),
+            LooseSegIndex::Seq(seg) => SegIndex::SeqLoose(seg),
+        }
+    }
+}
+
+impl From<LooseSegIndex> for LooseIndex {
+    fn from(seg: LooseSegIndex) -> Self {
+        match seg {
+            LooseSegIndex::Lone(seg) => LooseIndex::LoneSeg(seg),
+            LooseSegIndex::Seq(seg) => LooseIndex::SeqSeg(seg),
+        }
+    }
+}
+
+impl TryFrom<SegIndex> for LooseSegIndex {
+    type Error = (); // TODO.
+
+    fn try_from(index: SegIndex) -> Result<LooseSegIndex, ()> {
+        Ok(match index {
+            SegIndex::LoneLoose(index) => LooseSegIndex::Lone(index),
+            SegIndex::SeqLoose(index) => LooseSegIndex::Seq(index),
+            _ => return Err(()),
+        })
+    }
+}
+
+impl TryFrom<LooseIndex> for LooseSegIndex {
+    type Error = (); // TODO.
+
+    fn try_from(index: LooseIndex) -> Result<LooseSegIndex, ()> {
+        Ok(match index {
+            LooseIndex::LoneSeg(index) => LooseSegIndex::Lone(index),
+            LooseIndex::SeqSeg(index) => LooseSegIndex::Seq(index),
+            _ => return Err(()),
+        })
+    }
+}
+
 impl From<SegIndex> for PrimitiveIndex {
     fn from(seg: SegIndex) -> Self {
         match seg {
@@ -39,12 +89,12 @@ impl TryFrom<PrimitiveIndex> for SegIndex {
     type Error = (); // TODO.
 
     fn try_from(index: PrimitiveIndex) -> Result<SegIndex, ()> {
-        match index {
+        Ok(match index {
-            PrimitiveIndex::FixedSeg(index) => Ok(SegIndex::Fixed(index)),
+            PrimitiveIndex::FixedSeg(index) => SegIndex::Fixed(index),
-            PrimitiveIndex::LoneLooseSeg(index) => Ok(SegIndex::LoneLoose(index)),
+            PrimitiveIndex::LoneLooseSeg(index) => SegIndex::LoneLoose(index),
-            PrimitiveIndex::SeqLooseSeg(index) => Ok(SegIndex::SeqLoose(index)),
+            PrimitiveIndex::SeqLooseSeg(index) => SegIndex::SeqLoose(index),
-            _ => Err(()),
+            _ => return Err(()),
-        }
+        })
     }
 }
 

src/layout/layout.rs

@@ -389,7 +389,7 @@ impl<R: AccessRules> Layout<R> {
                 (loose, shape)
             })
             .filter_map(|(loose, shape)| {
-                let band_uid = self.drawing.loose_band_uid(loose);
+                let band_uid = self.drawing.loose_band_uid(loose).ok()?;
                 let loose_hline = orig_hline.orthogonal_through(&match shape {
                     PrimitiveShape::Seg(seg) => {
                         let seg_hline = NormalLine::from(seg.middle_line());

src/router/draw.rs

@@ -105,7 +105,7 @@ impl<R: AccessRules> Draw for Layout<R> {
                         maybe_net,
                     }),
                 )
-                .map(BandTermsegIndex::Straight),
+                .map(BandTermsegIndex::Lone),
             DotIndex::Loose(dot) => self
                 .add_seq_loose_seg(
                     recorder,
@@ -117,7 +117,7 @@ impl<R: AccessRules> Draw for Layout<R> {
                         maybe_net,
                     }),
                 )
-                .map(BandTermsegIndex::Bended),
+                .map(BandTermsegIndex::Seq),
         }
         .map_err(|err| {
             // move the head back to where it came from