mirror of
https://github.com/storytold/cadcraft.git
synced 2026-11-03 00:42:46 -06:00
* DWG: a table cell's text height is an override only when the file flags it (#2501) acadrust's DXF writer writes group 140 for any cell with a style record, even a default 0.18 with no override flag, and CADCraft read it as a per-cell height, so cells ignored the table style's height. The bridge now clears the height of a cell style that has neither the TEXT_HEIGHT property flag nor the content's height override bit (0 reads as no override). * Parity checklist: regenerated after merging main * Tests: inherited table cell heights must stay unset (review on #2588) A cell that inherits its height must read back as None, not an explicit 0.0625 override. * Parity checklist: regenerated after merging main * Test: a cell style with the TEXT_HEIGHT flag, content override bit 128 and a cell with no style record keep the right height (review on #2588) * DWG: a table cell's content height override (group 144) is kept (review on #2588) * Parity checklist: regenerated after merging main * DWG: table cell height takes the first content's override and the style's override flags (review on #2588) Only the first content's bit-128 height stands for the cell (one height for its text); an invalid content height no longer keeps the writer's default 0.18; a height marked only in the style's override flags (newer files) is kept.
This commit is contained in:
Books
@@ -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() {
|
||||
|
||||
@@ -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<f64> {
|
||||
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);
|
||||
}
|
||||
@@ -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 | % |
|
||||
|---|---|---|---|
|
||||
|
||||
Reference in New Issue
Block a user