From d50e346706613daaa2e64da1e4405f896ac73e6f Mon Sep 17 00:00:00 2001 From: Mikolaj Wielgus Date: Wed, 22 Oct 2025 01:36:29 +0200 Subject: [PATCH] feat(autorouter/autorouter): Remove "compare detours" This was a time-expensive way to presort ratlines, it's much faster to instead operate on ratline line segments. --- committed.toml | 1 - crates/topola-egui/src/actions.rs | 8 -- crates/topola-egui/src/menu_bar.rs | 11 --- locales/en-US/main.ftl | 3 + src/autorouter/autorouter.rs | 45 ----------- src/autorouter/compare_detours.rs | 126 ----------------------------- src/autorouter/execution.rs | 22 ----- src/autorouter/invoker.rs | 4 - src/autorouter/mod.rs | 1 - 9 files changed, 3 insertions(+), 218 deletions(-) delete mode 100644 src/autorouter/compare_detours.rs diff --git a/committed.toml b/committed.toml index fc1c1e1..6629fd7 100644 --- a/committed.toml +++ b/committed.toml @@ -20,7 +20,6 @@ allowed_scopes = [ # `find src -type f | awk '!/lib.rs|mod.rs/ { print "\"" substr($1, 1 + 4, length($1) - 4 - 3) "\","; }' | sort`. "autorouter/anterouter", "autorouter/autorouter", - "autorouter/compare_detours", "autorouter/compass_direction", "autorouter/connected_components", "autorouter/execution", diff --git a/crates/topola-egui/src/actions.rs b/crates/topola-egui/src/actions.rs index 2852d3b..8678b0c 100644 --- a/crates/topola-egui/src/actions.rs +++ b/crates/topola-egui/src/actions.rs @@ -390,19 +390,12 @@ impl RouteActions { } pub struct InspectActions { - pub compare_detours: Trigger, pub measure_length: Trigger, } impl InspectActions { pub fn new(tr: &Translator) -> Self { Self { - compare_detours: Action::new( - tr.text("tr-menu-inspect-compare-detours"), - egui::Modifiers::NONE, - egui::Key::Minus, - ) - .into_trigger(), measure_length: Action::new( tr.text("tr-menu-inspect-measure-length"), egui::Modifiers::NONE, @@ -414,7 +407,6 @@ impl InspectActions { pub fn render_menu(&mut self, ctx: &Context, ui: &mut Ui, workspace_activities_enabled: bool) { ui.add_enabled_ui(workspace_activities_enabled, |ui| { - self.compare_detours.button(ctx, ui); self.measure_length.button(ctx, ui); }); } diff --git a/crates/topola-egui/src/menu_bar.rs b/crates/topola-egui/src/menu_bar.rs index be47271..226bd0e 100644 --- a/crates/topola-egui/src/menu_bar.rs +++ b/crates/topola-egui/src/menu_bar.rs @@ -363,17 +363,6 @@ impl MenuBar { self.multilayer_autoroute_options.planar, ) }); - } else if actions - .inspect - .compare_detours - .consume_key_triggered(ctx, ui) - { - schedule(error_dialog, workspace, |selection| { - Command::CompareDetours( - selection.pin_selection, - self.multilayer_autoroute_options.planar, - ) - }); } else if actions .inspect .measure_length diff --git a/locales/en-US/main.ftl b/locales/en-US/main.ftl index 7bf63a8..ab4d0a3 100644 --- a/locales/en-US/main.ftl +++ b/locales/en-US/main.ftl @@ -68,7 +68,10 @@ tr-menu-route-options-wrap-around-bands = Wrap around Bands ## tr-menu-inspect = Inspect + +# Unused. tr-menu-inspect-compare-detours = Compare Detours + tr-menu-inspect-measure-length = Measure Length tr-menu-preferences = Preferences diff --git a/src/autorouter/autorouter.rs b/src/autorouter/autorouter.rs index 2765e98..8e37d55 100644 --- a/src/autorouter/autorouter.rs +++ b/src/autorouter/autorouter.rs @@ -26,7 +26,6 @@ use crate::{ }; use super::{ - compare_detours::CompareDetoursExecutionStepper, measure_length::MeasureLengthExecutionStepper, place_via::PlaceViaExecutionStepper, planar_autoroute::PlanarAutorouteExecutionStepper, @@ -138,29 +137,6 @@ impl Autorouter { ) } - pub(super) fn planar_autoroute_ratlines( - &mut self, - ratlines: Vec, - options: PlanarAutorouteOptions, - ) -> Result { - PlanarAutorouteExecutionStepper::new(self, ratlines, options) - } - - pub(super) fn undo_planar_autoroute_ratlines( - &mut self, - ratlines: Vec, - ) -> Result<(), AutorouterError> { - for ratline in ratlines.iter() { - let band = ratline.ref_(self).band_termseg(); - self.board - .layout_mut() - .remove_band(&mut LayoutEdit::new(), band) - .map_err(|_| AutorouterError::CouldNotRemoveBand(band))?; - } - - Ok(()) - } - pub fn topo_autoroute( &mut self, selection: &PinSelection, @@ -272,27 +248,6 @@ impl Autorouter { todo!(); } - pub fn compare_detours( - &mut self, - selection: &PinSelection, - options: PlanarAutorouteOptions, - ) -> Result { - let ratlines = self.selected_ratlines(selection, options.principal_layer); - if ratlines.len() < 2 { - return Err(AutorouterError::NeedExactlyTwoRatlines); - } - self.compare_detours_ratlines(ratlines[0], ratlines[1], options) - } - - pub(super) fn compare_detours_ratlines( - &mut self, - ratline1: RatlineUid, - ratline2: RatlineUid, - options: PlanarAutorouteOptions, - ) -> Result { - CompareDetoursExecutionStepper::new(self, ratline1, ratline2, options) - } - pub fn measure_length( &self, selection: &BandSelection, diff --git a/src/autorouter/compare_detours.rs b/src/autorouter/compare_detours.rs deleted file mode 100644 index 8ade3e2..0000000 --- a/src/autorouter/compare_detours.rs +++ /dev/null @@ -1,126 +0,0 @@ -// SPDX-FileCopyrightText: 2024 Topola contributors -// -// SPDX-License-Identifier: MIT - -//! Manages the comparison of detours between two ratlines, tracking their -//! routing statuses and recording their lengths. - -use std::ops::ControlFlow; - -use crate::{ - board::AccessMesadata, - drawing::graph::PrimitiveIndex, - geometry::{primitive::PrimitiveShape, shape::MeasureLength}, - graph::MakeRef, - router::{navcord::Navcord, navmesh::Navmesh, thetastar::ThetastarStepper}, - stepper::{EstimateProgress, Step}, -}; - -use super::{ - invoker::GetDebugOverlayData, - planar_autoroute::{PlanarAutorouteContinueStatus, PlanarAutorouteExecutionStepper}, - ratline::RatlineUid, - Autorouter, AutorouterError, PlanarAutorouteOptions, -}; - -pub struct CompareDetoursExecutionStepper { - autoroute: PlanarAutorouteExecutionStepper, - next_autoroute: Option, - ratline1: RatlineUid, - ratline2: RatlineUid, - total_length1: f64, - total_length2: f64, - done: bool, -} - -impl CompareDetoursExecutionStepper { - pub fn new( - autorouter: &mut Autorouter, - ratline1: RatlineUid, - ratline2: RatlineUid, - options: PlanarAutorouteOptions, - ) -> Result { - Ok(Self { - autoroute: autorouter.planar_autoroute_ratlines(vec![ratline1, ratline2], options)?, - next_autoroute: Some( - autorouter.planar_autoroute_ratlines(vec![ratline2, ratline1], options)?, - ), - ratline1, - ratline2, - total_length1: 0.0, - total_length2: 0.0, - done: false, - }) - } -} - -// XXX: Do we really need this to be a stepper? We don't use at the moment, as sorting functions -// aren't steppable either. It may be useful for debugging later on tho. -impl Step, (f64, f64)> for CompareDetoursExecutionStepper { - type Error = AutorouterError; - - fn step( - &mut self, - autorouter: &mut Autorouter, - ) -> Result, AutorouterError> { - if self.done { - return Ok(ControlFlow::Break((self.total_length1, self.total_length2))); - } - - match self.autoroute.step(autorouter)? { - ControlFlow::Continue( - PlanarAutorouteContinueStatus::Running | PlanarAutorouteContinueStatus::Skipped(_), - ) => Ok(ControlFlow::Continue(())), - ControlFlow::Continue(PlanarAutorouteContinueStatus::Routed(band_termseg)) => { - let length = band_termseg - .ref_(autorouter.board.layout().drawing()) - .length(); - - if self.next_autoroute.is_some() { - self.total_length1 += length; - } else { - self.total_length2 += length; - } - - Ok(ControlFlow::Continue(())) - } - ControlFlow::Break(..) => { - if let Some(next_autoroute) = self.next_autoroute.take() { - autorouter - .undo_planar_autoroute_ratlines(vec![self.ratline1, self.ratline2])?; - self.autoroute = next_autoroute; - - Ok(ControlFlow::Continue(())) - } else { - self.done = true; - autorouter - .undo_planar_autoroute_ratlines(vec![self.ratline2, self.ratline1])?; - - Ok(ControlFlow::Break((self.total_length1, self.total_length2))) - } - } - } - } -} - -impl EstimateProgress for CompareDetoursExecutionStepper { - type Value = f64; -} - -impl GetDebugOverlayData for CompareDetoursExecutionStepper { - fn maybe_thetastar(&self) -> Option<&ThetastarStepper> { - self.autoroute.maybe_thetastar() - } - - fn maybe_navcord(&self) -> Option<&Navcord> { - self.autoroute.maybe_navcord() - } - - fn ghosts(&self) -> &[PrimitiveShape] { - self.autoroute.ghosts() - } - - fn obstacles(&self) -> &[PrimitiveIndex] { - self.autoroute.obstacles() - } -} diff --git a/src/autorouter/execution.rs b/src/autorouter/execution.rs index b866cc6..2e1234c 100644 --- a/src/autorouter/execution.rs +++ b/src/autorouter/execution.rs @@ -20,7 +20,6 @@ use crate::{ }; use super::{ - compare_detours::CompareDetoursExecutionStepper, invoker::{GetDebugOverlayData, Invoker, InvokerError}, measure_length::MeasureLengthExecutionStepper, place_via::PlaceViaExecutionStepper, @@ -44,7 +43,6 @@ pub enum Command { }, PlaceVia(ViaWeight), RemoveBands(BandSelection), - CompareDetours(Type, PlanarAutorouteOptions), MeasureLength(BandSelection), } @@ -55,7 +53,6 @@ pub enum ExecutionStepper { TopoAutoroute(ng::AutorouteExecutionStepper), PlaceVia(PlaceViaExecutionStepper), RemoveBands(RemoveBandsExecutionStepper), - CompareDetours(CompareDetoursExecutionStepper), MeasureLength(MeasureLengthExecutionStepper), } @@ -124,18 +121,6 @@ impl ExecutionStepper { let edit = remove_bands.doit(autorouter)?; ControlFlow::Break((edit, "finished removing bands".to_string())) } - ExecutionStepper::CompareDetours(compare_detours) => { - match compare_detours.step(autorouter)? { - ControlFlow::Continue(()) => ControlFlow::Continue(()), - ControlFlow::Break((total_length1, total_length2)) => ControlFlow::Break(( - None, - format!( - "total detour lengths are {} and {}", - total_length1, total_length2 - ), - )), - } - } ExecutionStepper::MeasureLength(measure_length) => { let length = measure_length.doit(autorouter)?; ControlFlow::Break((None, format!("Total length of selected bands: {}", length))) @@ -181,7 +166,6 @@ impl Abort> for ExecutionStepper { } ExecutionStepper::PlaceVia(_place_via) => (), //place_via.abort(), ExecutionStepper::RemoveBands(_remove_bands) => (), //remove_bands.abort(), - ExecutionStepper::CompareDetours(_compare_detours) => (), //compare_detours.abort(), ExecutionStepper::MeasureLength(_measure_length) => (), //measure_length.abort(), } } @@ -199,9 +183,6 @@ impl EstimateProgress for ExecutionStepper { ExecutionStepper::TopoAutoroute(toporoute) => toporoute.estimate_progress_value(), ExecutionStepper::PlaceVia(place_via) => place_via.estimate_progress_value(), ExecutionStepper::RemoveBands(remove_bands) => remove_bands.estimate_progress_value(), - ExecutionStepper::CompareDetours(compare_detours) => { - compare_detours.estimate_progress_value() - } ExecutionStepper::MeasureLength(measure_length) => { measure_length.estimate_progress_value() } @@ -217,9 +198,6 @@ impl EstimateProgress for ExecutionStepper { ExecutionStepper::TopoAutoroute(toporoute) => toporoute.estimate_progress_maximum(), ExecutionStepper::PlaceVia(place_via) => place_via.estimate_progress_maximum(), ExecutionStepper::RemoveBands(remove_bands) => remove_bands.estimate_progress_maximum(), - ExecutionStepper::CompareDetours(compare_detours) => { - compare_detours.estimate_progress_maximum() - } ExecutionStepper::MeasureLength(measure_length) => { measure_length.estimate_progress_maximum() } diff --git a/src/autorouter/invoker.rs b/src/autorouter/invoker.rs index 15c65c7..4fb6e1f 100644 --- a/src/autorouter/invoker.rs +++ b/src/autorouter/invoker.rs @@ -28,7 +28,6 @@ use crate::{ }; use super::{ - compare_detours::CompareDetoursExecutionStepper, execution::{Command, ExecutionStepper}, history::{History, HistoryError}, measure_length::MeasureLengthExecutionStepper, @@ -206,9 +205,6 @@ impl Invoker { Command::RemoveBands(selection) => { ExecutionStepper::RemoveBands(self.autorouter.remove_bands(selection)?) } - Command::CompareDetours(selection, options) => ExecutionStepper::CompareDetours( - self.autorouter.compare_detours(selection, *options)?, - ), Command::MeasureLength(selection) => { ExecutionStepper::MeasureLength(self.autorouter.measure_length(selection)?) } diff --git a/src/autorouter/mod.rs b/src/autorouter/mod.rs index a7a8fca..18d2d20 100644 --- a/src/autorouter/mod.rs +++ b/src/autorouter/mod.rs @@ -4,7 +4,6 @@ pub mod anterouter; mod autorouter; -pub mod compare_detours; pub mod compass_direction; pub mod connected_components; pub mod execution;