diff --git a/crates/rustmotion-components/src/intrinsic.rs b/crates/rustmotion-components/src/intrinsic.rs index eb016cf2..88c0dc0b 100644 --- a/crates/rustmotion-components/src/intrinsic.rs +++ b/crates/rustmotion-components/src/intrinsic.rs @@ -875,17 +875,18 @@ impl IntrinsicMeasure for TerminalIntrinsic { // Table intrinsic measurer // ───────────────────────────────────────────────────────────────────────────── -use crate::table::{ - Table, DEFAULT_CELL_PADDING, DEFAULT_FONT_SIZE as TABLE_FONT_SIZE, DEFAULT_ROW_HEIGHT_RATIO, -}; +use crate::table::{Table, DEFAULT_FONT_SIZE as TABLE_FONT_SIZE, DEFAULT_ROW_HEIGHT_RATIO}; /// Intrinsic measurer for [`Table`]. /// -/// Natural size formula (matches the painter exactly): +/// Natural size formula (matches the painter exactly, since both now read +/// the same per-column distribution — see [`Table::natural_column_widths`]): /// - `row_height = font_size × DEFAULT_ROW_HEIGHT_RATIO` /// - `height = (1 + row_count) × row_height` (header + data rows) -/// - `width`: if `column_widths` are provided, their sum; otherwise each -/// column gets `max(header_text_width + 2 × cell_padding, min_col_width)`. +/// - `width`: if `column_widths` are provided, their sum; otherwise the sum +/// of `Table::natural_column_widths`, which the painter's own +/// `resolve_column_widths` scales proportionally to whatever width the +/// box actually laid out at. pub struct TableIntrinsic { row_height: f32, row_count: usize, // data rows only; header adds 1 @@ -909,44 +910,12 @@ impl TableIntrinsic { } fn compute_width(t: &Table, font_size: f32) -> f32 { - // Explicit column widths provided → sum them. if let Some(widths) = &t.column_widths { if !widths.is_empty() { return widths.iter().sum(); } } - - // Measure each header with the bold font; add 2× cell_padding per column. - let font_style = skia_safe::FontStyle::bold(); - let family = t.style.font_family.as_deref().unwrap_or("Inter"); - let Ok(typeface) = typeface_with_fallback(family, font_style) else { - // Font unavailable: fall back to col_count × a reasonable minimum. - let col_count = t.headers.len().max(1) as f32; - return col_count * (TABLE_FONT_SIZE * 8.0 + DEFAULT_CELL_PADDING * 2.0); - }; - let font = Font::from_typeface(typeface, font_size); - let emoji_font = emoji_typeface().map(|tf| Font::from_typeface(tf, font_size)); - let cell_padding = t.cell_padding; - - // Also consider data cell widths to size columns appropriately. - let col_count = t.headers.len().max(1); - let mut col_widths: Vec = vec![0.0; col_count]; - - for (i, header) in t.headers.iter().enumerate() { - let w = measure_text_with_fallback(header, &font, &emoji_font, 0.0); - col_widths[i] = col_widths[i].max(w + cell_padding * 2.0); - } - for row in &t.rows { - for (i, cell) in row.iter().enumerate() { - if i >= col_count { - break; - } - let w = measure_text_with_fallback(cell, &font, &emoji_font, 0.0); - col_widths[i] = col_widths[i].max(w + cell_padding * 2.0); - } - } - - col_widths.iter().sum() + t.natural_column_widths(font_size).iter().sum() } } diff --git a/crates/rustmotion-components/src/table.rs b/crates/rustmotion-components/src/table.rs index 05fbc996..ab882664 100644 --- a/crates/rustmotion-components/src/table.rs +++ b/crates/rustmotion-components/src/table.rs @@ -99,8 +99,14 @@ impl Table { Some(skia_safe::Font::from_typeface(typeface, font_size)) } - /// Resolve column widths: use explicit widths if provided, else equal distribution. - fn resolve_column_widths(&self, total_w: f32) -> Vec { + /// Resolve column widths: explicit widths if provided (padded with an + /// equal share for any column left unspecified); otherwise + /// [`Self::natural_column_widths`] scaled proportionally so the columns + /// still sum to exactly `total_w`, whatever that box actually laid out + /// at (which — since taffy's intrinsic measurement and this painted + /// width can diverge, e.g. a `card` giving the table less room than its + /// natural size — is not always identical to the natural total). + fn resolve_column_widths(&self, total_w: f32, font_size: f32) -> Vec { let col_count = self.headers.len().max(1); if let Some(widths) = &self.column_widths { let mut result: Vec = widths.to_vec(); @@ -111,10 +117,52 @@ impl Table { result.push(remaining / remaining_cols as f32); } result.truncate(col_count); - result - } else { - vec![total_w / col_count as f32; col_count] + return result; + } + let natural = self.natural_column_widths(font_size); + let natural_total: f32 = natural.iter().sum(); + if natural_total <= 0.0 || total_w <= 0.0 { + return vec![total_w / col_count as f32; col_count]; + } + let scale = total_w / natural_total; + natural.into_iter().map(|w| w * scale).collect() + } + + /// Per-column natural width: each column's own header/cell text + /// (measured with the bold header font, matching `paint`'s header row) + /// plus `2 × cell_padding`, indexed like `headers`. Shared by + /// `TableIntrinsic::from_table` (which sums this for the box's natural + /// total width) and `resolve_column_widths` above (which scales it to + /// whatever width the box actually laid out at) — a single source for + /// the per-column distribution keeps the two from drifting apart the + /// way an even split and a content-fitted sum used to. + pub(crate) fn natural_column_widths(&self, font_size: f32) -> Vec { + let col_count = self.headers.len().max(1); + let font_style = skia_safe::FontStyle::bold(); + let family = self.style.font_family.as_deref().unwrap_or("Inter"); + let Ok(typeface) = typeface_with_fallback(family, font_style) else { + let min_col_w = DEFAULT_FONT_SIZE * 8.0 + DEFAULT_CELL_PADDING * 2.0; + return vec![min_col_w; col_count]; + }; + let font = skia_safe::Font::from_typeface(typeface, font_size); + let emoji_font = emoji_typeface().map(|tf| skia_safe::Font::from_typeface(tf, font_size)); + let cell_padding = self.cell_padding; + + let mut col_widths: Vec = vec![0.0; col_count]; + for (i, header) in self.headers.iter().enumerate() { + let w = measure_text_with_fallback(header, &font, &emoji_font, 0.0); + col_widths[i] = col_widths[i].max(w + cell_padding * 2.0); + } + for row in &self.rows { + for (i, cell) in row.iter().enumerate() { + if i >= col_count { + break; + } + let w = measure_text_with_fallback(cell, &font, &emoji_font, 0.0); + col_widths[i] = col_widths[i].max(w + cell_padding * 2.0); + } } + col_widths } /// Get alignment for a specific column. @@ -152,7 +200,7 @@ impl Table { 14.0, ); let col_count = self.headers.len().max(1); - let col_widths = self.resolve_column_widths(w); + let col_widths = self.resolve_column_widths(w, font_size); let row_h = self.row_height(font_size); let header_color = self.header_color.as_deref().unwrap_or("#374151"); diff --git a/crates/rustmotion-components/tests/audit_ws_i.rs b/crates/rustmotion-components/tests/audit_ws_i.rs index d29ff05c..bfd5247d 100644 --- a/crates/rustmotion-components/tests/audit_ws_i.rs +++ b/crates/rustmotion-components/tests/audit_ws_i.rs @@ -1,5 +1,4 @@ -//! Regression tests for workstream I of the 2026-09 audit (components and -//! the CSS cascade). +//! Regression tests for components and the CSS cascade: //! //! - `apply_intrinsic_overrides`'s default-size branches resolved //! `font-size` through the context-free `font_size_px_or`, which returns @@ -8,8 +7,8 @@ //! - `cascade::inherit_from` computes inherited `color`/`font-*` but //! nothing on the render path ever reads the result — every painter reads //! its own component's un-cascaded `style` field instead. -//! - `TableIntrinsic` measures content-fitted per-column widths, but -//! the painter splits the box evenly across columns regardless. +//! - `TableIntrinsic` measures content-fitted per-column widths, but the +//! painter splits the box evenly across columns regardless. use rustmotion_components::box_builder::{build_scene_with_anim, BuildAnimationCtx}; use rustmotion_components::legacy_dispatch::LegacyPaintDispatcher; @@ -30,7 +29,7 @@ fn single_child_scene(json: serde_json::Value) -> ChildComponent { } } -// ─── default sizes must resolve against the node's font-size ─────────────── +// ─── relative font-size on box_builder's default-size branches ──────────── #[test] fn marquee_with_relative_font_size_gets_a_positive_height() { @@ -68,10 +67,10 @@ fn marquee_with_relative_font_size_gets_a_positive_height() { ); } -// ─── ────────────────────────────────────────────────────────────────── +// ─── CSS cascade reaching the painter and the intrinsic measurer ────────── -const RM07_W: i32 = 600; -const RM07_H: i32 = 500; +const CASCADE_SCENE_W: i32 = 600; +const CASCADE_SCENE_H: i32 = 500; struct PaintedScene { pixels: Vec, @@ -84,7 +83,7 @@ fn paint_card_with_text_child(card_json: serde_json::Value) -> PaintedScene { let built = build_scene_with_anim( &children, - (RM07_W as f32, RM07_H as f32), + (CASCADE_SCENE_W as f32, CASCADE_SCENE_H as f32), BuildAnimationCtx { time: 0.0, scenario_time: 0.0, @@ -94,14 +93,14 @@ fn paint_card_with_text_child(card_json: serde_json::Value) -> PaintedScene { ); let layout = run_layout( &built.root, - (RM07_W as f32, RM07_H as f32), + (CASCADE_SCENE_W as f32, CASCADE_SCENE_H as f32), &ConversionContext::default(), ); let text_id = built.root.children[0].children[0].id; let text_layout_height = layout.get(text_id).expect("text laid out").height; - let mut surface = - skia_safe::surfaces::raster_n32_premul((RM07_W, RM07_H)).expect("raster surface"); + let mut surface = skia_safe::surfaces::raster_n32_premul((CASCADE_SCENE_W, CASCADE_SCENE_H)) + .expect("raster surface"); let canvas = surface.canvas(); canvas.clear(skia_safe::Color::BLACK); let dispatcher = LegacyPaintDispatcher::for_scene(&built); @@ -110,17 +109,17 @@ fn paint_card_with_text_child(card_json: serde_json::Value) -> PaintedScene { scenario_time: 0.0, frame_index: 0, fps: 30, - video_width: RM07_W as u32, - video_height: RM07_H as u32, + video_width: CASCADE_SCENE_W as u32, + video_height: CASCADE_SCENE_H as u32, scene_duration: 1.0, camera: None, }; paint_tree(canvas, &built.root, &layout, &frame, &dispatcher); - let row_bytes = RM07_W as usize * 4; - let mut pixels = vec![0u8; row_bytes * RM07_H as usize]; + let row_bytes = CASCADE_SCENE_W as usize * 4; + let mut pixels = vec![0u8; row_bytes * CASCADE_SCENE_H as usize]; let info = skia_safe::ImageInfo::new( - (RM07_W, RM07_H), + (CASCADE_SCENE_W, CASCADE_SCENE_H), skia_safe::ColorType::RGBA8888, skia_safe::AlphaType::Premul, None, @@ -212,3 +211,93 @@ fn text_own_color_wins_over_cascaded_card_color_at_paint_time() { found {red_pixels} red-dominant pixels" ); } + +// ─── table column widths: painter vs. intrinsic measurer ────────────────── + +#[test] +fn table_columns_are_sized_by_content_not_split_evenly() { + // `TableIntrinsic::compute_width` measures each column's own natural + // width (header/cell text + padding) and sums them for the box's + // reserved size, but `Table::resolve_column_widths` (the *painter*'s own + // distribution) just divides the laid-out width evenly across columns + // regardless. A table whose columns have very different natural widths + // ("ID" vs. a long description) used to get a column border sitting at + // the arithmetic midpoint of the box, not near the natural split the + // measurer already computed. + use rustmotion_components::Component; + use rustmotion_core::engine::animator::AnimatedProperties; + use rustmotion_core::engine::layout_pass::BoxLayout; + use rustmotion_core::traits::{PaintCtx, Painter}; + + let component: Component = serde_json::from_value(serde_json::json!({ + "type": "table", + "headers": ["ID", "Description of the incident"], + "rows": [["1", "Something happened during the incident"]] + })) + .expect("deserialize table"); + let Component::Table(table) = component else { + panic!("expected a table component"); + }; + + const W: i32 = 500; + const H: i32 = 100; + let mut surface = skia_safe::surfaces::raster_n32_premul((W, H)).expect("raster surface"); + let canvas = surface.canvas(); + canvas.clear(skia_safe::Color::BLACK); + let layout = BoxLayout { + x: 0.0, + y: 0.0, + width: W as f32, + height: H as f32, + ..Default::default() + }; + let ctx = PaintCtx { + time: 0.0, + scenario_time: 0.0, + scene_duration: 1.0, + frame_index: 0, + fps: 30, + video_width: W as u32, + video_height: H as u32, + stagger_offset: 0.0, + }; + table.paint_content(canvas, &layout, &AnimatedProperties::default(), &ctx); + + let snapshot = surface.image_snapshot(); + let info = skia_safe::ImageInfo::new( + (W, H), + skia_safe::ColorType::RGBA8888, + skia_safe::AlphaType::Premul, + None, + ); + let mut buf = vec![0u8; (W * H * 4) as usize]; + assert!(snapshot.read_pixels( + &info, + &mut buf, + (W * 4) as usize, + skia_safe::IPoint::new(0, 0), + skia_safe::image::CachingHint::Disallow, + )); + + let y = 15usize; + let border_distance = |x: usize| -> i32 { + let idx = (y * W as usize + x) * 4; + let (r, g, b) = (buf[idx] as i32, buf[idx + 1] as i32, buf[idx + 2] as i32); + (r - 0x4B).abs() + (g - 0x55).abs() + (b - 0x63).abs() + }; + let boundary_x = (20usize..(W as usize - 20)) + .min_by_key(|&x| border_distance(x)) + .expect("scan range is non-empty"); + assert!( + border_distance(boundary_x) < 90, + "no column-boundary border line found in the interior of the table \ + (closest match at x={boundary_x}, distance={})", + border_distance(boundary_x) + ); + assert!( + boundary_x < 200, + "the ID/Description column boundary should sit near the natural \ + header-width split, not near the evenly-split midpoint (250) — \ + found it at x={boundary_x}" + ); +}