forked from 1b-agent/opencadstudio-upstream
fix(layers): allocate real handles for new layers (#67)
New layers were added to the table with a NULL handle, and the first patch used the non-advancing `next_handle()` getter, so two new layers (or a layer and the next allocated object) shared one handle. The DWG format is handle-based, so the collision dropped the layer on save; DXF is name-based and survived, which is why a rename appeared to fix it. Allocate a unique advancing handle via `allocate_handle()` in LAYER NEW (panel + command line) and on rename when the source handle is invalid. Rename now also clones the whole record so colour/linetype/lineweight are kept, not just the flags. Adds DWG/DXF round-trip tests covering one and multiple new layers. Co-Authored-By: Claude Opus 4.8 <[email protected]>
This commit is contained in:
1 parent
ff10f8a89a
commit
6520195220
3 files changed
+64
-11
No files matched your search
+4
-1
@@ -3288,7 +3288,10 @@ impl OpenCADStudio {
|
||||
self.command_line
|
||||
.push_error(&format!("LAYER: '{}' already exists.", name));
|
||||
} else {
|
||||
let layer = Layer::new(&name);
|
||||
let mut layer = Layer::new(&name);
|
||||
// Allocate a unique handle so the layer survives a
|
||||
// DWG save (handle-based format; issue #67).
|
||||
layer.handle = self.tabs[i].scene.document.allocate_handle();
|
||||
let _ = self.tabs[i].scene.document.layers.add(layer);
|
||||
self.push_undo_snapshot(i, "LAYER NEW");
|
||||
self.tabs[i].dirty = true;
|
||||
|
||||
+19
-10
@@ -1637,11 +1637,14 @@ impl OpenCADStudio {
|
||||
};
|
||||
self.push_undo_snapshot(i, "LAYER NEW");
|
||||
use acadrust::tables::layer::Layer as DocLayer;
|
||||
let _ = self.tabs[i]
|
||||
.scene
|
||||
.document
|
||||
.layers
|
||||
.add(DocLayer::new(&new_name));
|
||||
// A layer needs a real handle or it is dropped on a DWG save
|
||||
// (the format is handle-based; issue #67).
|
||||
let mut dl = DocLayer::new(&new_name);
|
||||
// `allocate_handle` advances the seed so the layer gets a
|
||||
// unique handle; the non-advancing `next_handle` getter hands
|
||||
// out the same value twice and the later object overwrites it.
|
||||
dl.handle = self.tabs[i].scene.document.allocate_handle();
|
||||
let _ = self.tabs[i].scene.document.layers.add(dl);
|
||||
self.tabs[i].dirty = true;
|
||||
let doc_layers = self.tabs[i].scene.document.layers.clone();
|
||||
let vp_info = self.tabs[i].scene.viewport_list();
|
||||
@@ -1742,11 +1745,17 @@ impl OpenCADStudio {
|
||||
&& !self.tabs[i].scene.document.layers.contains(&new_name)
|
||||
{
|
||||
self.push_undo_snapshot(i, "LAYER RENAME");
|
||||
if let Some(old_layer) = self.tabs[i].scene.document.layers.get(&old_name) {
|
||||
use acadrust::tables::layer::Layer as DocLayer;
|
||||
let mut nl = DocLayer::new(&new_name);
|
||||
nl.color = old_layer.color.clone();
|
||||
nl.flags = old_layer.flags.clone();
|
||||
// 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.get(&old_name).cloned()
|
||||
{
|
||||
nl.name = new_name.clone();
|
||||
if !nl.handle.is_valid() {
|
||||
nl.handle = self.tabs[i].scene.document.allocate_handle();
|
||||
}
|
||||
let _ = self.tabs[i].scene.document.layers.add(nl);
|
||||
}
|
||||
self.tabs[i].scene.document.layers.remove(&old_name);
|
||||
|
||||
@@ -403,3 +403,44 @@ fn fix_dxf_dimension_rotations(doc: &mut CadDocument) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod layer_roundtrip_tests {
|
||||
use super::*;
|
||||
use acadrust::tables::layer::Layer as DocLayer;
|
||||
|
||||
// Add `count` new layers the way the UI does (allocate_handle, then add),
|
||||
// round-trip through `ext`, and return whether every one survived.
|
||||
fn roundtrip_layers(ext: &str, count: usize) -> bool {
|
||||
let mut doc = CadDocument::new();
|
||||
crate::linetypes::populate_document(&mut doc);
|
||||
let names: Vec<String> = (0..count).map(|n| format!("Layer{}", n + 1)).collect();
|
||||
for name in &names {
|
||||
let mut dl = DocLayer::new(name);
|
||||
dl.handle = doc.allocate_handle();
|
||||
doc.layers.add(dl).unwrap();
|
||||
}
|
||||
let path = std::env::temp_dir().join(format!("ocs_layer_rt_{count}.{ext}"));
|
||||
save_as_version(&doc, &path, acadrust::DxfVersion::AC1032).expect("save");
|
||||
let loaded = load_file(&path).expect("load");
|
||||
let _ = std::fs::remove_file(&path);
|
||||
names.iter().all(|n| loaded.layers.contains(n))
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dwg_preserves_new_layer() {
|
||||
assert!(roundtrip_layers("dwg", 1), "DWG dropped the new layer (issue #67)");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dxf_preserves_new_layer() {
|
||||
assert!(roundtrip_layers("dxf", 1), "DXF dropped the new layer");
|
||||
}
|
||||
|
||||
// Each new layer must get a distinct handle, or they collide and all but
|
||||
// the last are dropped on a handle-based DWG save (issue #67).
|
||||
#[test]
|
||||
fn dwg_preserves_multiple_new_layers() {
|
||||
assert!(roundtrip_layers("dwg", 3), "DWG dropped colliding new layers (issue #67)");
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user