diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 07e10293..c584463d 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -12,8 +12,8 @@ jobs: fmt: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@stable + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + - uses: dtolnay/rust-toolchain@6bed0761d98439e5a578e2877258200ad565ba87 # stable channel, 2026-09-22 with: components: rustfmt - name: Check formatting @@ -22,27 +22,74 @@ jobs: clippy: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - name: Install system dependencies # webkit/gtk/xdo: required to compile rustmotion-studio (dioxus desktop) # asound: required by cpal, which rodio pulls in for preview audio run: sudo apt-get update && sudo apt-get install -y libfontconfig1-dev libfreetype6-dev libwebkit2gtk-4.1-dev libgtk-3-dev libxdo-dev libasound2-dev - - uses: dtolnay/rust-toolchain@stable + - uses: dtolnay/rust-toolchain@6bed0761d98439e5a578e2877258200ad565ba87 # stable channel, 2026-09-22 with: components: clippy - - uses: Swatinem/rust-cache@v2 + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 - name: Clippy run: cargo clippy --workspace --all-targets -- -D warnings test: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - name: Install system dependencies # webkit/gtk/xdo: required to compile rustmotion-studio (dioxus desktop) # asound: required by cpal, which rodio pulls in for preview audio run: sudo apt-get update && sudo apt-get install -y libfontconfig1-dev libfreetype6-dev libwebkit2gtk-4.1-dev libgtk-3-dev libxdo-dev libasound2-dev - - uses: dtolnay/rust-toolchain@stable - - uses: Swatinem/rust-cache@v2 + - uses: dtolnay/rust-toolchain@6bed0761d98439e5a578e2877258200ad565ba87 # stable channel, 2026-09-22 + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 - name: Run tests run: cargo test --workspace + + audit: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + - uses: dtolnay/rust-toolchain@6bed0761d98439e5a578e2877258200ad565ba87 # stable channel, 2026-09-22 + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + - name: Install cargo-audit + run: cargo install cargo-audit --locked + # Blocking on any advisory not listed below — a newly introduced + # vulnerability fails this job. Every `--ignore` is a pre-existing + # transitive-dependency advisory tolerated today because the fix is a + # dependency version bump, and version bumps for the published + # `rustmotion` crate are being handled separately from this workstream + # (crates/rustmotion/Cargo.toml, orchestrator-owned). Unmaintained/ + # unsound/yanked advisories (17 as of 2026-09-22) print but do not fail + # the job — that's `cargo audit`'s own default, left unchanged here. + # + # Review by 2026-12-22, or sooner once the dependency bumps land: + # RUSTSEC-2025-0008 — openh264-sys2 0.6.6, heap overflow in decoding. + # Direct dependency of the published `rustmotion` crate. Fix: openh264 >=0.8.0. + # RUSTSEC-2026-0204 — crossbeam-epoch 0.9.18, invalid pointer deref in `fmt::Pointer`. + # Via rayon-core <- exr <- image, reaches rustmotion-core/-components. Fix: >=0.9.20. + # RUSTSEC-2026-0195, RUSTSEC-2026-0194 — quick-xml 0.38.4 / 0.39.4, DoS + quadratic runtime. + # 0.38.4 via syntect reaches the published crates; 0.39.4 via dioxus-desktop/rfd is + # rustmotion-studio-only (Linux/Wayland file dialogs). Fix: >=0.41.0. + # RUSTSEC-2026-0285 — rustls 0.23.37, TLS 1.3 handshake level-boundary bug. + # Via ureq, used by rustmotion/rustmotion-core for Google Fonts + Iconify fetches. Fix: >=0.23.45. + # RUSTSEC-2026-0104, RUSTSEC-2026-0098, RUSTSEC-2026-0099, RUSTSEC-2026-0049 — rustls-webpki + # 0.103.9, four CRL/name-constraint parsing bugs. Same ureq path as rustls above. + # Fix: >=0.103.13,<0.104.0-alpha.1 (or the matching 0.104 alpha per advisory). + # RUSTSEC-2026-0257 — webbrowser 1.2.1, BROWSER env argument injection on Unix. + # Via dioxus-desktop, rustmotion-studio only (`publish = false`, never reaches a published + # crate). Fix: >=1.2.2. + - name: Audit dependencies + run: > + cargo audit + --ignore RUSTSEC-2025-0008 + --ignore RUSTSEC-2026-0204 + --ignore RUSTSEC-2026-0195 + --ignore RUSTSEC-2026-0194 + --ignore RUSTSEC-2026-0285 + --ignore RUSTSEC-2026-0104 + --ignore RUSTSEC-2026-0098 + --ignore RUSTSEC-2026-0099 + --ignore RUSTSEC-2026-0049 + --ignore RUSTSEC-2026-0257 diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml index 8a70df11..c30152cd 100644 --- a/.github/workflows/publish.yaml +++ b/.github/workflows/publish.yaml @@ -9,7 +9,7 @@ jobs: publish: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - name: Install system dependencies # Doit rester identique à ci.yaml : l'étape « Run tests » ci-dessous lance @@ -20,7 +20,7 @@ jobs: # asound: required by cpal, which rodio pulls in for preview audio run: sudo apt-get update && sudo apt-get install -y libfontconfig1-dev libfreetype6-dev libwebkit2gtk-4.1-dev libgtk-3-dev libxdo-dev libasound2-dev - - uses: dtolnay/rust-toolchain@stable + - uses: dtolnay/rust-toolchain@6bed0761d98439e5a578e2877258200ad565ba87 # stable channel, 2026-09-22 # La version se lit via `cargo metadata`, pas en grepant un manifeste : elle # est déclarée dans `[workspace.package]` et héritée, donc un `grep diff --git a/README.md b/README.md index f9b34b6a..c119f1d9 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,8 @@ A CLI tool that renders motion design videos from JSON scenarios. No browser, no [![docs.rs](https://docs.rs/rustmotion/badge.svg)](https://docs.rs/rustmotion) [![License: MIT](https://img.shields.io/badge/license-MIT-blue.svg)](LICENSE) +MIT-licensed: no licence key, no telemetry, no per-render billing. See [Non-goals](docs/non-goals.md) for this and everything else rustmotion deliberately doesn't do (no embeddable Player/browser/React, no vendor-cloud deploy target, ...). + ## Install ```bash @@ -87,19 +89,117 @@ Once installed, Claude Code automatically loads the skills when you work in that ## CLI Reference +### `rustmotion validate` + +Schema + geometry checks, with no render. This is the gate every generated scenario is expected to pass before use. + +| Flag | Description | Default | +|---|---|---| +| `-f, --file ` | Path to the JSON scenario file | (required) | +| `--report ` | Write a machine-readable JSON report of all violations | | +| `--fix` | Auto-fix safe violations in place (`auto_scroll: true`, drop `white-space` back to wrap, `text-autofit: true`) — refuses templated scenarios (`include`/`for-each`/`use`) | `false` | +| `--strict-anim` | Sample animated frames and reapply renderer transforms to detect per-frame viewport overflow (slower) | `false` | +| `--lenient` | Treat geometry violations as warnings instead of errors | `false` | +| `--props ` | Load variable overrides from a JSON object file | | +| `--var ` | Set a single variable override (repeatable); `--var` wins over `--props` | | + ### `rustmotion render` | Flag | Description | Default | |---|---|---| -| `input` | Path to the JSON scenario file | (required) | +| `-f, --file ` | Path to the JSON scenario file (or `--json ` for inline input) | (required) | | `-o, --output` | Output file path | `output.mp4` | | `--frame ` | Render a single frame to PNG (0-indexed) | | +| `--frames ` | Render only frames `START..=END` as a standalone segment with its own windowed audio slice, for joining later with `rustmotion concat`. Mutually exclusive with `--frame`/`--watch`; only mp4/webm/mov are implemented for a range | | | `--codec ` | Video codec: `h264`, `h265`, `vp9`, `prores` | `h264` | | `--crf <0-51>` | Constant Rate Factor (lower = better quality) | `23` | | `--format ` | Output format: `mp4`, `webm`, `mov`, `gif`, `png-seq` | auto from extension | | `--transparent` | Transparent background (PNG sequence, WebM, ProRes 4444) | `false` | +| `--hardware-acceleration` | Probe `ffmpeg -encoders` and use this machine's hardware encoder (VideoToolbox/NVENC/QSV/AMF) when available; explicit message and software fallback otherwise | `false` | +| `-w, --watch` | Watch the input file and re-render on change (not compatible with `--props`/`--var`) | `false` | +| `--no-validate` | Skip the implicit validate pass (schema + geometry + variables) before rendering | `false` | +| `--lenient` | Treat geometry violations as warnings during the implicit validate pass | `false` | +| `--strict-anim` | Sample animated frames for per-frame viewport overflow during the implicit validate pass | `false` | +| `--props ` | Load variable overrides from a JSON object file | | +| `--var ` | Set a single variable override (repeatable); `--var` wins over `--props` | | | `--output-format json` | Machine-readable JSON output for CI pipelines | | | `-q, --quiet` | Suppress all output except errors | | +| `--threads ` | Number of parallel rendering threads (global flag) | all cores | + +### `rustmotion concat` + +Joins segment files — e.g. several `render --frames a-b` outputs from the same scenario — via ffmpeg's concat demuxer (`-c copy`, no re-encoding). Requires ffmpeg on PATH. + +```bash +rustmotion concat seg1.mp4 seg2.mp4 -o out.mp4 +``` + +### `rustmotion still` + +Exports a single frame as a still image (PNG/JPEG/WebP). + +| Flag | Description | Default | +|---|---|---| +| `-f, --file ` | Path to the JSON scenario file | (required) | +| `-o, --output` | Output file path | `still.png` | +| `--time ` | Time to capture | `0.0` | +| `--format ` | Image format: `png`, `jpeg`, `webp` | from extension | +| `--quality <1-100>` | JPEG quality | `90` | +| `--props` / `--var` | Variable overrides, same as `render` | | + +### `rustmotion captions` + +Generates word-level caption timings from audio (via a local `whisper.cpp` binary) or by importing subtitles. + +```bash +rustmotion captions voice.mp3 -o words.json +rustmotion captions --from-srt subs.srt -o words.json +``` + +| Flag | Description | Default | +|---|---|---| +| `audio` | Audio file to transcribe (mutually exclusive with `--from-srt`/`--from-vtt`) | | +| `-o, --output` | Output JSON file (stdout if omitted) | | +| `--model` | Whisper model name (`tiny`, `base`, `small`, `medium`, `large-v3`) or a path to a `.bin` | `base` | +| `--lang` | Spoken language code (auto-detected if omitted) | | +| `--from-srt` / `--from-vtt` | Import cues from a subtitle file instead of transcribing | | + +### `rustmotion batch` + +Renders one video per line of a JSONL data file, substituting each line's fields as variable overrides. + +| Flag | Description | Default | +|---|---|---| +| `-f, --file ` | Path to the scenario template (JSON or HTML dialect) | (required) | +| `--data ` | JSONL file, one object of variable overrides per line | (required) | +| `--output-dir ` | Directory to write output files into | (required) | +| `--name-template` | Output filename template (`{field}`, `{index}`) | `"{index}.mp4"` | +| `--codec` / `--crf` / `--format` / `--transparent` | Same as `render` | | +| `--jobs ` | Videos to render in parallel (the render itself already uses all cores via rayon) | `1` | + +### `rustmotion schema` + +Prints the JSON Schema for scenario files (editor autocompletion, LLM prompts). + +```bash +rustmotion schema -o schema.json +``` + +### `rustmotion info` + +Shows information about a scenario (duration, scene count, dimensions, ...). + +```bash +rustmotion info scenario.json +``` + +### `rustmotion skills` + +Manages the built-in Claude Code skills — `install [--global]`, `uninstall [--global]`, `list`, `show `. See [Claude Code Skills](#claude-code-skills). + +### `rustmotion completions` + +Generates or installs shell completions — `install`, `uninstall`, `generate `. See [Shell Completions](#shell-completions). --- @@ -123,7 +223,6 @@ Once installed, Claude Code automatically loads the skills when you work in that "height": 1920, "fps": 30, "background": "#0f172a", - "codec": "h264", "crf": 23 } } @@ -135,7 +234,7 @@ Once installed, Claude Code automatically loads the skills when you work in that | `height` | `u32` | (required) | Video height in pixels (must be even) | | `fps` | `u32` | `30` | Frames per second | | `background` | `string` | `"#000000"` | Default background color (hex) | -| `codec` | `string` | `"h264"` | Video codec: `h264`, `h265`, `vp9`, `prores` | +| `codec` | `string` | | Accepted by the schema (`h264`, `h265`, `vp9`, `prores`) but not yet read by the encoder — set the codec with `render --codec`/`batch --codec` instead | | `crf` | `u8` | `23` | Constant Rate Factor (0-51, lower = better quality) | ### Audio Tracks @@ -2025,42 +2124,42 @@ Transparency is supported with `--transparent` for PNG sequences, WebM (VP9), an - **JSON Schema:** schemars (auto-generated from Rust types) - **Parallelism:** rayon (multi-threaded frame rendering) -## Architecture +rustmotion ships 60 components, each implementing the `Painter` trait, through a CSS-inspired **box_tree → layout_pass → paint_pass** pipeline: -rustmotion uses a Flutter-inspired **measure → layout → paint** pipeline built on Skia: +1. **box_tree** — builds a tree of `BoxNode { css: CssStyle, children, intrinsic }` from the resolved JSON components +2. **layout_pass** — runs [taffy](https://github.com/DioxusLabs/taffy) to compute each node's `BoxLayout { x, y, width, height }`; leaves that carry an `IntrinsicMeasure` (text, images, codeblocks, ...) are measured through a `measure_fn` +3. **paint_pass** — walks the tree top-down, applies transform/opacity, paints decorations (background, border, shadow), and delegates content painting to the component's `Painter` implementation -``` -src/ -├── components/ # 51 components (each implements Widget trait) -│ ├── chart/ # Chart sub-modules (bar, line, pie, radar, etc.) -│ └── *.rs # One file per component -├── engine/ -│ ├── render/ # Render pipeline (component, scene, background, transforms) -│ ├── codeblock/ # Codeblock rendering (highlight, chrome, reveal, diff) -│ ├── animator.rs # Animation resolver, easing, spring solver -│ └── renderer.rs # Skia drawing primitives -├── schema/ # Data models -│ ├── scenario.rs # Scenario, View, Scene, VideoConfig -│ ├── style.rs # LayerStyle, FontWeight, layout types -│ ├── background.rs # Animated backgrounds -│ ├── animation.rs # EasingType, presets -│ └── video.rs # AnimationEffect, shapes, fills -├── layout/ # Flex/grid layout engines -├── traits/ # Widget, Styled, Animatable, Timed, Container -└── macros.rs # impl_traits! macro +```rust +pub trait Painter { + fn paint_content(&self, canvas: &Canvas, layout: &BoxLayout, props: &AnimatedProperties, ctx: &PaintCtx); + fn intrinsic_size(&self, available: AvailableSize, ctx: &MeasureCtx) -> Option<(f32, f32)> { None } +} ``` -Every component implements the `Widget` trait: +`PaintCtx` carries `time`, `scene_duration`, `fps`, `frame_index`, `video_width`, `video_height`, `stagger_offset`. -```rust -trait Widget { - fn paint(&self, canvas: &Canvas, ctx: &PaintContext) -> Result<()>; - fn measure(&self, constraints: &Constraints) -> (f32, f32); - fn layout(&self, constraints: &Constraints) -> LayoutNode; -} +### Workspace layout + +``` +crates/ +├── rustmotion-core/src/ +│ ├── css/ # CssStyle, units, cascade, taffy bridge, animation resolution +│ ├── engine/ # box_tree, layout_pass, paint_pass, animator, transitions, Skia primitives +│ ├── schema/ # Scenario, Scene, VideoConfig, style, background, animation, codeblock models +│ └── traits/ # Painter, Animatable, Timed, Styled +├── rustmotion-components/src/ +│ ├── lib.rs # `Component` enum (60 variants) + dispatch (as_painter, as_animatable, ...) +│ ├── box_builder.rs # JSON components → BuiltScene (box tree + stagger delays) +│ ├── chart/ # bar/line/pie/radar/scatter/radial/funnel/waterfall sub-modules +│ └── *.rs # one file per component (Painter implementation) +└── rustmotion/src/ + ├── cli/ # the `rustmotion` binary (clap subcommands) + ├── encode/ # video/audio encoders and muxing + └── loader.rs # JSON/HTML → ResolvedScenario ``` -`PaintContext` provides timing, layout dimensions, parent info, and resolved animated properties in a single struct. +The `rustmotion` crate is where the binary lives — a crate with only a `[lib]` target installs nothing executable via `cargo install`. ## License diff --git a/crates/rustmotion-components/Cargo.toml b/crates/rustmotion-components/Cargo.toml index adf8f0e1..db478ee6 100644 --- a/crates/rustmotion-components/Cargo.toml +++ b/crates/rustmotion-components/Cargo.toml @@ -2,7 +2,7 @@ name = "rustmotion-components" version.workspace = true edition = "2021" -description = "Component library for rustmotion (51 components)" +description = "Component library for rustmotion (60 components)" license = "MIT" repository = "https://github.com/LeadcodeDev/rustmotion" readme = "../../README.md" diff --git a/crates/rustmotion-components/src/badge.rs b/crates/rustmotion-components/src/badge.rs index 5e19ea9f..f4f2cdbb 100644 --- a/crates/rustmotion-components/src/badge.rs +++ b/crates/rustmotion-components/src/badge.rs @@ -441,7 +441,9 @@ mod tests { // Solid variant text is always white — probe for white ink // specifically, since the pill background paints regardless. let text_ink = buf - .chunks_exact(4) + .as_chunks::<4>() + .0 + .iter() .filter(|p| p[3] > 0 && p[0] > 200 && p[1] > 200 && p[2] > 200) .count(); assert!( diff --git a/crates/rustmotion-components/src/box_builder.rs b/crates/rustmotion-components/src/box_builder.rs index 21d151ff..f5ab158b 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,9 +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. `Component::with_cascaded_style` folds it into +/// whichever component variants read inherited typography off their own +/// style before this function's match ever sees them, so the reserved box +/// always matches what those components' painters (which fold the same +/// cascade in at paint time) actually draw. fn component_intrinsic( component: &Component, + cascaded_css: &CssStyle, ) -> Option> { + let cascaded_component = component.with_cascaded_style(cascaded_css); + let component = cascaded_component.as_ref().unwrap_or(component); + use Component::*; match component { Text(t) => Some(Arc::new(crate::intrinsic::TextIntrinsic::from_text(t))), @@ -1495,7 +1508,9 @@ fn apply_intrinsic_overrides(component: &Component, css: &mut CssStyle) { css.width = Some(CSize::Length(CLP::Px(c.width))); } if css.height.is_none() { - let font_size = c.style.font_size_px_or(16.0); + let font_size = c + .style + .font_size_px_ctx(&crate::intrinsic::measure_time_font_size_ctx(0.0), 16.0); let line_height = font_size * 1.3; let n = c.items.len() as f32; let h = n * line_height + (n - 1.0).max(0.0) * c.gap; @@ -1640,7 +1655,9 @@ fn apply_intrinsic_overrides(component: &Component, css: &mut CssStyle) { // box is fit exactly to the unwrapped text width, the painter's // own `wrap_text(text, font, Some(text_area_w))` never has a // reason to wrap, so painted output matches this box exactly. - let font_size = t.style.font_size_px_or(16.0); + let font_size = t + .style + .font_size_px_ctx(&crate::intrinsic::measure_time_font_size_ctx(0.0), 16.0); let family = t.style.font_family_or("Inter"); let text_w = measure_text_line_width(&t.text, font_size, family, false); let h_pad = 12.0; // callout.rs's own `let padding = 12.0;` @@ -1660,7 +1677,10 @@ fn apply_intrinsic_overrides(component: &Component, css: &mut CssStyle) { // Same shape as Callout above; padding value borrowed from // callout.rs since tooltip.rs's own paint() centers text in the // body with no defined constant of its own. - let font_size = t.style.font_size_px_or(t.font_size); + let font_size = t.style.font_size_px_ctx( + &crate::intrinsic::measure_time_font_size_ctx(0.0), + t.font_size, + ); let family = t.style.font_family_or("Inter"); let text_w = measure_text_line_width(&t.text, font_size, family, false); let h_pad = 12.0; @@ -1685,7 +1705,9 @@ fn apply_intrinsic_overrides(component: &Component, css: &mut CssStyle) { // formula (h_pad = font_size*1.2 per side, `gap` before/after/ // between every pill) using the same public fields and the same // `measure_text_with_fallback` call it makes internally. - let font_size = p.style.font_size_px_or(14.0); + let font_size = p + .style + .font_size_px_ctx(&crate::intrinsic::measure_time_font_size_ctx(0.0), 14.0); let family = p.style.font_family_or("Inter"); let h_pad = font_size * 1.2; let n = p.items.len() as f32; @@ -1713,7 +1735,10 @@ fn apply_intrinsic_overrides(component: &Component, css: &mut CssStyle) { // height ratio both of those same real usages share: // `font_size: 24` paired with `style.height: 48`, i.e. // `2 × font_size`. - let font_size = m.style.font_size_px_or(m.font_size); + let font_size = m.style.font_size_px_ctx( + &crate::intrinsic::measure_time_font_size_ctx(0.0), + m.font_size, + ); apply_default_size(css, 800.0, font_size * 2.0); } Stepper(s) => { @@ -2125,7 +2150,7 @@ pub fn component_kind(c: &Component) -> &'static str { Particle(_) => "particle", PillNav(_) => "pill_nav", Progress(_) => "progress", - QrCode(_) => "qrcode", + QrCode(_) => "qr_code", NumberWheel(_) => "number_wheel", SuccessCheck(_) => "success_check", Pointer(_) => "pointer", @@ -2147,7 +2172,11 @@ pub fn component_kind(c: &Component) -> &'static str { Flex(_) => "flex", Grid(_) => "grid", Card(_) => "card", - Container(_) => "container", + // The schema tag is `div` (`#[serde(rename = "div", alias = + // "container")]` on the enum in `lib.rs`) — `container` only + // survives as a deserialize alias, so naming it that way here told + // an author to look for a tag their scenario cannot contain. + Container(_) => "div", AudioSpectrum(_) => "audio_spectrum", Waveform(_) => "waveform", } diff --git a/crates/rustmotion-components/src/callout.rs b/crates/rustmotion-components/src/callout.rs index 22e8bf97..8092d4aa 100644 --- a/crates/rustmotion-components/src/callout.rs +++ b/crates/rustmotion-components/src/callout.rs @@ -267,7 +267,9 @@ mod tests { // for near-white ink specifically, since the bubble background // paints regardless of font-size. let text_ink = buf - .chunks_exact(4) + .as_chunks::<4>() + .0 + .iter() .filter(|p| p[3] > 0 && p[0] > 200 && p[1] > 200 && p[2] > 200) .count(); assert!( diff --git a/crates/rustmotion-components/src/counter.rs b/crates/rustmotion-components/src/counter.rs index ebe1108e..e67af735 100644 --- a/crates/rustmotion-components/src/counter.rs +++ b/crates/rustmotion-components/src/counter.rs @@ -403,7 +403,7 @@ mod tests { skia_safe::image::CachingHint::Disallow, ); assert!(ok, "pixel read should succeed"); - let lit = buf.chunks_exact(4).filter(|p| p[3] > 0).count(); + let lit = buf.as_chunks::<4>().0.iter().filter(|p| p[3] > 0).count(); assert!( lit > 20, "counter at font-size: 2rem must paint visible ink, got {lit} lit pixels" diff --git a/crates/rustmotion-components/src/gif.rs b/crates/rustmotion-components/src/gif.rs index e1738e04..5a0d918d 100644 --- a/crates/rustmotion-components/src/gif.rs +++ b/crates/rustmotion-components/src/gif.rs @@ -86,8 +86,42 @@ fn clear_rect(composed: &mut [u8], canvas_w: u32, canvas_h: u32, frame: &gif::Fr /// `gif_cache` stores. type DecodedGif = (Vec<(Vec, u32, u32)>, Vec, f64); +/// Artificial per-decode stall, settable only from this file's own tests +/// (`DECODE_STALL_MS`) to open a deterministic race window around the +/// cache-miss branch without timing-dependent sleeps sprinkled through the +/// test itself. Zero by default, and compiled out entirely in a non-test +/// build — no production cost. +#[cfg(test)] +static DECODE_STALL_MS: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); + +#[cfg(test)] +fn stall_decode_for_tests() { + let stall_ms = DECODE_STALL_MS.load(std::sync::atomic::Ordering::SeqCst); + if stall_ms > 0 { + std::thread::sleep(std::time::Duration::from_millis(stall_ms)); + } +} + +/// 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 @@ -97,7 +131,7 @@ type DecodedGif = (Vec<(Vec, u32, u32)>, Vec, f64); /// 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) => { @@ -123,12 +157,37 @@ 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(); + let mut frames: Vec<(Vec, u32, u32)> = Vec::new(); let mut cumulative_times: Vec = Vec::new(); let mut accumulated = 0.0; 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()); @@ -163,6 +222,25 @@ fn decode_composed_frames(src: &str) -> Option { Some((frames, cumulative_times, accumulated)) } +/// Cache lookup with the actual decode folded in, single-flighted through +/// `DashMap::entry`: a vacant entry holds its shard's write lock for as long +/// as the closure runs, so a second caller racing the same cache-cold `src` +/// blocks on that lock instead of starting its own redundant decode. Decode +/// 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, max_canvas_w: u32, max_canvas_h: u32) -> Option> { + gif_cache() + .entry(src.to_string()) + .or_try_insert_with(|| { + decode_composed_frames(src, max_canvas_w, max_canvas_h) + .map(Arc::new) + .ok_or(()) + }) + .ok() + .map(|entry| entry.clone()) +} + impl Painter for Gif { fn paint_content( &self, @@ -171,17 +249,8 @@ impl Painter for Gif { _props: &AnimatedProperties, ctx: &PaintCtx, ) { - let gcache = gif_cache(); - - let cached = if let Some(cached) = gcache.get(&self.src) { - cached.clone() - } else { - let Some(decoded) = decode_composed_frames(&self.src) else { - return; - }; - let cached = Arc::new(decoded); - gcache.insert(self.src.clone(), cached.clone()); - cached + let Some(cached) = cached_decode(&self.src, ctx.video_width, ctx.video_height) else { + return; }; let (ref frames, ref cumulative_times, total_duration) = *cached; @@ -257,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"); @@ -287,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"))), @@ -302,4 +372,182 @@ mod tests { frame.top = 3; assert_eq!(frame_rect(4, 4, &frame), (3, 3, 1, 1)); } + + /// Releases `DECODE_STALL_MS` back to zero even if the test body + /// panics mid-assertion, so a failing run never leaks a stall into + /// whatever other test in this binary decodes a GIF next. + struct StallGuard; + + impl Drop for StallGuard { + fn drop(&mut self) { + DECODE_STALL_MS.store(0, std::sync::atomic::Ordering::SeqCst); + } + } + + /// N callers racing a cache-cold `src` must decode it once, not N + /// times. A 120ms stall (`DECODE_STALL_MS`) right after the header is + /// read opens a race window wide enough that every thread reaches the + /// cache-miss branch before any of them can finish decoding and insert — + /// pre-fix, that means eight independent decodes, each producing its own + /// `Arc` allocation. Comparing pointers (not content — a deterministic + /// decode produces byte-identical content either way) is what proves + /// only one of the eight actually ran. + #[test] + fn concurrent_paints_of_the_same_uncached_gif_decode_exactly_once() { + let _guard = StallGuard; + DECODE_STALL_MS.store(120, std::sync::atomic::Ordering::SeqCst); + + let path = std::env::temp_dir().join(format!( + "rustmotion_gif_stampede_{}_{}.gif", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos(), + )); + write_two_frame_gif(&path); + let src = path.to_str().expect("utf-8 path").to_string(); + + const THREADS: usize = 8; + let barrier = std::sync::Arc::new(std::sync::Barrier::new(THREADS)); + let handles: Vec<_> = (0..THREADS) + .map(|_| { + let barrier = barrier.clone(); + let src = src.clone(); + std::thread::spawn(move || { + barrier.wait(); + cached_decode(&src, 4, 2) + }) + }) + .collect(); + + let results: Vec>> = + handles.into_iter().map(|h| h.join().unwrap()).collect(); + std::fs::remove_file(&path).ok(); + + let first = results[0].as_ref().expect("gif must decode"); + for (i, result) in results.iter().enumerate() { + let result = result.as_ref().unwrap_or_else(|| { + panic!("thread {i} did not get a decoded result"); + }); + assert!( + Arc::ptr_eq(first, result), + "thread {i} observed a different Arc than thread 0 — the GIF was decoded \ + more than once for the same cache-cold source" + ); + } + } + + /// 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 the audit's own 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/kbd.rs b/crates/rustmotion-components/src/kbd.rs index ecc065b8..6874cbf3 100644 --- a/crates/rustmotion-components/src/kbd.rs +++ b/crates/rustmotion-components/src/kbd.rs @@ -339,7 +339,9 @@ mod tests { // face — probe for near-white ink specifically, since the face/ // border/shadow paint regardless of font-size. let text_ink = buf - .chunks_exact(4) + .as_chunks::<4>() + .0 + .iter() .filter(|p| p[3] > 0 && p[0] > 180 && p[1] > 180 && p[2] > 180) .count(); assert!( diff --git a/crates/rustmotion-components/src/legacy_dispatch.rs b/crates/rustmotion-components/src/legacy_dispatch.rs index da4fdc58..f3fb225f 100644 --- a/crates/rustmotion-components/src/legacy_dispatch.rs +++ b/crates/rustmotion-components/src/legacy_dispatch.rs @@ -2,13 +2,23 @@ //! taffy-laid-out `BoxNode` tree back to component-typed `Painter` impls. //! //! Naming kept as "legacy" for now to avoid churn in callers; this is the -//! sole dispatcher in use since all 51 components implement `Painter`. +//! sole dispatcher in use since every component implements `Painter`. //! //! Containers (Card / Flex / Grid / Container / Positioned) are intentionally //! 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. `Component::with_cascaded_style` rebuilds the +//! subset of components that read inherited typography off their own style +//! with `css` folded in before painting, the same call +//! `box_builder::component_intrinsic` makes for the intrinsic measurers 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; @@ -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, ) { @@ -115,7 +125,9 @@ impl<'a> PaintDispatcher for LegacyPaintDispatcher<'a> { return; } - let Some(painter) = child.component.as_painter() else { + let cascaded_component = child.component.with_cascaded_style(css); + let effective_component = cascaded_component.as_ref().unwrap_or(&child.component); + let Some(painter) = effective_component.as_painter() else { return; }; diff --git a/crates/rustmotion-components/src/lib.rs b/crates/rustmotion-components/src/lib.rs index 2ac0556f..8a9b8615 100644 --- a/crates/rustmotion-components/src/lib.rs +++ b/crates/rustmotion-components/src/lib.rs @@ -67,7 +67,8 @@ pub mod world_bitmap; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use rustmotion_core::traits::{Animatable, Painter, Styled, Timed}; +use rustmotion_core::css::CssStyle; +use rustmotion_core::traits::{Animatable, Painter, Styled, StyledMut, Timed}; pub use arrow::Arrow; pub use audio_spectrum::AudioSpectrum; @@ -615,8 +616,11 @@ impl Component { } } - /// Returns the Painter trait. All 51 components are migrated to the new - /// pipeline; the dispatcher always uses Painter::paint_content. + /// Returns the Painter trait. Every `Component` variant is migrated to + /// the new pipeline; the dispatcher always uses Painter::paint_content. + /// `tests/audit_ws_i.rs::cascaded_components_match_the_verified_set` + /// exercises every variant — not a count here, which would just go + /// stale again the next time a component is added. pub fn as_painter(&self) -> Option<&dyn Painter> { match self { Component::AudioSpectrum(c) => Some(c), @@ -681,4 +685,134 @@ impl Component { Component::Connector(c) => Some(c), } } + + /// `Some(clone)` for the components whose `Painter`/intrinsic measurer + /// read inherited typography (`color`, `font-*`, `text-align`, + /// `white-space`, ...) off their own `style` field with `resolved`'s + /// twelve `cascade::inherit_from` properties folded in; `None` for every + /// other component, since the cascade cannot affect anything they draw. + /// `resolved` is the caller's own `CssStyle` post-cascade (`box_builder` + /// and `LegacyPaintDispatcher` both already have it on hand). + /// + /// The `true` set below was built by reading every component's own + /// `paint`/`paint_content` (and, where one exists, its `*Intrinsic` + /// measurer) for a direct `self.style.color`/`font_family`/`font_size`/ + /// `font_weight`/`font_style` read with no cascade in between — not by + /// guessing from the component's name. Several read only `font-size`/ + /// `font-family` and keep their own dedicated field for text colour + /// (`Kbd::text_color`, `PillNav::text_color`, `Terminal`'s theme) — + /// still members, since those two properties alone are enough for the + /// same defect: a `font-size` set on a card never reaching the child. + /// + /// Exhaustive on purpose, no wildcard arm: adding a new `Component` + /// variant is a compile error here until this match says whether it + /// belongs to that set — the same completeness `as_painter`/ + /// `as_animatable`/`as_timed`/`as_styled` above already enforce for + /// their own questions, extended to this one. + /// `tests/audit_ws_i.rs::cascaded_components_match_the_verified_set` + /// pins the current membership directly, so a variant silently + /// reclassified here still fails a test even though the compiler has + /// nothing to object to. + /// + /// Builds the clone via a `serde_json` round-trip rather than `Clone`: + /// `Component`'s inner types are already `Serialize + Deserialize` (the + /// whole scenario tree is built that way), but not every one of them is + /// `Clone` — `Caption`'s `CaptionWord`/`CaptionStyle` (`rustmotion-core`) + /// are not, and adding it there is outside this crate. The round trip + /// costs more than a field-wise clone would, but only for the + /// components in the `Some` arm, and only once per box-tree build / + /// paint call — the same per-frame cost class `box_builder` already + /// pays elsewhere. + pub fn with_cascaded_style(&self, resolved: &CssStyle) -> Option { + let is_typographic = match self { + Component::Text(_) + | Component::GradientText(_) + | Component::Caption(_) + | Component::RichText(_) + | Component::Badge(_) + | Component::Callout(_) + | Component::Counter(_) + | Component::Divider(_) + | Component::Icon(_) + | Component::Kbd(_) + | Component::List(_) + | Component::Marquee(_) + | Component::Notification(_) + | Component::NumberWheel(_) + | Component::PillNav(_) + | Component::Table(_) + | Component::Terminal(_) + | Component::Tooltip(_) => true, + Component::AudioSpectrum(_) + | Component::Shape(_) + | Component::Image(_) + | Component::Svg(_) + | Component::Video(_) + | Component::Gif(_) + | Component::Cursor(_) + | Component::Codeblock(_) + | Component::Connector(_) + | Component::Avatar(_) + | Component::AvatarGroup(_) + | Component::Arrow(_) + | Component::Chart(_) + | Component::Comparison(_) + | Component::Countdown(_) + | Component::DotMap(_) + | Component::Gauge(_) + | Component::Heatmap(_) + | Component::Line(_) + | Component::Lottie(_) + | Component::Mockup(_) + | Component::Particle(_) + | Component::Progress(_) + | Component::QrCode(_) + | Component::SuccessCheck(_) + | Component::Pointer(_) + | Component::Rating(_) + | Component::Skeleton(_) + | Component::Slider(_) + | Component::Sparkline(_) + | Component::Stat(_) + | Component::Stepper(_) + | Component::Switch(_) + | Component::TagCloud(_) + | Component::Timeline(_) + | Component::Treemap(_) + | Component::Positioned(_) + | Component::Flex(_) + | Component::Grid(_) + | Component::Card(_) + | Component::Container(_) + | Component::Waveform(_) => false, + }; + if !is_typographic { + return None; + } + let value = serde_json::to_value(self).ok()?; + let mut clone: Component = serde_json::from_value(value).ok()?; + let style = match &mut clone { + Component::Text(c) => c.style_config_mut(), + Component::GradientText(c) => c.style_config_mut(), + Component::Caption(c) => c.style_config_mut(), + Component::RichText(c) => c.style_config_mut(), + Component::Badge(c) => c.style_config_mut(), + Component::Callout(c) => c.style_config_mut(), + Component::Counter(c) => c.style_config_mut(), + Component::Divider(c) => c.style_config_mut(), + Component::Icon(c) => c.style_config_mut(), + Component::Kbd(c) => c.style_config_mut(), + Component::List(c) => c.style_config_mut(), + Component::Marquee(c) => c.style_config_mut(), + Component::Notification(c) => c.style_config_mut(), + Component::NumberWheel(c) => c.style_config_mut(), + Component::PillNav(c) => c.style_config_mut(), + Component::Table(c) => c.style_config_mut(), + Component::Terminal(c) => c.style_config_mut(), + Component::Tooltip(c) => c.style_config_mut(), + _ => unreachable!("classified as typographic by the match above"), + }; + rustmotion_core::css::cascade::inherit_from(resolved, style); + Some(clone) + } } diff --git a/crates/rustmotion-components/src/list.rs b/crates/rustmotion-components/src/list.rs index d58723cd..4e7fd71e 100644 --- a/crates/rustmotion-components/src/list.rs +++ b/crates/rustmotion-components/src/list.rs @@ -333,7 +333,7 @@ mod tests { skia_safe::image::CachingHint::Disallow, ); assert!(ok, "pixel read should succeed"); - let lit = buf.chunks_exact(4).filter(|p| p[3] > 0).count(); + let lit = buf.as_chunks::<4>().0.iter().filter(|p| p[3] > 0).count(); assert!( lit > 20, "list at font-size: 2rem must paint visible ink, got {lit} lit pixels" diff --git a/crates/rustmotion-components/src/lottie.rs b/crates/rustmotion-components/src/lottie.rs index e2b08b5c..6fee0988 100644 --- a/crates/rustmotion-components/src/lottie.rs +++ b/crates/rustmotion-components/src/lottie.rs @@ -525,7 +525,7 @@ mod tests { let mut red_sum: u64 = 0; let mut blue_sum: u64 = 0; - for px in buf.chunks_exact(4) { + for px in buf.as_chunks::<4>().0.iter() { let r = px[0] as u64; let _g = px[1] as u64; let b = px[2] as u64; diff --git a/crates/rustmotion-components/src/marquee.rs b/crates/rustmotion-components/src/marquee.rs index 08e0379c..401baaa1 100644 --- a/crates/rustmotion-components/src/marquee.rs +++ b/crates/rustmotion-components/src/marquee.rs @@ -224,7 +224,7 @@ mod tests { skia_safe::image::CachingHint::Disallow, ); assert!(ok, "pixel read should succeed"); - let lit = buf.chunks_exact(4).filter(|p| p[3] > 0).count(); + let lit = buf.as_chunks::<4>().0.iter().filter(|p| p[3] > 0).count(); assert!( lit > 20, "marquee at font-size: 2rem must paint visible ink, got {lit} lit pixels" diff --git a/crates/rustmotion-components/src/notification.rs b/crates/rustmotion-components/src/notification.rs index 436f2d35..4d63a057 100644 --- a/crates/rustmotion-components/src/notification.rs +++ b/crates/rustmotion-components/src/notification.rs @@ -451,7 +451,9 @@ mod tests { // Title text is white (#FFFFFF default) on a dark #1E293B card — // probe for near-white ink specifically. let text_ink = buf - .chunks_exact(4) + .as_chunks::<4>() + .0 + .iter() .filter(|p| p[3] > 0 && p[0] > 200 && p[1] > 200 && p[2] > 200) .count(); assert!( diff --git a/crates/rustmotion-components/src/pill_nav.rs b/crates/rustmotion-components/src/pill_nav.rs index 3b3d038d..3039de35 100644 --- a/crates/rustmotion-components/src/pill_nav.rs +++ b/crates/rustmotion-components/src/pill_nav.rs @@ -328,7 +328,7 @@ mod tests { skia_safe::image::CachingHint::Disallow, ); assert!(ok, "pixel read should succeed"); - let lit = buf.chunks_exact(4).filter(|p| p[3] > 0).count(); + let lit = buf.as_chunks::<4>().0.iter().filter(|p| p[3] > 0).count(); assert!( lit > 20, "pill_nav at font-size: 2rem must paint visible ink, got {lit} lit pixels" diff --git a/crates/rustmotion-components/src/table.rs b/crates/rustmotion-components/src/table.rs index 05fbc996..06e90ff4 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,7 +419,9 @@ 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 - .chunks_exact(4) + .as_chunks::<4>() + .0 + .iter() .filter(|p| p[3] > 0 && p[0] > 200 && p[1] > 200 && p[2] > 200) .count(); assert!( diff --git a/crates/rustmotion-components/src/terminal.rs b/crates/rustmotion-components/src/terminal.rs index 0e22037b..be6224ec 100644 --- a/crates/rustmotion-components/src/terminal.rs +++ b/crates/rustmotion-components/src/terminal.rs @@ -536,7 +536,9 @@ mod tests { // specifically: pixels that are not the dark theme background color // (#1E1E1E) and not fully transparent. let text_ink = buf - .chunks_exact(4) + .as_chunks::<4>() + .0 + .iter() .filter(|p| p[3] > 0 && !(p[0] < 40 && p[1] < 40 && p[2] < 40)) .count(); assert!( diff --git a/crates/rustmotion-components/src/tooltip.rs b/crates/rustmotion-components/src/tooltip.rs index 44cd1a54..432c1690 100644 --- a/crates/rustmotion-components/src/tooltip.rs +++ b/crates/rustmotion-components/src/tooltip.rs @@ -280,7 +280,9 @@ mod tests { // Text is near-white (#E2E8F0 default) on a dark #1E293B body — // probe for near-white ink specifically. let text_ink = buf - .chunks_exact(4) + .as_chunks::<4>() + .0 + .iter() .filter(|p| p[3] > 0 && p[0] > 180 && p[1] > 180 && p[2] > 180) .count(); assert!( diff --git a/crates/rustmotion-components/src/video.rs b/crates/rustmotion-components/src/video.rs index 0d64739f..cae7b018 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; @@ -68,15 +176,13 @@ impl Painter for Video { let img_info = ImageInfo::new( (fw as i32, fh as i32), ColorType::RGBA8888, - skia_safe::AlphaType::Premul, + skia_safe::AlphaType::Unpremul, None, ); 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..7ac9dbd8 --- /dev/null +++ b/crates/rustmotion-components/tests/audit_ws_d.rs @@ -0,0 +1,323 @@ +//! 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" + ); +} + +// ─── cached-frame draw path mistagged straight alpha as premultiplied ────── + +/// ffmpeg's `-pix_fmt rgba` output — what fills the video-frame cache — is +/// straight (unpremultiplied) alpha. Tagging that buffer `AlphaType::Premul` +/// makes Skia treat the RGB channels as already scaled by alpha instead of +/// scaling them itself, which brightens (here: doubles) every +/// semi-transparent pixel's channels once composited. +/// +/// A straight-alpha (200, 100, 50, 128) pixel, composited over black: +/// correctly tagged `Unpremul`, Skia premultiplies it to +/// (200×128/255, 100×128/255, 50×128/255) ≈ (100, 50, 25) before compositing +/// over black, landing there almost exactly (the `(1 - alpha) * 0` background +/// term vanishes either way). Mistagged `Premul`, Skia uses the raw channel +/// values directly as if already scaled — (200, 100, 50) — composited over +/// black with no further scaling, landing at roughly double the correct +/// result. +#[test] +fn cached_frame_straight_alpha_composites_correctly_not_doubled() { + let src = unique_src("alpha"); + let cache_key = format!("{src}:10x10"); + let straight = [200u8, 100, 50, 128]; + video_frame_cache().insert( + cache_key, + Arc::new(vec![(0.0, solid_rgba(straight, 2, 2), 2, 2)]), + ); + + let v = video(&src, ImageFit::Fill, None, None, None); + let ctx = ctx_at(0.0); + let pixels = paint_and_read(&v, &ctx, 10, 10, false); + let composited = px(&pixels, 10, 5, 5); + + let close = |actual: u8, expected: u8| (actual as i16 - expected as i16).abs() <= 4; + assert!( + close(composited[0], 100) && close(composited[1], 50) && close(composited[2], 25), + "straight-alpha (200,100,50,128) over black must composite to roughly (100,50,25), \ + got {composited:?} — a value near (200,100,50) means the buffer is still mistagged \ + as premultiplied and its channels are being used unscaled" + ); +} diff --git a/crates/rustmotion-components/tests/audit_ws_i.rs b/crates/rustmotion-components/tests/audit_ws_i.rs new file mode 100644 index 00000000..46a94cbd --- /dev/null +++ b/crates/rustmotion-components/tests/audit_ws_i.rs @@ -0,0 +1,891 @@ +//! 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 +//! `0.0` for a relative unit (`rem`/`vw`/`vh`) instead of resolving it — +//! collapsing `marquee`/`list`/`callout`/`tooltip`/`pill_nav` to a 0px box. +//! - `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. + +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"); + ChildComponent { + component, + position: Some(PositionMode::Absolute { x: 0.0, y: 0.0 }), + x: None, + y: None, + z_index: None, + bleed: false, + } +} + +// ─── relative font-size on box_builder's default-size branches ──────────── + +#[test] +fn marquee_with_relative_font_size_gets_a_positive_height() { + // Reproduction: `font_size_px_or` returns 0.0 for any relative unit + // (`.px()` can't resolve `%`/`em`/`rem`/`vw`/`vh`). Five sites in + // `apply_intrinsic_overrides` still called it — Marquee among them — + // so a marquee declaring `"font-size": "2rem"` and no explicit height + // used to collapse `apply_default_size(css, 800.0, 0.0)` to a 0px-tall + // box, and `paint_pass`'s `height <= 0.0` guard then skipped it + // entirely: the marquee never appeared on screen. + let child = single_child_scene(serde_json::json!({ + "type": "marquee", + "content": "BREAKING NEWS", + "style": { "font-size": "2rem" } + })); + let children = vec![child]; + + let built = build_scene_with_anim( + &children, + (1920.0, 1080.0), + BuildAnimationCtx { + time: 0.0, + scenario_time: 0.0, + scene_duration: 1.0, + fps: 30, + }, + ); + let layout = run_layout(&built.root, (1920.0, 1080.0), &ConversionContext::default()); + let marquee_id = built.root.children[0].id; + let l = layout.get(marquee_id).expect("marquee laid out"); + assert!( + l.height > 0.0, + "marquee at font-size: 2rem must lay out with a positive height, got {}", + 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, + child_layout_height: f32, +} + +fn paint_card_with_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 child_id = built.root.children[0].children[0].id; + let child_layout_height = layout.get(child_id).expect("child 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, + child_layout_height, + } +} + +fn count_dominant(pixels: &[u8], dominant: usize, muted: &[usize]) -> usize { + pixels + .as_chunks::<4>() + .0 + .iter() + .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_child(serde_json::json!({ + "type": "card", + "style": { + "color": "#ff0000", + "font-size": 200, + "width": 500, + "flex-direction": "column" + }, + "children": [ + { "type": "text", "content": "WWWW" } + ] + })); + + assert!( + scene.child_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.child_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_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" + ); +} + +fn any_ink_above_black_below_y(pixels: &[u8], width: i32, y_threshold: i32) -> bool { + pixels.as_chunks::<4>().0.iter().enumerate().any(|(i, p)| { + let y = i as i32 / width; + y > y_threshold && (p[0] > 20 || p[1] > 20 || p[2] > 20) + }) +} + +#[test] +fn card_font_size_cascades_to_measured_and_painted_gradient_text_child() { + // `gradient_text` was orphaned by the original Text-only special case in + // `LegacyPaintDispatcher::dispatch` — nobody owned this file, so the + // cascade fix for `text` never extended to it even though its painter + // and `GradientTextIntrinsic` read `font-size`/`font-family`/etc. off + // `self.style` exactly the same way. `gradient_text` paints its own + // `colors` gradient rather than `style.color`, so `font-size` (which + // affects both the measured box and the ink's vertical extent) is the + // observable half of the cascade here, not color. + let scene = paint_card_with_child(serde_json::json!({ + "type": "card", + "style": { + "font-size": 200, + "width": 500, + "flex-direction": "column" + }, + "children": [ + { "type": "gradient_text", "content": "W" } + ] + })); + + assert!( + scene.child_layout_height > 150.0, + "gradient_text child with no font-size of its own must be measured \ + at the card's cascaded 200px, not the 48px default — got layout \ + height {}", + scene.child_layout_height + ); + assert!( + any_ink_above_black_below_y(&scene.pixels, CASCADE_SCENE_W, 100), + "gradient_text painted at the card's cascaded 200px must have ink \ + reaching well past y=100 — a 48px default line would not" + ); +} + +#[test] +fn card_color_cascades_to_painted_caption_child() { + // Same orphaned-file gap as gradient_text, for `caption`: its inactive + // words paint in `style.color_str_or("#FFFFFF")`, which never saw the + // card's cascaded color either. + let scene = paint_card_with_child(serde_json::json!({ + "type": "card", + "style": { + "color": "#ff0000", + "width": 500, + "flex-direction": "column" + }, + "children": [ + { + "type": "caption", + "words": [{ "text": "HELLO", "start": 10.0, "end": 20.0 }] + } + ] + })); + + let red_pixels = count_dominant(&scene.pixels, 0, &[1, 2]); + assert!( + red_pixels > 20, + "caption's inactive word (painted at time=0, outside its \ + [10,20) window) must use the card's cascaded red, not the \ + painter's white fallback — found {red_pixels} red-dominant pixels" + ); +} + +#[test] +fn card_color_cascades_to_painted_rich_text_child() { + // Same gap for `rich_text`: `resolve_span_fonts`'s `default_color` + // (used by any span that doesn't set its own `color`) reads + // `style.color_str_or("#FFFFFF")` on the component's own un-cascaded + // style. + let scene = paint_card_with_child(serde_json::json!({ + "type": "card", + "style": { + "color": "#ff0000", + "width": 500, + "flex-direction": "column" + }, + "children": [ + { "type": "rich_text", "spans": [{ "text": "WWWW" }] } + ] + })); + + let red_pixels = count_dominant(&scene.pixels, 0, &[1, 2]); + assert!( + red_pixels > 20, + "rich_text span 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 cascaded_components_match_the_verified_set() { + // Pins the current membership of `Component::with_cascaded_style`'s + // exhaustive classifier as an executable fact, not just a convention. + // The match itself guarantees every *future* variant gets a decision — + // adding a `Component` variant without extending that match is a + // compile error, the same completeness `as_painter`/`as_animatable`/ + // `as_timed`/`as_styled` already enforce for their own questions. That + // guarantee says nothing about an *existing* variant silently drifting + // (a copy-paste that drops one into the wrong arm, or a "cleanup" that + // moves one back) — this test is what catches that, by covering all + // sixty variants directly rather than a sample. + // + // The eighteen `true` cases (beyond `text`, already fixed by a prior + // pass) were found by reading every `impl Painter for X` in this crate + // for a direct `self.style.color`/`font_family`/`font_size`/ + // `font_weight`/`font_style` read with no cascade in between — not by + // trusting the component's name. `stat` was checked and ruled out: it + // reads only `self.style.background_color_str` (not inheritable) — its + // text colours come from its own dedicated `value_color`/`label_color` + // fields. + use rustmotion_components::Component; + use rustmotion_core::css::style::{Color, CssStyle as CoreCssStyle}; + use rustmotion_core::css::Length; + + let resolved = CoreCssStyle { + color: Some(Color::String("#ff0000".into())), + font_size: Some(Length::Px(40.0)), + ..Default::default() + }; + + let cases: &[(&str, bool, serde_json::Value)] = &[ + ( + "text", + true, + serde_json::json!({"type":"text","content":"x"}), + ), + ( + "gradient_text", + true, + serde_json::json!({"type":"gradient_text","content":"x"}), + ), + ( + "caption", + true, + serde_json::json!({"type":"caption","words":[{"text":"x","start":0.0,"end":0.0}]}), + ), + ( + "rich_text", + true, + serde_json::json!({"type":"rich_text","spans":[{"text":"x"}]}), + ), + ( + "badge", + true, + serde_json::json!({"type":"badge","text":"x"}), + ), + ( + "callout", + true, + serde_json::json!({"type":"callout","text":"x"}), + ), + ( + "counter", + true, + serde_json::json!({"type":"counter","from":0.0,"to":1.0}), + ), + ("divider", true, serde_json::json!({"type":"divider"})), + ( + "icon", + true, + serde_json::json!({"type":"icon","icon":"lucide:home"}), + ), + ("kbd", true, serde_json::json!({"type":"kbd","key":"A"})), + ( + "list", + true, + serde_json::json!({"type":"list","items":[{"text":"x"}]}), + ), + ( + "marquee", + true, + serde_json::json!({"type":"marquee","content":"x"}), + ), + ( + "notification", + true, + serde_json::json!({"type":"notification","title":"x"}), + ), + ( + "number_wheel", + true, + serde_json::json!({"type":"number_wheel","value":"1"}), + ), + ( + "pill_nav", + true, + serde_json::json!({"type":"pill_nav","items":["a"]}), + ), + ( + "table", + true, + serde_json::json!({"type":"table","headers":["A"],"rows":[]}), + ), + ( + "terminal", + true, + serde_json::json!({"type":"terminal","lines":[]}), + ), + ( + "tooltip", + true, + serde_json::json!({"type":"tooltip","text":"x"}), + ), + ( + "audio_spectrum", + false, + serde_json::json!({"type":"audio_spectrum"}), + ), + ( + "shape", + false, + serde_json::json!({"type":"shape","shape":"rect"}), + ), + ( + "image", + false, + serde_json::json!({"type":"image","src":"x.png"}), + ), + ("svg", false, serde_json::json!({"type":"svg"})), + ( + "video", + false, + serde_json::json!({"type":"video","src":"x.mp4"}), + ), + ( + "gif", + false, + serde_json::json!({"type":"gif","src":"x.gif"}), + ), + ("cursor", false, serde_json::json!({"type":"cursor"})), + ( + "codeblock", + false, + serde_json::json!({"type":"codeblock","code":"x"}), + ), + ( + "connector", + false, + serde_json::json!({"type":"connector","from":{"x":0.0,"y":0.0},"to":{"x":1.0,"y":1.0}}), + ), + ( + "avatar", + false, + serde_json::json!({"type":"avatar","src":"x.png"}), + ), + ( + "avatar_group", + false, + serde_json::json!({"type":"avatar_group","avatars":[]}), + ), + ( + "arrow", + false, + serde_json::json!({"type":"arrow","x2":10.0,"y2":10.0}), + ), + ( + "chart", + false, + serde_json::json!({"type":"chart","chart_type":"bar"}), + ), + ( + "comparison", + false, + serde_json::json!({"type":"comparison"}), + ), + ("countdown", false, serde_json::json!({"type":"countdown"})), + ( + "dot_map", + false, + serde_json::json!({"type":"dot_map","points":[]}), + ), + ("gauge", false, serde_json::json!({"type":"gauge"})), + ( + "heatmap", + false, + serde_json::json!({"type":"heatmap","data":[]}), + ), + ( + "line", + false, + serde_json::json!({"type":"line","x2":10.0,"y2":10.0}), + ), + ("lottie", false, serde_json::json!({"type":"lottie"})), + ( + "mockup", + false, + serde_json::json!({"type":"mockup","device":"iphone","src":"x.png"}), + ), + ( + "particle", + false, + serde_json::json!({"type":"particle","particle_type":"confetti"}), + ), + ("progress", false, serde_json::json!({"type":"progress"})), + ( + "qr_code", + false, + serde_json::json!({"type":"qr_code","content":"x"}), + ), + ( + "success_check", + false, + serde_json::json!({"type":"success_check"}), + ), + ("pointer", false, serde_json::json!({"type":"pointer"})), + ("rating", false, serde_json::json!({"type":"rating"})), + ("skeleton", false, serde_json::json!({"type":"skeleton"})), + ("slider", false, serde_json::json!({"type":"slider"})), + ( + "sparkline", + false, + serde_json::json!({"type":"sparkline","data":[]}), + ), + ( + "stat", + false, + serde_json::json!({"type":"stat","value":"1"}), + ), + ( + "stepper", + false, + serde_json::json!({"type":"stepper","steps":[]}), + ), + ("switch", false, serde_json::json!({"type":"switch"})), + ( + "tag_cloud", + false, + serde_json::json!({"type":"tag_cloud","tags":[]}), + ), + ( + "timeline", + false, + serde_json::json!({"type":"timeline","steps":[]}), + ), + ( + "treemap", + false, + serde_json::json!({"type":"treemap","data":[]}), + ), + ( + "positioned", + false, + serde_json::json!({"type":"positioned"}), + ), + ("flex", false, serde_json::json!({"type":"flex"})), + ("grid", false, serde_json::json!({"type":"grid"})), + ("card", false, serde_json::json!({"type":"card"})), + ("div", false, serde_json::json!({"type":"div"})), + ("waveform", false, serde_json::json!({"type":"waveform"})), + ]; + + assert_eq!( + cases.len(), + 60, + "this table must cover every Component variant (currently 60) — the \ + compiler enforces that with_cascaded_style itself classifies every \ + variant, but only this list enforces that the classification stays \ + the one this workstream verified" + ); + + for (name, expected_typographic, json) in cases { + let component: Component = + serde_json::from_value(json.clone()).unwrap_or_else(|e| panic!("{name}: {e}")); + let got = component.with_cascaded_style(&resolved).is_some(); + assert_eq!( + got, *expected_typographic, + "{name}: with_cascaded_style returned is_some()={got}, expected \ + {expected_typographic}" + ); + } +} + +// ─── 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}" + ); +} + +// ─── the cascade reaches every typographic component, not just `text` ───── +// +// `card_font_size_cascades_to_measured_and_painted_gradient_text_child`, +// `card_color_cascades_to_painted_caption_child` and +// `card_color_cascades_to_painted_rich_text_child` above cover the three +// components the brief named directly. The tests below cover the "own value +// still wins" half for `caption`, plus every other component this sweep +// found reading un-cascaded typography off its own `style`. + +#[test] +fn caption_own_color_wins_over_cascaded_card_color_at_paint_time() { + let scene = paint_card_with_child(serde_json::json!({ + "type": "card", + "style": { "color": "#ff0000", "width": 500, "flex-direction": "column" }, + "children": [ + { + "type": "caption", + "words": [{ "text": "WWWW", "start": 0.0, "end": 0.0 }], + "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, + "caption's own explicit color must still be painted — found \ + {green_pixels} green-dominant pixels" + ); + assert_eq!( + red_pixels, 0, + "caption's own explicit color must win over the card's cascaded red \ + — found {red_pixels} red-dominant pixels" + ); +} + +// ─── the fourteen other components that read `self.style` typography ────── +// +// Found by sweeping every `impl Painter for X` in this crate for a direct +// `self.style.color`/`font_family`/`font_size`/`font_weight`/`font_style` +// read with no cascade in between (the same shape as the three tested +// above), not by re-checking only the candidates the brief named. `stat` +// was checked and ruled out: it reads only `self.style.background_color_str` +// (not inheritable) — its text colours come from its own dedicated +// `value_color`/`label_color` fields. + +#[test] +fn newly_cascaded_components_inherit_unset_typography_from_the_parent() { + use rustmotion_components::Component; + use rustmotion_core::css::style::{Color, CssStyle as CoreCssStyle}; + use rustmotion_core::css::Length; + + let parent = CoreCssStyle { + color: Some(Color::String("#ff0000".into())), + font_size: Some(Length::Px(200.0)), + ..Default::default() + }; + + let cases: &[(&str, serde_json::Value)] = &[ + ("badge", serde_json::json!({"type":"badge","text":"x"})), + ("callout", serde_json::json!({"type":"callout","text":"x"})), + ( + "counter", + serde_json::json!({"type":"counter","from":0.0,"to":1.0}), + ), + ("divider", serde_json::json!({"type":"divider"})), + ( + "icon", + serde_json::json!({"type":"icon","icon":"lucide:home"}), + ), + ("kbd", serde_json::json!({"type":"kbd","key":"A"})), + ( + "list", + serde_json::json!({"type":"list","items":[{"text":"x"}]}), + ), + ( + "marquee", + serde_json::json!({"type":"marquee","content":"x"}), + ), + ( + "notification", + serde_json::json!({"type":"notification","title":"x"}), + ), + ( + "number_wheel", + serde_json::json!({"type":"number_wheel","value":"1"}), + ), + ( + "pill_nav", + serde_json::json!({"type":"pill_nav","items":["a"]}), + ), + ( + "table", + serde_json::json!({"type":"table","headers":["A"],"rows":[]}), + ), + ( + "terminal", + serde_json::json!({"type":"terminal","lines":[]}), + ), + ("tooltip", serde_json::json!({"type":"tooltip","text":"x"})), + ]; + + for (name, json) in cases { + let component: Component = + serde_json::from_value(json.clone()).unwrap_or_else(|e| panic!("{name}: {e}")); + let cascaded = component + .with_cascaded_style(&parent) + .unwrap_or_else(|| panic!("{name} must be classified as typographic")); + let style = cascaded.as_styled().style_config(); + assert_eq!( + style.color, parent.color, + "{name} with no color of its own must inherit the parent's" + ); + assert_eq!( + style.font_size, parent.font_size, + "{name} with no font-size of its own must inherit the parent's" + ); + } +} + +#[test] +fn table_own_color_wins_over_cascaded_card_color() { + use rustmotion_components::Component; + use rustmotion_core::css::style::{Color, CssStyle as CoreCssStyle}; + + let parent = CoreCssStyle { + color: Some(Color::String("#ff0000".into())), + ..Default::default() + }; + let component: Component = serde_json::from_value(serde_json::json!({ + "type": "table", + "headers": ["A"], + "rows": [], + "style": { "color": "#00ff00" } + })) + .expect("deserialize table"); + let cascaded = component + .with_cascaded_style(&parent) + .expect("table is typographic"); + assert_eq!( + cascaded.as_styled().style_config().color, + Some(Color::String("#00ff00".into())), + "table's own explicit color must win over the cascaded parent's" + ); +} + +#[test] +fn divider_color_cascades_from_card_to_painted_line() { + // `Divider::paint_content` reads `self.style.color_str_or("#FFFFFF")` + // directly for the line it draws — the same un-cascaded read as the + // text-bearing components above, just for a component with no text at + // all. A card's `color` is the CSS analogue of `border-color: + // currentColor`: a divider with no colour of its own should match the + // ambient text colour, not fall back to white. + let scene = paint_card_with_child(serde_json::json!({ + "type": "card", + "style": { + "color": "#ff0000", + "width": 500, + "height": 200, + "flex-direction": "column" + }, + "children": [ { "type": "divider" } ] + })); + + let red_pixels = count_dominant(&scene.pixels, 0, &[1, 2]); + assert!( + red_pixels > 0, + "divider with no color of its own must paint its line in the card's \ + cascaded red, not the painter's white fallback — found \ + {red_pixels} red-dominant pixels" + ); +} + +#[test] +fn table_color_and_font_size_cascade_to_painted_cells() { + // Complements `table_columns_are_sized_by_content_not_split_evenly` + // above (painter vs. intrinsic column widths) with the cascade half: + // `Table::paint`'s body-cell colour (`self.style.color_str_or`) and + // font-size both used to ignore the card's cascaded style. The header + // row keeps its own hardcoded `header_text_color` default regardless, + // so this only proves the body cells. + let scene = paint_card_with_child(serde_json::json!({ + "type": "card", + "style": { + "color": "#ff0000", + "font-size": 40, + "width": 500, + "flex-direction": "column" + }, + "children": [ + { "type": "table", "headers": ["ID"], "rows": [["1"]] } + ] + })); + + let red_pixels = count_dominant(&scene.pixels, 0, &[1, 2]); + assert!( + red_pixels > 20, + "a table body cell with no color of its own must paint in the \ + card's cascaded red — found {red_pixels} red-dominant pixels" + ); +} diff --git a/crates/rustmotion-components/tests/caption_presets.rs b/crates/rustmotion-components/tests/caption_presets.rs index 2f074ae8..59ecf7b9 100644 --- a/crates/rustmotion-components/tests/caption_presets.rs +++ b/crates/rustmotion-components/tests/caption_presets.rs @@ -82,7 +82,11 @@ fn render_caption_at(json: serde_json::Value, time: f64, y: f32) -> Vec { } fn count_pixels(buf: &[u8], pred: impl Fn(&[u8]) -> bool) -> usize { - buf.chunks_exact(4).filter(|p| pred(p)).count() + buf.as_chunks::<4>() + .0 + .iter() + .filter(|p| pred(p.as_slice())) + .count() } fn lit_pixels(buf: &[u8]) -> usize { diff --git a/crates/rustmotion-components/tests/codeblock_auto_scroll.rs b/crates/rustmotion-components/tests/codeblock_auto_scroll.rs index 40a4f2eb..e1668a31 100644 --- a/crates/rustmotion-components/tests/codeblock_auto_scroll.rs +++ b/crates/rustmotion-components/tests/codeblock_auto_scroll.rs @@ -42,7 +42,9 @@ fn text_ink_pixels(buf: &[u8]) -> usize { // Background is #2b303b ~ (43, 48, 59). Count pixels that deviate from // that by a wide margin in any channel — syntect's theme colors are all // much brighter than the near-black background. - buf.chunks_exact(4) + buf.as_chunks::<4>() + .0 + .iter() .filter(|p| { let (r, g, b, a) = (p[0] as i32, p[1] as i32, p[2] as i32, p[3] as i32); a > 200 && ((r - 43).abs() > 40 || (g - 48).abs() > 40 || (b - 59).abs() > 40) diff --git a/crates/rustmotion-components/tests/relative_font_size.rs b/crates/rustmotion-components/tests/relative_font_size.rs index 21965697..c40e8fe0 100644 --- a/crates/rustmotion-components/tests/relative_font_size.rs +++ b/crates/rustmotion-components/tests/relative_font_size.rs @@ -77,7 +77,7 @@ fn render(json: serde_json::Value) -> Vec { } fn lit_pixels(buf: &[u8]) -> usize { - buf.chunks_exact(4).filter(|p| p[3] > 0).count() + buf.as_chunks::<4>().0.iter().filter(|p| p[3] > 0).count() } #[test] diff --git a/crates/rustmotion-core/src/css/taffy_bridge.rs b/crates/rustmotion-core/src/css/taffy_bridge.rs index 9376ebb7..7ee1fbce 100644 --- a/crates/rustmotion-core/src/css/taffy_bridge.rs +++ b/crates/rustmotion-core/src/css/taffy_bridge.rs @@ -20,6 +20,28 @@ pub struct ConversionContext { pub length: LengthContext, } +impl ConversionContext { + /// Anchor `vw`/`vh` resolution to a real output viewport. + /// + /// [`ConversionContext::default()`] carries a 1920×1080 viewport, which is + /// only ever correct by coincidence. Any caller that knows the video's real + /// dimensions must build its context here instead: on a 1080×1920 vertical + /// render, the default resolves `50vw` to 960px where the truth is 540px, + /// and misses `vh` by the same margin in the other axis. + /// + /// `font-size` / `root-font-size` stay at the CSS initial 16px: nothing + /// upstream resolves and threads a root font-size through yet. + pub fn for_viewport(viewport_width: f32, viewport_height: f32) -> Self { + Self { + length: LengthContext { + viewport_width, + viewport_height, + ..LengthContext::default() + }, + } + } +} + /// Convert a [`CssStyle`] into a [`taffy::Style`]. Properties not relevant to /// layout are ignored. Unsupported / unset properties fall back to taffy /// defaults (which match CSS initial values). @@ -216,6 +238,67 @@ pub fn to_taffy_style(css: &CssStyle, ctx: &ConversionContext) -> tf::Style { style } +/// Resolve `padding` + `border` width into a single content-box inset, in +/// px, per axis: `(horizontal, vertical)` i.e. `(left + right, top + +/// bottom)`. +/// +/// Mirrors what taffy 0.10.1's own `compute_leaf_layout` (`content_box_inset +/// = padding + border`) subtracts from a leaf's `available_space` before +/// handing it to the measure function — see [`crate::engine::layout_pass`], +/// which uses this to bring `known_dimensions` (still border-box) into that +/// same content-box space (RM-27). Percentage padding/border resolves +/// against `ctx.length.parent_size` like every other percentage in this +/// module; a leaf's own known/available width is not threaded here, so a +/// percentage inset on a leaf is only as accurate as that shared context. +pub(crate) fn content_box_inset(css: &CssStyle, ctx: &ConversionContext) -> (f32, f32) { + let (pt, pr, pb, pl) = css.padding.as_ref().map(Edges::resolve).unwrap_or_default(); + let padding = ( + pt.resolve(&ctx.length), + pr.resolve(&ctx.length), + pb.resolve(&ctx.length), + pl.resolve(&ctx.length), + ); + let border = resolve_border_widths_px(css.border.as_ref(), ctx); + ( + padding.1 + padding.3 + border.1 + border.3, + padding.0 + padding.2 + border.0 + border.2, + ) +} + +fn resolve_border_widths_px( + b: Option<&super::style::BorderEdges>, + ctx: &ConversionContext, +) -> (f32, f32, f32, f32) { + let Some(b) = b else { + return (0.0, 0.0, 0.0, 0.0); + }; + let uniform = b.width.as_ref().map(Edges::resolve); + let pick_side = |side: Option<&super::style::BorderSide>, idx: usize| -> f32 { + if let Some(side) = side { + if let Some(w) = side.width.as_ref() { + return w.resolve(&ctx.length); + } + } + if let Some((t, r, btm, l)) = uniform.as_ref() { + let pick = match idx { + 0 => t, + 1 => r, + 2 => btm, + 3 => l, + _ => t, + }; + return pick.resolve(&ctx.length); + } + 0.0 + }; + ( + pick_side(b.top.as_ref(), 0), + pick_side(b.right.as_ref(), 1), + pick_side(b.bottom.as_ref(), 2), + pick_side(b.left.as_ref(), 3), + ) +} + fn align_items_to_taffy(a: AlignItems) -> tf::AlignItems { match a { AlignItems::Stretch => tf::AlignItems::Stretch, diff --git a/crates/rustmotion-core/src/engine/animator.rs b/crates/rustmotion-core/src/engine/animator.rs index 0cac4401..ba343334 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 @@ -500,8 +533,7 @@ fn spring_value_raw(t: f64, damping: f64, stiffness: f64, mass: f64) -> f64 { // Underdamped let omega_d = omega * (1.0 - zeta * zeta).sqrt(); let decay = (-zeta * omega * t).exp(); - 1.0 - decay - * ((zeta * omega * t / omega_d).sin() * (zeta * omega / omega_d) + (omega_d * t).cos()) + 1.0 - decay * ((omega_d * t).sin() * (zeta * omega / omega_d) + (omega_d * t).cos()) } else if (zeta - 1.0).abs() < 1e-6 { // Critically damped let decay = (-omega * t).exp(); @@ -624,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) } } } @@ -2838,19 +2864,22 @@ mod spring_duration_tests { // // damping=6, stiffness=120, mass=1 (the same "underdamped" preset // this file already uses for elastic_in / kf_anim_spring_underdamped) - // at t=0.8s: spring_value_raw(0.8, 6, 120, 1) ~= 1.043467 — 4.35% - // past the target, an order of magnitude outside any reasonable - // rest_threshold (default 0.5%). An author asking this spring to - // "finish at 0.8s" got a value nowhere near rest. + // at t=0.8s: spring_value_raw(0.8, 6, 120, 1) ~= 1.027616 — 2.76% + // past the target, well outside any reasonable rest_threshold + // (default 0.5%). An author asking this spring to "finish at 0.8s" + // got a value nowhere near rest. Reference recomputed for RM-09 + // (issue #220): the solver's underdamped branch fed the wrong + // argument to its sine term, so this captured value moved when that + // was corrected. let v = spring_value_raw(0.8, 6.0, 120.0, 1.0); assert!( - (v - 1.043467).abs() < 1e-5, - "captured red-phase reference value drifted: got {v}, expected ~1.043467" + (v - 1.027616).abs() < 1e-5, + "captured red-phase reference value drifted: got {v}, expected ~1.027616" ); assert!( - (v - 1.0).abs() > 0.04, + (v - 1.0).abs() > 0.02, "red-phase claim: at t=duration the unscaled spring must still be far from rest \ - (got diff {:.6}, expected > 0.04)", + (got diff {:.6}, expected > 0.02)", (v - 1.0).abs() ); } @@ -2867,7 +2896,7 @@ mod spring_duration_tests { let threshold = DEFAULT_SPRING_REST_THRESHOLD; // Green phase: the same (damping, stiffness, mass) that the - // red-phase test above showed is 4.35% off at t=0.8s without a + // red-phase test above showed is 2.76% off at t=0.8s without a // `duration` must now be within `threshold` of rest at t=0.8s. let v_at_duration = spring_value(0.8, &config); assert!( diff --git a/crates/rustmotion-core/src/engine/layout_pass.rs b/crates/rustmotion-core/src/engine/layout_pass.rs index 026a16a4..e9a03c5e 100644 --- a/crates/rustmotion-core/src/engine/layout_pass.rs +++ b/crates/rustmotion-core/src/engine/layout_pass.rs @@ -5,7 +5,8 @@ use std::collections::HashMap; use taffy::prelude as tf; use taffy::TaffyTree; -use crate::css::taffy_bridge::{to_taffy_style, ConversionContext}; +use crate::css::taffy_bridge::{content_box_inset, to_taffy_style, ConversionContext}; +use crate::css::units::LengthContext; use crate::engine::box_tree::{BoxNode, IntrinsicMeasure, NodeId}; /// Resolved geometry for a single node, in absolute viewport coordinates. @@ -69,10 +70,20 @@ impl LayoutResult { /// Per-node user data stored in the taffy tree to keep the link between /// taffy nodes and our `BoxNode` ids + intrinsic measurers. +/// +/// `inset_width`/`inset_height` are this node's own resolved padding+border +/// (RM-27): taffy's `compute_leaf_layout` already subtracts them from +/// `available_space` before calling the measure function, but forwards +/// `known_dimensions` — the outer border-box size — untouched, handing an +/// `IntrinsicMeasure` implementor two arguments in different coordinate +/// spaces. The measure closure below subtracts the same inset from `known` +/// so both arguments describe the content box. struct NodeData { #[allow(dead_code)] box_id: NodeId, intrinsic: Option>, + inset_width: f32, + inset_height: f32, } /// Run taffy on a [`BoxNode`] tree and return the resolved layouts. @@ -85,7 +96,7 @@ pub fn run_layout(root: &BoxNode, viewport: (f32, f32), ctx: &ConversionContext) let mut node_map: HashMap = HashMap::new(); // Build the taffy tree top-down. - let root_tf = build(&mut tree, &mut node_map, root, ctx); + let root_tf = build(&mut tree, &mut node_map, root, ctx, ctx.length.font_size); let viewport_size = tf::Size { width: tf::AvailableSpace::Definite(viewport.0), @@ -101,8 +112,12 @@ pub fn run_layout(root: &BoxNode, viewport: (f32, f32), ctx: &ConversionContext) let Some(intr) = ctx.intrinsic.as_ref() else { return tf::Size::ZERO; }; + let content_known = ( + known.width.map(|w| (w - ctx.inset_width).max(0.0)), + known.height.map(|h| (h - ctx.inset_height).max(0.0)), + ); let (w, h) = intr.measure( - (known.width, known.height), + content_known, (available.width.into(), available.height.into()), ); tf::Size { @@ -119,33 +134,58 @@ pub fn run_layout(root: &BoxNode, viewport: (f32, f32), ctx: &ConversionContext) LayoutResult { layouts } } +/// Build one taffy node and, recursively, its subtree. +/// +/// `inherited_font_size` is the already-resolved (px) font-size of `node`'s +/// parent (RM-26): CSS resolves `em` on every layout property against the +/// element's *own* computed font-size, and font-size itself inherits down +/// the tree unless overridden. `to_taffy_style` and [`content_box_inset`] +/// only ever see the single `ConversionContext` handed to them, so their +/// `em` resolution is only as correct as the per-node context built here — +/// a call site building one shared `ConversionContext` for the whole tree +/// (as every production caller of `to_taffy_style` still does directly) +/// resolves every node's `em` against that one context's `font_size` +/// instead. fn build( tree: &mut TaffyTree, map: &mut HashMap, node: &BoxNode, ctx: &ConversionContext, + inherited_font_size: f32, ) -> tf::NodeId { - let style = to_taffy_style(&node.css, ctx); + let parent_font_ctx = LengthContext { + font_size: inherited_font_size, + ..ctx.length + }; + let own_font_size = node + .css + .font_size_px_ctx(&parent_font_ctx, inherited_font_size); + let node_ctx = ConversionContext { + length: LengthContext { + font_size: own_font_size, + ..ctx.length + }, + }; + + let style = to_taffy_style(&node.css, &node_ctx); + let (inset_width, inset_height) = content_box_inset(&node.css, &node_ctx); let data = NodeData { box_id: node.id, intrinsic: node.intrinsic.clone(), + inset_width, + inset_height, }; - let tf_id = if node.intrinsic.is_some() { - // Leaf with intrinsic measurement. - tree.new_leaf_with_context(style, data) - .expect("taffy new_leaf") - } else if node.children.is_empty() { + let tf_id = if node.intrinsic.is_some() || node.children.is_empty() { tree.new_leaf_with_context(style, data) .expect("taffy new_leaf") } else { let mut child_ids = Vec::with_capacity(node.children.len()); for c in &node.children { - child_ids.push(build(tree, map, c, ctx)); + child_ids.push(build(tree, map, c, ctx, own_font_size)); } let id = tree .new_with_children(style, &child_ids) .expect("taffy new_with_children"); - // We still want context on internal nodes (for box_id mapping). tree.set_node_context(id, Some(data)).ok(); id }; diff --git a/crates/rustmotion-core/src/engine/paint_pass.rs b/crates/rustmotion-core/src/engine/paint_pass.rs index 496cdea0..4b93fe75 100644 --- a/crates/rustmotion-core/src/engine/paint_pass.rs +++ b/crates/rustmotion-core/src/engine/paint_pass.rs @@ -356,17 +356,28 @@ fn paint_node(canvas: &Canvas, node: &BoxNode, ctx: &PaintContext, tree_depth: u } } + // Hoisted from step 8 below: the layer-bounds computation right after + // this needs it too, to decide whether descendant ink painted outside + // the border-box (legitimate under `overflow: visible`) must stay + // reachable by the opacity/filter layer opened next. + let overflow = node.css.overflow.unwrap_or(Overflow::Visible); + // 4. opacity / filter layer — one shared layer carries both the group // alpha and the CSS `filter` chain (applies to the node and its - // subtree). Bounded to the node's own box (padded by the filter chain's - // blur/drop-shadow bleed so those still bleed past the edge, unclipped): - // an unbounded `SaveLayerRec` sizes the layer against the current clip — - // usually the whole viewport — so every faded/filtered node allocates - // and composites a full-frame layer regardless of how small it is - // (measured on this repo's release binary, 1080x1920/60 frames, 30 small - // `opacity: 0.5` shapes, `--threads 1`: ~42-60s wall time unbounded vs. - // ~0.5s bounded — roughly two orders of magnitude, not a rounding - // error; cost scales with viewport area, not node size). + // subtree). Bounded to the node's own box, padded by: the filter + // chain's blur/drop-shadow bleed, this node's own outset box-shadow + // extent (painted inside this same layer at step 5, outside the + // border-box), and — when `overflow` leaves descendant ink free to + // paint past the border-box — the union of the whole subtree's layout + // boxes. An unbounded `SaveLayerRec` sizes the layer against the + // current clip — usually the whole viewport — so every faded/filtered + // node allocates and composites a full-frame layer regardless of how + // small it is (measured on this repo's release binary, 1080x1920/60 + // frames, 30 small `opacity: 0.5` shapes, `--threads 1`: ~42-60s wall + // time unbounded vs. ~0.5s bounded — roughly two orders of magnitude, + // not a rounding error; cost scales with viewport area, not node + // size), so the bound stays tight to the content that can actually + // paint rather than falling back to the viewport. let opacity = node.css.opacity.unwrap_or(1.0).clamp(0.0, 1.0); let content_filter = node .css @@ -381,18 +392,30 @@ fn paint_node(canvas: &Canvas, node: &BoxNode, ctx: &PaintContext, tree_depth: u if let Some(filter) = content_filter { paint.set_image_filter(filter); } - let bleed = node + let filter_bleed_px = node .css .filter .as_deref() .map(|list| filter_bleed(list, &length_ctx)) .unwrap_or(0.0); - let bounds = Rect::from_xywh( + let shadow_bleed_px = node + .css + .box_shadow + .as_deref() + .map(|shadows| box_shadow_bleed(shadows, &length_ctx)) + .unwrap_or(0.0); + let bleed = filter_bleed_px.max(shadow_bleed_px); + let mut bounds = Rect::from_xywh( box_layout.x - bleed, box_layout.y - bleed, box_layout.width + bleed * 2.0, box_layout.height + bleed * 2.0, ); + if overflow == Overflow::Visible { + if let Some(descendants) = subtree_layout_bounds(node, ctx.layout) { + bounds = Rect::join2(bounds, descendants); + } + } let rec = SaveLayerRec::default().paint(&paint).bounds(&bounds); canvas.save_layer(&rec); true @@ -448,8 +471,8 @@ fn paint_node(canvas: &Canvas, node: &BoxNode, ctx: &PaintContext, tree_depth: u // 8. clip overflow:hidden / clip — scoped to this node's own content and // its children only (see step 5-7's comment for why the box's own - // decorations must stay outside this clip). - let overflow = node.css.overflow.unwrap_or(Overflow::Visible); + // decorations must stay outside this clip). `overflow` was hoisted + // above step 4. let opened_overflow_clip = if matches!( overflow, Overflow::Hidden | Overflow::Clip | Overflow::Scroll | Overflow::Auto @@ -632,6 +655,65 @@ fn filter_bleed(list: &[crate::css::style::FilterFn], ctx: &LengthContext) -> f3 bleed } +/// Conservative outward bleed (px) a node's own outset `box_shadow` list +/// paints beyond its border-box — the same role `filter_bleed` plays for +/// `filter`, and sized the same way (offset + spread pushes the shadow rect +/// out, `1.5x` blur radius covers the Gaussian falloff). Inset shadows are +/// clipped to the padding-box by `paint_box_shadow` and never bleed outward, +/// so they are skipped here. +fn box_shadow_bleed(shadows: &[BoxShadow], ctx: &LengthContext) -> f32 { + let mut bleed = 0.0f32; + for shadow in shadows { + if shadow.inset.unwrap_or(false) { + continue; + } + let offset = shadow + .offset_x + .resolve(ctx) + .abs() + .max(shadow.offset_y.resolve(ctx).abs()); + let spread = shadow + .spread + .as_ref() + .map(|s| s.resolve(ctx).max(0.0)) + .unwrap_or(0.0); + let blur_bleed = shadow + .blur + .as_ref() + .map(|b| b.resolve(ctx).max(0.0) * 1.5) + .unwrap_or(0.0); + bleed = bleed.max(offset + spread + blur_bleed); + } + bleed +} + +/// Bounding box (viewport coordinates) of every descendant's own layout box, +/// recursively — the same "leave the layer big enough to hold what can +/// legitimately paint outside the border-box" contract as `filter_bleed`, +/// applied to `overflow: visible` subtrees instead of a filter chain. Each +/// descendant contributes only its plain layout rect (not its own +/// filter/shadow bleed or transform): a tight bound for the common cases — +/// absolutely-positioned children, `marquee`, a taller-than-parent flow — +/// without walking the whole subtree's CSS. +fn subtree_layout_bounds(node: &BoxNode, layout: &LayoutResult) -> Option { + let mut bounds: Option = None; + for child in &node.children { + if let Some(child_layout) = layout.get(child.id) { + let rect = Rect::from_xywh( + child_layout.x, + child_layout.y, + child_layout.width, + child_layout.height, + ); + bounds = Some(bounds.map_or(rect, |b| Rect::join2(b, rect))); + } + if let Some(child_bounds) = subtree_layout_bounds(child, layout) { + bounds = Some(bounds.map_or(child_bounds, |b| Rect::join2(b, child_bounds))); + } + } + bounds +} + // ---- CSS filters ---- /// Build a Skia `ImageFilter` chain from a CSS `filter`/`backdrop-filter` @@ -1255,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 fdd65d9f..97345599 100644 --- a/crates/rustmotion-core/src/engine/renderer/assets.rs +++ b/crates/rustmotion-core/src/engine/renderer/assets.rs @@ -1,5 +1,6 @@ use std::path::{Path, PathBuf}; use std::sync::{Arc, OnceLock}; +use std::time::Duration; use dashmap::DashMap; @@ -35,6 +36,40 @@ pub fn gif_cache() -> &'static GifCacheMap { GIF_CACHE.get_or_init(|| Arc::new(DashMap::new())) } +// ─── Shared HTTP agent ────────────────────────────────────────────────────── + +/// The [`ureq::Agent`] every outbound HTTP call in this crate must go +/// through — the icon fetch below, and the remote `include` fetch in the +/// `rustmotion` crate (`crates/rustmotion/src/include.rs`), which imports +/// [`http_agent`] rather than building its own. +/// +/// `ureq::get(...)`, the free function used before this fix, always resolves +/// to an *unconfigured* default agent. In ureq 3.x every field of +/// `Timeouts` defaults to `None` except `await_100` (`config.rs`'s `impl +/// Default for Timeouts`), so a host that accepts the TCP connection and +/// then never answers — or trickles one byte a minute — hangs the calling +/// thread forever; ureq's 10 MB body cap bounds bytes, not time. On the icon +/// path that thread can be a render worker with nobody at the keyboard to +/// notice (RM-41). +/// +/// `Config::builder()` starts from `Config::default()`, which already +/// resolves a proxy from `HTTPS_PROXY`/`https_proxy`/`HTTP_PROXY`/ +/// `http_proxy`/`ALL_PROXY` via `Proxy::try_from_env()` — the same audit +/// separately found every network call here ignoring a configured egress +/// proxy, and routing through the builder rather than hand-building a +/// `Config` fixes that as a side effect, not a separate change. +static HTTP_AGENT: OnceLock = OnceLock::new(); + +pub fn http_agent() -> &'static ureq::Agent { + HTTP_AGENT.get_or_init(|| { + let config = ureq::config::Config::builder() + .timeout_global(Some(Duration::from_secs(20))) + .timeout_connect(Some(Duration::from_secs(5))) + .build(); + ureq::Agent::new_with_config(config) + }) +} + // ─── Icon fetching ────────────────────────────────────────────────────────── /// How much larger than the *target* (layout) size icons are rasterized, so @@ -138,7 +173,8 @@ pub fn fetch_icon_svg_in( "https://api.iconify.design/{}/{}.svg?color=%23{}&width={}&height={}", prefix, name, hex_color, width, height ); - let response = ureq::get(&url) + let response = http_agent() + .get(&url) .call() .map_err(|e| RustmotionError::IconFetch { icon: icon.to_string(), @@ -215,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 (RM-42). 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 (RM-42)" + ))); + } + 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/engine/renderer/text.rs b/crates/rustmotion-core/src/engine/renderer/text.rs index d607dadf..6d578977 100644 --- a/crates/rustmotion-core/src/engine/renderer/text.rs +++ b/crates/rustmotion-core/src/engine/renderer/text.rs @@ -975,7 +975,7 @@ mod emoji_presentation_tests { surface.read_pixels(&info, &mut buf, (W * 4) as usize, (0, 0)); let (mut n, mut sr, mut sg, mut sb) = (0usize, 0f64, 0f64, 0f64); - for px in buf.chunks_exact(4) { + for px in buf.as_chunks::<4>().0.iter() { if px[3] > 40 { n += 1; sr += px[0] as f64; diff --git a/crates/rustmotion-core/src/engine/transition.rs b/crates/rustmotion-core/src/engine/transition.rs index b256a080..22e5923f 100644 --- a/crates/rustmotion-core/src/engine/transition.rs +++ b/crates/rustmotion-core/src/engine/transition.rs @@ -970,9 +970,9 @@ mod camera_pan_tests { ); // blend_fade(10, 200, 0.5) = (10*0.5 + 200*0.5 + 0.5) as u8 = 105. - for px in out.chunks_exact(4) { + for px in out.as_chunks::<4>().0.iter() { assert_eq!( - px, + *px, [105, 105, 105, 255], "mid-pan Static frame must be a blend of bg_a and bg_b, not a copy of either" ); diff --git a/crates/rustmotion-core/src/expand.rs b/crates/rustmotion-core/src/expand.rs index 497a7d66..c47860e1 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 (RM-43). +/// [`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 (RM-43). 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/src/variables.rs b/crates/rustmotion-core/src/variables.rs index 230a6fcc..85b0fdcb 100644 --- a/crates/rustmotion-core/src/variables.rs +++ b/crates/rustmotion-core/src/variables.rs @@ -4,10 +4,48 @@ use crate::error::Result; use serde_json::Value; use crate::error::RustmotionError; -use crate::schema::VariableDefinition; +use crate::schema::{VariableDefinition, VariableType}; + +/// Whether `value`'s JSON type matches `var_type` — a `number`-typed +/// variable's default or override must actually be a JSON number, etc. +fn value_matches_declared_type(value: &Value, var_type: &VariableType) -> bool { + match var_type { + VariableType::String => value.is_string(), + VariableType::Number => value.is_number(), + VariableType::Boolean => value.is_boolean(), + VariableType::Object => value.is_object(), + VariableType::Array => value.is_array(), + } +} + +fn json_type_name(value: &Value) -> &'static str { + match value { + Value::Null => "null", + Value::Bool(_) => "boolean", + Value::Number(_) => "number", + Value::String(_) => "string", + Value::Array(_) => "array", + Value::Object(_) => "object", + } +} + +fn declared_type_name(var_type: &VariableType) -> &'static str { + match var_type { + VariableType::String => "string", + VariableType::Number => "number", + VariableType::Boolean => "boolean", + VariableType::Object => "object", + VariableType::Array => "array", + } +} /// Build the final variable map: start from defaults, then apply overrides. -/// Returns an error if an override references a variable not in the definitions. +/// Returns an error if an override references a variable not in the +/// definitions, or if a default/override's JSON type doesn't match the +/// variable's declared `type` — the `type` field on `config` entries used to +/// be decorative (schema/scenario.rs's own `VariableDefinition::var_type` +/// was parsed and never read), so `{ "type": "number", "default": "oops" }` +/// silently accepted a string. This is the sole enforcement point. fn merge_variables( definitions: &HashMap, overrides: Option<&HashMap>, @@ -17,17 +55,33 @@ fn merge_variables( // Start with defaults for (name, def) in definitions { + if !value_matches_declared_type(&def.default, &def.var_type) { + return Err(RustmotionError::Generic(format!( + "Variable '${name}' in '{path}' is declared as type \"{declared}\" but its \ + default value is a {actual}", + declared = declared_type_name(&def.var_type), + actual = json_type_name(&def.default), + ))); + } merged.insert(name.clone(), def.default.clone()); } // Apply overrides if let Some(ovr) = overrides { for (name, value) in ovr { - if !definitions.contains_key(name) { - return Err(RustmotionError::UndefinedVariable { + let def = definitions + .get(name) + .ok_or_else(|| RustmotionError::UndefinedVariable { name: name.clone(), path: path.to_string(), - }); + })?; + if !value_matches_declared_type(value, &def.var_type) { + return Err(RustmotionError::Generic(format!( + "Variable '${name}' in '{path}' is declared as type \"{declared}\" but the \ + override value is a {actual}", + declared = declared_type_name(&def.var_type), + actual = json_type_name(value), + ))); } merged.insert(name.clone(), value.clone()); } @@ -432,6 +486,64 @@ mod tests { assert!(result.is_err()); } + #[test] + fn test_merge_variables_rejects_type_mismatched_override() { + let mut defs = HashMap::new(); + defs.insert( + "count".to_string(), + VariableDefinition { + var_type: crate::schema::VariableType::Number, + default: json!(0), + description: None, + }, + ); + let mut overrides = HashMap::new(); + overrides.insert("count".to_string(), json!("not a number")); + + let result = merge_variables(&defs, Some(&overrides), "test.json"); + assert!( + result.is_err(), + "a string override for a declared `number` variable must be rejected" + ); + } + + #[test] + fn test_merge_variables_accepts_type_matched_override() { + let mut defs = HashMap::new(); + defs.insert( + "count".to_string(), + VariableDefinition { + var_type: crate::schema::VariableType::Number, + default: json!(0), + description: None, + }, + ); + let mut overrides = HashMap::new(); + overrides.insert("count".to_string(), json!(42)); + + let result = merge_variables(&defs, Some(&overrides), "test.json").unwrap(); + assert_eq!(result["count"], json!(42)); + } + + #[test] + fn test_merge_variables_rejects_type_mismatched_default() { + let mut defs = HashMap::new(); + defs.insert( + "flag".to_string(), + VariableDefinition { + var_type: crate::schema::VariableType::Boolean, + default: json!("yes"), + description: None, + }, + ); + + let result = merge_variables(&defs, None, "test.json"); + assert!( + result.is_err(), + "a string default for a declared `boolean` variable must be rejected" + ); + } + #[test] fn test_interpolation_type_error() { let mut val = json!({ diff --git a/crates/rustmotion-core/tests/audit_ws_a.rs b/crates/rustmotion-core/tests/audit_ws_a.rs new file mode 100644 index 00000000..7ad93749 --- /dev/null +++ b/crates/rustmotion-core/tests/audit_ws_a.rs @@ -0,0 +1,282 @@ +//! Regression tests for the workstream A (animation & paint) audit findings +//! tracked in issue #220: RM-01, RM-09, RM-10, RM-37. + +use rustmotion_core::css::style::{ + 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}; +use rustmotion_core::engine::animator::spring_value; +use rustmotion_core::engine::box_tree::{BoxKind, BoxNode}; +use rustmotion_core::engine::layout_pass::run_layout; +use rustmotion_core::engine::paint_pass::{paint_tree, NoopDispatcher, PaintFrame}; +use rustmotion_core::schema::SpringConfig; + +fn test_frame(w: u32, h: u32) -> PaintFrame { + PaintFrame { + time: 0.0, + scenario_time: 0.0, + frame_index: 0, + fps: 30, + video_width: w, + video_height: h, + scene_duration: 1.0, + camera: None, + } +} + +fn render_pixels(root: &mut BoxNode, w: u32, h: u32) -> Vec { + root.assign_ids(0); + let layout = run_layout(root, (w as f32, h as f32), &ConversionContext::default()); + let mut surface = skia_safe::surfaces::raster_n32_premul((w as i32, h as i32)).unwrap(); + paint_tree( + surface.canvas(), + root, + &layout, + &test_frame(w, h), + &NoopDispatcher, + ); + let info = skia_safe::ImageInfo::new( + (w as i32, h as i32), + skia_safe::ColorType::RGBA8888, + skia_safe::AlphaType::Unpremul, + None, + ); + let mut buf = vec![0u8; (w * h * 4) as usize]; + surface.read_pixels(&info, &mut buf, (w * 4) as usize, (0, 0)); + buf +} + +fn root_node(w: f32, h: f32, background: &str, children: Vec) -> BoxNode { + BoxNode { + id: 0, + kind: BoxKind::Container, + css: CssStyle { + display: Some(Display::Flex), + flex_direction: Some(FlexDirection::Column), + width: Some(CSize::Length(CLP::Px(w))), + height: Some(CSize::Length(CLP::Px(h))), + background: Some(Background::Color(CssColor::String(background.to_string()))), + ..Default::default() + }, + children, + intrinsic: None, + source_path: None, + window: None, + } +} + +fn probe(buf: &[u8], w: u32, x: usize, y: usize) -> (u8, u8, u8) { + let i = (y * w as usize + x) * 4; + (buf[i], buf[i + 1], buf[i + 2]) +} + +// ---- RM-01: opacity layer must not clip the node's own outset box-shadow ---- + +fn card_with_shadow(opacity: Option) -> BoxNode { + let css = CssStyle { + position: Some(Position::Absolute), + left: Some(CLP::Px(50.0)), + top: Some(CLP::Px(50.0)), + width: Some(CSize::Length(CLP::Px(100.0))), + height: Some(CSize::Length(CLP::Px(100.0))), + background: Some(Background::Color(CssColor::String("#ffffff".into()))), + box_shadow: Some(vec![BoxShadow { + offset_x: Length::Px(0.0), + offset_y: Length::Px(0.0), + blur: None, + spread: Some(Length::Px(20.0)), + color: Some(CssColor::String("#ff0000".into())), + inset: None, + }]), + opacity, + ..Default::default() + }; + BoxNode { + id: 0, + kind: BoxKind::Container, + css, + children: vec![], + intrinsic: None, + source_path: None, + window: None, + } +} + +#[test] +fn opacity_layer_does_not_clip_own_outset_box_shadow() { + // 100x100 white card at (50,50) on a 200x200 black canvas, outset + // box-shadow (red, spread 20, blur 0 -> hard-edged halo rect from + // (30,30) to (170,170)). Probe point (100,45) sits in the halo band + // above the card, outside its own border-box. `opacity: 0.999` forces + // the opacity/filter SaveLayerRec open without visibly dimming the + // probed color. + let opaque = { + let mut root = root_node(200.0, 200.0, "#000000", vec![card_with_shadow(None)]); + render_pixels(&mut root, 200, 200) + }; + let faded = { + let mut root = root_node(200.0, 200.0, "#000000", vec![card_with_shadow(Some(0.999))]); + render_pixels(&mut root, 200, 200) + }; + + let above_opaque = probe(&opaque, 200, 100, 45); + assert!( + above_opaque.0 > 200 && above_opaque.1 < 50, + "sanity: shadow halo must be visible without an opacity layer, got {above_opaque:?}" + ); + + let above_faded = probe(&faded, 200, 100, 45); + assert!( + above_faded.0 > 200 && above_faded.1 < 50, + "an opacity<1 layer must not clip the node's own outset box-shadow, got {above_faded:?}" + ); +} + +// ---- RM-09: underdamped spring step response must start from rest ---- + +fn spring_config(damping: f64, stiffness: f64, mass: f64) -> SpringConfig { + SpringConfig { + damping, + stiffness, + mass, + duration: None, + rest_threshold: None, + } +} + +#[test] +fn underdamped_spring_step_response_starts_at_rest() { + // Shipped defaults named in the audit finding: damping=15, stiffness=100, + // mass=1 -> zeta=0.75 (underdamped). A step response that starts from + // rest has ~0 velocity at t=0; a wrong sine-argument formula produces a + // jolt of about 6.21/s instead. + let config = spring_config(15.0, 100.0, 1.0); + let h = 1e-5; + let v0 = spring_value(0.0, &config); + assert!(v0.abs() < 1e-9, "sanity: spring must start at 0, got {v0}"); + + let vh = spring_value(h, &config); + let slope = (vh - v0) / h; + assert!( + slope.abs() < 0.05, + "underdamped step response must start at rest (~0 initial velocity), got slope {slope}" + ); +} + +#[test] +fn underdamped_spring_matches_the_analytic_closed_form() { + // Reference computed independently of the engine's implementation from + // the textbook closed form for an underdamped step response: + // 1 - e^{-zeta*omega*t} * [cos(omega_d*t) + (zeta*omega/omega_d)*sin(omega_d*t)] + let damping = 15.0_f64; + let stiffness = 100.0_f64; + let mass = 1.0_f64; + let omega = (stiffness / mass).sqrt(); + let zeta = damping / (2.0 * (stiffness * mass).sqrt()); + let omega_d = omega * (1.0 - zeta * zeta).sqrt(); + let reference = |t: f64| -> f64 { + let decay = (-zeta * omega * t).exp(); + 1.0 - decay * ((omega_d * t).cos() + (zeta * omega / omega_d) * (omega_d * t).sin()) + }; + + let config = spring_config(damping, stiffness, mass); + for t in [0.0, 1.0 / 60.0, 0.1, 0.3, 0.6, 1.0] { + let expected = reference(t); + let actual = spring_value(t, &config); + assert!( + (actual - expected).abs() < 1e-6, + "t={t}: expected {expected} (analytic reference), got {actual}" + ); + } +} + +// ---- RM-10: 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:?}" + ); +} + +// ---- RM-37: `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 new file mode 100644 index 00000000..4f02d02f --- /dev/null +++ b/crates/rustmotion-core/tests/audit_ws_h.rs @@ -0,0 +1,109 @@ +//! Regression tests for the workstream H (untrusted scenario ingestion) +//! audit findings that live in `rustmotion-core`: RM-41, RM-42, RM-43. + +use rustmotion_core::engine::renderer::{extract_video_frame, http_agent}; + +// ---- RM-41: no timeout on any HTTP call — a bare `ureq::get` always uses +// the default, untimed agent, so a stalled host hangs a render forever ---- + +#[test] +fn shared_http_agent_has_finite_global_and_connect_timeouts() { + let timeouts = http_agent().config().timeouts(); + assert!( + timeouts.global.is_some(), + "ureq 3.x's default Timeouts::global is None — the shared agent must override it \ + so a stalled host cannot hang a render forever" + ); + assert!( + timeouts.connect.is_some(), + "a hung TCP handshake must not hang forever either" + ); +} + +// ---- RM-42: a scenario's `video.src` reaches `ffmpeg -i` verbatim, with no +// protocol allowlist — a remote-looking src turns into an SSRF primitive ---- + +#[test] +fn extract_video_frame_rejects_a_remote_src_before_it_ever_reaches_ffmpeg() { + // 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!( + msg.contains("does not fetch video over the network"), + "expected the scheme-rejection error, not an ffmpeg spawn/exit failure: {msg}" + ); +} + +#[test] +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 over the network"), + "a plain local path must fail on ffmpeg/the missing file, not on the scheme check: {err}" + ); +} + +// ---- RM-43: `for-each` expansion has a depth ceiling but no node budget — +// nesting is multiplicative, so a handful of small arrays nested a few +// levels deep can declare a product in the millions ---- + +#[test] +fn for_each_node_budget_rejects_a_declared_product_that_exceeds_the_cap() { + fn items(n: usize) -> serde_json::Value { + serde_json::Value::Array( + (0..n) + .map(|i| serde_json::json!({ "v": i })) + .collect::>(), + ) + } + + // Three levels of 200 elements nested directly in each other's + // `template.children`: a declared product of 200^3 = 8,000,000 nodes, + // comfortably past a low-millions cap. The array literals themselves + // (200 small JSON objects, three times) are cheap to build — the + // assertion is that expansion refuses the *product*, not that it + // finishes computing it. + let mut doc = serde_json::json!({ + "video": { "width": 100, "height": 100 }, + "scenes": [{ + "duration": 1.0, + "children": [{ + "for-each": items(200), + "template": { + "type": "card", + "children": [{ + "for-each": items(200), + "template": { + "type": "card", + "children": [{ + "for-each": items(200), + "template": { "type": "text", "content": "$v" } + }] + } + }] + } + }] + }] + }); + + let err = rustmotion_core::expand::expand_directives(&mut doc, "test.json") + .expect_err("a declared product this far past the cap must be rejected"); + let msg = err.to_string(); + assert!( + msg.contains("budget"), + "error should name the node-budget ceiling it exceeded: {msg}" + ); +} diff --git a/crates/rustmotion-core/tests/audit_ws_j.rs b/crates/rustmotion-core/tests/audit_ws_j.rs new file mode 100644 index 00000000..4da80594 --- /dev/null +++ b/crates/rustmotion-core/tests/audit_ws_j.rs @@ -0,0 +1,126 @@ +//! Regression tests for the workstream J (layout pass & CSS unit resolution) +//! audit findings: RM-26, RM-27. + +use std::sync::{Arc, Mutex}; + +use rustmotion_core::css::style::{CssStyle, Edges, Size as CSize}; +use rustmotion_core::css::taffy_bridge::ConversionContext; +use rustmotion_core::css::units::{Length, LengthPercentage as CLP}; +use rustmotion_core::engine::box_tree::{AvailableSpace, BoxNode, IntrinsicMeasure}; +use rustmotion_core::engine::layout_pass::run_layout; + +// ---- RM-26: `em` on layout properties must resolve against the element's +// own (inherited) font-size, not a constant 16px ---- + +/// A card sized by its parent's explicit `font-size` (48px) inherits that +/// font-size down to its own layout resolution even though it never sets +/// `font-size` itself. Its `padding: "1em"` must therefore resolve to 48px +/// (`1 * inherited_font_size`), not the pre-fix constant 16px. +#[test] +fn em_padding_resolves_against_inherited_font_size_not_a_constant_16px() { + let mut root = BoxNode::container( + CssStyle { + font_size: Some(Length::Px(48.0)), + width: Some(CSize::Length(CLP::Px(400.0))), + height: Some(CSize::Length(CLP::Px(400.0))), + ..Default::default() + }, + vec![BoxNode::container( + CssStyle { + padding: Some(Edges::Uniform(CLP::String("1em".into()))), + width: Some(CSize::Length(CLP::Px(200.0))), + height: Some(CSize::Length(CLP::Px(200.0))), + ..Default::default() + }, + vec![], + )], + ); + root.assign_ids(1); + + let res = run_layout(&root, (400.0, 400.0), &ConversionContext::default()); + let child = res.get(2).expect("child laid out"); + let (content_x, _, content_w, _) = child.content_box(); + + assert_eq!( + content_x, 48.0, + "1em padding must resolve against the inherited 48px font-size" + ); + assert_eq!(content_w, 200.0 - 2.0 * 48.0); +} + +// ---- RM-27: a leaf's `IntrinsicMeasure::measure` must receive `known` and +// `available` in the same (content-box) coordinate space ---- + +type RecordedCall = ((Option, Option), (AvailableSpace, AvailableSpace)); + +#[derive(Default)] +struct RecordingIntrinsic { + calls: Mutex>, +} + +impl IntrinsicMeasure for RecordingIntrinsic { + fn measure( + &self, + known: (Option, Option), + available: (AvailableSpace, AvailableSpace), + ) -> (f32, f32) { + self.calls.lock().unwrap().push((known, available)); + (50.0, 30.0) + } +} + +/// A leaf with 20px uniform padding, stretched to its column-flex parent's +/// full 300px content width but auto-height (so taffy must measure its +/// intrinsic height with the width already resolved). Any call where +/// `known.0` is definite must agree with `available.0` when that is also +/// definite: both describe the same box, and per taffy 0.10.1's own +/// `compute_leaf_layout`, `available_space` has already had padding+border +/// subtracted before reaching the measure function. +#[test] +fn measure_fn_known_and_available_agree_on_content_box_width() { + use rustmotion_core::css::style::{AlignItems, Display, FlexDirection}; + + let recorder = Arc::new(RecordingIntrinsic::default()); + let leaf = BoxNode::leaf( + CssStyle { + padding: Some(Edges::Uniform(CLP::Px(20.0))), + ..Default::default() + }, + recorder.clone(), + ); + let mut root = BoxNode::container( + CssStyle { + display: Some(Display::Flex), + flex_direction: Some(FlexDirection::Column), + align_items: Some(AlignItems::Stretch), + width: Some(CSize::Length(CLP::Px(300.0))), + height: Some(CSize::Length(CLP::Px(300.0))), + ..Default::default() + }, + vec![leaf], + ); + root.assign_ids(1); + + run_layout(&root, (300.0, 300.0), &ConversionContext::default()); + + let calls = recorder.calls.lock().unwrap(); + let definite_known_calls: Vec<_> = calls + .iter() + .filter(|(known, _)| known.0.is_some()) + .collect(); + assert!( + !definite_known_calls.is_empty(), + "expected at least one measure call with a definite known width, got {calls:?}" + ); + + for (known, available) in definite_known_calls { + if let AvailableSpace::Definite(available_w) = available.0 { + assert_eq!( + known.0.unwrap(), + available_w, + "known.width and available.width must describe the same \ + (content-box) box; known={known:?} available={available:?}" + ); + } + } +} diff --git a/crates/rustmotion-core/tests/audit_ws_k.rs b/crates/rustmotion-core/tests/audit_ws_k.rs new file mode 100644 index 00000000..c6a68899 --- /dev/null +++ b/crates/rustmotion-core/tests/audit_ws_k.rs @@ -0,0 +1,264 @@ +//! Regression tests for the workstream K (docs, schema, CI) audit findings: +//! RM-02, RM-47, RM-52. + +use std::collections::BTreeSet; +use std::fs; +use std::path::{Path, PathBuf}; + +/// Text immediately following the component count in the README's +/// Architecture section (see `README.md`'s "rustmotion ships N components, +/// each implementing the `Painter` trait" sentence). +const README_COUNT_MARKER: &str = " components, each implementing the `Painter` trait"; + +/// Text immediately following the component count in +/// `crates/rustmotion-components/Cargo.toml`'s `description` — the string +/// crates.io displays for the published crate. +const CARGO_TOML_COUNT_MARKER: &str = " components)\""; + +/// Read the integer that appears immediately before `marker` in `haystack`, +/// skipping trailing whitespace. Panics with the marker text on failure so a +/// reworded sentence names exactly what moved instead of a bare parse error. +fn number_before(haystack: &str, marker: &str, haystack_name: &str) -> u32 { + let idx = haystack.find(marker).unwrap_or_else(|| { + panic!( + "marker {marker:?} not found in {haystack_name} — did the component count sentence \ + move or get reworded? Update this test's marker to match." + ) + }); + let digits: String = haystack[..idx] + .chars() + .rev() + .skip_while(|c| c.is_whitespace()) + .take_while(|c| c.is_ascii_digit()) + .collect(); + let digits: String = digits.chars().rev().collect(); + digits.parse().unwrap_or_else(|_| { + panic!("no number found immediately before marker {marker:?} in {haystack_name}") + }) +} + +/// Count the variants of `pub enum Component` in +/// `crates/rustmotion-components/src/lib.rs`, by counting non-empty, +/// non-attribute lines between its opening `{` and closing `}`. Every +/// variant in that enum is declared on its own line (`Name(Type),`); the +/// only other lines in the block are `#[serde(...)]` attributes. +fn count_component_variants(lib_rs: &str) -> usize { + let start_marker = "pub enum Component {"; + let start = lib_rs.find(start_marker).unwrap_or_else(|| { + panic!("{start_marker:?} not found in rustmotion-components/src/lib.rs") + }) + start_marker.len(); + let rest = &lib_rs[start..]; + let end = rest + .find("\n}") + .expect("no closing '}' found for `pub enum Component` block"); + let body = &rest[..end]; + body.lines() + .map(str::trim) + .filter(|line| !line.is_empty() && !line.starts_with('#')) + .count() +} + +/// RM-02 / RM-47: README.md and `rustmotion-components/Cargo.toml` (the text +/// crates.io shows for the published crate) both claimed "51 components" +/// while `Component` actually had 60 variants — and nothing kept the two in +/// sync. Locks the documented counts to the real one so a future component +/// addition/removal that forgets to update the docs fails CI instead of +/// drifting silently again. +#[test] +fn documented_component_count_matches_enum_variant_count() { + let lib_rs = include_str!("../../rustmotion-components/src/lib.rs"); + let actual = count_component_variants(lib_rs); + assert!( + actual > 0, + "found zero variants in `pub enum Component` — count parsing is broken" + ); + + let readme = include_str!("../../../README.md"); + let readme_count = number_before(readme, README_COUNT_MARKER, "README.md"); + assert_eq!( + readme_count as usize, actual, + "README.md claims {readme_count} components but `Component` has {actual} variants" + ); + + let cargo_toml = include_str!("../../rustmotion-components/Cargo.toml"); + let cargo_toml_count = number_before( + cargo_toml, + CARGO_TOML_COUNT_MARKER, + "rustmotion-components/Cargo.toml", + ); + assert_eq!( + cargo_toml_count as usize, actual, + "rustmotion-components/Cargo.toml's description claims {cargo_toml_count} components but \ + `Component` has {actual} variants" + ); +} + +/// Public schema fields that parse successfully but are not read anywhere +/// outside `crates/rustmotion-core/src/schema/` — kept out of +/// `every_public_schema_field_is_read_somewhere_or_allowlisted`'s failure so +/// a *known, tracked* gap doesn't block CI, while a *new* one still does. +/// Each entry names the finding that tracks closing it and the reason it +/// isn't closed by workstream K itself. Removing an entry once the field is +/// wired (or deleted) is the expected way this list shrinks. +const KNOWN_INERT_FIELDS: &[(&str, &str)] = &[ + ( + "codec", + "RM-50: VideoConfig.codec is not threaded into the encode path. Honouring it means \ + editing crates/rustmotion/src/cli/mod.rs (the Render/Batch/Still command handlers) and \ + crates/rustmotion/src/encode/, both outside workstream K's owned files — see the \ + workstream K report's handover for the exact wiring point.", + ), + ( + "intensity", + "RM-51: MotionBlurConfig.intensity is read into AnimatedProperties.motion_blur \ + (crates/rustmotion-core/src/engine/animator.rs:259) but nothing reads that field \ + afterwards. Wiring it into the ghost-opacity math, or deleting it, both touch \ + crates/rustmotion-components/src/box_builder.rs and crates/rustmotion-core/src/engine/\ + animator.rs — outside workstream K's owned files.", + ), + ( + "version", + "Not one of workstream K's 9 named findings — surfaced by this guard test itself. \ + Scenario.version defaults to \"1.0\" and deserializes, but crates/rustmotion/src/\ + loader.rs never reads it back (no version-gating or migration logic exists yet). \ + Wiring or removing it touches loader.rs, outside workstream K's owned files; flagged \ + in the workstream K report for triage.", + ), + ( + "target", + "Not one of workstream K's 9 named findings — surfaced by this guard test itself, with \ + a caveat this test can't resolve on its own: Annotation.target is written by \ + rustmotion-studio (crates/rustmotion-studio/src/editor/annotations.rs:94) as raw JSON \ + (a `\"target\": {...}` object literal, not a `.target` field access — this grep-based \ + check only matches Rust member access), so it may be consumed by the `apply-annotations` \ + Claude Code skill reading the scenario file's raw JSON rather than by any Rust code path. \ + Allowlisted rather than asserted dead; flagged in the workstream K report for a human to \ + confirm one way or the other.", + ), +]; + +/// `crates/rustmotion-core` -> `crates` -> ``. +fn workspace_root() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")) + .parent() + .and_then(Path::parent) + .expect("rustmotion-core is expected at /crates/rustmotion-core") + .to_path_buf() +} + +/// Recursively collect every `.rs` file under `dir`, skipping `target/`. +fn collect_rs_files(dir: &Path, out: &mut Vec) { + let entries = fs::read_dir(dir).unwrap_or_else(|e| panic!("read_dir {}: {e}", dir.display())); + for entry in entries { + let entry = entry.unwrap_or_else(|e| panic!("read_dir entry in {}: {e}", dir.display())); + let path = entry.path(); + if path.is_dir() { + if path.file_name().and_then(|n| n.to_str()) == Some("target") { + continue; + } + collect_rs_files(&path, out); + } else if path.extension().and_then(|e| e.to_str()) == Some("rs") { + out.push(path); + } + } +} + +/// Extract the names of `pub : ,`-style struct fields from Rust +/// source text. Deliberately crude (no parser): looks for lines whose +/// trimmed text starts with `"pub "` and contains a `:` before any +/// non-identifier character, which matches plain field declarations +/// (`pub width: u32,`) while excluding `pub fn`/`pub struct`/`pub enum` +/// (no top-level `:`, or one buried behind non-identifier characters like +/// `(`/`<`/`&`/spaces that fail the identifier check below). +fn extract_pub_field_names(source: &str) -> Vec { + let mut names = Vec::new(); + for line in source.lines() { + let trimmed = line.trim(); + let Some(rest) = trimmed.strip_prefix("pub ") else { + continue; + }; + let Some(colon_idx) = rest.find(':') else { + continue; + }; + let candidate = rest[..colon_idx].trim(); + let is_plain_identifier = !candidate.is_empty() + && candidate + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '_'); + if is_plain_identifier { + names.push(candidate.to_string()); + } + } + names +} + +/// RM-52: nothing stopped a schema field from parsing successfully and then +/// being read by no code path — RM-50 (`VideoConfig.codec`) and RM-51 +/// (`MotionBlurConfig.intensity`) are exactly that defect, and the format's +/// credibility as an LLM generation target rests on a field either doing +/// something or failing to parse. This test greps the workspace for a +/// member-access on every public schema field name and fails if one isn't +/// found anywhere outside its own definition — unless it's in +/// `KNOWN_INERT_FIELDS`, so a newly introduced inert field still fails CI. +#[test] +fn every_public_schema_field_is_read_somewhere_or_allowlisted() { + let root = workspace_root(); + let schema_dir = root.join("crates/rustmotion-core/src/schema"); + + let mut schema_files = Vec::new(); + collect_rs_files(&schema_dir, &mut schema_files); + assert!( + !schema_files.is_empty(), + "expected {} to contain schema files", + schema_dir.display() + ); + + let mut field_names: BTreeSet = BTreeSet::new(); + for path in &schema_files { + let content = + fs::read_to_string(path).unwrap_or_else(|e| panic!("read {}: {e}", path.display())); + field_names.extend(extract_pub_field_names(&content)); + } + assert!( + field_names.len() > 50, + "expected well over 50 public schema fields, found {} — field extraction is probably broken", + field_names.len() + ); + + let self_path = root + .join(file!()) + .canonicalize() + .unwrap_or_else(|e| panic!("canonicalize {}: {e}", root.join(file!()).display())); + + let mut other_files = Vec::new(); + collect_rs_files(&root.join("crates"), &mut other_files); + let other_sources: Vec = other_files + .iter() + .filter(|path| !schema_files.contains(path)) + .filter(|path| path.canonicalize().map(|p| p != self_path).unwrap_or(true)) + .map(|path| { + fs::read_to_string(path).unwrap_or_else(|e| panic!("read {}: {e}", path.display())) + }) + .collect(); + + let mut unread_fields = Vec::new(); + for name in &field_names { + if KNOWN_INERT_FIELDS.iter().any(|(f, _)| f == name) { + continue; + } + let access_pattern = format!(".{name}"); + let is_read = other_sources + .iter() + .any(|content| content.contains(&access_pattern)); + if !is_read { + unread_fields.push(name.clone()); + } + } + + assert!( + unread_fields.is_empty(), + "public schema field(s) never read outside crates/rustmotion-core/src/schema/: \ + {unread_fields:?}\nEither wire them into the engine/CLI, or add them to \ + KNOWN_INERT_FIELDS above with a reason and the finding that tracks closing it." + ); +} diff --git a/crates/rustmotion-html/src/element.rs b/crates/rustmotion-html/src/element.rs index c308265a..0722aa9d 100644 --- a/crates/rustmotion-html/src/element.rs +++ b/crates/rustmotion-html/src/element.rs @@ -2,7 +2,9 @@ use markup5ever_rcdom::{Handle, NodeData}; use serde_json::{Map, Value}; use crate::style::{coerce_value, parse_anim_attr, parse_inline_style}; -use crate::{element_attrs, tag_name, HtmlError}; +use crate::{check_known_attrs, element_attrs, tag_name, HtmlError}; + +const KNOWN_NATIVE_ATTRS: &[&str] = &["style", "anim"]; enum TagKind { Container, @@ -35,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("