From fa417797f2fe2d99c65158b7b3474067e3612b5f Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Tue, 22 Sep 2026 10:59:05 +0200 Subject: [PATCH] fix(geometry): compare terminal height in a single box model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `bbox.h` is `layout.height`, the border box. `LegacyPaintDispatcher::dispatch` treats only `Component::Codeblock(_)` as self-padding (`let is_self_padding = matches!(child.component, Component::Codeblock(_));`); every other painter, terminal included, receives a `BoxLayout` whose height is `layout.content_box()`'s `ch`. So a `terminal` with `auto_scroll: false` and `style.padding: "32px"` has 64px less drawable height than the check credits it with: content needing 500px in a 520px border box (456px content box) overflows by 44px and is reported clean. The codeblock arm just above is correct, because codeblock really is handed the border box — so the two arms of the same violation kind need different boxes and currently use the same one. Refs #220 --- .../rustmotion/src/cli/commands/geometry.rs | 37 ++++--- crates/rustmotion/tests/audit_ws_b.rs | 98 +++++++++++++++++++ 2 files changed, 121 insertions(+), 14 deletions(-) diff --git a/crates/rustmotion/src/cli/commands/geometry.rs b/crates/rustmotion/src/cli/commands/geometry.rs index f90c2b08..22796887 100644 --- a/crates/rustmotion/src/cli/commands/geometry.rs +++ b/crates/rustmotion/src/cli/commands/geometry.rs @@ -307,15 +307,7 @@ fn walk( out, ); } - check_auto_scroll( - &child.component, - &child_path, - &raw_bbox, - viewport, - vi, - si, - out, - ); + check_auto_scroll(&child.component, &child_path, layout, viewport, vi, si, out); // Suppressed under a clipping ancestor (parent_clips) exactly // like check_viewport, and when the node clips its own overflow // (paint_pass applies a node's own `overflow: hidden`/clip/ @@ -1017,10 +1009,20 @@ fn check_content_overflows_box( /// `(None, None)`/`MaxContent` yields each component's natural (unbounded) /// size, exactly like `check_unwrappable_text`/`check_content_overflows_box` /// already do for the text-family intrinsics. +/// +/// the codeblock and terminal arms compare against different boxes, +/// on purpose. `LegacyPaintDispatcher::is_self_padding` matches only +/// `Component::Codeblock` — a codeblock is handed the raw (border) layout +/// box and paints its own padding inside it (`compute_code_dimensions` +/// already bakes `style.padding_px()` into `natural_h`, so comparing against +/// the border box is the byte-for-byte-correct pairing). Every other +/// painter, terminal included, is handed `layout.content_box()` instead — +/// so the terminal arm compares against that, not the border box, or it +/// under-reports by exactly the node's own padding. fn check_auto_scroll( component: &Component, path: &str, - bbox: &BBox, + layout: &BoxLayout, viewport: (u32, u32), vi: usize, si: usize, @@ -1031,6 +1033,7 @@ fn check_auto_scroll( Component::Codeblock(cb) if !cb.auto_scroll => { let (_, natural_h) = CodeblockIntrinsic::from_codeblock(cb).measure((None, None), max_content); + let bbox = bbox_of(layout); if natural_h > bbox.h + 0.5 { out.push(GeometryViolation { view_index: vi, @@ -1039,7 +1042,7 @@ fn check_auto_scroll( component: "codeblock".to_string(), axis: Axis::Y, kind: ViolationKind::AutoScrollDisabledOverflow, - bbox: *bbox, + bbox, viewport, hint: format!( "codeblock content needs ~{:.0}px but box is {:.0}px — enable auto_scroll or shorten code", @@ -1051,7 +1054,8 @@ fn check_auto_scroll( Component::Terminal(t) if !t.auto_scroll => { let (_, natural_h) = TerminalIntrinsic::from_terminal(t).measure((None, None), max_content); - if natural_h > bbox.h + 0.5 { + let (cx, cy, cw, ch) = layout.content_box(); + if natural_h > ch + 0.5 { out.push(GeometryViolation { view_index: vi, scene_index: si, @@ -1059,11 +1063,16 @@ fn check_auto_scroll( component: "terminal".to_string(), axis: Axis::Y, kind: ViolationKind::AutoScrollDisabledOverflow, - bbox: *bbox, + bbox: BBox { + x: cx, + y: cy, + w: cw, + h: ch, + }, viewport, hint: format!( "terminal content needs ~{:.0}px but box is {:.0}px — enable auto_scroll or remove lines", - natural_h, bbox.h + natural_h, ch ), }); } diff --git a/crates/rustmotion/tests/audit_ws_b.rs b/crates/rustmotion/tests/audit_ws_b.rs index c9c53469..0b13a3e5 100644 --- a/crates/rustmotion/tests/audit_ws_b.rs +++ b/crates/rustmotion/tests/audit_ws_b.rs @@ -446,3 +446,101 @@ fn nowrap_text_taller_than_its_box_is_still_flagged() { .expect("expected a content_overflows_box violation"); assert_eq!(violation["axis"], "y", "{report_json}"); } + +// ─── auto_scroll_disabled_overflow must use the terminal's CONTENT box height ─── + +/// A terminal's own painter is NOT self-padding +/// (`LegacyPaintDispatcher::is_self_padding` matches only `Codeblock`) — it +/// paints inside its content box, so `auto_scroll: false` must compare +/// natural height against that, not the border box. 10 lines ≈ 288px +/// natural height (36px chrome + 32px internal terminal padding + 10×22px +/// lines at the default 14px font); border box height is 400px, content box +/// height is 400 - 300 (150px top+bottom CSS padding) = 100px. +#[test] +fn terminal_auto_scroll_disabled_overflow_is_measured_against_the_content_box() { + let scenario = ScratchFile::new("rm33-scenario"); + let report = ScratchFile::new("rm33-report"); + let lines: String = (1..=10) + .map(|i| format!(r##"{{ "text": "line {i}" }}"##)) + .collect::>() + .join(","); + let json = format!( + r##"{{ + "video": {{ "width": 1920, "height": 1080 }}, + "scenes": [{{ + "duration": 1.0, + "children": [{{ + "type": "terminal", + "lines": [{lines}], + "auto_scroll": false, + "position": "absolute", + "x": 50, "y": 50, + "style": {{ + "width": "800px", "height": "400px", + "padding": {{ "top": "150px", "bottom": "150px" }} + }} + }}] + }}] + }}"## + ); + 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(), + "~288px of natural content is far past a 100px content box (400px border box minus \ + 300px of padding); report={report_json}" + ); + let violation = find_kind(&report_json, "auto_scroll_disabled_overflow") + .expect("expected an auto_scroll_disabled_overflow violation"); + let height = violation["bbox"]["h"].as_f64().expect("bbox.h is a number"); + assert!( + (height - 100.0).abs() < 1.0, + "violation bbox should be the 100px CONTENT box, not the 400px border box: {report_json}" + ); +} + +/// Negative control: a codeblock genuinely IS self-padding +/// (`LegacyPaintDispatcher::is_self_padding`), so its own natural-height +/// formula already bakes its padding in — the codeblock arm must keep +/// comparing against the BORDER box, unaffected by this fix. Same 60px +/// padding fixture as the pre-existing +/// `codeblock_auto_scroll_check_honours_explicit_padding_not_a_hardcoded_16px` +/// internal test, driven through the CLI instead. +#[test] +fn codeblock_auto_scroll_disabled_overflow_still_uses_the_border_box() { + let scenario = ScratchFile::new("rm33-codeblock-scenario"); + let report = ScratchFile::new("rm33-codeblock-report"); + let code_lines: String = (1..=10) + .map(|i| i.to_string()) + .collect::>() + .join("\\n"); + let json = format!( + r##"{{ + "video": {{ "width": 1920, "height": 1080 }}, + "scenes": [{{ + "duration": 1.0, + "children": [{{ + "type": "codeblock", + "code": "{code_lines}", + "auto_scroll": false, + "style": {{ "width": "600px", "height": "250px", "padding": "60px" }} + }}] + }}] + }}"## + ); + 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(), "report={report_json}"); + let violation = find_kind(&report_json, "auto_scroll_disabled_overflow") + .expect("expected an auto_scroll_disabled_overflow violation"); + let height = violation["bbox"]["h"].as_f64().expect("bbox.h is a number"); + assert!( + (height - 250.0).abs() < 1.0, + "codeblock is self-padding: the reported bbox must stay the 250px BORDER box, \ + not a content box: {report_json}" + ); +}