Skip to content

fix(text): honour letter-spacing when breaking lines #132

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
LeadcodeDev merged 1 commit into from
Aug 2, 2026
Merged

fix(text): honour letter-spacing when breaking lines #132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 55 additions & 9 deletions crates/rustmotion-components/src/gradient_text.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rustmotion_core::engine::animator::AnimatedProperties;
use rustmotion_core::engine::layout_pass::BoxLayout;
use rustmotion_core::engine::renderer::{
draw_text_with_fallback, emoji_typeface, measure_text_with_fallback, paint_from_hex,
parse_hex_color, typeface_with_fallback, wrap_text_with_fallback,
parse_hex_color, typeface_with_fallback, wrap_text_with_tracking,
};
use rustmotion_core::schema::TimelineStep;
use rustmotion_core::traits::{PaintCtx, Painter, TimingConfig};
Expand Down Expand Up @@ -83,16 +83,38 @@ impl GradientText {
}

impl GradientText {
fn paint(&self, canvas: &Canvas, layout_width: f32, time: f64) {
fn paint(&self, canvas: &Canvas, layout_width: f32, time: f64, ctx: &PaintCtx) {
if self.content.is_empty() || self.colors.is_empty() {
return;
}

let Some((font, emoji_font)) = self.resolve_font() else {
return;
};
// `font-size` stays context-free — see the identical note in
// `text.rs`'s `paint`: resolving its own `em`/`%` correctly needs a
// cascade.rs change (parent font-size as a resolved px value, not a
// raw `Length`), which is out of scope here (issue #125 §2).
let font_size = self.style.font_size_px_or(48.0);
let line_height_val = self.style.line_height_for(font_size);
// `letter-spacing`/`line-height`'s `em`/`%` are relative to this
// element's own font-size, no cascade dependency — a real
// `LengthContext` is available here, so use the context-aware
// resolvers (issue #125 §2: correctly handles `vw`/`vh`/`rem`/
// line-height-`%`). `letter_spacing` itself: this component never
// read `style.letter-spacing` before this fix — the wrap/measure/
// draw calls below always tracked at 0.0 regardless of what was
// set, which is the same class of measure/paint disagreement issue
// #125 §1 describes elsewhere. It's threaded through consistently
// now.
let type_ctx = rustmotion_core::css::units::LengthContext {
viewport_width: ctx.video_width as f32,
viewport_height: ctx.video_height as f32,
parent_size: layout_width.max(0.0),
font_size,
root_font_size: 16.0,
};
let line_height_val = self.style.line_height_for_ctx(font_size, &type_ctx);
let letter_spacing = self.style.letter_spacing_px_ctx(&type_ctx);

// M1: `white-space: nowrap|pre` keeps the whole content on one line
// even past `layout_width` (it bleeds); anything else word-wraps at
Expand All @@ -108,7 +130,10 @@ impl GradientText {
} else {
None
};
let lines = wrap_text_with_fallback(&self.content, &font, &emoji_font, wrap_at);
// Tracking-aware wrap (issue #125 §1), consistent with the
// real-tracking measurement/draw below.
let lines =
wrap_text_with_tracking(&self.content, &font, &emoji_font, wrap_at, letter_spacing);

// Measure the overall (possibly multi-line) bounding box. The
// gradient is defined once across this whole block rather than
Expand All @@ -119,7 +144,7 @@ impl GradientText {
let descent = metrics.descent;
let text_w = lines
.iter()
.map(|l| measure_text_with_fallback(l, &font, &emoji_font, 0.0))
.map(|l| measure_text_with_fallback(l, &font, &emoji_font, letter_spacing))
.fold(0.0f32, f32::max);
let text_h = (lines.len().max(1) - 1) as f32 * line_height_val + ascent + descent;

Expand Down Expand Up @@ -185,7 +210,16 @@ impl GradientText {
continue;
}
let y = i as f32 * line_height_val + ascent;
draw_text_with_fallback(canvas, line, &font, &emoji_font, 0.0, 0.0, y, &fill_paint);
draw_text_with_fallback(
canvas,
line,
&font,
&emoji_font,
letter_spacing,
0.0,
y,
&fill_paint,
);
}
}
}
Expand All @@ -198,7 +232,7 @@ impl Painter for GradientText {
_props: &AnimatedProperties,
ctx: &PaintCtx,
) {
self.paint(canvas, layout.width, ctx.time);
self.paint(canvas, layout.width, ctx.time, ctx);
}
}

Expand Down Expand Up @@ -248,6 +282,18 @@ mod tests {
.collect()
}

fn test_ctx() -> PaintCtx {
PaintCtx {
time: 0.0,
scene_duration: 1.0,
frame_index: 0,
fps: 30,
video_width: 1920,
video_height: 1080,
stagger_offset: 0.0,
}
}

fn has_ink_in(grid: &[u8], surface_width: i32, x0: i32, x1: i32, y0: i32, y1: i32) -> bool {
for y in y0..y1 {
for x in x0..x1 {
Expand All @@ -271,7 +317,7 @@ mod tests {
const H: i32 = 200;
let mut surface = skia_safe::surfaces::raster_n32_premul((W, H)).expect("raster surface");
let canvas = surface.canvas();
gt.paint(canvas, 80.0, 0.0);
gt.paint(canvas, 80.0, 0.0, &test_ctx());
let grid = alpha_grid(&mut surface, W, H);

assert!(
Expand All @@ -291,7 +337,7 @@ mod tests {
const H: i32 = 200;
let mut surface = skia_safe::surfaces::raster_n32_premul((W, H)).expect("raster surface");
let canvas = surface.canvas();
gt.paint(canvas, 80.0, 0.0);
gt.paint(canvas, 80.0, 0.0, &test_ctx());
let grid = alpha_grid(&mut surface, W, H);

assert!(
Expand Down
26 changes: 24 additions & 2 deletions crates/rustmotion-components/src/intrinsic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rustmotion_core::css::style::{
use rustmotion_core::engine::box_tree::{AvailableSpace, IntrinsicMeasure};
use rustmotion_core::engine::renderer::{
emoji_typeface, format_counter_value, measure_text_with_fallback, typeface_with_fallback,
wrap_text_with_fallback,
wrap_text_with_tracking,
};

use crate::badge::{Badge, BadgeSize};
Expand Down Expand Up @@ -58,6 +58,18 @@ impl TextIntrinsic {
/// wrap:true unconditionally so their measured size still matches what
/// those painters actually draw.
pub fn from_parts(content: &str, style: &CssStyle, max_width: Option<f32>) -> Self {
// No `LengthContext` is reachable here without changing this
// constructor's signature — its only callers are `box_builder.rs`
// and `rustmotion-cli/src/commands/geometry.rs`, both outside this
// workstream's scope (box_builder.rs is a sibling's live file this
// wave; the geometry validator re-measures via this exact type and
// must keep agreeing with it byte-for-byte, so changing what it
// needs to pass in is not a call to make unilaterally here). So
// `font_size`/`line_height` stay on the context-free accessors
// (issue #125 §2's `vw`/`vh`/`rem`/`%` gap is not closed for this
// constructor) — only `letter_spacing` below, which is used
// exclusively by the wrap fix in `measure()`, no signature change
// needed for it.
let font_size = style.font_size_px_or(48.0);
let line_height_resolved = style.line_height_for(font_size);
Self {
Expand Down Expand Up @@ -115,7 +127,17 @@ impl IntrinsicMeasure for TextIntrinsic {
let emoji_font = emoji_typeface().map(|tf| Font::from_typeface(tf, self.font_size));

let wrap_at = if self.wrap { max_width } else { None };
let lines = wrap_text_with_fallback(&self.content, &font, &emoji_font, wrap_at);
// Tracking-aware wrap (issue #125 §1): matches the real
// `letter_spacing` used to measure each line's width just below, so
// the box this measurer reserves and what `Text::paint` (also fixed,
// same tracking) actually paints agree on line count.
let lines = wrap_text_with_tracking(
&self.content,
&font,
&emoji_font,
wrap_at,
self.letter_spacing,
);

let mut max_w = 0.0f32;
for line in &lines {
Expand Down
16 changes: 14 additions & 2 deletions crates/rustmotion-components/src/shape.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustmotion_core::engine::animator::AnimatedProperties;
use rustmotion_core::engine::layout_pass::BoxLayout;
use rustmotion_core::engine::renderer::{
build_shape_path, color4f_from_hex, draw_shape_path, draw_text_with_fallback, emoji_typeface,
measure_text_with_fallback, paint_from_hex, typeface_with_fallback, wrap_text_with_fallback,
measure_text_with_fallback, paint_from_hex, typeface_with_fallback, wrap_text_with_tracking,
};
use rustmotion_core::schema::{
Fill, FontWeight, GradientType, ShapeText, ShapeType, Stroke, TextAlign, TimelineStep,
Expand Down Expand Up @@ -187,7 +187,19 @@ fn render_shape_text(
};
let letter_spacing = text.letter_spacing.unwrap_or(0.0);

let lines = wrap_text_with_fallback(&text.content, &font, &emoji_font, Some(area_w));
// Tracking-aware wrap (issue #125 §1): the fit test measures with the
// same `letter_spacing` used below for `line_width`/`draw_text_with_
// fallback`, so the wrap decision agrees with what's actually painted.
// `ShapeText`'s `font_size`/`letter_spacing`/`line_height` are plain
// `f32` (not `CssStyle`/`Length`), so issue #125 §2's relative-unit gap
// doesn't apply here — there is no unit string to resolve.
let lines = wrap_text_with_tracking(
&text.content,
&font,
&emoji_font,
Some(area_w),
letter_spacing,
);
let descent = metrics.descent;
let total_h = if lines.len() > 1 {
(lines.len() - 1) as f32 * line_height + ascent + descent
Expand Down
43 changes: 31 additions & 12 deletions crates/rustmotion-components/src/text.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rustmotion_core::engine::animator::AnimatedProperties;
use rustmotion_core::engine::layout_pass::BoxLayout;
use rustmotion_core::engine::renderer::{
draw_text_with_fallback, emoji_typeface, measure_text_with_fallback, paint_from_hex,
typeface_with_fallback, wrap_text_with_fallback,
typeface_with_fallback, wrap_text_with_tracking,
};
use rustmotion_core::schema::{
AnimationEffect, CharAnimPreset, CharAnimation, FontStyleType, FontWeight, Stroke, TextAlign,
Expand Down Expand Up @@ -360,7 +360,27 @@ impl Text {
props: &AnimatedProperties,
ctx: &PaintCtx,
) -> Result<()> {
// `font-size` stays on the context-free accessor: resolving a
// relative unit here correctly (`em`/`%`) would need the parent's
// *actual computed* font-size, which `cascade.rs` doesn't provide
// today (it inherits `font-size` as a raw, unresolved `Length` —
// see the module note on `CssStyle::font_size_px_ctx`, issue #125
// §2). That cascade fix stays out of scope here; `vw`/`vh`/`rem`
// font-size still fall back to 0px with a loud warning via `.px()`.
let font_size = self.style.font_size_px_or(48.0);
// `letter-spacing` and `line-height`'s `em`/`%` are relative to this
// element's *own* (just-resolved) font-size, which has no cascade
// dependency — a real `LengthContext` is available here (real
// viewport dims from `ctx`, `font_size` just above), so these use
// the context-aware resolvers and correctly handle `vw`/`vh`/`rem`/
// line-height-`%` (issue #125 §2).
let type_ctx = rustmotion_core::css::units::LengthContext {
viewport_width: ctx.video_width as f32,
viewport_height: ctx.video_height as f32,
parent_size: layout_width.max(0.0),
font_size,
root_font_size: 16.0,
};
// Animated color (timeline style-state transitions) overrides the
// static style color.
let color = props
Expand All @@ -386,8 +406,8 @@ impl Text {
Some(CssTextAlign::Right | CssTextAlign::End) => TextAlign::Right,
_ => TextAlign::Left,
};
let line_height_val = self.style.line_height_for(font_size);
let letter_spacing = self.style.letter_spacing_px();
let line_height_val = self.style.line_height_for_ctx(font_size, &type_ctx);
let letter_spacing = self.style.letter_spacing_px_ctx(&type_ctx);

let slant = match font_style_type {
FontStyleType::Normal => skia_safe::font_style::Slant::Upright,
Expand Down Expand Up @@ -443,7 +463,13 @@ impl Text {
self.content.clone()
};

let lines = wrap_text_with_fallback(&content, &font, &emoji_font, wrap_width);
// Tracking-aware wrap (issue #125 §1): the fit test now measures
// with this element's real `letter_spacing`, matching the
// measurements below (`align_width`, per-line `advance_width`) that
// already used it — the box this wraps for and the pixels painted
// into it now agree.
let lines =
wrap_text_with_tracking(&content, &font, &emoji_font, wrap_width, letter_spacing);
let (_, metrics) = font.metrics();
let ascent = -metrics.ascent;
let descent = metrics.descent;
Expand All @@ -455,14 +481,7 @@ impl Text {
let shadows: Vec<rustmotion_core::schema::TextShadow> = if let Some(s) = &self.text_shadow {
vec![s.clone()]
} else if let Some(list) = &self.style.text_shadow {
let lctx = rustmotion_core::css::units::LengthContext {
viewport_width: ctx.video_width as f32,
viewport_height: ctx.video_height as f32,
parent_size: layout_width.max(0.0),
font_size,
root_font_size: 16.0,
};
list.iter().map(|s| s.to_schema(&lctx)).collect()
list.iter().map(|s| s.to_schema(&type_ctx)).collect()
} else {
Vec::new()
};
Expand Down
Loading
Loading