diff --git a/src/app/commands/styleprops.rs b/src/app/commands/styleprops.rs index 0d7afbbc6..6a80fd799 100644 --- a/src/app/commands/styleprops.rs +++ b/src/app/commands/styleprops.rs @@ -4154,13 +4154,7 @@ impl OpenCADStudio { } } cmd if cmd == "DDPTYPE" || cmd == "PTYPE" => { - // The dialog shows the magnitude; the sign (relative/absolute) - // is driven by the radio buttons. A positive PDSIZE is absolute; - // zero or negative is relative. - let pdsize = self.tabs[i].scene.document.header.point_display_size; - self.point_size_relative = pdsize <= 0.0; - self.point_size_buf = format!("{}", pdsize.abs()); - self.active_modal = Some(super::super::ModalKind::PointStyle); + self.begin_point_style_dialog(); } "LWDISPLAY" => { use crate::command::ValuePromptCommand; diff --git a/src/app/mod.rs b/src/app/mod.rs index 7f6516b3d..1b1839b23 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -1247,6 +1247,12 @@ pub(super) struct OpenCADStudio { marketplace_status: String, /// PDSIZE text buffer for the Point Style (DDPTYPE) dialog. point_size_buf: String, + /// Tab that owns the staged Point Style edits, so OK cannot write to a + /// different document if the active tab changes while the dialog is open. + point_style_tab_id: Option, + /// Staged PDMODE selection; the document is changed only when the dialog + /// is accepted. + point_mode_buf: Option, /// Point Style size mode: `true` = relative to screen, `false` = absolute. /// Tracked separately from the PDSIZE sign so a size of 0 (sign-less) still /// remembers which radio is active. @@ -5024,6 +5030,8 @@ impl OpenCADStudio { plugin_readme_loading: rustc_hash::FxHashSet::default(), marketplace_status: String::new(), point_size_buf: String::new(), + point_style_tab_id: None, + point_mode_buf: None, point_size_relative: true, default_assoc_prompted: false, check_missing_fonts: true, diff --git a/src/app/update/mod.rs b/src/app/update/mod.rs index 6417fbecc..be58a2101 100644 --- a/src/app/update/mod.rs +++ b/src/app/update/mod.rs @@ -196,6 +196,10 @@ impl OpenCADStudio { ) { self.style_stage_discard(); } + if self.active_modal == Some(PointStyle) { + self.point_style_tab_id = None; + self.point_mode_buf = None; + } if self.active_modal == Some(ScaleManager) { self.scale_stage_discard(); } @@ -10588,7 +10592,6 @@ impl OpenCADStudio { } Message::PointStyleSizeRelative(relative) => { self.point_size_relative = relative; - self.apply_point_size(); Task::none() } Message::PointStyleSizeInput(s) => { @@ -10596,12 +10599,13 @@ impl OpenCADStudio { Task::none() } Message::PointStyleApplySize => { - self.apply_point_size(); + self.validate_point_size_input(); Task::none() } Message::PointStyleOk => { - self.apply_point_size(); - self.close_active_modal(); + if self.commit_point_style_dialog() { + self.close_active_modal(); + } Task::none() } @@ -12933,3 +12937,106 @@ fn web_autosave_due(savetime_min: i32) -> bool { fire }) } + +#[cfg(test)] +mod point_style_dialog_tests { + use super::{Message, OpenCADStudio}; + use crate::app::ModalKind; + + fn open_with_values(mode: i16, size: f64) -> OpenCADStudio { + let mut app = OpenCADStudio::new_for_test(); + app.tabs[app.active_tab] + .scene + .document + .header + .point_display_mode = mode; + app.tabs[app.active_tab] + .scene + .document + .header + .point_display_size = size; + app.begin_point_style_dialog(); + app + } + + #[test] + fn closing_point_style_discards_glyph_and_size_edits() { + let mut app = open_with_values(2, 3.0); + + let _ = app.update(Message::PointStyleSetMode(3)); + let _ = app.update(Message::PointStyleSizeInput("8".into())); + let _ = app.update(Message::PointStyleApplySize); + let _ = app.update(Message::CloseModal); + + let header = &app.tabs[app.active_tab].scene.document.header; + assert_eq!(header.point_display_mode, 2); + assert_eq!(header.point_display_size, 3.0); + } + + #[test] + fn invalid_point_size_is_reported_and_does_not_close_or_write() { + let mut app = open_with_values(2, 3.0); + let old_error_revision = app.command_line.error_revision; + + let _ = app.update(Message::PointStyleSizeInput("abc".into())); + let _ = app.update(Message::PointStyleApplySize); + + assert_eq!( + app.tabs[app.active_tab] + .scene + .document + .header + .point_display_size, + 3.0 + ); + assert_eq!(app.active_modal, Some(ModalKind::PointStyle)); + assert!(app.command_line.error_revision > old_error_revision); + + let _ = app.update(Message::PointStyleOk); + assert_eq!(app.active_modal, Some(ModalKind::PointStyle)); + assert_eq!( + app.tabs[app.active_tab] + .scene + .document + .header + .point_display_mode, + 2 + ); + } + + #[test] + fn accepting_point_style_commits_both_values_as_one_undo_step() { + let mut app = open_with_values(2, 3.0); + let i = app.active_tab; + let undo_depth = app.tabs[i].history.undo_stack.len(); + + let _ = app.update(Message::PointStyleSetMode(35)); + let _ = app.update(Message::PointStyleSizeInput("8".into())); + let _ = app.update(Message::PointStyleOk); + + let header = &app.tabs[i].scene.document.header; + assert_eq!(header.point_display_mode, 35); + assert_eq!(header.point_display_size, 8.0); + assert_eq!(app.tabs[i].history.undo_stack.len(), undo_depth + 1); + assert!(app.tabs[i].dirty); + assert_eq!(app.active_modal, None); + } + + #[test] + fn accepting_relative_point_size_preserves_the_relative_sign() { + let mut app = open_with_values(2, 3.0); + + let _ = app.update(Message::PointStyleSizeRelative(true)); + let _ = app.update(Message::PointStyleSizeInput("5".into())); + let _ = app.update(Message::PointStyleOk); + + assert_eq!( + app.tabs[app.active_tab] + .scene + .document + .header + .point_display_size, + -5.0 + ); + } +} diff --git a/src/app/update/viewport.rs b/src/app/update/viewport.rs index 511c45c63..fd0e3f725 100644 --- a/src/app/update/viewport.rs +++ b/src/app/update/viewport.rs @@ -8,7 +8,7 @@ use crate::app::helpers::{ polar_constrain_if_near, polar_constrain_near, ucs_rotate_vec, ucs_to_wcs, ucs_z_axis, CoordKind, }; -use crate::app::{Message, OpenCADStudio, POLY_START_DELAY_MS}; +use crate::app::{Message, ModalKind, OpenCADStudio, POLY_START_DELAY_MS}; use crate::modules::ModuleEvent; use crate::scene::model::object::GripApply; use crate::scene::pick::grip::{ @@ -548,49 +548,102 @@ impl OpenCADStudio { Some(hit) } - /// Write PDSIZE from the dialog buffer with the current relative/absolute - /// sign. A relative size is stored negative; absolute positive. Switching to - /// absolute with an empty/zero size seeds a positive value from the current - /// on-screen size so the point stays representable (PDSIZE 0 always reads as - /// relative, so absolute needs a non-zero magnitude). - pub(in crate::app) fn apply_point_size(&mut self) { + /// Begin a staged Point Style edit. The active drawing is untouched until + /// the user accepts the dialog. + pub(in crate::app) fn begin_point_style_dialog(&mut self) { let i = self.active_tab; - let mut mag = self + let header = &self.tabs[i].scene.document.header; + let mode = header.point_display_mode; + let size = header.point_display_size; + self.point_style_tab_id = Some(self.tabs[i].id); + self.point_mode_buf = Some(mode); + self.point_size_relative = size <= 0.0; + self.point_size_buf = format!("{}", size.abs()); + self.active_modal = Some(ModalKind::PointStyle); + } + + /// Parse and validate the dialog's magnitude without changing the drawing. + pub(in crate::app) fn validate_point_size_input(&mut self) -> bool { + let valid = self .point_size_buf .trim() .parse::() - .unwrap_or(0.0) - .abs(); - if !self.point_size_relative && mag == 0.0 { + .is_ok_and(f64::is_finite); + if !valid { + self.command_line + .push_error(crate::t!("Point size must be a valid finite number.").as_ref()); + } + valid + } + + /// Commit the staged glyph and size together as one drawing undo step. + /// Returns false on invalid text so OK can leave the dialog open for repair. + pub(in crate::app) fn commit_point_style_dialog(&mut self) -> bool { + let Some(magnitude) = self + .point_size_buf + .trim() + .parse::() + .ok() + .filter(|value| value.is_finite()) + .map(f64::abs) + else { + self.command_line + .push_error(crate::t!("Point size must be a valid finite number.").as_ref()); + return false; + }; + let Some(tab_id) = self.point_style_tab_id else { + return false; + }; + let Some(i) = self.tabs.iter().position(|tab| tab.id == tab_id) else { + self.command_line + .push_error(crate::t!("The drawing for Point Style is no longer open.").as_ref()); + return false; + }; + + let mut magnitude = magnitude; + if !self.point_size_relative && magnitude == 0.0 { let wpp = self.tabs[i].scene.world_per_pixel().unwrap_or(0.0); let viewport_height = self.tabs[i].scene.selection.borrow().view.vp_size.1; - mag = if wpp > 0.0 { + magnitude = if wpp > 0.0 { crate::entities::point::relative_world_size(0.0, wpp, viewport_height) } else { 1.0 }; - self.point_size_buf = format!("{mag:.4}"); + self.point_size_buf = format!("{magnitude:.4}"); } - let next = if self.point_size_relative { -mag } else { mag }; - self.push_undo_snapshot(i, "PDSIZE"); - self.tabs[i].scene.document.header.point_display_size = next; + let next_size = if self.point_size_relative { + -magnitude + } else { + magnitude + }; + let next_mode = self + .point_mode_buf + .unwrap_or(self.tabs[i].scene.document.header.point_display_mode); + let header = &self.tabs[i].scene.document.header; + if header.point_display_mode == next_mode && header.point_display_size == next_size { + return true; + } + + self.push_undo_snapshot(i, "DDPTYPE"); + let header = &mut self.tabs[i].scene.document.header; + header.point_display_mode = next_mode; + header.point_display_size = next_size; self.tabs[i].scene.invalidate_point_dependencies(); self.tabs[i].dirty = true; + true } - /// Replace the `mask` bits of PDMODE with `value`, rebuild the point glyphs - /// and mark the document dirty. Used by the Point Style (DDPTYPE) dialog. + /// Stage a replacement for the `mask` bits of PDMODE from the Point Style + /// (DDPTYPE) dialog. The drawing is changed only when OK commits the stage. pub(in crate::app) fn set_point_mode_bits(&mut self, mask: i16, value: i16) { - let i = self.active_tab; - let cur = self.tabs[i].scene.document.header.point_display_mode; - let next = (cur & !mask) | (value & mask); - if next == cur { + if self.point_style_tab_id.is_none() { return; } - self.push_undo_snapshot(i, "PDMODE"); - self.tabs[i].scene.document.header.point_display_mode = next; - self.tabs[i].scene.invalidate_point_dependencies(); - self.tabs[i].dirty = true; + let Some(cur) = self.point_mode_buf else { + return; + }; + let next = (cur & !mask) | (value & mask); + self.point_mode_buf = Some(next); } /// Mirror the live grid display + grid-snap toggles onto tab `i`'s active diff --git a/src/app/view/modal.rs b/src/app/view/modal.rs index 237be84ed..fb137baf7 100644 --- a/src/app/view/modal.rs +++ b/src/app/view/modal.rs @@ -1847,11 +1847,13 @@ impl OpenCADStudio { } super::super::ModalKind::PointStyle => sized_flow(ex, 360, 470, |flow| { crate::ui::style::point_style::view_window( - self.tabs[self.active_tab] - .scene - .document - .header - .point_display_mode, + self.point_mode_buf.unwrap_or_else(|| { + self.tabs[self.active_tab] + .scene + .document + .header + .point_display_mode + }), self.point_size_relative, &self.point_size_buf, flow, diff --git a/src/ui/style/point_style.rs b/src/ui/style/point_style.rs index e4a28a45c..45d195761 100644 --- a/src/ui/style/point_style.rs +++ b/src/ui/style/point_style.rs @@ -194,7 +194,13 @@ pub fn view_window<'a>( ] .spacing(6); - let ok = dialog_button(t!("OK"), Message::PointStyleOk, true); + let actions = row![ + Space::new().width(width), + dialog_button(t!("OK"), Message::PointStyleOk, true), + dialog_button(t!("Cancel"), Message::CloseModal, false), + ] + .spacing(8) + .align_y(iced::Center); container( column![ @@ -206,7 +212,7 @@ pub fn view_window<'a>( Space::new().height(8), radios, Space::new().height(12), - row![Space::new().width(width), ok].width(width), + actions.width(width), ] .spacing(4) .padding(20)