diff --git a/parley/src/layout/alignment.rs b/parley/src/layout/alignment.rs index a0703bfb..cf30f399 100644 --- a/parley/src/layout/alignment.rs +++ b/parley/src/layout/alignment.rs @@ -141,7 +141,7 @@ fn align_impl( // gaps to adjust. In that case, start-align, i.e., left-align for LTR text and // right-align for RTL text. if matches!(line.break_reason, BreakReason::None | BreakReason::Explicit) - || line.num_spaces == 0 + || line.num_non_trailing_spaces == 0 { if is_rtl { line.metrics.offset += free_space; @@ -149,8 +149,8 @@ fn align_impl( continue; } - let adjustment = - free_space / line.num_spaces as f32 * if UNDO_JUSTIFICATION { -1. } else { 1. }; + let adjustment = free_space / line.num_non_trailing_spaces as f32 + * if UNDO_JUSTIFICATION { -1. } else { 1. }; let mut applied = 0; // Iterate over text runs in the line and clusters in the text run // - Iterate forwards for even bidi levels (which represent LTR runs) @@ -172,7 +172,7 @@ fn align_impl( &mut clusters.iter_mut() }; clusters.for_each(|cluster| { - if applied == line.num_spaces { + if applied == line.num_non_trailing_spaces { return; } if cluster.info.whitespace().is_space_or_nbsp() { diff --git a/parley/src/layout/data.rs b/parley/src/layout/data.rs index b93a7534..152cf120 100644 --- a/parley/src/layout/data.rs +++ b/parley/src/layout/data.rs @@ -165,8 +165,9 @@ pub(crate) struct LineData { pub(crate) break_reason: BreakReason, /// Maximum advance for the line. pub(crate) max_advance: f32, - /// Number of justified clusters on the line. - pub(crate) num_spaces: usize, + /// The number of non-trailing spaces on the line. + /// Used for justification. + pub(crate) num_non_trailing_spaces: usize, } impl LineData { diff --git a/parley/src/layout/line_break.rs b/parley/src/layout/line_break.rs index 7b3b56d0..9eb7b3a5 100644 --- a/parley/src/layout/line_break.rs +++ b/parley/src/layout/line_break.rs @@ -39,7 +39,7 @@ struct LineState { x: f32, items: Range, clusters: Range, - num_spaces: usize, + num_non_trailing_spaces: usize, /// Of the line currently being built, the maximum line height seen so far. /// This represents a lower-bound on the eventual line height of the line. running_line_height: f32, @@ -356,7 +356,7 @@ impl<'a, B: Brush> BreakLines<'a, B> { self.state.append_cluster_to_line(next_x, line_height); self.state.cluster_idx += 1; if is_space { - self.state.line.num_spaces += 1; + self.state.line.num_non_trailing_spaces += 1; } } // Else we attempt to line break: @@ -368,14 +368,14 @@ impl<'a, B: Brush> BreakLines<'a, B> { // Case: cluster is a space character (and wrapping is enabled) // // We hang any overflowing whitespace and then line-break. + // "Hanging" means we add the space to the line but continue processing + // rather than committing the line. The trailing whitespace is allowed + // to overflow and will be accounted for when we hit a non-space that + // requires a break. if is_space && text_wrap_mode == TextWrapMode::Wrap { let line_height = run.metrics().line_height; self.state.append_cluster_to_line(next_x, line_height); - if try_commit_line!(BreakReason::Regular) { - // TODO: can this be hoisted out of the conditional? - self.state.cluster_idx += 1; - return self.start_new_line(); - } + self.state.cluster_idx += 1; } // Case: we have previously encountered a REGULAR line-breaking opportunity in the current line // @@ -571,29 +571,45 @@ impl<'a, B: Brush> BreakLines<'a, B> { // Compute size of line's trailing whitespace. "Trailing" is considered the right edge // for LTR text and the left edge for RTL text. - let run = if self.layout.is_rtl() { + let trailing_run = if self.layout.is_rtl() { self.lines.line_items[line.item_range.clone()].first() } else { self.lines.line_items[line.item_range.clone()].last() }; - line.metrics.trailing_whitespace = run + let (trailing_whitespace_advance, trailing_whitespace_count) = trailing_run .filter(|item| item.is_text_run() && item.has_trailing_whitespace) .map(|run| { - fn whitespace_advance<'c, I: Iterator>(clusters: I) -> f32 { - clusters - .take_while(|cluster| cluster.info.whitespace() != Whitespace::None) - .map(|cluster| cluster.advance) - .sum() + fn whitespace_stats<'c, I: Iterator>( + clusters: I, + ) -> (f32, usize) { + let mut advance = 0.0; + let mut count = 0; + for cluster in clusters { + if cluster.info.whitespace() == Whitespace::None { + break; + } + advance += cluster.advance; + if cluster.info.whitespace().is_space_or_nbsp() { + count += 1; + } + } + (advance, count) } let clusters = &self.layout.data.clusters[run.cluster_range.clone()]; if run.is_rtl() { - whitespace_advance(clusters.iter()) + whitespace_stats(clusters.iter()) } else { - whitespace_advance(clusters.iter().rev()) + whitespace_stats(clusters.iter().rev()) } }) - .unwrap_or(0.0); + .unwrap_or((0.0, 0)); + line.metrics.trailing_whitespace = trailing_whitespace_advance; + // Line computation has ended, but we haven't yet removed trailing whitespace from the line, + // so mutate `num_non_trailing_spaces` to reflect the actual number of trailing whitespace spaces. + line.num_non_trailing_spaces = line + .num_non_trailing_spaces + .saturating_sub(trailing_whitespace_count); if !have_metrics { // Line consisting entirely of whitespace? @@ -877,17 +893,11 @@ fn try_commit_line( // return false; // } - // Q: why this special case? - let mut num_spaces = state.num_spaces; - if break_reason == BreakReason::Regular { - num_spaces = num_spaces.saturating_sub(1); - } - lines.lines.push(LineData { item_range: start_item_idx..end_item_idx, max_advance, break_reason, - num_spaces, + num_non_trailing_spaces: state.num_non_trailing_spaces, metrics: LineMetrics { advance: state.x, ..Default::default() @@ -896,7 +906,7 @@ fn try_commit_line( }); // Reset state for the new line - state.num_spaces = 0; + state.num_non_trailing_spaces = 0; if committed_text_run { state.clusters.start = state.clusters.end; } diff --git a/parley/src/tests/test_basic.rs b/parley/src/tests/test_basic.rs index 3f2d4f02..a8424858 100644 --- a/parley/src/tests/test_basic.rs +++ b/parley/src/tests/test_basic.rs @@ -251,6 +251,116 @@ fn leading_whitespace() { } } +#[test] +fn hanging_whitespace_does_not_contribute_to_align() { + let mut env = TestEnv::new(test_name!(), None); + + for (alignment, test_case_name) in [ + (Alignment::Start, "start"), + (Alignment::End, "end"), + (Alignment::Center, "center"), + (Alignment::Justify, "justify"), + ] { + let text = "First Second"; + let builder = env.ranged_builder(text); + let mut layout = builder.build(text); + layout.break_all_lines(Some(50.0)); + layout.align(None, alignment, AlignmentOptions::default()); + + assert_eq!(layout.lines().count(), 2); + env.with_name(test_case_name).check_layout_snapshot(&layout); + } +} + +/// Test that `num_non_trailing_spaces` is correctly computed when there is trailing/hanging whitespace. +#[test] +fn hanging_whitespace_num_non_trailing_spaces() { + let mut env = TestEnv::new(test_name!(), None); + + // Test 1: Single word followed by many spaces then another word + // Line 1 should have "First" with trailing whitespace hanging - num_spaces = 0 + // Line 2 should have "Second" with no spaces - num_spaces = 0 + { + let text = "First Second"; + let builder = env.ranged_builder(text); + let mut layout = builder.build(text); + layout.break_all_lines(Some(50.0)); + + assert_eq!(layout.lines().count(), 2, "Expected 2 lines"); + assert_eq!( + layout.data.lines[0].num_non_trailing_spaces, 0, + "First line should have no inter-word spaces (trailing whitespace doesn't count)" + ); + assert_eq!( + layout.data.lines[1].num_non_trailing_spaces, 0, + "Second line should have no spaces" + ); + + env.with_name("single_word").check_layout_snapshot(&layout); + } + + // Test 2: Multiple words with spaces that fit, followed by word that wraps + // Line 1: "AAA BBB " - has one space between words (trailing space excluded) + // Line 2: "CCC" - no spaces + { + let text = "AAA BBB CCC"; + let builder = env.ranged_builder(text); + let mut layout = builder.build(text); + // Use a max_advance that fits "AAA BBB " but not "AAA BBB CCC" + layout.break_all_lines(Some(75.0)); + layout.align(None, Alignment::Justify, AlignmentOptions::default()); + + assert_eq!(layout.lines().count(), 2, "Expected 2 lines"); + // The space between AAA and BBB should count, the trailing space before CCC should not + assert_eq!( + layout.data.lines[0].num_non_trailing_spaces, 1, + "First line should have 1 inter-word space" + ); + assert_eq!( + layout.data.lines[1].num_non_trailing_spaces, 0, + "Second line should have no spaces" + ); + + env.with_name("multiword").check_layout_snapshot(&layout); + } + + // Test 3: Words with multiple spaces between them + // Line 1: "AA BB " (two spaces between words, one trailing) - 2 inter-word spaces count + // Line 2: "CC" - no spaces + { + let text = "AA BB CC"; + let builder = env.ranged_builder(text); + let mut layout = builder.build(text); + layout.break_all_lines(Some(55.0)); + layout.align(None, Alignment::Justify, AlignmentOptions::default()); + + assert_eq!(layout.lines().count(), 2, "Expected 2 lines"); + // Both spaces between AA and BB should count, trailing space before CC should not + assert_eq!( + layout.data.lines[0].num_non_trailing_spaces, 2, + "First line should have 2 inter-word spaces" + ); + + env.with_name("double_space").check_layout_snapshot(&layout); + } + + // Test 4: Final line should have its trailing whitespace spaces removed + { + let text = "AAA BBB CCC "; + let builder = env.ranged_builder(text); + let mut layout = builder.build(text); + layout.break_all_lines(Some(75.0)); + layout.align(None, Alignment::Justify, AlignmentOptions::default()); + + assert_eq!( + layout.data.lines[1].num_non_trailing_spaces, 0, + "Second line should have no spaces" + ); + + env.with_name("final_line").check_layout_snapshot(&layout); + } +} + #[test] fn nested_span_inheritance() { let ts = |c: AlphaColor| TextStyle { diff --git a/parley/tests/snapshots/hanging_whitespace_does_not_contribute_to_align-0.png b/parley/tests/snapshots/hanging_whitespace_does_not_contribute_to_align-0.png new file mode 100644 index 00000000..f68f63a1 Binary files /dev/null and b/parley/tests/snapshots/hanging_whitespace_does_not_contribute_to_align-0.png differ diff --git a/parley/tests/snapshots/hanging_whitespace_does_not_contribute_to_align-center.png b/parley/tests/snapshots/hanging_whitespace_does_not_contribute_to_align-center.png new file mode 100644 index 00000000..f68f63a1 Binary files /dev/null and b/parley/tests/snapshots/hanging_whitespace_does_not_contribute_to_align-center.png differ diff --git a/parley/tests/snapshots/hanging_whitespace_does_not_contribute_to_align-end.png b/parley/tests/snapshots/hanging_whitespace_does_not_contribute_to_align-end.png new file mode 100644 index 00000000..7e241742 Binary files /dev/null and b/parley/tests/snapshots/hanging_whitespace_does_not_contribute_to_align-end.png differ diff --git a/parley/tests/snapshots/hanging_whitespace_does_not_contribute_to_align-justify.png b/parley/tests/snapshots/hanging_whitespace_does_not_contribute_to_align-justify.png new file mode 100644 index 00000000..29689962 Binary files /dev/null and b/parley/tests/snapshots/hanging_whitespace_does_not_contribute_to_align-justify.png differ diff --git a/parley/tests/snapshots/hanging_whitespace_does_not_contribute_to_align-start.png b/parley/tests/snapshots/hanging_whitespace_does_not_contribute_to_align-start.png new file mode 100644 index 00000000..29689962 Binary files /dev/null and b/parley/tests/snapshots/hanging_whitespace_does_not_contribute_to_align-start.png differ diff --git a/parley/tests/snapshots/hanging_whitespace_num_non_trailing_spaces-double_space.png b/parley/tests/snapshots/hanging_whitespace_num_non_trailing_spaces-double_space.png new file mode 100644 index 00000000..f6cf1f8d Binary files /dev/null and b/parley/tests/snapshots/hanging_whitespace_num_non_trailing_spaces-double_space.png differ diff --git a/parley/tests/snapshots/hanging_whitespace_num_non_trailing_spaces-final_line.png b/parley/tests/snapshots/hanging_whitespace_num_non_trailing_spaces-final_line.png new file mode 100644 index 00000000..ccf69b12 Binary files /dev/null and b/parley/tests/snapshots/hanging_whitespace_num_non_trailing_spaces-final_line.png differ diff --git a/parley/tests/snapshots/hanging_whitespace_num_non_trailing_spaces-multiword.png b/parley/tests/snapshots/hanging_whitespace_num_non_trailing_spaces-multiword.png new file mode 100644 index 00000000..ccf69b12 Binary files /dev/null and b/parley/tests/snapshots/hanging_whitespace_num_non_trailing_spaces-multiword.png differ diff --git a/parley/tests/snapshots/hanging_whitespace_num_non_trailing_spaces-single_word.png b/parley/tests/snapshots/hanging_whitespace_num_non_trailing_spaces-single_word.png new file mode 100644 index 00000000..29689962 Binary files /dev/null and b/parley/tests/snapshots/hanging_whitespace_num_non_trailing_spaces-single_word.png differ