diff --git a/crates/doc/src/vector.rs b/crates/doc/src/vector.rs index 1538de2a6..fd6d48072 100644 --- a/crates/doc/src/vector.rs +++ b/crates/doc/src/vector.rs @@ -184,6 +184,13 @@ fn redirect_last_segment(net: &mut VectorNetwork, loop_segs: &[usize], from: usi } } +/// Which segments are in a loop, and which segments join each pair of points: the lookups +/// [`VectorNetwork::closing_segment`] reads, built once per pass by [`VectorNetwork::loop_index`]. +pub struct LoopIndex { + in_loop: Vec, + by_ends: std::collections::BTreeMap<(usize, usize), Vec>, +} + impl VectorNetwork { /// A network from a polyline / polygon. pub fn from_points(points: &[Point], closed: bool) -> Self { @@ -471,6 +478,7 @@ impl VectorNetwork { // The curves go between the two segments wherever a loop walks through the point, in the // order the loop meets them. Each loop is rebuilt in one pass, so a polygon with many // rounded corners isn't quadratic. + let index = self.loop_index(); for lp in out.regions.iter_mut().flat_map(|r| r.loops.iter_mut()) { if lp.is_empty() { continue; @@ -478,7 +486,7 @@ impl VectorNetwork { let start_v = self.loop_start(lp).unwrap_or(0); let mut rebuilt = Vec::with_capacity(lp.len() + by_vertex.len().min(lp.len()) + 1); let mut cur = start_v; - let closing = self.closing_segment(lp); + let closing = self.closing_segment(lp, &index); for &x in lp.iter().chain(closing.iter()) { rebuilt.push(x); let Some(s) = self.segments.get(x) else { continue }; @@ -506,13 +514,29 @@ impl VectorNetwork { } } + /// The lookups [`Self::closing_segment`] needs, built once for a pass over every loop. + pub fn loop_index(&self) -> LoopIndex { + let mut in_loop = vec![false; self.segments.len()]; + for &x in self.regions.iter().flat_map(|r| r.loops.iter().flatten()) { + if let Some(f) = in_loop.get_mut(x) { + *f = true; + } + } + let mut by_ends: std::collections::BTreeMap<(usize, usize), Vec> = std::collections::BTreeMap::new(); + for (i, s) in self.segments.iter().enumerate() { + by_ends.entry((s.a.min(s.b), s.a.max(s.b))).or_default().push(i); + } + LoopIndex { in_loop, by_ends } + } + /// The segment that joins the two ends of an open loop `lp` when it is in no loop of its own /// (and the only such segment): a corner cut off by a segment drawn before the corner point was /// deleted leaves the loop open there, and the stroke follows that segment. Only a loop that /// walks in order as one continuous chain has one. Rounding walks it as the loop's last /// segment, so the points at the loop's ends are cut by arcs as the stroke's are, and deleting - /// points or segments puts it into the loop (#1951). - pub fn closing_segment(&self, lp: &[usize]) -> Option { + /// points or segments puts it into the loop (#1951). `index` is this network's [`Self::loop_index`], + /// shared by every loop of one pass. + pub fn closing_segment(&self, lp: &[usize], index: &LoopIndex) -> Option { let start_v = self.loop_start(lp)?; let mut cur = start_v; for &x in lp { @@ -528,9 +552,8 @@ impl VectorNetwork { if cur == start_v { return None; } - let in_loop = |i: usize| self.regions.iter().flat_map(|r| r.loops.iter().flatten()).any(|&x| x == i); - let ends = |s: &Segment| (s.a == cur && s.b == start_v) || (s.b == cur && s.a == start_v); - let mut joining = self.segments.iter().enumerate().filter(|(i, s)| ends(s) && !in_loop(*i)).map(|(i, _)| i); + let mut joining = + index.by_ends.get(&(cur.min(start_v), cur.max(start_v)))?.iter().copied().filter(|&i| !index.in_loop.get(i).copied().unwrap_or(false)); let first = joining.next()?; joining.next().is_none().then_some(first) } diff --git a/crates/engine/src/canvas/vectoredit/ops.rs b/crates/engine/src/canvas/vectoredit/ops.rs index 733885de1..f4609dc1e 100644 --- a/crates/engine/src/canvas/vectoredit/ops.rs +++ b/crates/engine/src/canvas/vectoredit/ops.rs @@ -470,17 +470,28 @@ fn reopen_loops(net: &mut VectorNetwork) { } // An open chain whose two ends a segment in no loop already joins (a bridge drawn between two // points before the point between them was deleted) closes through that segment, at the end so - // the loop's winding is kept, as `connect` closes it (#1951). One loop at a time, so two loops - // can't both take one segment. - let places: Vec<(usize, usize)> = net.regions.iter().enumerate().flat_map(|(ri, r)| (0..r.loops.len()).map(move |li| (ri, li))).collect(); - for (ri, li) in places { - let Some(lp) = net.regions.get(ri).and_then(|r| r.loops.get(li)) else { continue }; - if lp.len() < 2 { - continue; + // the loop's winding is kept, as `connect` closes it (#1951). Every loop that walks the same + // chain (two regions over one ring) takes it, as the render does; a different chain can't + // 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(); + for (ri, r) in net.regions.iter().enumerate() { + for (li, lp) in r.loops.iter().enumerate() { + if lp.len() < 2 { + continue; + } + 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 let Some(closing) = net.closing_segment(lp) - && let Some(lp) = net.regions.get_mut(ri).and_then(|r| r.loops.get_mut(li)) - { + } + 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); } } @@ -1209,4 +1220,91 @@ mod tests { assert!(ends.iter().any(|x| (x - (centre.x - 0.5)).abs() < 0.01), "{ends:?}"); assert!(ends.iter().any(|x| (x - (centre.x + 0.5)).abs() < 0.01), "{ends:?}"); } + + /// An open chain of four segments over five points whose two ends a fifth segment (in no + /// loop) joins, as a bridge drawn before the point between them was deleted. + fn bridged_open_chain() -> VectorNetwork { + let corners = [ + Point::new(0.0, 0.0), + Point::new(525.0, 525.0), + Point::new(0.0, 525.0), + Point::new(333.0, 0.0), + Point::new(525.0, 183.0), + Point::new(900.0, 900.0), + ]; + let mut n = VectorNetwork::from_points(&corners, false); + n.segments = [(0, 3), (1, 2), (2, 0), (4, 1), (3, 4)].iter().map(|&(a, b)| Segment::line(a, b)).collect(); + n.regions.clear(); + for v in &mut n.vertices { + v.radius = 125.0; + } + n + } + + #[test] + fn regions_over_one_open_chain_all_close_through_the_joining_segment() { + use figcraft_geom::BezPath; + use figcraft_geom::kurbo::ParamCurveArclen; + let mut n = bridged_open_chain(); + for _ in 0..2 { + n.regions.push(Region { loops: vec![vec![3, 1, 2, 0]], winding: WindingRule::NonZero, fills: None, ..Default::default() }); + } + delete_vertices(&mut n, &[5], false); + for (i, r) in n.regions.iter().enumerate() { + assert_eq!(r.loops[0], vec![3, 1, 2, 0, 4], "region {i} closes through the segment: {:?}", n.regions); + } + let length = |path: &BezPath| path.segments().map(|s| s.arclen(1e-9)).sum::(); + let stroke = length(&n.stroke_path()); + let rounded = n.rounded().unwrap(); + for i in 0..2 { + let fill = length(&rounded.region_path(i).unwrap()); + assert!((fill - stroke).abs() < 1e-6, "region {i}: fill {fill} and stroke {stroke} trace the same outline"); + } + } + + #[test] + fn a_segment_joining_two_different_chains_goes_to_the_first_only() { + let mut n = bridged_open_chain(); + n.regions.push(Region { loops: vec![vec![3, 1, 2, 0]], winding: WindingRule::NonZero, fills: None, ..Default::default() }); + n.regions.push(Region { loops: vec![vec![0, 2, 1, 3]], winding: WindingRule::NonZero, fills: None, ..Default::default() }); + reopen_loops(&mut n); + assert_eq!(n.regions[0].loops[0].len(), 5); + assert_eq!(n.regions[1].loops[0].len(), 5, "the same segment set is the same chain"); + let mut m = bridged_open_chain(); + m.segments.push(Segment::line(1, 2)); + m.regions.push(Region { loops: vec![vec![3, 1, 2, 0]], winding: WindingRule::NonZero, fills: None, ..Default::default() }); + m.regions.push(Region { loops: vec![vec![3, 5, 2, 0]], winding: WindingRule::NonZero, fills: None, ..Default::default() }); + reopen_loops(&mut m); + let closed = m.regions.iter().filter(|r| r.loops[0].len() == 5).count(); + assert!(closed <= 1, "different chains don't share the segment: {:?}", m.regions); + } + + #[test] + fn many_open_loops_with_no_closing_segment_round_as_before() { + let mut n = VectorNetwork::from_points(&[Point::new(0.0, 0.0), Point::new(10.0, 0.0)], false); + n.segments.clear(); + n.vertices.clear(); + for k in 0..40 { + let x = f64::from(k) * 100.0; + let base = n.vertices.len(); + for p in [(x, 0.0), (x + 50.0, 0.0), (x + 50.0, 50.0), (x, 50.0)] { + n.vertices.push(Vertex { radius: 8.0, ..Vertex::new(p.0, p.1) }); + } + let first = n.segments.len(); + // Three sides of a square, an open loop with nothing to close it. + for (a, b) in [(0, 1), (1, 2), (2, 3)] { + n.segments.push(Segment::line(base + a, base + b)); + } + n.regions.push(Region { + loops: vec![vec![first, first + 1, first + 2]], + winding: WindingRule::NonZero, + fills: None, + ..Default::default() + }); + } + let r = n.rounded().unwrap(); + assert_eq!(r.regions.len(), 40); + // Each loop gained only its two inner corners' arcs (two corners, one arc each). + assert!(r.regions.iter().all(|reg| reg.loops[0].len() == 5), "{:?}", r.regions[0]); + } }