From 03a70a882d0826fc94444460845a3c9ebe7dede3 Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Tue, 22 Sep 2026 10:58:30 +0200 Subject: [PATCH] fix(geometry): resolve transform lengths per axis and per font-size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `apply_transform_chain` resolves `translate`/`translateX`/`translateY` lengths through this context. The paint pass resolves the very same functions through `LengthContext { ..., parent_size: box_layout.width.max(height), font_size: node.css.font_size_px_or(16.0), ... }` and then splits it into per-axis `length_ctx_x`/`length_ctx_y` (crates/rustmotion-core/src/engine/paint_pass.rs:229-250, used at :953-962). Two divergences: (a) `em` — a `font-size: 96px` text with `transform: "translateX(-10em)"` is moved -960px by the renderer but only -160px by the validator, so a component at x=200 that renders at x=-760 (fully off-frame) validates clean; (b) percentage translate — the validator uses `max(w, h)` on both axes, the exact mistake paint_pass.rs:236-242 calls out in a comment ("never max(width, height) on both axes"), which over-estimates on the short axis and produces false positives. Note the `--strict-anim` path funnels animated transforms through this same function (geometry.rs:1482), so it inherits both errors. Refs #220 --- .../rustmotion/src/cli/commands/geometry.rs | 49 +++++++++-- crates/rustmotion/tests/audit_ws_b.rs | 88 +++++++++++++++++++ 2 files changed, 130 insertions(+), 7 deletions(-) diff --git a/crates/rustmotion/src/cli/commands/geometry.rs b/crates/rustmotion/src/cli/commands/geometry.rs index 0fd7d069..e1c10bf4 100644 --- a/crates/rustmotion/src/cli/commands/geometry.rs +++ b/crates/rustmotion/src/cli/commands/geometry.rs @@ -473,6 +473,16 @@ fn container_clips(c: &Component) -> bool { /// path, out of scope for this fix. Animated transform-producing presets are /// folded separately in `walk_anim`; this only handles what a component /// declares directly in `style.transform`. +/// +/// `font_size` is the NODE's own resolved font-size +/// (`css.font_size_px_or(16.0)`), not a hardcoded 16px — an `em` length in +/// `transform` must scale with the element it's declared on, exactly like +/// `paint_pass.rs`'s `length_ctx` does for the same field. Percentage +/// lengths inside `transform` resolve per axis (`ctx_x`/`ctx_y`, mirroring +/// `paint_pass.rs`'s `length_ctx_x`/`length_ctx_y`) rather than against a +/// single `bbox.w.max(bbox.h)` shared by both axes — see +/// `apply_transform_chain`'s doc comment for why a shared context there was +/// wrong for every non-square box. fn apply_static_node_transform(bbox: &BBox, css: &CssStyle, viewport: (f32, f32)) -> BBox { let transform = match css.transform.as_deref() { Some(t) if !t.is_empty() => t, @@ -482,9 +492,17 @@ fn apply_static_node_transform(bbox: &BBox, css: &CssStyle, viewport: (f32, f32) viewport_width: viewport.0, viewport_height: viewport.1, parent_size: bbox.w.max(bbox.h), - font_size: 16.0, + font_size: css.font_size_px_or(16.0), root_font_size: 16.0, }; + let ctx_x = LengthContext { + parent_size: bbox.w, + ..ctx + }; + let ctx_y = LengthContext { + parent_size: bbox.h, + ..ctx + }; let (pivot_x, pivot_y) = resolve_transform_origin_2d(css.transform_origin.as_ref(), bbox, &ctx); let corners = [ (bbox.x, bbox.y), @@ -498,7 +516,7 @@ fn apply_static_node_transform(bbox: &BBox, css: &CssStyle, viewport: (f32, f32) let mut min_y = f32::INFINITY; let mut max_y = f32::NEG_INFINITY; for (cx, cy) in corners { - let (tx, ty) = apply_transform_chain(transform, cx - pivot_x, cy - pivot_y, &ctx); + let (tx, ty) = apply_transform_chain(transform, cx - pivot_x, cy - pivot_y, &ctx_x, &ctx_y); let (wx, wy) = (pivot_x + tx, pivot_y + ty); min_x = min_x.min(wx); max_x = max_x.max(wx); @@ -584,15 +602,32 @@ fn resolve_transform_origin_2d( /// clockwise on a y-down canvas, i.e. `x' = x·cosθ − y·sinθ`, /// `y' = x·sinθ + y·cosθ`); `Skew`/`SkewX`/`SkewY` match `Canvas::skew` /// (`x' = x + y·tan(skew_x)`, `y' = y + x·tan(skew_y)`). -fn apply_transform_chain(list: &[TransformFn], x: f32, y: f32, ctx: &LengthContext) -> (f32, f32) { +/// +/// `ctx_x`/`ctx_y` are separate contexts differing only in +/// `parent_size` (the box's own width / height respectively), used for +/// `Translate`/`TranslateX`/`TranslateY`/`Translate3d`'s percentage +/// resolution — CSS resolves a translate's x-component percentage against +/// the box's own WIDTH and the y-component against its own HEIGHT, never a +/// single value shared by both axes (that's only correct for square boxes). +/// `Scale`/`Rotate`/`Skew` take unitless factors/degrees and never consult +/// either context. +fn apply_transform_chain( + list: &[TransformFn], + x: f32, + y: f32, + ctx_x: &LengthContext, + ctx_y: &LengthContext, +) -> (f32, f32) { let (mut x, mut y) = (x, y); for f in list.iter().rev() { let (nx, ny) = match f { - TransformFn::Translate { x: tx, y: ty } => (x + tx.resolve(ctx), y + ty.resolve(ctx)), - TransformFn::TranslateX { x: tx } => (x + tx.resolve(ctx), y), - TransformFn::TranslateY { y: ty } => (x, y + ty.resolve(ctx)), + TransformFn::Translate { x: tx, y: ty } => { + (x + tx.resolve(ctx_x), y + ty.resolve(ctx_y)) + } + TransformFn::TranslateX { x: tx } => (x + tx.resolve(ctx_x), y), + TransformFn::TranslateY { y: ty } => (x, y + ty.resolve(ctx_y)), TransformFn::Translate3d { x: tx, y: ty, .. } => { - (x + tx.resolve(ctx), y + ty.resolve(ctx)) + (x + tx.resolve(ctx_x), y + ty.resolve(ctx_y)) } TransformFn::Scale { x: sx, y: sy } => (x * sx, y * sy), TransformFn::ScaleX { x: sx } => (x * sx, y), diff --git a/crates/rustmotion/tests/audit_ws_b.rs b/crates/rustmotion/tests/audit_ws_b.rs index 6d8ee899..25b511ac 100644 --- a/crates/rustmotion/tests/audit_ws_b.rs +++ b/crates/rustmotion/tests/audit_ws_b.rs @@ -194,3 +194,91 @@ fn strict_anim_also_measures_vw_against_the_real_viewport_width() { "--strict-anim pass (line ~1347) must also be clean: {report_json}" ); } + +// ─── static transform lengths use the node's own font-size and per-axis size ─── + +/// `translateX(-10em)` on a 96px-font node must resolve against ITS OWN +/// font-size (960px), not the hardcoded 16px `apply_static_node_transform` +/// used to assume (160px). At x=200 with a 100px-wide box, the correct +/// translate pushes the box to x=-760 (fully off-frame, a real violation); +/// the buggy 160px translate only reaches x=40 (still on-frame, silent). +#[test] +fn static_translate_em_resolves_against_the_nodes_own_font_size() { + let scenario = ScratchFile::new("rm15-em-scenario"); + let report = ScratchFile::new("rm15-em-report"); + let json = r##"{ + "video": { "width": 1920, "height": 1080 }, + "scenes": [{ + "duration": 1.0, + "children": [{ + "type": "shape", + "shape": "rect", + "position": "absolute", + "x": 200, "y": 400, + "style": { + "width": "100px", "height": "50px", + "font-size": "96px", + "transform": [{ "fn": "translate-x", "x": "-10em" }] + }, + "fill": "#ff0000" + }] + }] + }"##; + std::fs::write(&scenario.0, json).expect("write scenario"); + + let output = run_validate(&scenario.0, Some(&report.0), false, false); + let report_json = read_report(&report.0); + assert!( + !output.status.success(), + "a -10em translate on a 96px-font node must push the shape off-frame; report={report_json}" + ); + let violation = find_kind(&report_json, "viewport_overflow") + .expect("expected a viewport_overflow violation"); + let x = violation["bbox"]["x"].as_f64().expect("bbox.x is a number"); + assert!( + (x - (-760.0)).abs() < 2.0, + "expected bbox.x ~ -760 (200 - 10*96), got {x}: {report_json}" + ); +} + +/// `translateY(50%)` on a WIDE, SHORT box (1000×100) must resolve against +/// its OWN height (50px), not `max(width, height)` (500px) — the exact +/// mistake `paint_pass.rs`'s `length_ctx_x`/`length_ctx_y` split exists to +/// avoid. The correct 50px shift keeps the box on-frame; the buggy 500px +/// shift pushes its bottom edge to 1400px, past the 1080px-tall viewport. +#[test] +fn static_translate_percent_resolves_per_axis_not_against_max_of_both() { + let scenario = ScratchFile::new("rm15-percent-scenario"); + let report = ScratchFile::new("rm15-percent-report"); + let json = r##"{ + "video": { "width": 1920, "height": 1080 }, + "scenes": [{ + "duration": 1.0, + "children": [{ + "type": "shape", + "shape": "rect", + "position": "absolute", + "x": 100, "y": 800, + "style": { + "width": "1000px", "height": "100px", + "transform": [{ "fn": "translate-y", "y": "50%" }] + }, + "fill": "#ff0000" + }] + }] + }"##; + std::fs::write(&scenario.0, json).expect("write scenario"); + + let output = run_validate(&scenario.0, Some(&report.0), false, false); + let report_json = read_report(&report.0); + assert!( + output.status.success(), + "a 50% translateY on a 1000x100 box must resolve against its own 100px height \ + (50px shift, still on-frame), not max(1000,100); report={report_json}" + ); + assert_eq!( + count_kind(&report_json, "viewport_overflow"), + 0, + "{report_json}" + ); +}