Fix max-content width of inline boxes following a mandatory break

parley/src/layout/data.rs

@@ -357,6 +357,7 @@ impl<B: Brush> LayoutData<B> {
                     }
                     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<B: Brush> LayoutData<B> {
                             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);

parley_bench/benches/main.rs

@@ -5,13 +5,14 @@
 
 use tango_bench::tango_benchmarks;
 
-use parley_bench::benches::{defaults, repeated_justification, spacing, styled};
+use parley_bench::benches::{defaults, long_line, repeated_justification, spacing, styled};
 use parley_bench::fontique_benches::system_fonts_init;
 
 tango_benchmarks!(
     defaults(),
     styled(),
     spacing(),
     repeated_justification(),
+    long_line(),
     system_fonts_init()
 );

parley_bench/src/benches.rs

@@ -211,3 +211,51 @@ pub fn styled() -> Vec<Benchmark> {
         })
         .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<Benchmark> {
+    const DISPLAY_SCALE: f32 = 1.0;
+    const QUANTIZE: bool = true;
+    const REPEAT: usize = 4;
+
+    fn layout_long_line(text: &str, max_advance: Option<f32>, 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<ColorBrush> = 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()
+}

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);

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);
+}