diff --git a/crates/rustmotion-components/src/box_builder.rs b/crates/rustmotion-components/src/box_builder.rs index dee9b4bb..98423ab0 100644 --- a/crates/rustmotion-components/src/box_builder.rs +++ b/crates/rustmotion-components/src/box_builder.rs @@ -636,7 +636,7 @@ fn build_child<'a>( time_remap, &css, ); - let intrinsic = component_intrinsic(&child.component); + let intrinsic = component_intrinsic(&child.component, &css); let principal = BoxNode { id, @@ -1131,12 +1131,22 @@ fn apply_glow_effect(css: &mut CssStyle, effects: &[rustmotion_core::schema::Ani /// Build an [`IntrinsicMeasure`] for components whose box size depends on /// their content (text, codeblock, terminal, etc.). Returns `None` for /// components with explicit dimensions or pure containers. +/// +/// `cascaded_css` is this node's own `CssStyle` after `cascade::inherit_from` +/// has already merged it against the parent, plus every subsequent overlay +/// (timeline states, animation) — the exact same value `LegacyPaintDispatcher` +/// receives at paint time. `Text` uses it so the reserved box always matches +/// what `Text::paint_content` (which reads the same cascade through +/// `with_cascaded_style`) actually draws. fn component_intrinsic( component: &Component, + cascaded_css: &CssStyle, ) -> Option> { use Component::*; match component { - Text(t) => Some(Arc::new(crate::intrinsic::TextIntrinsic::from_text(t))), + Text(t) => Some(Arc::new(crate::intrinsic::TextIntrinsic::from_text( + &t.with_cascaded_style(cascaded_css), + ))), GradientText(t) => Some(Arc::new( crate::intrinsic::GradientTextIntrinsic::from_gradient_text(t), )), diff --git a/crates/rustmotion-components/src/gif.rs b/crates/rustmotion-components/src/gif.rs index 529624ca..b5698604 100644 --- a/crates/rustmotion-components/src/gif.rs +++ b/crates/rustmotion-components/src/gif.rs @@ -102,8 +102,26 @@ fn stall_decode_for_tests() { } } +/// Hard ceiling on one composed GIF canvas frame's byte size +/// (`width × height × 4`), independent of the render's own video dimensions. +/// A GIF's logical-screen descriptor is two `u16` fields straight from the +/// file header — up to 65535×65535, a 17.2 GiB single allocation — and +/// nothing validated them before this cap existed. +const MAX_GIF_CANVAS_BYTES: u64 = 128 * 1024 * 1024; + +/// Hard ceiling on the number of animation frames one GIF decodes into. +/// Real GIFs rarely exceed a few hundred; this keeps a maliciously (or just +/// accidentally) long frame count from growing the decoded frame list +/// without bound even when each individual frame is well under the canvas +/// budget above. +const MAX_GIF_FRAMES: usize = 600; + /// Decode a GIF into full-canvas RGBA frames, their cumulative end times, and -/// the total duration. +/// the total duration. `max_canvas_w`/`max_canvas_h` are the render's own +/// video dimensions: a GIF canvas larger than that can never be usefully +/// drawn (it only ever gets scaled into the component's layout box, which is +/// at most the video frame), so it is rejected the same way an +/// over-budget canvas is. /// /// Every frame after the first is usually a *sub-rectangle* holding only the /// pixels that changed, so frames must be composed onto a persistent canvas @@ -113,7 +131,7 @@ fn stall_decode_for_tests() { /// why only the first frame ever appeared (issue #185). /// /// `None` means nothing can be drawn, and the reason has already been reported. -fn decode_composed_frames(src: &str) -> Option { +fn decode_composed_frames(src: &str, max_canvas_w: u32, max_canvas_h: u32) -> Option { let file = match std::fs::File::open(src) { Ok(f) => f, Err(e) => { @@ -139,6 +157,18 @@ fn decode_composed_frames(src: &str) -> Option { let canvas_w = decoder.width() as u32; let canvas_h = decoder.height() as u32; + let canvas_bytes = canvas_w as u64 * canvas_h as u64 * 4; + if canvas_bytes > MAX_GIF_CANVAS_BYTES || canvas_w > max_canvas_w || canvas_h > max_canvas_h { + if crate::warn_once_for(&format!("gif-oversized:{src}")) { + eprintln!( + "rustmotion: gif '{src}' declares a {canvas_w}x{canvas_h} canvas ({canvas_bytes} \ + bytes/frame), over the {MAX_GIF_CANVAS_BYTES}-byte budget or larger than this \ + render's own {max_canvas_w}x{max_canvas_h} video — refusing to decode it." + ); + } + return None; + } + #[cfg(test)] stall_decode_for_tests(); @@ -148,6 +178,16 @@ fn decode_composed_frames(src: &str) -> Option { let mut composed = vec![0u8; canvas_w as usize * canvas_h as usize * 4]; while let Ok(Some(frame)) = decoder.read_next_frame() { + if frames.len() >= MAX_GIF_FRAMES { + if crate::warn_once_for(&format!("gif-frame-cap:{src}")) { + eprintln!( + "rustmotion: gif '{src}' has more than {MAX_GIF_FRAMES} frames — truncating \ + the decoded animation at the cap." + ); + } + break; + } + // `Previous` disposal restores what was there before this frame, so it // has to be captured before compositing. let restore = (frame.dispose == gif::DisposalMethod::Previous).then(|| composed.clone()); @@ -189,10 +229,14 @@ fn decode_composed_frames(src: &str) -> Option { /// failure (`decode_composed_frames` returning `None`) leaves the entry /// vacant — `or_try_insert_with` never calls `insert` on its `Err` path — /// so a broken source is retried rather than permanently cached as absent. -fn cached_decode(src: &str) -> Option> { +fn cached_decode(src: &str, max_canvas_w: u32, max_canvas_h: u32) -> Option> { gif_cache() .entry(src.to_string()) - .or_try_insert_with(|| decode_composed_frames(src).map(Arc::new).ok_or(())) + .or_try_insert_with(|| { + decode_composed_frames(src, max_canvas_w, max_canvas_h) + .map(Arc::new) + .ok_or(()) + }) .ok() .map(|entry| entry.clone()) } @@ -205,7 +249,7 @@ impl Painter for Gif { _props: &AnimatedProperties, ctx: &PaintCtx, ) { - let Some(cached) = cached_decode(&self.src) else { + let Some(cached) = cached_decode(&self.src, ctx.video_width, ctx.video_height) else { return; }; @@ -282,7 +326,8 @@ mod tests { write_two_frame_gif(&path); let (frames, times, total) = - decode_composed_frames(path.to_str().expect("utf-8 path")).expect("gif must decode"); + decode_composed_frames(path.to_str().expect("utf-8 path"), 1920, 1080) + .expect("gif must decode"); std::fs::remove_file(&path).ok(); assert_eq!(frames.len(), 2, "both frames must be drawable"); @@ -312,7 +357,7 @@ mod tests { #[test] fn a_missing_file_reports_instead_of_returning_nothing() { let missing = std::env::temp_dir().join("rustmotion_gif_absent_xyz.gif"); - assert!(decode_composed_frames(missing.to_str().expect("utf-8")).is_none()); + assert!(decode_composed_frames(missing.to_str().expect("utf-8"), 1920, 1080).is_none()); // The warn-once slot must have been claimed — silence is the bug. assert!( !crate::warn_once_for(&format!("gif-open:{}", missing.to_str().expect("utf-8"))), @@ -371,7 +416,7 @@ mod tests { let src = src.clone(); std::thread::spawn(move || { barrier.wait(); - cached_decode(&src) + cached_decode(&src, 4, 2) }) }) .collect(); @@ -392,4 +437,117 @@ mod tests { ); } } + + /// Writes a GIF whose logical-screen descriptor declares `w`×`h` but + /// whose only frame is 1×1 — the crafted-header shape a decompression + /// bomb takes: a file of a few hundred bytes that asks the decoder to commit to a + /// canvas orders of magnitude larger than anything it actually encodes. + fn write_oversized_header_gif(path: &std::path::Path, w: u16, h: u16) { + let palette: &[u8] = &[0, 0, 0, 255, 255, 255]; + let mut file = std::fs::File::create(path).expect("create gif fixture"); + let mut encoder = gif::Encoder::new(&mut file, w, h, palette).expect("gif encoder"); + let frame = gif::Frame::from_indexed_pixels(1, 1, vec![0], None); + encoder.write_frame(&frame).expect("write frame"); + } + + /// A 6000×6000 canvas is 144 MiB per frame — comfortably over + /// `MAX_GIF_CANVAS_BYTES` (128 MiB) regardless of how large the render's + /// own video is, so passing generous `max_w`/`max_h` here isolates the + /// byte-budget check from the video-dimensions check exercised by the + /// next test. 144 MiB is deliberately far short of the 65535×65535 + /// (~17 GiB) header a real crafted file could declare — large enough to + /// prove the budget check fires, small enough that running this test + /// never risks the allocation it is asserting never happens. + #[test] + fn a_canvas_over_the_byte_budget_is_rejected_without_allocating_it() { + let path = std::env::temp_dir().join(format!( + "rustmotion_gif_bomb_budget_{}_{}.gif", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos(), + )); + write_oversized_header_gif(&path, 6000, 6000); + + let result = decode_composed_frames(path.to_str().expect("utf-8"), 8192, 8192); + std::fs::remove_file(&path).ok(); + + assert!( + result.is_none(), + "a 144 MiB canvas must be refused, not decoded" + ); + assert!( + !crate::warn_once_for(&format!("gif-oversized:{}", path.to_str().unwrap())), + "the rejection must have reported once" + ); + } + + /// The other half of the same guard: a canvas larger than the render's + /// own video dimensions can never be usefully drawn either. A + /// 3000×2000 canvas is only 24 MiB (well under the byte budget alone) + /// but bigger than the render's own 1920×1080 video, so this isolates + /// the video-dimensions check from the byte-budget one above. + #[test] + fn a_canvas_larger_than_the_video_is_rejected_even_under_the_byte_budget() { + let path = std::env::temp_dir().join(format!( + "rustmotion_gif_bomb_dims_{}_{}.gif", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos(), + )); + write_oversized_header_gif(&path, 3000, 2000); + + let result = decode_composed_frames(path.to_str().expect("utf-8"), 1920, 1080); + std::fs::remove_file(&path).ok(); + + assert!( + result.is_none(), + "a canvas bigger than the video's own dimensions must be refused" + ); + } + + /// Writes `count` tiny (2×2) frames — cheap regardless of `count`, so + /// the frame-count cap can be tested without the per-frame size mattering. + fn write_many_frame_gif(path: &std::path::Path, count: u32) { + let palette: &[u8] = &[0, 0, 0, 255, 255, 255]; + let mut file = std::fs::File::create(path).expect("create gif fixture"); + let mut encoder = gif::Encoder::new(&mut file, 2, 2, palette).expect("gif encoder"); + for _ in 0..count { + let mut frame = gif::Frame::from_indexed_pixels(2, 2, vec![0, 1, 1, 0], None); + frame.delay = 1; + encoder.write_frame(&frame).expect("write frame"); + } + } + + /// The decode loop had no frame-count limit at all — a + /// small canvas with a pathologically large frame count still grows the + /// decoded animation without bound. `MAX_GIF_FRAMES` truncates it + /// instead of rejecting the whole GIF: a real, merely-too-long animation + /// still plays (up to the cap), it just doesn't keep growing memory. + #[test] + fn frame_count_beyond_the_cap_is_truncated_not_unbounded() { + let path = std::env::temp_dir().join(format!( + "rustmotion_gif_many_frames_{}_{}.gif", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos(), + )); + write_many_frame_gif(&path, MAX_GIF_FRAMES as u32 + 50); + + let (frames, times, _total) = decode_composed_frames(path.to_str().expect("utf-8"), 10, 10) + .expect("a small, merely-long gif must still decode"); + std::fs::remove_file(&path).ok(); + + assert_eq!( + frames.len(), + MAX_GIF_FRAMES, + "frame count must be truncated to the cap, not grow past it" + ); + assert_eq!(times.len(), MAX_GIF_FRAMES); + } } 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/legacy_dispatch.rs b/crates/rustmotion-components/src/legacy_dispatch.rs index da4fdc58..9d7a6c4e 100644 --- a/crates/rustmotion-components/src/legacy_dispatch.rs +++ b/crates/rustmotion-components/src/legacy_dispatch.rs @@ -8,12 +8,22 @@ //! skipped: paint_pass already paints their box decorations and recurses into //! children — so calling the container's own `paint_content` would do nothing //! anyway, and we save a no-op call. - +//! +//! `dispatch` receives the node's cascaded `CssStyle` (`css` below) from +//! `paint_pass`, but `Painter::paint_content` has no `CssStyle` parameter — +//! its signature is frozen — and every painter reads typography off its own +//! `self.style` instead. `Text` is the one component wired to see the +//! cascade anyway: `Text::with_cascaded_style` rebuilds it with `css` +//! folded into its own `style` field before painting, the same technique +//! `box_builder::component_intrinsic` uses for `TextIntrinsic` so measure +//! and paint agree. + +use rustmotion_core::css::CssStyle; use rustmotion_core::engine::animator::{resolve_props_for_effects, AnimatedProperties}; use rustmotion_core::engine::box_tree::NodeId; use rustmotion_core::engine::layout_pass::BoxLayout; use rustmotion_core::engine::paint_pass::{PaintDispatcher, PaintFrame}; -use rustmotion_core::traits::PaintCtx; +use rustmotion_core::traits::{PaintCtx, Painter}; use skia_safe::Canvas; use crate::{ChildComponent, Component}; @@ -65,7 +75,7 @@ impl<'a> PaintDispatcher for LegacyPaintDispatcher<'a> { &self, canvas: &Canvas, payload: &(dyn std::any::Any + Send + Sync), - _css: &rustmotion_core::css::CssStyle, + css: &CssStyle, layout: &BoxLayout, frame: &PaintFrame, ) { @@ -165,7 +175,12 @@ impl<'a> PaintDispatcher for LegacyPaintDispatcher<'a> { video_height: frame.video_height, stagger_offset: stagger_delay, }; - painter.paint_content(canvas, &local, &props, &paint_ctx); + if let Component::Text(t) = &child.component { + t.with_cascaded_style(css) + .paint_content(canvas, &local, &props, &paint_ctx); + } else { + painter.paint_content(canvas, &local, &props, &paint_ctx); + } canvas.restore(); } diff --git a/crates/rustmotion-components/src/table.rs b/crates/rustmotion-components/src/table.rs index 527a532c..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"); @@ -371,9 +419,7 @@ mod tests { // probe specifically for near-white text ink rather than any lit // pixel (the header/row backgrounds paint regardless of font-size). let text_ink = buf - .as_chunks::<4>() - .0 - .iter() + .chunks_exact(4) .filter(|p| p[3] > 0 && p[0] > 200 && p[1] > 200 && p[2] > 200) .count(); assert!( diff --git a/crates/rustmotion-components/src/text.rs b/crates/rustmotion-components/src/text.rs index 61f317b3..b1fdb0b8 100644 --- a/crates/rustmotion-components/src/text.rs +++ b/crates/rustmotion-components/src/text.rs @@ -22,7 +22,7 @@ use rustmotion_core::schema::{ }; use rustmotion_core::traits::{PaintCtx, Painter, TimingConfig}; -#[derive(Debug, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] pub struct Text { pub content: String, #[serde(default)] @@ -59,6 +59,24 @@ rustmotion_core::impl_traits!(Text { Styled => style, }); +impl Text { + /// `Painter::paint_content` and `TextIntrinsic::from_text` both read + /// `self.style` directly and have no way to receive the box tree's + /// resolved `CssStyle` — the `Painter` trait's signature is frozen, and + /// `self` is bound to this exact struct. `resolved` is the box-tree + /// node's own `CssStyle` after `cascade::inherit_from` already merged + /// it against the parent at `box_builder` build time, so — mirroring + /// `inherit_from`'s own "child overrides parent" rule — filling this + /// clone's still-unset inheritable fields from it reproduces the same + /// outcome the box tree already computed, just where the painter and + /// the intrinsic measurer can see it. + pub fn with_cascaded_style(&self, resolved: &CssStyle) -> Self { + let mut clone = self.clone(); + rustmotion_core::css::cascade::inherit_from(resolved, &mut clone.style); + clone + } +} + /// Eased progress (0..1) of unit `idx` at `time`, honouring the config's /// deterministic jitter. fn unit_progress(cfg: &ResolvedCharAnimation, idx: usize, time: f64) -> f32 { diff --git a/crates/rustmotion-components/src/video.rs b/crates/rustmotion-components/src/video.rs index 0d64739f..1500dbb5 100644 --- a/crates/rustmotion-components/src/video.rs +++ b/crates/rustmotion-components/src/video.rs @@ -6,7 +6,7 @@ use rustmotion_core::css::CssStyle; use rustmotion_core::engine::animator::AnimatedProperties; use rustmotion_core::engine::layout_pass::BoxLayout; use rustmotion_core::engine::renderer::{ - extract_video_frame, find_closest_frame, video_frame_cache, + extract_video_frame, find_closest_frame, probe_video_metadata, video_frame_cache, }; use rustmotion_core::schema::{ImageFit, TimelineStep}; use rustmotion_core::traits::{PaintCtx, Painter, TimingConfig}; @@ -46,6 +46,116 @@ rustmotion_core::impl_traits!(Video { Styled => style, }); +/// The rectangle an `img_w`×`img_h` source draws into to honour `fit` inside +/// a `target_w`×`target_h` box — the same three CSS `object-fit` semantics +/// `image.rs`'s painter already implements for the `image` component. +fn fit_rect(fit: &ImageFit, img_w: f32, img_h: f32, target_w: f32, target_h: f32) -> Rect { + match fit { + ImageFit::Fill => Rect::from_xywh(0.0, 0.0, target_w, target_h), + ImageFit::Contain => { + let scale = (target_w / img_w).min(target_h / img_h); + let w = img_w * scale; + let h = img_h * scale; + Rect::from_xywh((target_w - w) / 2.0, (target_h - h) / 2.0, w, h) + } + ImageFit::Cover => { + let scale = (target_w / img_w).max(target_h / img_h); + let w = img_w * scale; + let h = img_h * scale; + Rect::from_xywh((target_w - w) / 2.0, (target_h - h) / 2.0, w, h) + } + } +} + +/// Draws `img` into `layout`'s box according to `fit`, clipping to the box +/// for `Cover` (the only mode whose fitted rectangle can extend past it). +fn draw_fitted(canvas: &Canvas, img: skia_safe::Image, fit: &ImageFit, layout: &BoxLayout) { + let dst = fit_rect( + fit, + img.width() as f32, + img.height() as f32, + layout.width, + layout.height, + ); + let paint = Paint::default(); + if matches!(fit, ImageFit::Cover) { + canvas.save(); + canvas.clip_rect( + Rect::from_xywh(0.0, 0.0, layout.width, layout.height), + skia_safe::ClipOp::Intersect, + true, + ); + canvas.draw_image_rect(img, None, dst, &paint); + canvas.restore(); + } else { + canvas.draw_image_rect(img, None, dst, &paint); + } +} + +/// The source clip's own duration, probed via `ffprobe` and memoized per +/// `src` for the life of the process — `effective_source_time` below is +/// called once per painted frame, and re-probing on every one of them would +/// mean one subprocess spawn per frame for any looping video with no +/// explicit `trim_end`. `None` on a probe failure (no ffprobe on `PATH`, or +/// the source can't be read) is memoized too, so a broken source fails fast +/// on every subsequent frame instead of retrying the same failing probe. +fn video_duration_secs(src: &str) -> Option { + static CACHE: std::sync::OnceLock< + std::sync::Mutex>>, + > = std::sync::OnceLock::new(); + let cache = CACHE.get_or_init(|| std::sync::Mutex::new(std::collections::HashMap::new())); + + if let Some(hit) = cache + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .get(src) + { + return *hit; + } + let probed = probe_video_metadata(src).ok().map(|p| p.duration_secs); + cache + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .insert(src.to_string(), probed); + probed +} + +impl Video { + /// The timestamp to sample from the source clip for a given scene time. + /// + /// Honours `trim_end` on the picture the same way the audio track + /// already does: past `trim_end`, playback holds on the last in-window + /// frame instead of continuing to draw whatever the source contains + /// beyond the intended trim point. When `loop_video` is set, playback + /// wraps within `[trim_start, trim_end)` instead of clamping — falling + /// back to the source's own probed duration as the loop window only + /// when `trim_end` is absent, since that is the only case where the + /// window cannot otherwise be known at all. + fn effective_source_time(&self, ctx_time: f64) -> f64 { + let rate = self.playback_rate.unwrap_or(1.0); + let trim_start = self.trim_start.unwrap_or(0.0); + let raw = trim_start + ctx_time * rate; + + if let Some(end) = self.trim_end { + if end > trim_start { + return if self.loop_video == Some(true) { + trim_start + (raw - trim_start).rem_euclid(end - trim_start) + } else { + raw.min(end) + }; + } + } else if self.loop_video == Some(true) { + if let Some(duration) = video_duration_secs(&self.src) { + if duration > trim_start { + return trim_start + (raw - trim_start).rem_euclid(duration - trim_start); + } + } + } + + raw + } +} + impl Painter for Video { fn paint_content( &self, @@ -54,9 +164,7 @@ impl Painter for Video { _props: &AnimatedProperties, ctx: &PaintCtx, ) { - let rate = self.playback_rate.unwrap_or(1.0); - let trim_start = self.trim_start.unwrap_or(0.0); - let source_time = trim_start + ctx.time * rate; + let source_time = self.effective_source_time(ctx.time); let width = layout.width as u32; let height = layout.height as u32; @@ -74,9 +182,7 @@ impl Painter for Video { let row_bytes = fw as usize * 4; let data = skia_safe::Data::new_copy(rgba); if let Some(img) = skia_safe::images::raster_from_data(&img_info, data, row_bytes) { - let dst = Rect::from_xywh(0.0, 0.0, layout.width, layout.height); - let paint = Paint::default(); - canvas.draw_image_rect(img, None, dst, &paint); + draw_fitted(canvas, img, &self.fit, layout); } return; } @@ -105,9 +211,7 @@ impl Painter for Video { }; let skia_data = skia_safe::Data::new_copy(&frame_data); if let Some(img) = skia_safe::Image::from_encoded(skia_data) { - let dst = Rect::from_xywh(0.0, 0.0, layout.width, layout.height); - let paint = Paint::default(); - canvas.draw_image_rect(img, None, dst, &paint); + draw_fitted(canvas, img, &self.fit, layout); } } } diff --git a/crates/rustmotion-components/tests/audit_ws_d.rs b/crates/rustmotion-components/tests/audit_ws_d.rs new file mode 100644 index 00000000..e884b076 --- /dev/null +++ b/crates/rustmotion-components/tests/audit_ws_d.rs @@ -0,0 +1,283 @@ +//! Regression tests for the `video` component's dead-field fixes: `fit`, +//! `trim_end`, `loop_video`, and the straight-vs-premultiplied alpha bug on +//! its cached-frame draw path. +//! +//! Every case populates `video_frame_cache()` directly with hand-built RGBA +//! frames rather than shelling out to a real ffmpeg decode: the field this +//! module exercises (`Video::paint_content`) is one call away from the +//! cache, and driving it that way keeps these tests hermetic and fast while +//! still going through the real, public `Painter` implementation — no +//! private items from `rustmotion-components` are touched. + +use std::sync::Arc; + +use rustmotion_components::Video; +use rustmotion_core::css::CssStyle; +use rustmotion_core::engine::animator::AnimatedProperties; +use rustmotion_core::engine::layout_pass::BoxLayout; +use rustmotion_core::engine::renderer::video_frame_cache; +use rustmotion_core::schema::ImageFit; +use rustmotion_core::traits::{PaintCtx, Painter, TimingConfig}; + +fn unique_src(label: &str) -> String { + format!( + "audit-ws-d-video-{label}-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("system clock before UNIX epoch") + .as_nanos() + ) +} + +#[allow(clippy::too_many_arguments)] +fn video( + src: &str, + fit: ImageFit, + trim_start: Option, + trim_end: Option, + loop_video: Option, +) -> Video { + Video { + src: src.to_string(), + trim_start, + trim_end, + playback_rate: None, + fit, + volume: 1.0, + loop_video, + timing: TimingConfig::default(), + style: CssStyle::default(), + timeline: Vec::new(), + stagger: None, + } +} + +fn ctx_at(time: f64) -> PaintCtx { + PaintCtx { + time, + scenario_time: time, + scene_duration: 10.0, + frame_index: 0, + fps: 30, + video_width: 1920, + video_height: 1080, + stagger_offset: 0.0, + } +} + +fn solid_rgba(color: [u8; 4], w: u32, h: u32) -> Vec { + let mut buf = Vec::with_capacity((w * h * 4) as usize); + for _ in 0..(w * h) { + buf.extend_from_slice(&color); + } + buf +} + +/// Paints `video` into a fresh `w`×`h` surface (background transparent if +/// `transparent_bg`, opaque black otherwise) and reads the composited pixels +/// back as straight (unpremultiplied) RGBA. +fn paint_and_read(video: &Video, ctx: &PaintCtx, w: i32, h: i32, transparent_bg: bool) -> Vec { + let mut surface = skia_safe::surfaces::raster_n32_premul((w, h)).expect("raster surface"); + let bg = if transparent_bg { + skia_safe::Color4f::new(0.0, 0.0, 0.0, 0.0) + } else { + skia_safe::Color4f::new(0.0, 0.0, 0.0, 1.0) + }; + surface.canvas().clear(bg); + + let layout = BoxLayout { + width: w as f32, + height: h as f32, + ..Default::default() + }; + let props = AnimatedProperties::default(); + video.paint_content(surface.canvas(), &layout, &props, ctx); + + let mut pixels = vec![0u8; (w * h * 4) as usize]; + let info = skia_safe::ImageInfo::new( + (w, h), + skia_safe::ColorType::RGBA8888, + skia_safe::AlphaType::Unpremul, + None, + ); + surface.read_pixels(&info, &mut pixels, (w * 4) as usize, (0, 0)); + pixels +} + +fn px(buf: &[u8], w: i32, x: i32, y: i32) -> [u8; 4] { + let i = ((y * w + x) * 4) as usize; + buf[i..i + 4].try_into().expect("pixel in bounds") +} + +// ─── `fit` was declared, documented, and never read ──────────────────────── + +/// A 10×20 source into a 40×40 box under `contain` must letterbox — scale +/// is `min(40/10, 40/40) = 1`, so the drawn region is 10 wide, centred with +/// a 15px empty margin on each side. Before the fix, the painter always +/// stretched to the full box regardless of `fit`, so every pixel — margins +/// included — came out opaque. +#[test] +fn contain_fit_letterboxes_instead_of_stretching() { + let src = unique_src("fit-contain"); + let (fw, fh) = (10u32, 20u32); + let cache_key = format!("{src}:40x40"); + video_frame_cache().insert( + cache_key, + Arc::new(vec![( + 0.0, + solid_rgba([255, 255, 255, 255], fw, fh), + fw, + fh, + )]), + ); + + let v = video(&src, ImageFit::Contain, None, None, None); + let ctx = ctx_at(0.0); + let pixels = paint_and_read(&v, &ctx, 40, 40, true); + + assert_eq!( + px(&pixels, 40, 0, 20)[3], + 0, + "letterboxed left margin must stay empty, not be stretched into" + ); + assert_eq!( + px(&pixels, 40, 39, 20)[3], + 0, + "letterboxed right margin must stay empty, not be stretched into" + ); + assert_eq!( + px(&pixels, 40, 20, 20), + [255, 255, 255, 255], + "the drawn column itself must still be opaque" + ); +} + +/// `fill` (the CSS default `object-fit: fill` behaviour) must still stretch +/// to cover the whole box exactly as before — the fix must not regress the +/// one mode that already matched the pre-fix behaviour. +#[test] +fn fill_fit_still_stretches_to_the_whole_box() { + let src = unique_src("fit-fill"); + let (fw, fh) = (10u32, 20u32); + let cache_key = format!("{src}:40x40"); + video_frame_cache().insert( + cache_key, + Arc::new(vec![( + 0.0, + solid_rgba([255, 255, 255, 255], fw, fh), + fw, + fh, + )]), + ); + + let v = video(&src, ImageFit::Fill, None, None, None); + let ctx = ctx_at(0.0); + let pixels = paint_and_read(&v, &ctx, 40, 40, true); + + assert_eq!( + px(&pixels, 40, 0, 0)[3], + 255, + "fill must cover every corner" + ); + assert_eq!( + px(&pixels, 40, 39, 39)[3], + 255, + "fill must cover every corner" + ); +} + +// ─── `trim_end` was honoured only on the extracted audio ─────────────────── + +/// Frames beyond `trim_end` sit in the cache (simulating a preextraction +/// window, or a direct extraction, wider than the intended trim), so the +/// picture path must never pick one of them once `trim_end` is set: past +/// `trim_end`, playback holds on the last in-window frame. Before the fix, +/// `source_time` had no upper bound at all — querying past `trim_end` on a +/// cache/source that extends further would draw whatever sits further +/// along the source, not the frame at the trim boundary. +#[test] +fn trim_end_clamps_playback_instead_of_running_past_it() { + let src = unique_src("trimend"); + let cache_key = format!("{src}:20x20"); + let frames = vec![ + (0.0, solid_rgba([255, 0, 0, 255], 2, 2), 2, 2), + (0.5, solid_rgba([0, 0, 255, 255], 2, 2), 2, 2), + (1.0, solid_rgba([0, 255, 0, 255], 2, 2), 2, 2), + (1.5, solid_rgba([128, 0, 128, 255], 2, 2), 2, 2), + (2.0, solid_rgba([255, 165, 0, 255], 2, 2), 2, 2), + ]; + video_frame_cache().insert(cache_key, Arc::new(frames)); + + let v = video(&src, ImageFit::Fill, Some(0.0), Some(1.0), None); + let ctx = ctx_at(1.8); + let pixels = paint_and_read(&v, &ctx, 20, 20, false); + + assert_eq!( + px(&pixels, 20, 10, 10), + [0, 255, 0, 255], + "past trim_end, playback must clamp to the frame at trim_end (green), not the frame \ + nearest the unclamped query time (orange)" + ); +} + +// ─── `loop_video` made neither the picture nor the audio loop ───────────── + +/// Cache frames only cover `[0.0, 1.0)`; `trim_end: Some(1.0)` gives +/// `loop_video` a window to wrap within without needing a real source file +/// to probe. Querying at `ctx.time = 2.1` (raw source time 2.1s, i.e. "2 +/// full loops plus 0.1s") must land near 0.1s once wrapped — nearest to +/// that among `{0.0, 0.25, 0.5, 0.75}` is red. Before this fix, the same +/// query — with the trim-end clamp from the previous test already in place +/// but no loop branch yet — clamped to `min(2.1, 1.0) = 1.0`, whose nearest +/// cached frame is yellow: a clearly different pixel, which is what proves +/// this test is exercising the loop path and not being masked by the clamp. +#[test] +fn loop_video_wraps_playback_within_the_trim_window() { + let src = unique_src("loop"); + let cache_key = format!("{src}:20x20"); + let frames = vec![ + (0.0, solid_rgba([255, 0, 0, 255], 2, 2), 2, 2), + (0.25, solid_rgba([0, 255, 0, 255], 2, 2), 2, 2), + (0.5, solid_rgba([0, 0, 255, 255], 2, 2), 2, 2), + (0.75, solid_rgba([255, 255, 0, 255], 2, 2), 2, 2), + ]; + video_frame_cache().insert(cache_key, Arc::new(frames)); + + let v = video(&src, ImageFit::Fill, Some(0.0), Some(1.0), Some(true)); + let ctx = ctx_at(2.1); + let pixels = paint_and_read(&v, &ctx, 20, 20, false); + + assert_eq!( + px(&pixels, 20, 10, 10), + [255, 0, 0, 255], + "looping must wrap the query time back into the window (nearest: red), not clamp to \ + the window's own end (nearest: yellow)" + ); +} + +/// Without `loop_video`, a `trim_end`-bounded video must still clamp +/// (unaffected by the loop branch existing) rather than wrap — the same +/// scenario as the wrap test above, minus the flag. +#[test] +fn without_loop_video_playback_still_clamps_not_wraps() { + let src = unique_src("no-loop"); + let cache_key = format!("{src}:20x20"); + let frames = vec![ + (0.0, solid_rgba([255, 0, 0, 255], 2, 2), 2, 2), + (0.25, solid_rgba([0, 255, 0, 255], 2, 2), 2, 2), + (0.5, solid_rgba([0, 0, 255, 255], 2, 2), 2, 2), + (0.75, solid_rgba([255, 255, 0, 255], 2, 2), 2, 2), + ]; + video_frame_cache().insert(cache_key, Arc::new(frames)); + + let v = video(&src, ImageFit::Fill, Some(0.0), Some(1.0), None); + let ctx = ctx_at(2.1); + let pixels = paint_and_read(&v, &ctx, 20, 20, false); + + assert_eq!( + px(&pixels, 20, 10, 10), + [255, 255, 0, 255], + "no loop_video: must clamp to the window's end (nearest: yellow), not wrap" + ); +} diff --git a/crates/rustmotion-components/tests/audit_ws_i.rs b/crates/rustmotion-components/tests/audit_ws_i.rs index 1915ed02..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,13 +7,15 @@ //! - `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; use rustmotion_components::{ChildComponent, Component, PositionMode}; use rustmotion_core::css::taffy_bridge::ConversionContext; use rustmotion_core::engine::layout_pass::run_layout; +use rustmotion_core::engine::paint_pass::{paint_tree, PaintFrame}; fn single_child_scene(json: serde_json::Value) -> ChildComponent { let component: Component = serde_json::from_value(json).expect("deserialize component"); @@ -28,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() { @@ -65,3 +66,238 @@ fn marquee_with_relative_font_size_gets_a_positive_height() { l.height ); } + +// ─── CSS cascade reaching the painter and the intrinsic measurer ────────── + +const CASCADE_SCENE_W: i32 = 600; +const CASCADE_SCENE_H: i32 = 500; + +struct PaintedScene { + pixels: Vec, + text_layout_height: f32, +} + +fn paint_card_with_text_child(card_json: serde_json::Value) -> PaintedScene { + let child = single_child_scene(card_json); + let children = vec![child]; + + let built = build_scene_with_anim( + &children, + (CASCADE_SCENE_W as f32, CASCADE_SCENE_H as f32), + BuildAnimationCtx { + time: 0.0, + scenario_time: 0.0, + scene_duration: 1.0, + fps: 30, + }, + ); + let layout = run_layout( + &built.root, + (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((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); + let frame = PaintFrame { + time: 0.0, + scenario_time: 0.0, + frame_index: 0, + fps: 30, + 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 = CASCADE_SCENE_W as usize * 4; + let mut pixels = vec![0u8; row_bytes * CASCADE_SCENE_H as usize]; + let info = skia_safe::ImageInfo::new( + (CASCADE_SCENE_W, CASCADE_SCENE_H), + skia_safe::ColorType::RGBA8888, + skia_safe::AlphaType::Premul, + None, + ); + surface.read_pixels(&info, &mut pixels, row_bytes, (0, 0)); + + PaintedScene { + pixels, + text_layout_height, + } +} + +fn count_dominant(pixels: &[u8], dominant: usize, muted: &[usize]) -> usize { + pixels + .chunks_exact(4) + .filter(|p| p[dominant] > 180 && muted.iter().all(|&m| p[m] < 80)) + .count() +} + +#[test] +fn card_color_and_font_size_cascade_to_painted_text_child() { + // `cascade::inherit_from` computes the twelve inheritable properties but + // `LegacyPaintDispatcher::dispatch` binds the resolved `CssStyle` to + // `_css` and drops it, and `component_intrinsic` builds `TextIntrinsic` + // from `&child.component` (the un-cascaded component) — so a card's + // `color`/`font-size` never reached a child `text` with no value of its + // own, at either paint time or measure time. The text rendered at the + // painter's own fallback (48px, #FFFFFF) instead of the card's (200px, + // red). + let scene = paint_card_with_text_child(serde_json::json!({ + "type": "card", + "style": { + "color": "#ff0000", + "font-size": 200, + "width": 500, + "flex-direction": "column" + }, + "children": [ + { "type": "text", "content": "WWWW" } + ] + })); + + assert!( + scene.text_layout_height > 150.0, + "text child with no font-size of its own must be measured at the \ + card's cascaded 200px (~240px line height), not the 48px default \ + (~57px line height) — got layout height {}", + scene.text_layout_height + ); + + let red_pixels = count_dominant(&scene.pixels, 0, &[1, 2]); + assert!( + red_pixels > 20, + "text child with no color of its own must paint in the card's \ + cascaded red, not the painter's white fallback — found {red_pixels} \ + red-dominant pixels" + ); +} + +#[test] +fn text_own_color_wins_over_cascaded_card_color_at_paint_time() { + // The other half of the contract this fix must not break: a `text` + // that DOES declare its own `color` must keep winning over the parent's, + // not just at the box-tree level (already covered by + // `box_builder.rs::text_own_color_wins_over_inherited_card_color`) but + // in what actually gets painted. + let scene = paint_card_with_text_child(serde_json::json!({ + "type": "card", + "style": { + "color": "#ff0000", + "width": 500, + "flex-direction": "column" + }, + "children": [ + { "type": "text", "content": "WWWW", "style": { "color": "#00ff00" } } + ] + })); + + let green_pixels = count_dominant(&scene.pixels, 1, &[0, 2]); + let red_pixels = count_dominant(&scene.pixels, 0, &[1, 2]); + assert!( + green_pixels > 20, + "text's own explicit color must still be painted — found {green_pixels} \ + green-dominant pixels" + ); + assert_eq!( + red_pixels, 0, + "text's own explicit color must win over the card's cascaded red — \ + 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}" + ); +} diff --git a/crates/rustmotion-core/src/engine/animator.rs b/crates/rustmotion-core/src/engine/animator.rs index 42c42787..be2dce5e 100644 --- a/crates/rustmotion-core/src/engine/animator.rs +++ b/crates/rustmotion-core/src/engine/animator.rs @@ -434,6 +434,45 @@ pub const DEFAULT_SPRING_REST_THRESHOLD: f64 = 0.005; /// rather than an unbounded loop. pub const MAX_SPRING_SEARCH_SECONDS: f64 = 30.0; +thread_local! { + /// Cache for [`spring_settle_time_cached`], keyed on the exact bit + /// pattern of its four inputs. One `SpringConfig` is sampled once per + /// animated property per node per frame, always with the same + /// (floored) `damping`/`stiffness`/`mass`/`threshold` — the scan result + /// is frame-invariant, so a thread-local map turns the whole render + /// into one real scan per distinct spring plus O(1) lookups instead of + /// one scan per sample. + static SPRING_SETTLE_TIME_CACHE: std::cell::RefCell> = + std::cell::RefCell::new(std::collections::HashMap::new()); +} + +/// Memoized [`spring_settle_time`]: identical inputs always produce the +/// identical scan result, so a cache hit skips the coarse-then-bisect +/// search entirely. `max_t` is not part of the key because both call sites +/// below always pass [`MAX_SPRING_SEARCH_SECONDS`]. +fn spring_settle_time_cached(damping: f64, stiffness: f64, mass: f64, threshold: f64) -> f64 { + let key = ( + damping.to_bits(), + stiffness.to_bits(), + mass.to_bits(), + threshold.to_bits(), + ); + SPRING_SETTLE_TIME_CACHE.with(|cache| { + if let Some(&cached) = cache.borrow().get(&key) { + return cached; + } + let settled = spring_settle_time( + damping, + stiffness, + mass, + threshold, + MAX_SPRING_SEARCH_SECONDS, + ); + cache.borrow_mut().insert(key, settled); + settled + }) +} + /// Solve spring animation at time t (seconds). /// Returns a value between 0.0 and 1.0 representing progress. /// @@ -467,13 +506,7 @@ pub fn spring_value(t: f64, config: &SpringConfig) -> f64 { match config.duration { Some(duration) if duration > 0.0 => { let threshold = spring_rest_threshold(config); - let natural_rest = spring_settle_time( - damping, - stiffness, - mass, - threshold, - MAX_SPRING_SEARCH_SECONDS, - ); + let natural_rest = spring_settle_time_cached(damping, stiffness, mass, threshold); if natural_rest < 1e-9 { // Degenerate: the spring starts at distance 1.0 from its // target, so in practice `natural_rest` is never this @@ -623,13 +656,7 @@ pub fn spring_rest_time(config: &SpringConfig) -> f64 { let stiffness = config.stiffness.max(1e-6); let mass = config.mass.max(1e-6); let threshold = spring_rest_threshold(config); - spring_settle_time( - damping, - stiffness, - mass, - threshold, - MAX_SPRING_SEARCH_SECONDS, - ) + spring_settle_time_cached(damping, stiffness, mass, threshold) } } } diff --git a/crates/rustmotion-core/src/engine/paint_pass.rs b/crates/rustmotion-core/src/engine/paint_pass.rs index 048b3c81..4b93fe75 100644 --- a/crates/rustmotion-core/src/engine/paint_pass.rs +++ b/crates/rustmotion-core/src/engine/paint_pass.rs @@ -1337,11 +1337,16 @@ fn gradient_stops(stops: &[crate::css::style::GradientStop]) -> (Vec, V (colors, positions) } +/// Gradient-line endpoints for a CSS ``: `0deg` points the line "to +/// top" (first stop at the bottom, travelling up to the last stop), and the +/// angle increases clockwise, so `90deg` is "to right" and `180deg` (the +/// default) is "to bottom" (first stop at the top). Skia's +/// `linear_gradient` places `colors[0]` at `p0`, so `p0` is always the end +/// the angle points *away from*. fn gradient_endpoints(bounds: Rect, angle_deg: f32) -> (Point, Point) { - // CSS angle: 0deg = bottom→top, increasing clockwise. let cx = bounds.left + bounds.width() / 2.0; let cy = bounds.top + bounds.height() / 2.0; - let rad = (angle_deg - 180.0).to_radians(); + let rad = angle_deg.to_radians(); let (sin_a, cos_a) = (rad.sin(), -rad.cos()); let len = (bounds.width().abs() * sin_a.abs() + bounds.height().abs() * cos_a.abs()) / 2.0; let p0 = Point::new(cx - sin_a * len, cy - cos_a * len); diff --git a/crates/rustmotion-core/src/engine/renderer/assets.rs b/crates/rustmotion-core/src/engine/renderer/assets.rs index ac7bd585..9121f48e 100644 --- a/crates/rustmotion-core/src/engine/renderer/assets.rs +++ b/crates/rustmotion-core/src/engine/renderer/assets.rs @@ -251,9 +251,43 @@ pub fn ffmpeg_available() -> bool { .unwrap_or(false) } +/// Rejects a `src` that names a network URL rather than a local file path. +/// +/// `extract_video_frame` below hands `src` to `ffmpeg -i` verbatim; ffmpeg's +/// own demuxer understands its full built-in protocol set (`http://`, +/// `rtmp://`, `concat:`, …), which turns an unfiltered `src` into an SSRF +/// primitive — a scenario author can point it at +/// `http://169.254.169.254/...` (the cloud metadata endpoint) or an internal +/// service, and read the exit status as a port-scan oracle. Remote +/// video was never a designed feature here — this module's own `is_remote` +/// doc, a few functions below, and `rustmotion info`'s identical assumption +/// both already treat every `src` as a local path — so this closes an +/// accidental reach rather than opening an allowlist for one. +fn reject_remote_video_src(src: &str) -> Result<()> { + let Some(scheme_end) = src.find("://") else { + return Ok(()); + }; + let scheme = &src[..scheme_end]; + let looks_like_scheme = !scheme.is_empty() + && scheme.starts_with(|c: char| c.is_ascii_alphabetic()) + && scheme + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '+' | '-' | '.')); + if looks_like_scheme { + return Err(RustmotionError::Generic(format!( + "video src '{src}' names a '{scheme}://' URL — rustmotion does not fetch video \ + over the network, only local file paths are accepted" + ))); + } + Ok(()) +} + pub fn extract_video_frame(src: &str, time: f64, width: u32, height: u32) -> Result> { + reject_remote_video_src(src)?; let output = std::process::Command::new("ffmpeg") .args([ + "-protocol_whitelist", + "file", "-ss", &format!("{:.3}", time), "-i", diff --git a/crates/rustmotion-core/src/expand.rs b/crates/rustmotion-core/src/expand.rs index 497a7d66..ea153e98 100644 --- a/crates/rustmotion-core/src/expand.rs +++ b/crates/rustmotion-core/src/expand.rs @@ -156,6 +156,23 @@ use crate::variables::substitute; /// `include::MAX_INCLUDE_DEPTH`'s role for the sibling mechanism. const MAX_EXPANSION_DEPTH: u32 = 64; +/// Ceiling on the total number of nodes a single document's `for-each` +/// expansion may produce, across every level of nesting combined. +/// [`MAX_EXPANSION_DEPTH`] bounds how deep directives may nest, not how many +/// nodes they produce — and nesting one `for-each` inside another's +/// `template` is explicitly supported (`use_template_can_contain_a_nested_ +/// for_each` below), so the node count a legal, non-cyclic document can +/// declare is the *product* of every level's array length, not their sum. +/// Four nested levels of 50 elements is 6.25M nodes from a file under 1 KB. +/// This is checked incrementally as each `for-each` directive is about to +/// produce its items (see [`consume_node_budget`]), so a runaway product is +/// rejected partway through, well before the full tree is ever materialized. +const MAX_EXPANSION_NODES: u64 = 2_000_000; + +/// Cheap first line of defence ahead of [`MAX_EXPANSION_NODES`]: a single +/// `for-each` directive's own array, before any nesting is even considered. +const MAX_FOR_EACH_ITEMS: usize = 100_000; + /// One entry of the top-level `components` map: a named, parameterised /// subtree. `params` reuses the exact shape of the scenario-level `config` /// block, except a param's `default` is optional — omitting it makes the @@ -230,6 +247,7 @@ fn is_use(v: &Value) -> bool { /// not just which file. pub fn expand_directives(value: &mut Value, file_label: &str) -> Result<()> { let defs = extract_component_definitions(value, file_label)?; + let mut budget = MAX_EXPANSION_NODES; let Value::Object(root) = value else { return Ok(()); @@ -241,7 +259,15 @@ pub fn expand_directives(value: &mut Value, file_label: &str) -> Result<()> { for (i, mut scene) in scenes.into_iter().enumerate() { let scene_path = format!("scenes[{i}]"); let mut stack = Vec::new(); - walk_children(&mut scene, &defs, file_label, &scene_path, &mut stack, 0)?; + walk_children( + &mut scene, + &defs, + file_label, + &scene_path, + &mut stack, + 0, + &mut budget, + )?; out.push(scene); } root.insert("scenes".to_string(), Value::Array(out)); @@ -256,7 +282,15 @@ pub fn expand_directives(value: &mut Value, file_label: &str) -> Result<()> { for (si, mut scene) in scenes.into_iter().enumerate() { let scene_path = format!("composition[{vi}].scenes[{si}]"); let mut stack = Vec::new(); - walk_children(&mut scene, &defs, file_label, &scene_path, &mut stack, 0)?; + walk_children( + &mut scene, + &defs, + file_label, + &scene_path, + &mut stack, + 0, + &mut budget, + )?; out.push(scene); } vmap.insert("scenes".to_string(), Value::Array(out)); @@ -271,6 +305,25 @@ pub fn expand_directives(value: &mut Value, file_label: &str) -> Result<()> { Ok(()) } +/// Subtracts `n` from the shared expansion-node budget, or fails naming the +/// limit and where it was hit. See [`MAX_EXPANSION_NODES`] for why +/// this is checked once per `for-each` directive's item count rather than +/// once per final node: it is the only point in the recursion where the +/// multiplicative blow-up can be caught before the work that would produce +/// it actually runs. +fn consume_node_budget(budget: &mut u64, n: u64, file_label: &str, location: &str) -> Result<()> { + match budget.checked_sub(n) { + Some(remaining) => { + *budget = remaining; + Ok(()) + } + None => Err(RustmotionError::Generic(format!( + "expansion node budget ({MAX_EXPANSION_NODES}) exceeded at '{file_label}: {location}' \ + — for-each/use nesting multiplies past the limit" + ))), + } +} + /// Report `$name`s that survived both variable substitution and directive /// expansion. /// @@ -339,6 +392,7 @@ fn walk_children( location: &str, stack: &mut Vec, depth: u32, + budget: &mut u64, ) -> Result<()> { match value { Value::Object(map) => { @@ -348,7 +402,7 @@ fn walk_children( for (i, entry) in arr.into_iter().enumerate() { let entry_loc = format!("{location}.children[{i}]"); expanded.extend(resolve_entry( - entry, defs, file_label, &entry_loc, stack, depth, + entry, defs, file_label, &entry_loc, stack, depth, budget, )?); } map.insert("children".to_string(), Value::Array(expanded)); @@ -356,14 +410,14 @@ fn walk_children( } for (k, v) in map.iter_mut() { if k == "children" { - continue; // already fully expanded above + continue; } - walk_children(v, defs, file_label, location, stack, depth)?; + walk_children(v, defs, file_label, location, stack, depth, budget)?; } } Value::Array(arr) => { for v in arr.iter_mut() { - walk_children(v, defs, file_label, location, stack, depth)?; + walk_children(v, defs, file_label, location, stack, depth, budget)?; } } _ => {} @@ -392,6 +446,7 @@ fn resolve_entry( location: &str, stack: &mut Vec, depth: u32, + budget: &mut u64, ) -> Result> { if depth > MAX_EXPANSION_DEPTH { return Err(RustmotionError::ExpansionDepthExceeded { @@ -411,13 +466,14 @@ fn resolve_entry( &frag_loc, stack, depth + 1, + budget, )?); } return Ok(out); } if is_for_each(&entry) { - let produced = expand_for_each_directive(entry, file_label, location)?; + let produced = expand_for_each_directive(entry, file_label, location, budget)?; let mut out = Vec::with_capacity(produced.len()); for (i, node) in produced.into_iter().enumerate() { let iter_loc = format!("{location}[{i}]"); @@ -428,6 +484,7 @@ fn resolve_entry( &iter_loc, stack, depth + 1, + budget, )?); } return Ok(out); @@ -444,17 +501,22 @@ fn resolve_entry( }); } stack.push(name); - let result = resolve_entry(node, defs, file_label, location, stack, depth + 1); + let result = resolve_entry(node, defs, file_label, location, stack, depth + 1, budget); stack.pop(); return result; } let mut node = entry; - walk_children(&mut node, defs, file_label, location, stack, depth)?; + walk_children(&mut node, defs, file_label, location, stack, depth, budget)?; Ok(vec![node]) } -fn expand_for_each_directive(entry: Value, file_label: &str, location: &str) -> Result> { +fn expand_for_each_directive( + entry: Value, + file_label: &str, + location: &str, + budget: &mut u64, +) -> Result> { let directive: ForEachDirective = serde_json::from_value(entry).map_err(|e| RustmotionError::ForEachDirectiveInvalid { path: format!("{file_label}: {location}"), @@ -471,6 +533,15 @@ fn expand_for_each_directive(entry: Value, file_label: &str, location: &str) -> } }; + if items.len() > MAX_FOR_EACH_ITEMS { + return Err(RustmotionError::Generic(format!( + "for-each at '{file_label}: {location}' has {} items, exceeding the per-directive \ + cap of {MAX_FOR_EACH_ITEMS}", + items.len() + ))); + } + consume_node_budget(budget, items.len() as u64, file_label, location)?; + let mut out = Vec::with_capacity(items.len()); for (idx, element) in items.into_iter().enumerate() { let mut bindings: HashMap = HashMap::new(); diff --git a/crates/rustmotion-core/tests/audit_ws_a.rs b/crates/rustmotion-core/tests/audit_ws_a.rs index d42a55bf..d264fbae 100644 --- a/crates/rustmotion-core/tests/audit_ws_a.rs +++ b/crates/rustmotion-core/tests/audit_ws_a.rs @@ -2,8 +2,8 @@ //! tracked in issue #220. use rustmotion_core::css::style::{ - Background, BoxShadow, Color as CssColor, CssStyle, Display, FlexDirection, Position, - Size as CSize, + Background, BackgroundLayer, BoxShadow, Color as CssColor, CssStyle, Display, FlexDirection, + GradientStop, Position, Size as CSize, }; use rustmotion_core::css::taffy_bridge::ConversionContext; use rustmotion_core::css::units::{Length, LengthPercentage as CLP}; @@ -191,3 +191,92 @@ fn underdamped_spring_matches_the_analytic_closed_form() { ); } } + +// ---- linear-gradient(180deg, ...) must put the first stop at the top ---- + +fn gradient_card(w: f32, h: f32, angle: f32) -> BoxNode { + let css = CssStyle { + width: Some(CSize::Length(CLP::Px(w))), + height: Some(CSize::Length(CLP::Px(h))), + background: Some(Background::Single(BackgroundLayer::LinearGradient { + angle: Some(angle), + stops: vec![ + GradientStop { + color: CssColor::String("#ffffff".into()), + offset: Some(0.0), + }, + GradientStop { + color: CssColor::String("#000000".into()), + offset: Some(1.0), + }, + ], + })), + ..Default::default() + }; + BoxNode { + id: 0, + kind: BoxKind::Container, + css, + children: vec![], + intrinsic: None, + source_path: None, + window: None, + } +} + +#[test] +fn linear_gradient_180deg_puts_the_first_stop_at_the_top() { + // CSS: `angle: 180` ("to bottom") points the gradient line downward, so + // the first stop lands at the top and the last stop at the bottom. + let mut root = gradient_card(100.0, 100.0, 180.0); + let buf = render_pixels(&mut root, 100, 100); + + let top = probe(&buf, 100, 50, 2); + let bottom = probe(&buf, 100, 50, 97); + assert!( + top.0 > 200 && top.1 > 200 && top.2 > 200, + "angle: 180 must put the white first stop at the top, got {top:?}" + ); + assert!( + bottom.0 < 50 && bottom.1 < 50 && bottom.2 < 50, + "angle: 180 must put the black last stop at the bottom, got {bottom:?}" + ); +} + +// ---- `spring_settle_time` must not be rescanned on every `spring_value` call ---- + +#[test] +fn spring_settle_time_is_memoized_not_rescanned_every_call() { + // `SpringConfig::duration` routes every `spring_value` sample through + // `spring_settle_time`'s 2k-20k-step coarse-then-bisect scan (see + // animator.rs). The scan result depends only on the spring's own + // (damping, stiffness, mass, threshold) — invariant across every frame + // an animation is sampled at — so repeating it per call is pure waste. + // Measured uncached on this parameter set: 2000 calls take ~530ms in a + // debug build; memoized, the same 2000 calls (one real scan, the rest + // cache hits) complete in well under a tenth of that. + let config = SpringConfig { + damping: 37.0, + stiffness: 733.0, + mass: 1.0, + duration: Some(0.42), + rest_threshold: None, + }; + let start = std::time::Instant::now(); + let mut acc = 0.0; + for i in 0..2000 { + let t = (i as f64) * 1e-4; + acc += spring_value(t, &config); + } + let elapsed = start.elapsed(); + assert!( + acc.is_finite(), + "sanity: accumulated spring values must be finite" + ); + assert!( + elapsed.as_millis() < 150, + "2000 spring_value calls with the same spring parameters took {elapsed:?}; \ + spring_settle_time must be memoized on (damping, stiffness, mass, threshold) rather \ + than re-scanned on every call" + ); +} diff --git a/crates/rustmotion-core/tests/audit_ws_h.rs b/crates/rustmotion-core/tests/audit_ws_h.rs index 0e8d7242..0fd3a042 100644 --- a/crates/rustmotion-core/tests/audit_ws_h.rs +++ b/crates/rustmotion-core/tests/audit_ws_h.rs @@ -25,15 +25,23 @@ fn shared_http_agent_has_finite_global_and_connect_timeouts() { #[test] fn extract_video_frame_rejects_a_remote_src_before_it_ever_reaches_ffmpeg() { - let result = extract_video_frame( + // Asserting `is_err()` alone would also pass for the wrong reason: on a + // machine with no route to this address, ffmpeg itself fails to connect + // and returns a non-zero exit status. The fix under test is that the + // src is refused *before* any subprocess runs at all — so the assertion + // has to be on the specific rejection message, which only the fix + // produces; an ffmpeg spawn/exit failure would carry a different one. + let err = extract_video_frame( "http://169.254.169.254/latest/meta-data/iam/security-credentials/", 0.0, 16, 16, - ); + ) + .expect_err("a scheme-prefixed src must be rejected outright, not handed to ffmpeg"); + let msg = err.to_string(); assert!( - result.is_err(), - "a scheme-prefixed src must be rejected outright, not hand ffmpeg a URL to dereference" + msg.contains("does not fetch video over the network"), + "expected the scheme-rejection error, not an ffmpeg spawn/exit failure: {msg}" ); } @@ -42,7 +50,8 @@ fn extract_video_frame_does_not_reject_a_plain_local_path() { let result = extract_video_frame("/no/such/file/on/disk.mp4", 0.0, 16, 16); let err = result.expect_err("a missing local file is still an error"); assert!( - !err.to_string().contains("does not fetch video"), + !err.to_string() + .contains("does not fetch video over the network"), "a plain local path must fail on ffmpeg/the missing file, not on the scheme check: {err}" ); } diff --git a/crates/rustmotion-html/src/element.rs b/crates/rustmotion-html/src/element.rs index b370f658..0722aa9d 100644 --- a/crates/rustmotion-html/src/element.rs +++ b/crates/rustmotion-html/src/element.rs @@ -37,20 +37,48 @@ fn tag_kind(tag: &str) -> TagKind { } } -/// Concatenated text of an element and all its descendants. -pub(crate) fn inner_text(handle: &Handle) -> String { +/// Concatenated text of an element and all its descendants. Nested inline +/// formatting tags (`strong`/`em`/`label`/…) flatten in, matching real HTML; +/// `

"##; + let v = html_to_scenario_value(html).expect("