diff --git a/crates/dwg/src/lib.rs b/crates/dwg/src/lib.rs index a5de9bffe..e5463aaec 100644 --- a/crates/dwg/src/lib.rs +++ b/crates/dwg/src/lib.rs @@ -332,12 +332,51 @@ mod native { /// can cover rows × cols cells, so the total is bounded too. const MAX_MERGE_CELLS: usize = 1 << 24; + /// A table cell's own text height only when the file says it overrides the table style's. + /// acadrust's DXF writer writes a cell style's height (group 140) whenever the cell has a + /// style record, even a default 0.18 with no override flag, and CADCraft reads a 140 as an + /// override (#2501). The override is the cell style's TEXT_HEIGHT property flag or the + /// content's format override bit 128; without either the height is cleared (0 is no + /// override), so the table style's height applies. + fn table_cell_heights(doc: &mut acadrust::CadDocument) { + use acadrust::entities::table::CellStylePropertyFlags; + const CONTENT_TEXT_HEIGHT: i32 = 128; + for e in doc.entities_mut() { + let acadrust::entities::EntityType::Table(t) = e else { continue }; + for cell in t.rows.iter_mut().flat_map(|r| r.cells.iter_mut()) { + // The first content's own height (group 144) when its format override sets bit 128. + // CADCraft keeps one height for the cell's text, so a later content's override + // must not stand in for the first content's. + let content_height = cell + .contents + .first() + .filter(|c| c.format_override_flags & CONTENT_TEXT_HEIGHT != 0) + .map(|c| c.text_height) + .filter(|h| h.is_finite() && *h > 0.0); + if let Some(h) = content_height { + // CADCraft reads only the style's height (group 140), so carry it there. + cell.style.get_or_insert_with(acadrust::entities::table::CellStyle::new).text_height = h; + continue; + } + // A style overrides through its property flags or, in newer files, its override + // flags; anything else is the writer's default and is cleared. + if let Some(style) = cell.style.as_mut() + && !style.property_flags.contains(CellStylePropertyFlags::TEXT_HEIGHT) + && style.override_flags & CellStylePropertyFlags::TEXT_HEIGHT.bits() as i32 == 0 + { + style.text_height = 0.0; + } + } + } + } + /// The fix-ups made to a document after the DWG read, returning how many orphaned DGN /// components were dropped. The document comes from a third-party reader and is hostile. fn post_process(doc: &mut acadrust::CadDocument) -> usize { super::fail_in_test("post_process"); current_style_names(doc); table_merges(doc); + table_cell_heights(doc); table_style_rows(doc, false); keep_sort_handles(doc); drop_orphaned_dgn_components(doc) @@ -821,6 +860,34 @@ mod native { assert_eq!(table_merges_within(&mut doc, usize::MAX), 100); } + /// A height marked only in the style's override flags (as newer files store it) is kept, + /// and the same height with no flag at all is cleared (review on #2588). Tested on the + /// read document because acadrust's DWG writer doesn't write the override flags. + #[test] + fn a_style_override_flag_keeps_the_cell_height() { + use acadrust::entities::table::{CellStyle, CellStylePropertyFlags}; + let height = |override_flags: i32| { + let mut doc = doc_with_table(1, 1, 0); + for e in doc.entities_mut() { + if let EntityType::Table(t) = e { + for cell in t.rows.iter_mut().flat_map(|r| r.cells.iter_mut()) { + let mut s = CellStyle::new(); + s.text_height = 0.5; + s.override_flags = override_flags; + cell.style = Some(s); + } + } + } + table_cell_heights(&mut doc); + doc.entities().find_map(|e| match e { + EntityType::Table(t) => t.rows.first()?.cells.first()?.style.as_ref().map(|s| s.text_height), + _ => None, + }) + }; + assert_eq!(height(CellStylePropertyFlags::TEXT_HEIGHT.bits() as i32), Some(0.5)); + assert_eq!(height(0), Some(0.0)); + } + /// A panic in the post-read fix-ups is an error, not a crash (#1934). #[test] fn a_panic_in_post_processing_is_an_error() { diff --git a/crates/io/tests/dwg_table_cell_height.rs b/crates/io/tests/dwg_table_cell_height.rs new file mode 100644 index 000000000..11ee98c9d --- /dev/null +++ b/crates/io/tests/dwg_table_cell_height.rs @@ -0,0 +1,173 @@ +//! TABLE heights inherit from the table style unless the file records a cell override. +#![cfg(not(target_arch = "wasm32"))] + +use cadcraft_doc::{Common, Drawing, EntityKind, Space, Table, TableCell, TableStyle}; + +fn drawing() -> Drawing { + let mut d = Drawing::new_imperial(); + d.table_styles.push(TableStyle { name: "Small".into(), text_height: 0.0625, title: false, header: false, ..TableStyle::default() }); + let table = Table { + style: "Small".into(), + text_height: 0.0625, + title: false, + header: false, + row_heights: vec![0.2], + col_widths: vec![2.0; 3], + cells: vec![vec![ + TableCell { text: "plain".into(), ..TableCell::default() }, + TableCell { text: "{\\C256;formatted}".into(), ..TableCell::default() }, + TableCell { text: "explicit".into(), text_height: Some(0.125), ..TableCell::default() }, + ]], + ..Table::default() + }; + d.add(&Space::Model, Common::default(), EntityKind::Table(table)).unwrap(); + d +} + +fn check(d: &Drawing) { + let t = d + .model + .iter() + .find_map(|e| match &e.kind { + EntityKind::Table(t) => Some(t), + _ => None, + }) + .unwrap(); + assert_eq!(t.style, "Small"); + assert_eq!(t.text_height, 0.0625); + for cell in &t.cells[0][..2] { + assert!(cell.text_height.is_none(), "unexpected override: {:?}", cell.text_height); + } + assert_eq!(t.cells[0][2].text_height, Some(0.125)); +} + +#[test] +fn table_cell_heights_keep_inheritance_through_dwg() { + let bytes = cadcraft_io::write(&drawing(), "height.dwg").unwrap(); + // A style record can be present for metadata without overriding its text height. + // Write that synthetic representation directly so no earlier DXF conversion can + // turn the unused default into an explicit override before this import. + let mut doc = acadrust::DwgReader::from_stream(std::io::Cursor::new(bytes)).read().unwrap(); + let table = doc + .entities_mut() + .find_map(|e| match e { + acadrust::entities::EntityType::Table(t) => Some(t), + _ => None, + }) + .unwrap(); + table.rows[0].cells[0].style = Some(acadrust::entities::table::CellStyle::new()); + let bytes = acadrust::DwgWriter::write_to_vec(&doc).unwrap(); + check(&cadcraft_io::read(&bytes, "height.dwg").unwrap()); +} + +#[test] +fn table_cell_heights_keep_inheritance_through_dxf() { + let bytes = cadcraft_io::write(&drawing(), "height.dxf").unwrap(); + check(&cadcraft_io::read(&bytes, "height.dxf").unwrap()); +} + +// ---- cells with their own style record (review on #2588) ---- + +use acadrust::entities::table::{CellContent, CellStyle, CellStylePropertyFlags}; + +/// Write the drawing as a DWG, edit its first cell through acadrust (`edit`), write it again as a +/// DWG (or, with `as_dxf`, as a DXF by acadrust's writer) and read it with CADCraft. Returns the first cell's +/// text height override. +fn first_cell_height(as_dxf: bool, edit: impl FnOnce(&mut acadrust::entities::table::TableCell)) -> Option { + let bytes = cadcraft_io::write(&drawing(), "height.dwg").unwrap(); + let mut doc = acadrust::DwgReader::from_stream(std::io::Cursor::new(bytes)).read().unwrap(); + let table = doc + .entities_mut() + .find_map(|e| match e { + acadrust::entities::EntityType::Table(t) => Some(t), + _ => None, + }) + .unwrap(); + edit(&mut table.rows[0].cells[0]); + let d = if as_dxf { + let bytes = acadrust::DxfWriter::new(&doc).write_to_vec().unwrap(); + cadcraft_io::read(&bytes, "height.dxf").unwrap() + } else { + let bytes = acadrust::DwgWriter::write_to_vec(&doc).unwrap(); + cadcraft_io::read(&bytes, "height.dwg").unwrap() + }; + d.model + .iter() + .find_map(|e| match &e.kind { + EntityKind::Table(t) => Some(t.cells[0][0].text_height), + _ => None, + }) + .unwrap() +} + +fn styled(height: f64, flags: CellStylePropertyFlags) -> CellStyle { + let mut s = CellStyle::new(); + s.text_height = height; + s.property_flags = flags; + s +} + +#[test] +fn a_cell_style_with_the_text_height_flag_keeps_its_height() { + let h = first_cell_height(false, |c| c.style = Some(styled(0.5, CellStylePropertyFlags::TEXT_HEIGHT))); + assert_eq!(h, Some(0.5)); +} + +#[test] +fn content_with_format_override_bit_128_keeps_its_own_height() { + // acadrust writes the content's height as group 144 and the style's as 140; they differ here + // so the result shows which one CADCraft honoured. + let h = first_cell_height(false, |c| { + c.style = Some(styled(0.5, CellStylePropertyFlags::NONE)); + if c.contents.is_empty() { + c.contents.push(CellContent::new()); + } + c.contents[0].format_override_flags |= 128; + c.contents[0].text_height = 0.3125; + }); + assert_eq!(h, Some(0.3125)); +} + +/// Only the first content's override stands for the cell: a later content's height must not +/// replace the first content's inherited one (review on #2588). +#[test] +fn a_later_contents_height_does_not_replace_the_first_contents() { + let h = first_cell_height(false, |c| { + c.style = Some(styled(0.18, CellStylePropertyFlags::NONE)); + c.contents.clear(); + c.contents.push(CellContent::new()); + let mut later = CellContent::new(); + later.format_override_flags |= 128; + later.text_height = 0.3125; + c.contents.push(later); + }); + assert_eq!(h, None); +} + +/// Bit 128 with an unusable content height copies nothing, so the writer's default style height +/// is still cleared (review on #2588). +#[test] +fn bit_128_with_an_invalid_height_still_clears_the_default() { + for bad in [0.0, -1.0, f64::NAN] { + let h = first_cell_height(false, |c| { + c.style = Some(styled(0.18, CellStylePropertyFlags::NONE)); + if c.contents.is_empty() { + c.contents.push(CellContent::new()); + } + c.contents[0].format_override_flags |= 128; + c.contents[0].text_height = bad; + }); + assert_eq!(h, None, "content height {bad}"); + } +} + +#[test] +fn a_cell_with_no_style_record_inherits_the_table_style() { + assert_eq!(first_cell_height(false, |c| c.style = None), None); +} + +/// A style record with a height and neither override is not an override, through DWG. +#[test] +fn a_style_height_without_an_override_is_cleared() { + assert_eq!(first_cell_height(false, |c| c.style = Some(styled(0.18, CellStylePropertyFlags::NONE))), None); +} diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 53ffffc96..a91ee2f25 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,10 +1,10 @@ # Parity checklist -> **Generated:** 2026-10-30 by `cargo xtask parity`. Do not edit by hand. +> **Generated:** 2026-11-01 by `cargo xtask parity`. Do not edit by hand. Menu-breadth parity: a reference menu item counts as covered when a CADCraft command is registered under the same menu label. It measures breadth, not depth; see [target-app-parity.md](target-app-parity.md) for the weighted assessment. -**Menu breadth: 298 / 491 items (61%)** · 444 registered commands. +**Menu breadth: 298 / 491 items (61%)** · 449 registered commands. | Menu | Items | Covered | % | |---|---|---|---|