fix(geometry): check height overflow when white-space is nowrap

crates/rustmotion/src/cli/commands/geometry.rs

@@ -300,7 +300,7 @@ fn walk(
                 check_unwrappable_text(
                     &child.component,
                     &child_path,
-                    &raw_bbox,
+                    layout,
                     viewport,
                     vi,
                     si,
@@ -791,10 +791,32 @@ fn measurer_and_nowrap(component: &Component) -> Option<(Box<dyn IntrinsicMeasur
     }
 }
 
+/// natural (unwrapped) width vs the node's own CONTENT box, not its
+/// border box. `LegacyPaintDispatcher::dispatch` hands every non-codeblock
+/// painter (`Text`/`GradientText`/`Caption` included) a synthetic
+/// `BoxLayout` built from `layout.content_box()`, translated to the
+/// content-box origin — so the painter wraps and draws inside the content
+/// box, not the raw taffy layout box this walker reads. Comparing against
+/// the border box (as this used to) under-reports by exactly
+/// `padding.left + padding.right + border.left + border.right`, mirroring
+/// the same fix `check_content_overflows_box` already applies for the
+/// wrapped case.
+///
+/// Measured via the same cosmic-text–backed intrinsic the layout engine
+/// uses. Width is bounded by the node's own resolved content-box width
+/// (not `MaxContent`) so a `text-autofit: true` node can shrink to fit it —
+/// see `measurer_and_nowrap`'s `TextIntrinsic`/`GradientTextIntrinsic` arms
+/// and `CssStyle::text_autofit`'s doc comment. For a non-autofit node this
+/// changes nothing: `TextIntrinsic::measure` only reads the width
+/// constraint at all when `text_autofit` is on (see its early return), and
+/// `nowrap` already forces a single unwrapped line here regardless of what
+/// width is offered — so `natural_w` below is "natural" in the non-autofit
+/// case exactly as before, and "shrunk to fit, if that's enough" when the
+/// author declared it.
 fn check_unwrappable_text(
     component: &Component,
     path: &str,
-    bbox: &BBox,
+    layout: &BoxLayout,
     viewport: (u32, u32),
     vi: usize,
     si: usize,
@@ -806,22 +828,12 @@ fn check_unwrappable_text(
     if !nowrap {
         return;
     }
-    // Measure via the same cosmic-text–backed intrinsic the layout engine
-    // uses. Width is bounded by the node's own resolved `bbox.w` (not
-    // `MaxContent`) so a `text-autofit: true` node can shrink to fit it —
-    // see `measurer_and_nowrap`'s `TextIntrinsic`/`GradientTextIntrinsic`
-    // arms and `CssStyle::text_autofit`'s doc comment. For a non-autofit
-    // node this changes nothing: `TextIntrinsic::measure` only reads the
-    // width constraint at all when `text_autofit` is on (see its early
-    // return), and `nowrap` already forces a single unwrapped line here
-    // regardless of what width is offered — so `natural_w` below is
-    // "natural" in the non-autofit case exactly as before, and "shrunk to
-    // fit, if that's enough" when the author declared it.
+    let (cx, cy, cw, ch) = layout.content_box();
     let (natural_w, _) = intrinsic.measure(
         (None, None),
-        (AvailableSpace::Definite(bbox.w), AvailableSpace::MaxContent),
+        (AvailableSpace::Definite(cw), AvailableSpace::MaxContent),
     );
-    if natural_w > bbox.w + 0.5 {
+    if natural_w > cw + 0.5 {
         let kind = component_kind(component);
         out.push(GeometryViolation {
             view_index: vi,
@@ -830,11 +842,16 @@ fn check_unwrappable_text(
             component: kind.to_string(),
             axis: Axis::X,
             kind: ViolationKind::UnwrappableTextOverflow,
-            bbox: *bbox,
+            bbox: BBox {
+                x: cx,
+                y: cy,
+                w: cw,
+                h: ch,
+            },
             viewport,
             hint: format!(
                 "{kind} natural width is {natural_w:.0}px but only {:.0}px available — remove style.white-space: nowrap (or set it to normal) so it can wrap, or reduce style.font-size",
-                bbox.w
+                cw
             ),
         });
     }
@@ -856,14 +873,24 @@ fn check_unwrappable_text(
 /// (`codeblock`/`terminal` are deliberately excluded: their `auto_scroll`
 /// escape hatch makes a smaller-than-natural box intentional).
 ///
-/// Complementary to `check_unwrappable_text`, not overlapping with it:
-/// that one covers `white-space: nowrap`/`pre` (single unwrapped line, width
-/// only, measured at natural/unconstrained width). This one covers the
-/// default wrapping case — measured at the width the box actually *has*
-/// (`content_box().2`, unconstrained height) so it also catches a single
-/// unbreakable word/token/URL that's wider than the box even though wrap is
-/// on (wrapping can't break within a word), plus the width axis stays
-/// consistent with what will actually be painted.
+/// Complementary to `check_unwrappable_text`, not overlapping with it on the
+/// WIDTH axis: that one covers `white-space: nowrap`/`pre` (single unwrapped
+/// line, measured at natural/unconstrained width). This function covers the
+/// default wrapping case's width — measured at the width the box actually
+/// *has* (`content_box().2`, unconstrained height) so it also catches a
+/// single unbreakable word/token/URL that's wider than the box even though
+/// wrap is on (wrapping can't break within a word), plus the width axis
+/// stays consistent with what will actually be painted.
+///
+/// the HEIGHT axis is this function's job regardless of `nowrap` — a
+/// nowrap node used to return here before measuring height at all, so a
+/// single unwrapped line taller than its box validated clean. Re-measuring
+/// nowrap's WIDTH at a constrained space would wrap text that actually
+/// paints as one (too-wide) line, which is exactly why `check_unwrappable_
+/// text` owns that axis instead — but a single line's height is exactly one
+/// `line_height`, independent of any width constraint, so it's measured at
+/// `(MaxContent, Definite(ch))` and reported on `Axis::Y` only, leaving
+/// `Axis::X` to `check_unwrappable_text`.
 fn check_content_overflows_box(
     component: &Component,
     path: &str,
@@ -876,19 +903,44 @@ fn check_content_overflows_box(
     let Some((intrinsic, nowrap)) = measurer_and_nowrap(component) else {
         return;
     };
-    // nowrap/pre is check_unwrappable_text's territory: re-measuring it
-    // here at a constrained width would wrap text that will actually
-    // paint as one (too-wide) line, producing a height number that
-    // doesn't correspond to anything that gets painted.
-    if nowrap {
-        return;
-    }
 
     let (cx, cy, cw, ch) = layout.content_box();
     if cw <= 0.0 || ch <= 0.0 {
         return;
     }
 
+    if nowrap {
+        let (_, natural_h) = intrinsic.measure(
+            (None, None),
+            (AvailableSpace::MaxContent, AvailableSpace::Definite(ch)),
+        );
+        let eps = 0.5;
+        if natural_h <= ch + eps {
+            return;
+        }
+        let kind = component_kind(component);
+        out.push(GeometryViolation {
+            view_index: vi,
+            scene_index: si,
+            path: path.to_string(),
+            component: kind.to_string(),
+            axis: Axis::Y,
+            kind: ViolationKind::ContentOverflowsBox,
+            bbox: BBox {
+                x: cx,
+                y: cy,
+                w: cw,
+                h: ch,
+            },
+            viewport,
+            hint: format!(
+                "{kind} line is {natural_h:.0}px tall but its box is only {:.0}px tall — increase style.height (or the parent's), or reduce style.font-size",
+                ch
+            ),
+        });
+        return;
+    }
+
     // Height is bounded by the node's own resolved content-box height `ch`
     // (not `MaxContent`) for the same reason width is bounded by `cw`: a
     // `text-autofit: true` node can only try to shrink into a target it's

crates/rustmotion/src/cli/commands/validate.rs

@@ -108,6 +108,25 @@ fn refuse_fix(input: &Path, raw_source: &str) -> Option<FixRefusal> {
     None
 }
 
+/// The JSON `--fix` writes back, sourced independently of
+/// `LoadedScenario::raw`.
+///
+/// `loaded.raw` is captured *after* `rustmotion::assets::rebase_relative_paths`
+/// runs (`validation.rs`), which rewrites every `src`/`track` naming an
+/// existing file next to the scenario into a canonicalised ABSOLUTE path —
+/// serialising it back would silently replace `"assets/logo.png"` with
+/// this machine's own absolute path, which resolves nowhere else. By the
+/// time `refuse_fix` has let a file reach this function, it carries no
+/// `config`/`$var`/`include`/`for-each`/`use`, so variable substitution and
+/// directive expansion are no-ops on it too — parsing `raw_source` (the
+/// exact bytes still on disk) fresh yields the identical tree
+/// `apply_fixes`/`navigate`'s path indices were computed against, minus the
+/// rebase.
+fn fixable_source(raw_source: &str) -> Result<serde_json::Value> {
+    serde_json::from_str(raw_source)
+        .map_err(|e| RustmotionError::Generic(format!("re-parse source for --fix: {}", e)))
+}
+
 pub fn cmd_validate(
     input: &PathBuf,
     report: Option<&Path>,
@@ -142,7 +161,7 @@ pub fn cmd_validate(
         if let Some(refusal) = refuse_fix(input, &raw_source) {
             return Err(RustmotionError::Generic(refusal.explain(input)));
         }
-        let mut json_value = loaded.raw.clone();
+        let mut json_value = fixable_source(&raw_source)?;
         applied_fixes = apply_fixes(&mut json_value, &report_out.geom_violations);
         if applied_fixes > 0 {
             let pretty = serde_json::to_string_pretty(&json_value)

crates/rustmotion/tests/audit_ws_b.rs

@@ -282,3 +282,167 @@ fn static_translate_percent_resolves_per_axis_not_against_max_of_both() {
         "{report_json}"
     );
 }
+
+// ─── --fix must not bake in machine-absolute asset paths ───────────
+
+/// `--fix` used to serialise `LoadedScenario::raw`, captured AFTER
+/// `rustmotion::assets::rebase_relative_paths` rewrites every existing-file
+/// `src`/`track` into a canonicalised ABSOLUTE path — so fixing an
+/// unrelated violation (here, a too-wide nowrap text) silently replaced
+/// `"assets/logo.png"` with this machine's own absolute path. The asset
+/// file only needs to EXIST (rebasing is gated on `Path::is_file()`); its
+/// content is irrelevant here since geometry validation never decodes it
+/// (`Image` uses a fixed 400×300 default intrinsic size, not real pixel
+/// dimensions).
+#[test]
+fn fix_leaves_relative_asset_paths_untouched() {
+    let dir = ScratchDir::new("rm16");
+    std::fs::create_dir_all(dir.0.join("assets")).expect("mkdir assets");
+    std::fs::write(
+        dir.0.join("assets/logo.png"),
+        b"not a real png, just needs to exist",
+    )
+    .expect("write asset");
+    let scenario_path = dir.0.join("scenario.json");
+    let json = r##"{
+        "video": { "width": 1920, "height": 4000 },
+        "scenes": [{
+            "duration": 1.0,
+            "children": [
+                {
+                    "type": "image",
+                    "src": "assets/logo.png",
+                    "style": { "width": "200px", "height": "150px" }
+                },
+                {
+                    "type": "text",
+                    "content": "This is a fairly long sentence with several short words that will wrap nicely across many lines without any single word being too wide for the box.",
+                    "style": {
+                        "width": "300px", "height": "2000px",
+                        "color": "#ffffff", "font-size": "32px", "white-space": "nowrap"
+                    }
+                }
+            ]
+        }]
+    }"##;
+    std::fs::write(&scenario_path, json).expect("write scenario");
+
+    let output = run_validate(&scenario_path, None, /*fix=*/ true, false);
+    assert!(
+        output.status.success(),
+        "the only violation (the nowrap text) is fixed in place, so this run should now \
+         validate clean; stdout={} stderr={}",
+        String::from_utf8_lossy(&output.stdout),
+        String::from_utf8_lossy(&output.stderr)
+    );
+
+    let fixed: serde_json::Value = serde_json::from_str(
+        &std::fs::read_to_string(&scenario_path).expect("read fixed scenario"),
+    )
+    .expect("fixed scenario is valid JSON");
+    let src = fixed["scenes"][0]["children"][0]["src"]
+        .as_str()
+        .expect("image src is a string");
+    assert_eq!(
+        src, "assets/logo.png",
+        "the image src must stay exactly as authored, not rewritten to an absolute path: {fixed}"
+    );
+
+    let text_style = &fixed["scenes"][0]["children"][1]["style"];
+    assert!(
+        text_style.get("white-space").is_none(),
+        "the actual violation --fix targeted must still be fixed: {fixed}"
+    );
+}
+
+// ─── unwrappable_text_overflow must measure the CONTENT box ────────
+
+/// A nowrap text's own painter draws inside its CONTENT box
+/// (`LegacyPaintDispatcher` hands it `layout.content_box()`, not the raw
+/// layout box, for every component except `codeblock`) — so the geometry
+/// check must compare the natural line width against the content box too.
+/// Content box width here is 2000 - 1900 = 100px (950px of padding on each
+/// side); the border box is 2000px. Any real natural width for this
+/// string/font-size sits comfortably in between, so the violation fires if
+/// and only if the content box is used.
+#[test]
+fn unwrappable_text_overflow_is_measured_against_the_content_box() {
+    let scenario = ScratchFile::new("rm31-scenario");
+    let report = ScratchFile::new("rm31-report");
+    let json = r##"{
+        "video": { "width": 2400, "height": 1080 },
+        "scenes": [{
+            "duration": 1.0,
+            "children": [{
+                "type": "text",
+                "content": "Hello World Example",
+                "position": "absolute",
+                "x": 50, "y": 50,
+                "style": {
+                    "width": "2000px", "height": "300px",
+                    "padding": { "top": "20px", "right": "950px", "bottom": "20px", "left": "950px" },
+                    "white-space": "nowrap",
+                    "font-size": "48px",
+                    "color": "#ffffff"
+                }
+            }]
+        }]
+    }"##;
+    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(),
+        "the 100px content box (2000px border box minus 1900px of padding) is too narrow \
+         for this nowrap line; report={report_json}"
+    );
+    let violation = find_kind(&report_json, "unwrappable_text_overflow")
+        .expect("expected an unwrappable_text_overflow violation");
+    let width = violation["bbox"]["w"].as_f64().expect("bbox.w is a number");
+    assert!(
+        (width - 100.0).abs() < 1.0,
+        "violation bbox should be the 100px CONTENT box, not the 2000px border box: {report_json}"
+    );
+}
+
+// ─── white-space: nowrap must not silence the height check ─────────
+
+/// A single unwrapped 120px-font line is ~144px tall, well past a 40px-tall
+/// box — the exact case `content_overflows_box` already catches for
+/// wrapping text. `white-space: nowrap` used to return before measuring
+/// height at all, so this validated clean.
+#[test]
+fn nowrap_text_taller_than_its_box_is_still_flagged() {
+    let scenario = ScratchFile::new("rm32-scenario");
+    let report = ScratchFile::new("rm32-report");
+    let json = r##"{
+        "video": { "width": 1920, "height": 1080 },
+        "scenes": [{
+            "duration": 1.0,
+            "children": [{
+                "type": "text",
+                "content": "Hi",
+                "position": "absolute",
+                "x": 50, "y": 50,
+                "style": {
+                    "width": "500px", "height": "40px",
+                    "white-space": "nowrap",
+                    "font-size": "120px",
+                    "color": "#ffffff"
+                }
+            }]
+        }]
+    }"##;
+    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 120px-font single line is far taller than a 40px box; report={report_json}"
+    );
+    let violation = find_kind(&report_json, "content_overflows_box")
+        .expect("expected a content_overflows_box violation");
+    assert_eq!(violation["axis"], "y", "{report_json}");
+}