From 8df64e760712b019d3628f4c92f814cc586f4138 Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Tue, 29 Sep 2026 14:17:45 +0200 Subject: [PATCH 1/2] fix(animation): make every animated property reach layout and the frame Two issues, one architecture. #426 reported that an animation touching only border_radius, clip_path_progress or font_size never applied; #430 reported letter_spacing having no effect on rich_text and gradient_text, animated tracking overflowing because layout used the static value, and a static opacity: 0 cancelling a fade-in. Three separate defects sat behind them. props_has_paint_overrides gated whether apply_animated_props ran at all. It listed translate, scale, rotation, opacity, blur, glow, perspective, width and height, and had gone stale against every property added to AnimatedProperties since. The gate is deleted rather than completed: apply_animated_props already guards each branch against the resting value, so the list bought nothing and left the same trap for the next property anyone adds. with_cascaded_style folded the box's own resolved style onto a clone of the same component through inherit_from, which only fills fields that are None. A component declaring its own font-size or letter-spacing therefore never saw the animated value. inherit_from is right for what it is named after -- parent to child -- so the re-cascade gets its own function, overlay_resolved_typography, which overwrites. Conflating the two was the bug. rich_text ignored style.letter-spacing outright, static or animated: compute_layout discarded the resolved value into a binding named _letter_spacing_unused and resolve_span_fonts defaulted every span to 0.0. Neither fix above would have reached it. An animation on opacity now replaces the declared static value instead of multiplying with it, which is how every other property in apply_animated_props already treats its declaration. This changes the render of a scenario that declares an opacity and animates it: a node at opacity 0.6 with a pulse used to paint 0.6 * pulse and now paints the pulse. Effects still compose with each other -- that happens upstream in AnimatedProperties::merge and is untouched. Closes #426 Closes #430 --- .../rustmotion-components/src/box_builder.rs | 530 +++++++++++++++++- crates/rustmotion-components/src/lib.rs | 55 +- crates/rustmotion-components/src/rich_text.rs | 17 +- crates/rustmotion-core/src/css/animation.rs | 109 +++- crates/rustmotion-core/src/css/cascade.rs | 90 +++ 5 files changed, 768 insertions(+), 33 deletions(-) diff --git a/crates/rustmotion-components/src/box_builder.rs b/crates/rustmotion-components/src/box_builder.rs index 3bc225f..ac6e0da 100644 --- a/crates/rustmotion-components/src/box_builder.rs +++ b/crates/rustmotion-components/src/box_builder.rs @@ -259,9 +259,7 @@ fn ghost_css_for( } if let Some(ghost_effects) = effective_effects(&child.component, extra_delay, ghost_time) { let props = resolve_props_for_effects(&ghost_effects, ghost_time, scene_duration); - if props_has_paint_overrides(&props) { - apply_animated_props(&mut css, &props); - } + apply_animated_props(&mut css, &props); apply_glow_effect(&mut css, &ghost_effects); carry_paint_pass_effects(&mut css, &ghost_effects); apply_directional_blur_props(&mut css, &props); @@ -559,9 +557,7 @@ fn build_child<'a>( if let Some(actx) = local_actx { if let Some(effects) = effective_effects(&child.component, anim_delay, actx.time) { let props = resolve_props_for_effects(&effects, actx.time, actx.scene_duration); - if props_has_paint_overrides(&props) { - apply_animated_props(&mut css, &props); - } + apply_animated_props(&mut css, &props); apply_glow_effect(&mut css, &effects); carry_paint_pass_effects(&mut css, &effects); apply_directional_blur_props(&mut css, &props); @@ -1036,22 +1032,6 @@ pub(crate) fn resolve_transition_css_overrides( out } -fn props_has_paint_overrides(p: &AnimatedProperties) -> bool { - p.translate_x != 0.0 - || p.translate_y != 0.0 - || (p.scale_x - 1.0).abs() > 1e-4 - || (p.scale_y - 1.0).abs() > 1e-4 - || p.rotation.abs() > 1e-3 - || p.rotate_x.abs() > 1e-3 - || p.rotate_y.abs() > 1e-3 - || (p.opacity - 1.0).abs() > 1e-4 - || p.blur > 0.0 - || (p.glow_radius > 0.0 && p.glow_intensity > 0.0) - || p.perspective > 0.0 - || p.width >= 0.0 - || p.height >= 0.0 -} - fn carry_paint_pass_effects( css: &mut CssStyle, effects: &[rustmotion_core::schema::AnimationEffect], @@ -3502,4 +3482,510 @@ mod tests { other => panic!("expected Blur, got {:?}", other), } } + + fn read_pixel(surface: &mut skia_safe::Surface, x: i32, y: i32) -> [u8; 4] { + let snapshot = surface.image_snapshot(); + let info = skia_safe::ImageInfo::new( + (1, 1), + skia_safe::ColorType::RGBA8888, + skia_safe::AlphaType::Premul, + None, + ); + let mut buf = [0u8; 4]; + assert!( + snapshot.read_pixels( + &info, + &mut buf, + 4, + skia_safe::IPoint::new(x, y), + skia_safe::image::CachingHint::Disallow, + ), + "pixel read should succeed" + ); + buf + } + + fn ink_extent_x( + surface: &mut skia_safe::Surface, + width: i32, + height: i32, + y0: i32, + y1: i32, + ) -> Option<(i32, i32)> { + let snapshot = surface.image_snapshot(); + let info = skia_safe::ImageInfo::new( + (width, height), + skia_safe::ColorType::RGBA8888, + skia_safe::AlphaType::Premul, + None, + ); + let mut buf = vec![0u8; (width * height * 4) as usize]; + assert!( + snapshot.read_pixels( + &info, + &mut buf, + (width * 4) as usize, + skia_safe::IPoint::new(0, 0), + skia_safe::image::CachingHint::Disallow, + ), + "pixel read should succeed" + ); + let mut min_x: Option = None; + let mut max_x: Option = None; + for y in y0.max(0)..y1.min(height) { + for x in 0..width { + let alpha = buf[((y * width + x) * 4 + 3) as usize]; + if alpha > 0 { + min_x = Some(min_x.map_or(x, |m| m.min(x))); + max_x = Some(max_x.map_or(x, |m| m.max(x))); + } + } + } + min_x.zip(max_x) + } + + fn paint_scene_at( + built: &BuiltScene, + layout: &rustmotion_core::engine::layout_pass::LayoutResult, + width: i32, + height: i32, + time: f64, + ) -> skia_safe::Surface { + let mut surface = + skia_safe::surfaces::raster_n32_premul((width, height)).expect("raster surface"); + let canvas = surface.canvas(); + let dispatcher = LegacyPaintDispatcher::for_scene(built); + let frame = PaintFrame { + light: Default::default(), + time, + scenario_time: time, + frame_index: (time * 30.0).round() as u32, + fps: 30, + video_width: width as u32, + video_height: height as u32, + scene_duration: 1.0, + camera: None, + }; + paint_tree(canvas, &built.root, layout, &frame, &dispatcher); + surface + } + + #[test] + fn an_animation_touching_only_border_radius_reaches_the_painted_frame() { + const SIZE: i32 = 200; + let scene = vec![child_from_json(json!({ + "type": "div", + "style": { + "width": SIZE, "height": SIZE, + "background": "#FF2D55", + "animation": [{ "name": "keyframes", "duration": 1.0, "keyframes": [ + { "property": "border_radius", "easing": "linear", "keyframes": [ + { "time": 0.0, "value": 0.0 }, { "time": 1.0, "value": 100.0 } + ] } + ] }] + } + }))]; + let anim = BuildAnimationCtx { + time: 1.0, + scenario_time: 1.0, + scene_duration: 1.0, + fps: 30, + }; + let built = build_scene_at_time( + &scene, + (SIZE as f32, SIZE as f32), + default_root_css((SIZE as f32, SIZE as f32)), + anim, + ); + let layout = run_layout( + &built.root, + (SIZE as f32, SIZE as f32), + &ConversionContext::default(), + ); + let mut surface = paint_scene_at(&built, &layout, SIZE, SIZE, 1.0); + + let corner = read_pixel(&mut surface, 2, 2); + assert_eq!( + corner[3], 0, + "a 200x200 box animated to a 100px border-radius (t=1) must clip its very corner to \ + transparent; an animation touching only border_radius must still reach paint \ + through box_builder's css, got rgba {:?}", + corner + ); + } + + #[test] + fn an_animation_touching_only_clip_path_progress_reaches_the_painted_frame() { + const SIZE: i32 = 200; + let scene = vec![child_from_json(json!({ + "type": "div", + "style": { + "width": SIZE, "height": SIZE, + "background": "#00AAFF", + "clip-path": { + "kind": "morph", + "from": { "kind": "circle", "radius": 10 }, + "to": { "kind": "circle", "radius": 150 }, + "progress": 0 + }, + "animation": [{ "name": "keyframes", "duration": 1.0, "keyframes": [ + { "property": "clip_path_progress", "easing": "linear", "keyframes": [ + { "time": 0.0, "value": 0.0 }, { "time": 1.0, "value": 1.0 } + ] } + ] }] + } + }))]; + let anim = BuildAnimationCtx { + time: 1.0, + scenario_time: 1.0, + scene_duration: 1.0, + fps: 30, + }; + let built = build_scene_at_time( + &scene, + (SIZE as f32, SIZE as f32), + default_root_css((SIZE as f32, SIZE as f32)), + anim, + ); + let layout = run_layout( + &built.root, + (SIZE as f32, SIZE as f32), + &ConversionContext::default(), + ); + let mut surface = paint_scene_at(&built, &layout, SIZE, SIZE, 1.0); + + let corner = read_pixel(&mut surface, 5, 5); + assert!( + corner[3] > 200, + "at t=1 the clip-path morph must have swept to its 150px-radius `to` circle, which \ + covers this corner; an animation touching only clip_path_progress must still reach \ + paint through box_builder's css, got rgba {:?}", + corner + ); + } + + #[test] + fn an_animation_touching_only_gap_reaches_the_painted_frame() { + const W: i32 = 400; + const H: i32 = 80; + let scene = vec![child_from_json(json!({ + "type": "div", + "style": { + "display": "flex", "flex-direction": "row", + "width": W, "height": H, + "animation": [{ "name": "keyframes", "duration": 1.0, "keyframes": [ + { "property": "gap", "easing": "linear", "keyframes": [ + { "time": 0.0, "value": 0.0 }, { "time": 1.0, "value": 100.0 } + ] } + ] }] + }, + "children": [ + { "type": "shape", "shape": "rect", "fill": "#00AA00", + "style": { "width": 80, "height": 80 } }, + { "type": "shape", "shape": "rect", "fill": "#0000AA", + "style": { "width": 80, "height": 80 } } + ] + }))]; + let anim = BuildAnimationCtx { + time: 1.0, + scenario_time: 1.0, + scene_duration: 1.0, + fps: 30, + }; + let built = build_scene_at_time( + &scene, + (W as f32, H as f32), + default_root_css((W as f32, H as f32)), + anim, + ); + let layout = run_layout( + &built.root, + (W as f32, H as f32), + &ConversionContext::default(), + ); + let mut surface = paint_scene_at(&built, &layout, W, H, 1.0); + + let between = read_pixel(&mut surface, 130, 40); + assert_eq!( + between[3], 0, + "at t=1 a 100px gap must leave a transparent strip between the two 80px shapes \ + (x=80..180); an animation touching only gap must still reach layout through \ + box_builder's css, got rgba {:?}", + between + ); + } + + #[test] + fn an_animation_touching_only_padding_reaches_the_painted_frame() { + const SIZE: i32 = 200; + let scene = vec![child_from_json(json!({ + "type": "div", + "style": { + "width": SIZE, "height": SIZE, + "animation": [{ "name": "keyframes", "duration": 1.0, "keyframes": [ + { "property": "padding", "easing": "linear", "keyframes": [ + { "time": 0.0, "value": 0.0 }, { "time": 1.0, "value": 60.0 } + ] } + ] }] + }, + "children": [ + { "type": "shape", "shape": "rect", "fill": "#FFAA00", + "style": { "width": 40, "height": 40 } } + ] + }))]; + let anim = BuildAnimationCtx { + time: 1.0, + scenario_time: 1.0, + scene_duration: 1.0, + fps: 30, + }; + let built = build_scene_at_time( + &scene, + (SIZE as f32, SIZE as f32), + default_root_css((SIZE as f32, SIZE as f32)), + anim, + ); + let layout = run_layout( + &built.root, + (SIZE as f32, SIZE as f32), + &ConversionContext::default(), + ); + let mut surface = paint_scene_at(&built, &layout, SIZE, SIZE, 1.0); + + let near_origin = read_pixel(&mut surface, 10, 10); + assert_eq!( + near_origin[3], 0, + "at t=1 a 60px padding must push the 40px child away from the container's top-left \ + corner; an animation touching only padding must still reach layout through \ + box_builder's css, got rgba {:?}", + near_origin + ); + } + + #[test] + fn an_animated_font_size_reaches_layout_not_only_paint() { + let make_text = |style_extra: serde_json::Value| { + child_from_json(json!({ + "type": "text", + "content": "The quick brown fox jumps over the lazy dog", + "style": style_extra + })) + }; + + let baseline_scene = vec![make_text(json!({ + "font-size": 20, "white-space": "nowrap", "align-self": "flex-start" + }))]; + let baseline_built = build_scene(&baseline_scene, (4000.0, 400.0)); + let baseline_layout = run_layout( + &baseline_built.root, + (4000.0, 400.0), + &ConversionContext::default(), + ); + let baseline_width = baseline_layout + .get(baseline_built.root.children[0].id) + .expect("baseline text laid out") + .width; + + let animated_scene = vec![make_text(json!({ + "font-size": 20, + "white-space": "nowrap", + "align-self": "flex-start", + "animation": [{ "name": "keyframes", "duration": 1.0, "keyframes": [ + { "property": "font_size", "easing": "linear", "keyframes": [ + { "time": 0.0, "value": 20.0 }, { "time": 1.0, "value": 120.0 } + ] } + ] }] + }))]; + let anim = BuildAnimationCtx { + time: 1.0, + scenario_time: 1.0, + scene_duration: 1.0, + fps: 30, + }; + let animated_built = build_scene_at_time( + &animated_scene, + (4000.0, 400.0), + default_root_css((4000.0, 400.0)), + anim, + ); + let animated_layout = run_layout( + &animated_built.root, + (4000.0, 400.0), + &ConversionContext::default(), + ); + let animated_width = animated_layout + .get(animated_built.root.children[0].id) + .expect("animated text laid out") + .width; + + assert!( + animated_width > baseline_width * 2.0, + "at t=1 the font_size animation has reached 120px (6x the declared static 20px); \ + the LAYOUT width must reflect that, not just the painted glyphs — issue #426/#430's \ + 'paint right, layout stale' symptom. baseline={baseline_width}, \ + animated={animated_width}" + ); + } + + #[test] + fn an_animated_letter_spacing_on_text_reaches_layout_not_only_paint() { + let make_text = |style_extra: serde_json::Value| { + child_from_json(json!({ + "type": "text", + "content": "WWWWWWWWWW", + "style": style_extra + })) + }; + + let baseline_scene = vec![make_text(json!({ + "letter-spacing": 0, "font-size": 40, "white-space": "nowrap", + "align-self": "flex-start" + }))]; + let baseline_built = build_scene(&baseline_scene, (4000.0, 400.0)); + let baseline_layout = run_layout( + &baseline_built.root, + (4000.0, 400.0), + &ConversionContext::default(), + ); + let baseline_width = baseline_layout + .get(baseline_built.root.children[0].id) + .expect("baseline text laid out") + .width; + + let animated_scene = vec![make_text(json!({ + "letter-spacing": 0, "font-size": 40, "white-space": "nowrap", + "align-self": "flex-start", + "animation": [{ "name": "keyframes", "duration": 1.0, "keyframes": [ + { "property": "letter_spacing", "easing": "linear", "keyframes": [ + { "time": 0.0, "value": 0.0 }, { "time": 1.0, "value": 40.0 } + ] } + ] }] + }))]; + let anim = BuildAnimationCtx { + time: 1.0, + scenario_time: 1.0, + scene_duration: 1.0, + fps: 30, + }; + let animated_built = build_scene_at_time( + &animated_scene, + (4000.0, 400.0), + default_root_css((4000.0, 400.0)), + anim, + ); + let animated_layout = run_layout( + &animated_built.root, + (4000.0, 400.0), + &ConversionContext::default(), + ); + let animated_width = animated_layout + .get(animated_built.root.children[0].id) + .expect("animated text laid out") + .width; + + assert!( + animated_width > baseline_width + 200.0, + "at t=1 a 40px letter-spacing animation must widen the measured LAYOUT box, or the \ + box the text was given overflows — issue #430's overflow symptom exactly. \ + baseline={baseline_width}, animated={animated_width}" + ); + } + + #[test] + fn an_animated_letter_spacing_reaches_rich_text_through_the_full_pipeline() { + const W: i32 = 800; + const H: i32 = 120; + let scene = vec![child_from_json(json!({ + "type": "rich_text", + "spans": [{ "text": "WWWWWWWWWW", "color": "#FFFFFF" }], + "style": { + "letter-spacing": 0, "font-size": 60, + "animation": [{ "name": "keyframes", "duration": 1.0, "keyframes": [ + { "property": "letter_spacing", "easing": "linear", "keyframes": [ + { "time": 0.0, "value": 0.0 }, { "time": 1.0, "value": 40.0 } + ] } + ] }] + } + }))]; + + let render_extent_at = |time: f64| -> (i32, i32) { + let anim = BuildAnimationCtx { + time, + scenario_time: time, + scene_duration: 1.0, + fps: 30, + }; + let built = build_scene_at_time( + &scene, + (W as f32, H as f32), + default_root_css((W as f32, H as f32)), + anim, + ); + let layout = run_layout( + &built.root, + (W as f32, H as f32), + &ConversionContext::default(), + ); + let mut surface = paint_scene_at(&built, &layout, W, H, time); + ink_extent_x(&mut surface, W, H, 0, H).expect("rich_text must paint some ink") + }; + + let (start0, end0) = render_extent_at(0.0); + let (start1, end1) = render_extent_at(1.0); + let extent0 = end0 - start0; + let extent1 = end1 - start1; + + assert!( + extent1 > extent0 + 100, + "an animated style.letter-spacing (no per-span override) must widen rich_text's \ + painted ink extent — both the box_builder wiring and rich_text's own static/\ + animated fallback must be in place (issue #430); extent at t=0 was {extent0}px \ + [{start0},{end0}], at t=1 {extent1}px [{start1},{end1}]" + ); + } + + #[test] + fn a_static_opacity_zero_is_overridden_by_an_active_fade_in_animation_end_to_end() { + const SIZE: i32 = 100; + let scene = vec![child_from_json(json!({ + "type": "div", + "style": { + "width": SIZE, "height": SIZE, + "background": "#00FF00", + "opacity": 0, + "animation": [{ "name": "keyframes", "duration": 1.0, "keyframes": [ + { "property": "opacity", "easing": "linear", "keyframes": [ + { "time": 0.0, "value": 0.0 }, { "time": 1.0, "value": 0.8 } + ] } + ] }] + } + }))]; + let anim = BuildAnimationCtx { + time: 1.0, + scenario_time: 1.0, + scene_duration: 1.0, + fps: 30, + }; + let built = build_scene_at_time( + &scene, + (SIZE as f32, SIZE as f32), + default_root_css((SIZE as f32, SIZE as f32)), + anim, + ); + let layout = run_layout( + &built.root, + (SIZE as f32, SIZE as f32), + &ConversionContext::default(), + ); + let mut surface = paint_scene_at(&built, &layout, SIZE, SIZE, 1.0); + + let center = read_pixel(&mut surface, SIZE / 2, SIZE / 2); + assert!( + center[3] > 100, + "the fade-in animation resolves opacity to 0.8 at t=1; a declared `opacity: 0` \ + authored as the animation's own starting point must not multiply it back to zero \ + for the rest of the run (issue #430), got rgba {:?}", + center + ); + } } diff --git a/crates/rustmotion-components/src/lib.rs b/crates/rustmotion-components/src/lib.rs index f03ee75..28b839a 100644 --- a/crates/rustmotion-components/src/lib.rs +++ b/crates/rustmotion-components/src/lib.rs @@ -735,7 +735,60 @@ impl Component { Component::Tooltip(c) => c.style_config_mut(), _ => unreachable!("classified as typographic by the match above"), }; - rustmotion_core::css::cascade::inherit_from(resolved, style); + rustmotion_core::css::cascade::overlay_resolved_typography(resolved, style); Some(clone) } } + +#[cfg(test)] +mod with_cascaded_style_tests { + use super::*; + use rustmotion_core::css::units::Length; + + #[test] + fn a_declared_static_font_size_does_not_shadow_the_resolved_animated_one() { + let text: Component = serde_json::from_value(serde_json::json!({ + "type": "text", + "content": "hi", + "style": { "font-size": 20 } + })) + .expect("component deserializes"); + + let resolved = CssStyle { + font_size: Some(Length::Px(120.0)), + ..text.as_styled().style_config().clone() + }; + + let cascaded = text + .with_cascaded_style(&resolved) + .expect("text is typographic"); + assert_eq!( + cascaded.as_styled().style_config().font_size, + Some(Length::Px(120.0)), + "measurement/paint must see the resolved (animated) font-size, not the component's \ + own declared 20px — this is the exact bug behind issues #426/#430" + ); + } + + #[test] + fn a_component_with_no_static_font_size_still_inherits_the_resolved_one() { + let text: Component = serde_json::from_value(serde_json::json!({ + "type": "text", + "content": "hi" + })) + .expect("component deserializes"); + + let resolved = CssStyle { + font_size: Some(Length::Px(64.0)), + ..text.as_styled().style_config().clone() + }; + + let cascaded = text + .with_cascaded_style(&resolved) + .expect("text is typographic"); + assert_eq!( + cascaded.as_styled().style_config().font_size, + Some(Length::Px(64.0)) + ); + } +} diff --git a/crates/rustmotion-components/src/rich_text.rs b/crates/rustmotion-components/src/rich_text.rs index 64cd4a0..dc57cfa 100644 --- a/crates/rustmotion-components/src/rich_text.rs +++ b/crates/rustmotion-components/src/rich_text.rs @@ -131,6 +131,7 @@ fn resolve_span_fonts( spans: &[RichTextSpan], style: &CssStyle, default_size: f32, + default_letter_spacing: f32, ) -> Vec> { let default_color = style.color_str_or("#FFFFFF"); let default_family = style.font_family_or("Inter"); @@ -154,7 +155,7 @@ fn resolve_span_fonts( let weight = span.font_weight.as_ref().unwrap_or(&default_weight); let fstyle = span.font_style.as_ref().unwrap_or(&default_font_style); let color = span.color.as_deref().unwrap_or(default_color).to_string(); - let letter_spacing = span.letter_spacing.unwrap_or(0.0); + let letter_spacing = span.letter_spacing.unwrap_or(default_letter_spacing); make_font(family, weight, fstyle, size).map(|font| SpanFontInfo { font, color, @@ -198,9 +199,9 @@ impl RichText { viewport_height, wrap_width.unwrap_or(0.0), ); - let (default_size, _letter_spacing_unused, line_height_val) = + let (default_size, default_letter_spacing, line_height_val) = style.typography_px_ctx(&base_ctx, 48.0); - let span_fonts = resolve_span_fonts(spans, style, default_size); + let span_fonts = resolve_span_fonts(spans, style, default_size, default_letter_spacing); let emoji_tf = emoji_typeface(); let texts: Vec = if visible_chars_progress >= 0.0 { @@ -409,8 +410,14 @@ impl RichText { ctx.video_height as f32, wrap_width.unwrap_or(0.0), ); - let default_size = self.style.font_size_px_ctx(&base_ctx, 48.0); - let span_fonts = resolve_span_fonts(&self.spans, &self.style, default_size); + let (default_size, default_letter_spacing, _) = + self.style.typography_px_ctx(&base_ctx, 48.0); + let span_fonts = resolve_span_fonts( + &self.spans, + &self.style, + default_size, + default_letter_spacing, + ); let emoji_tf = emoji_typeface(); let align_width = if layout_width.is_finite() && layout_width > 0.0 { diff --git a/crates/rustmotion-core/src/css/animation.rs b/crates/rustmotion-core/src/css/animation.rs index 3803170..e5cf05b 100644 --- a/crates/rustmotion-core/src/css/animation.rs +++ b/crates/rustmotion-core/src/css/animation.rs @@ -1,5 +1,5 @@ use crate::css::style::CssStyle; -use crate::css::style::{BorderRadius, ClipPath, FilterFn, Size, TransformFn}; +use crate::css::style::{BorderRadius, ClipPath, Edges, FilterFn, Gap, Size, TransformFn}; use crate::css::units::{Length, LengthPercentage}; use crate::engine::animator::AnimatedProperties; @@ -39,8 +39,7 @@ pub fn apply_animated_props(css: &mut CssStyle, props: &AnimatedProperties) { } if (props.opacity - 1.0).abs() > 1e-4 { - let base = css.opacity.unwrap_or(1.0); - css.opacity = Some(base * props.opacity); + css.opacity = Some(props.opacity); } let mut filters: Vec = Vec::new(); @@ -88,6 +87,22 @@ pub fn apply_animated_props(css: &mut CssStyle, props: &AnimatedProperties) { *progress = props.clip_path_progress; } } + + if props.font_size >= 0.0 { + css.font_size = Some(Length::Px(props.font_size)); + } + + if props.letter_spacing.is_finite() { + css.letter_spacing = Some(Length::Px(props.letter_spacing)); + } + + if props.gap >= 0.0 { + css.gap = Some(Gap::Uniform(LengthPercentage::Px(props.gap))); + } + + if props.padding >= 0.0 { + css.padding = Some(Edges::Uniform(LengthPercentage::Px(props.padding))); + } } #[cfg(test)] @@ -116,7 +131,7 @@ mod tests { } #[test] - fn opacity_is_multiplied_with_existing_css_opacity() { + fn an_active_opacity_animation_replaces_the_declared_static_opacity() { let mut css = CssStyle { opacity: Some(0.5), ..CssStyle::default() @@ -126,7 +141,32 @@ mod tests { ..AnimatedProperties::default() }; apply_animated_props(&mut css, &props); - assert!((css.opacity.unwrap() - 0.25).abs() < 1e-6); + assert!( + (css.opacity.unwrap() - 0.5).abs() < 1e-6, + "an animation touching opacity must overwrite the static declaration outright, the \ + same last-value-written rule every other animated property in this function \ + follows — not multiply against it, got {:?}", + css.opacity + ); + } + + #[test] + fn a_static_opacity_of_zero_is_overridden_by_an_active_fade_in_animation() { + let mut css = CssStyle { + opacity: Some(0.0), + ..CssStyle::default() + }; + let props = AnimatedProperties { + opacity: 0.8, + ..AnimatedProperties::default() + }; + apply_animated_props(&mut css, &props); + assert!( + (css.opacity.unwrap() - 0.8).abs() < 1e-6, + "a declared opacity: 0 authored as the fade-in's own starting point must not cancel \ + the animation for the rest of its run (issue #430); got {:?}", + css.opacity + ); } #[test] @@ -255,4 +295,63 @@ mod tests { assert!(matches!(filters[0], FilterFn::Blur { .. })); assert!(matches!(filters[1], FilterFn::DropShadow { .. })); } + + #[test] + fn an_animation_touching_only_font_size_reaches_the_css_style() { + let mut css = CssStyle::default(); + let props = AnimatedProperties { + font_size: 120.0, + ..AnimatedProperties::default() + }; + apply_animated_props(&mut css, &props); + assert_eq!(css.font_size, Some(Length::Px(120.0))); + } + + #[test] + fn an_animation_touching_only_letter_spacing_reaches_the_css_style() { + let mut css = CssStyle::default(); + let props = AnimatedProperties { + letter_spacing: 6.0, + ..AnimatedProperties::default() + }; + apply_animated_props(&mut css, &props); + assert_eq!(css.letter_spacing, Some(Length::Px(6.0))); + } + + #[test] + fn an_untouched_letter_spacing_nan_resting_value_leaves_css_untouched() { + let mut css = CssStyle::default(); + let props = AnimatedProperties::default(); + apply_animated_props(&mut css, &props); + assert!( + css.letter_spacing.is_none(), + "letter_spacing's resting value is NaN precisely so 'not animated' can be told apart \ + from 'animated to zero'; it must not be written as a css value" + ); + } + + #[test] + fn an_animation_touching_only_gap_reaches_the_css_style() { + let mut css = CssStyle::default(); + let props = AnimatedProperties { + gap: 24.0, + ..AnimatedProperties::default() + }; + apply_animated_props(&mut css, &props); + assert_eq!(css.gap, Some(Gap::Uniform(LengthPercentage::Px(24.0)))); + } + + #[test] + fn an_animation_touching_only_padding_reaches_the_css_style() { + let mut css = CssStyle::default(); + let props = AnimatedProperties { + padding: 32.0, + ..AnimatedProperties::default() + }; + apply_animated_props(&mut css, &props); + assert_eq!( + css.padding, + Some(Edges::Uniform(LengthPercentage::Px(32.0))) + ); + } } diff --git a/crates/rustmotion-core/src/css/cascade.rs b/crates/rustmotion-core/src/css/cascade.rs index 59ee551..a25c0ca 100644 --- a/crates/rustmotion-core/src/css/cascade.rs +++ b/crates/rustmotion-core/src/css/cascade.rs @@ -39,6 +39,45 @@ pub fn inherit_from(parent: &CssStyle, child: &mut CssStyle) { } } +pub fn overlay_resolved_typography(resolved: &CssStyle, own: &mut CssStyle) { + if resolved.color.is_some() { + own.color = resolved.color.clone(); + } + if resolved.font_family.is_some() { + own.font_family = resolved.font_family.clone(); + } + if resolved.font_size.is_some() { + own.font_size = resolved.font_size.clone(); + } + if resolved.font_weight.is_some() { + own.font_weight = resolved.font_weight.clone(); + } + if resolved.font_style.is_some() { + own.font_style = resolved.font_style; + } + if resolved.line_height.is_some() { + own.line_height = resolved.line_height.clone(); + } + if resolved.letter_spacing.is_some() { + own.letter_spacing = resolved.letter_spacing.clone(); + } + if resolved.text_align.is_some() { + own.text_align = resolved.text_align; + } + if resolved.white_space.is_some() { + own.white_space = resolved.white_space; + } + if resolved.overflow_wrap.is_some() { + own.overflow_wrap = resolved.overflow_wrap; + } + if resolved.visibility.is_some() { + own.visibility = resolved.visibility; + } + if resolved.text_decoration.is_some() { + own.text_decoration = resolved.text_decoration.clone(); + } +} + #[cfg(test)] mod tests { use super::*; @@ -97,4 +136,55 @@ mod tests { assert_eq!(child.font_style, Some(FontStyle::Italic)); assert_eq!(child.text_align, Some(TextAlign::Center)); } + + #[test] + fn overlay_resolved_typography_replaces_a_component_own_static_font_size() { + use crate::css::units::Length; + + let resolved = CssStyle { + font_size: Some(Length::Px(120.0)), + ..Default::default() + }; + let mut own = CssStyle { + font_size: Some(Length::Px(20.0)), + ..Default::default() + }; + overlay_resolved_typography(&resolved, &mut own); + assert_eq!( + own.font_size, + Some(Length::Px(120.0)), + "the box's own already-animated resolved font-size must win over the component's \ + declared static one, unlike inherit_from's fill-only-if-none rule" + ); + } + + #[test] + fn overlay_resolved_typography_replaces_a_component_own_static_letter_spacing() { + use crate::css::units::Length; + + let resolved = CssStyle { + letter_spacing: Some(Length::Px(18.0)), + ..Default::default() + }; + let mut own = CssStyle { + letter_spacing: Some(Length::Px(0.0)), + ..Default::default() + }; + overlay_resolved_typography(&resolved, &mut own); + assert_eq!(own.letter_spacing, Some(Length::Px(18.0))); + } + + #[test] + fn overlay_resolved_typography_leaves_a_field_alone_when_resolved_lacks_it() { + let resolved = CssStyle::default(); + let mut own = CssStyle { + color: Some(Color::String("#00ff00".into())), + ..Default::default() + }; + overlay_resolved_typography(&resolved, &mut own); + match own.color { + Some(Color::String(s)) => assert_eq!(s, "#00ff00"), + other => panic!("resolved had no color; own's must be left alone, got {other:?}"), + } + } } From 41b59893687f1d8f88902c665c6176a4abc44266 Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Tue, 29 Sep 2026 14:34:45 +0200 Subject: [PATCH 2/2] test(cascade): call with_cascaded_style the way the render path calls it with_cascaded_style had two meanings depending on who called it. The render path passes the node's own already-resolved style, where overwriting is right because that style has already applied the component's own declaration over its parent's. These two tests passed it a bare parent style instead, where filling only the gaps was right. Making the function overwrite fixed the first caller and broke the second, which is what turned table_own_color_wins_over_cascaded_card_color red on CI while every local scoped run stayed green. The invariant both tests assert is unchanged and still worth asserting. They now build the resolved style the box tree builds -- the component's own, with inherit_from filling what it left unset -- and pass that, which is the shape the render path actually uses. --- crates/rustmotion-components/tests/audit_ws_i.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/rustmotion-components/tests/audit_ws_i.rs b/crates/rustmotion-components/tests/audit_ws_i.rs index dc25755..331e8eb 100644 --- a/crates/rustmotion-components/tests/audit_ws_i.rs +++ b/crates/rustmotion-components/tests/audit_ws_i.rs @@ -672,8 +672,10 @@ fn newly_cascaded_components_inherit_unset_typography_from_the_parent() { for (name, json) in cases { let component: Component = serde_json::from_value(json.clone()).unwrap_or_else(|e| panic!("{name}: {e}")); + let mut resolved = component.as_styled().style_config().clone(); + rustmotion_core::css::cascade::inherit_from(&parent, &mut resolved); let cascaded = component - .with_cascaded_style(&parent) + .with_cascaded_style(&resolved) .unwrap_or_else(|| panic!("{name} must be classified as typographic")); let style = cascaded.as_styled().style_config(); assert_eq!( @@ -703,8 +705,11 @@ fn table_own_color_wins_over_cascaded_card_color() { "style": { "color": "#00ff00" } })) .expect("deserialize table"); + let mut resolved = component.as_styled().style_config().clone(); + rustmotion_core::css::cascade::inherit_from(&parent, &mut resolved); + let cascaded = component - .with_cascaded_style(&parent) + .with_cascaded_style(&resolved) .expect("table is typographic"); assert_eq!( cascaded.as_styled().style_config().color,