diff --git a/crates/engine/src/cmd/buildcmds.rs b/crates/engine/src/cmd/buildcmds.rs index a4f620e9b..3f7da1629 100644 --- a/crates/engine/src/cmd/buildcmds.rs +++ b/crates/engine/src/cmd/buildcmds.rs @@ -354,7 +354,7 @@ fn sb_merge(s: &mut Session, p: &Value) -> Result { for (i, (sh, leaf)) in map.shapes.iter().zip(&leaves).enumerate() { let pieces: Vec = map.lines.iter().enumerate().filter(|(_, (l, _))| l.key == i as u64).map(|(k, _)| k).collect(); if !pieces.is_empty() { - // An open path loses the pieces deleted and those between merged (or deleted) regions. + // An open path loses the pieces deleted and those between two merged (or deleted) regions. let keep: Vec = pieces.iter().copied().filter(|k| !touched.lines.contains(k) && !map.line_between(*k, &touched.regions)).collect(); if keep.len() == pieces.len() { diff --git a/crates/engine/src/tests_build.rs b/crates/engine/src/tests_build.rs index da2215a90..801fb9e6f 100644 --- a/crates/engine/src/tests_build.rs +++ b/crates/engine/src/tests_build.rs @@ -505,6 +505,27 @@ fn shape_builder_click_on_a_side_of_a_line_splits_the_shape() { assert_eq!(open_paths(&s, &out), vec![(2, Rect::new(50.0, -20.0, 50.0, 120.0))]); } +/// A line that ends inside a shape has the same region on both sides there: a click or a drag +/// across it, or deleting the region, leaves it whole. +#[test] +fn shape_builder_merge_keeps_a_line_that_ends_inside_the_shape() { + let mut s = session(); + let a = rect(&mut s, 0.0, 0.0, 100.0, 100.0); + set_fill(&mut s, a, Color::rgb(1.0, 0.0, 0.0)); + let l = line(&mut s, (50.0, -20.0), (50.0, 99.0)); + s.execute("select.set", &json!({"ids": [a.0, l.0]})).unwrap(); + let mut lines = vec![]; + for params in [json!({"points": [[25, 50]]}), json!({"points": [[25, 50], [75, 50]]}), json!({"points": [[25, 50]], "erase": true})] { + let r = s.execute("shapeBuilder.merge", ¶ms).unwrap(); + assert_eq!(r["regions"], 1, "{params}"); + let merged = r["merged"].as_u64().map(|m| (area(&s, NodeId(m)) - 10000.0).abs() < 1.0); + assert_eq!(merged, params["erase"].is_null().then_some(true), "{params}"); + lines.push(open_paths(&s, &ids(&r))); + s.execute("edit.undo", &json!({})).unwrap(); + } + assert_eq!(lines, vec![vec![(2, Rect::new(50.0, -20.0, 50.0, 99.0))]; 3]); +} + #[test] fn shape_builder_merge_across_a_line_removes_the_piece_between() { let mut s = session(); diff --git a/crates/tools/src/builder.rs b/crates/tools/src/builder.rs index 2142769ec..1a13609ef 100644 --- a/crates/tools/src/builder.rs +++ b/crates/tools/src/builder.rs @@ -479,10 +479,11 @@ impl BuilderMap { [m + off, m - off].map(|q| self.region_at(q)) } - /// Does line piece `i` run between two of `regions` (an edge that merging or deleting them - /// removes)? + /// Does line piece `i` run between two different regions of `regions` (an edge that merging or + /// deleting them removes)? A piece with the same region on both sides, such as the end of a + /// line inside a shape, bounds nothing and stays. pub fn line_between(&self, i: usize, regions: &[usize]) -> bool { - self.lines.get(i).is_some_and(|(l, _)| self.sides(l).iter().all(|r| r.is_some_and(|r| regions.contains(&r)))) + self.lines.get(i).is_some_and(|(l, _)| matches!(self.sides(l), [Some(a), Some(b)] if a != b && regions.contains(&a) && regions.contains(&b))) } /// The edges `touched` deletes: those that bound none of its regions (deleting a region takes