diff --git a/crates/rustmotion/src/engine/render/background.rs b/crates/rustmotion/src/engine/render/background.rs index 2c5d719b..ef95320e 100644 --- a/crates/rustmotion/src/engine/render/background.rs +++ b/crates/rustmotion/src/engine/render/background.rs @@ -567,6 +567,12 @@ fn draw_bg_pixel_grid( } /// Tiled heropattern background. +/// +/// The tile is rasterized once at `cfg.scale`, using `heropattern_raster_size` +/// and a matching `resvg` render transform, then tiled 1:1 by the shader. +/// It used to be rasterized at 1x and magnified by the shader's own matrix +/// instead, which turned the vector source into hard nearest-neighbour +/// blocks above `scale: 1` and aliased it below `scale: 1`. fn draw_bg_heropattern( canvas: &Canvas, cfg: &HeropatternConfig, @@ -584,7 +590,6 @@ fn draw_bg_heropattern( return; } - // Build the SVG source with color/opacity substituted let svg_content = format!( r#"{}"#, def.width, @@ -596,20 +601,23 @@ fn draw_bg_heropattern( .replace("{{opacity}}", &cfg.opacity.to_string()), ); - // Render one tile via usvg/resvg let opt = usvg::Options::default(); let Ok(tree) = usvg::Tree::from_data(svg_content.as_bytes(), &opt) else { return; }; - let pw = def.width.ceil() as u32; - let ph = def.height.ceil() as u32; + let (pw, ph) = heropattern_raster_size(def.width, def.height, cfg.scale); let Some(mut pixmap) = tiny_skia::Pixmap::new(pw, ph) else { return; }; - resvg::render(&tree, tiny_skia::Transform::default(), &mut pixmap.as_mut()); + let render_scale_x = pw as f32 / def.width; + let render_scale_y = ph as f32 / def.height; + resvg::render( + &tree, + tiny_skia::Transform::from_scale(render_scale_x, render_scale_y), + &mut pixmap.as_mut(), + ); - // Convert to Skia image let info = ImageInfo::new( (pw as i32, ph as i32), ColorType::RGBA8888, @@ -625,16 +633,10 @@ fn draw_bg_heropattern( return; }; - // Build a tiled shader from the tile image - let matrix = if cfg.scale != 1.0 { - Some(skia_safe::Matrix::scale((cfg.scale, cfg.scale))) - } else { - None - }; let Some(shader) = tile_image.to_shader( (skia_safe::TileMode::Repeat, skia_safe::TileMode::Repeat), - skia_safe::SamplingOptions::default(), - matrix.as_ref(), + skia_safe::SamplingOptions::new(skia_safe::FilterMode::Linear, skia_safe::MipmapMode::None), + None, ) else { return; }; @@ -655,6 +657,21 @@ fn draw_bg_heropattern( ); } +/// Pixel size to rasterize one heropattern tile at, so the vector source is +/// re-rendered crisp at `scale` instead of rasterized at the pattern's +/// native `(width, height)` and then magnified. Clamped to `MAX_TILE_PX` +/// per axis: `HeropatternConfig::scale` has no upper bound in the schema, so +/// an unclamped scale could ask for an arbitrarily large pixmap allocation. +/// A clamped tile still tiles seamlessly with itself — it just renders +/// smaller than an extreme `scale` asked for, which is the trade the "sane +/// maximum" this is named for is making. +fn heropattern_raster_size(width: f32, height: f32, scale: f32) -> (u32, u32) { + const MAX_TILE_PX: f32 = 4096.0; + let pw = (width * scale).ceil().clamp(1.0, MAX_TILE_PX) as u32; + let ph = (height * scale).ceil().clamp(1.0, MAX_TILE_PX) as u32; + (pw, ph) +} + /// Interpolate two AnimatedBackground structs. `t` goes from 0.0 (fully `a`) to 1.0 (fully `b`). #[allow(dead_code)] pub(super) fn interpolate_animated_bg( @@ -1523,3 +1540,50 @@ mod heropattern_period_tests { assert!(spacing_x >= 20.0); } } + +#[cfg(test)] +mod heropattern_raster_tests { + //! The heropattern tile used to be rasterized at 1x (the + //! pattern's native width/height) and then magnified by the shader's + //! own matrix with nearest-neighbour sampling — blocky above `scale: 1`, + //! aliased below it. `heropattern_raster_size` must honour `scale` + //! directly in the raster resolution instead. + + use super::*; + + #[test] + fn raster_size_scales_with_cfg_scale_not_pinned_to_1x() { + let (pw, ph) = heropattern_raster_size(32.0, 64.0, 4.0); + assert_eq!( + (pw, ph), + (128, 256), + "the pixmap must be sized for the scaled tile, not the pattern's native 32x64" + ); + } + + #[test] + fn raster_size_matches_the_pattern_exactly_at_scale_1() { + assert_eq!(heropattern_raster_size(32.0, 64.0, 1.0), (32, 64)); + } + + #[test] + fn raster_size_is_clamped_for_an_unbounded_scale() { + let (pw, ph) = heropattern_raster_size(32.0, 64.0, 100_000.0); + assert!( + pw <= 4096 && ph <= 4096, + "an extreme scale must not attempt an unbounded pixmap allocation, got {pw}x{ph}" + ); + } + + #[test] + fn draw_bg_heropattern_does_not_panic_at_an_extreme_scale() { + let mut surface = skia_safe::surfaces::raster_n32_premul((64, 64)).expect("surface"); + let cfg = HeropatternConfig { + pattern: "aztec".to_string(), + color: "#FFFFFF".to_string(), + opacity: 0.1, + scale: 100_000.0, + }; + draw_bg_heropattern(surface.canvas(), &cfg, 0.0, 64.0, 64.0); + } +} diff --git a/crates/rustmotion/src/engine/render/scene.rs b/crates/rustmotion/src/engine/render/scene.rs index dde851e3..cf7d96f2 100644 --- a/crates/rustmotion/src/engine/render/scene.rs +++ b/crates/rustmotion/src/engine/render/scene.rs @@ -553,6 +553,13 @@ fn render_with_new_pipeline_iter<'a, I>( /// Paint a decorative leaf (e.g. Particle) over the full viewport without /// going through taffy. Resolves animations and dispatches to /// `Painter::paint_content` directly with a viewport-sized `BoxLayout`. +/// +/// Visibility and effects go through the same `PaintWindow::contains` and +/// `effective_effects` the ordinary `paint_tree` dispatch uses (see +/// `box_builder::effective_effects`'s doc comment), rather than re-deriving +/// both by hand — a component whose `timeline`/`style.transition` state or +/// exact `end_at` boundary only worked in one of the two dispatch paths used +/// to be invisible to tests written against either one alone. fn paint_decorative_fullscreen( canvas: &Canvas, child: &ChildComponent, @@ -560,34 +567,22 @@ fn paint_decorative_fullscreen( viewport_h: f32, ctx: &RenderContext, ) { + use rustmotion_components::box_builder::effective_effects; use rustmotion_core::engine::animator::{resolve_props_for_effects, AnimatedProperties}; + use rustmotion_core::engine::box_tree::PaintWindow; use rustmotion_core::engine::layout_pass::BoxLayout; use rustmotion_core::traits::PaintCtx; let time = ctx.time.seconds(); if let Some(timed) = child.component.as_timed() { - let (start_at, end_at) = timed.timing(); - if let Some(s) = start_at { - if time < s { - return; - } - } - if let Some(e) = end_at { - if time > e { - return; - } + let (start, end) = timed.timing(); + if !(PaintWindow { start, end }).contains(time) { + return; } } - let props = match child.component.as_animatable() { - Some(a) => { - let effects = a.animation_effects(); - if effects.is_empty() { - AnimatedProperties::default() - } else { - resolve_props_for_effects(effects, time, ctx.scene_duration) - } - } + let props = match effective_effects(&child.component, 0.0) { + Some(effects) => resolve_props_for_effects(&effects, time, ctx.scene_duration), None => AnimatedProperties::default(), }; if props.opacity <= 0.0 { diff --git a/crates/rustmotion/tests/audit_ws_e.rs b/crates/rustmotion/tests/audit_ws_e.rs index 5a2f31eb..9b0e92ab 100644 --- a/crates/rustmotion/tests/audit_ws_e.rs +++ b/crates/rustmotion/tests/audit_ws_e.rs @@ -1,18 +1,143 @@ -//! Regression tests — audit round chantier/audit-2026-09, workstream E -//! (animated backgrounds & the scene render path). +//! Regression tests — workstream E (animated backgrounds & the scene render +//! path). //! -//! Two of the four findings are bugs in fully private -//! rendering internals (`background.rs`'s `tile_spacing`/`compute_scroll_offset`/ +//! Some of the covered defects are bugs in fully private rendering +//! internals (`background.rs`'s `tile_spacing`/`compute_scroll_offset`/ //! `draw_bg_heropattern`) that this crate never exposes past its `pub` //! surface — an external integration test crate like this one cannot name //! them. Their regression tests live as `#[cfg(test)]` modules inside //! `crates/rustmotion/src/engine/render/background.rs` itself, following //! that file's own pre-existing convention (`scroll_offset_wrap_tests`, //! `pixel_grid_tests`, `grid_lines_tests`, `halo_opacity_tests`) for testing -//! renderer-private logic directly. This file carries the findings that are +//! renderer-private logic directly. This file carries the defects that are //! genuinely reachable through the crate's public API. -//! -//! One section per finding: the paint path, then colour templating. use rustmotion::encode::video::{build_frame_tasks, render_frame_task, FrameTask}; use rustmotion::loader::load_scenario_from_source; + +/// Render a single `FrameTask::WorldFrame` at `frame_in_view`, decoded from +/// `scenario_json`. Panics (with a message naming the missing frame) if no +/// such world frame exists in the built schedule — a test bug, not a +/// render-time failure, should fail loudly here. +fn render_world_frame(scenario_json: &serde_json::Value, frame_in_view: u32) -> Vec { + let scenario = load_scenario_from_source(None, Some(&scenario_json.to_string())) + .expect("scenario is schema-valid"); + let tasks = build_frame_tasks(&scenario); + let task = tasks + .iter() + .find( + |t| matches!(t, FrameTask::WorldFrame { frame_in_view: f, .. } if *f == frame_in_view), + ) + .unwrap_or_else(|| panic!("no WorldFrame task at frame_in_view={frame_in_view}")); + render_frame_task(&scenario.video, &scenario, task).expect("frame renders") +} + +/// Count pixels that read as the particle's pure-green marker colour +/// (`#00FF00`) against the scenario's plain black background — a stand-in +/// for "is the decorative child visible in this frame" that doesn't depend +/// on knowing any particle's exact on-screen position. +fn green_pixel_count(buf: &[u8]) -> usize { + buf.chunks_exact(4) + .filter(|px| px[1] > 100 && px[0] < 80 && px[2] < 80) + .count() +} + +mod decorative_dispatch_parity { + //! `paint_decorative_fullscreen` (the world-view-only path that paints + //! decorative children like `particle` without going through the box + //! tree) used to re-derive visibility and effects by hand instead of + //! calling `PaintWindow::contains` / `box_builder::effective_effects` + //! like every other paint path does. Two independent symptoms: an + //! inclusive `end_at` (visible one frame too long) and dropped + //! `timeline` animation effects. + + use super::*; + + const FPS: u32 = 10; + + fn world_scenario(particle_extra: serde_json::Value) -> serde_json::Value { + let mut particle = serde_json::json!({ + "type": "particle", + "particle_type": "snow", + "count": 30, + "colors": ["#00FF00"], + "size_range": {"min": 6, "max": 6}, + "speed": 0.0, + }); + particle + .as_object_mut() + .unwrap() + .extend(particle_extra.as_object().unwrap().clone()); + + serde_json::json!({ + "video": {"width": 64, "height": 64, "fps": FPS, "background": "#000000"}, + "composition": [ + {"type": "world", "scenes": [ + {"duration": 2.0, "children": [particle]} + ]} + ] + }) + } + + /// `end_at` is a half-open window: the child must already be gone + /// exactly at `end_at`, not still visible for one extra frame past it. + #[test] + fn end_at_is_a_half_open_window_not_inclusive() { + let end_at = 0.5; + let scenario = world_scenario(serde_json::json!({ "end_at": end_at })); + let frame_just_before_end_at = (end_at * FPS as f64) as u32 - 1; + let frame_at_end_at = (end_at * FPS as f64) as u32; + + let before = render_world_frame(&scenario, frame_just_before_end_at); + assert!( + green_pixel_count(&before) > 0, + "the particle must still be visible just before its end_at" + ); + + let at_boundary = render_world_frame(&scenario, frame_at_end_at); + assert_eq!( + green_pixel_count(&at_boundary), + 0, + "end_at is a half-open window ([start, end)): the particle must already be gone \ + exactly at end_at, not one extra frame later" + ); + } + + /// A `timeline` step's `animation` entries must be folded into the + /// resolved props like every other paint path does, not silently + /// dropped because only `style.animation` was read. + #[test] + fn timeline_animation_effects_are_not_silently_dropped() { + let fade_out_at = 0.5; + let scenario = world_scenario(serde_json::json!({ + "timeline": [{ + "at": 0.0, + "animation": [{ + "name": "keyframes", + "keyframes": [{ + "property": "opacity", + "keyframes": [ + {"time": 0.0, "value": 1.0}, + {"time": fade_out_at, "value": 0.0} + ] + }] + }] + }] + })); + + let early = render_world_frame(&scenario, 0); + assert!( + green_pixel_count(&early) > 0, + "the particle should be visible at t=0, before the timeline fade-out completes" + ); + + let frame_well_past_fade_out = (fade_out_at * FPS as f64) as u32 * 3; + let late = render_world_frame(&scenario, frame_well_past_fade_out); + assert_eq!( + green_pixel_count(&late), + 0, + "a timeline step's animation effects must apply to a decorative child, not be \ + silently dropped" + ); + } +}