From c5b1269caeca90b6ecd99dfd5d0a0f3b2f92b7e3 Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Sun, 11 Oct 2026 23:15:18 +0300 Subject: [PATCH] fix(dimension): keep picture placement consistent with world-space edits #1681 places a dimension's picture block by its insertion point, rotation and scale. Edits (move/rotate/scale/mirror, copy, paste) transform the picture's sub-entities in world space, and the codec's dimension transform moves the insertion point too, so a rotated or copied dimension would get the placement applied twice. Fold the placement into the subs before the edit and leave the dimension at the identity placement afterwards. The picture insert no longer takes the dimension's extrusion: OCS bakes picture sub-entities in world coordinates, so only the stored insertion point goes through the OCS. A zero insertion-scale component counts as 1. Co-Authored-By: Claude Opus 5.5 --- src/app/command_driver/clipboard.rs | 8 +++ src/scene/modify.rs | 39 +++++++++++--- src/scene/render_graph.rs | 84 +++++++++++++++++++++++------ 3 files changed, 106 insertions(+), 25 deletions(-) diff --git a/src/app/command_driver/clipboard.rs b/src/app/command_driver/clipboard.rs index a794d200d..4afccaa48 100644 --- a/src/app/command_driver/clipboard.rs +++ b/src/app/command_driver/clipboard.rs @@ -512,9 +512,11 @@ pub(crate) fn paste_entities_kernel( .iter() .cloned() .map(|mut entity| { + let picture_placement = crate::scene::render_graph::dim_picture_placement(&entity); if let Some(t) = translate { crate::scene::view::dispatch::apply_transform(&mut entity, t); } + crate::scene::render_graph::clear_dim_picture_placement(&mut entity); // Same `*D`-block re-point as `finalize_paste` (#290, #161): the // baked dimension geometry lives in WCS, so the paste gets its own // transformed copy of the snapshotted block. @@ -527,6 +529,12 @@ pub(crate) fn paste_entities_kernel( .find(|b| b.name.eq_ignore_ascii_case(&bn)) .map(|def| def.entities.clone()) { + let mut subs = subs; + if let Some(placement) = &picture_placement { + for sub in &mut subs { + crate::scene::view::dispatch::apply_transform(sub, placement); + } + } let bt = translate.cloned().unwrap_or( crate::command::EntityTransform::Translate(glam::DVec3::ZERO), ); diff --git a/src/scene/modify.rs b/src/scene/modify.rs index 41475e3f0..66267234f 100644 --- a/src/scene/modify.rs +++ b/src/scene/modify.rs @@ -263,25 +263,30 @@ impl Scene { // block, and the render draws those sub-entities directly (not the // definition points). Transform them with the dimension, or it would // stay drawn in place while only its def points move. - let dim_block_subs: Vec = handles + let dim_block_subs: Vec<(Handle, Option)> = handles .iter() .filter_map(|&h| match self.document.get_entity(h) { - Some(EntityType::Dimension(d)) => { + Some(entity @ EntityType::Dimension(d)) => { let bn = d.base().block_name.clone(); if bn.trim().is_empty() { None } else { - Some(bn) + Some((bn, render_graph::dim_picture_placement(entity))) } } _ => None, }) - .filter_map(|bn| { + .filter_map(|(bn, placement)| { self.document .block_records .iter() .find(|br| br.name.eq_ignore_ascii_case(&bn)) - .map(|br| br.entity_handles.clone()) + .map(|br| { + br.entity_handles + .iter() + .map(|&sh| (sh, placement.clone())) + .collect::>() + }) }) .flatten() .collect(); @@ -294,6 +299,7 @@ impl Scene { } if let Some(entity) = self.document.get_entity_mut(h) { view::dispatch::apply_transform(entity, t); + render_graph::clear_dim_picture_placement(entity); if mirror_true { mirror_true_text_flags(entity); } @@ -359,12 +365,15 @@ impl Scene { // Move the baked dimension-block sub-entities too (collected above). // These are entities not named in the reported change set, so a delta // must capture their before-images here or a dimension move won't undo. - for h in &dim_block_subs { + for (h, placement) in &dim_block_subs { if self.is_recording_undo() { let before = self.document.get_entity_arc(*h); self.record_undo_before(*h, before); } if let Some(entity) = self.document.get_entity_mut(*h) { + if let Some(placement) = placement { + view::dispatch::apply_transform(entity, placement); + } transform_dimension_part(entity, t); } } @@ -588,7 +597,12 @@ impl Scene { /// transformed block — otherwise it still references the source block and /// renders on top of the original instead of at the copy. Returns None when /// the source block is missing or empty. (#161) - fn clone_transformed_block(&mut self, src_name: &str, t: &EntityTransform) -> Option { + fn clone_transformed_block( + &mut self, + src_name: &str, + placement: Option<&EntityTransform>, + t: &EntityTransform, + ) -> Option { let sub_handles = self .document .block_records @@ -598,6 +612,12 @@ impl Scene { let subs: Vec = sub_handles .iter() .filter_map(|&sh| self.document.get_entity(sh).cloned()) + .map(|mut sub| { + if let Some(placement) = placement { + view::dispatch::apply_transform(&mut sub, placement); + } + sub + }) .collect(); self.define_transformed_block(&subs, t) } @@ -694,7 +714,9 @@ impl Scene { } else { None }; + let picture_placement = render_graph::dim_picture_placement(&entity); view::dispatch::apply_transform(&mut entity, t); + render_graph::clear_dim_picture_placement(&mut entity); if let Some(o) = &text_orient { restore_text_orient(&mut entity, o); } @@ -707,7 +729,8 @@ impl Scene { if let EntityType::Dimension(d) = &entity { let bn = d.base().block_name.clone(); if !bn.trim().is_empty() { - if let Some(new_bn) = self.clone_transformed_block(&bn, t) { + let placement = picture_placement.as_ref(); + if let Some(new_bn) = self.clone_transformed_block(&bn, placement, t) { // Delta-undo: a copied dimension adds a fresh anonymous // *D block record — non-entity state a pure-entity delta // can't restore. Poison so the app keeps a full snapshot. diff --git a/src/scene/render_graph.rs b/src/scene/render_graph.rs index e5b3595ae..9a42a1ed9 100644 --- a/src/scene/render_graph.rs +++ b/src/scene/render_graph.rs @@ -621,6 +621,72 @@ pub fn block_base_point(document: &CadDocument, block_name: &str) -> Vector3 { .unwrap_or(Vector3::ZERO) } +/// The block reference that places a dimension's picture block: at the +/// dimension's insertion point, with its insertion rotation and scale. The +/// point is stored in the dimension's OCS; the picture's sub-entities are in +/// world coordinates, so only the point goes through the OCS. A zero scale +/// component (an unread field) counts as 1 rather than collapsing the picture. +pub(crate) fn dimension_picture_insert( + base: &codec::entities::dimension::DimensionBase, + block_name: String, +) -> Insert { + let scale = |s: f64| if s.abs() < 1e-12 { 1.0 } else { s }; + let point = Matrix3::arbitrary_axis(base.normal) * base.insertion_point; + Insert::new(block_name, point) + .with_scale( + scale(base.insertion_scale.x), + scale(base.insertion_scale.y), + scale(base.insertion_scale.z), + ) + .with_rotation(base.insertion_rotation) +} + +/// World transform of a dimension's picture placement, or None when it is +/// the identity (the usual case: OCS bakes its pictures in place). +pub(crate) fn dimension_picture_placement( + base: &codec::entities::dimension::DimensionBase, +) -> Option { + let insert = dimension_picture_insert(base, String::new()); + let p = insert.insert_point; + let identity = p.x.abs() < 1e-9 + && p.y.abs() < 1e-9 + && p.z.abs() < 1e-9 + && insert.rotation.abs() < 1e-12 + && (insert.x_scale() - 1.0).abs() < 1e-12 + && (insert.y_scale() - 1.0).abs() < 1e-12 + && (insert.z_scale() - 1.0).abs() < 1e-12; + (!identity).then(|| insert.get_transform()) +} + +/// A dimension's picture placement as a world-space edit, or None when it is +/// the identity. Edits move the picture's sub-entities in world space, so a +/// placed picture has its placement folded into the subs and the dimension's +/// own placement cleared (`clear_dim_picture_placement`); a placement left on +/// the dimension would be applied again on top of the moved subs. +pub(crate) fn dim_picture_placement(entity: &EntityType) -> Option { + let EntityType::Dimension(d) = entity else { + return None; + }; + if d.base().block_name.trim().is_empty() { + return None; + } + dimension_picture_placement(d.base()).map(crate::command::EntityTransform::Affine) +} + +/// Reset a dimension's picture placement to the identity once its subs carry +/// it. Also undoes the transform moving the placement point along with the +/// dimension, which would offset a picture baked in place. +pub(crate) fn clear_dim_picture_placement(entity: &mut EntityType) { + if let EntityType::Dimension(d) = entity { + let base = d.base_mut(); + if !base.block_name.trim().is_empty() { + base.insertion_point = Vector3::ZERO; + base.insertion_rotation = 0.0; + base.insertion_scale = Vector3::new(1.0, 1.0, 1.0); + } + } +} + pub fn insert_transform(document: &CadDocument, insert: &Insert) -> Transform { let base = block_base_point(document, &insert.block_name); Transform::from_translation(Vector3::new(-base.x, -base.y, -base.z)) @@ -790,23 +856,7 @@ fn entity_owned_block_uses( .filter(|record| !record.entity_handles.is_empty()) { let base = dimension.base(); - // The picture block draws the dimension in its own frame, and - // the dimension says where that frame sits. Placing it at the - // origin instead draws the whole dimension offset by the - // insertion point — far off the geometry it measures whenever - // the file carries a non-zero one, which is what a dimension - // saved inside a block definition usually has. - let mut insert = Insert::new(record.name.clone(), base.insertion_point) - .with_scale( - base.insertion_scale.x, - base.insertion_scale.y, - base.insertion_scale.z, - ) - .with_rotation(base.insertion_rotation); - // The insertion point is in the dimension's own object - // coordinates, so the picture follows its extrusion the way - // any block reference does. - insert.normal = base.normal; + let mut insert = dimension_picture_insert(base, record.name.clone()); insert.common = base.common.clone(); let picture = !crate::scene::annotative::is_annotative(document, entity); vec![block_use(