From cc7894cd925d80eeca07d2478593cd04ff8e720f Mon Sep 17 00:00:00 2001 From: gianlucafiore Date: Tue, 11 Aug 2026 22:41:26 -0300 Subject: [PATCH] fix: honor drawing units in annotation and viewport scales --- src/app/commands/layers.rs | 22 ++++++++++ src/app/update/command.rs | 29 +++++++++--- src/entities/viewport.rs | 26 ----------- src/scene/mod.rs | 90 +++++++++++++++++++++++++++++++++----- 4 files changed, 125 insertions(+), 42 deletions(-) diff --git a/src/app/commands/layers.rs b/src/app/commands/layers.rs index c695036e1..6afeea5ca 100644 --- a/src/app/commands/layers.rs +++ b/src/app/commands/layers.rs @@ -876,6 +876,28 @@ impl OpenCADStudio { } reframed += 1; } + + // Keep the active model-space annotation scale in sync as well. + // annotation_scale is drawing/paper, while CANNOSCALEVALUE stores + // the reciprocal paper/drawing factor. + if self.tabs[i].scene.annotation_scale.abs() > 1.0e-12 { + self.tabs[i].scene.annotation_scale *= factor as f32; + } + + if self.tabs[i] + .scene + .document + .header + .annotation_scale_value + .abs() + > 1.0e-12 + { + self.tabs[i] + .scene + .document + .header + .annotation_scale_value /= factor; + } // Sizes the drawing keeps as settings rather than as geometry: dash // lengths, default heights and widths, the radii the fillet and chamfer // commands start from. They are all lengths in the unit that just diff --git a/src/app/update/command.rs b/src/app/update/command.rs index 0035ec898..773ec8db4 100644 --- a/src/app/update/command.rs +++ b/src/app/update/command.rs @@ -1720,12 +1720,29 @@ pub(super) fn on_tab_close(&mut self, idx: usize) -> Task { Task::none() } - pub(super) fn on_prop_geom_choice_changed(&mut self, field: &'static str, value: String) -> Task { - let i = self.active_tab; - let handles = self.property_target_handles(i); - if !handles.is_empty() { - self.push_undo_snapshot(i, "CHPROP"); - if field == "vp_ucs_name" { + pub(super) fn on_prop_geom_choice_changed( + &mut self, + field: &'static str, + value: String, + ) -> Task { + let i = self.active_tab; + let handles = self.property_target_handles(i); + + if !handles.is_empty() { + self.push_undo_snapshot(i, "CHPROP"); + + if field == "vscale_std" { + for &handle in &handles { + if matches!( + self.tabs[i].scene.document.get_entity(handle), + Some(acadrust::EntityType::Viewport(_)) + ) { + let _ = self.tabs[i] + .scene + .set_viewport_scale_named_for(handle, &value); + } + } + } else if field == "vp_ucs_name" { // Resolve UCS name → cloned data, then mutate viewports. let ucs_data = self.tabs[i] .scene diff --git a/src/entities/viewport.rs b/src/entities/viewport.rs index 5660b925a..6e897c1c5 100644 --- a/src/entities/viewport.rs +++ b/src/entities/viewport.rs @@ -24,19 +24,6 @@ const STANDARD_SCALES: &[(&str, f64)] = &[ ("10:1", 10.0), ]; -/// Parse a scale ratio name to its paper/drawing factor: "1:50" -> 0.02, -/// "2:1" -> 2.0. Also accepts a plain decimal ("0.02"). -fn parse_scale_ratio(label: &str) -> Option { - if let Some((a, b)) = label.split_once(':') { - let n: f64 = a.trim().parse().ok()?; - let d: f64 = b.trim().parse().ok()?; - if d.abs() > 1e-12 { - return Some(n / d); - } - return None; - } - label.trim().parse::().ok() -} fn scale_label(scale: f64) -> String { for (label, val) in STANDARD_SCALES { @@ -248,19 +235,6 @@ fn apply_geom_prop(vp: &mut Viewport, field: &str, value: &str) { _ => {} } - // Scale picker. The label is a ratio name from the drawing's scale list - // ("1:50", "2:1"); parse it to the paper/drawing factor so any named - // scale resolves, not just the built-in set. - if field == "vscale_std" { - if let Some(scale) = parse_scale_ratio(value) { - vp.custom_scale = scale; - if scale > 1e-9 { - vp.view_height = vp.height / scale; - } - } - return; - } - // Render mode picker. if field == "vp_render" { if let Some(&(_, mode)) = RENDER_MODES.iter().find(|(label, _)| *label == value) { diff --git a/src/scene/mod.rs b/src/scene/mod.rs index ad9c53115..58773dc38 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -3584,6 +3584,26 @@ impl Scene { Self::DEFAULT_METRIC_SCALES } } + /// Conversion from one drawing unit to the paper unit used by the + /// standard annotation-scale family: millimetres for metric drawings + /// and inches for imperial drawings. + /// + /// The built-in scale factors are defined assuming model and paper use + /// the same base unit. A drawing in metres therefore needs an extra + /// factor of 1000 when its paper side is measured in millimetres. + fn annotation_scale_unit_factor(&self) -> f64 { + let paper_unit = if self.prefers_imperial_scales() == Some(true) { + 1 // Inches + } else { + 4 // Millimeters + }; + + crate::modules::draw::units::conversion_factor( + self.document.header.insertion_units, + paper_unit, + ) + .unwrap_or(1.0) + } fn is_architectural_scale_name(name: &str) -> bool { name.contains('=') && name.contains('"') && name.contains('\'') @@ -3642,6 +3662,7 @@ impl Scene { } pub fn scale_list(&self) -> Vec<(String, f32, f64)> { + let unit_factor = self.annotation_scale_unit_factor(); let mut list: Vec<(String, f32, f64)> = self .document .objects @@ -3656,7 +3677,11 @@ impl Scene { && !s.name.contains('|') && !s.name.to_ascii_uppercase().ends_with("_XREF") => { - Some((s.name.clone(), s.inverse_factor() as f32, s.factor())) + Some(( + s.name.clone(), + (s.inverse_factor() / unit_factor) as f32, + s.factor() * unit_factor, + )) } _ => None, }) @@ -3668,10 +3693,14 @@ impl Scene { // annotation / viewport scale picker would be empty and so appear // broken. Substitute the standard ratio set — file scales still // win whenever the drawing actually defines any. (#154) + list = self .default_scales() .iter() - .map(|&(label, vp)| (label.to_string(), (1.0 / vp) as f32, vp)) + .map(|&(label, base_vp)| { + let vp = base_vp * unit_factor; + (label.to_string(), (1.0 / vp) as f32, vp) + }) .collect(); } list @@ -3709,10 +3738,15 @@ impl Scene { } }); if !has_matching_family { + let unit_factor = self.annotation_scale_unit_factor(); + visible.extend( self.default_scales() .iter() - .map(|&(label, vp)| (label.to_string(), (1.0 / vp) as f32, vp)), + .map(|&(label, base_vp)| { + let vp = base_vp * unit_factor; + (label.to_string(), (1.0 / vp) as f32, vp) + }), ); } @@ -4005,6 +4039,7 @@ impl Scene { return Some(h); } } + let fallback = self .default_scales() .iter() @@ -4454,9 +4489,13 @@ impl Scene { } fn viewport_annotation_multiplier(&self, viewport: Handle) -> f32 { + let unit_factor = self.annotation_scale_unit_factor(); + self.viewport_scale_handle(viewport) .and_then(|handle| match self.document.objects.get(&handle) { - Some(ObjectType::Scale(scale)) => Some(scale.inverse_factor() as f32), + Some(ObjectType::Scale(scale)) => { + Some((scale.inverse_factor() / unit_factor) as f32) + } _ => None, }) .unwrap_or(self.annotation_scale) @@ -4483,27 +4522,45 @@ impl Scene { } pub fn set_viewport_scale_named(&mut self, name: &str) -> Option { + let viewport = self.explicit_viewport_handle()?; + self.set_viewport_scale_named_for(viewport, name) + } + + pub fn set_viewport_scale_named_for( + &mut self, + viewport: Handle, + name: &str, + ) -> Option { let scale_handle = self.scale_handle_ensuring(name)?; + let unit_factor = self.annotation_scale_unit_factor(); + let factor = match self.document.objects.get(&scale_handle) { - Some(ObjectType::Scale(scale)) => scale.factor(), + Some(ObjectType::Scale(scale)) => scale.factor() * unit_factor, _ => return None, }; - let viewport = self.explicit_viewport_handle()?; + let locked = matches!( self.document.get_entity(viewport), Some(EntityType::Viewport(vp)) if vp.status.locked ); + if locked || factor <= 1.0e-9 { return None; } + if let Some(EntityType::Viewport(vp)) = self.document.get_entity_mut(viewport) { vp.custom_scale = factor; vp.view_height = vp.height / factor; + } else { + return None; } + self.document .set_viewport_annotation_scale(viewport, scale_handle); + self.resident_wire_sets.borrow_mut().clear(); self.bump_geometry(); + Some(scale_handle) } @@ -4512,14 +4569,25 @@ impl Scene { let EntityType::Viewport(vp) = self.document.get_entity(viewport)? else { return None; }; + let scale = self.viewport_scale_handle(viewport)?; let ObjectType::Scale(scale) = self.document.objects.get(&scale)? else { return None; }; - let effective = vp_effective_scale(vp.custom_scale, vp.view_height, vp.height); - Some((effective - scale.factor()).abs() <= 1.0e-6 * scale.factor().max(1.0)) - } + let effective = vp_effective_scale( + vp.custom_scale, + vp.view_height, + vp.height, + ); + + let expected = scale.factor() * self.annotation_scale_unit_factor(); + + Some( + (effective - expected).abs() + <= 1.0e-6 * expected.max(1.0), + ) + } pub fn sync_viewport_annotation_scale(&mut self) -> bool { let Some(viewport) = self.explicit_viewport_handle() else { return false; @@ -4527,8 +4595,10 @@ impl Scene { let Some(scale_handle) = self.viewport_scale_handle(viewport) else { return false; }; + let unit_factor = self.annotation_scale_unit_factor(); + let factor = match self.document.objects.get(&scale_handle) { - Some(ObjectType::Scale(scale)) => scale.factor(), + Some(ObjectType::Scale(scale)) => scale.factor() * unit_factor, _ => return false, }; let Some(EntityType::Viewport(vp)) = self.document.get_entity_mut(viewport) else {