From 2d2be155dd97212ecc3bb81dec0fe31bcb15d123 Mon Sep 17 00:00:00 2001 From: smacdmac101 <152641296+smacdmac101@users.noreply.github.com> Date: Thu, 29 Oct 2026 21:38:15 -0400 Subject: [PATCH] fix(engine): refresh attribute baselines after value changes (#2214) * xtask: the test suite and clippy pass on Windows Two Windows-only failures stopped `cargo xtask ci` before it got going on a Windows checkout (CI runs the gate on Linux only, so neither showed): - parity::tests::civil_dates built a SystemTime 115,740,740 days after the epoch (year 318857). Windows' SystemTime ends in the year 30828, so the addition panicked ("overflow when adding duration to `SystemTime`"). That case now checks `civil_from_days` on the day count, which is what it was about, on every platform. - stats.rs imported `walk` for a test that only exists on Unix, so the import was unused on Windows and `clippy -D warnings` refused it. The Unix test calls `super::walk`, as its neighbour calls `super::walk_inner`. Checked on Windows 11 x64: `cargo test --workspace` and `cargo clippy --workspace --all-targets -- -D warnings` both pass. Co-Authored-By: Claude Opus 5.5 * fix(engine): refresh attribute baselines after value changes CC-046: invalidate the derived XY baseline for changed justified attribute values, including small changes inside the writer's imported-font tolerance. Preserve no-op placement, alignment anchors and Aligned/Fit endpoints. Regression reproduced on current upstream; all three focused tests pass. Full cargo xtask ci passed all seven stages on Windows (2558 passing test executions, zero failures, one ignored). Release Trunk web build passed. Browser interaction verification is pending user dismissal of a Windows Security prompt; do not treat the build as a runtime pass. Windows validation prerequisite: original upstream PR #2198 commit a2dd814. Independent of editor/ribbon changes; reverting restores only this setter, its regression tests and corresponding file-format parity record. * fix(io): preserve insertion elevation when refreshing text baselines CC-046 review follow-up to PR #2214 comment 4377706777. Regression reproduced insertion Z 9 becoming 7. Preserve group 10 Z; test DXF independent elevations and document the DWG bridge shared elevation. Full cargo xtask ci passes all seven gates. This separate commit is reversible; reverting it restores elevation loss during baseline recomputation. Existing browser UI verification predates this writer correction. * test(engine): cover global attribute no-ops and two-point exports CC-046 review 5653054817: verify equal-value global JSON and scripted -ATTEDIT preserve imported placement, and Aligned/Fit DXF save/reopen retain both placement points and justification. Validation: cargo xtask ci passed all seven gates on this branch; git diff --check passed. Test-only follow-up, no application change. Depends on CC-046 and unchanged Windows prerequisite #2198. Revert this commit independently to remove only these assertions. Runtime evidence remains separately recorded for the production commits. --------- Co-authored-by: 2ez4gcx <108074836+2ez4gcx@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 --- crates/engine/src/cmd/blocks/attedit.rs | 22 +- .../tests/it/attribute_value_baseline.rs | 203 ++++++++++++++++++ crates/engine/tests/it/main.rs | 1 + crates/io/src/dxf_write/common.rs | 3 +- docs/file-format-parity.md | 5 +- 5 files changed, 228 insertions(+), 6 deletions(-) create mode 100644 crates/engine/tests/it/attribute_value_baseline.rs diff --git a/crates/engine/src/cmd/blocks/attedit.rs b/crates/engine/src/cmd/blocks/attedit.rs index ad123f0b3..be59474c5 100644 --- a/crates/engine/src/cmd/blocks/attedit.rs +++ b/crates/engine/src/cmd/blocks/attedit.rs @@ -3,7 +3,7 @@ //! time or by replacing a string in all of them. Attribute tags match in any case, as in AutoCAD. use cadcraft_doc::color::Color; -use cadcraft_doc::{AttribProps, Common, EntityKind, Handle}; +use cadcraft_doc::{AttribProps, Common, EntityKind, HAlign, Handle, Text, VAlign}; use serde_json::{Map, Value, json}; use crate::cmd::curves::KW; @@ -67,7 +67,7 @@ fn run(s: &mut Session, p: &Value) -> Result { if let EntityKind::Insert(i) = &mut e.kind { for a in &mut i.attribs { if let Some(v) = value_for(&vals, &a.tag) { - a.text.value = v; + set_value(&mut a.text, v); n += 1; } } @@ -130,13 +130,29 @@ fn edit(s: &mut Session, (h, k): (Handle, usize), f: impl FnOnce(&str) -> Option && let Some(a) = ins.attribs.get_mut(k) && let Some(v) = f(&a.text.value).filter(|v| *v != a.text.value) { - a.text.value = v; + set_value(&mut a.text, v); changed = true; } })?; Ok(changed) } +/// Refresh the derived baseline after a value change, even one within the writer's +/// imported-font tolerance. Equal XY points request recomputation on export; retain +/// both elevations and the independent endpoints used by Aligned/Fit text. +fn set_value(text: &mut Text, value: String) { + if text.value == value { + return; + } + text.value = value; + if !matches!((text.halign, text.valign), (HAlign::Left, VAlign::Baseline) | (HAlign::Aligned | HAlign::Fit, _)) + && let Some(anchor) = text.align_pt + { + text.insert.x = anchor.x; + text.insert.y = anchor.y; + } +} + /// Change the properties set on attribute `k` of block reference `h`. fn set_props(s: &mut Session, (h, k): (Handle, usize), f: impl FnOnce(&mut AttribProps)) -> Result<()> { s.doc_mut()?.modify_entity(h, |e| { diff --git a/crates/engine/tests/it/attribute_value_baseline.rs b/crates/engine/tests/it/attribute_value_baseline.rs new file mode 100644 index 000000000..8f7523e0b --- /dev/null +++ b/crates/engine/tests/it/attribute_value_baseline.rs @@ -0,0 +1,203 @@ +//! Small value changes must not export an imported justified attribute's old baseline. +use cadcraft_doc::{Attrib, Drawing, EntityKind, HAlign, Handle, Text, VAlign}; +use cadcraft_engine::{Input, Session}; +use cadcraft_geom::Vec3; +use serde_json::json; + +const BEFORE: &str = "POWER ADAPTER"; +const AFTER: &str = "POWER ADAPTER."; + +fn fixture() -> (Session, Handle, Handle) { + let mut s = Session::new(); + for (tag, value, y) in [("T", BEFORE, 15), ("U", "ADPT-01", 0)] { + s.execute("attdef", &json!({"tag":tag,"default":value,"at":[0,y],"height":3})).unwrap(); + } + s.execute("line", &json!({"points":[[0,-5],[40,-5]]})).unwrap(); + let handles: Vec<_> = s.doc().unwrap().model.iter().map(|e| e.handle.hex()).collect(); + s.execute("block", &json!({"name":"Equipment","handles":handles,"base":[0,0],"keep":"delete"})).unwrap(); + let a = s.execute("insert", &json!({"name":"Equipment","at":[100,200],"scale":2,"rotation":30})).unwrap(); + let b = s.execute("insert", &json!({"name":"Equipment","at":[300,200],"attribs":{"T":"Other"}})).unwrap(); + let a = Handle::parse_hex(a["handle"].as_str().unwrap()).unwrap(); + let b = Handle::parse_hex(b["handle"].as_str().unwrap()).unwrap(); + // Set existing document fields directly: this regression does not require Text Options. + s.doc_mut() + .unwrap() + .modify_entity(a, |e| { + let EntityKind::Insert(i) = &mut e.kind else { panic!("insert") }; + let t = &mut i.attribs[0].text; + t.insert.z = 7.0; + t.align_pt = Some(t.insert); + t.halign = HAlign::Center; + t.valign = VAlign::Top; + }) + .unwrap(); + let imported = cadcraft_io::read_dxf(cadcraft_io::write_dxf(s.doc().unwrap()).as_bytes()).unwrap(); + let mut s = Session::empty(); + s.open_drawing(imported, "Imported", None); + (s, a, b) +} + +fn attributes(d: &Drawing, h: Handle) -> Vec { + let EntityKind::Insert(i) = &d.entity(h).unwrap().kind else { panic!("insert") }; + i.attribs.clone() +} + +fn baseline(d: &Drawing, t: &Text) -> cadcraft_geom::Vec2 { + let style = d.text_style(&t.style).unwrap(); + let (h, v) = cadcraft_render::text_align(t.halign, t.valign); + cadcraft_fonts::baseline_start( + &cadcraft_fonts::TextFont::resolve(&style.font), + &t.value, + &cadcraft_fonts::TextParams { + insert: t.insert.xy(), + align_pt: t.align_pt.map(|p| p.xy()), + height: t.height, + rotation: t.rotation, + width_factor: t.width_factor, + oblique: t.oblique, + h, + v, + backwards: style.backwards, + upside_down: style.upside_down, + }, + ) +} + +#[test] +fn all_attribute_value_paths_refresh_small_justified_export_changes() { + for path in ["json", "interactive", "global-json", "global-interactive"] { + let (mut s, a, b) = fixture(); + let before = attributes(s.doc().unwrap(), a); + let other = attributes(s.doc().unwrap(), b); + let definition = s.doc().unwrap().block("Equipment").unwrap().clone(); + match path { + "json" => { + s.execute("attedit", &json!({"handle":a.hex(),"values":{"t":AFTER}})).unwrap(); + } + "interactive" => { + s.start("attedit").unwrap(); + s.input(Input::Pick(vec![a])).unwrap(); + s.input(Input::Text(AFTER.into())).unwrap(); + s.input(Input::Enter).unwrap(); + assert!(s.running.is_none()); + } + "global-json" => { + s.execute("-attedit", &json!({"handles":[a.hex()],"tag":"T","find":"ADAPTER","replace":"ADAPTER."})).unwrap(); + } + "global-interactive" => { + s.script("-attedit\nn\nn\nEquipment\nT\n\nADAPTER\nADAPTER.\n").unwrap(); + assert!(s.running.is_none(), "{}", s.prompt_text()); + } + _ => panic!("path"), + } + let edited = attributes(s.doc().unwrap(), a); + assert_eq!(edited[0].text.value, AFTER, "{path}"); + assert_eq!(edited[0].text.align_pt, before[0].text.align_pt); + assert_eq!(edited[0].text.insert.z, 7.0); + assert_eq!(edited[1], before[1]); + assert_eq!(attributes(s.doc().unwrap(), b), other); + assert_eq!(s.doc().unwrap().block("Equipment").unwrap(), &definition); + let expected = baseline(s.doc().unwrap(), &edited[0].text); + assert!(expected.dist(before[0].text.insert.xy()) > 0.01, "fixture must change width"); + for name in ["edited.dxf", "edited.dwg"] { + let reopened = cadcraft_io::read(&cadcraft_io::write(s.doc().unwrap(), name).unwrap(), name).unwrap(); + let restored = attributes(&reopened, a); + assert!( + restored[0].text.insert.xy().dist(expected) < 1e-8, + "{path} {name}: stale {:?}, expected {expected:?}", + restored[0].text.insert.xy() + ); + assert_eq!(restored[0].text.align_pt, before[0].text.align_pt); + assert_eq!(restored[0].text.value, AFTER); + assert_eq!(restored[0].text.insert.z, 7.0); + } + s.undo().unwrap(); + assert_eq!(attributes(s.doc().unwrap(), a), before, "{path} undo"); + s.redo().unwrap(); + assert_eq!(attributes(s.doc().unwrap(), a), edited, "{path} redo"); + } +} + +#[test] +fn equal_attribute_values_preserve_an_imported_font_baseline() { + let (mut s, a, _) = fixture(); + // A small imported-font offset must survive a no-op edit (writer tolerance is deliberate). + s.doc_mut() + .unwrap() + .modify_entity(a, |e| { + let EntityKind::Insert(i) = &mut e.kind else { panic!("insert") }; + i.attribs[0].text.insert.x += 0.1; + }) + .unwrap(); + let before = attributes(s.doc().unwrap(), a); + s.execute("attedit", &json!({"handle":a.hex(),"values":{"T":BEFORE}})).unwrap(); + assert_eq!(attributes(s.doc().unwrap(), a), before); + s.start("attedit").unwrap(); + s.input(Input::Pick(vec![a])).unwrap(); + s.input(Input::Enter).unwrap(); + s.input(Input::Enter).unwrap(); + assert_eq!(attributes(s.doc().unwrap(), a), before); + s.execute("-attedit", &json!({"handles":[a.hex()],"tag":"T","find":"ADAPTER","replace":"ADAPTER"})).unwrap(); + assert_eq!(attributes(s.doc().unwrap(), a), before, "global JSON no-op"); + s.script("-attedit\nn\nn\nEquipment\nT\n\nADAPTER\nADAPTER\n").unwrap(); + assert!(s.running.is_none(), "{}", s.prompt_text()); + assert_eq!(attributes(s.doc().unwrap(), a), before, "global interactive no-op"); +} + +/// Recomputing a justified baseline must not replace its independent insertion elevation. +#[test] +fn value_edits_preserve_distinct_insertion_and_alignment_elevations_on_export() { + let (mut s, a, _) = fixture(); + s.doc_mut() + .unwrap() + .modify_entity(a, |e| { + let EntityKind::Insert(i) = &mut e.kind else { panic!("insert") }; + i.attribs[0].text.insert.z = 9.0; + }) + .unwrap(); + let anchor = attributes(s.doc().unwrap(), a)[0].text.align_pt; + s.execute("attedit", &json!({"handle":a.hex(),"values":{"T":AFTER}})).unwrap(); + for name in ["elevations.dxf", "elevations.dwg"] { + let reopened = cadcraft_io::read(&cadcraft_io::write(s.doc().unwrap(), name).unwrap(), name).unwrap(); + let restored = attributes(&reopened, a); + assert_eq!(restored[0].text.insert.z, 9.0, "{name}: insertion elevation"); + assert_eq!(restored[0].text.align_pt.map(|p| p.xy()), anchor.map(|p| p.xy()), "{name}: alignment XY"); + if name.ends_with(".dxf") { + assert_eq!(restored[0].text.align_pt, anchor, "DXF retains independent alignment elevation"); + } else { + // The DWG bridge uses one text elevation for both points. + assert_eq!(restored[0].text.align_pt.unwrap().z, 9.0); + } + } +} + +#[test] +fn value_edits_preserve_left_baseline_and_two_point_placements() { + for horizontal in [HAlign::Left, HAlign::Aligned, HAlign::Fit] { + let (mut s, a, _) = fixture(); + s.doc_mut() + .unwrap() + .modify_entity(a, |e| { + let EntityKind::Insert(i) = &mut e.kind else { panic!("insert") }; + let t = &mut i.attribs[0].text; + t.halign = horizontal; + t.valign = VAlign::Baseline; + t.insert = Vec3::new(10.0, 20.0, 7.0); + t.align_pt = (horizontal != HAlign::Left).then_some(Vec3::new(40.0, 30.0, 9.0)); + }) + .unwrap(); + let before = attributes(s.doc().unwrap(), a); + s.execute("attedit", &json!({"handle":a.hex(),"values":{"T":AFTER}})).unwrap(); + let mut expected = before; + expected[0].text.value = AFTER.into(); + assert_eq!(attributes(s.doc().unwrap(), a), expected); + if horizontal != HAlign::Left { + let reopened = cadcraft_io::read_dxf(cadcraft_io::write_dxf(s.doc().unwrap()).as_bytes()).unwrap(); + let restored = attributes(&reopened, a); + assert_eq!(restored[0].text.insert, expected[0].text.insert); + assert_eq!(restored[0].text.align_pt, expected[0].text.align_pt); + assert_eq!(restored[0].text.halign, horizontal); + assert_eq!(restored[0].text.value, AFTER); + } + } +} diff --git a/crates/engine/tests/it/main.rs b/crates/engine/tests/it/main.rs index 8e20e6fa8..ed88a2316 100644 --- a/crates/engine/tests/it/main.rs +++ b/crates/engine/tests/it/main.rs @@ -10,6 +10,7 @@ mod attedit_prompts; mod attrib_modes; mod attrib_props; mod attribute_tools; +mod attribute_value_baseline; mod block_description; mod block_keep; mod block_redefine; diff --git a/crates/io/src/dxf_write/common.rs b/crates/io/src/dxf_write/common.rs index bf8fad449..6ba3ce8ef 100644 --- a/crates/io/src/dxf_write/common.rs +++ b/crates/io/src/dxf_write/common.rs @@ -237,7 +237,8 @@ fn computed_text_insert(d: &Drawing, t: &Text) -> Vec3 { upside_down: ts.upside_down, }; let q = cadcraft_fonts::baseline_start(&cadcraft_fonts::TextFont::resolve(&ts.font), &t.value, &p); - if q.is_finite() { Vec3::new(q.x, q.y, a.z) } else { t.insert } + // Justification recalculates the baseline in XY; group 10 keeps its own elevation. + if q.is_finite() { Vec3::new(q.x, q.y, t.insert.z) } else { t.insert } } pub(super) fn text_tags(w: &mut W, d: &Drawing, t: &Text, attrib: Option<(&Attrib, Version)>, attdef_prompt: Option<&str>) { diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 291c9b202..d2af85803 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -1,6 +1,6 @@ # File-format parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-25 · **Change:** minor (LAYOUT SAveas writes a layout as a DWT template) · **Target:** Autodesk AutoCAD 2027 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-28 · **Change:** minor (attribute value edits refresh exported baselines) · **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 @@ -75,7 +75,7 @@ on read with a note, so such files no longer exceed the DXF limit (#1482). **DWG |---|---|---|---| | 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; multiline attributes (embedded MTEXT) read and drawn | yes; multiline attributes written as embedded MTEXT (DXF 2007 and later; older versions: one line) | ATTDEF prompt kept (#137) | +| TEXT, MTEXT, ATTDEF, INSERT + ATTRIB | yes; multiline attributes (embedded MTEXT) read and drawn | yes; multiline attributes written as embedded MTEXT (DXF 2007 and later; older versions: one line); changed attribute values refresh justified baselines, including small width changes | ATTDEF prompt kept (#137); unchanged values retain imported-font placement | | 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 for every associated kind — linear/aligned, angular, radial/diametric, ordinate and arc length — and read back without CADCraft's xdata (#1651) | | 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; angular block text honors DIMDSEP, including per-dimension overrides (#1646) | | DIMENSION (all 7 kinds) | yes | yes, with anonymous `*D` blocks; their text keeps the diameter sign as `%%c`, so SHX styles plot it (#1645) | a reopened dimension's `*D` block is regenerated under its name, unreferenced ones dropped (#495); DIMASSOC objects written | @@ -140,6 +140,7 @@ our output in AutoCAD black-box. | Date | Change | Summary | |---|---|---| +| 2026-10-26 | minor | CC-046: ATTEDIT and -ATTEDIT value changes invalidate derived justified baselines before DXF/DWG export; preserve alignment anchors, unchanged values and Aligned/Fit endpoints. Review follow-up: baseline recomputation preserves insertion elevation independently of alignment elevation. The shared writer correction also applies to justified TEXT. Independent of attribute editor UI; reverting the follow-up restores elevation loss on recomputation, while reverting the value setter restores stale XY baselines. | | 2026-10-25 | minor | LAYOUT SAveas (`layout.saveas`) writes a layout as a drawing template: its page setup and paper space objects, no model space objects or other layouts, and only the named objects they use | | 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) |