From 0a4cd818969a8716d0fac4412139403a3dc13dec Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Tue, 22 Sep 2026 11:05:20 +0200 Subject: [PATCH] fix(video): honour the fit property Refs #220 --- crates/rustmotion-components/src/video.rs | 54 ++++- .../rustmotion-components/tests/audit_ws_d.rs | 188 ++++++++++++++++++ 2 files changed, 236 insertions(+), 6 deletions(-) create mode 100644 crates/rustmotion-components/tests/audit_ws_d.rs diff --git a/crates/rustmotion-components/src/video.rs b/crates/rustmotion-components/src/video.rs index 0d64739f..10921bce 100644 --- a/crates/rustmotion-components/src/video.rs +++ b/crates/rustmotion-components/src/video.rs @@ -46,6 +46,52 @@ 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); + } +} + impl Painter for Video { fn paint_content( &self, @@ -74,9 +120,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 +149,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..3cb43373 --- /dev/null +++ b/crates/rustmotion-components/tests/audit_ws_d.rs @@ -0,0 +1,188 @@ +//! 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" + ); +}