diff --git a/crates/engine/src/cmd/pathops.rs b/crates/engine/src/cmd/pathops.rs index a67fbad44..cbed03515 100644 --- a/crates/engine/src/cmd/pathops.rs +++ b/crates/engine/src/cmd/pathops.rs @@ -300,7 +300,6 @@ 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 { @@ -465,29 +464,14 @@ 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 name = match op { + 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. PathfinderOp::MinusFront => names.first().cloned().flatten(), PathfinderOp::MinusBack => names.last().cloned().flatten(), _ => names.into_iter().flatten().next(), }; - 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 - } + 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 f774801f6..e4cc2218f 100644 --- a/crates/engine/src/tests_pathops.rs +++ b/crates/engine/src/tests_pathops.rs @@ -205,31 +205,31 @@ 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. +/// #2178: Unite on separated shapes is one compound path; Release releases each subpath (Illustrator parity). #[test] -fn pathfinder_unite_non_overlapping_grouped_shapes_stay_ungroupable() { +fn pathfinder_unite_non_overlapping_shapes_become_compound_path_releasable() { 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); + let layer = s.doc().unwrap().doc.parent_of(a).unwrap(); 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 united = node(&s, result); + assert!(matches!(united.kind, NodeKind::Compound { .. })); + assert_eq!(united.children().unwrap().len(), 2); + s.execute("object.compoundPath.release", &json!({})).unwrap(); let d = &s.doc().unwrap().doc; - assert!(d.node(g).is_none()); + assert!(d.node(result).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. +/// #2178: Unite keeps disjoint geometry in one compound path (not a group of separate path objects). #[test] -fn pathfinder_unite_groups_disjoint_islands_not_subpaths_of_one_island() { +fn pathfinder_unite_disjoint_islands_stay_one_compound_path() { 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); @@ -237,15 +237,10 @@ fn pathfinder_unite_groups_disjoint_islands_not_subpaths_of_one_island() { 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]); + assert!(matches!(node(&s, result).kind, NodeKind::Compound { .. })); + assert_eq!(path_of(&s, result).0.subpaths.len(), 3); } #[test]