diff --git a/crates/rustmotion-components/src/gif.rs b/crates/rustmotion-components/src/gif.rs index b5698604..5a0d918d 100644 --- a/crates/rustmotion-components/src/gif.rs +++ b/crates/rustmotion-components/src/gif.rs @@ -455,9 +455,9 @@ mod tests { /// own video is, so passing generous `max_w`/`max_h` here isolates the /// byte-budget check from the video-dimensions check exercised by the /// next test. 144 MiB is deliberately far short of the 65535×65535 - /// (~17 GiB) header a real crafted file could declare — large enough to - /// prove the budget check fires, small enough that running this test - /// never risks the allocation it is asserting never happens. + /// (~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!( diff --git a/crates/rustmotion-components/src/table.rs b/crates/rustmotion-components/src/table.rs index ab882664..06e90ff4 100644 --- a/crates/rustmotion-components/src/table.rs +++ b/crates/rustmotion-components/src/table.rs @@ -419,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/video.rs b/crates/rustmotion-components/src/video.rs index 1500dbb5..cae7b018 100644 --- a/crates/rustmotion-components/src/video.rs +++ b/crates/rustmotion-components/src/video.rs @@ -176,7 +176,7 @@ 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; diff --git a/crates/rustmotion-components/tests/audit_ws_d.rs b/crates/rustmotion-components/tests/audit_ws_d.rs index e884b076..7ac9dbd8 100644 --- a/crates/rustmotion-components/tests/audit_ws_d.rs +++ b/crates/rustmotion-components/tests/audit_ws_d.rs @@ -281,3 +281,43 @@ fn without_loop_video_playback_still_clamps_not_wraps() { "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 index 358e2c9c..46a94cbd 100644 --- a/crates/rustmotion-components/tests/audit_ws_i.rs +++ b/crates/rustmotion-components/tests/audit_ws_i.rs @@ -134,7 +134,9 @@ fn paint_card_with_child(card_json: serde_json::Value) -> PaintedScene { fn count_dominant(pixels: &[u8], dominant: usize, muted: &[usize]) -> usize { pixels - .chunks_exact(4) + .as_chunks::<4>() + .0 + .iter() .filter(|p| p[dominant] > 180 && muted.iter().all(|&m| p[m] < 80)) .count() } @@ -213,7 +215,7 @@ fn text_own_color_wins_over_cascaded_card_color_at_paint_time() { } fn any_ink_above_black_below_y(pixels: &[u8], width: i32, y_threshold: i32) -> bool { - pixels.chunks_exact(4).enumerate().any(|(i, p)| { + 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) }) diff --git a/crates/rustmotion-core/src/css/taffy_bridge.rs b/crates/rustmotion-core/src/css/taffy_bridge.rs index e30401ce..7ee1fbce 100644 --- a/crates/rustmotion-core/src/css/taffy_bridge.rs +++ b/crates/rustmotion-core/src/css/taffy_bridge.rs @@ -246,7 +246,7 @@ pub fn to_taffy_style(css: &CssStyle, ctx: &ConversionContext) -> tf::Style { /// = 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. Percentage padding/border resolves +/// 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. diff --git a/crates/rustmotion-core/src/engine/animator.rs b/crates/rustmotion-core/src/engine/animator.rs index be2dce5e..ba343334 100644 --- a/crates/rustmotion-core/src/engine/animator.rs +++ b/crates/rustmotion-core/src/engine/animator.rs @@ -2867,7 +2867,7 @@ mod spring_duration_tests { // 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 when the + // 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. diff --git a/crates/rustmotion-core/src/engine/layout_pass.rs b/crates/rustmotion-core/src/engine/layout_pass.rs index 386c201d..e9a03c5e 100644 --- a/crates/rustmotion-core/src/engine/layout_pass.rs +++ b/crates/rustmotion-core/src/engine/layout_pass.rs @@ -72,7 +72,7 @@ impl LayoutResult { /// taffy nodes and our `BoxNode` ids + intrinsic measurers. /// /// `inset_width`/`inset_height` are this node's own resolved padding+border -///: taffy's `compute_leaf_layout` already subtracts them from +/// (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 @@ -137,7 +137,7 @@ pub fn run_layout(root: &BoxNode, viewport: (f32, f32), ctx: &ConversionContext) /// Build one taffy node and, recursively, its subtree. /// /// `inherited_font_size` is the already-resolved (px) font-size of `node`'s -/// parent: CSS resolves `em` on every layout property against the +/// 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 diff --git a/crates/rustmotion-core/src/engine/renderer/assets.rs b/crates/rustmotion-core/src/engine/renderer/assets.rs index 9121f48e..97345599 100644 --- a/crates/rustmotion-core/src/engine/renderer/assets.rs +++ b/crates/rustmotion-core/src/engine/renderer/assets.rs @@ -50,7 +50,7 @@ pub fn gif_cache() -> &'static GifCacheMap { /// 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. +/// notice (RM-41). /// /// `Config::builder()` starts from `Config::default()`, which already /// resolves a proxy from `HTTPS_PROXY`/`https_proxy`/`HTTP_PROXY`/ @@ -258,7 +258,7 @@ pub fn ffmpeg_available() -> bool { /// `rtmp://`, `concat:`, …), which turns an unfiltered `src` into an SSRF /// primitive — a scenario author can point it at /// `http://169.254.169.254/...` (the cloud metadata endpoint) or an internal -/// service, and read the exit status as a port-scan oracle. Remote +/// 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 @@ -276,7 +276,7 @@ fn reject_remote_video_src(src: &str) -> Result<()> { 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" + over the network, only local file paths are accepted (RM-42)" ))); } Ok(()) diff --git a/crates/rustmotion-core/src/expand.rs b/crates/rustmotion-core/src/expand.rs index ea153e98..c47860e1 100644 --- a/crates/rustmotion-core/src/expand.rs +++ b/crates/rustmotion-core/src/expand.rs @@ -157,7 +157,7 @@ use crate::variables::substitute; 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. +/// 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_ @@ -306,7 +306,7 @@ pub fn expand_directives(value: &mut Value, file_label: &str) -> Result<()> { } /// Subtracts `n` from the shared expansion-node budget, or fails naming the -/// limit and where it was hit. See [`MAX_EXPANSION_NODES`] for why +/// 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 diff --git a/crates/rustmotion-core/tests/audit_ws_a.rs b/crates/rustmotion-core/tests/audit_ws_a.rs index d264fbae..7ad93749 100644 --- a/crates/rustmotion-core/tests/audit_ws_a.rs +++ b/crates/rustmotion-core/tests/audit_ws_a.rs @@ -1,5 +1,5 @@ //! Regression tests for the workstream A (animation & paint) audit findings -//! tracked in issue #220. +//! 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, @@ -72,7 +72,7 @@ fn probe(buf: &[u8], w: u32, x: usize, y: usize) -> (u8, u8, u8) { (buf[i], buf[i + 1], buf[i + 2]) } -// ---- opacity layer must not clip the node's own outset box-shadow ---- +// ---- RM-01: opacity layer must not clip the node's own outset box-shadow ---- fn card_with_shadow(opacity: Option) -> BoxNode { let css = CssStyle { @@ -134,7 +134,7 @@ fn opacity_layer_does_not_clip_own_outset_box_shadow() { ); } -// ---- underdamped spring step response must start from rest ---- +// ---- RM-09: underdamped spring step response must start from rest ---- fn spring_config(damping: f64, stiffness: f64, mass: f64) -> SpringConfig { SpringConfig { @@ -192,7 +192,7 @@ fn underdamped_spring_matches_the_analytic_closed_form() { } } -// ---- linear-gradient(180deg, ...) must put the first stop at the top ---- +// ---- 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 { @@ -243,7 +243,7 @@ fn linear_gradient_180deg_puts_the_first_stop_at_the_top() { ); } -// ---- `spring_settle_time` must not be rescanned on every `spring_value` call ---- +// ---- 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() { diff --git a/crates/rustmotion-core/tests/audit_ws_h.rs b/crates/rustmotion-core/tests/audit_ws_h.rs index 0fd3a042..4f02d02f 100644 --- a/crates/rustmotion-core/tests/audit_ws_h.rs +++ b/crates/rustmotion-core/tests/audit_ws_h.rs @@ -1,9 +1,9 @@ //! Regression tests for the workstream H (untrusted scenario ingestion) -//! audit findings that live in `rustmotion-core`. +//! audit findings that live in `rustmotion-core`: RM-41, RM-42, RM-43. use rustmotion_core::engine::renderer::{extract_video_frame, http_agent}; -// ---- no timeout on any HTTP call — a bare `ureq::get` always uses +// ---- 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] @@ -20,7 +20,7 @@ fn shared_http_agent_has_finite_global_and_connect_timeouts() { ); } -// ---- a scenario's `video.src` reaches `ffmpeg -i` verbatim, with no +// ---- 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] @@ -56,7 +56,7 @@ fn extract_video_frame_does_not_reject_a_plain_local_path() { ); } -// ---- `for-each` expansion has a depth ceiling but no node budget — +// ---- 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 ---- diff --git a/crates/rustmotion-core/tests/audit_ws_j.rs b/crates/rustmotion-core/tests/audit_ws_j.rs index 6b3501b9..4da80594 100644 --- a/crates/rustmotion-core/tests/audit_ws_j.rs +++ b/crates/rustmotion-core/tests/audit_ws_j.rs @@ -1,5 +1,5 @@ //! Regression tests for the workstream J (layout pass & CSS unit resolution) -//! audit findings on unit resolution and intrinsic measurement. +//! audit findings: RM-26, RM-27. use std::sync::{Arc, Mutex}; @@ -9,7 +9,7 @@ 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; -// ---- `em` on layout properties must resolve against the element's +// ---- 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 @@ -48,7 +48,7 @@ fn em_padding_resolves_against_inherited_font_size_not_a_constant_16px() { assert_eq!(content_w, 200.0 - 2.0 * 48.0); } -// ---- a leaf's `IntrinsicMeasure::measure` must receive `known` and +// ---- 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)); diff --git a/crates/rustmotion-html/tests/audit_ws_f.rs b/crates/rustmotion-html/tests/audit_ws_f.rs index e8ab981a..00c40637 100644 --- a/crates/rustmotion-html/tests/audit_ws_f.rs +++ b/crates/rustmotion-html/tests/audit_ws_f.rs @@ -1,17 +1,17 @@ //! Regression tests for workstream F's 4 confirmed audit findings on the //! HTML dialect transpiler: an attribute outside a hardcoded allowlist -//! vanishing with no error, a CSS shorthand transpiling to a string -//! the core length parser cannot read and silently resolving to 0px, +//! vanishing with no error (RM-03), a CSS shorthand transpiling to a string +//! the core length parser cannot read and silently resolving to 0px (RM-04), //! `