From 2d1ee0d9367abed9d9db6d64c53b59e95d3617dd Mon Sep 17 00:00:00 2001 From: gianlucafiore Date: Thu, 13 Aug 2026 11:58:54 -0300 Subject: [PATCH] fix: improve classic leader behavior and annotation linkage --- src/app/command_driver.rs | 64 +++++++++++-- src/entities/leader.rs | 56 ++++++++++- src/modules/annotate/leader_cmd.rs | 148 +++++++++++++++++++---------- src/modules/draw/mod.rs | 12 +-- src/scene/annotative.rs | 10 +- src/scene/modify.rs | 144 +++++++++++++++++++++++++++- src/scene/selection.rs | 57 ++++++++++- 7 files changed, 418 insertions(+), 73 deletions(-) diff --git a/src/app/command_driver.rs b/src/app/command_driver.rs index 4491f33d..8cc2e744 100644 --- a/src/app/command_driver.rs +++ b/src/app/command_driver.rs @@ -1176,10 +1176,27 @@ impl OpenCADStudio { // Link the leader to its annotation so the pair edits as a unit // (double-click on the leader resolves to the text entity). if let (Some(lh), Some(ah)) = (leader_handle, edit_handle) { - if let Some(acadrust::EntityType::Leader(l)) = + let linked = if let Some(acadrust::EntityType::Leader(l)) = self.tabs[i].scene.document.get_entity_mut(lh) { l.annotation_handle = ah; + true + } else { + false + }; + + if linked { + // The LEADER may already have received its annotation context while + // annotation_handle was still NULL. Refresh it now that the MTEXT link + // is known so the context represents the finished leader. + self.tabs[i] + .scene + .sync_displayed_annotation_context(lh); + + self.tabs[i].scene.bump_entities(&[( + lh, + crate::scene::ChangeKind::Modified, + )]); } } self.tabs[i].dirty = true; @@ -3367,6 +3384,45 @@ impl OpenCADStudio { _ => glam::DVec3::ZERO, }; self.merge_clipboard_ext_objects(i, &by_index, annotation_delta); + // Source handles stored in the clipboard map one-to-one to the freshly + // pasted handles. Use that map to reconnect LEADER -> copied annotation. + let mut handle_map = rustc_hash::FxHashMap::default(); + + for (source, &copied) in self.clipboard.iter().zip(by_index.iter()) { + if !copied.is_null() { + handle_map.insert(source.common().handle, copied); + } + } + + let leader_links: Vec<(Handle, Handle)> = self + .clipboard + .iter() + .filter_map(|source| { + let acadrust::EntityType::Leader(leader) = source else { + return None; + }; + + let copied_leader = handle_map.get(&source.common().handle).copied()?; + let copied_annotation = handle_map + .get(&leader.annotation_handle) + .copied() + .unwrap_or(Handle::NULL); + + Some((copied_leader, copied_annotation)) + }) + .collect(); + + for (leader_handle, annotation_handle) in leader_links { + if let Some(acadrust::EntityType::Leader(leader)) = + self.tabs[i].scene.document.get_entity_mut(leader_handle) + { + leader.annotation_handle = annotation_handle; + } + + let _ = self.tabs[i] + .scene + .sync_displayed_annotation_context(leader_handle); + } // Recreate any group whose whole membership was copied, so a pasted // group stays grouped — cross-drawing too, since the groups were // snapshotted into the clipboard at copy time. `by_index` is aligned @@ -3374,12 +3430,6 @@ impl OpenCADStudio { // its clipboard clone to its new handle. Same shared `recreate_groups` // the in-drawing COPY path uses. (#440) if !self.clipboard_deps.groups.is_empty() { - let mut handle_map = rustc_hash::FxHashMap::default(); - for (src, &new) in self.clipboard.iter().zip(by_index.iter()) { - if !new.is_null() { - handle_map.insert(src.common().handle, new); - } - } let groups = self.clipboard_deps.groups.clone(); self.tabs[i].scene.recreate_groups(groups, &handle_map); } diff --git a/src/entities/leader.rs b/src/entities/leader.rs index 7d989e53..f42f8abc 100644 --- a/src/entities/leader.rs +++ b/src/entities/leader.rs @@ -159,7 +159,59 @@ fn grips(leader: &Leader) -> Vec { fn apply_grip(leader: &mut Leader, grip_id: usize, apply: GripApply) { let n = leader.vertices.len(); + if grip_id < n { + if n >= 3 && leader.creation_type == LeaderCreationType::WithText { + // Grip del codo: mueve el codo libremente, pero arrastra también + // el extremo del renglón manteniendo la distancia relativa. + if grip_id == n - 2 { + let old_elbow = leader.vertices[n - 2]; + let old_end = leader.vertices[n - 1]; + + let delta = match apply { + GripApply::Absolute(p) => acadrust::types::Vector3::new( + p.x as f64 - old_elbow.x, + p.y as f64 - old_elbow.y, + p.z as f64 - old_elbow.z, + ), + GripApply::Translate(d) => acadrust::types::Vector3::new( + d.x as f64, + d.y as f64, + d.z as f64, + ), + }; + + leader.vertices[n - 2].x = old_elbow.x + delta.x; + leader.vertices[n - 2].y = old_elbow.y + delta.y; + leader.vertices[n - 2].z = old_elbow.z + delta.z; + + leader.vertices[n - 1].x = old_end.x + delta.x; + leader.vertices[n - 1].y = old_end.y + delta.y; + leader.vertices[n - 1].z = old_end.z + delta.z; + + return; + } + + // Grip del extremo horizontal: sólo debe estirar en X; + // Y/Z quedan pegados al codo para que siga horizontal. + if grip_id == n - 1 { + let elbow = leader.vertices[n - 2]; + + match apply { + GripApply::Absolute(p) => { + leader.vertices[n - 1].x = p.x as f64; + } + GripApply::Translate(d) => { + leader.vertices[n - 1].x += d.x as f64; + } + } + + leader.vertices[n - 1].y = elbow.y; + leader.vertices[n - 1].z = elbow.z; + return; + } + } + if let Some(v) = leader.vertices.get_mut(grip_id) { match apply { GripApply::Absolute(p) => { @@ -176,7 +228,9 @@ fn apply_grip(leader: &mut Leader, grip_id: usize, apply: GripApply) { } } else if let GripApply::Translate(d) = apply { leader.translate(acadrust::types::Vector3::new( - d.x as f64, d.y as f64, d.z as f64, + d.x as f64, + d.y as f64, + d.z as f64, )); } } diff --git a/src/modules/annotate/leader_cmd.rs b/src/modules/annotate/leader_cmd.rs index 1f67116f..48b6d4ad 100644 --- a/src/modules/annotate/leader_cmd.rs +++ b/src/modules/annotate/leader_cmd.rs @@ -53,6 +53,7 @@ impl LeaderCommand { } else { defaults.scale }; + Self { verts: Vec::new(), plane: WorkingPlane::default(), @@ -65,6 +66,78 @@ impl LeaderCommand { annotative: defaults.annotative, } } + + fn finish(&self) -> CmdResult { + if self.verts.len() < 2 { + return CmdResult::Cancel; + } + + let local: Vec = self + .verts + .iter() + .map(|point| self.plane.to_local(*point)) + .collect(); + + let displayed_height = self.text_height * self.display_scale; + + let displayed_landing = if self.arrow_size > 1.0e-9 { + self.arrow_size * self.display_scale + } else { + displayed_height * 1.5 + }; + + // The user supplies only the arrow point and elbow. The horizontal + // landing is stored as a real third LEADER vertex so it can have its own grip. + let mut leader_points = local.clone(); + + let first = local[0]; + let elbow = local[1]; + let sign = if elbow.x >= first.x { 1.0 } else { -1.0 }; + + let landing_end = DVec3::new( + elbow.x + sign * displayed_landing, + elbow.y, + elbow.z, + ); + + leader_points.push(landing_end); + + let leader = build_leader( + &leader_points, + Mat4::IDENTITY, + &self.dimension_style, + self.text_height, + self.gap, + self.arrow_size, + ); + + // The MTEXT starts at the real end of the landing. + let (anchor, attach) = + annotation_anchor(&leader_points, 0.0, Mat4::IDENTITY); + + // Store the MTEXT at its native model-space size for the current + // annotation scale. Its annotation contexts then scale it relatively + // when another representation becomes active. + let mtext_height = displayed_height; + + let mtext = build_mtext( + "", + anchor, + mtext_height, + attach, + Mat4::IDENTITY, + &self.text_style, + self.annotative, + ); + + CmdResult::CommitManyAndEditText { + entities: vec![ + self.plane.place_entity(EntityType::Leader(leader)), + self.plane.place_entity(EntityType::MText(mtext)), + ], + edit_index: 1, + } + } } impl CadCommand for LeaderCommand { @@ -80,61 +153,22 @@ impl CadCommand for LeaderCommand { if self.verts.is_empty() { t!("LEADER Specify arrowhead point:").into_owned() } else { - t!( - "LEADER Specify next point [%{count} pts — Enter to place text]:", - count = self.verts.len() - ) - .into_owned() + t!("LEADER Specify landing point:").into_owned() } } fn on_point(&mut self, pt: DVec3) -> CmdResult { self.verts.push(pt); - CmdResult::NeedPoint + + if self.verts.len() >= 2 { + self.finish() + } else { + CmdResult::NeedPoint + } } fn on_enter(&mut self) -> CmdResult { - if self.verts.len() < 2 { - return CmdResult::Cancel; - } - // Place the leader plus an empty MText annotation, link them, then open - // the in-place MText editor so the user types the annotation text. - let local: Vec = self - .verts - .iter() - .map(|point| self.plane.to_local(*point)) - .collect(); - let leader = build_leader( - &local, - Mat4::IDENTITY, - &self.dimension_style, - self.text_height, - self.gap, - self.arrow_size, - ); - let displayed_height = self.text_height * self.display_scale; - let (anchor, attach) = annotation_anchor(&local, displayed_height, Mat4::IDENTITY); - let mtext_height = if self.annotative { - self.text_height - } else { - displayed_height - }; - let mtext = build_mtext( - "", - anchor, - mtext_height, - attach, - Mat4::IDENTITY, - &self.text_style, - self.annotative, - ); - CmdResult::CommitManyAndEditText { - entities: vec![ - self.plane.place_entity(EntityType::Leader(leader)), - self.plane.place_entity(EntityType::MText(mtext)), - ], - edit_index: 1, - } + self.finish() } fn on_escape(&mut self) -> CmdResult { @@ -189,7 +223,10 @@ fn build_leader( ) -> Leader { let mut l = Leader::from_vertices(verts.iter().map(|p| dv3(*p)).collect()); l.creation_type = LeaderCreationType::WithText; - l.hookline_enabled = true; + + // New leaders store the landing as their final real vertex, so the renderer + // must not append a second synthetic hookline. + l.hookline_enabled = false; l.dimension_style = dimension_style.to_string(); l.text_height = text_height; l.dimension_gap = gap; @@ -206,21 +243,28 @@ fn build_leader( /// left-pointing landing, to the left of a right-pointing one). fn annotation_anchor( verts: &[DVec3], - text_height: f64, + landing_length: f64, ucs: Mat4, ) -> (DVec3, AttachmentPoint) { let last = *verts.last().unwrap(); let prev = verts[verts.len() - 2]; - // Side + landing run along the UCS X axis (identity = world). - let ux = ucs.transform_vector3(Vec3::X).normalize_or(Vec3::X).as_dvec3(); + + let ux = ucs + .transform_vector3(Vec3::X) + .normalize_or(Vec3::X) + .as_dvec3(); + let to_right = (last - prev).dot(ux) >= 0.0; let sign = if to_right { 1.0_f64 } else { -1.0_f64 }; - let anchor = last + ux * (sign * text_height * 1.5); + + let anchor = last + ux * (sign * landing_length); + let attach = if to_right { AttachmentPoint::MiddleLeft } else { AttachmentPoint::MiddleRight }; + (anchor, attach) } diff --git a/src/modules/draw/mod.rs b/src/modules/draw/mod.rs index 1f265e43..9b860ea1 100644 --- a/src/modules/draw/mod.rs +++ b/src/modules/draw/mod.rs @@ -160,18 +160,18 @@ impl CadModule for DrawModule { label: "Leader", icon: leader_cmd::ICON, items: vec![ - ( - mleader_cmd::tool().id, - mleader_cmd::tool().label, - mleader_cmd::tool().icon, - ), ( leader_cmd::tool().id, leader_cmd::tool().label, leader_cmd::tool().icon, ), + ( + mleader_cmd::tool().id, + mleader_cmd::tool().label, + mleader_cmd::tool().icon, + ), ], - default: "MLEADER", + default: "LEADER", }, ], }, diff --git a/src/scene/annotative.rs b/src/scene/annotative.rs index 2badd3a7..bef6d168 100644 --- a/src/scene/annotative.rs +++ b/src/scene/annotative.rs @@ -480,7 +480,15 @@ pub fn annotative_offscale_for( }), } } - +pub(crate) fn annotation_scale_handles_for_entity( + doc: &CadDocument, + entity_handle: Handle, +) -> Vec { + object_scale_memberships(doc, entity_handle) + .into_iter() + .map(|(_, scale_handle)| scale_handle) + .collect() +} pub fn scale_handle_by_name(doc: &CadDocument, name: &str) -> Option { doc.objects.iter().find_map(|(handle, object)| match object { ObjectType::Scale(scale) diff --git a/src/scene/modify.rs b/src/scene/modify.rs index ae167651..c215ef12 100644 --- a/src/scene/modify.rs +++ b/src/scene/modify.rs @@ -551,11 +551,43 @@ impl Scene { } pub fn copy_entities(&mut self, handles: &[Handle], t: &EntityTransform) -> Vec { + let copy_handles = self.handles_expanded_for_leader_annotations(handles); + + // LEADER + attached MTEXT are a logical pair. Their entity clones must not + // retain the source extension dictionary, otherwise both copies share the + // same annotation-context objects. + let leader_pair_handles: Vec = copy_handles + .iter() + .flat_map(|&handle| { + let annotation = match self.document.get_entity(handle) { + Some(EntityType::Leader(leader)) if !leader.annotation_handle.is_null() => { + Some(leader.annotation_handle) + } + _ => None, + }; + + std::iter::once(handle).chain(annotation) + }) + .collect(); + // Objects on a locked layer can be selected but not copied. - let clones: Vec<(Handle, EntityType)> = handles + let clones: Vec<(Handle, EntityType, Vec)> = copy_handles .iter() .filter(|&&h| !self.is_layer_locked(h)) - .filter_map(|&h| self.document.get_entity(h).cloned().map(|e| (h, e))) + .filter_map(|&h| { + let entity = self.document.get_entity(h)?.clone(); + + let annotation_scales = if leader_pair_handles.contains(&h) { + crate::scene::annotative::annotation_scale_handles_for_entity( + &self.document, + h, + ) + } else { + Vec::new() + }; + + Some((h, entity, annotation_scales)) + }) .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 @@ -567,7 +599,7 @@ impl Scene { let mut new_handles = Vec::with_capacity(clones.len()); let mut handle_map = rustc_hash::FxHashMap::default(); let mut refresh_solid_handles = Vec::new(); - for (src_handle, mut entity) in clones { + for (src_handle, mut entity, annotation_scales) in clones { let text_orient = if preserve_text_orientation { capture_text_orient(&entity) } else { @@ -598,9 +630,31 @@ impl Scene { } } Self::reset_clone_subhandles(&mut self.document, &mut entity); + + // An annotative LEADER/MTEXT pair must receive a fresh extension dictionary. + // Keeping this handle would make the copy share the source's context tree. + if !annotation_scales.is_empty() { + entity.common_mut().xdictionary_handle = None; + } + entity.common_mut().handle = Handle::NULL; let h = self.document.add_entity(entity).unwrap_or(Handle::NULL); if !h.is_null() { + if !annotation_scales.is_empty() { + for scale_handle in annotation_scales { + crate::scene::annotative::create_annotation_context( + &mut self.document, + h, + scale_handle, + ); + } + + // Annotation contexts add dictionary/object records outside the entity + // delta itself, so keep undo on the safe full-snapshot path. + if self.is_recording_undo() { + self.poison_undo_recording(); + } + } // Delta-undo: a copy's before-image is "nothing" (undo erases it). if self.is_recording_undo() { self.record_undo_before(h, None); @@ -640,7 +694,35 @@ impl Scene { handle_map.insert(src_handle, h); } } + // A copied LEADER must reference the copied annotation, never the + // source annotation. Both entities now exist, so remap the stored handle. + let leader_links: Vec<(Handle, Handle)> = handle_map + .iter() + .filter_map(|(&source_handle, &copied_handle)| { + let EntityType::Leader(source_leader) = + self.document.get_entity(source_handle)? + else { + return None; + }; + let copied_annotation = handle_map + .get(&source_leader.annotation_handle) + .copied() + .unwrap_or(Handle::NULL); + + Some((copied_handle, copied_annotation)) + }) + .collect(); + + for (leader_handle, annotation_handle) in leader_links { + if let Some(EntityType::Leader(leader)) = + self.document.get_entity_mut(leader_handle) + { + leader.annotation_handle = annotation_handle; + } + + let _ = self.sync_displayed_annotation_context(leader_handle); + } // Complete group copies record their new Group objects and dictionary // entry as targeted object deltas inside copy_complete_groups. self.copy_complete_groups(&handle_map); @@ -956,10 +1038,66 @@ impl Scene { .get_entity(handle) .and_then(crate::entities::solid3d::point_of_reference) .map(|p| [p.x, p.y, p.z]); + // A LEADER's final vertex is the end of its horizontal landing. + // Remember its old position and linked MTEXT so the annotation can follow + // when that grip stretches the landing. + let leader_landing_before = self.document.get_entity(handle).and_then(|entity| { + let EntityType::Leader(leader) = entity else { + return None; + }; + let n = leader.vertices.len(); + if n < 3 || (grip_id != n - 1 && grip_id != n - 2) || leader.annotation_handle.is_null() { + return None; + } + + let point = leader.vertices.last()?; + + Some(( + leader.annotation_handle, + glam::DVec3::new(point.x, point.y, point.z), + )) + }); if let Some(entity) = self.document.get_entity_mut(handle) { view::dispatch::apply_grip(entity, grip_id, apply); } + if let Some((annotation_handle, old_landing)) = leader_landing_before { + let new_landing = self.document.get_entity(handle).and_then(|entity| { + let EntityType::Leader(leader) = entity else { + return None; + }; + + let point = leader.vertices.last()?; + Some(glam::DVec3::new(point.x, point.y, point.z)) + }); + + if let Some(new_landing) = new_landing { + let delta = new_landing - old_landing; + + if delta.length_squared() > 1.0e-20 { + if self.is_recording_undo() { + if let Some(before) = self.document.get_entity_arc(annotation_handle) { + self.record_undo_before(annotation_handle, Some(before)); + } + } + + if let Some(annotation) = self.document.get_entity_mut(annotation_handle) { + view::dispatch::apply_transform( + annotation, + &crate::command::EntityTransform::Translate(delta), + ); + } + + if self.sync_displayed_annotation_context(annotation_handle) { + self.poison_undo_recording(); + } + self.bump_entities(&[( + annotation_handle, + crate::scene::ChangeKind::Modified, + )]); + } + } + } if self.sync_displayed_annotation_context(handle) { self.poison_undo_recording(); } diff --git a/src/scene/selection.rs b/src/scene/selection.rs index 80ac65af..b073a195 100644 --- a/src/scene/selection.rs +++ b/src/scene/selection.rs @@ -3,12 +3,46 @@ use super::*; impl Scene { // ── Selection ───────────────────────────────────────────────────────── + /// Treat a classic LEADER and its attached annotation as one logical object. + /// Clicking/copying/deleting either side expands to the complete pair. + pub(crate) fn handles_expanded_for_leader_annotations( + &self, + handles: &[Handle], + ) -> Vec { + let mut expanded = handles.to_vec(); + for &handle in handles { + // LEADER -> annotation. + if let Some(EntityType::Leader(leader)) = self.document.get_entity(handle) { + if !leader.annotation_handle.is_null() { + expanded.push(leader.annotation_handle); + } + } + + // Annotation -> LEADER. + expanded.extend(self.document.entities().filter_map(|entity| match entity { + EntityType::Leader(leader) + if !leader.annotation_handle.is_null() + && leader.annotation_handle == handle => + { + Some(entity.common().handle) + } + _ => None, + })); + } + + expanded.sort_unstable_by_key(|handle| handle.value()); + expanded.dedup(); + expanded + } pub fn select_entity(&mut self, handle: Handle, exclusive: bool) { + let handles = self.handles_expanded_for_leader_annotations(&[handle]); + if exclusive { self.selected.clear(); } - self.selected.insert(handle); + + self.selected.extend(handles); self.bump_selection(); } @@ -21,6 +55,12 @@ impl Scene { /// only when its contents actually changed. History/file/command paths must /// use this instead of assigning `selected` directly. pub(crate) fn replace_selection(&mut self, selected: HashSet) { + let handles: Vec = selected.iter().copied().collect(); + let selected: HashSet = self + .handles_expanded_for_leader_annotations(&handles) + .into_iter() + .collect(); + if self.selected != selected { self.selected = selected; self.bump_selection(); @@ -29,7 +69,14 @@ impl Scene { /// Remove a single entity from the selection (Shift+click subtractive pick). pub fn deselect_entity(&mut self, handle: Handle) { - if self.selected.remove(&handle) { + let handles = self.handles_expanded_for_leader_annotations(&[handle]); + let mut changed = false; + + for handle in handles { + changed |= self.selected.remove(&handle); + } + + if changed { self.bump_selection(); } } @@ -608,10 +655,14 @@ impl Scene { // ── Erase ───────────────────────────────────────────────────────────── pub fn erase_entities(&mut self, handles: &[Handle]) { + + let erase_handles = self.handles_expanded_for_leader_annotations(handles); + let mut handle_set: HashSet = HashSet::default(); let mut erased: Vec<(Handle, ChangeKind)> = Vec::new(); let mut highlight_changed = false; - for &h in handles { + + for &h in &erase_handles { // Objects on a locked layer can't be erased. if self.is_layer_locked(h) { continue;