From 1f8ace9c77e99d2c7b429e20b1bb5c93107b0e34 Mon Sep 17 00:00:00 2001 From: Mikolaj Wielgus Date: Wed, 16 Jul 2025 00:21:25 +0200 Subject: [PATCH] fix(geometry/recording_with_rtree): Do not inadvertedly invalidate bend bboxes This fixes a crash that was happening when undoing autoroutings. Bends are still not restored correctly, I will fix that soon. --- src/geometry/recording_with_rtree.rs | 59 +----------- src/geometry/with_rtree.rs | 133 +++++++++++++++++++++++---- 2 files changed, 116 insertions(+), 76 deletions(-) diff --git a/src/geometry/recording_with_rtree.rs b/src/geometry/recording_with_rtree.rs index e4718a5..f23562e 100644 --- a/src/geometry/recording_with_rtree.rs +++ b/src/geometry/recording_with_rtree.rs @@ -361,63 +361,6 @@ impl< for RecordingGeometryWithRtree { fn apply(&mut self, edit: &GeometryEdit) { - for (compound, (maybe_old_data, ..)) in &edit.compounds { - if maybe_old_data.is_some() { - self.geometry_with_rtree.remove_compound(*compound); - } - } - - for (bend, (maybe_old_data, ..)) in &edit.bends { - if maybe_old_data.is_some() { - self.geometry_with_rtree.remove_bend(*bend); - } - } - - for (seg, (maybe_old_data, ..)) in &edit.segs { - if maybe_old_data.is_some() { - self.geometry_with_rtree.remove_seg(*seg); - } - } - - for (dot, (maybe_old_data, ..)) in &edit.dots { - if maybe_old_data.is_some() { - self.geometry_with_rtree.remove_dot(*dot); - } - } - - for (dot, (.., maybe_new_data)) in &edit.dots { - if let Some(weight) = maybe_new_data { - self.geometry_with_rtree.add_dot_at_index(*dot, *weight); - } - } - - for (seg, (.., maybe_new_data)) in &edit.segs { - if let Some(((from, to), weight)) = maybe_new_data { - self.geometry_with_rtree - .add_seg_at_index(*seg, *from, *to, *weight); - } - } - - for (bend, (.., maybe_new_data)) in &edit.bends { - if let Some(((from, to, core), weight)) = maybe_new_data { - self.geometry_with_rtree - .add_bend_at_index(*bend, *from, *to, *core, *weight); - } - } - - for (compound, (.., maybe_new_data)) in &edit.compounds { - if let Some((members, weight)) = maybe_new_data { - self.geometry_with_rtree - .add_compound_at_index(*compound, weight.clone()); - - for (entry_label, member) in members { - self.geometry_with_rtree.add_to_compound( - GenericIndex::::new(member.petgraph_index()), - *entry_label, - *compound, - ); - } - } - } + self.geometry_with_rtree.apply(edit); } } diff --git a/src/geometry/with_rtree.rs b/src/geometry/with_rtree.rs index 0627d7f..821fdc6 100644 --- a/src/geometry/with_rtree.rs +++ b/src/geometry/with_rtree.rs @@ -18,6 +18,8 @@ use crate::{ graph::{GenericIndex, GetPetgraphIndex}, }; +use super::edit::{ApplyGeometryEdit, GeometryEdit}; + #[derive(Debug, Clone, Copy, PartialEq)] pub struct Bbox { pub aabb: AABB<[f64; 3]>, @@ -155,24 +157,6 @@ impl< bend } - pub(super) fn add_bend_at_index + GetLayer>( - &mut self, - bend: BI, - from: DI, - to: DI, - core: DI, - weight: W, - ) { - self.geometry.add_bend_at_index( - GenericIndex::::new(bend.petgraph_index()), - from, - to, - core, - weight, - ); - self.init_bend_bbox(bend); - } - pub(super) fn add_compound_at_index(&mut self, compound: GenericIndex, weight: CW) { self.geometry .add_compound_at_index(GenericIndex::::new(compound.petgraph_index()), weight); @@ -491,3 +475,116 @@ impl< self.geometry.compounds(node) } } + +impl< + PW: GetWidth + GetLayer + TryInto + TryInto + TryInto + Retag + Copy, + DW: AccessDotWeight + Into + GetLayer, + SW: AccessSegWeight + Into + GetLayer, + BW: AccessBendWeight + Into + GetLayer, + CW: Clone, + Cel: Copy, + PI: GetPetgraphIndex + TryInto + TryInto + TryInto + Eq + Ord + Copy, + DI: GetPetgraphIndex + Into + Eq + Ord + Copy, + SI: GetPetgraphIndex + Into + Eq + Ord + Copy, + BI: GetPetgraphIndex + Into + Eq + Ord + Copy, + > ApplyGeometryEdit + for GeometryWithRtree +{ + fn apply(&mut self, edit: &GeometryEdit) { + // First we remove every node that is in the edit. But we have to do + // this in a correct order, because otherwise some removals of some + // nodes may fail due to inadvertent invalidation of the bboxes of these + // nodes. + // + // We chose to first remove compounds, then bends and segs, and only + // then dots. + + for (compound, (maybe_old_data, ..)) in &edit.compounds { + if maybe_old_data.is_some() { + self.remove_compound(*compound); + } + } + + // Because removal of a bend will invalidate bboxes of the bends wrapped + // around it, we first remove the bends from the R-tree, and their nodes + // from the graph only afterwards. + + for (bend, (maybe_old_data, ..)) in &edit.bends { + if maybe_old_data.is_some() { + Self::rtree_remove_must_be_successful( + self.rtree.remove(&self.make_bend_bbox(*bend)), + ); + } + } + + for (bend, (maybe_old_data, ..)) in &edit.bends { + if maybe_old_data.is_some() { + self.geometry.remove_primitive((*bend).into()); + } + } + + for (seg, (maybe_old_data, ..)) in &edit.segs { + if maybe_old_data.is_some() { + self.remove_seg(*seg); + } + } + + for (dot, (maybe_old_data, ..)) in &edit.dots { + if maybe_old_data.is_some() { + self.remove_dot(*dot); + } + } + + // Now we add every node that is created or modified (but not removed) + // in the edit. This also has to be done in a right order, which we + // chose to be exactly the opposite of the order of the removal which we + // just did. + + for (dot, (.., maybe_new_data)) in &edit.dots { + if let Some(weight) = maybe_new_data { + self.add_dot_at_index(*dot, *weight); + } + } + + for (seg, (.., maybe_new_data)) in &edit.segs { + if let Some(((from, to), weight)) = maybe_new_data { + self.add_seg_at_index(*seg, *from, *to, *weight); + } + } + + // Just as with removal, we handle bend layout nodes and their bboxes + // separately to prevent failures from inadvertent bbox invalidation. + + for (bend, (.., maybe_new_data)) in &edit.bends { + if let Some(((from, to, core), weight)) = maybe_new_data { + self.geometry.add_bend_at_index( + GenericIndex::::new(bend.petgraph_index()), + *from, + *to, + *core, + *weight, + ); + } + } + + for (bend, (.., maybe_new_data)) in &edit.bends { + if let Some(..) = maybe_new_data { + self.init_bend_bbox(*bend); + } + } + + for (compound, (.., maybe_new_data)) in &edit.compounds { + if let Some((members, weight)) = maybe_new_data { + self.add_compound_at_index(*compound, weight.clone()); + + for (entry_label, member) in members { + self.geometry.add_to_compound( + GenericIndex::::new(member.petgraph_index()), + *entry_label, + *compound, + ); + } + } + } + } +}