diff --git a/crates/openpencil-shell-core/src/widgets/property_panel_sections.rs b/crates/openpencil-shell-core/src/widgets/property_panel_sections.rs index 043258875..4aac8af10 100644 --- a/crates/openpencil-shell-core/src/widgets/property_panel_sections.rs +++ b/crates/openpencil-shell-core/src/widgets/property_panel_sections.rs @@ -745,18 +745,10 @@ pub fn paint_export_section( y } -/// Y-extent of the export section, used by the hit-test layout -/// walker so a click anywhere inside emits OpenExportDialog. -pub fn export_section_rect(x: f32, y_top: f32, width: f32) -> Rect { - // section_label + 1 row of pills + 12px gap. Mirrors the paint - // layout above; if you change one, change both. - let label_h = 28.0; - let total_h = label_h + INPUT_HEIGHT + 12.0; - Rect { - origin: Point2D::new(x, y_top), - size: Point2D::new(width, total_h), - } -} +// NOTE: a future `export_section_rect` walker will live here once +// `hit_test_action` is extended to emit `OpenExportDialog` for +// clicks inside this section. Until then the section is preview- +// only — open the dialog via File menu / Cmd+Shift+P. // All shared paint primitives + layout constants are imported // from `property_panel_inputs` via the `pub use` block earlier in diff --git a/crates/openpencil-shell-native/src/boolean_ops.rs b/crates/openpencil-shell-native/src/boolean_ops.rs index 16123b69a..01e24d218 100644 --- a/crates/openpencil-shell-native/src/boolean_ops.rs +++ b/crates/openpencil-shell-native/src/boolean_ops.rs @@ -85,7 +85,12 @@ pub fn apply_boolean_op(doc: &mut Document, op: BooleanOp, next_id: &mut u64) -> new_node.bounds = openpencil_shell_core::Rect { origin, size }; let id_set: std::collections::HashSet = path_ids.iter().copied().collect(); if let Some(page) = doc.pages.get_mut(active) { - page.children.retain(|n| !id_set.contains(&n.id)); + // Recursive removal — sources may be nested inside groups / + // frames (codex CONCERN: `retain` only on top-level left + // originals behind + duplicated the result). Walk every + // children Vec depth-first and drop any node whose id is in + // the source set. + remove_nodes_recursively(&mut page.children, &id_set); page.children.push(new_node); } doc.selected_set.clear(); @@ -131,6 +136,16 @@ fn extract_points(path: &SkPath) -> Vec { out } +fn remove_nodes_recursively( + children: &mut Vec, + targets: &std::collections::HashSet, +) { + children.retain(|n| !targets.contains(&n.id)); + for child in children.iter_mut() { + remove_nodes_recursively(&mut child.children, targets); + } +} + fn bbox_of(points: &[Point2D]) -> (Point2D, Point2D) { let mut min_x = f32::INFINITY; let mut min_y = f32::INFINITY; @@ -226,6 +241,50 @@ mod tests { assert_eq!(doc.history.past.len(), 0); } + #[test] + fn boolean_op_removes_nested_paths_not_just_top_level() { + // Codex BLOCK: when source paths live inside a group/frame, + // the previous top-level-only `retain` left them in the + // group + appended the result at top level — duplication. + // Fix removes from anywhere in the children tree. + let mut doc = Document::empty(); + let page = doc.pages.get_mut(0).unwrap(); + page.children.clear(); + let mut a = Node::leaf(10, NodeKind::Path, "a"); + a.points = vec![ + Point2D::new(0.0, 0.0), + Point2D::new(20.0, 0.0), + Point2D::new(20.0, 20.0), + Point2D::new(0.0, 20.0), + ]; + a.bounds = Rect::xywh(0.0, 0.0, 20.0, 20.0); + let mut b = Node::leaf(11, NodeKind::Path, "b"); + b.points = vec![ + Point2D::new(10.0, 10.0), + Point2D::new(30.0, 10.0), + Point2D::new(30.0, 30.0), + Point2D::new(10.0, 30.0), + ]; + b.bounds = Rect::xywh(10.0, 10.0, 20.0, 20.0); + // Wrap both paths inside a Group. + let group = Node::with_children(99, NodeKind::Group, "g", vec![a, b]); + page.children.push(group); + doc.selected_set = vec![NodeId::new(10), NodeId::new(11)]; + doc.selected = NodeId::new(11); + let mut next = 100u64; + assert!(apply_boolean_op(&mut doc, BooleanOp::Union, &mut next)); + // Group still exists but is now empty of paths; result Path + // lives at top level — total page.children = 2 (empty group + result). + let page = doc.active_page().unwrap(); + assert_eq!(page.children.len(), 2); + let group_after = &page.children[0]; + assert!(matches!(group_after.kind, NodeKind::Group)); + assert!( + group_after.children.is_empty(), + "source paths must be removed from their group, not duplicated" + ); + } + #[test] fn boolean_op_skips_non_path_nodes_in_selection() { let mut doc = doc_with_two_squares();