From 39ae993b4823e10417c90a16f8cc8de12a487593 Mon Sep 17 00:00:00 2001 From: Jonathan Date: Thu, 23 Jul 2026 13:07:29 +0200 Subject: [PATCH] Fix: group copies --- src/app/history.rs | 33 ++++++++++---- src/scene/group_layer.rs | 80 +++++++++++++++++++++++++++++++++- src/scene/model/solid_model.rs | 4 +- src/scene/modify.rs | 42 ++++++++++++------ 4 files changed, 133 insertions(+), 26 deletions(-) diff --git a/src/app/history.rs b/src/app/history.rs index 9b95eb071..3fe5d2ee7 100644 --- a/src/app/history.rs +++ b/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)); } } } diff --git a/src/scene/group_layer.rs b/src/scene/group_layer.rs index 22d04a007..3e7d5f161 100644 --- a/src/scene/group_layer.rs +++ b/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, + ) -> 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(); } - } diff --git a/src/scene/model/solid_model.rs b/src/scene/model/solid_model.rs index 791af6e93..e4e666ffc 100644 --- a/src/scene/model/solid_model.rs +++ b/src/scene/model/solid_model.rs @@ -104,9 +104,7 @@ pub fn edge_wires(solid: &Solid) -> Vec { 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; } diff --git a/src/scene/modify.rs b/src/scene/modify.rs index 85efab741..98a51fbc9 100644 --- a/src/scene/modify.rs +++ b/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 = - handles.iter().copied().filter(|&h| !self.is_layer_locked(h)).collect(); + let handles: Vec = 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 { // Objects on a locked layer can't be copied (they can't be selected). - let clones: Vec = 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 }