diff --git a/crates/engine/src/cmd/pathops.rs b/crates/engine/src/cmd/pathops.rs index cbed03515..a67fbad44 100644 --- a/crates/engine/src/cmd/pathops.rs +++ b/crates/engine/src/cmd/pathops.rs @@ -300,6 +300,7 @@ fn shape_node_styled(d: &mut Document, path: PathData, style: &Style) -> Node { n } + /// Whether `ids` are separate objects, as [`Document::remove_many`] and [`Document::insert_beside`] /// take them: each one is in the tree and listed once, and none lies inside another listed one. pub(crate) fn separate(d: &Document, ids: &[NodeId]) -> bool { @@ -464,14 +465,29 @@ fn run_pf(s: &mut Session, op: PathfinderOp) -> Result { let r = results.into_iter().find(|r| !r.path.is_empty()).ok_or_else(|| EngineError::Other(format!("{label} produced an empty result")))?; let style = styles.get(r.key as usize).ok_or_else(|| EngineError::Other(format!("{label}: lost track of a shape's style")))?; - let mut n = shape_node_styled(d, r.path, style); - n.name = match op { - // A subtraction keeps its surviving source's identity, even when unnamed. + let name = match op { PathfinderOp::MinusFront => names.first().cloned().flatten(), PathfinderOp::MinusBack => names.last().cloned().flatten(), _ => names.into_iter().flatten().next(), }; - n + let path = r.path; + if op == PathfinderOp::Unite { + let pieces = super::draw2::components(&path); + if pieces.len() > 1 { + let children = pieces.into_iter().map(|p| Arc::new(shape_node_styled(d, p, style))).collect(); + let mut g = Node::group(d.alloc_id(), children); + g.name = name; + g + } else { + let mut n = shape_node_styled(d, path, style); + n.name = name; + n + } + } else { + let mut n = shape_node_styled(d, path, style); + n.name = name; + n + } } else { let mut children = vec![]; for r in results { diff --git a/crates/engine/src/tests_pathops.rs b/crates/engine/src/tests_pathops.rs index 384fa6593..f774801f6 100644 --- a/crates/engine/src/tests_pathops.rs +++ b/crates/engine/src/tests_pathops.rs @@ -205,6 +205,49 @@ fn a_group_selected_alone_is_combined_member_by_member() { assert_eq!((bounds.x0, bounds.x1), (0.0, 10.0), "the back member less the front one"); } +/// #2178: Unite on separated shapes should group them so Object › Ungroup releases each piece. +#[test] +fn pathfinder_unite_non_overlapping_grouped_shapes_stay_ungroupable() { + let mut s = session(); + let a = rect(&mut s, 0.0, 0.0, 20.0, 20.0); + let b = rect(&mut s, 30.0, 0.0, 20.0, 20.0); + s.execute("select.set", &json!({"ids": [a.0, b.0]})).unwrap(); + let g = NodeId(s.execute("object.group", &json!({})).unwrap()["id"].as_u64().unwrap()); + let layer = s.doc().unwrap().doc.parent_of(g).unwrap(); + s.execute("object.pathfinder.unite", &json!({})).unwrap(); + let result = s.doc().unwrap().selection.objects[0]; + assert!(matches!(node(&s, result).kind, NodeKind::Group { .. })); + assert_eq!(node(&s, result).children().unwrap().len(), 2); + s.execute("object.ungroup", &json!({})).unwrap(); + let d = &s.doc().unwrap().doc; + assert!(d.node(g).is_none()); + assert_eq!(s.doc().unwrap().selection.len(), 2); + for id in &s.doc().unwrap().selection.objects { + assert_eq!(d.parent_of(*id), Some(layer)); + } +} + +/// #2178: each connected island stays one object; holes stay with their outer contour, not separate group children. +#[test] +fn pathfinder_unite_groups_disjoint_islands_not_subpaths_of_one_island() { + let mut s = session(); + let outer = rect(&mut s, 0.0, 0.0, 40.0, 40.0); + let hole = rect(&mut s, 10.0, 10.0, 10.0, 10.0); + s.execute("select.set", &json!({"ids": [outer.0, hole.0]})).unwrap(); + let ring = NodeId(s.execute("object.compoundPath.make", &json!({})).unwrap()["id"].as_u64().unwrap()); + let separate = rect(&mut s, 60.0, 0.0, 20.0, 20.0); + s.execute("select.set", &json!({"ids": [ring.0, separate.0]})).unwrap(); + s.execute("object.group", &json!({})).unwrap(); + s.execute("object.pathfinder.unite", &json!({})).unwrap(); + let result = s.doc().unwrap().selection.objects[0]; + let g = node(&s, result); + assert!(matches!(g.kind, NodeKind::Group { .. })); + let kids = g.children().unwrap(); + assert_eq!(kids.len(), 2); + let subpath_counts: Vec = kids.iter().map(|c| path_of(&s, c.id).0.subpaths.len()).collect(); + assert_eq!(subpath_counts, [2, 1]); +} + #[test] fn unite_with_group() { let mut s = session();