fix(pointstyle): stage dialog edits until accepted

Keep PDMODE and PDSIZE changes in the Point Style dialog until OK. Cancel and close discard staged edits. Invalid or non-finite size input reports an error and keeps the dialog open. Commit both values with one undo snapshot.

test: cancel discards staged glyph and size edits

test: invalid point size reports error and stays open

test: OK commits glyph and size in one undo step

test: relative point size keeps its sign
This commit is contained in:
Karim Jerbi committed 2026-10-18 09:21:43 +01:00
1 parent 248c9aec7b
commit c0baeda6a6
6 files changed
+214 -44

No files matched your search

+1 -7
View File
@@ -4154,13 +4154,7 @@ impl OpenCADStudio {
} }
} }
cmd if cmd == "DDPTYPE" || cmd == "PTYPE" => { cmd if cmd == "DDPTYPE" || cmd == "PTYPE" => {
// The dialog shows the magnitude; the sign (relative/absolute) self.begin_point_style_dialog();
// 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);
} }
"LWDISPLAY" => { "LWDISPLAY" => {
use crate::command::ValuePromptCommand; use crate::command::ValuePromptCommand;
+8
View File
@@ -1247,6 +1247,12 @@ pub(super) struct OpenCADStudio {
marketplace_status: String, marketplace_status: String,
/// PDSIZE text buffer for the Point Style (DDPTYPE) dialog. /// PDSIZE text buffer for the Point Style (DDPTYPE) dialog.
point_size_buf: String, 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<u64>,
/// Staged PDMODE selection; the document is changed only when the dialog
/// is accepted.
point_mode_buf: Option<i16>,
/// Point Style size mode: `true` = relative to screen, `false` = absolute. /// Point Style size mode: `true` = relative to screen, `false` = absolute.
/// Tracked separately from the PDSIZE sign so a size of 0 (sign-less) still /// Tracked separately from the PDSIZE sign so a size of 0 (sign-less) still
/// remembers which radio is active. /// remembers which radio is active.
@@ -5024,6 +5030,8 @@ impl OpenCADStudio {
plugin_readme_loading: rustc_hash::FxHashSet::default(), plugin_readme_loading: rustc_hash::FxHashSet::default(),
marketplace_status: String::new(), marketplace_status: String::new(),
point_size_buf: String::new(), point_size_buf: String::new(),
point_style_tab_id: None,
point_mode_buf: None,
point_size_relative: true, point_size_relative: true,
default_assoc_prompted: false, default_assoc_prompted: false,
check_missing_fonts: true, check_missing_fonts: true,
+111 -4
View File
@@ -196,6 +196,10 @@ impl OpenCADStudio {
) { ) {
self.style_stage_discard(); 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) { if self.active_modal == Some(ScaleManager) {
self.scale_stage_discard(); self.scale_stage_discard();
} }
@@ -10588,7 +10592,6 @@ impl OpenCADStudio {
} }
Message::PointStyleSizeRelative(relative) => { Message::PointStyleSizeRelative(relative) => {
self.point_size_relative = relative; self.point_size_relative = relative;
self.apply_point_size();
Task::none() Task::none()
} }
Message::PointStyleSizeInput(s) => { Message::PointStyleSizeInput(s) => {
@@ -10596,12 +10599,13 @@ impl OpenCADStudio {
Task::none() Task::none()
} }
Message::PointStyleApplySize => { Message::PointStyleApplySize => {
self.apply_point_size(); self.validate_point_size_input();
Task::none() Task::none()
} }
Message::PointStyleOk => { Message::PointStyleOk => {
self.apply_point_size(); if self.commit_point_style_dialog() {
self.close_active_modal(); self.close_active_modal();
}
Task::none() Task::none()
} }
@@ -12933,3 +12937,106 @@ fn web_autosave_due(savetime_min: i32) -> bool {
fire 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
);
}
}
+79 -26
View File
@@ -8,7 +8,7 @@ use crate::app::helpers::{
polar_constrain_if_near, polar_constrain_near, ucs_rotate_vec, ucs_to_wcs, ucs_z_axis, polar_constrain_if_near, polar_constrain_near, ucs_rotate_vec, ucs_to_wcs, ucs_z_axis,
CoordKind, 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::modules::ModuleEvent;
use crate::scene::model::object::GripApply; use crate::scene::model::object::GripApply;
use crate::scene::pick::grip::{ use crate::scene::pick::grip::{
@@ -548,49 +548,102 @@ impl OpenCADStudio {
Some(hit) Some(hit)
} }
/// Write PDSIZE from the dialog buffer with the current relative/absolute /// Begin a staged Point Style edit. The active drawing is untouched until
/// sign. A relative size is stored negative; absolute positive. Switching to /// the user accepts the dialog.
/// absolute with an empty/zero size seeds a positive value from the current pub(in crate::app) fn begin_point_style_dialog(&mut self) {
/// 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) {
let i = self.active_tab; 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 .point_size_buf
.trim() .trim()
.parse::<f64>() .parse::<f64>()
.unwrap_or(0.0) .is_ok_and(f64::is_finite);
.abs(); if !valid {
if !self.point_size_relative && mag == 0.0 { 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::<f64>()
.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 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; 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) crate::entities::point::relative_world_size(0.0, wpp, viewport_height)
} else { } else {
1.0 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 }; let next_size = if self.point_size_relative {
self.push_undo_snapshot(i, "PDSIZE"); -magnitude
self.tabs[i].scene.document.header.point_display_size = next; } 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].scene.invalidate_point_dependencies();
self.tabs[i].dirty = true; self.tabs[i].dirty = true;
true
} }
/// Replace the `mask` bits of PDMODE with `value`, rebuild the point glyphs /// Stage a replacement for the `mask` bits of PDMODE from the Point Style
/// and mark the document dirty. Used by the Point Style (DDPTYPE) dialog. /// (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) { pub(in crate::app) fn set_point_mode_bits(&mut self, mask: i16, value: i16) {
let i = self.active_tab; if self.point_style_tab_id.is_none() {
let cur = self.tabs[i].scene.document.header.point_display_mode;
let next = (cur & !mask) | (value & mask);
if next == cur {
return; return;
} }
self.push_undo_snapshot(i, "PDMODE"); let Some(cur) = self.point_mode_buf else {
self.tabs[i].scene.document.header.point_display_mode = next; return;
self.tabs[i].scene.invalidate_point_dependencies(); };
self.tabs[i].dirty = true; 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 /// Mirror the live grid display + grid-snap toggles onto tab `i`'s active
+7 -5
View File
@@ -1847,11 +1847,13 @@ impl OpenCADStudio {
} }
super::super::ModalKind::PointStyle => sized_flow(ex, 360, 470, |flow| { super::super::ModalKind::PointStyle => sized_flow(ex, 360, 470, |flow| {
crate::ui::style::point_style::view_window( crate::ui::style::point_style::view_window(
self.tabs[self.active_tab] self.point_mode_buf.unwrap_or_else(|| {
.scene self.tabs[self.active_tab]
.document .scene
.header .document
.point_display_mode, .header
.point_display_mode
}),
self.point_size_relative, self.point_size_relative,
&self.point_size_buf, &self.point_size_buf,
flow, flow,
+8 -2
View File
@@ -194,7 +194,13 @@ pub fn view_window<'a>(
] ]
.spacing(6); .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( container(
column![ column![
@@ -206,7 +212,7 @@ pub fn view_window<'a>(
Space::new().height(8), Space::new().height(8),
radios, radios,
Space::new().height(12), Space::new().height(12),
row![Space::new().width(width), ok].width(width), actions.width(width),
] ]
.spacing(4) .spacing(4)
.padding(20) .padding(20)