From b75101cb8373977289d529ef7be5d2eb84d393f8 Mon Sep 17 00:00:00 2001 From: Mikolaj Wielgus Date: Mon, 19 May 2025 11:21:07 +0200 Subject: [PATCH] fix(router/router): Replace LoS-based A* termination with correctly searched one --- src/drawing/band.rs | 15 +++++++++++--- src/drawing/drawing.rs | 11 ++++++++++ src/layout/layout.rs | 4 ++++ src/router/astar.rs | 4 ++-- src/router/navcord.rs | 46 ++++++++++++++++++++++++++++++------------ src/router/router.rs | 31 +++++++++++++++++++--------- 6 files changed, 84 insertions(+), 27 deletions(-) diff --git a/src/drawing/band.rs b/src/drawing/band.rs index eaad831..9ae98f8 100644 --- a/src/drawing/band.rs +++ b/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 for LooseIndex { - fn from(terminating_seg: BandTermsegIndex) -> Self { - match terminating_seg { + fn from(termseg: BandTermsegIndex) -> Self { + match termseg { BandTermsegIndex::Straight(seg) => LooseIndex::LoneSeg(seg), BandTermsegIndex::Bended(seg) => LooseIndex::SeqSeg(seg), } } } +impl From 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> for BandTermsegIndex { type Output = BandRef<'a, CW, Cel, R>; fn ref_(&self, drawing: &'a Drawing) -> BandRef<'a, CW, Cel, R> { diff --git a/src/drawing/drawing.rs b/src/drawing/drawing.rs index cb9ed63..3e96252 100644 --- a/src/drawing/drawing.rs +++ b/src/drawing/drawing.rs @@ -406,6 +406,17 @@ impl Drawing { 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, + 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()))] diff --git a/src/layout/layout.rs b/src/layout/layout.rs index 54340ef..2fef52f 100644 --- a/src/layout/layout.rs +++ b/src/layout/layout.rs @@ -102,6 +102,10 @@ impl Layout { 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`] diff --git a/src/router/astar.rs b/src/router/astar.rs index f88868e..b5613cf 100644 --- a/src/router/astar.rs +++ b/src/router/astar.rs @@ -132,7 +132,7 @@ where pub maybe_curr_node: Option, // FIXME: To work around edge references borrowing from the graph we collect then reiterate over them. pub edge_ids: VecDeque, - // 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); diff --git a/src/router/navcord.rs b/src/router/navcord.rs index 978f2a9..a48e23e 100644 --- a/src/router/navcord.rs +++ b/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 currently running routing process. +/// The `Navcord` is a data structure that holds the movable non-borrowing data +/// 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, /// 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, /// 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( @@ -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 - // `to` onto the currently attempted path to start from it on - // the next `.step_to(...)` call or retreat from it later using + self.final_termseg = Some(layout.finish(navmesh, self, to_dot).unwrap()); + + // 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,11 +135,16 @@ impl Navcord { &mut self, layout: &mut Layout, ) -> Result<(), NavcorderException> { - if let Head::Cane(head) = self.head { - self.head = layout.undo_cane(&mut self.recorder, head).unwrap(); + if let Some(final_termseg) = self.final_termseg { + layout.remove_termseg(&mut self.recorder, final_termseg); + self.final_termseg = None; } else { - // "can't unwrap" - return Err(NavcorderException::CannotWrap); + 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 diff --git a/src/router/router.rs b/src/router/router.rs index b63de5e..88dd9f9 100644 --- a/src/router/router.rs +++ b/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 AstarStrategy for RouterAst ) -> Option { let new_path = tracker.reconstruct_path_to(vertex); - self.layout - .rework_path(navmesh, self.navcord, &new_path[..]) - .unwrap(); + if vertex == navmesh.destination_navvertex() { + self.layout + .rework_path(navmesh, self.navcord, &new_path[..new_path.len() - 1]) + .unwrap(); - self.layout.finish(navmesh, self.navcord, self.target).ok() + // 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 + } } fn place_probe(&mut self, navmesh: &Navmesh, edge: NavmeshEdgeReference) -> Option { - 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;