Merge PR #446: fix group copies and arc hover-picking

Two fixes from @UserDevtec (Jonathan):
- Copying a complete group now recreates the Group object and dictionary
  entry for the copies (unique NAME_COPYn), so the copy is treated as a
  group too; partial copies stay ungrouped. Undo recording is poisoned
  when groups are copied, since Group objects are non-entity state a
  pure-entity delta cannot restore.
- set_wire_aabb now widens the cull box over the wire's tessellated
  points, not just pick_tris, so bulge arcs that bow outside the stored
  vertex box are no longer rejected before segment hit-testing — arcs
  are hoverable/pickable again.

Builds clean; the AABB change is strictly box-growing so it can only
fix under-sized boxes.

Closes #446

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

src/app/history.rs

@@ -78,14 +78,23 @@ impl OpenCADStudio {
         }
     }
 
-    /// Copy is delta-safe only when no target is a Dimension: copying a
-    /// dimension clones a fresh anonymous `*D` block record (non-entity state a
-    /// pure-entity delta can't restore).
+    /// Copy is delta-safe only when no target is a Dimension and no complete
+    /// group is copied: dimensions clone fresh anonymous `*D` block records,
+    /// and complete group copies add Group objects / dictionary entries. Both
+    /// are non-entity state a pure-entity delta can't restore.
     pub(super) fn delta_copy_safe(&self, i: usize, handles: &[Handle]) -> bool {
+        use acadrust::objects::ObjectType;
         let doc = &self.tabs[i].scene.document;
-        !handles
+        let copies_dimension = handles
             .iter()
-            .any(|h| matches!(doc.get_entity(*h), Some(EntityType::Dimension(_))))
+            .any(|h| matches!(doc.get_entity(*h), Some(EntityType::Dimension(_))));
+        let copies_complete_group = doc.objects.values().any(|o| match o {
+            ObjectType::Group(g) => {
+                !g.entities.is_empty() && g.entities.iter().all(|h| handles.contains(h))
+            }
+            _ => false,
+        });
+        !copies_dimension && !copies_complete_group
     }
 
     /// Erase is delta-safe only when no target belongs to a group: erasing a
@@ -213,7 +222,9 @@ impl OpenCADStudio {
 
     pub(super) fn restore_history_snapshot(&mut self, i: usize, snapshot: FullSnapshot) {
         self.tabs[i].scene.document = snapshot.document;
-        self.tabs[i].scene.set_current_layout(snapshot.current_layout);
+        self.tabs[i]
+            .scene
+            .set_current_layout(snapshot.current_layout);
         // Force a re-tessellation: the cached wires were keyed against the
         // outgoing document / layout and would be returned unchanged
         // otherwise (`set_current_layout` only bumps on actual change).
@@ -278,7 +289,10 @@ impl OpenCADStudio {
                     // delta rides to the redo stack (it still holds the after
                     // side) — no current-state capture needed.
                     self.apply_delta(i, &d, true);
-                    self.tabs[i].history.redo_stack.push(HistorySnapshot::Delta(d));
+                    self.tabs[i]
+                        .history
+                        .redo_stack
+                        .push(HistorySnapshot::Delta(d));
                 }
             }
         }
@@ -309,7 +323,10 @@ impl OpenCADStudio {
                 }
                 HistorySnapshot::Delta(d) => {
                     self.apply_delta(i, &d, false);
-                    self.tabs[i].history.undo_stack.push(HistorySnapshot::Delta(d));
+                    self.tabs[i]
+                        .history
+                        .undo_stack
+                        .push(HistorySnapshot::Delta(d));
                 }
             }
         }

src/scene/convert/tess.rs

@@ -140,21 +140,28 @@ fn section_symbol_wires(
         [vx0 / vlen, vy0 / vlen]
     });
 
-    let (show_arrows, show_plane_line, show_end_lines, arrow_size, arrow_ext, label_h) =
-        match style {
-            Some(st) => (
-                st.show_arrows,
-                st.show_plane_line,
-                st.show_end_lines,
-                st.arrow_size,
-                st.arrow_extension,
-                st.label_height,
-            ),
-            None => {
-                let t = s.tick_a.abs().max(s.tick_b.abs());
-                (true, false, true, (t * 0.66).max(2.5), t.max(5.0), t.max(2.5))
-            }
-        };
+    let (show_arrows, show_plane_line, show_end_lines, arrow_size, arrow_ext, label_h) = match style
+    {
+        Some(st) => (
+            st.show_arrows,
+            st.show_plane_line,
+            st.show_end_lines,
+            st.arrow_size,
+            st.arrow_extension,
+            st.label_height,
+        ),
+        None => {
+            let t = s.tick_a.abs().max(s.tick_b.abs());
+            (
+                true,
+                false,
+                true,
+                (t * 0.66).max(2.5),
+                t.max(5.0),
+                t.max(2.5),
+            )
+        }
+    };
     // Arrowhead via the shared dimension/leader arrow table: the style's arrow
     // block handle (null → ClosedFilled), sized from the style.
     let arrow_kind = match style {
@@ -180,11 +187,14 @@ fn section_symbol_wires(
     }
 
     // Each end: (endpoint, tick length, outward sign along the cut).
-    for (ex, ey, tick, osign) in [(ax, ay, s.tick_a.abs(), 1.0), (bx, by, s.tick_b.abs(), -1.0)] {
+    for (ex, ey, tick, osign) in [
+        (ax, ay, s.tick_a.abs(), 1.0),
+        (bx, by, s.tick_b.abs(), -1.0),
+    ] {
         let (ox, oy) = (ux * osign, uy * osign); // outward along the cut
         let tip = [ex + ox * tick, ey + oy * tick, 0.0]; // outer tick tip
-        // End segment: the short drawn extension past the end (the "broken"
-        // section line when show_plane_line is off).
+                                                         // End segment: the short drawn extension past the end (the "broken"
+                                                         // section line when show_plane_line is off).
         if show_end_lines && tick > 1e-9 {
             lines.push(nan);
             lines.push([ex, ey, 0.0]);
@@ -563,7 +573,11 @@ pub(crate) fn tessellate_entity(
                 // Text labels: draw the glyph strokes (simplex.shx etc. are
                 // single-stroke fonts, so the outline is the character).
                 for t in &dec.texts {
-                    let font = t.font.trim().trim_end_matches(".shx").trim_end_matches(".SHX");
+                    let font = t
+                        .font
+                        .trim()
+                        .trim_end_matches(".shx")
+                        .trim_end_matches(".SHX");
                     let font = if font.is_empty() { "standard" } else { font };
                     let (strokes, _) = crate::scene::text::lff::tessellate_text_ex(
                         [0.0, 0.0],
@@ -674,10 +688,7 @@ pub(crate) fn tessellate_entity(
             line_weight_px: 1.0,
             snap_pts: vec![],
             tangent_geoms: vec![],
-            key_vertices: vec![
-                [x0, y0, 0.0],
-                [x1, y1, 0.0],
-            ],
+            key_vertices: vec![[x0, y0, 0.0], [x1, y1, 0.0]],
             aabb: [x0 as f32, y0 as f32, x1 as f32, y1 as f32],
             plinegen: true,
             fill_tris: vec![],
@@ -1547,34 +1558,43 @@ pub(crate) fn wire_points_aabb(w: &WireModel) -> [f32; 4] {
 }
 
 /// Assign `entity_box` as `w`'s cullable box, widened to cover the wire's
-/// pick-only geometry.
+/// actual tessellated geometry and pick-only geometry.
 ///
 /// `entity_aabb`'s box comes from acadrust's `bounding_box()`, which for a
-/// polyline is the box of its stored vertices — it knows nothing about the band
-/// a width paints around them, nor the wall a thickness extrudes. Hit-testing
-/// rejects on this box before it looks at `pick_tris`, so a box that stops short
-/// of them makes them silently unpickable: a donut's vertices are two points on
-/// one horizontal line, giving a zero-height box that rejects every click on the
-/// disc it draws.
-///
-/// A no-op for the entities that have no `pick_tris`, which is nearly all.
+/// polyline is often the box of its stored vertices — it may know nothing about
+/// bulge arcs between them, the band a width paints around them, or the wall a
+/// thickness extrudes. Hit-testing rejects on this box before it looks at the
+/// wire segments or `pick_tris`, so a box that stops short of the drawn geometry
+/// makes that geometry silently unpickable.
 pub(crate) fn set_wire_aabb(w: &mut WireModel, entity_box: [f32; 4]) {
-    if w.pick_tris.is_empty() || entity_box == WireModel::UNBOUNDED_AABB {
-        w.aabb = entity_box;
-        return;
-    }
-    let [mut x0, mut y0, mut x1, mut y1] = entity_box;
-    for (i, p) in w.pick_tris.iter().enumerate() {
-        let lo = w.pick_tris_low.get(i).copied().unwrap_or([0.0; 3]);
+    let mut out = if entity_box == WireModel::UNBOUNDED_AABB {
+        wire_points_aabb(w)
+    } else {
+        entity_box
+    };
+
+    let mut extend = |p: [f32; 3], lo: [f32; 3]| {
         let (x, y) = (p[0] + lo[0], p[1] + lo[1]);
         if x.is_finite() && y.is_finite() {
-            x0 = x0.min(x);
-            y0 = y0.min(y);
-            x1 = x1.max(x);
-            y1 = y1.max(y);
+            if out == WireModel::UNBOUNDED_AABB {
+                out = [x, y, x, y];
+            } else {
+                out[0] = out[0].min(x);
+                out[1] = out[1].min(y);
+                out[2] = out[2].max(x);
+                out[3] = out[3].max(y);
+            }
         }
+    };
+
+    for (i, &p) in w.points.iter().enumerate() {
+        extend(p, w.points_low.get(i).copied().unwrap_or([0.0; 3]));
     }
-    w.aabb = [x0, y0, x1, y1];
+    for (i, &p) in w.pick_tris.iter().enumerate() {
+        extend(p, w.pick_tris_low.get(i).copied().unwrap_or([0.0; 3]));
+    }
+
+    w.aabb = out;
 }
 
 pub(crate) fn entity_aabb(e: &acadrust::EntityType) -> [f32; 4] {

src/scene/group_layer.rs

@@ -36,6 +36,85 @@ impl Scene {
         gh
     }
 
+    /// Recreate every group whose full membership was copied.
+    ///
+    /// Partial group copies intentionally remain ungrouped: copying one member
+    /// out of a group should not create a new one-member fragment. When the
+    /// whole source group is in `handle_map`, the new handles get their own
+    /// Group object so later selection/editing treats the copy as a group too.
+    pub fn copy_complete_groups(
+        &mut self,
+        handle_map: &rustc_hash::FxHashMap<Handle, Handle>,
+    ) -> usize {
+        if handle_map.is_empty() {
+            return 0;
+        }
+
+        let group_dict_handle = self.document.header.acad_group_dict_handle;
+        let sources: Vec<_> = self
+            .document
+            .objects
+            .values()
+            .filter_map(|obj| match obj {
+                ObjectType::Group(g)
+                    if !g.entities.is_empty()
+                        && g.entities.iter().all(|h| handle_map.contains_key(h)) =>
+                {
+                    Some(g.clone())
+                }
+                _ => None,
+            })
+            .collect();
+
+        let mut copied = 0;
+        for source in sources {
+            let name = self.unique_group_copy_name(&source.name);
+            let mut group = source;
+            group.handle = self.document.allocate_handle();
+            group.owner = group_dict_handle;
+            group.name = name.clone();
+            group.entities = group
+                .entities
+                .iter()
+                .filter_map(|h| handle_map.get(h).copied())
+                .collect();
+            let gh = group.handle;
+            self.document.objects.insert(gh, ObjectType::Group(group));
+            if let Some(ObjectType::Dictionary(dict)) =
+                self.document.objects.get_mut(&group_dict_handle)
+            {
+                dict.add_entry(&name, gh);
+            }
+            copied += 1;
+        }
+        copied
+    }
+
+    fn unique_group_copy_name(&self, source: &str) -> String {
+        let group_dict_handle = self.document.header.acad_group_dict_handle;
+        let exists = |name: &str| {
+            self.document
+                .objects
+                .get(&group_dict_handle)
+                .and_then(|obj| match obj {
+                    ObjectType::Dictionary(dict) => Some(
+                        dict.entries
+                            .iter()
+                            .any(|(entry, _)| entry.eq_ignore_ascii_case(name)),
+                    ),
+                    _ => None,
+                })
+                .unwrap_or(false)
+        };
+        for n in 1.. {
+            let candidate = format!("{source}_COPY{n}");
+            if !exists(&candidate) {
+                return candidate;
+            }
+        }
+        unreachable!()
+    }
+
     /// Dissolves all groups that contain any of the given handles.
     /// Returns the number of groups removed.
     pub fn delete_groups_containing(&mut self, handles: &[Handle]) -> usize {
@@ -80,5 +159,4 @@ impl Scene {
         }
         self.bump_selection();
     }
-
 }

src/scene/model/solid_model.rs

@@ -104,9 +104,7 @@ pub fn edge_wires(solid: &Solid) -> Vec<acadrust::entities::Wire> {
     let mut wires = Vec::new();
     for shell in solid.boundaries() {
         for edge in shell.edge_iter() {
-            if let TruckTessResult::Lines(pts, pts_low) =
-                tessellate_edge(&edge)
-            {
+            if let TruckTessResult::Lines(pts, pts_low) = tessellate_edge(&edge) {
                 if pts.len() < 2 {
                     continue;
                 }

src/scene/modify.rs

@@ -82,8 +82,11 @@ impl Scene {
     pub fn transform_entities(&mut self, handles: &[Handle], t: &EntityTransform) {
         // Never transform objects on a locked layer (defense-in-depth: the pick
         // path already excludes them, but programmatic callers may not).
-        let handles: Vec<Handle> =
-            handles.iter().copied().filter(|&h| !self.is_layer_locked(h)).collect();
+        let handles: Vec<Handle> = handles
+            .iter()
+            .copied()
+            .filter(|&h| !self.is_layer_locked(h))
+            .collect();
         let handles = &handles[..];
         // MIRRTEXT (header.mirror_text): when false AutoCAD positions text /
         // mtext / shape by the mirror but keeps the original rotation +
@@ -146,14 +149,10 @@ impl Scene {
             if self.hatches.contains_key(&h) {
                 let existing_color = self.hatches[&h].color;
                 let new_model = match self.document.get_entity(h) {
-                    Some(EntityType::Hatch(dxf)) => {
-                        Self::hatch_model_from_dxf(dxf, existing_color)
-                    }
+                    Some(EntityType::Hatch(dxf)) => Self::hatch_model_from_dxf(dxf, existing_color),
                     // A DXF SOLID renders as a solid-fill hatch; rebuild it from
                     // the moved corners so the fill follows the transform.
-                    Some(EntityType::Solid(s)) => {
-                        Some(Self::solid_hatch_model(s, existing_color))
-                    }
+                    Some(EntityType::Solid(s)) => Some(Self::solid_hatch_model(s, existing_color)),
                     _ => None,
                 };
                 if let Some(model) = new_model {
@@ -281,7 +280,9 @@ impl Scene {
         let mut block_end = BlockEnd::new();
         block_end.common.handle = end_handle;
         block_end.common.owner_handle = br_handle;
-        self.document.add_entity(EntityType::BlockEnd(block_end)).ok()?;
+        self.document
+            .add_entity(EntityType::BlockEnd(block_end))
+            .ok()?;
         for sub in subs {
             let mut sub = sub.clone();
             view::dispatch::apply_transform(&mut sub, t);
@@ -295,10 +296,10 @@ impl Scene {
 
     pub fn copy_entities(&mut self, handles: &[Handle], t: &EntityTransform) -> Vec<Handle> {
         // Objects on a locked layer can't be copied (they can't be selected).
-        let clones: Vec<EntityType> = handles
+        let clones: Vec<(Handle, EntityType)> = handles
             .iter()
             .filter(|&&h| !self.is_layer_locked(h))
-            .filter_map(|&h| self.document.get_entity(h).cloned())
+            .filter_map(|&h| self.document.get_entity(h).cloned().map(|e| (h, e)))
             .collect();
         // MIRRTEXT also governs the copy path (default MIRROR keeps the source
         // and adds a mirrored copy): keep the copied text right-reading when the
@@ -308,7 +309,8 @@ impl Scene {
         let mirror_true =
             matches!(t, EntityTransform::Mirror { .. }) && self.document.header.mirror_text;
         let mut new_handles = Vec::with_capacity(clones.len());
-        for mut entity in clones {
+        let mut handle_map = rustc_hash::FxHashMap::default();
+        for (src_handle, mut entity) in clones {
             let text_orient = if preserve_text_orientation {
                 capture_text_orient(&entity)
             } else {
@@ -362,12 +364,24 @@ impl Scene {
                 }
             }
             new_handles.push(h);
+            if !h.is_null() {
+                handle_map.insert(src_handle, h);
+            }
+        }
+
+        let copied_groups = self.copy_complete_groups(&handle_map);
+        if copied_groups > 0 && self.is_recording_undo() {
+            // Group copies add Group objects / dictionary entries, which a pure
+            // entity delta cannot restore.
+            self.poison_undo_recording();
         }
         // The copies are new handles (natural memo misses, tessellated fresh)
         // and reference only already-cached blocks — no block defn changes.
         // Report them as additions so derived caches patch in exactly the copies.
-        let changes: Vec<(Handle, ChangeKind)> =
-            new_handles.iter().map(|&h| (h, ChangeKind::Added)).collect();
+        let changes: Vec<(Handle, ChangeKind)> = new_handles
+            .iter()
+            .map(|&h| (h, ChangeKind::Added))
+            .collect();
         self.bump_entities(&changes);
         new_handles
     }