DXF: reopen-and-save keeps one *D block per dimension and stable handles (fixes #495) (#508)

Squash of PR #508 by @vangel76.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
Valentin KleinandClaude Opus 5.5 authored and GitHub committed 2026-10-11 11:47:11 -04:00
1 parent 7ecac5fb66
commit 78944a8341
6 files changed
+297 -18

No files matched your search

+4 -2
View File
@@ -5,8 +5,8 @@
//! constraints).
//!
//! CADCraft's own data lives under the registered application `CADCRAFT` (xdata) and the
//! named-object-dictionary entries `CADCRAFT_CONSTRAINTS`, `CADCRAFT_LAYERSTATES` and
//! `CADCRAFT_PLOTSTYLES` (XRECORDs). Other readers keep or ignore them.
//! named-object-dictionary entries `CADCRAFT_CONSTRAINTS`, `CADCRAFT_LAYERSTATES`,
//! `CADCRAFT_PLOTSTYLES` and `CADCRAFT_HANDSEED` (XRECORDs). Other readers keep or ignore them.
use std::collections::HashMap;
@@ -30,6 +30,8 @@ pub(crate) const ANNOTATION_KEY: &str = "CADCRAFT_ANNOTATION";
/// Named-object-dictionary key of the plot style names dictionary (DXF Reference): its entries
/// name `ACDBPLACEHOLDER` objects, and layers and objects point at them with group 390.
pub(crate) const PLOT_STYLE_NAMES_KEY: &str = "ACAD_PLOTSTYLENAME";
/// Named-object-dictionary key of the XRECORD holding the drawing's next entity handle (hex).
pub(crate) const HANDSEED_KEY: &str = "CADCRAFT_HANDSEED";
/// Most plot style tables and plot style names read from a file.
pub(crate) const MAX_PLOT_STYLE_TABLES: usize = 1000;
pub(crate) const MAX_PLOT_STYLE_NAMES: usize = 100_000;
+4 -1
View File
@@ -1508,6 +1508,9 @@ pub fn read(bytes: &[u8]) -> Result<Drawing> {
_ => {}
}
}
// A file CADCraft wrote names its own next handle (#495): its table records and objects
// are numbered above it and rebuilt on save, so following $HANDSEED would drift every save.
let own_seed = objs.xrecord_text(crate::dxf_ext::HANDSEED_KEY).and_then(|t| Handle::parse_hex(t.trim()));
// Layouts.
let mut paper_block_for: HashMap<String, String> = HashMap::new();
layout_objs.sort_by_key(|l| l.1);
@@ -1588,7 +1591,7 @@ pub fn read(bytes: &[u8]) -> Result<Drawing> {
crate::dxf_image::resolve(&secs, &mut d);
// Keep handles unique against the header's seed.
let seed = d.header.str("HANDSEED", "");
if let Some(h) = Handle::parse_hex(&seed) {
if let Some(h) = own_seed.or_else(|| Handle::parse_hex(&seed)) {
d.bump_handseed(Handle(h.0.saturating_sub(1)));
}
if d.layer("0").is_none() {
+91 -12
View File
@@ -39,7 +39,7 @@ impl W {
}
fn h(&mut self) -> String {
let h = format!("{:X}", self.next);
self.next += 1;
self.next = self.next.saturating_add(1);
h
}
/// A `102 {NAME` … `102 }` group of handles.
@@ -992,6 +992,65 @@ fn hatch_xdata(w: &mut W, h: &Hatch) {
w.s(1000, h.background.map(Color::name).unwrap_or_default());
}
/// What happens to the anonymous `*D` blocks read from a file (#495): a dimension that still
/// names its block regenerates its geometry under that name, keeping the block's entity
/// handles, and `*D` blocks nothing else references are not written again.
struct DimBlockPlan {
/// Dimension (model or paper space) → the read block whose name and handles it reuses.
reuse: HashMap<Handle, std::sync::Arc<Block>>,
/// Upper-case names of `*D` blocks not written as ordinary blocks.
skip: std::collections::HashSet<String>,
}
fn is_dim_block(name: &str) -> bool {
name.get(..2).is_some_and(|p| p.eq_ignore_ascii_case("*D"))
}
fn dim_block_plan(d: &Drawing) -> DimBlockPlan {
let spaces: Vec<&EntityStore> = std::iter::once(&d.model).chain(d.layouts.iter().map(|l| &l.entities)).collect();
// Block names other objects use: inserts and multileader contents anywhere, and dimensions
// inside block definitions (their blocks are written as read).
let mut used = std::collections::HashSet::new();
let note = |e: &Entity, dims: bool, used: &mut std::collections::HashSet<String>| match &e.kind {
EntityKind::Insert(i) => {
used.insert(i.block.to_ascii_uppercase());
}
EntityKind::MLeader(m) => used.extend(m.block.as_ref().map(|b| b.block.to_ascii_uppercase())),
EntityKind::Dimension(dm) if dims => used.extend(dm.block.as_ref().map(|n| n.to_ascii_uppercase())),
_ => {}
};
for e in spaces.iter().flat_map(|s| s.iter()) {
note(e, false, &mut used);
}
for e in d.blocks.values().flat_map(|b| b.entities.iter()) {
note(e, true, &mut used);
}
let skip = d.blocks.keys().filter(|n| is_dim_block(n)).map(|n| n.to_ascii_uppercase()).filter(|n| !used.contains(n)).collect();
let mut plan = DimBlockPlan { reuse: HashMap::new(), skip };
let mut claimed = std::collections::HashSet::new();
for e in spaces.iter().flat_map(|s| s.iter()) {
let EntityKind::Dimension(dm) = &e.kind else { continue };
let Some(b) = dm.block.as_deref().filter(|n| is_dim_block(n)).and_then(|n| d.block(n)) else { continue };
let key = b.name.to_ascii_uppercase();
if plan.skip.contains(&key) && claimed.insert(key) {
plan.reuse.insert(e.handle, b.clone());
}
}
plan
}
/// Give regenerated dimension geometry the handles of the block it replaces, in order; the rest
/// keep handle 0 and get fresh ones.
fn reuse_handles(ents: &mut [Entity], old: &Block, taken: &mut std::collections::HashSet<Handle>) {
let mut handles = old.entities.iter().map(|e| e.handle).filter(|h| h.0 != 0);
for e in ents.iter_mut() {
match handles.by_ref().find(|h| taken.insert(*h)) {
Some(h) => e.handle = h,
None => break,
}
}
}
/// Anonymous dimension blocks (`*D1`…) with the rendered geometry, as consumers expect.
fn dim_block_entities(d: &Drawing, dm: &Dimension, layer: &str) -> Vec<Entity> {
// The dimension as drawn: its overrides, DIMSCALE (0 = the drawing's) and text font.
@@ -1279,7 +1338,7 @@ pub fn write(d: &Drawing) -> String {
/// The drawing's DXF tags, declaring `version` in `$ACADVER`.
pub fn tags(d: &Drawing, version: Version) -> Vec<Tag> {
// Structural handles start above every entity handle.
let mut w = W { t: Vec::new(), next: d.handseed.max(0x100) + 0x1000, pstyles: HashMap::new() };
let mut w = W { t: Vec::new(), next: d.handseed.max(0x100).saturating_add(0x1000), pstyles: HashMap::new() };
// Pre-allocate handles for block records.
let mut paper_layouts: Vec<&Layout> = d.layouts.iter().collect();
paper_layouts.sort_by_key(|l| l.tab_order);
@@ -1290,7 +1349,13 @@ pub fn tags(d: &Drawing, version: Version) -> Vec<Tag> {
.map(|(i, l)| (if i == 0 { "*Paper_Space".to_string() } else { format!("*Paper_Space{}", i - 1) }, String::new(), *l))
.collect();
let ps_brs: Vec<(String, String, &Layout)> = ps_brs.into_iter().map(|(n, _, l)| (n, w.h(), l)).collect();
let mut user_blocks: Vec<(String, String, &Block)> = d.blocks.values().map(|b| (b.name.clone(), String::new(), b.as_ref())).collect();
let dim_plan = dim_block_plan(d);
let mut user_blocks: Vec<(String, String, &Block)> = d
.blocks
.values()
.filter(|b| !dim_plan.skip.contains(&b.name.to_ascii_uppercase()))
.map(|b| (b.name.clone(), String::new(), b.as_ref()))
.collect();
user_blocks.iter_mut().for_each(|b| b.1 = format!("{:X}", 0));
let user_blocks: Vec<(String, String, &Block)> = user_blocks.into_iter().map(|(n, _, b)| (n, w.h(), b)).collect();
let mut cx = Ctx { paper_views: paper_views(d), ..Ctx::default() };
@@ -1298,17 +1363,28 @@ pub fn tags(d: &Drawing, version: Version) -> Vec<Tag> {
// Dimension blocks.
let mut dim_defs: Vec<(String, String, Vec<Entity>)> = Vec::new(); // filled below
let mut n = 1;
let mut reused_handles = std::collections::HashSet::new();
let all_spaces: Vec<&EntityStore> = std::iter::once(&d.model).chain(d.layouts.iter().map(|l| &l.entities)).collect();
for st in &all_spaces {
for e in st.iter() {
if let EntityKind::Dimension(dm) = &e.kind {
let mut name = format!("*D{n}");
while d.block(&name).is_some() {
n += 1;
name = format!("*D{n}");
}
n += 1;
let ents = dim_block_entities(d, dm, &e.common.layer);
let reused = dim_plan.reuse.get(&e.handle);
let mut ents = dim_block_entities(d, dm, &e.common.layer);
let name = match reused {
Some(b) => {
reuse_handles(&mut ents, b, &mut reused_handles);
b.name.clone()
}
None => {
let mut name = format!("*D{n}");
while d.block(&name).is_some() {
n += 1;
name = format!("*D{n}");
}
n += 1;
name
}
};
let brh = w.h();
cx.dim_blocks.insert(e.handle, name.clone());
dim_defs.push((name, brh, ents));
@@ -1403,6 +1479,9 @@ pub fn tags(d: &Drawing, version: Version) -> Vec<Tag> {
let table_style_dict = w.h();
// CADCraft data in XRECORDs of the named object dictionary: (key, handle, text chunks).
let mut xrecords: Vec<(&str, String, Vec<String>)> = Vec::new();
// The drawing's own next handle: table records and objects are numbered above it, so
// reading it back (instead of $HANDSEED) keeps their handles across saves (#495).
xrecords.push((dxf_ext::HANDSEED_KEY, w.h(), vec![format!("{:X}", d.handseed)]));
if let Some(chunks) = dxf_ext::constraint_chunks(&d.constraints, &d.parametric) {
xrecords.push((dxf_ext::CONSTRAINTS_KEY, w.h(), chunks));
}
@@ -1800,9 +1879,9 @@ pub fn tags(d: &Drawing, version: Version) -> Vec<Tag> {
}
// Generated geometry entities (dimension, arrowhead and table blocks) need handles.
for (_, _, ents) in dim_defs.iter_mut().chain(arrow_defs.iter_mut()).chain(table_defs.iter_mut()) {
for e in ents.iter_mut() {
for e in ents.iter_mut().filter(|e| e.handle.0 == 0) {
e.handle = Handle(w.next);
w.next += 1;
w.next = w.next.saturating_add(1);
}
}
for (n, h, ents) in &dim_defs {
@@ -0,0 +1,195 @@
//! Reopening and saving a drawing keeps one anonymous `*D` block per dimension and the same
//! handles: the block read with a dimension is regenerated under its own name instead of being
//! kept next to a fresh copy, `*D` blocks nothing references are dropped, and table records and
//! objects keep their handles because CADCraft's own next handle is saved with the drawing
//! (issue #495, gaps FF7).
use std::sync::Arc;
use cadcraft_doc::{Block, Common, DimKind, Dimension, Drawing, Entity, EntityKind, Handle, Insert, Line, Space};
use cadcraft_geom::Vec3;
fn linear(a: [f64; 2], b: [f64; 2], at: [f64; 2]) -> Dimension {
Dimension {
kind: DimKind::Linear { rotation: 0.0 },
defpt: Vec3::new(at[0], at[1], 0.0),
text_mid: Vec3::ZERO,
p13: Vec3::new(a[0], a[1], 0.0),
p14: Vec3::new(b[0], b[1], 0.0),
p15: Vec3::ZERO,
p16: Vec3::ZERO,
text: String::new(),
style: "Standard".into(),
measurement: 0.0,
text_rotation: 0.0,
user_text_pos: false,
block: None,
overrides: Default::default(),
assoc: Vec::new(),
}
}
fn insert(block: &str) -> Insert {
Insert {
block: block.into(),
insert: Vec3::ZERO,
scale: Vec3::new(1.0, 1.0, 1.0),
rotation: 0.0,
attribs: Vec::new(),
cols: 1,
rows: 1,
col_spacing: 0.0,
row_spacing: 0.0,
}
}
/// (code, value) pairs of an ASCII DXF.
fn pairs(dxf: &str) -> Vec<(i32, String)> {
let lines: Vec<&str> = dxf.lines().collect();
lines.chunks(2).filter_map(|c| Some((c.first()?.trim().parse().ok()?, c.get(1)?.trim().to_string()))).collect()
}
/// Names of the BLOCK definitions and of the blocks DIMENSIONs and INSERTs reference.
fn blocks_and_refs(dxf: &str) -> (Vec<String>, Vec<String>) {
let p = pairs(dxf);
let (mut blocks, mut refs) = (Vec::new(), Vec::new());
let mut kind = String::new();
for (code, v) in p {
if code == 0 {
kind = v;
} else if code == 2 && kind == "BLOCK" {
blocks.push(v);
} else if code == 2 && (kind == "DIMENSION" || kind == "INSERT") {
refs.push(v);
}
}
(blocks, refs)
}
fn dim_blocks(dxf: &str) -> Vec<String> {
let mut v: Vec<String> = blocks_and_refs(dxf).0.into_iter().filter(|n| n.starts_with("*D")).collect();
v.sort();
v
}
fn resave(dxf: &str) -> String {
cadcraft_io::write_dxf(&cadcraft_io::read_dxf(dxf.as_bytes()).unwrap())
}
fn check_cycles(d: &Drawing, dims: usize) {
let d0 = cadcraft_io::write_dxf(d);
let d1 = resave(&d0);
let d2 = resave(&d1);
let d3 = resave(&d2);
for (i, f) in [&d0, &d1, &d2, &d3].into_iter().enumerate() {
let (_, refs) = blocks_and_refs(f);
let blocks = dim_blocks(f);
assert_eq!(blocks.len(), dims, "cycle {i}: {blocks:?}");
assert!(blocks.iter().all(|b| refs.contains(b)), "cycle {i}: unreferenced {blocks:?} / {refs:?}");
let back = cadcraft_io::read_dxf(f.as_bytes()).unwrap();
let n = back.model.iter().filter(|e| matches!(e.kind, EntityKind::Dimension(_))).count();
assert_eq!(n, dims, "cycle {i}");
}
// Same names, same handles, same bytes once the drawing has been through one save.
assert_eq!(dim_blocks(&d1), dim_blocks(&d0));
assert_eq!(d2, d1, "second reopen-and-save changed the file");
assert_eq!(d3, d2, "third reopen-and-save changed the file");
}
#[test]
fn one_dimension_keeps_one_block_over_three_cycles() {
let mut d = Drawing::new_metric();
d.add(&Space::Model, Common::default(), EntityKind::Dimension(linear([0.0, 0.0], [10.0, 0.0], [5.0, 3.0]))).unwrap();
check_cycles(&d, 1);
}
#[test]
fn two_dimensions_and_a_user_block_keep_their_blocks() {
let mut d = Drawing::new_metric();
d.add(&Space::Model, Common::default(), EntityKind::Dimension(linear([0.0, 0.0], [10.0, 0.0], [5.0, 3.0]))).unwrap();
d.add(&Space::Model, Common::default(), EntityKind::Dimension(linear([0.0, 5.0], [20.0, 5.0], [10.0, 9.0]))).unwrap();
let mut b = Block::new("Mark");
let h = d.new_handle();
b.entities.push(Entity::new(h, EntityKind::Line(Line { a: Vec3::ZERO, b: Vec3::new(1.0, 1.0, 0.0) })));
d.blocks.insert("Mark".into(), Arc::new(b));
let unused = Block::new("Spare");
d.blocks.insert("Spare".into(), Arc::new(unused));
let ins = insert("Mark");
d.add(&Space::Model, Common::default(), EntityKind::Insert(ins)).unwrap();
check_cycles(&d, 2);
let back = cadcraft_io::read_dxf(resave(&cadcraft_io::write_dxf(&d)).as_bytes()).unwrap();
assert!(back.block("Mark").is_some_and(|b| b.entities.len() == 1));
assert!(back.block("Spare").is_some(), "an unused user block is not purged");
}
#[test]
fn stale_dimension_blocks_are_dropped_and_referenced_ones_kept() {
let mut d = Drawing::new_metric();
d.add(&Space::Model, Common::default(), EntityKind::Dimension(linear([0.0, 0.0], [10.0, 0.0], [5.0, 3.0]))).unwrap();
// What older versions left behind: copies nothing references, and one an INSERT uses.
for name in ["*D7", "*D8"] {
let mut b = Block::new(name);
let h = d.new_handle();
b.entities.push(Entity::new(h, EntityKind::Line(Line { a: Vec3::ZERO, b: Vec3::new(2.0, 0.0, 0.0) })));
d.blocks.insert(name.into(), Arc::new(b));
}
d.add(&Space::Model, Common::default(), EntityKind::Insert(insert("*D8"))).unwrap();
let out = cadcraft_io::write_dxf(&d);
let blocks = dim_blocks(&out);
assert!(!blocks.contains(&"*D7".to_string()), "{blocks:?}");
assert!(blocks.contains(&"*D8".to_string()), "{blocks:?}");
assert_eq!(blocks.len(), 2, "{blocks:?}");
}
#[test]
fn reopened_dimension_reuses_its_block_name_and_handles() {
let mut d = Drawing::new_metric();
let h = d.add(&Space::Model, Common::default(), EntityKind::Dimension(linear([0.0, 0.0], [10.0, 0.0], [5.0, 3.0]))).unwrap();
let back = cadcraft_io::read_dxf(cadcraft_io::write_dxf(&d).as_bytes()).unwrap();
let Some(EntityKind::Dimension(dm)) = back.entity(h).map(|e| &e.kind) else { panic!("dimension lost") };
let name = dm.block.clone().unwrap();
let before: Vec<Handle> = back.block(&name).unwrap().entities.iter().map(|e| e.handle).collect();
let again = cadcraft_io::read_dxf(cadcraft_io::write_dxf(&back).as_bytes()).unwrap();
let Some(EntityKind::Dimension(dm2)) = again.entity(h).map(|e| &e.kind) else { panic!("dimension lost") };
assert_eq!(dm2.block.as_deref(), Some(name.as_str()));
let after: Vec<Handle> = again.block(&name).unwrap().entities.iter().map(|e| e.handle).collect();
assert_eq!(after, before);
assert_eq!(again.handseed, back.handseed);
}
#[test]
fn hostile_handle_seed_does_not_panic() {
let mut d = Drawing::new_metric();
d.add(&Space::Model, Common::default(), EntityKind::Dimension(linear([0.0, 0.0], [10.0, 0.0], [5.0, 3.0]))).unwrap();
let p = pairs(&cadcraft_io::write_dxf(&d));
// The dictionary entry names the XRECORD; its group 1 holds the seed.
let key = p.iter().position(|(c, v)| *c == 3 && v == "CADCRAFT_HANDSEED").unwrap();
let h = p[key + 1].1.clone();
let rec = p.iter().position(|(c, v)| *c == 5 && *v == h).unwrap();
let at = rec + p[rec..].iter().position(|(c, _)| *c == 1).unwrap();
assert_eq!(p[at].1, format!("{:X}", d.handseed));
for bad in ["FFFFFFFFFFFFFFFF", "not-hex", "", "-1"] {
let mut q = p.clone();
q[at].1 = bad.to_string();
let text: String = q.iter().map(|(c, v)| format!("{c}\r\n{v}\r\n")).collect();
let back = cadcraft_io::read_dxf(text.as_bytes()).unwrap();
let again = cadcraft_io::write_dxf(&back);
assert_eq!(dim_blocks(&again).len(), 1, "{bad}");
}
}
#[test]
fn dwg_reopen_and_save_keeps_one_block_per_dimension() {
let mut d = Drawing::new_metric();
d.add(&Space::Model, Common::default(), EntityKind::Dimension(linear([0.0, 0.0], [10.0, 0.0], [5.0, 3.0]))).unwrap();
let mut bytes = cadcraft_io::write(&d, "a.dwg").unwrap();
let mut saves = Vec::new();
for _ in 0..3 {
let back = cadcraft_io::read(&bytes, "a.dwg").unwrap();
let dims: Vec<String> = back.blocks.keys().filter(|n| n.starts_with("*D")).cloned().collect();
assert_eq!(dims.len(), 1, "{dims:?}");
bytes = cadcraft_io::write(&back, "a.dwg").unwrap();
saves.push(bytes.clone());
}
assert!(saves.windows(2).all(|w| w[0] == w[1]), "DWG reopen-and-save changed the file");
}
+2 -2
View File
@@ -73,7 +73,7 @@ early (#175), but the throughput problem remains. **DWG ≈ 30% read, ≈ 15% wr
| LINE, POINT, CIRCLE, ARC, ELLIPSE, LWPOLYLINE (widths, bulges, elevation), SPLINE, RAY, XLINE | yes | yes | round-trip tests |
| POLYLINE 2D/3D | yes | yes (as LWPOLYLINE / 3D POLYLINE) | polyface and polygon meshes flattened: lossy |
| TEXT, MTEXT, ATTDEF, INSERT + ATTRIB | yes | yes | ATTDEF prompt kept (#137) |
| DIMENSION (all 7 kinds) | yes | yes, with anonymous `*D` blocks | `*D` blocks duplicate on re-save (known); DIMASSOC objects written |
| DIMENSION (all 7 kinds) | yes | yes, with anonymous `*D` blocks | a reopened dimension's `*D` block is regenerated under its name, unreferenced ones dropped (#495); DIMASSOC objects written |
| LEADER | yes | yes | |
| MULTILEADER | **no** (becomes Unknown) | **no**: our MLeaders are written as LEADER + MTEXT | MULTILEADER is AutoCAD's default leader since 2008 |
| HATCH pattern / solid | yes | yes | |
@@ -90,7 +90,7 @@ early (#175), but the throughput problem remains. **DWG ≈ 30% read, ≈ 15% wr
| Objects: LAYOUT, DICTIONARY, XRECORD, TABLESTYLE, DIMASSOC | yes | yes | |
| Objects: GROUP, MLINESTYLE, MLEADERSTYLE, IMAGEDEF, SCALE, VISUALSTYLE, PLOTSETTINGS, materials, field objects | ignored | not written (ACAD_GROUP empty) | groups lost |
| Foreign xdata, extension dictionaries, reactors | **not kept** | — | third-party application data lost |
| Handles and owners | regenerated | consistent | handles change on every save |
| Handles and owners | entity handles kept; others regenerated | consistent | stable across CADCraft re-saves (#495); other programs' table and object handles renumbered once |
**DXF ≈ 50% read, ≈ 30% write** (estimated; weights: common 2D entities 50%, annotation objects
20%, tables/objects 15%, foreign data preservation 15%).
+1 -1
View File
@@ -83,7 +83,7 @@ Detail: [file-format-parity.md](file-format-parity.md).
| FF4 | Code pages (`$DWGCODEPAGE`, ANSI_932/936/949/950/125x) when reading pre-2007 files | Medium-high (Asia, Eastern Europe) | 3–5 |
| FF5 | **[alpha blocker]** Real-file corpus (`storytold/cadcraft-corpus`), open → save → reopen tests, black-box check in AutoCAD | High | 10–15 + human |
| FF6 | **[alpha blocker]** DWG throughput: a 10 MB DWG took ~2 min to fail (#57) | High for real projects | 6–10 |
| FF7 | `*D` dimension blocks duplicate on re-save; handles regenerate every save | Medium | 3–5 |
| FF7 | Table record and object handles from other programs' files are renumbered on the first save (entity handles are kept; `*D` blocks no longer duplicate and once reopened, CADCraft's own files keep every handle across saves, #495) | Low | 4–6 |
| FF8 | SHX fonts (shape and big fonts) not read | Medium-high (text metrics differ) | 8–12 |
| FF9 | Templates (DWT), user `.lin`/`.pat` files, `.ctb`/`.stb`, `.pc3`, `.pgp` | Medium | 20–30 |
| FF10 | PDF: text as real fonts, layers (OCG), multi-sheet; PDF import and underlays; raster image attach | Medium | 30–50 |