Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 20 additions & 0 deletions parley/src/layout/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ pub(crate) struct RunData {
/// If the metrics change within the run, this is `None`. The run's boxes should then be
/// resolved per style.
pub(crate) run_box: Option<BoxMetrics>,
/// Whether this run has atoms with styles changing mid-atom.
///
/// This can happen when ligatures span across styles, or (awkwardly) if a style changes within
/// a grapheme.
pub(crate) has_mixed_style_atoms: bool,
}

/// The metrics of a run's box. This differs from the style-derived box by taking into account any
Expand Down Expand Up @@ -300,6 +305,7 @@ impl<B: Brush> LayoutData<B> {
/// Processes a shaped run into [`RunData`] and a [`LayoutItem`].
///
/// `char_styles` are the style indices of the text's characters.
#[expect(clippy::cast_possible_truncation, reason = "deferred")]
pub(crate) fn process_shaped_run(
&mut self,
shaped_run_idx: usize,
Expand Down Expand Up @@ -343,6 +349,19 @@ impl<B: Brush> LayoutData<B> {
}
};

// An atom has mixed styles iff a style changes at one of its characters other than its
// first.
let has_mixed_style_atoms = {
let chars = shaped_run.range.char_range.clone();
let slice = self.shaped_text.run_slice(shaped_run_idx as u32);
(chars.start + 1..chars.end).any(|char_idx| {
char_styles[char_idx - 1] != char_styles[char_idx]
&& slice
.atom_at_char(char_idx as u32)
.is_some_and(|atom| atom.char_range().start != char_idx as u32)
})
};

let font = &self.shaped_text.fonts()[shaped_run.font_index];
let run = RunData {
font_attrs: fontique::Attributes {
Expand All @@ -354,6 +373,7 @@ impl<B: Brush> LayoutData<B> {
line_height,
spacing,
run_box,
has_mixed_style_atoms,
};

self.runs.push(run);
Expand Down
47 changes: 34 additions & 13 deletions parley/src/layout/line_break.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use crate::{

use core::ops::Range;
use parley_engine::Atom;
use parley_engine::shape::Whitespace;
use parley_engine::shape::{Character, Whitespace};
use smallvec::SmallVec;

#[derive(Default)]
Expand Down Expand Up @@ -300,13 +300,14 @@ impl LineBoxMetrics {
}

/// Add the glyphs of a text atom of `style_index` in layout item `item_idx` and text run
/// `run_idx`.
/// `run_idx`. The atom's characters are `characters`, the first of which has style
/// `style_index`.
///
/// This adds the style's span box together with its ancestors. When the style's line height
/// is [`LineHeight::MetricsRelative`] (which corresponds to CSS `line-height: normal`), it also
/// adds the run's box, which matters when the run was shaped with a fallback font whose metrics
/// differ from the style's first available font, the font the span box is built from. See
/// [`run_box_metrics`].
/// This adds the span boxes of the atom's styles together with its ancestors. When the first
/// style's line height is [`LineHeight::MetricsRelative`] (which corresponds to CSS
/// `line-height: normal`), it also adds the run's box, which matters when the run was shaped
/// with a fallback font whose metrics differ from the style's first available font, the font
/// the span box is built from. See [`run_box_metrics`]
///
/// [`LineHeight::MetricsRelative`]: crate::LineHeight::MetricsRelative
#[inline]
Expand All @@ -315,16 +316,25 @@ impl LineBoxMetrics {
item_idx: usize,
run_idx: usize,
style_index: u16,
characters: &[Character],
data: &LayoutData<B>,
contributed: &mut Vec<u16>,
) {
self.has_content = true;
// Consecutive atoms almost always come from the same run and style, whose boxes are then
// already on the line.
if self.last_text == (item_idx, style_index) {
// already on the line, so we can exit early. In case the run has atoms with mixed styles,
// new boxes may still be added.
if self.last_text == (item_idx, style_index) && !data.runs[run_idx].has_mixed_style_atoms {
return;
}
self.add_text_boxes(item_idx, run_idx, style_index, data, contributed);
self.add_text_boxes(
item_idx,
run_idx,
style_index,
characters,
data,
contributed,
);
}

/// The part of [`Self::add_text`] for atoms whose boxes may not be on the line yet.
Expand All @@ -336,13 +346,22 @@ impl LineBoxMetrics {
item_idx: usize,
run_idx: usize,
style_index: u16,
characters: &[Character],
data: &LayoutData<B>,
contributed: &mut Vec<u16>,
) {
self.last_text = (item_idx, style_index);
if contributed.last() != Some(&style_index) {
self.add_style(style_index, &data.style_metrics, contributed);
}
if data.runs[run_idx].has_mixed_style_atoms {
// Add the spans of all the atom's other styles.
for character in characters.iter().skip(1) {
if character.style_index != style_index {
self.add_style(character.style_index, &data.style_metrics, contributed);
}
}
Comment on lines +359 to +363

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this could probably skip consecutive chars with the same style?

@tomcur tomcur Oct 1, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could, but I'm not sure it would save a lot except for very pathological cases. The add_style call already does a scan to check if the style has been added previously. So then, three things have to line up for this to matter: a lot of atoms with more than one character, actually having styles crossing a grapheme or a ligature across styles, and having a lot of styles.

What might be useful is to check per-atom whether it has mixed styles, but the version I tried which did that during line breaking regresses it by 4%.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up adding a harmless-enough guard that should catch most of those pathological cases, by checking whether the style is different from the atom's first style.

}
let style = usize::from(style_index);
let shaped_run = &data.shaped_text.runs()[run_idx];
let Some(run_box) = data.runs[run_idx]
Expand Down Expand Up @@ -549,9 +568,9 @@ impl Default for BreakerState {
impl BreakerState {
/// Add the atom currently being evaluated to the current line.
///
/// The box of the atom's run (see [`LineBoxMetrics::add_text`]) is added to the line box.
/// `style_index` is the atom's style, whose span box (and those of its ancestors) is added to
/// the line too. `is_word_separator` is `true` iff the atom is a
/// `style_index` is the style of the atom's first character. The span boxes of the atom's
/// styles (and those of their ancestors) are added to the line too, as is the box of the atom's
/// run, see [`LineBoxMetrics::add_text`]. `is_word_separator` is `true` iff the atom is a
/// [word separator](`is_word_separator`), i.e., a justification opportunity.
#[inline]
fn append_atom_to_line<B: Brush>(
Expand All @@ -571,6 +590,7 @@ impl BreakerState {
self.item_idx,
self.run_idx,
style_index,
atom.characters(),
data,
&mut self.contributed,
);
Expand Down Expand Up @@ -1413,6 +1433,7 @@ impl<'a, B: Brush> BreakLines<'a, B> {
index,
index,
style_index,
&[],
&self.layout.data,
&mut self.state.contributed,
);
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
24 changes: 24 additions & 0 deletions parley_tests/tests/lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,30 @@ fn line_height_changes_per_line() {
assert!(lines.next().is_none());
}

#[test]
fn line_height_change_inside_ligature() {
let text = "ffi ffi";
let small = 20.0;
let large = 40.0;

let mut env = TestEnv::new(test_name!(), None);
let mut builder = env.ranged_builder(text);
builder.push_default(LineHeight::Absolute(small));

// The ligature forms, and gets the bigger line height from the middle "f"
builder.push(LineHeight::Absolute(large), 5..6);

// Narrow enough for one word per line.
let mut layout: Layout<ColorBrush> = builder.build(text);
layout.break_all_lines(Some(1.0));

env.render_and_check_snapshot(&layout, None, &[]);

let mut lines = layout.lines();
assert_eq!(lines.next().unwrap().metrics().line_height, small);
assert_eq!(lines.next().unwrap().metrics().line_height, large);
}

/// Metrics contributed by content that is moved to the next line when a word does not fit must
/// not leak into the line it was reverted from.
#[test]
Expand Down
Loading