diff --git a/parley/src/layout/data.rs b/parley/src/layout/data.rs index f87c33a8b..4f1218d05 100644 --- a/parley/src/layout/data.rs +++ b/parley/src/layout/data.rs @@ -357,6 +357,7 @@ impl LayoutData { } for atom in slice.atoms_start() { let character = &atom.characters()[0]; + let whitespace = character.info.whitespace(); let boundary = character.info.boundary(); let style = &self.styles[character.style_index as usize]; let prev_text_wrap_mode = text_wrap_mode; @@ -369,16 +370,32 @@ impl LayoutData { let trailing_whitespace = whitespace_advance(prev_atom); min_width = min_width.max(running_min_width - trailing_whitespace); running_min_width = 0.0; - if boundary == Boundary::Mandatory { - max_width = max_width.max(running_max_width - trailing_whitespace); - running_max_width = 0.0; - } } - let advance = spacing.atom_advance(&atom); - running_min_width += advance; - running_max_width += advance; - if !is_rtl { - prev_atom = Some((character.info.whitespace(), advance)); + + // Handle `Whitespace::Newline` rather than relying on `Boundary::Mandatory`, + // because `Boundary::Mandatory` is only set on the character *following* a break, + // at which point it is too late to handle inline boxes between the line break + // and the following character. + // + // This function doesn't have special handling for CRLF because two linebreaks + // immediately following each other are equivalent to one linebreak for the purpose + // of width calculation. + if whitespace == Whitespace::Newline { + let trailing_whitespace = whitespace_advance(prev_atom); + min_width = min_width.max(running_min_width - trailing_whitespace); + max_width = max_width.max(running_max_width - trailing_whitespace); + running_min_width = 0.0; + running_max_width = 0.0; + if !is_rtl { + prev_atom = None; + } + } else { + let advance = spacing.atom_advance(&atom); + running_min_width += advance; + running_max_width += advance; + if !is_rtl { + prev_atom = Some((whitespace, advance)); + } } } let trailing_whitespace = whitespace_advance(prev_atom); diff --git a/parley/src/layout/line.rs b/parley/src/layout/line.rs index 60ff8bece..29b9e737b 100644 --- a/parley/src/layout/line.rs +++ b/parley/src/layout/line.rs @@ -6,11 +6,12 @@ use crate::layout::data::BreakReason; use crate::layout::data::{LayoutItemKind, LineData}; use crate::layout::layout::Layout; use crate::layout::run::Run; +use crate::layout::spacing::EffectiveSpacing; use crate::style::Brush; use crate::{InlineBox, InlineBoxKind}; use core::ops::Range; -use parley_engine::Glyph; +use parley_engine::{Atom, Atoms, Glyph}; /// Line in a text layout. #[derive(Copy, Clone)] @@ -107,6 +108,7 @@ impl<'a, B: Brush> Line<'a, B> { GlyphRunIter { line: self.clone(), item_index: 0, + atoms: None, glyph_start: 0, offset: 0., } @@ -296,10 +298,79 @@ impl<'a, B: Brush> GlyphRun<'a, B> { } } +/// Peekable iterator over the atoms of a run in visual order, resumable across +/// [`GlyphRunIter::next`] calls. +/// +/// This walks the run's shaped data ([`Atom`]s) directly rather than materialising +/// [`Cluster`](crate::Cluster)s and their composed glyph iterators. It computes the same glyph +/// counts and advances as [`Run::glyphs_in`] over the same atoms. +#[derive(Clone)] +struct AtomIter<'a> { + atoms: Atoms<'a>, + spacing: EffectiveSpacing, + is_rtl: bool, + /// The next atom in visual order, if it has been peeked but not yet consumed. + peeked: Option>, +} + +impl<'a> AtomIter<'a> { + fn new(run: Run<'a, B>) -> Self { + let slice = run.line_slice(); + let is_rtl = run.is_rtl(); + Self { + atoms: if is_rtl { + slice.atoms_end() + } else { + slice.atoms_start() + }, + spacing: run.line_spacing(), + is_rtl, + peeked: None, + } + } + + #[inline] + fn peek(&mut self) -> Option> { + if self.peeked.is_none() { + self.peeked = if self.is_rtl { + self.atoms.prev() + } else { + self.atoms.next() + }; + } + self.peeked + } + + /// Consume the atom returned by [`Self::peek`], so the next peek moves on. + #[inline] + fn consume(&mut self) { + self.peeked = None; + } + + /// The number of glyphs in `atom` and the sum of their advances (including spacing gaps), + /// matching what [`Run::glyphs_in`] yields for the atom's clusters. + #[inline] + fn measure(&self, atom: &Atom<'a>) -> (usize, f32) { + let glyph_count: usize = atom + .shaped_clusters() + .iter() + .map(|cluster| usize::from(cluster.glyph_len())) + .sum(); + let mut advance = atom.advance(); + if glyph_count != 0 && !self.spacing.is_zero() { + advance += self.spacing.gaps(atom).total(); + } + (glyph_count, advance) + } +} + #[derive(Clone)] struct GlyphRunIter<'a, B: Brush> { line: Line<'a, B>, item_index: usize, + /// Iterator over the visual atoms of the run at `item_index`, positioned at the first atom + /// not yet yielded as part of a glyph run. `None` before entering a run. + atoms: Option>, glyph_start: usize, offset: f32, } @@ -333,27 +404,32 @@ impl<'a, B: Brush> Iterator for GlyphRunIter<'a, B> { })); } LineItem::Run(run) => { - // TODO: this is taking the glyphs and style indices from `parley`'s `Cluster`, - // which means style indices are taken from the atom's first character. We could - // get the style index from `parley_engine`'s `ShapedCluster` instead, which - // would be somewhat finer-grained. - let mut glyphs = run - .visual_clusters() - .flat_map(|c| { - let style_index = c.style_index(); - c.glyphs().map(move |glyph| (glyph, style_index)) - }) - .skip(self.glyph_start); - - if let Some((first_glyph, first_style_index)) = glyphs.next() { - let mut advance = first_glyph.advance; - let mut glyph_count = 1; - for (glyph, _) in - glyphs.take_while(|(_, style_index)| *style_index == first_style_index) - { - glyph_count += 1; - advance += glyph.advance; + // TODO: style indices are taken from the atom's first character, matching + // `Cluster::style_index`. We could get the style index from `parley_engine`'s + // `ShapedCluster` instead, which would be somewhat finer-grained. + let atoms = self.atoms.get_or_insert_with(|| AtomIter::new(run)); + + // Styles are uniform within an atom, so glyph runs always end on atom + // boundaries and the iterator only needs to resume at atom granularity. + // Atoms without glyphs don't take part. + let mut advance = 0.0; + let mut glyph_count = 0; + let mut style_index: Option = None; + while let Some(atom) = atoms.peek() { + let (atom_glyph_count, atom_advance) = atoms.measure(&atom); + if atom_glyph_count != 0 { + let atom_style_index = atom.characters()[0].style_index; + if style_index.is_some_and(|s| s != atom_style_index) { + break; + } + style_index = Some(atom_style_index); + glyph_count += atom_glyph_count; + advance += atom_advance; } + atoms.consume(); + } + + if let Some(first_style_index) = style_index { let glyph_start = self.glyph_start; self.glyph_start += glyph_count; let offset = self.offset; @@ -370,6 +446,8 @@ impl<'a, B: Brush> Iterator for GlyphRunIter<'a, B> { advance, })); } + // Any atoms left have no glyphs and thus nothing to yield. + self.atoms = None; self.item_index += 1; self.glyph_start = 0; } diff --git a/parley_bench/benches/main.rs b/parley_bench/benches/main.rs index afcfc5275..27eca5dc1 100644 --- a/parley_bench/benches/main.rs +++ b/parley_bench/benches/main.rs @@ -5,13 +5,17 @@ use tango_bench::tango_benchmarks; -use parley_bench::benches::{defaults, repeated_justification, spacing, styled}; +use parley_bench::benches::{ + defaults, iterate_styled_items, long_line, repeated_justification, spacing, styled, +}; use parley_bench::fontique_benches::system_fonts_init; tango_benchmarks!( defaults(), styled(), + iterate_styled_items(), spacing(), repeated_justification(), + long_line(), system_fonts_init() ); diff --git a/parley_bench/src/benches.rs b/parley_bench/src/benches.rs index d1c4485f1..0f01088e5 100644 --- a/parley_bench/src/benches.rs +++ b/parley_bench/src/benches.rs @@ -7,8 +7,8 @@ use crate::{ColorBrush, FONT_FAMILY_LIST, get_samples, with_contexts}; use parley::{ - Alignment, AlignmentOptions, FontFamily, FontStyle, FontWeight, Layout, RangedBuilder, - StyleProperty, + Alignment, AlignmentOptions, FontFamily, FontStyle, FontWeight, Layout, PositionedLayoutItem, + RangedBuilder, StyleProperty, }; use std::hint::black_box; use tango_bench::{Benchmark, benchmark_fn}; @@ -141,8 +141,8 @@ pub fn repeated_justification() -> [Benchmark; 1] { )] } -/// Benchmark for styled text. -pub fn styled() -> Vec { +/// Build a styled layout for `text`, changing style every few characters. +fn build_styled_layout(text: &str) -> Layout { const DISPLAY_SCALE: f32 = 1.0; const QUANTIZE: bool = true; const MAX_ADVANCE: f32 = 200.0 * DISPLAY_SCALE; @@ -163,6 +163,39 @@ pub fn styled() -> Vec { } } + with_contexts(|font_cx, layout_cx| { + let mut builder = layout_cx.ranged_builder(font_cx, text, DISPLAY_SCALE, QUANTIZE); + builder.push_default(FontFamily::from(FONT_FAMILY_LIST)); + + // Apply different styles every `style_interval` characters + let style_interval = (text.len() / 5).min(10); + { + let mut chunk_start = 0; + let mut style_idx = 0; + + for (char_count, (byte_idx, _)) in text.char_indices().enumerate() { + if char_count != 0 && char_count % style_interval == 0 { + apply_style(&mut builder, style_idx, chunk_start..byte_idx); + chunk_start = byte_idx; + style_idx += 1; + } + } + + // Apply style to the last chunk if there's remaining text + if chunk_start < text.len() { + apply_style(&mut builder, style_idx, chunk_start..text.len()); + } + } + + let mut layout: Layout = builder.build(text); + layout.break_all_lines(Some(MAX_ADVANCE)); + layout.align(Alignment::Start, AlignmentOptions::default()); + layout + }) +} + +/// Benchmark for styled text. +pub fn styled() -> Vec { let samples = get_samples(); samples @@ -172,42 +205,96 @@ pub fn styled() -> Vec { format!("Styled - {} {}", sample.name, sample.modification), |b| { b.iter(|| { - let text = &sample.text; + black_box(build_styled_layout(&sample.text)); + }) + }, + ) + }) + .collect() +} - with_contexts(|font_cx, layout_cx| { - let mut builder = - layout_cx.ranged_builder(font_cx, text, DISPLAY_SCALE, QUANTIZE); - builder.push_default(FontFamily::from(FONT_FAMILY_LIST)); +/// Benchmark for iterating the positioned glyph runs and glyphs of a styled layout, as a renderer +/// would. +pub fn iterate_styled_items() -> Vec { + let samples = get_samples(); - // Apply different styles every `style_interval` characters - let style_interval = (text.len() / 5).min(10); - { - let mut chunk_start = 0; - let mut style_idx = 0; - - for (char_count, (byte_idx, _)) in text.char_indices().enumerate() { - if char_count != 0 && char_count % style_interval == 0 { - apply_style(&mut builder, style_idx, chunk_start..byte_idx); - chunk_start = byte_idx; - style_idx += 1; + samples + .iter() + .map(|sample| { + benchmark_fn( + format!("Styled Items - {} {}", sample.name, sample.modification), + |b| { + let layout = build_styled_layout(&sample.text); + b.iter(move || { + let mut glyph_count = 0_usize; + let mut advance = 0.0_f32; + for line in layout.lines() { + for item in line.items() { + match item { + PositionedLayoutItem::GlyphRun(glyph_run) => { + for glyph in glyph_run.positioned_glyphs() { + glyph_count += 1; + advance += glyph.advance; + } + } + PositionedLayoutItem::InlineBox(inline_box) => { + advance += inline_box.width; } - } - - // Apply style to the last chunk if there's remaining text - if chunk_start < text.len() { - apply_style(&mut builder, style_idx, chunk_start..text.len()); } } - - let mut layout: Layout = builder.build(text); - layout.break_all_lines(Some(MAX_ADVANCE)); - layout.align(Alignment::Start, AlignmentOptions::default()); - - black_box(layout); - }); + } + black_box((glyph_count, advance)) }) }, ) }) .collect() } + +/// Benchmark for a single very long line (no wrapping) with and without justification. +/// +/// This exercises per-line work that scales with line length. +pub fn long_line() -> Vec { + const DISPLAY_SCALE: f32 = 1.0; + const QUANTIZE: bool = true; + const REPEAT: usize = 4; + + fn layout_long_line(text: &str, max_advance: Option, alignment: Alignment) { + with_contexts(|font_cx, layout_cx| { + let mut builder = layout_cx.ranged_builder(font_cx, text, DISPLAY_SCALE, QUANTIZE); + builder.push_default(FontFamily::from(FONT_FAMILY_LIST)); + + let mut layout: Layout = builder.build(text); + layout.break_all_lines(max_advance); + layout.align(alignment, AlignmentOptions::default()); + + black_box(layout); + }); + } + + let samples = get_samples(); + + samples + .iter() + .filter(|sample| sample.modification == "4 paragraph") + .flat_map(|sample| { + let text: &'static str = Box::leak( + sample + .text + .replace('\n', " ") + .repeat(REPEAT) + .into_boxed_str(), + ); + [ + benchmark_fn(format!("Long Line - {}", sample.name), move |b| { + b.iter(move || layout_long_line(text, None, Alignment::Start)) + }), + benchmark_fn(format!("Long Line Justify - {}", sample.name), move |b| { + // Justification requires a finite `max_advance`; use one wide enough that the + // text still lays out as a single line. + b.iter(move || layout_long_line(text, Some(1.0e7), Alignment::Justify)) + }), + ] + }) + .collect() +} diff --git a/parley_tests/snapshots/inline_boxes_after_newline_max_content_width-inline_boxes_after_newline.png b/parley_tests/snapshots/inline_boxes_after_newline_max_content_width-inline_boxes_after_newline.png new file mode 100644 index 000000000..af29cc526 Binary files /dev/null and b/parley_tests/snapshots/inline_boxes_after_newline_max_content_width-inline_boxes_after_newline.png differ diff --git a/parley_tests/snapshots/justify_consecutive_spaces-0.png b/parley_tests/snapshots/justify_consecutive_spaces-0.png new file mode 100644 index 000000000..b3718abde Binary files /dev/null and b/parley_tests/snapshots/justify_consecutive_spaces-0.png differ diff --git a/parley_tests/tests/basic.rs b/parley_tests/tests/basic.rs index b28f52466..c15df2002 100644 --- a/parley_tests/tests/basic.rs +++ b/parley_tests/tests/basic.rs @@ -597,6 +597,40 @@ fn justify_with_overflowing_trailing_space() { ); } +#[test] +fn justify_consecutive_spaces() { + let mut env = TestEnv::new(test_name!(), None); + + // Each space is a separate justification opportunity, including when they are consecutive + // (without whitespace collapsing). The second paragraph here has a run of three consecutive + // spaces. + let text = "Foo bar baz supercalifragilistic.\n\nFoo bar baz supercalifragilistic."; + let builder = env.ranged_builder(text); + let mut layout = builder.build(text); + layout.break_all_lines(Some(150.0)); + layout.align(Alignment::Justify, AlignmentOptions::default()); + + env.check_layout_snapshot(&layout); + + // Check that the second paragraph's interior spaces on the first line all stretch by the same + // amount. + let space_advances: Vec<_> = layout + .get(3) + .unwrap() + .runs() + .flat_map(|run| run.clusters()) + .filter(|cluster| cluster.is_space_or_nbsp()) + .map(|cluster| cluster.advance()) + .collect(); + assert_eq!(space_advances.len(), 5, "four spaces plus one trailing"); + assert!( + space_advances[..4] + .iter() + .all(|advance| (advance - space_advances[0]).abs() < 0.001), + "The interior spaces should stretch equally" + ); +} + #[test] fn content_widths() { let mut env = TestEnv::new(test_name!(), None); diff --git a/parley_tests/tests/issues.rs b/parley_tests/tests/issues.rs index a06e99c32..089b7195a 100644 --- a/parley_tests/tests/issues.rs +++ b/parley_tests/tests/issues.rs @@ -172,3 +172,38 @@ fn issue_748_language_subtags() { layout.align(Alignment::Start, AlignmentOptions::default()); env.check_layout_snapshot(&layout); } + +/// Test that inline boxes directly following a mandatory line break all +/// contribute to the max content width of the line they end up on. +#[test] +fn inline_boxes_after_newline_max_content_width() { + let mut env = TestEnv::new(test_name!(), None); + + // A newline followed by three inline boxes separated by spaces. + let text = "\n "; + let mut builder = env.ranged_builder(text); + for (id, index) in [(0_u64, 1_usize), (1, 2), (2, 3)] { + builder.push_inline_box(InlineBox { + id, + kind: InlineBoxKind::InFlow, + index, + width: 14.0, + height: 30.0, + baseline: None, + }); + } + let mut layout = builder.build(text); + + let content_widths = layout.calculate_content_widths(); + + layout.break_all_lines(Some(content_widths.max)); + layout.align(Alignment::Start, AlignmentOptions::default()); + assert!( + layout.width() <= content_widths.max, + "Layout should never be wider than the max content width (width: {}, max: {})", + layout.width(), + content_widths.max + ); + env.with_name("inline_boxes_after_newline") + .check_layout_snapshot(&layout); +}