fix(router/router): Replace LoS-based A* termination with correctly searched one

src/drawing/band.rs

@@ -15,7 +15,7 @@ use super::{
     loose::{GetPrevNextLoose, LooseIndex},
     primitive::MakePrimitiveShape,
     rules::AccessRules,
-    seg::{LoneLooseSegIndex, SeqLooseSegIndex},
+    seg::{LoneLooseSegIndex, SegIndex, SeqLooseSegIndex},
     Drawing,
 };
 
@@ -29,14 +29,23 @@ pub enum BandTermsegIndex {
 }
 
 impl From<BandTermsegIndex> for LooseIndex {
-    fn from(terminating_seg: BandTermsegIndex) -> Self {
+    fn from(termseg: BandTermsegIndex) -> Self {
-        match terminating_seg {
+        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>;
     fn ref_(&self, drawing: &'a Drawing<CW, Cel, R>) -> BandRef<'a, CW, Cel, R> {

src/drawing/drawing.rs

@@ -406,6 +406,17 @@ impl<CW: Clone, Cel: Copy, R: AccessRules> Drawing<CW, Cel, R> {
         Ok(seg)
     }
 
+    #[debug_ensures(self.recording_geometry_with_rtree.graph().node_count() == old(self.recording_geometry_with_rtree.graph().node_count() - 1))]
+    #[debug_ensures(self.recording_geometry_with_rtree.graph().edge_count() == old(self.recording_geometry_with_rtree.graph().edge_count() - 2))]
+    pub fn remove_termseg(
+        &mut self,
+        recorder: &mut DrawingEdit<CW, Cel>,
+        termseg: BandTermsegIndex,
+    ) {
+        self.recording_geometry_with_rtree
+            .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))]
     #[debug_ensures(ret.is_ok() -> self.recording_geometry_with_rtree.graph().edge_count() >= old(self.recording_geometry_with_rtree.graph().edge_count() + 2))]
     #[debug_ensures(ret.is_err() -> self.recording_geometry_with_rtree.graph().node_count() == old(self.recording_geometry_with_rtree.graph().node_count()))]

src/layout/layout.rs

@@ -102,6 +102,10 @@ impl<R: AccessRules> Layout<R> {
         self.drawing.remove_cane(recorder, cane, face)
     }
 
+    pub fn remove_termseg(&mut self, recorder: &mut LayoutEdit, termseg: BandTermsegIndex) {
+        self.drawing.remove_termseg(recorder, termseg)
+    }
+
     #[debug_ensures(ret.is_ok() -> self.drawing.node_count() == old(self.drawing.node_count()) + weight.to_layer - weight.from_layer + 2)]
     #[debug_ensures(ret.is_err() -> self.drawing.node_count() == old(self.drawing.node_count()))]
     /// Insert [`Via`] into the [`Layout`]

src/router/astar.rs

@@ -132,7 +132,7 @@ where
     pub maybe_curr_node: Option<G::NodeId>,
     // FIXME: To work around edge references borrowing from the graph we collect then reiterate over them.
     pub edge_ids: VecDeque<G::EdgeId>,
-    // TODO: Rewrite this to be a well-designed state machine.
+    // TODO: Rewrite this to be a well-designed state machine. Booleans are a code smell.
     pub is_probing: bool,
 }
 
@@ -233,7 +233,7 @@ where
 
             if let Some(edge_id) = self.edge_ids.pop_front() {
                 // This lookup can be unwrapped without fear of panic since the node was
-                // necessarily scored before adding it to `visit_next`.
+                // necessarily scored before adding it to `.visit_next`.
                 let node_score = self.scores[&curr_node];
                 let edge = (&self.graph).edge_ref(edge_id);
 

src/router/navcord.rs

@@ -7,6 +7,7 @@ use petgraph::data::DataMap;
 
 use crate::{
     drawing::{
+        band::BandTermsegIndex,
         dot::FixedDotIndex,
         head::{BareHead, CaneHead, Head},
         rules::AccessRules,
@@ -16,12 +17,12 @@ use crate::{
 
 use super::{
     draw::Draw,
-    navcorder::NavcorderException,
+    navcorder::{Navcorder, NavcorderException},
     navmesh::{BinavvertexNodeIndex, Navmesh, NavvertexIndex},
 };
 
-/// The navcord is a data structure that holds the movable non-borrowing data of
+/// The `Navcord` is a data structure that holds the movable non-borrowing data
-/// the currently running routing process.
+/// of the currently running routing process.
 ///
 /// Note that this data structure is not a stepper, since steppers always
 /// progress linearly, whereas `Navcord` branches out to different states
@@ -37,6 +38,8 @@ pub struct Navcord {
     pub path: Vec<NavvertexIndex>,
     /// The head of the currently routed band.
     pub head: Head,
+    /// If the band is finished, stores the termseg that was used to finish it.
+    pub final_termseg: Option<BandTermsegIndex>,
     /// The width of the currently routed band.
     pub width: f64,
 }
@@ -53,6 +56,7 @@ impl Navcord {
             recorder,
             path: vec![source_navvertex],
             head: BareHead { face: source }.into(),
+            final_termseg: None,
             width,
         }
     }
@@ -94,7 +98,6 @@ impl Navcord {
 
     /// Advance the navcord and the currently routed band by one step to the
     /// navvertex `to`.
-    #[debug_ensures(ret.is_ok() -> matches!(self.head, Head::Cane(..)))]
     #[debug_ensures(ret.is_ok() -> self.path.len() == old(self.path.len() + 1))]
     #[debug_ensures(ret.is_err() -> self.path.len() == old(self.path.len()))]
     pub fn step_to<R: AccessRules>(
@@ -103,14 +106,26 @@ impl Navcord {
         navmesh: &Navmesh,
         to: NavvertexIndex,
     ) -> Result<(), NavcorderException> {
-        self.head = self.wrap(layout, navmesh, self.head, to)?.into();
+        if to == navmesh.destination_navvertex() {
+            let to_node_weight = navmesh.node_weight(to).unwrap();
+            let BinavvertexNodeIndex::FixedDot(to_dot) = to_node_weight.node else {
+                unreachable!();
+            };
 
-        // Now that the new head has been created, push the navvertex
+            self.final_termseg = Some(layout.finish(navmesh, self, to_dot).unwrap());
-        // `to` onto the currently attempted path to start from it on
+
-        // the next `.step_to(...)` call or retreat from it later using
+            // NOTE: We don't update the head here because there is currently
+            // no head variant that consists only of a seg, and I'm not sure if
+            // there should be one.
+        } else {
+            self.head = self.wrap(layout, navmesh, self.head, to)?.into();
+        }
+
+        // Now that the new part of the trace has been created, push the
+        // navvertex `to` onto the currently attempted path to start from it
+        // on the next `.step_to(...)` call or retreat from it later using
         // `.step_back(...)`.
         self.path.push(to);
-
         Ok(())
     }
 
@@ -120,12 +135,17 @@ impl Navcord {
         &mut self,
         layout: &mut Layout<R>,
     ) -> Result<(), NavcorderException> {
+        if let Some(final_termseg) = self.final_termseg {
+            layout.remove_termseg(&mut self.recorder, final_termseg);
+            self.final_termseg = None;
+        } else {
             if let Head::Cane(head) = self.head {
                 self.head = layout.undo_cane(&mut self.recorder, head).unwrap();
             } else {
                 // "can't unwrap"
                 return Err(NavcorderException::CannotWrap);
             }
+        }
 
         // Now that the last head of the currently routed band was deleted, pop
         // the last navvertex from the currently attempted path so that it is up

src/router/router.rs

@@ -19,7 +19,7 @@ use crate::{
         primitive::PrimitiveShape,
         shape::{AccessShape, MeasureLength},
     },
-    graph::{GetPetgraphIndex, MakeRef},
+    graph::MakeRef,
     layout::{Layout, LayoutEdit},
 };
 
@@ -69,21 +69,34 @@ impl<R: AccessRules> AstarStrategy<Navmesh, f64, BandTermsegIndex> for RouterAst
     ) -> Option<BandTermsegIndex> {
         let new_path = tracker.reconstruct_path_to(vertex);
 
+        if vertex == navmesh.destination_navvertex() {
+            self.layout
+                .rework_path(navmesh, self.navcord, &new_path[..new_path.len() - 1])
+                .unwrap();
+
+            // Set navcord members for consistency. The code would probably work
+            // without this, since A* will terminate now.
+            self.navcord.final_termseg = Some(
+                self.layout
+                    .finish(navmesh, self.navcord, self.target)
+                    .unwrap(),
+            );
+            self.navcord.path.push(vertex);
+
+            self.navcord.final_termseg
+        } else {
             self.layout
                 .rework_path(navmesh, self.navcord, &new_path[..])
                 .unwrap();
-
+            None
-        self.layout.finish(navmesh, self.navcord, self.target).ok()
+        }
     }
 
     fn place_probe(&mut self, navmesh: &Navmesh, edge: NavmeshEdgeReference) -> Option<f64> {
-        if edge.target().petgraph_index() == self.target.petgraph_index() {
-            return None;
-        }
-
         let old_head = self.navcord.head;
         let prev_head_length = old_head.ref_(self.layout.drawing()).length();
         let result = self.navcord.step_to(self.layout, navmesh, edge.target());
+
         let probe_length = self.navcord.head.ref_(self.layout.drawing()).length()
             + old_head.ref_(self.layout.drawing()).length()
             - prev_head_length;