Skip to content

Commit a252cd5

Browse files
committed
fix(geometry): measure text overflow against the content box
`bbox` here is `raw_bbox = bbox_of(layout)` (geometry.rs:272, 376-383), i.e. `layout.width` — the BORDER box. But `LegacyPaintDispatcher::dispatch` (crates/rustmotion-components/src/legacy_dispatch.rs) hands every non-codeblock painter a synthetic `BoxLayout { width: cw, height: ch, border/padding: zero }` taken from `layout.content_box()` and translates the canvas to the content-box origin. So `Text::paint` wraps and draws inside `cw`, not `layout.width`. A `white-space: nowrap` text with `style.padding: "0 24px"` in a 300px box has `cw = 252`; a natural line width of 280px paints 28px past the box's right edge, yet `280 > 300` is false and nothing is reported. The sibling check `check_content_overflows_box` (geometry.rs:848) does this correctly via `layout.content_box()` — the two checks disagree about which box text lives in, and the flagship `unwrappable_text_overflow` is the one that is wrong. The gap equals `padding.left + padding.right + border.left + border.right`. Refs #220
1 parent 0e47992 commit a252cd5

2 files changed

Lines changed: 85 additions & 17 deletions

File tree

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

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ fn walk(
300300
check_unwrappable_text(
301301
&child.component,
302302
&child_path,
303-
&raw_bbox,
303+
layout,
304304
viewport,
305305
vi,
306306
si,
@@ -791,10 +791,32 @@ fn measurer_and_nowrap(component: &Component) -> Option<(Box<dyn IntrinsicMeasur
791791
}
792792
}
793793

794+
/// natural (unwrapped) width vs the node's own CONTENT box, not its
795+
/// border box. `LegacyPaintDispatcher::dispatch` hands every non-codeblock
796+
/// painter (`Text`/`GradientText`/`Caption` included) a synthetic
797+
/// `BoxLayout` built from `layout.content_box()`, translated to the
798+
/// content-box origin — so the painter wraps and draws inside the content
799+
/// box, not the raw taffy layout box this walker reads. Comparing against
800+
/// the border box (as this used to) under-reports by exactly
801+
/// `padding.left + padding.right + border.left + border.right`, mirroring
802+
/// the same fix `check_content_overflows_box` already applies for the
803+
/// wrapped case.
804+
///
805+
/// Measured via the same cosmic-text–backed intrinsic the layout engine
806+
/// uses. Width is bounded by the node's own resolved content-box width
807+
/// (not `MaxContent`) so a `text-autofit: true` node can shrink to fit it —
808+
/// see `measurer_and_nowrap`'s `TextIntrinsic`/`GradientTextIntrinsic` arms
809+
/// and `CssStyle::text_autofit`'s doc comment. For a non-autofit node this
810+
/// changes nothing: `TextIntrinsic::measure` only reads the width
811+
/// constraint at all when `text_autofit` is on (see its early return), and
812+
/// `nowrap` already forces a single unwrapped line here regardless of what
813+
/// width is offered — so `natural_w` below is "natural" in the non-autofit
814+
/// case exactly as before, and "shrunk to fit, if that's enough" when the
815+
/// author declared it.
794816
fn check_unwrappable_text(
795817
component: &Component,
796818
path: &str,
797-
bbox: &BBox,
819+
layout: &BoxLayout,
798820
viewport: (u32, u32),
799821
vi: usize,
800822
si: usize,
@@ -806,22 +828,12 @@ fn check_unwrappable_text(
806828
if !nowrap {
807829
return;
808830
}
809-
// Measure via the same cosmic-text–backed intrinsic the layout engine
810-
// uses. Width is bounded by the node's own resolved `bbox.w` (not
811-
// `MaxContent`) so a `text-autofit: true` node can shrink to fit it —
812-
// see `measurer_and_nowrap`'s `TextIntrinsic`/`GradientTextIntrinsic`
813-
// arms and `CssStyle::text_autofit`'s doc comment. For a non-autofit
814-
// node this changes nothing: `TextIntrinsic::measure` only reads the
815-
// width constraint at all when `text_autofit` is on (see its early
816-
// return), and `nowrap` already forces a single unwrapped line here
817-
// regardless of what width is offered — so `natural_w` below is
818-
// "natural" in the non-autofit case exactly as before, and "shrunk to
819-
// fit, if that's enough" when the author declared it.
831+
let (cx, cy, cw, ch) = layout.content_box();
820832
let (natural_w, _) = intrinsic.measure(
821833
(None, None),
822-
(AvailableSpace::Definite(bbox.w), AvailableSpace::MaxContent),
834+
(AvailableSpace::Definite(cw), AvailableSpace::MaxContent),
823835
);
824-
if natural_w > bbox.w + 0.5 {
836+
if natural_w > cw + 0.5 {
825837
let kind = component_kind(component);
826838
out.push(GeometryViolation {
827839
view_index: vi,
@@ -830,11 +842,16 @@ fn check_unwrappable_text(
830842
component: kind.to_string(),
831843
axis: Axis::X,
832844
kind: ViolationKind::UnwrappableTextOverflow,
833-
bbox: *bbox,
845+
bbox: BBox {
846+
x: cx,
847+
y: cy,
848+
w: cw,
849+
h: ch,
850+
},
834851
viewport,
835852
hint: format!(
836853
"{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",
837-
bbox.w
854+
cw
838855
),
839856
});
840857
}

‎crates/rustmotion/tests/audit_ws_b.rs‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,3 +354,54 @@ fn fix_leaves_relative_asset_paths_untouched() {
354354
"the actual violation --fix targeted must still be fixed: {fixed}"
355355
);
356356
}
357+
358+
// ─── unwrappable_text_overflow must measure the CONTENT box ────────
359+
360+
/// A nowrap text's own painter draws inside its CONTENT box
361+
/// (`LegacyPaintDispatcher` hands it `layout.content_box()`, not the raw
362+
/// layout box, for every component except `codeblock`) — so the geometry
363+
/// check must compare the natural line width against the content box too.
364+
/// Content box width here is 2000 - 1900 = 100px (950px of padding on each
365+
/// side); the border box is 2000px. Any real natural width for this
366+
/// string/font-size sits comfortably in between, so the violation fires if
367+
/// and only if the content box is used.
368+
#[test]
369+
fn unwrappable_text_overflow_is_measured_against_the_content_box() {
370+
let scenario = ScratchFile::new("rm31-scenario");
371+
let report = ScratchFile::new("rm31-report");
372+
let json = r##"{
373+
"video": { "width": 2400, "height": 1080 },
374+
"scenes": [{
375+
"duration": 1.0,
376+
"children": [{
377+
"type": "text",
378+
"content": "Hello World Example",
379+
"position": "absolute",
380+
"x": 50, "y": 50,
381+
"style": {
382+
"width": "2000px", "height": "300px",
383+
"padding": { "top": "20px", "right": "950px", "bottom": "20px", "left": "950px" },
384+
"white-space": "nowrap",
385+
"font-size": "48px",
386+
"color": "#ffffff"
387+
}
388+
}]
389+
}]
390+
}"##;
391+
std::fs::write(&scenario.0, json).expect("write scenario");
392+
393+
let output = run_validate(&scenario.0, Some(&report.0), false, false);
394+
let report_json = read_report(&report.0);
395+
assert!(
396+
!output.status.success(),
397+
"the 100px content box (2000px border box minus 1900px of padding) is too narrow \
398+
for this nowrap line; report={report_json}"
399+
);
400+
let violation = find_kind(&report_json, "unwrappable_text_overflow")
401+
.expect("expected an unwrappable_text_overflow violation");
402+
let width = violation["bbox"]["w"].as_f64().expect("bbox.w is a number");
403+
assert!(
404+
(width - 100.0).abs() < 1.0,
405+
"violation bbox should be the 100px CONTENT box, not the 2000px border box: {report_json}"
406+
);
407+
}

0 commit comments

Comments
 (0)