From 7beb729d8549789581bcc54a57de367c7e97fb60 Mon Sep 17 00:00:00 2001 From: Karim Jerbi Date: Thu, 27 Aug 2026 19:13:40 +0100 Subject: [PATCH] refactor(app): single-source CHPROP/MATCHPROP 4-step recipe via apply_property_op MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract the push-undo → per-handle → invalidate → dirty → refresh contract into one helper on OpenCADStudio. The helper is the single source of truth for property-op side effects, so future property handlers can't forget the cache invalidation or the panel refresh. Note: MATCHPROP now no-ops when the destination selection is empty (previously it marked the tab dirty and refreshed the panel). This is a minor correctness improvement; restore the old behaviour with a 2-line guard at the MATCHPROP call site if desired. test app::properties::apply_property_op_tests::empty_handles_does_not_invoke_or_dirty test app::properties::apply_property_op_tests::applies_to_each_handle_sets_dirty_and_mutates test app::properties::apply_property_op_tests::missing_entity_does_not_abort_loop --- src/app/command_driver.rs | 25 +++--- src/app/commands/styleprops.rs | 44 ++++----- src/app/properties.rs | 131 +++++++++++++++++++++++++++ src/app/update/command.rs | 44 ++++----- src/app/update/mod.rs | 160 +++++++++++++-------------------- 5 files changed, 247 insertions(+), 157 deletions(-) diff --git a/src/app/command_driver.rs b/src/app/command_driver.rs index 09eb09718..2a0cc6f23 100644 --- a/src/app/command_driver.rs +++ b/src/app/command_driver.rs @@ -2245,11 +2245,10 @@ impl OpenCADStudio { .map(|e| crate::entities::dim_override::pairs(&e.common().extended_data)); if let Some(common) = src_common { - self.push_undo_snapshot(i, "MATCHPROP"); - for h in &dest { + self.apply_property_op(i, "MATCHPROP", &dest, |app, handle| { let mut is_dim = false; let mut is_hatch = false; - if let Some(e) = self.tabs[i].scene.document.get_entity_mut(*h) { + if let Some(e) = app.tabs[i].scene.document.get_entity_mut(handle) { e.as_entity_mut().set_layer(common.layer.clone()); crate::scene::view::dispatch::apply_color(e, common.color); crate::scene::view::dispatch::apply_line_weight(e, common.line_weight); @@ -2284,8 +2283,8 @@ impl OpenCADStudio { if is_hatch { if let Some(values) = &hatch_background_xdata { crate::scene::view::dispatch::set_entity_xdata( - &mut self.tabs[i].scene.document, - *h, + &mut app.tabs[i].scene.document, + handle, "HATCHBACKGROUNDCOLOR", values.clone(), ); @@ -2296,7 +2295,7 @@ impl OpenCADStudio { // stale raw record survives. if dstyle_xdata.is_some() || matches!( - self.tabs[i].scene.document.get_entity(*h), + app.tabs[i].scene.document.get_entity(handle), Some( acadrust::EntityType::Dimension(_) | acadrust::EntityType::Leader(_) @@ -2304,7 +2303,7 @@ impl OpenCADStudio { ) { if matches!( - self.tabs[i].scene.document.get_entity(*h), + app.tabs[i].scene.document.get_entity(handle), Some( acadrust::EntityType::Dimension(_) | acadrust::EntityType::Leader(_) @@ -2317,8 +2316,8 @@ impl OpenCADStudio { ) ) { crate::entities::dim_override::replace( - &mut self.tabs[i].scene.document, - *h, + &mut app.tabs[i].scene.document, + handle, dstyle_xdata.clone().unwrap_or_default(), ); } @@ -2326,12 +2325,11 @@ impl OpenCADStudio { // A restyled dimension renders from its baked *D block — // drop the stale block so the new style shows (#398). if is_dim { - self.tabs[i].scene.invalidate_dim_block_recorded(*h); + app.tabs[i].scene.invalidate_dim_block_recorded(handle); } // Hatch fills render from a prebuilt model (#415). - self.tabs[i].scene.refresh_fill_model(*h); - } - self.tabs[i].dirty = true; + app.tabs[i].scene.refresh_fill_model(handle); + }); // Color / linetype / lineweight are baked into the cached // wires at tessellation time; re-tessellate only the matched // objects instead of rebuilding a large drawing. @@ -2341,7 +2339,6 @@ impl OpenCADStudio { .map(|handle| (handle, crate::scene::ChangeKind::Modified)) .collect(); self.tabs[i].scene.bump_entities(&changes); - self.refresh_properties(); self.command_line .push_info(crate::tf!("Properties matched to {} object(s).", dest.len()).as_ref()); // Clear the consumed target selection and keep prompting. diff --git a/src/app/commands/styleprops.rs b/src/app/commands/styleprops.rs index 39d4cd93c..c4ecd2565 100644 --- a/src/app/commands/styleprops.rs +++ b/src/app/commands/styleprops.rs @@ -633,11 +633,20 @@ impl OpenCADStudio { "CHPROP: invalid value '{}' for {}.", value, prop ).as_ref()); + } else { + let known = matches!( + prop.as_str(), + "LAYER" | "LINETYPE" | "LT" | "LTSCALE" | "COLOR" | "TRANSPARENCY" + ); + if !known { + self.command_line.push_error(crate::tf!( + "CHPROP: unknown property '{}'. Use: LAYER COLOR LINETYPE LTSCALE TRANSPARENCY", prop + ).as_ref()); } else { let mut changed = 0usize; - for handle in &handles { + self.apply_property_op(i, "CHPROP", &handles, |app, handle| { if let Some(entity) = - self.tabs[i].scene.document.get_entity_mut(*handle) + app.tabs[i].scene.document.get_entity_mut(handle) { let common = entity.common_mut(); match prop.as_str() { @@ -661,30 +670,21 @@ impl OpenCADStudio { common.transparency = transparency_val.unwrap(); changed += 1; } - _ => { - self.command_line.push_error(crate::tf!( - "CHPROP: unknown property '{}'. Use: LAYER COLOR LINETYPE LTSCALE TRANSPARENCY", prop - ).as_ref()); - break; - } + _ => unreachable!(), } } - } - if changed > 0 { - self.push_undo_snapshot(i, "CHPROP"); - self.tabs[i].dirty = true; - // Colour / linetype / ltscale / transparency / - // layer are baked into the cached wire geometry — - // re-tessellate the changed entities so they - // repaint immediately (issue #231 class). - self.invalidate_property_targets(i, &handles); - self.command_line.push_output(crate::tf!( - "CHPROP: {} entity/entities updated.", - changed - ).as_ref()); - } + }); + // Colour / linetype / ltscale / transparency / + // layer are baked into the cached wire geometry — + // re-tessellate the changed entities so they + // repaint immediately (issue #231 class). + self.command_line.push_output(crate::tf!( + "CHPROP: {} entity/entities updated.", + changed + ).as_ref()); } } + } } } diff --git a/src/app/properties.rs b/src/app/properties.rs index 883e0e646..7ab2e5603 100644 --- a/src/app/properties.rs +++ b/src/app/properties.rs @@ -2436,6 +2436,34 @@ impl OpenCADStudio { self.tabs[i].scene.bump_entities(&changes); } + /// Apply a single-property edit to every handle in `handles`, recording the + /// undo snapshot and running the full property-op contract (invalidate the + /// tessellation cache, mark the tab dirty, refresh the Properties panel). + /// + /// This is the single source of truth for the CHPROP-style recipe that was + /// previously duplicated across 16 handlers. `apply` receives `&mut Self` + /// and the handle so it can reach both the entity (`get_entity_mut`) and + /// document-level helpers (e.g. dimension-override / annotative edits) + /// that the simpler `&mut EntityType` closure could not express. + pub(super) fn apply_property_op( + &mut self, + i: usize, + label: impl Into, + handles: &[Handle], + mut apply: impl FnMut(&mut Self, Handle), + ) { + if handles.is_empty() { + return; + } + self.push_undo_snapshot(i, label); + for &handle in handles { + apply(self, handle); + } + self.invalidate_property_targets(i, handles); + self.tabs[i].dirty = true; + self.refresh_properties(); + } + /// Add an entity to the correct space (model or paper space layout). pub(super) fn commit_entity(&mut self, entity: acadrust::EntityType) { let _ = self.commit_entity_handle(entity); @@ -3429,3 +3457,106 @@ mod insert_unit_scale_tests { assert_eq!(ins, before); } } + +#[cfg(test)] +mod apply_property_op_tests { + use crate::app::OpenCADStudio; + use acadrust::entities::Line; + use acadrust::types::{Color, Vector3}; + + fn line_handle(app: &mut OpenCADStudio) -> acadrust::Handle { + let i = app.active_tab; + let mut line = Line::new(); + line.start = Vector3::ZERO; + line.end = Vector3::new(1.0, 0.0, 0.0); + app.commit_entity_handle(acadrust::EntityType::Line(line)) + .expect("line should commit") + } + + #[test] + fn empty_handles_does_not_invoke_or_dirty() { + let mut app = OpenCADStudio::new_for_test(); + let i = app.active_tab; + let before = app.tabs[i].dirty; + let called = std::rc::Rc::new(std::cell::Cell::new(false)); + let called2 = called.clone(); + app.apply_property_op(i, "TEST", &[], |_app, _h| { + called2.set(true); + }); + assert!(!called.get(), "closure must not run for empty handles"); + assert_eq!( + app.tabs[i].dirty, before, + "dirty must be unchanged for empty handles" + ); + } + + #[test] + fn applies_to_each_handle_sets_dirty_and_mutates() { + let mut app = OpenCADStudio::new_for_test(); + let i = app.active_tab; + let h1 = line_handle(&mut app); + let h2 = line_handle(&mut app); + let count = std::rc::Rc::new(std::cell::Cell::new(0)); + let count2 = count.clone(); + app.tabs[i].dirty = false; + app.apply_property_op(i, "CHPROP", &[h1, h2], |app2, h| { + count2.set(count2.get() + 1); + if let Some(e) = app2.tabs[app2.active_tab] + .scene + .document + .get_entity_mut(h) + { + e.common_mut().color = Color::Index(7); + } + }); + assert_eq!(count.get(), 2, "closure runs once per handle"); + assert!(app.tabs[i].dirty, "tab marked dirty"); + let e = app.tabs[i] + .scene + .document + .get_entity(h1) + .expect("entity present"); + assert_eq!( + e.common().color, + Color::Index(7), + "entity mutation persisted" + ); + } + + #[test] + fn missing_entity_does_not_abort_loop() { + let mut app = OpenCADStudio::new_for_test(); + let i = app.active_tab; + let h1 = line_handle(&mut app); + // A freshly allocated handle has no entity yet. + let missing = app.tabs[i].scene.document.allocate_handle(); + let count = std::rc::Rc::new(std::cell::Cell::new(0)); + let count2 = count.clone(); + app.tabs[i].dirty = false; + app.apply_property_op(i, "CHPROP", &[h1, missing], |app2, h| { + count2.set(count2.get() + 1); + if let Some(e) = app2.tabs[app2.active_tab] + .scene + .document + .get_entity_mut(h) + { + e.common_mut().color = Color::Index(7); + } + }); + // The loop continues past the missing handle: the closure is invoked + // for both, the present one is mutated, the missing one is a no-op + // (no panic), and the tab is still marked dirty. + assert_eq!(count.get(), 2, "closure still called for the missing handle"); + assert!(app.tabs[i].dirty, "tab marked dirty even with a missing handle"); + let e = app.tabs[i] + .scene + .document + .get_entity(h1) + .expect("entity present"); + assert_eq!( + e.common().color, + Color::Index(7), + "the present handle was mutated; the missing one was skipped" + ); + } +} diff --git a/src/app/update/command.rs b/src/app/update/command.rs index 016902beb..81b3007b0 100644 --- a/src/app/update/command.rs +++ b/src/app/update/command.rs @@ -1608,19 +1608,17 @@ pub(super) fn on_tab_close(&mut self, idx: usize) -> Task { } else { // Apply to the selection; leave the creation default alone // ("Make current" is a separate action). - self.push_undo_snapshot(i, "CHPROP"); - for &handle in &handles { - if let Some(entity) = self.tabs[i].scene.document.get_entity_mut(handle) { - crate::scene::view::dispatch::apply_common_prop(entity, "layer", &layer); - } - } // Layer drives by-layer colour/linetype/lineweight, which are // baked into the cached wire geometry — re-tessellate so the // change shows immediately (issue #231 class). - self.invalidate_property_targets(i, &handles); - self.tabs[i].dirty = true; + self.apply_property_op(i, "CHPROP", &handles, |app, handle| { + if let Some(entity) = + app.tabs[i].scene.document.get_entity_mut(handle) + { + crate::scene::view::dispatch::apply_common_prop(entity, "layer", &layer); + } + }); self.ribbon.active_layer = layer; - self.refresh_properties(); } Task::none() } @@ -1641,16 +1639,14 @@ pub(super) fn on_tab_close(&mut self, idx: usize) -> Task { self.tabs[i].dirty = true; self.ribbon.active_color = color; } else { - self.push_undo_snapshot(i, "CHPROP"); - for &handle in &handles { - if let Some(entity) = self.tabs[i].scene.document.get_entity_mut(handle) { + self.apply_property_op(i, "CHPROP", &handles, |app, handle| { + if let Some(entity) = + app.tabs[i].scene.document.get_entity_mut(handle) + { crate::scene::view::dispatch::apply_color(entity, color); } - } - self.invalidate_property_targets(i, &handles); - self.tabs[i].dirty = true; + }); self.ribbon.active_color = color; - self.refresh_properties(); } Task::none() } @@ -1679,19 +1675,17 @@ pub(super) fn on_tab_close(&mut self, idx: usize) -> Task { self.tabs[i].dirty = true; self.ribbon.active_linetype = lt; } else { - self.push_undo_snapshot(i, "CHPROP"); - for &handle in &handles { - if let Some(entity) = self.tabs[i].scene.document.get_entity_mut(handle) { - crate::scene::view::dispatch::apply_common_prop(entity, "linetype", <); - } - } // Linetype is baked into the cached wire geometry — // re-tessellate so the dashed/solid look updates immediately // (issue #231 class). - self.invalidate_property_targets(i, &handles); - self.tabs[i].dirty = true; + self.apply_property_op(i, "CHPROP", &handles, |app, handle| { + if let Some(entity) = + app.tabs[i].scene.document.get_entity_mut(handle) + { + crate::scene::view::dispatch::apply_common_prop(entity, "linetype", <); + } + }); self.ribbon.active_linetype = lt; - self.refresh_properties(); } Task::none() } diff --git a/src/app/update/mod.rs b/src/app/update/mod.rs index 4c276d921..cb501dc7d 100644 --- a/src/app/update/mod.rs +++ b/src/app/update/mod.rs @@ -4576,19 +4576,17 @@ impl OpenCADStudio { self.tabs[i].dirty = true; self.ribbon.active_lineweight = lw; } else { - self.push_undo_snapshot(i, "CHPROP"); - for &handle in &handles { - if let Some(entity) = self.tabs[i].scene.document.get_entity_mut(handle) { - crate::scene::view::dispatch::apply_line_weight(entity, lw); - } - } // Lineweight is baked into the cached wire geometry — // re-tessellate so the change shows immediately (issue #231 // class). - self.invalidate_property_targets(i, &handles); - self.tabs[i].dirty = true; + self.apply_property_op(i, "CHPROP", &handles, |app, handle| { + if let Some(entity) = + app.tabs[i].scene.document.get_entity_mut(handle) + { + crate::scene::view::dispatch::apply_line_weight(entity, lw); + } + }); self.ribbon.active_lineweight = lw; - self.refresh_properties(); } Task::none() } @@ -4603,17 +4601,15 @@ impl OpenCADStudio { self.refresh_properties(); return task; } - self.push_undo_snapshot(i, "CHPROP"); - for &handle in &handles { - if let Some(entity) = self.tabs[i].scene.document.get_entity_mut(handle) { + self.apply_property_op(i, "CHPROP", &handles, |app, handle| { + if let Some(entity) = + app.tabs[i].scene.document.get_entity_mut(handle) + { crate::scene::view::dispatch::apply_common_prop( entity, "layer", &layer, ); } - } - self.invalidate_property_targets(i, &handles); - self.tabs[i].dirty = true; - self.refresh_properties(); + }); Task::none() } @@ -4626,17 +4622,15 @@ impl OpenCADStudio { self.refresh_properties(); return task; } - self.push_undo_snapshot(i, "CHPROP"); - for &handle in &handles { - if let Some(entity) = self.tabs[i].scene.document.get_entity_mut(handle) { + self.apply_property_op(i, "CHPROP", &handles, |app, handle| { + if let Some(entity) = + app.tabs[i].scene.document.get_entity_mut(handle) + { crate::scene::view::dispatch::apply_color(entity, color); } - } - self.invalidate_property_targets(i, &handles); + }); self.tabs[i].properties.color_picker_open = false; self.tabs[i].properties.color_palette_open = false; - self.tabs[i].dirty = true; - self.refresh_properties(); Task::none() } @@ -4653,15 +4647,13 @@ impl OpenCADStudio { self.refresh_properties(); return Task::none(); } - self.push_undo_snapshot(i, "CHPROP"); - for &handle in &handles { - if let Some(entity) = self.tabs[i].scene.document.get_entity_mut(handle) { + self.apply_property_op(i, "CHPROP", &handles, |app, handle| { + if let Some(entity) = + app.tabs[i].scene.document.get_entity_mut(handle) + { crate::scene::view::dispatch::apply_line_weight(entity, lw); } - } - self.invalidate_property_targets(i, &handles); - self.tabs[i].dirty = true; - self.refresh_properties(); + }); Task::none() } @@ -4673,17 +4665,15 @@ impl OpenCADStudio { self.refresh_properties(); return task; } - self.push_undo_snapshot(i, "CHPROP"); - for &handle in &handles { - if let Some(entity) = self.tabs[i].scene.document.get_entity_mut(handle) { + self.apply_property_op(i, "CHPROP", &handles, |app, handle| { + if let Some(entity) = + app.tabs[i].scene.document.get_entity_mut(handle) + { crate::scene::view::dispatch::apply_common_prop( entity, "linetype", <, ); } - } - self.invalidate_property_targets(i, &handles); - self.tabs[i].dirty = true; - self.refresh_properties(); + }); Task::none() } @@ -4765,8 +4755,7 @@ impl OpenCADStudio { let i = self.active_tab; let handles = self.property_target_handles(i); if !handles.is_empty() { - self.push_undo_snapshot(i, "CHPROP"); - for &handle in &handles { + self.apply_property_op(i, "CHPROP", &handles, |app, handle| { match field { // Per-object annotative toggle: MTEXT/MULTILEADER carry // a native flag; single-line TEXT is annotative purely @@ -4775,7 +4764,7 @@ impl OpenCADStudio { // representation and turning it off removes it (not just // the flag). "is_annotative" | "enable_annotation_scale" | "annotative_ctx" => { - let doc = &self.tabs[i].scene.document; + let doc = &app.tabs[i].scene.document; let cur = match doc.get_entity(handle) { Some(acadrust::EntityType::MText(t)) => t.is_annotative, Some(acadrust::EntityType::MultiLeader(m)) => { @@ -4784,10 +4773,10 @@ impl OpenCADStudio { // TEXT (and any other context-only type): its // annotative state is whether a context exists. Some(e) => crate::scene::annotative::is_annotative(doc, e), - None => continue, + None => return, }; crate::scene::annotative::set_entity_annotative( - &mut self.tabs[i].scene.document, + &mut app.tabs[i].scene.document, handle, !cur, ); @@ -4798,10 +4787,10 @@ impl OpenCADStudio { // object. Off is handled inside set_entity_*. if !cur { if let Some(sh) = - self.tabs[i].scene.creation_annotation_scale_handle() + app.tabs[i].scene.creation_annotation_scale_handle() { crate::scene::annotative::create_annotation_context( - &mut self.tabs[i].scene.document, + &mut app.tabs[i].scene.document, handle, sh, ); @@ -4810,7 +4799,7 @@ impl OpenCADStudio { } "invisible" => { if let Some(entity) = - self.tabs[i].scene.document.get_entity_mut(handle) + app.tabs[i].scene.document.get_entity_mut(handle) { crate::scene::view::dispatch::toggle_invisible(entity); } @@ -4819,22 +4808,22 @@ impl OpenCADStudio { // (#427): checking collapses Y/Z onto X; unchecking // only switches the panel to per-axis rows. "ins_uniform" => { - let scales = match self.tabs[i].scene.document.get_entity(handle) { + let scales = match app.tabs[i].scene.document.get_entity(handle) { Some(acadrust::EntityType::Insert(ins)) => { Some((ins.x_scale(), ins.y_scale(), ins.z_scale())) } _ => None, }; - let Some((sx, sy, sz)) = scales else { continue }; + let Some((sx, sy, sz)) = scales else { return }; let eq = (sx - sy).abs() < 1e-12 && (sx - sz).abs() < 1e-12; let checked = - eq && !self.props_asym_scale.contains(&handle.value()); + eq && !app.props_asym_scale.contains(&handle.value()); if checked { - self.props_asym_scale.insert(handle.value()); + app.props_asym_scale.insert(handle.value()); } else { - self.props_asym_scale.remove(&handle.value()); + app.props_asym_scale.remove(&handle.value()); if let Some(acadrust::EntityType::Insert(ins)) = - self.tabs[i].scene.document.get_entity_mut(handle) + app.tabs[i].scene.document.get_entity_mut(handle) { ins.set_y_scale(sx); ins.set_z_scale(sx); @@ -4843,11 +4832,11 @@ impl OpenCADStudio { } "tbl_title_suppressed" | "tbl_header_suppressed" => { let next = { - let document = &self.tabs[i].scene.document; + let document = &app.tabs[i].scene.document; let Some(acadrust::EntityType::Table(table)) = document.get_entity(handle) else { - continue; + return; }; let table_style = table.table_style_handle.and_then(|style_handle| { document.objects.get(&style_handle).and_then(|object| { @@ -4873,7 +4862,7 @@ impl OpenCADStudio { !current }; if let Some(entity) = - self.tabs[i].scene.document.get_entity_mut(handle) + app.tabs[i].scene.document.get_entity_mut(handle) { crate::scene::view::dispatch::apply_geom_prop( entity, @@ -4884,7 +4873,7 @@ impl OpenCADStudio { } _ => { if let Some(entity) = - self.tabs[i].scene.document.get_entity_mut(handle) + app.tabs[i].scene.document.get_entity_mut(handle) { crate::scene::view::dispatch::apply_geom_prop( entity, field, "toggle", @@ -4892,19 +4881,14 @@ impl OpenCADStudio { } } } - } - if field.starts_with("tbl_") { - for &handle in &handles { + if field.starts_with("tbl_") { if let Some(acadrust::EntityType::Table(table)) = - self.tabs[i].scene.document.get_entity_mut(handle) + app.tabs[i].scene.document.get_entity_mut(handle) { table.block_record_handle = None; } } - } - self.invalidate_property_targets(i, &handles); - self.tabs[i].dirty = true; - self.refresh_properties(); + }); } Task::none() } @@ -5034,9 +5018,8 @@ impl OpenCADStudio { let i = self.active_tab; let handles = self.property_target_handles(i); if !handles.is_empty() { - self.push_undo_snapshot(i, "CHPROP"); - for &handle in &handles { - match self.tabs[i].scene.document.get_entity_mut(handle) { + self.apply_property_op(i, "CHPROP", &handles, |app, handle| { + match app.tabs[i].scene.document.get_entity_mut(handle) { Some(acadrust::EntityType::MText(m)) => { m.background_color = color.clone(); // Picking a colour turns the background on in Fill @@ -5049,11 +5032,8 @@ impl OpenCADStudio { } _ => {} } - } - self.invalidate_property_targets(i, &handles); + }); self.tabs[i].properties.bg_color_picker_open = false; - self.tabs[i].dirty = true; - self.refresh_properties(); } Task::none() } @@ -5109,11 +5089,10 @@ impl OpenCADStudio { }) .collect(); if !targets.is_empty() { - self.push_undo_snapshot(i, "CHPROP"); - for &handle in &targets { + self.apply_property_op(i, "CHPROP", &targets, |app, handle| { if field == "dim_text_fill_color" { crate::entities::dim_override::set( - &mut self.tabs[i].scene.document, + &mut app.tabs[i].scene.document, handle, crate::entities::dim_override::DIMTFILL, Some(acadrust::xdata::XDataValue::Integer16( @@ -5121,20 +5100,17 @@ impl OpenCADStudio { )), ); if fill_mode != Some(2) { - continue; + return; } } crate::entities::dim_override::set( - &mut self.tabs[i].scene.document, + &mut app.tabs[i].scene.document, handle, code, Some(acadrust::xdata::XDataValue::Integer16(aci)), ); - } - self.invalidate_property_targets(i, &targets); + }); self.tabs[i].properties.open_color_field = None; - self.tabs[i].dirty = true; - self.refresh_properties(); } return Task::none(); } @@ -5144,16 +5120,15 @@ impl OpenCADStudio { ) { let cell_index = self.tabs[i].properties.prop_vertex; if !handles.is_empty() { - self.push_undo_snapshot(i, "TABLE CELL COLOR"); - for &handle in &handles { + self.apply_property_op(i, "TABLE CELL COLOR", &handles, |app, handle| { let Some(acadrust::EntityType::Table(table)) = - self.tabs[i].scene.document.get_entity_mut(handle) + app.tabs[i].scene.document.get_entity_mut(handle) else { - continue; + return; }; let columns = table.column_count(); if columns == 0 { - continue; + return; } if let Some(cell) = table.cell_mut( cell_index / columns, @@ -5164,7 +5139,7 @@ impl OpenCADStudio { CellStateFlags::FORMAT_LOCKED | CellStateFlags::FORMAT_READ_ONLY, ) { - continue; + return; } let style = cell.style.get_or_insert_with(Default::default); if field == "tbl_cell_content_color" { @@ -5181,20 +5156,16 @@ impl OpenCADStudio { } } table.block_record_handle = None; - } - self.invalidate_property_targets(i, &handles); + }); self.tabs[i].properties.open_color_field = None; - self.tabs[i].dirty = true; - self.refresh_properties(); } return Task::none(); } if !handles.is_empty() { - self.push_undo_snapshot(i, "CHPROP"); let idx = if field == "gradient_color_2" { 1 } else { 0 }; - for &handle in &handles { + self.apply_property_op(i, "CHPROP", &handles, |app, handle| { if let Some(acadrust::EntityType::Hatch(h)) = - self.tabs[i].scene.document.get_entity_mut(handle) + app.tabs[i].scene.document.get_entity_mut(handle) { while h.gradient_color.colors.len() <= idx { let value = if h.gradient_color.colors.is_empty() { @@ -5211,14 +5182,11 @@ impl OpenCADStudio { } h.gradient_color.colors[idx].color = color.clone(); } - } + }); // Rebuild hatch seeds so the gradient fill picks up the new // colour (synced_hatch_models only patches the main colour). self.tabs[i].scene.populate_hatches_from_document(); - self.invalidate_property_targets(i, &handles); self.tabs[i].properties.open_color_field = None; - self.tabs[i].dirty = true; - self.refresh_properties(); } Task::none() }