From 6cc1bf9755da926eeb14369af8a94da881229d9d Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Sat, 15 Aug 2026 19:56:35 +0300 Subject: [PATCH] fix(layer): centralize rename flow --- Cargo.lock | 4 +- Cargo.toml | 4 +- crates/ocs_plugin_api/Cargo.toml | 2 +- crates/ocs_web_worker/Cargo.toml | 2 +- src/app/commands/styleprops.rs | 53 ++++------------------ src/app/document.rs | 21 ++++++++- src/app/history.rs | 19 +++++++- src/app/update/command.rs | 77 +++++++++++--------------------- src/scene/entity.rs | 8 ++++ src/scene/mod.rs | 9 ++-- 10 files changed, 92 insertions(+), 107 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9841c6ed7..df14f11e1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -85,7 +85,7 @@ dependencies = [ [[package]] name = "acadrust" version = "0.4.1" -source = "git+https://github.com/HakanSeven12/cadcodec.git?rev=4736846#473684644e8bbacd1310a7eccdb4b3180d7013db" +source = "git+https://github.com/HakanSeven12/cadcodec.git?rev=c28b2cd#c28b2cd25851ca0ee555b7cfac673927456ae04e" dependencies = [ "ahash 0.8.12", "anyhow", @@ -891,7 +891,7 @@ checksum = "fc652a48c352aef3ea3aed32080501cf3ef6ed5da78602a020c991775b0aff04" [[package]] name = "cadkernel" version = "0.1.0" -source = "git+https://github.com/HakanSeven12/cadkernel.git?rev=1fe426d#1fe426d1922dfc79685b8f6d34ce66ab15d97814" +source = "git+https://github.com/HakanSeven12/cadkernel.git?rev=617bdec#617bdecabe97c0140ffdecef3d2af2e2b5e612b1" dependencies = [ "acadrust", "cavalier_contours", diff --git a/Cargo.toml b/Cargo.toml index c3272527c..d5d79e167 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,8 +31,8 @@ glam = { version = "0.33", features = ["bytemuck"] } rfd = "0.17" clap = { version = "4", features = ["derive"] } env_logger = "0.11" -acadrust = { git = "https://github.com/HakanSeven12/cadcodec.git", rev = "4736846", features = ["serde"] } -cadkernel = { git = "https://github.com/HakanSeven12/cadkernel.git", rev = "1fe426d", features = ["acis", "offset"] } +acadrust = { git = "https://github.com/HakanSeven12/cadcodec.git", rev = "c28b2cd", features = ["serde"] } +cadkernel = { git = "https://github.com/HakanSeven12/cadkernel.git", rev = "617bdec", features = ["acis", "offset"] } acadifc = { git = "https://github.com/OpenAEC-Foundation/acadifc.git", rev = "47b5603", optional = true } dwg-thumbnailer = { path = "crates/dwg-thumbnailer" } flate2 = "1" diff --git a/crates/ocs_plugin_api/Cargo.toml b/crates/ocs_plugin_api/Cargo.toml index da8c6bd39..4384a8afd 100644 --- a/crates/ocs_plugin_api/Cargo.toml +++ b/crates/ocs_plugin_api/Cargo.toml @@ -9,7 +9,7 @@ license = "GPL-3.0-only" # Pulled in only by the `host` feature, which adds the `acadrust`-typed # `HostApi` runtime surface. The default crate stays dependency-free so engine # crates and external tooling can depend on the manifest/ribbon contract cheaply. -acadrust = { git = "https://github.com/HakanSeven12/cadcodec.git", rev = "4736846", optional = true, features = ["serde"] } +acadrust = { git = "https://github.com/HakanSeven12/cadcodec.git", rev = "c28b2cd", optional = true, features = ["serde"] } # Runtime IPC and serialization (host feature only). interprocess = { version = "2", optional = true } diff --git a/crates/ocs_web_worker/Cargo.toml b/crates/ocs_web_worker/Cargo.toml index e850ebeb8..d6c465349 100644 --- a/crates/ocs_web_worker/Cargo.toml +++ b/crates/ocs_web_worker/Cargo.toml @@ -8,7 +8,7 @@ publish = false crate-type = ["cdylib"] [dependencies] -acadrust = { git = "https://github.com/HakanSeven12/cadcodec.git", rev = "4736846", features = ["serde"] } +acadrust = { git = "https://github.com/HakanSeven12/cadcodec.git", rev = "c28b2cd", features = ["serde"] } bincode = "1.3" serde = { version = "1", features = ["derive"] } console_error_panic_hook = "0.1" diff --git a/src/app/commands/styleprops.rs b/src/app/commands/styleprops.rs index cc800f9a8..5eba1ee64 100644 --- a/src/app/commands/styleprops.rs +++ b/src/app/commands/styleprops.rs @@ -1881,6 +1881,8 @@ impl OpenCADStudio { // INSERT reference; refuses anonymous/xref sources and // invalid/taken names. self.tabs[i].scene.rename_block(&old_name, &new_name) + } else if type_str == "LAYER" { + self.tabs[i].rename_layer(&old_name, &new_name) } else if known { rename_symbol( &mut self.tabs[i].scene.document, @@ -1892,14 +1894,8 @@ impl OpenCADStudio { false }; if ok { - // Renamed references resolve at tessellation time — - // rebuild so nothing renders off a stale name. - self.tabs[i].scene.bump_geometry_no_blocks(); - // The per-tab active layer tracks the name. - if type_str == "LAYER" - && self.tabs[i].active_layer.eq_ignore_ascii_case(&old_name) - { - self.tabs[i].active_layer = new_name.clone(); + if type_str != "LAYER" { + self.tabs[i].scene.bump_geometry_no_blocks(); } if type_str == "UCS" { if let Some(active) = self.tabs[i].active_ucs.as_mut() { @@ -1910,6 +1906,9 @@ impl OpenCADStudio { } } self.tabs[i].dirty = true; + if type_str == "LAYER" { + self.refresh_layer_panel(); + } self.command_line .push_output(crate::tf!("RENAME: '{}' → '{}'.", old_name, new_name).as_ref()); } else { @@ -2352,13 +2351,7 @@ impl OpenCADStudio { } } -/// RENAME for the name-keyed symbol tables (layer / text style / dim style / -/// linetype / UCS / view). `Table` stores entries in a map keyed by -/// UPPERCASE name, so a rename must remove + re-add — mutating `.name` in -/// place leaves the map keyed by the old name and every later -/// `get(new_name)` lookup silently fails. Name-based references (entity -/// layer/linetype/style fields, layer linetypes, header current-* names) are -/// chased case-insensitively, matching how the tables resolve names. +/// RENAME for the remaining name-keyed symbol tables. fn rename_symbol(doc: &mut acadrust::CadDocument, ty: &str, old: &str, new: &str) -> bool { use acadrust::{EntityType, Table, TableEntry}; @@ -2366,38 +2359,10 @@ fn rename_symbol(doc: &mut acadrust::CadDocument, ty: &str, old: &str, new: &str if !crate::scene::valid_block_name(new) { return false; } - // A case-only rename reuses its own key; any other target must be free. - if !old.eq_ignore_ascii_case(new) && table.contains(new) { - return false; - } - let Some(mut e) = table.remove(old) else { - return false; - }; - e.set_name(new.to_string()); - let _ = table.add(e); - true + table.rename(old, new.to_string()).is_ok() } match ty { - "LAYER" => { - // Layer 0 and Defpoints are fixed names; xref-dependent ("|") - // layers belong to the referenced file. - if old == "0" || old.eq_ignore_ascii_case("Defpoints") || old.contains('|') { - return false; - } - if !rekey(&mut doc.layers, old, new) { - return false; - } - for e in doc.entities_mut() { - if e.common().layer.eq_ignore_ascii_case(old) { - e.common_mut().layer = new.to_string(); - } - } - if doc.header.current_layer_name.eq_ignore_ascii_case(old) { - doc.header.current_layer_name = new.to_string(); - } - true - } "STYLE" | "TEXTSTYLE" => { if !rekey(&mut doc.text_styles, old, new) { return false; diff --git a/src/app/document.rs b/src/app/document.rs index 702d20296..09048da4f 100644 --- a/src/app/document.rs +++ b/src/app/document.rs @@ -7,7 +7,7 @@ use crate::scene::GripDef; use crate::scene::{ObjectIsolationState, Scene}; use crate::snap::SnapResult; use crate::ui::{LayerPanel, PropertiesPanel}; -use acadrust::tables::Ucs; +use acadrust::tables::{normalize_name, Ucs}; use acadrust::{CadDocument, EntityType, Handle}; use iced; use crate::t; @@ -222,6 +222,18 @@ pub(super) struct DocumentTab { } impl DocumentTab { + pub(super) fn rename_layer(&mut self, old_name: &str, new_name: &str) -> bool { + let active = normalize_name(&self.active_layer) == normalize_name(old_name); + if !self.scene.rename_layer(old_name, new_name) { + return false; + } + if active { + self.active_layer = new_name.to_string(); + } + self.dirty = true; + true + } + pub(super) fn active_block_edit_session(&self) -> Option<&BlockEditSession> { self.active_block_edit .and_then(|index| self.block_edits.get(index)) @@ -546,6 +558,11 @@ impl HistorySnapshot { ) .saturating_add(d.selected_before.len().saturating_mul(16)) .saturating_add(d.selected_after.len().saturating_mul(16)) + .saturating_add( + d.active_layer + .as_ref() + .map_or(0, |(before, after)| before.len().saturating_add(after.len())), + ) .saturating_add(d.label.len()), HistorySnapshot::ObjectVisibility(v) => v .before @@ -598,6 +615,7 @@ pub(super) struct DeltaSnapshot { pub(super) selected_after: Vec, pub(super) dirty_before: bool, pub(super) dirty_after: bool, + pub(super) active_layer: Option<(String, String)>, /// Opposite non-entity document state. `apply_delta_state` swaps this with /// the live structure, so the same allocation shuttles between undo/redo. pub(super) structure: Option, @@ -671,6 +689,7 @@ pub(super) struct HistoryState { pub(super) struct PendingHistorySnapshot { pub(super) label: String, pub(super) current_layout: String, + pub(super) active_layer: String, pub(super) selected_before: Vec, pub(super) dirty_before: bool, pub(super) structure_before: CadDocument, diff --git a/src/app/history.rs b/src/app/history.rs index 342f76046..45c933a0f 100644 --- a/src/app/history.rs +++ b/src/app/history.rs @@ -223,6 +223,7 @@ impl OpenCADStudio { selected_after: selected, dirty_before, dirty_after: true, + active_layer: None, structure: (!objects.is_empty()).then_some(StructureSnapshot::Objects(objects)), label: label.into(), }; @@ -313,10 +314,12 @@ impl OpenCADStudio { .collect(); let selected_after: Vec = self.tabs[i].scene.selected.iter().copied().collect(); let dirty_after = self.tabs[i].dirty; + let active_layer_after = self.tabs[i].active_layer.clone(); if entities.is_empty() && !structure_changed && pending.selected_before == selected_after && pending.dirty_before == dirty_after + && pending.active_layer == active_layer_after { return; } @@ -328,6 +331,8 @@ impl OpenCADStudio { selected_after, dirty_before: pending.dirty_before, dirty_after, + active_layer: (pending.active_layer != active_layer_after) + .then_some((pending.active_layer, active_layer_after)), structure: structure_changed.then_some(StructureSnapshot::Full(pending.structure_before)), label: pending.label, }; @@ -351,6 +356,7 @@ impl OpenCADStudio { self.tabs[i].history.pending = Some(PendingHistorySnapshot { label, current_layout, + active_layer: self.tabs[i].active_layer.clone(), selected_before, dirty_before, structure_before, @@ -433,6 +439,7 @@ impl OpenCADStudio { selected_after, dirty_before: pending.dirty_before, dirty_after: self.tabs[i].dirty, + active_layer: None, structure: Some(StructureSnapshot::Layers(entries)), label: pending.label, }; @@ -487,6 +494,7 @@ impl OpenCADStudio { selected_after: self.tabs[i].scene.selected.iter().copied().collect(), dirty_before: pending.dirty_before, dirty_after: self.tabs[i].dirty, + active_layer: None, structure: Some(StructureSnapshot::TextStyles(entries)), label: pending.label, }; @@ -541,6 +549,7 @@ impl OpenCADStudio { selected_after: self.tabs[i].scene.selected.iter().copied().collect(), dirty_before: pending.dirty_before, dirty_after: self.tabs[i].dirty, + active_layer: None, structure: Some(StructureSnapshot::DimStyles(entries)), label: pending.label, }; @@ -603,6 +612,7 @@ impl OpenCADStudio { selected_after: self.tabs[i].scene.selected.iter().copied().collect(), dirty_before: pending.dirty_before, dirty_after: self.tabs[i].dirty, + active_layer: None, structure: Some(StructureSnapshot::Objects(entries)), label: pending.label, }; @@ -629,6 +639,7 @@ impl OpenCADStudio { selected_after: self.tabs[i].scene.selected.iter().copied().collect(), dirty_before, dirty_after: true, + active_layer: None, structure: Some(StructureSnapshot::Styles { before, after, @@ -754,6 +765,7 @@ impl OpenCADStudio { selected_after, dirty_before: pending.dirty_before, dirty_after, + active_layer: None, structure, label: pending.label, }; @@ -908,6 +920,9 @@ impl OpenCADStudio { } else { d.current_layout_after.clone() }; + if let Some((before, after)) = &d.active_layer { + self.tabs[i].active_layer = if undo { before } else { after }.clone(); + } self.tabs[i].dirty = dirty; changes } @@ -1055,7 +1070,7 @@ impl OpenCADStudio { structure, StructureSnapshot::Full(_) | StructureSnapshot::Layers(_) ) - }) || d.entities.iter().any(|(_, before, after)| { + }) || d.active_layer.is_some() || d.entities.iter().any(|(_, before, after)| { before .as_deref() .is_some_and(|entity| matches!(entity, EntityType::Viewport(_))) @@ -1110,7 +1125,7 @@ impl OpenCADStudio { structure, StructureSnapshot::Full(_) | StructureSnapshot::Layers(_) ) - }) || d.entities.iter().any(|(_, before, after)| { + }) || d.active_layer.is_some() || d.entities.iter().any(|(_, before, after)| { before .as_deref() .is_some_and(|entity| matches!(entity, EntityType::Viewport(_))) diff --git a/src/app/update/command.rs b/src/app/update/command.rs index 4c039ccd2..d9bc73614 100644 --- a/src/app/update/command.rs +++ b/src/app/update/command.rs @@ -1103,60 +1103,14 @@ pub(super) fn on_tab_close(&mut self, idx: usize) -> Task { .get(idx) .map(|l| l.name.clone()) .unwrap_or_default(); - let old_key = old_name.to_uppercase(); - let case_only_rename = new_name.to_uppercase() == old_key; - if !new_name.is_empty() - && new_name != old_name - && (case_only_rename - || !self.tabs[i].scene.document.layers.contains(&new_name)) - { + if !new_name.is_empty() && new_name != old_name { self.push_undo_snapshot(i, "LAYER RENAME"); - // Keep the whole record (handle, color, linetype, - // lineweight, flags) and only change the name, so the - // renamed layer still has a valid handle and survives a - // DWG save (issue #67). - if let Some(mut nl) = - self.tabs[i].scene.document.layers.remove(&old_name) - { - nl.name = new_name.clone(); - if !nl.handle.is_valid() { - nl.handle = self.tabs[i].scene.document.allocate_handle(); - } - let layer_handle = nl.handle; - let _ = self.tabs[i].scene.document.layers.add(nl); - for e in self.tabs[i].scene.document.entities_mut() { - if e.as_entity().layer().to_uppercase() == old_key { - e.as_entity_mut().set_layer(new_name.clone()); - } - } - self.tabs[i].scene.invalidate_dependency_index(); - - if self.tabs[i].active_layer.to_uppercase() == old_key { - self.tabs[i].active_layer = new_name.clone(); - } - if self.tabs[i] - .scene - .document - .header - .current_layer_name - .to_uppercase() - == old_key - { - self.tabs[i].scene.document.header.current_layer_name = - new_name.clone(); - self.tabs[i].scene.document.header.current_layer_handle = - layer_handle; - } - self.tabs[i].dirty = true; + if !self.tabs[i].rename_layer(&old_name, &new_name) { + self.discard_last_undo_entry(i); } } - let doc_layers = self.tabs[i].scene.document.layers.clone(); - let vp_info = self.tabs[i].scene.viewport_list(); - self.tabs[i] - .layers - .sync_with_viewports(&doc_layers, vp_info); self.tabs[i].layers.edit_buf.clear(); - self.sync_ribbon_layers(); + self.refresh_layer_panel(); } Task::none() } @@ -2727,6 +2681,29 @@ mod layer_rename_tests { ); } + #[test] + fn layer_rename_undo_redo_restores_active_layer() { + let mut app = app_with_editing_layer(); + let i = app.active_tab; + let idx = app.tabs[i].layers.editing.expect("new layer is being edited"); + let old_name = app.tabs[i].layers.layers[idx].name.clone(); + app.tabs[i].layers.selected = Some(idx); + let _ = app.on_layer_set_current(); + + rename_layer(&mut app, &old_name, "Renamed"); + app.undo_active_tab(); + + assert!(app.tabs[i].scene.document.layers.contains(&old_name)); + assert_eq!(app.tabs[i].active_layer, old_name); + assert_eq!(app.ribbon.active_layer, app.tabs[i].active_layer); + + app.redo_active_tab(); + + assert!(app.tabs[i].scene.document.layers.contains("Renamed")); + assert_eq!(app.tabs[i].active_layer, "Renamed"); + assert_eq!(app.ribbon.active_layer, "Renamed"); + } + #[test] fn layer_rename_keeps_independent_current_layer_mirrors() { let mut app = app_with_editing_layer(); diff --git a/src/scene/entity.rs b/src/scene/entity.rs index 4a1b5f2f0..62ed52545 100644 --- a/src/scene/entity.rs +++ b/src/scene/entity.rs @@ -267,6 +267,14 @@ impl Scene { handle } + pub fn rename_layer(&mut self, old: &str, new: &str) -> bool { + if self.document.rename_layer(old, new).is_err() { + return false; + } + self.invalidate_dependency_index(); + true + } + /// Rename a block definition: re-key its record, update the Block marker's /// name, and repoint every INSERT that referenced the old name so all /// instances keep resolving. Returns false if `old` is missing or diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 406ca0738..4403fdbd8 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -149,6 +149,7 @@ use acadrust::entities::{ BoundaryEdge, BoundaryPath, Hatch as DxfHatch, PolylineEdge, Solid as DxfSolid, }; use acadrust::objects::ObjectType; +use acadrust::tables::normalize_name; use acadrust::types::Vector2; use acadrust::{CadDocument, EntityType, Handle, TableEntry}; use glam; @@ -8591,7 +8592,7 @@ impl Scene { .document .block_records .iter() - .map(|record| (record.handle, record.name.to_ascii_uppercase())) + .map(|record| (record.handle, normalize_name(&record.name))) .collect(); let membership: HashMap = self .document @@ -8618,7 +8619,7 @@ impl Scene { let EntityType::Insert(insert) = entity else { continue; }; - let target = insert.block_name.to_ascii_uppercase(); + let target = normalize_name(&insert.block_name); let common = &insert.common; let owner = if common.owner_handle.is_null() { membership @@ -8702,7 +8703,7 @@ impl Scene { extend_category(&mut index.annotation_geometry); } let add = |map: &mut HashMap, name: &str| { - let target = map.entry(name.to_ascii_uppercase()).or_default(); + let target = map.entry(normalize_name(name)).or_default(); target.render_handles.extend(render_handles.iter().copied()); target.source_handles.insert(common.handle); target.touches_block_definition |= inside_block; @@ -8841,7 +8842,7 @@ impl Scene { }; let mut combined = DependencyTargets::default(); for name in names { - let Some(target) = map.get(&name.to_ascii_uppercase()) else { + let Some(target) = map.get(&normalize_name(name)) else { continue; }; combined