review(#1630): one table rebuild loop that also refreshes formulas; one cell layout

- main already rebuilt an edited table's block; the PR added a second
  loop after it that could never fire. Formula refresh and the block
  cache bump now live in the one loop.
- Formula refresh copies the drawing only for a table that has a field.
- A field cell lays its text out once and boxes it from those cells.
- A zero text height at one level falls through to the next instead of
  ending the lookup.
- Field box constants and BACKGROUND keep their own doc comments.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Hakan SevenandClaude Opus 5.5 committed 2026-10-06 20:54:26 +03:00
1 parent 95d5eb73d9
commit 62d72b3a8d
3 files changed
+34 -41

No files matched your search

+12 -2
View File
@@ -137,7 +137,17 @@ pub fn update_fields(
/// Recompute a table's cell formulas into its block after the table changed;
/// returns the cell texts that changed.
pub fn refresh_table_formulas(document: &mut CadDocument, table: Handle) -> Vec<Handle> {
if document.fields.is_empty() {
// Copying the drawing is only worth it for a table with a field.
let has_field = matches!(
document.get_entity(table),
Some(codec::EntityType::Table(t)) if t
.rows
.iter()
.flat_map(|row| &row.cells)
.flat_map(|cell| &cell.contents)
.any(|content| content.field_handle.is_some())
);
if !has_field {
return Vec::new();
}
let snapshot = document.clone();
@@ -332,13 +342,13 @@ pub fn set_display(on: bool) {
FIELD_DISPLAY.store(on, std::sync::atomic::Ordering::Relaxed);
}
/// The gray the reference draws behind a field.
/// FIELDDISPLAY box extent around a line of field text, in text heights:
/// from a third of the height below the baseline to 1.19 heights above it
/// (the reference's box, measured at 2.5 and 10 unit text).
pub const BOX_BELOW: f64 = 0.345;
pub const BOX_ABOVE: f64 = 1.19;
/// The gray the reference draws behind a field.
pub const BACKGROUND: [f32; 4] = [203.0 / 255.0, 203.0 / 255.0, 203.0 / 255.0, 1.0];
/// Whether `entity` hosts a field: its extension dictionary has ACAD_FIELD.
+13 -24
View File
@@ -135,11 +135,14 @@ fn cell_text_height(
content: &codec::entities::table::CellContent,
) -> Option<f64> {
use codec::entities::table::CellStylePropertyFlags as P;
let set = |h: f64| (h > 1e-6).then_some(h);
let own = content.sets(P::TEXT_HEIGHT);
own.then_some(content.text_height)
.or_else(|| style_for_property(table, row, column, cell, P::TEXT_HEIGHT).map(|s| s.text_height))
.or(Some(content.text_height))
.filter(|h| *h > 1e-6)
own.then(|| set(content.text_height))
.flatten()
.or_else(|| {
style_for_property(table, row, column, cell, P::TEXT_HEIGHT).and_then(|s| set(s.text_height))
})
.or_else(|| set(content.text_height))
}
/// A cell's text style: the content's own, else the cell, row, column or
@@ -1960,6 +1963,8 @@ pub fn tessellate_table(
.map(|style| style.rotation as f32)
.unwrap_or(cell.rotation as f32)
};
// FIELDDISPLAY boxes come from this same layout's cells.
let field_boxes = content.field_handle.is_some() && crate::entities::field::display();
let layout = layout_mtext(&MTextRenderOpts {
columns: Default::default(),
value: text,
@@ -1974,31 +1979,15 @@ pub fn tessellate_table(
exact_line_spacing: false,
rectangle_height: 0.0,
vertical_text: false,
want_glyph_boxes: false,
want_glyph_boxes: field_boxes,
});
// FIELDDISPLAY: a field cell's text on a gray box, one per line,
// from the same layout's character cells (screen only).
if content.field_handle.is_some() && crate::entities::field::display() {
let cells = layout_mtext(&MTextRenderOpts {
columns: Default::default(),
value: text,
insertion: [to.x as f64, to.y as f64, to.z as f64],
height: cell_h,
rect_w: (col_width - margin_left - margin_right).max(0.0),
rotation: rot,
style: &resolved,
attach_h_anchor,
v_anchor,
line_spacing_factor: 1.0,
exact_line_spacing: false,
rectangle_height: 0.0,
vertical_text: false,
want_glyph_boxes: true,
})
.glyph_boxes;
if field_boxes {
let cells = &layout.glyph_boxes;
let (sin_r, cos_r) = (rot as f64).sin_cos();
let mut lines: Vec<[f64; 4]> = Vec::new();
for b in &cells {
for b in cells {
let [l, base, r, top] = b.local.map(|v| v as f64);
match lines.last_mut() {
Some(c) if (c[1] - base).abs() <= 1e-3 * (top - base).max(c[3]) => {
+9 -15
View File
@@ -3617,7 +3617,9 @@ impl Scene {
// (the *T block) redrawn from the cells. Tables keeping their picture
// (moved, restyled, opened from a file) are never redrawn: the block
// may hold authored content the redraw does not reproduce.
// Its cell formulas are worked out again into that block.
if !appearance_only {
let mut rebuilt = false;
for (handle, kind) in &changes {
if !matches!(kind, ChangeKind::Removed)
&& matches!(
@@ -3625,9 +3627,15 @@ impl Scene {
Some(EntityType::Table(t)) if t.block_record_handle.is_none()
)
{
self.document.refresh_table_block(*handle);
rebuilt |= self.document.refresh_table_block(*handle);
rebuilt |= !crate::entities::field::refresh_table_formulas(&mut self.document, *handle)
.is_empty();
}
}
// The block's cached drawing is stale once its texts change.
if rebuilt {
self.block_epoch = GEOMETRY_EPOCH.fetch_add(1, Ordering::Relaxed);
}
}
// A restyle (layer on/off, colour, text or dimension style) moves no
// geometry, so nothing measured from it is re-resolved: an
@@ -3695,20 +3703,6 @@ impl Scene {
}
}
}
// An edited table lets go of its block; rebuild it from the cells so
// the drawing shows and saves the cell texts and grid it now holds.
let mut rebuilt_table_block = false;
for (handle, kind) in &changes {
if !matches!(kind, ChangeKind::Removed)
&& matches!(self.document.get_entity(*handle), Some(EntityType::Table(t)) if t.block_record_handle.is_none())
{
rebuilt_table_block |= self.document.refresh_table_block(*handle);
rebuilt_table_block |= !crate::entities::field::refresh_table_formulas(&mut self.document, *handle).is_empty();
}
}
if rebuilt_table_block {
self.block_epoch = GEOMETRY_EPOCH.fetch_add(1, Ordering::Relaxed);
}
if !changes.is_empty() {
self.refresh_dependency_index_for_changes(&changes);
}