From 153fa08d077a6a94a9c5eb44c3878176f15329df Mon Sep 17 00:00:00 2001 From: Trevor Brown Date: Wed, 11 Nov 2026 04:38:33 -0500 Subject: [PATCH] M4: reopen_loops looks up each closing segment's chain in a map (#1951) Checking a loop's closing segment against every closing already accepted was quadratic in the number of open loops. A map from closing segment to the chain that took it first answers it in one lookup; behaviour is unchanged. A test closes 50 independent chains through their own segments. Co-Authored-By: Claude Opus 5.5 Model-Effort: Opus 5.5 drove and wrote --- crates/engine/src/canvas/vectoredit/ops.rs | 33 +++++++++++++++++++--- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/canvas/vectoredit/ops.rs b/crates/engine/src/canvas/vectoredit/ops.rs index f4609dc1e..f8b9d7d66 100644 --- a/crates/engine/src/canvas/vectoredit/ops.rs +++ b/crates/engine/src/canvas/vectoredit/ops.rs @@ -475,7 +475,9 @@ fn reopen_loops(net: &mut VectorNetwork) { // share it. The lookups are built once, so a segment one loop took doesn't hide it from the // next. let index = net.loop_index(); - let mut closings: Vec<(usize, usize, usize, Vec)> = Vec::new(); + // The chain that first took each closing segment, so a later loop is checked in one lookup. + let mut owner: std::collections::BTreeMap> = std::collections::BTreeMap::new(); + let mut closings: Vec<(usize, usize, usize)> = Vec::new(); for (ri, r) in net.regions.iter().enumerate() { for (li, lp) in r.loops.iter().enumerate() { if lp.len() < 2 { @@ -484,13 +486,13 @@ fn reopen_loops(net: &mut VectorNetwork) { if let Some(closing) = net.closing_segment(lp, &index) { let mut chain = lp.clone(); chain.sort_unstable(); - if closings.iter().all(|(_, _, c, other)| *c != closing || *other == chain) { - closings.push((ri, li, closing, chain)); + if *owner.entry(closing).or_insert_with(|| chain.clone()) == chain { + closings.push((ri, li, closing)); } } } } - for (ri, li, closing, _) in closings { + for (ri, li, closing) in closings { if let Some(lp) = net.regions.get_mut(ri).and_then(|r| r.loops.get_mut(li)) { lp.push(closing); } @@ -1262,6 +1264,29 @@ mod tests { } } + /// Many independent open chains, each with its own closing segment, all close through it. + #[test] + fn many_open_chains_each_close_through_their_own_segment() { + let one = bridged_open_chain(); + let (nv, ns) = (one.vertices.len(), one.segments.len()); + let mut n = VectorNetwork::default(); + for (i, dx) in (0..50usize).zip((0..50u32).map(|k| f64::from(k) * 1000.0)) { + n.vertices.extend(one.vertices.iter().map(|v| Vertex { x: v.x + dx, ..*v })); + n.segments.extend(one.segments.iter().map(|s| Segment { a: s.a + i * nv, b: s.b + i * nv, ..*s })); + let base = i * ns; + n.regions.push(Region { + loops: vec![vec![base + 3, base + 1, base + 2, base]], + winding: WindingRule::NonZero, + fills: None, + ..Default::default() + }); + } + reopen_loops(&mut n); + for (i, r) in n.regions.iter().enumerate() { + assert_eq!(r.loops[0].last(), Some(&(i * ns + 4)), "chain {i} closes through its own segment"); + } + } + #[test] fn a_segment_joining_two_different_chains_goes_to_the_first_only() { let mut n = bridged_open_chain();