diff --git a/src/app/block_actions.rs b/src/app/block_actions.rs index f96bd0fe0..95841f92d 100644 --- a/src/app/block_actions.rs +++ b/src/app/block_actions.rs @@ -62,14 +62,22 @@ impl OpenCADStudio { self.tabs[self.active_tab].active_cmd = Some(Box::new(command)); Task::none() } - fn apply_action_document(&mut self, i: usize, mut staged: codec::CadDocument) { + pub(super) fn apply_action_document(&mut self, i: usize, mut staged: codec::CadDocument) { let doc = &mut self.tabs[i].scene.document; let changes = staged .entities() .filter(|e| doc.get_entity(e.common().handle) != Some(*e)) .cloned() .collect::>(); + let removed = doc + .entities() + .map(|e| e.common().handle) + .filter(|h| staged.get_entity(*h).is_none()) + .collect::>(); doc.swap_structure(staged.snapshot_structure()); + for handle in removed { + doc.remove_entity_arc(handle); + } for entity in changes { let handle = entity.common().handle; if let Some(current) = doc.get_entity_mut(handle) { diff --git a/src/app/block_editor.rs b/src/app/block_editor.rs index aa4f66f03..cf5a56be1 100644 --- a/src/app/block_editor.rs +++ b/src/app/block_editor.rs @@ -161,6 +161,8 @@ impl OpenCADStudio { candidate_handles.insert(insert.common.handle); if let Some(record) = document.block_records.get(&insert.block_name) { candidate_handles.extend(record.entity_handles.iter().copied()); + candidate_handles.insert(record.block_entity_handle); + candidate_handles.insert(record.block_end_handle); } } } @@ -168,15 +170,10 @@ impl OpenCADStudio { } let mut sub_handles = Vec::new(); for &h in &candidate_handles { - let entity_opt = before_doc - .get_entity(h) - .or_else(|| tab.scene.document.get_entity(h)); - if let Some(EntityType::Insert(ins)) = entity_opt { - for attr in &ins.attributes { - sub_handles.push(attr.common.handle); - } - if let Some(seqend) = ins.seqend_handle { - sub_handles.push(seqend); + for document in [before_doc, &tab.scene.document] { + if let Some(EntityType::Insert(insert)) = document.get_entity(h) { + sub_handles.extend(insert.attributes.iter().map(|a| a.common.handle)); + sub_handles.extend(insert.seqend_handle); } } } @@ -578,6 +575,49 @@ impl OpenCADStudio { true } + /// Save-time representation edits belong to the preceding editor command, + /// so local Undo cannot leave untracked members or reuse their handles. + fn coalesce_saved_representations(&mut self, i: usize, before: &CadDocument) { + let tab = &mut self.tabs[i]; + let doc = &tab.scene.document; + if before == doc { + return; + } + let Some(HistorySnapshot::Delta(delta)) = tab.history.undo_stack.last_mut() else { + return; + }; + let handles = before + .entities() + .chain(doc.entities()) + .map(|e| e.common().handle) + .collect::>(); + for handle in handles { + let a = before.get_entity_arc(handle); + let b = doc.get_entity_arc(handle); + if a == b { + continue; + } + if let Some((_, _, after)) = delta.entities.iter_mut().find(|(h, _, _)| *h == handle) { + *after = b; + } else { + delta.entities.push((handle, a, b)); + } + } + if !delta + .structure + .as_ref() + .is_some_and(StructureSnapshot::is_full) + { + let mut previous = before.clone(); + let broad = StructureSnapshot::Full(previous.snapshot_structure()); + delta.structure = Some(match delta.structure.take() { + Some(specialized) => StructureSnapshot::Combined(vec![broad, specialized]), + None => broad, + }); + } + delta.dirty_after = true; + } + pub(super) fn save_block_edit(&mut self, i: usize) -> bool { let Some(session) = self.tabs[i].active_block_edit_session() else { self.command_line @@ -617,11 +657,33 @@ impl OpenCADStudio { // A dynamic block leaves the editor showing its first visibility // state, with erased objects dropped from its states. let definition = session.br_handle; - crate::scene::block_visibility::finish_editing( - &mut self.tabs[i].scene.document, - definition, - ); - self.tabs[i].scene.visibility_states.remove(&definition); + if self.block_edit_delta(i, true).is_some() { + self.reconcile_block_edit_membership(i); + self.finish_pending_history(i); + let before_sync = self.tabs[i].scene.document.clone(); + let mut staged = before_sync.clone(); + crate::scene::block_visibility::finish_editing(&mut staged, definition); + crate::scene::block_actions::prune_erased_targets(&mut staged, definition); + let before = &self.tabs[i] + .block_edit_history + .as_ref() + .unwrap() + .checkpoint + .document; + match crate::scene::block_actions::instance::refresh_after_edit( + &staged, before, definition, + ) { + Ok(document) => { + self.apply_action_document(i, document); + self.coalesce_saved_representations(i, &before_sync); + } + Err(error) => { + self.command_line.push_error(crate::t!(error).as_ref()); + return false; + } + } + self.tabs[i].scene.visibility_states.remove(&definition); + } if let Some(delta) = self.block_edit_delta(i, true) { let mut history = self.tabs[i].block_edit_history.take().unwrap(); std::mem::swap(&mut self.tabs[i].history, &mut history.main); diff --git a/src/app/block_editor_tests.rs b/src/app/block_editor_tests.rs index c095af535..47e911186 100644 --- a/src/app/block_editor_tests.rs +++ b/src/app/block_editor_tests.rs @@ -68,6 +68,338 @@ fn edit_action_set(app: &mut OpenCADStudio, action: Handle, mode: &str, targets: assert!(app.tabs[app.active_tab].active_cmd.is_none()); } +#[test] +fn block_actions_topology_save_updates_values_members_properties_and_history() { + use crate::scene::block_actions::{self as actions, Kind}; + let (mut app, reference) = fixture(); + let i = app.active_tab; + let second = app.tabs[i] + .scene + .document + .add_entity(EntityType::Insert(Insert::new( + "EDIT_ME", + Vector3::new(40., 0., 0.), + ))) + .unwrap(); + begin(&mut app, reference); + circle(&mut app, 3.); + let _ = app.run_command_line("BPARAMETER LINEAR 0,0 10,0 5,2"); + let def = app.tabs[i].active_block_edit_session().unwrap().br_handle; + let p = actions::parameters(&app.tabs[i].scene.document, def, Kind::Move)[0].handle; + let old_members = members(&app); + let a = actions::create( + &mut app.tabs[i].scene.document, + def, + Kind::Move, + p, + &old_members[..1], + glam::DVec3::ZERO, + ) + .unwrap(); + let _ = app.dispatch_command("BSAVE"); + let _ = app.dispatch_command("BCLOSE"); + for (insert, x) in [(reference, 12.), (second, 54.)] { + let _ = app.handle_block_action_result(crate::command::CmdResult::EvaluateBlockParameter { + insert: Some(insert), + document: app.tabs[i].id, + definition: def, + parameter: p, + value: actions::Value::Point(glam::DVec3::new(x, 0., 0.)), + }); + } + let check = |app: &OpenCADStudio, end: f64, center: f64| { + let doc = &app.tabs[app.active_tab].scene.document; + for (insert, delta) in [(reference, 2.), (second, 4.)] { + let EntityType::Insert(ins) = doc.get_entity(insert).unwrap() else { + panic!() + }; + let handles = &doc + .block_records + .get(&ins.block_name) + .unwrap() + .entity_handles; + assert_eq!(handles.len(), 2); + let EntityType::Line(line) = doc.get_entity(handles[0]).unwrap() else { + panic!() + }; + assert_eq!(line.start.x, delta); + assert_eq!(line.end.x, end + delta); + let EntityType::Circle(circle) = doc.get_entity(handles[1]).unwrap() else { + panic!() + }; + assert_eq!(circle.center.x, center); + } + }; + check(&app, 1., 3.); + begin(&mut app, reference); + app.push_undo_snapshot(i, "Definition edit"); + let EntityType::Line(line) = app.tabs[i] + .scene + .document + .get_entity_mut(old_members[0]) + .unwrap() + else { + panic!() + }; + line.end.x = 2.; + line.common.color = codec::types::Color::Index(3); + app.tabs[i].scene.erase_entities(&old_members[1..]); + app.finish_pending_history(i); + circle(&mut app, 7.); + let added = members(&app)[1]; + let _ = app.dispatch_command("BSAVE"); + check(&app, 2., 7.); + let saved = app.tabs[i].scene.document.clone(); + let depth = main_depth(&app); + let _ = app.dispatch_command("BSAVE"); + assert_eq!(main_depth(&app), depth); + assert_eq!( + app.tabs[i].scene.document, saved, + "saving unchanged authoring does not rebuild again" + ); + let _ = app.dispatch_command("BCLOSE"); + app.undo_steps(1); + check(&app, 1., 3.); + assert!(app.tabs[i].scene.document.get_entity(added).is_none()); + app.redo_steps(1); + check(&app, 2., 7.); + for insert in [reference, second] { + let doc = &app.tabs[i].scene.document; + let EntityType::Insert(ins) = doc.get_entity(insert).unwrap() else { + panic!() + }; + let member = doc + .block_records + .get(&ins.block_name) + .unwrap() + .entity_handles[0]; + assert_eq!( + doc.get_entity(member).unwrap().common().color, + codec::types::Color::Index(3) + ); + } + let bytes = codec::DwgWriter::write_to_vec(&app.tabs[i].scene.document).unwrap(); + app.tabs[i].scene.document = codec::DwgReader::from_stream(std::io::Cursor::new(bytes)) + .read() + .unwrap(); + check(&app, 2., 7.); + begin(&mut app, reference); + edit_action_set(&mut app, a, "Add", vec![added]); + let _ = app.dispatch_command("BSAVE"); + let _ = app.dispatch_command("BCLOSE"); + // New geometry can now join an action even on previously evaluated references. + for (insert, center) in [(reference, 9.), (second, 11.)] { + let doc = &app.tabs[i].scene.document; + let EntityType::Insert(ins) = doc.get_entity(insert).unwrap() else { + panic!() + }; + let h = doc + .block_records + .get(&ins.block_name) + .unwrap() + .entity_handles[1]; + let EntityType::Circle(circle) = doc.get_entity(h).unwrap() else { + panic!() + }; + assert_eq!(circle.center.x, center); + } +} + +#[test] +fn block_actions_topology_save_coalesces_bounded_layer_history_in_both_directions() { + use crate::scene::block_actions::{self as actions, Kind}; + let (mut app, reference) = fixture(); + begin(&mut app, reference); + let i = app.active_tab; + let _ = app.run_command_line("BPARAMETER LINEAR 0,0 10,0 5,2"); + let def = app.tabs[i].active_block_edit_session().unwrap().br_handle; + let p = actions::parameters(&app.tabs[i].scene.document, def, Kind::Move)[0].handle; + let targets = members(&app); + actions::create( + &mut app.tabs[i].scene.document, + def, + Kind::Move, + p, + &targets, + glam::DVec3::ZERO, + ) + .unwrap(); + let _ = app.dispatch_command("BSAVE"); + let _ = app.dispatch_command("BCLOSE"); + let _ = app.handle_block_action_result(crate::command::CmdResult::EvaluateBlockParameter { + insert: Some(reference), + document: app.tabs[i].id, + definition: def, + parameter: p, + value: actions::Value::Point(glam::DVec3::new(12., 0., 0.)), + }); + begin(&mut app, reference); + circle(&mut app, 3.); + let original = app.tabs[i].scene.document.layers.get("0").unwrap().color; + let pending = app.begin_layer_undo(i, "Layer color", &["0".into()]); + app.tabs[i] + .scene + .document + .layers + .get_mut("0") + .unwrap() + .color = codec::types::Color::Index(1); + app.commit_layer_undo(i, pending); + let _ = app.dispatch_command("BSAVE"); + assert!( + matches!(app.tabs[i].history.undo_stack.last(),Some(super::document::HistorySnapshot::Delta(d)) if matches!(d.structure,Some(super::document::StructureSnapshot::Combined(_)))) + ); + app.undo_steps(1); + assert_eq!( + app.tabs[i].scene.document.layers.get("0").unwrap().color, + original + ); + app.undo_steps(1); + assert_eq!(members(&app).len(), 1); + app.redo_steps(2); + let doc = &app.tabs[i].scene.document; + assert_eq!( + doc.layers.get("0").unwrap().color, + codec::types::Color::Index(1) + ); + let EntityType::Insert(ins) = doc.get_entity(reference).unwrap() else { + panic!() + }; + let record = doc.block_records.get(&ins.block_name).unwrap(); + assert_eq!(record.entity_handles.len(), 2); + assert!(record + .entity_handles + .iter() + .all(|h| doc.get_entity(*h).is_some())); + assert_eq!(members(&app).len(), 2); +} + +#[test] +fn block_actions_topology_local_undo_after_save_resave_has_no_orphan_members() { + use crate::scene::block_actions::{self as actions, Kind}; + let (mut app, reference) = fixture(); + begin(&mut app, reference); + let i = app.active_tab; + let _ = app.run_command_line("BPARAMETER LINEAR 0,0 10,0 5,2"); + let def = app.tabs[i].active_block_edit_session().unwrap().br_handle; + let p = actions::parameters(&app.tabs[i].scene.document, def, Kind::Move)[0].handle; + let targets = members(&app); + actions::create( + &mut app.tabs[i].scene.document, + def, + Kind::Move, + p, + &targets, + glam::DVec3::ZERO, + ) + .unwrap(); + let _ = app.dispatch_command("BSAVE"); + let _ = app.dispatch_command("BCLOSE"); + let _ = app.handle_block_action_result(crate::command::CmdResult::EvaluateBlockParameter { + insert: Some(reference), + document: app.tabs[i].id, + definition: def, + parameter: p, + value: actions::Value::Point(glam::DVec3::new(12., 0., 0.)), + }); + begin(&mut app, reference); + circle(&mut app, 3.); + let _ = app.dispatch_command("BSAVE"); + app.undo_steps(1); + circle(&mut app, 7.); + let _ = app.dispatch_command("BSAVE"); + let _ = app.dispatch_command("BCLOSE"); + let doc = &app.tabs[i].scene.document; + let EntityType::Insert(ins) = doc.get_entity(reference).unwrap() else { + panic!() + }; + let record = doc.block_records.get(&ins.block_name).unwrap(); + assert_eq!(record.entity_handles.len(), 2); + let owned = doc + .entities() + .filter(|e| { + e.common().owner_handle == record.handle + && !matches!(e, EntityType::Block(_) | EntityType::BlockEnd(_)) + }) + .count(); + assert_eq!(owned, 2); + let EntityType::Circle(circle) = doc.get_entity(record.entity_handles[1]).unwrap() else { + panic!() + }; + assert_eq!(circle.center.x, 7.); + let bytes = codec::DwgWriter::write_to_vec(doc).unwrap(); + let reopened = codec::DwgReader::from_stream(std::io::Cursor::new(bytes)) + .read() + .unwrap(); + let record = reopened.block_records.get(&ins.block_name).unwrap(); + assert_eq!(record.entity_handles.len(), 2); +} + +#[test] +fn block_actions_topology_erase_last_target_empty_definition_discard_and_save() { + use crate::scene::block_actions::{self as actions, Kind}; + let (mut app, reference) = fixture(); + begin(&mut app, reference); + let i = app.active_tab; + let _ = app.run_command_line("BPARAMETER LINEAR 0,0 10,0 5,2"); + let def = app.tabs[i].active_block_edit_session().unwrap().br_handle; + let p = actions::parameters(&app.tabs[i].scene.document, def, Kind::Move)[0].handle; + let targets = members(&app); + let a = actions::create( + &mut app.tabs[i].scene.document, + def, + Kind::Move, + p, + &targets, + glam::DVec3::ZERO, + ) + .unwrap(); + let _ = app.dispatch_command("BSAVE"); + let _ = app.dispatch_command("BCLOSE"); + let _ = app.handle_block_action_result(crate::command::CmdResult::EvaluateBlockParameter { + insert: Some(reference), + document: app.tabs[i].id, + definition: def, + parameter: p, + value: actions::Value::Point(glam::DVec3::new(12., 0., 0.)), + }); + let original = app.tabs[i].scene.document.clone(); + begin(&mut app, reference); + circle(&mut app, 5.); + let _ = app.dispatch_command("BEDIT_DISCARD"); + assert_eq!(app.tabs[i].scene.document, original); + begin(&mut app, reference); + app.push_undo_snapshot(i, "Erase targets"); + app.tabs[i].scene.erase_entities(&targets); + app.finish_pending_history(i); + let _ = app.dispatch_command("BSAVE"); + let _ = app.dispatch_command("BCLOSE"); + assert!(actions::actions(&app.tabs[i].scene.document, def)[0] + .1 + .dependencies + .is_empty()); + assert!(app.tabs[i].scene.document.objects.contains_key(&a)); + let EntityType::Insert(ins) = app.tabs[i].scene.document.get_entity(reference).unwrap() else { + panic!() + }; + assert!(app.tabs[i] + .scene + .document + .block_records + .get(&ins.block_name) + .unwrap() + .entity_handles + .is_empty()); + app.undo_steps(1); + assert_eq!( + app.tabs[i].scene.document.get_entity(reference), + original.get_entity(reference) + ); + assert!(app.tabs[i].scene.document.get_entity(targets[0]).is_some()); + app.redo_steps(1); + assert!(app.tabs[i].scene.document.get_entity(targets[0]).is_none()); +} + #[test] fn block_actions_set_edit_refresh_local_history_discard_save_and_drawing_history() { use crate::scene::block_actions::{self as actions, Kind}; diff --git a/src/app/document.rs b/src/app/document.rs index dfe2434c2..7ce08dc83 100644 --- a/src/app/document.rs +++ b/src/app/document.rs @@ -876,6 +876,8 @@ pub(super) struct DeltaSnapshot { #[derive(Clone)] pub(super) enum StructureSnapshot { + /// A broad save-time synchronization followed by a command's bounded edits. + Combined(Vec), /// Compatibility fallback for genuinely broad structural commands. Full(CadDocument), /// Exact layer-table entries touched by one command. @@ -901,6 +903,7 @@ pub(super) enum StructureSnapshot { impl StructureSnapshot { pub(super) fn estimated_bytes(&self) -> usize { match self { + Self::Combined(parts) => parts.iter().map(Self::estimated_bytes).sum(), Self::Full(doc) => doc.objects.len().saturating_mul(192), Self::Layers(entries) => entries.len().saturating_mul(256), Self::TextStyles(entries) => entries.len().saturating_mul(320), @@ -914,6 +917,7 @@ impl StructureSnapshot { pub(super) fn is_full(&self) -> bool { matches!(self, Self::Full(_)) + || matches!(self,Self::Combined(parts) if parts.iter().any(Self::is_full)) } } diff --git a/src/app/history.rs b/src/app/history.rs index daec60df5..00ab3cea5 100644 --- a/src/app/history.rs +++ b/src/app/history.rs @@ -962,6 +962,143 @@ impl OpenCADStudio { self.push_undo_entry(i, HistorySnapshot::Delta(Box::new(delta))); } + fn apply_structure_state(&mut self, i: usize, structure: &mut StructureSnapshot, undo: bool) { + match structure { + StructureSnapshot::Combined(parts) => { + if undo { + for part in parts { + self.apply_structure_state(i, part, undo); + } + } else { + for part in parts.iter_mut().rev() { + self.apply_structure_state(i, part, undo); + } + } + } + StructureSnapshot::Full(stored) => { + let inverse = self.tabs[i] + .scene + .document + .swap_structure(std::mem::replace(stored, codec::CadDocument::new())); + *stored = inverse; + self.tabs[i].scene.invalidate_dependency_index(); + self.tabs[i].scene.bump_layout_epoch(); + self.tabs[i].scene.bump_scale_epoch(); + self.tabs[i].scene.bump_ucs_epoch(); + } + StructureSnapshot::Layers(entries) => { + let names: Vec = entries.iter().map(|entry| entry.name.clone()).collect(); + for entry in entries { + let value = if undo { + entry.before.clone() + } else { + entry.after.clone() + }; + if let Some(layer) = value { + self.tabs[i].scene.document.layers.add_or_replace(layer); + } else { + self.tabs[i].scene.document.layers.remove(&entry.name); + } + } + self.tabs[i].scene.invalidate_layer_dependencies(&names); + } + StructureSnapshot::TextStyles(entries) => { + let names: Vec = entries.iter().map(|entry| entry.name.clone()).collect(); + for entry in entries { + let value = if undo { + entry.before.clone() + } else { + entry.after.clone() + }; + if let Some(style) = value { + self.tabs[i] + .scene + .document + .text_styles + .add_or_replace(style); + } else { + self.tabs[i].scene.document.text_styles.remove(&entry.name); + } + } + for name in names { + self.tabs[i].scene.invalidate_text_style_dependencies(&name); + } + } + StructureSnapshot::DimStyles(entries) => { + let names: Vec = entries.iter().map(|entry| entry.name.clone()).collect(); + for entry in entries { + let value = if undo { + entry.before.clone() + } else { + entry.after.clone() + }; + if let Some(style) = value { + self.tabs[i].scene.document.dim_styles.add_or_replace(style); + } else { + self.tabs[i].scene.document.dim_styles.remove(&entry.name); + } + } + for name in names { + self.tabs[i].scene.invalidate_dim_style_dependencies(&name); + } + } + StructureSnapshot::Objects(entries) => { + let mut layout_touched = false; + let mut scale_touched = false; + for entry in entries { + let value = if undo { + entry.before.clone() + } else { + entry.after.clone() + }; + // Either direction can add/remove a layout: undoing + // an insert removes it (`before` is `None`), redoing + // re-inserts it — so both sides must be inspected. + layout_touched |= + matches!(entry.before, Some(codec::objects::ObjectType::Layout(_))) + || matches!(entry.after, Some(codec::objects::ObjectType::Layout(_))); + scale_touched |= + matches!(entry.before, Some(codec::objects::ObjectType::Scale(_))) + || matches!(entry.after, Some(codec::objects::ObjectType::Scale(_))); + if let Some(object) = value { + self.tabs[i] + .scene + .document + .objects + .insert(entry.handle, object); + } else { + self.tabs[i].scene.document.objects.remove(&entry.handle); + } + } + if layout_touched { + self.tabs[i].scene.bump_layout_epoch(); + } + if scale_touched { + self.tabs[i].scene.bump_scale_epoch(); + } + } + StructureSnapshot::Styles { + before, + after, + text_names, + dim_names, + object_handles, + } => { + let snapshot = if undo { before } else { after }; + self.restore_style_state(i, snapshot); + self.tabs[i] + .scene + .invalidate_text_style_dependencies_many(text_names); + self.tabs[i] + .scene + .invalidate_dim_style_dependencies_many(dim_names); + self.tabs[i] + .scene + .invalidate_object_style_dependencies(object_handles); + } + } + } + /// Apply one side of a delta entry in place: `undo` restores each entity's /// before-image, `!undo` (redo) its after-image. `None` on the chosen side /// means the entity is absent there (erase it). Reports the exact touched @@ -977,138 +1114,7 @@ impl OpenCADStudio { // and UI invalidation are deferred until every requested undo/redo step // has been applied. if let Some(structure) = d.structure.as_mut() { - match structure { - StructureSnapshot::Full(stored) => { - let inverse = self.tabs[i] - .scene - .document - .swap_structure(std::mem::replace(stored, codec::CadDocument::new())); - *stored = inverse; - self.tabs[i].scene.invalidate_dependency_index(); - self.tabs[i].scene.bump_layout_epoch(); - self.tabs[i].scene.bump_scale_epoch(); - self.tabs[i].scene.bump_ucs_epoch(); - } - StructureSnapshot::Layers(entries) => { - let names: Vec = - entries.iter().map(|entry| entry.name.clone()).collect(); - for entry in entries { - let value = if undo { - entry.before.clone() - } else { - entry.after.clone() - }; - if let Some(layer) = value { - self.tabs[i].scene.document.layers.add_or_replace(layer); - } else { - self.tabs[i].scene.document.layers.remove(&entry.name); - } - } - self.tabs[i].scene.invalidate_layer_dependencies(&names); - } - StructureSnapshot::TextStyles(entries) => { - let names: Vec = - entries.iter().map(|entry| entry.name.clone()).collect(); - for entry in entries { - let value = if undo { - entry.before.clone() - } else { - entry.after.clone() - }; - if let Some(style) = value { - self.tabs[i] - .scene - .document - .text_styles - .add_or_replace(style); - } else { - self.tabs[i].scene.document.text_styles.remove(&entry.name); - } - } - for name in names { - self.tabs[i].scene.invalidate_text_style_dependencies(&name); - } - } - StructureSnapshot::DimStyles(entries) => { - let names: Vec = - entries.iter().map(|entry| entry.name.clone()).collect(); - for entry in entries { - let value = if undo { - entry.before.clone() - } else { - entry.after.clone() - }; - if let Some(style) = value { - self.tabs[i].scene.document.dim_styles.add_or_replace(style); - } else { - self.tabs[i].scene.document.dim_styles.remove(&entry.name); - } - } - for name in names { - self.tabs[i].scene.invalidate_dim_style_dependencies(&name); - } - } - StructureSnapshot::Objects(entries) => { - let mut layout_touched = false; - let mut scale_touched = false; - for entry in entries { - let value = if undo { - entry.before.clone() - } else { - entry.after.clone() - }; - // Either direction can add/remove a layout: undoing - // an insert removes it (`before` is `None`), redoing - // re-inserts it — so both sides must be inspected. - layout_touched |= - matches!(entry.before, Some(codec::objects::ObjectType::Layout(_))) - || matches!( - entry.after, - Some(codec::objects::ObjectType::Layout(_)) - ); - scale_touched |= - matches!(entry.before, Some(codec::objects::ObjectType::Scale(_))) - || matches!( - entry.after, - Some(codec::objects::ObjectType::Scale(_)) - ); - if let Some(object) = value { - self.tabs[i] - .scene - .document - .objects - .insert(entry.handle, object); - } else { - self.tabs[i].scene.document.objects.remove(&entry.handle); - } - } - if layout_touched { - self.tabs[i].scene.bump_layout_epoch(); - } - if scale_touched { - self.tabs[i].scene.bump_scale_epoch(); - } - } - StructureSnapshot::Styles { - before, - after, - text_names, - dim_names, - object_handles, - } => { - let snapshot = if undo { before } else { after }; - self.restore_style_state(i, snapshot); - self.tabs[i] - .scene - .invalidate_text_style_dependencies_many(text_names); - self.tabs[i] - .scene - .invalidate_dim_style_dependencies_many(dim_names); - self.tabs[i] - .scene - .invalidate_object_style_dependencies(object_handles); - } - } + self.apply_structure_state(i, structure, undo); } let mut changes = self.tabs[i].scene.apply_entity_delta(&d.entities, undo); let scene = &mut self.tabs[i].scene; @@ -1341,8 +1347,8 @@ impl OpenCADStudio { layer_panel_changed |= d.structure.as_ref().is_some_and(|structure| { matches!( structure, - StructureSnapshot::Full(_) | StructureSnapshot::Layers(_) - ) + StructureSnapshot::Layers(_) + ) || structure.is_full() }) || d.active_layer.is_some() || d.entities.iter().any(|(_, before, after)| { before @@ -1411,8 +1417,8 @@ impl OpenCADStudio { layer_panel_changed |= d.structure.as_ref().is_some_and(|structure| { matches!( structure, - StructureSnapshot::Full(_) | StructureSnapshot::Layers(_) - ) + StructureSnapshot::Layers(_) + ) || structure.is_full() }) || d.active_layer.is_some() || d.entities.iter().any(|(_, before, after)| { before diff --git a/src/scene/block_actions.rs b/src/scene/block_actions.rs index e1d762b01..5be67e5aa 100644 --- a/src/scene/block_actions.rs +++ b/src/scene/block_actions.rs @@ -10,6 +10,7 @@ use codec::{CadDocument, EntityType, Handle}; use glam::DVec3; pub(crate) mod instance; mod stretch; +mod representation; #[cfg(test)] mod tests; @@ -638,6 +639,35 @@ pub(crate) fn actions( }) .collect() } +/// Drop erased objects from action targets without replacing graph nodes. +pub(crate) fn prune_erased_targets(doc: &mut CadDocument, definition: Handle) { + let live = authoring::members(doc, definition) + .into_iter() + .collect::>(); + let handles = actions(doc, definition) + .into_iter() + .map(|(h, _, _, _)| h) + .collect::>(); + for h in handles { + let Some(ObjectType::DynamicBlock(o)) = doc.objects.get_mut(&h) else { + continue; + }; + let action = match &mut o.data { + DynamicBlockData::MoveAction(a) => &mut a.action, + DynamicBlockData::FlipAction(a) => &mut a.action, + DynamicBlockData::StretchAction(a) => { + a.handles.retain(|b| live.contains(&b.handle)); + &mut a.action + } + DynamicBlockData::ScaleAction(a) | DynamicBlockData::RotateAction(a) => { + &mut a.action.action + } + _ => continue, + }; + action.dependencies.retain(|h| live.contains(h)); + } +} + pub(crate) fn frame_outline(doc: &CadDocument, handle: Handle) -> Vec { let Some(ObjectType::DynamicBlock(o)) = doc.objects.get(&handle) else { return vec![]; diff --git a/src/scene/block_actions/instance.rs b/src/scene/block_actions/instance.rs index cd3800bbb..31decd0fb 100644 --- a/src/scene/block_actions/instance.rs +++ b/src/scene/block_actions/instance.rs @@ -308,7 +308,19 @@ pub(crate) fn evaluate_value( /// Refresh existing evaluated references after an authoring change. pub(crate) fn refresh(doc: &CadDocument, def: Handle) -> Result { + refresh_after_edit(doc, doc, def) +} + +pub(crate) fn refresh_after_edit( + doc: &CadDocument, + before: &CadDocument, + def: Handle, +) -> Result { + if authoring::graph(doc, def)?.is_none() { + return Ok(doc.clone()); + } let mut staged = doc.clone(); + authoring::tag_members(&mut staged, def); let inserts = doc .entities() .filter_map(|e| match e { @@ -331,7 +343,11 @@ pub(crate) fn refresh(doc: &CadDocument, def: Handle) -> Result Result { let mut scene = crate::scene::Scene::new(); scene.document = doc.clone(); + authoring::tag_members(&mut scene.document, def); scene.dynamic_representation(insert); let Some(EntityType::Insert(reference)) = scene.document.get_entity(insert) else { return Err("Missing block reference."); @@ -357,37 +374,6 @@ fn materialize( { return Err("Could not isolate the dynamic block reference."); } - // Rebuild all members so removed targets lose their old action effects. - // Preserve each representation member's identity and visibility. - let source = doc - .block_records - .iter() - .find(|b| b.handle == def) - .ok_or("Missing definition.")?; - let target = scene.document.block_records.get(&name).unwrap(); - if source.entity_handles.len() != target.entity_handles.len() { - return Err("The reference topology no longer matches its definition."); - } - let pairs = source - .entity_handles - .iter() - .copied() - .zip(target.entity_handles.iter().copied()) - .collect::>(); - for (from, to) in pairs { - let mut e = evaluated - .get_entity(from) - .cloned() - .ok_or("Missing action target.")?; - let target = scene - .document - .get_entity_mut(to) - .ok_or("Missing representation target.")?; - if std::mem::discriminant(&e) != std::mem::discriminant(target) { - return Err("The reference topology no longer matches its definition."); - } - *e.common_mut() = target.common().clone(); - *target = e; - } + super::representation::reconcile(&mut scene, def, &name, evaluated)?; Ok(scene.document) } diff --git a/src/scene/block_actions/representation.rs b/src/scene/block_actions/representation.rs new file mode 100644 index 000000000..ea50cd1e1 --- /dev/null +++ b/src/scene/block_actions/representation.rs @@ -0,0 +1,305 @@ +//! Synchronize evaluated members by native element identity, never by position. +use super::*; +use crate::scene::Scene; +use rustc_hash::FxHashMap; + +pub(super) fn reconcile( + scene: &mut Scene, + def: Handle, + name: &str, + evaluated: &CadDocument, +) -> Result<(), &'static str> { + authoring::tag_members(&mut scene.document, def); + let rep = scene + .document + .block_records + .get(name) + .ok_or("Missing representation.")? + .handle; + let sources = authoring::members(&scene.document, def); + let old = scene + .document + .entities() + .filter(|e| { + e.common().owner_handle == rep + && !matches!(e, EntityType::Block(_) | EntityType::BlockEnd(_)) + }) + .map(|e| e.common().handle) + .collect::>(); + let mut by_id = std::collections::BTreeMap::new(); + for h in &old { + let e = scene + .document + .get_entity(*h) + .ok_or("Missing representation member.")?; + let id = + authoring::element_id(e).ok_or("A representation member has no native element ID.")?; + if by_id.insert(id, *h).is_some() { + return Err("The representation has duplicate element IDs."); + } + } + let mut mapping = FxHashMap::default(); + let mut pairs = vec![]; + for &from in &sources { + let source = scene + .document + .get_entity(from) + .ok_or("Missing definition member.")?; + let id = + authoring::element_id(source).ok_or("A definition member has no native element ID.")?; + let existing = by_id.get(&id).copied().filter(|h| { + scene + .document + .get_entity(*h) + .is_some_and(|e| std::mem::discriminant(e) == std::mem::discriminant(source)) + }); + let to = existing.unwrap_or_else(|| scene.document.allocate_handle()); + mapping.insert(from, to); + pairs.push((from, to, id, existing.is_some())); + } + let mut ordered = vec![]; + for (from, to, id, existing) in pairs { + let mut entity = evaluated + .get_entity(from) + .cloned() + .ok_or("Missing evaluated member.")?; + let source_xdict = entity.common().xdictionary_handle; + if existing { + let previous = scene + .document + .get_entity(to) + .cloned() + .ok_or("Missing representation member.")?; + let current = previous.common(); + match (&mut entity, &previous) { + (EntityType::Insert(insert), EntityType::Insert(old)) => { + let mut used = std::collections::BTreeSet::new(); + for attribute in &mut insert.attributes { + let retained = old.attributes.iter().find(|a| { + a.tag.eq_ignore_ascii_case(&attribute.tag) + && !used.contains(&a.common.handle) + }); + attribute.common.handle = retained + .map(|a| a.common.handle) + .unwrap_or_else(|| scene.document.allocate_handle()); + attribute.common.owner_handle = to; + attribute.common.xdictionary_handle = + retained.and_then(|a| a.common.xdictionary_handle); + attribute.common.raw_record = None; + used.insert(attribute.common.handle); + } + insert.seqend_handle = if insert.attributes.is_empty() { + None + } else { + old.seqend_handle + .or_else(|| Some(scene.document.allocate_handle())) + }; + for attribute in &old.attributes { + if !used.contains(&attribute.common.handle) { + scene.document.remove_entity_arc(attribute.common.handle); + } + } + } + (EntityType::Polyline3D(poly), EntityType::Polyline3D(old)) => { + for (i, v) in poly.vertices.iter_mut().enumerate() { + v.handle = old + .vertices + .get(i) + .map(|v| v.handle) + .unwrap_or_else(|| scene.document.allocate_handle()); + } + } + _ => {} + } + entity.common_mut().xdictionary_handle = current.xdictionary_handle; + entity.common_mut().reactors = current.reactors.clone(); + if crate::scene::block_visibility::parameter(&scene.document, def) + .is_some_and(|p| p.all_blocks.contains(&from)) + { + entity.common_mut().invisible = current.invisible; + } + } else { + Scene::reset_clone_subhandles(&mut scene.document, &mut entity); + if let EntityType::Insert(insert) = &mut entity { + for attribute in &mut insert.attributes { + attribute.common.owner_handle = to; + } + if insert.seqend_handle.is_some() || !insert.attributes.is_empty() { + insert.seqend_handle = Some(scene.document.allocate_handle()); + } + } + entity.common_mut().xdictionary_handle = None; + entity.common_mut().reactors.clear(); + } + entity.common_mut().handle = to; + entity.common_mut().owner_handle = rep; + entity.common_mut().raw_record = None; + if existing { + *scene.document.get_entity_mut(to).unwrap() = entity; + } else { + scene + .document + .add_entity(entity) + .map_err(|_| "Could not add a representation member.")?; + if let Some(root) = source_xdict { + let cloned = Scene::clone_xdict_tree( + &mut scene.document, + from, + root, + to, + &mapping, + &FxHashMap::default(), + &mut vec![], + ) + .ok_or("Could not copy the new member's extension dictionary.")?; + scene + .document + .get_entity_mut(to) + .unwrap() + .common_mut() + .xdictionary_handle = Some(cloned); + } + } + let attributes = match scene.document.get_entity(to) { + Some(EntityType::Insert(i)) => i.attributes.clone(), + _ => vec![], + }; + for attribute in attributes { + if let Some(stored) = scene.document.get_entity_mut(attribute.common.handle) { + *stored = EntityType::AttributeEntity(attribute); + } + } + authoring::set_entity_tag(&mut scene.document, to, authoring::element_tag(id, to)); + ordered.push(to); + } + for h in old { + if !ordered.contains(&h) { + let attrs = match scene.document.get_entity(h) { + Some(EntityType::Insert(i)) => i + .attributes + .iter() + .map(|a| a.common.handle) + .collect::>(), + _ => vec![], + }; + for attr in attrs { + scene.document.remove_entity_arc(attr); + } + scene.document.remove_entity_arc(h); + } + } + scene + .document + .block_records + .get_mut(name) + .unwrap() + .entity_handles = ordered; + // Keep the anonymous block's native base point in step with its definition. + let source_name = scene + .document + .block_records + .iter() + .find(|b| b.handle == def) + .unwrap() + .name + .clone(); + let base = crate::scene::render_graph::block_base_point(&scene.document, &source_name); + let marker = scene + .document + .block_records + .get(name) + .unwrap() + .block_entity_handle; + if let Some(EntityType::Block(block)) = scene.document.get_entity_mut(marker) { + block.base_point = base; + } + scene + .document + .block_records + .get_mut(name) + .unwrap() + .base_point = base; + Ok(()) +} + +fn state_name(doc: &CadDocument, def: Handle, insert: Handle) -> Option<(String, usize)> { + let parameter = crate::scene::block_visibility::parameter(doc, def)?; + let EntityType::Insert(reference) = doc.get_entity(insert)? else { + return None; + }; + let rep = doc.block_records.get(&reference.block_name)?; + let by_id = rep + .entity_handles + .iter() + .filter_map(|h| { + let e = doc.get_entity(*h)?; + Some((authoring::element_id(e)?, !e.common().invisible)) + }) + .collect::>(); + parameter.states.iter().enumerate().find_map(|(i, state)| { + let matches = parameter.all_blocks.iter().all(|h| { + doc.get_entity(*h) + .and_then(authoring::element_id) + .and_then(|id| by_id.get(&id)) + .is_some_and(|shown| *shown == state.visible_blocks.contains(h)) + }); + matches.then(|| (state.name.clone(), i)) + }) +} + +pub(super) fn visibility_state( + doc: &CadDocument, + before: &CadDocument, + def: Handle, + insert: Handle, +) -> Option { + let current = crate::scene::block_visibility::parameter(doc, def)?; + let (name, index) = state_name(before, def, insert).or_else(|| state_name(doc, def, insert))?; + Some( + current + .states + .iter() + .position(|s| s.name == name) + .unwrap_or(index.min(current.states.len().saturating_sub(1))), + ) +} + +pub(super) fn apply_visibility( + doc: &mut CadDocument, + def: Handle, + insert: Handle, + state: usize, +) -> Result<(), &'static str> { + let parameter = crate::scene::block_visibility::parameter(doc, def) + .ok_or("Missing visibility parameter.")?; + let state = parameter + .states + .get(state) + .ok_or("Missing visibility state.")?; + let flags = parameter + .all_blocks + .iter() + .filter_map(|h| { + Some(( + authoring::element_id(doc.get_entity(*h)?)?, + !state.visible_blocks.contains(h), + )) + }) + .collect::>(); + let EntityType::Insert(reference) = doc.get_entity(insert).ok_or("Missing reference.")? else { + return Err("Missing reference."); + }; + let members = doc + .block_records + .get(&reference.block_name) + .ok_or("Missing representation.")? + .entity_handles + .clone(); + for h in members { + let id = doc.get_entity(h).and_then(authoring::element_id); + if let Some(hidden) = id.and_then(|id| flags.get(&id)) { + doc.get_entity_mut(h).unwrap().common_mut().invisible = *hidden; + } + } + Ok(()) +} diff --git a/src/scene/block_actions/tests.rs b/src/scene/block_actions/tests.rs index 708d4d3da..6812fe682 100644 --- a/src/scene/block_actions/tests.rs +++ b/src/scene/block_actions/tests.rs @@ -1079,3 +1079,285 @@ fn block_actions_stretch_duplicate_target_picks_create_one_binding() { let baseline = doc.clone(); assert!(evaluate(&mut doc, &baseline, def, p, DVec3::new(12., 0., 0.)).unwrap()); } + +#[test] +fn block_actions_topology_identity_reorder_add_delete_type_and_native_roundtrip() { + let (mut doc, def, targets) = fixture(); + let p = linear(&mut doc, def); + create(&mut doc, def, Kind::Move, p, &targets[..1], DVec3::ZERO).unwrap(); + let insert = doc + .add_entity(EntityType::Insert(codec::entities::Insert::new( + "ACTIONS", + Vector3::ZERO, + ))) + .unwrap(); + doc = instance::evaluate(&doc, insert, p, DVec3::new(12., 0., 0.)) + .unwrap() + .unwrap(); + let before = doc.clone(); + let EntityType::Insert(i) = doc.get_entity(insert).unwrap() else { + panic!() + }; + let old = doc + .block_records + .get(&i.block_name) + .unwrap() + .entity_handles + .clone(); + let old_id = authoring::element_id(doc.get_entity(targets[1]).unwrap()).unwrap(); + doc.remove_entity(targets[1]); + doc.block_records + .get_mut("ACTIONS") + .unwrap() + .entity_handles + .retain(|h| *h != targets[1]); + let mut line = + codec::entities::Line::from_points(Vector3::new(20., 0., 0.), Vector3::new(21., 0., 0.)); + line.common.owner_handle = def; + let new = doc.add_entity(EntityType::Line(line)).unwrap(); + authoring::tag_members(&mut doc, def); + assert!(authoring::element_id(doc.get_entity(new).unwrap()).unwrap() > old_id); + doc.block_records + .get_mut("ACTIONS") + .unwrap() + .entity_handles + .reverse(); + let doc = instance::refresh_after_edit(&doc, &before, def).unwrap(); + let EntityType::Insert(i) = doc.get_entity(insert).unwrap() else { + panic!() + }; + let members = &doc.block_records.get(&i.block_name).unwrap().entity_handles; + assert_eq!( + members[1], old[0], + "surviving member keeps identity despite reordering" + ); + assert_eq!(start(&doc, members[0]), Vector3::new(20., 0., 0.)); + assert_eq!(start(&doc, members[1]), Vector3::new(3., 0., 0.)); + assert!(doc.get_entity(old[1]).is_none()); + let mut changed = doc.clone(); + let mut circle = codec::entities::Circle::from_center_radius(Vector3::new(6., 6., 0.), 1.); + circle.common = changed.get_entity(new).unwrap().common().clone(); + *changed.get_entity_mut(new).unwrap() = EntityType::Circle(circle); + let changed = instance::refresh_after_edit(&changed, &doc, def).unwrap(); + for saved in [ + DwgReader::from_stream(Cursor::new(DwgWriter::write_to_vec(&changed).unwrap())) + .read() + .unwrap(), + DxfReader::from_reader(Cursor::new( + DxfWriter::new(&changed).write_to_vec().unwrap(), + )) + .unwrap() + .read() + .unwrap(), + ] { + let EntityType::Insert(i) = saved.get_entity(insert).unwrap() else { + panic!() + }; + let members = &saved + .block_records + .get(&i.block_name) + .unwrap() + .entity_handles; + assert!(matches!( + saved.get_entity(members[0]), + Some(EntityType::Circle(_)) + )); + assert_eq!(start(&saved, members[1]), Vector3::new(3., 0., 0.)); + } +} + +#[test] +fn block_actions_topology_copy_tags_and_extension_dictionary_do_not_share() { + use codec::objects::XRecord; + let (mut doc, def, targets) = fixture(); + let p = linear(&mut doc, def); + create(&mut doc, def, Kind::Move, p, &targets[..1], DVec3::ZERO).unwrap(); + let insert = doc + .add_entity(EntityType::Insert(codec::entities::Insert::new( + "ACTIONS", + Vector3::ZERO, + ))) + .unwrap(); + doc = instance::evaluate(&doc, insert, p, DVec3::new(12., 0., 0.)) + .unwrap() + .unwrap(); + let mut duplicate = doc.get_entity(targets[0]).unwrap().clone(); + duplicate.common_mut().handle = Handle::NULL; + let new = doc.add_entity(duplicate).unwrap(); + let original_id = authoring::element_id(doc.get_entity(targets[0]).unwrap()); + doc.block_records + .get_mut("ACTIONS") + .unwrap() + .entity_handles + .rotate_right(1); + authoring::tag_members(&mut doc, def); + assert_eq!( + authoring::element_id(doc.get_entity(targets[0]).unwrap()), + original_id + ); + assert_ne!( + authoring::element_id(doc.get_entity(new).unwrap()), + authoring::element_id(doc.get_entity(targets[0]).unwrap()) + ); + let root = doc.ensure_extension_dictionary(new); + let handle = doc.allocate_handle(); + let mut record = XRecord::default(); + record.handle = handle; + record.owner = root; + doc.objects.insert(handle, ObjectType::XRecord(record)); + let ObjectType::Dictionary(d) = doc.objects.get_mut(&root).unwrap() else { + panic!() + }; + d.add_entry("DATA", handle); + let before = doc.clone(); + let refreshed = instance::refresh_after_edit(&doc, &before, def).unwrap(); + let EntityType::Insert(i) = refreshed.get_entity(insert).unwrap() else { + panic!() + }; + let new_id = authoring::element_id(refreshed.get_entity(new).unwrap()); + let target = *refreshed + .block_records + .get(&i.block_name) + .unwrap() + .entity_handles + .iter() + .find(|h| authoring::element_id(refreshed.get_entity(**h).unwrap()) == new_id) + .unwrap(); + let cloned = refreshed + .get_entity(target) + .unwrap() + .common() + .xdictionary_handle + .unwrap(); + assert_ne!(cloned, root); + let ObjectType::Dictionary(d) = &refreshed.objects[&cloned] else { + panic!() + }; + assert_ne!(d.get("DATA"), Some(handle)); +} + +#[test] +fn block_actions_topology_preserves_visibility_choice_after_discriminator_deleted() { + use crate::scene::block_visibility as visibility; + let (mut doc, def, targets) = fixture(); + let p = linear(&mut doc, def); + create(&mut doc, def, Kind::Move, p, &targets[..1], DVec3::ZERO).unwrap(); + let visibility = visibility::create(&mut doc, def, Vector3::ZERO).unwrap(); + visibility::add_state(&mut doc, visibility, "Alternate", 0).unwrap(); + visibility::set_visible(&mut doc, visibility, 1, &targets[1..], false); + visibility::finish_editing(&mut doc, def); + let first = doc + .add_entity(EntityType::Insert(codec::entities::Insert::new( + "ACTIONS", + Vector3::ZERO, + ))) + .unwrap(); + let second = doc + .add_entity(EntityType::Insert(codec::entities::Insert::new( + "ACTIONS", + Vector3::new(40., 0., 0.), + ))) + .unwrap(); + doc = instance::evaluate(&doc, first, p, DVec3::new(18., 0., 0.)) + .unwrap() + .unwrap(); + doc = instance::evaluate(&doc, second, p, DVec3::new(12., 0., 0.)) + .unwrap() + .unwrap(); + representation::apply_visibility(&mut doc, def, second, 1).unwrap(); + let before = doc.clone(); + doc.remove_entity(targets[1]); + doc.block_records + .get_mut("ACTIONS") + .unwrap() + .entity_handles + .retain(|h| *h != targets[1]); + let mut entity = + codec::entities::Line::from_points(Vector3::new(20., 0., 0.), Vector3::new(21., 0., 0.)); + entity.common.owner_handle = def; + let new = doc.add_entity(EntityType::Line(entity)).unwrap(); + visibility::set_visible(&mut doc, visibility, 0, &[new], false); + visibility::finish_editing(&mut doc, def); + let refreshed = instance::refresh_after_edit(&doc, &before, def).unwrap(); + for (insert, hidden, start_x) in [(first, true, 9.), (second, false, 3.)] { + let EntityType::Insert(i) = refreshed.get_entity(insert).unwrap() else { + panic!() + }; + let members = &refreshed + .block_records + .get(&i.block_name) + .unwrap() + .entity_handles; + assert_eq!(members.len(), 2); + assert_eq!(start(&refreshed, members[0]).x, start_x); + assert_eq!( + refreshed.get_entity(members[1]).unwrap().common().invisible, + hidden + ); + } +} + +#[test] +fn block_actions_topology_nested_insert_attribute_handles_remain_independent() { + let (mut doc, def, targets) = fixture(); + let p = linear(&mut doc, def); + create(&mut doc, def, Kind::Move, p, &targets[..1], DVec3::ZERO).unwrap(); + let insert = doc + .add_entity(EntityType::Insert(codec::entities::Insert::new( + "ACTIONS", + Vector3::ZERO, + ))) + .unwrap(); + doc = instance::evaluate(&doc, insert, p, DVec3::new(12., 0., 0.)) + .unwrap() + .unwrap(); + let mut scene = Scene::new(); + scene.document = doc; + scene.define_block_raw( + "CHILD", + Vector3::ZERO, + vec![EntityType::Line(codec::entities::Line::from_points( + Vector3::ZERO, + Vector3::UNIT_Y, + ))], + ); + let mut child = codec::entities::Insert::new("CHILD", Vector3::new(20., 0., 0.)); + child.common.owner_handle = def; + let mut attribute = codec::entities::AttributeEntity::new("TAG".into(), "value".into()); + attribute.common.handle = scene.document.allocate_handle(); + child.attributes.push(attribute); + child.seqend_handle = Some(scene.document.allocate_handle()); + let source = scene + .document + .add_entity(EntityType::Insert(child)) + .unwrap(); + let before = scene.document.clone(); + let doc = instance::refresh_after_edit(&before, &before, def).unwrap(); + let doc = instance::refresh_after_edit(&doc, &doc, def).unwrap(); + let reopened = DwgReader::from_stream(Cursor::new(DwgWriter::write_to_vec(&doc).unwrap())) + .read() + .unwrap(); + let EntityType::Insert(parent) = reopened.get_entity(insert).unwrap() else { + panic!() + }; + let member = *reopened + .block_records + .get(&parent.block_name) + .unwrap() + .entity_handles + .last() + .unwrap(); + let EntityType::Insert(copied) = reopened.get_entity(member).unwrap() else { + panic!() + }; + let EntityType::Insert(original) = reopened.get_entity(source).unwrap() else { + panic!() + }; + assert_eq!(copied.attributes[0].value, "value"); + assert_ne!( + copied.attributes[0].common.handle, + original.attributes[0].common.handle + ); + assert_eq!(copied.attributes[0].common.owner_handle, member); + assert_ne!(copied.seqend_handle, original.seqend_handle); +} diff --git a/src/scene/block_authoring.rs b/src/scene/block_authoring.rs index 6713d2ca0..3abf4837f 100644 --- a/src/scene/block_authoring.rs +++ b/src/scene/block_authoring.rs @@ -441,7 +441,22 @@ pub(super) fn set_entity_tag( pub(crate) fn retag_members(document: &mut CadDocument, definition: Handle) { for handle in members(document, definition) { if let Some(id) = document.get_entity(handle).and_then(element_id) { - set_entity_tag(document, handle, element_tag(id, handle)); + let tagged = document.get_entity(handle).is_some_and(|entity| { + element_id(entity) == Some(id) + && entity + .common() + .extended_data + .get_record(ELEMENT_TAG) + .is_some_and(|record| { + record + .values + .iter() + .any(|v| matches!(v,XDataValue::Handle(h) if *h==handle)) + }) + }); + if !tagged { + set_entity_tag(document, handle, element_tag(id, handle)); + } } } } @@ -450,25 +465,121 @@ pub(crate) fn retag_members(document: &mut CadDocument, definition: Handle) { /// ones after the highest id in use, and record the next id on the block. pub(super) fn tag_members(document: &mut CadDocument, definition: Handle) { let members = members(document, definition); + let stored_next = document + .object_xdata(definition) + .and_then(|data| data.get_record(ELEMENT_TAG)) + .and_then(|record| { + record.values.iter().find_map(|v| match v { + XDataValue::Integer32(n) => Some(*n), + _ => None, + }) + }) + .unwrap_or(0); let mut next = members .iter() .filter_map(|handle| document.get_entity(*handle).and_then(element_id)) .max() - .map_or(0, |id| id + 1); + .map_or(0, |id| id.saturating_add(1)) + .max(stored_next); + let mut checked = std::collections::BTreeSet::new(); + let needs_ids = members.iter().any(|h| { + document.get_entity(*h).is_none_or(|e| { + element_id(e).is_none_or(|id| { + !checked.insert(id) + || !e + .common() + .extended_data + .get_record(ELEMENT_TAG) + .is_some_and(|r| { + r.values + .iter() + .any(|v| matches!(v,XDataValue::Handle(link) if link==h)) + }) + }) + }) + }); + if needs_ids { + // Undo can restore an older definition counter while evaluated copies + // still contain newer IDs. Never recycle any of those live identities. + let representations = document + .block_records + .iter() + .filter(|b| { + document + .object_xdata(b.handle) + .and_then(|d| d.get_record(REPRESENTATION_TAG)) + .is_some_and(|r| { + r.values + .iter() + .any(|v| matches!(v,XDataValue::Handle(h) if *h==definition)) + }) + }) + .map(|b| b.handle) + .collect::>(); + if let Some(high) = document + .entities() + .filter(|e| representations.contains(&e.common().owner_handle)) + .filter_map(element_id) + .max() + { + next = next.max(high.saturating_add(1)); + } + } + let mut seen = std::collections::BTreeSet::new(); for handle in members { let id = match document.get_entity(handle).and_then(element_id) { - Some(id) => id, - None => { - next += 1; - next - 1 + Some(id) + if id >= 0 + && document + .get_entity(handle) + .unwrap() + .common() + .extended_data + .get_record(ELEMENT_TAG) + .is_some_and(|r| { + r.values + .iter() + .any(|v| matches!(v,XDataValue::Handle(h) if *h==handle)) + }) + && seen.insert(id) => + { + id + } + _ => { + let id = next; + next = next.saturating_add(1); + seen.insert(id); + id } }; - set_entity_tag(document, handle, element_tag(id, handle)); + let tagged = document.get_entity(handle).is_some_and(|entity| { + element_id(entity) == Some(id) + && entity + .common() + .extended_data + .get_record(ELEMENT_TAG) + .is_some_and(|record| { + record + .values + .iter() + .any(|v| matches!(v,XDataValue::Handle(h) if *h==handle)) + }) + }); + if !tagged { + set_entity_tag(document, handle, element_tag(id, handle)); + } } let mut tag = ExtendedDataRecord::new(ELEMENT_TAG); tag.add_value(XDataValue::Integer16(1)); tag.add_value(XDataValue::Integer32(next)); - document.set_object_xdata_record(definition, tag); + if stored_next != next + || document + .object_xdata(definition) + .and_then(|d| d.get_record(ELEMENT_TAG)) + .is_none() + { + document.set_object_xdata_record(definition, tag); + } } pub(super) fn new_guid() -> String {