collective
Browse / github.com/storytold/cadcraft / patch
test(io): expand attribute justification round-trip coverage
test(io): expand attribute justification round-trip coverage ## Change Add the 210-case CC-003 regression matrix: TEXT/ATTRIB/ATTDEF, left/center/right, attribute field lengths 0/1/3/80, TEXT group 73 missing or 0/1/2/3/80, group 74 missing or 0–3, and DXF save/reopen. Assertions cover alignment points and values as well as justification. ## Dependencies and overlap - Existing #910 reader fix is preserved as original commit 533875e. Its merge adapts the existing fix to current attdef/insert_attribs helpers; this is not a competing reader implementation. Merge #910 first or retain equivalent behavior. Credit its original author and PR contributor @a
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- smacdmac101 (imported from pull request #2224, open)
- Base
d864a08b013353143086bd8bb568c26cb6256796- Size
- +108 −9 in 8 files
- Review
- pending
- Shared
- 2026-10-28
- Id
7fba442fb46e2d793d16e3a2
How it went
No other operator has reported using it yet.
Diff
raw · crates/io/src/dxf_read/entities.rs crates/io/src/dxf_read/text.rs crates/io/tests/attrib_valign.rs crates/io/tests/it/attribute_vertical_alignment.rs crates/io/tests/it/main.rs docs/file-format-parity.md xtask/src/parity.rs xtask/src/stats.rs
@@ -129,7 +129,7 @@ pub(super) fn entity_with_table_budget(kind: &str, tags: &[Tag], table_cells_lef "HELIX" => EntityKind::Helix(helix(tags)), "RAY" => EntityKind::Ray(RayLine { base: t.p(10), dir: t.p(11) }), "XLINE" => EntityKind::XLine(RayLine { base: t.p(10), dir: t.p(11) }),- "TEXT" => EntityKind::Text(text_from(&t)),+ "TEXT" => EntityKind::Text(text_from(&t, false)), "MTEXT" => EntityKind::MText(mtext_from(tags, &c)), "ATTDEF" => EntityKind::AttDef(attdef(tags, &c)), "INSERT" => {@@ -527,7 +527,7 @@ pub(super) fn insert_attribs(recs: &[(String, Vec<Tag>)], i: &mut usize, ins_lay let (own, embedded) = split_embedded(t2); let multiline = multiline_attrib(embedded, &common(&tt)); let own = T(own);- let mut text = text_from(&own);+ let mut text = text_from(&own, true); if let Some(m) = &multiline { text.value.clone_from(&m.read_as); }@@ -87,7 +87,9 @@ pub(super) fn mtext_from(tags: &[Tag], c: &Common) -> MText { } } -pub(super) fn text_from(t: &T) -> Text {+/// A TEXT, or with `attrib` an ATTRIB / ATTDEF (whose group 73 is the field length: their vertical+/// justification is group 74).+pub(super) fn text_from(t: &T, attrib: bool) -> Text { let h = match t.i(72).unwrap_or(0) { 1 => HAlign::Center, 2 => HAlign::Right,@@ -96,7 +98,7 @@ pub(super) fn text_from(t: &T) -> Text { 5 => HAlign::Fit, _ => HAlign::Left, };- let v = match t.i(73).or_else(|| t.i(74)).unwrap_or(0) {+ let v = match t.i(if attrib { 74 } else { 73 }).unwrap_or(0) { 1 => VAlign::Bottom, 2 => VAlign::Middle, 3 => VAlign::Top,@@ -158,7 +160,7 @@ pub(super) fn attdef(tags: &[Tag], c: &Common) -> Attrib { let (own, embedded) = split_embedded(tags); let multiline = multiline_attrib(embedded, c); let own = T(own);- let mut text = text_from(&own);+ let mut text = text_from(&own, true); if let Some(m) = &multiline { text.value.clone_from(&m.read_as); }new file mode 100644@@ -0,0 +1,32 @@+//! ATTRIB and ATTDEF keep their vertical justification in group 74 (group 73 is the field length).++use cadcraft_doc::{EntityKind, VAlign};++fn attrib(kind: &str) -> String {+ format!(" 0\n{kind}\n 8\n0\n 10\n1\n 20\n1\n 30\n0\n 40\n1.4\n 1\n3\n 2\nVAL\n 70\n0\n 72\n1\n 11\n1\n 21\n1\n 31\n0\n 73\n0\n 74\n2\n")+}++#[test]+fn attribute_vertical_justification_is_group_74() {+ let dxf = format!(+ " 0\nSECTION\n 2\nENTITIES\n{} 0\nINSERT\n 8\n0\n 2\nB\n 66\n1\n 10\n0\n 20\n0\n 30\n0\n{} 0\nSEQEND\n 0\nENDSEC\n 0\nEOF\n",+ attrib("ATTDEF"),+ attrib("ATTRIB")+ );+ let d = cadcraft_io::read_dxf(dxf.as_bytes()).unwrap();+ let mut seen = 0;+ for e in d.model.iter() {+ match &e.kind {+ EntityKind::AttDef(a) => {+ assert_eq!(a.text.valign, VAlign::Middle);+ seen += 1;+ }+ EntityKind::Insert(i) => {+ assert_eq!(i.attribs.first().map(|a| a.text.valign), Some(VAlign::Middle));+ seen += 1;+ }+ _ => {}+ }+ }+ assert_eq!(seen, 2);+}new file mode 100644@@ -0,0 +1,47 @@+//! Attribute group 73 is field length, whereas TEXT uses it for vertical alignment.+use cadcraft_doc::{EntityKind, HAlign, VAlign};+use cadcraft_io::{read_dxf, write_dxf};++#[test]+fn imported_attributes_use_group_74_and_keep_the_alignment_point() {+ for kind in ["ATTRIB", "ATTDEF", "TEXT"] {+ for (horizontal, expected_h) in [(0, HAlign::Left), (1, HAlign::Center), (2, HAlign::Right)] {+ let group_73: &[Option<i32>] =+ if kind == "TEXT" { &[None, Some(0), Some(1), Some(2), Some(3), Some(80)] } else { &[Some(0), Some(1), Some(3), Some(80)] };+ for &field_length in group_73 {+ for vertical in [None, Some(0), Some(1), Some(2), Some(3)] {+ let field = field_length.map_or(String::new(), |v| format!("73\n{v}\n"));+ let v = vertical.map_or(String::new(), |v| format!("74\n{v}\n"));+ let prefix = if kind == "ATTRIB" { "0\nINSERT\n2\nTEST\n66\n1\n" } else { "" };+ let suffix = if kind == "ATTRIB" { "0\nSEQEND\n" } else { "" };+ let source = format!(+ "0\nSECTION\n2\nENTITIES\n{prefix}0\n{kind}\n10\n1\n20\n2\n40\n1\n1\nValue\n2\nTAG\n72\n{horizontal}\n{field}{v}11\n7\n21\n9\n{suffix}0\nENDSEC\n0\nEOF\n"+ );+ let drawing = read_dxf(source.as_bytes()).expect("import synthetic DXF");+ let expected = match if kind == "TEXT" { field_length.unwrap_or(0) } else { vertical.unwrap_or(0) } {+ 1 => VAlign::Bottom,+ 2 => VAlign::Middle,+ 3 => VAlign::Top,+ _ => VAlign::Baseline,+ };+ for d in [&drawing, &read_dxf(write_dxf(&drawing).as_bytes()).expect("reopen")] {+ let entity = d.model.iter().next().expect("entity");+ let text = match &entity.kind {+ EntityKind::Insert(i) => &i.attribs.first().expect("attribute").text,+ EntityKind::AttDef(a) => &a.text,+ EntityKind::Text(t) => t,+ other => panic!("unexpected {other:?}"),+ };+ assert_eq!(text.valign, expected, "{kind} 73={field_length:?} 74={vertical:?}");+ assert_eq!(text.halign, expected_h);+ assert_eq!(+ text.align_pt.map(|p| (p.x, p.y, p.z)),+ (horizontal != 0 || expected != VAlign::Baseline).then_some((7.0, 9.0, 0.0))+ );+ assert_eq!(text.value, "Value");+ }+ }+ }+ }+ }+}@@ -6,6 +6,7 @@ mod annotative_roundtrip; mod assoc_array_roundtrip; mod attrib_modes; mod attrib_props;+mod attribute_vertical_alignment; mod block_expansion; mod closed_spline; mod codepages;@@ -1,6 +1,6 @@ # File-format parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-22 · **Change:** minor (DIMASSOC objects for angular, diametric and radial dimensions) · **Target:** Autodesk AutoCAD 2027+> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-26 · **Change:** minor (attribute alignment regression matrix) · **Target:** Autodesk AutoCAD 2027 Every format AutoCAD 2027 reads or writes, with CADCraft's support, fidelity and tests. Work items are in [gaps.md](gaps.md#file-formats); the weighted score feeds@@ -136,10 +136,25 @@ external validator runs in CI (ezdxf's audit was run by hand once, 2026-10-06). a licensed real-file corpus with an open → save → re-open → compare test per file, and checking our output in AutoCAD black-box. +## Attribute alignment regression coverage++CC-003 covers 210 synthetic TEXT/ATTRIB/ATTDEF combinations across left/center/right alignment,+attribute field lengths 0/1/3/80, TEXT group 73 missing or 0/1/2/3/80, and group 74 missing or 0–3. Each combination is+checked after import and DXF save/reopen, including the retained alignment point and value.+This contribution adapts the existing #910 reader correction to the current reader helpers+and adds regression coverage. The original reader correction remains a separately credited+prerequisite; the new matrix can be reverted independently.+It reproduced ATTRIB group 73=0 / 74=1 being read as Baseline instead of Bottom on d3ae8a6,+and passes with #910 adapted to the current extracted reader helpers. No private or+AutoCAD-produced drawing is included. This does not establish full drawing fidelity or cover+multiline subclass collisions (the separate #1942 proposal).+ ## Revision history | Date | Change | Summary | |---|---|---|+| 2026-10-26 | minor | CC-003 review follow-up: extend the matrix to 210 combinations with TEXT Middle and omitted group 73; no production change |+| 2026-10-26 | minor | CC-003 attribute alignment: adapted #910 reader correction plus 180 import/save/reopen combinations; clarified prerequisite scope after review | | 2026-10-22 | minor | DXF/DWG save: angular, diametric and radial dimensions get standard DIMASSOC objects (one centre reference to the circle for radial/diametric, the definition points for angular), so AutoCAD follows their geometry; the standard objects of those kinds are read back too, without CADCraft's xdata (#1651) | | 2026-10-19 | minor | DXF/DWG save: angular dimension block text honors DIMDSEP, including per-dimension overrides (#1646) | | 2026-10-19 | minor | A file's own arrowhead blocks with standard names (AutoCAD's `_Dot`, `_DotSmall`), named only by a dimension style, are kept and saved as they were instead of being replaced by CADCraft's solids; PURGE keeps arrowhead blocks dimension styles name |@@ -136,7 +136,9 @@ mod tests { assert_eq!(day(20_736), "2026-10-10"); assert_eq!(date(std::time::UNIX_EPOCH - std::time::Duration::from_secs(86_400)), "1970-01-01"); // Past chrono's range keeps the proleptic Gregorian date, not the epoch (old `civil_from_days`).- assert_eq!(day(115_740_740), "318857-05-20");+ // Checked on the day count: Windows' SystemTime ends in the year 30828, so building that+ // SystemTime panics there.+ assert_eq!(civil_from_days(115_740_740), (318_857, 5, 20)); // Windows' SystemTime can't hold u64::MAX / 2 seconds, so it is only checked where it exists. if let Some(far) = std::time::UNIX_EPOCH.checked_add(std::time::Duration::from_secs(u64::MAX / 2)) { assert_eq!(date(far), "292277026596-12-04");@@ -134,7 +134,7 @@ pub fn run(_root: &Path, exact: bool) -> Result<(), String> { #[cfg(test)] mod tests {- use super::{Counts, count_tests, walk};+ use super::{Counts, count_tests}; #[test] fn counts_attributes_and_proptest_blocks() {@@ -155,7 +155,7 @@ mod tests { // A directory symlink pointing back at `real` would be re-scanned if followed. symlink(&real, base.join("alias")).unwrap(); let mut c = Counts::default();- walk(&base, &mut c).unwrap();+ super::walk(&base, &mut c).unwrap(); assert_eq!(c.files, 1, "the aliased directory was followed and counted twice"); std::fs::remove_dir_all(&base).ok(); }