From 1971cee3a417dab168b8f75db033a6815da7ad34 Mon Sep 17 00:00:00 2001 From: Taj Pereira Date: Mon, 15 Dec 2025 05:16:53 +1030 Subject: [PATCH 1/3] Don't allow hanging whitespace to contribute to wrapping --- parley/src/layout/alignment.rs | 6 +- parley/src/layout/data.rs | 5 +- parley/src/layout/line_break.rs | 60 +++++---- parley/src/tests/test_basic.rs | 124 ++++++++++++++++++ .../breaking_whitespace_does_not_wrap-0.png | Bin 0 -> 593 bytes ...tespace_does_not_contribute_to_align-0.png | Bin 0 -> 595 bytes ...ce_does_not_contribute_to_align-center.png | Bin 0 -> 595 bytes ...space_does_not_contribute_to_align-end.png | Bin 0 -> 593 bytes ...e_does_not_contribute_to_align-justify.png | Bin 0 -> 593 bytes ...ace_does_not_contribute_to_align-start.png | Bin 0 -> 593 bytes ...e_num_non_trailing_spaces-double_space.png | Bin 0 -> 560 bytes ...ace_num_non_trailing_spaces-final_line.png | Bin 0 -> 660 bytes ...pace_num_non_trailing_spaces-multiword.png | Bin 0 -> 660 bytes ...ce_num_non_trailing_spaces-single_word.png | Bin 0 -> 593 bytes 14 files changed, 165 insertions(+), 30 deletions(-) create mode 100644 parley/tests/snapshots/breaking_whitespace_does_not_wrap-0.png create mode 100644 parley/tests/snapshots/hanging_whitespace_does_not_contribute_to_align-0.png create mode 100644 parley/tests/snapshots/hanging_whitespace_does_not_contribute_to_align-center.png create mode 100644 parley/tests/snapshots/hanging_whitespace_does_not_contribute_to_align-end.png create mode 100644 parley/tests/snapshots/hanging_whitespace_does_not_contribute_to_align-justify.png create mode 100644 parley/tests/snapshots/hanging_whitespace_does_not_contribute_to_align-start.png create mode 100644 parley/tests/snapshots/hanging_whitespace_num_non_trailing_spaces-double_space.png create mode 100644 parley/tests/snapshots/hanging_whitespace_num_non_trailing_spaces-final_line.png create mode 100644 parley/tests/snapshots/hanging_whitespace_num_non_trailing_spaces-multiword.png create mode 100644 parley/tests/snapshots/hanging_whitespace_num_non_trailing_spaces-single_word.png diff --git a/parley/src/layout/alignment.rs b/parley/src/layout/alignment.rs index a0703bfb4..fa79c6c09 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; @@ -150,7 +150,7 @@ fn align_impl( } let adjustment = - free_space / line.num_spaces as f32 * if UNDO_JUSTIFICATION { -1. } else { 1. }; + 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 b93a7534d..152cf120b 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 7b3b56d02..9eb7b3a5c 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 3f2d4f021..559794641 100644 --- a/parley/src/tests/test_basic.rs +++ b/parley/src/tests/test_basic.rs @@ -251,6 +251,130 @@ fn leading_whitespace() { } } +#[test] +fn breaking_whitespace_does_not_wrap() { + let mut env = TestEnv::new(test_name!(), None); + + 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::Start, AlignmentOptions::default()); + + //assert_eq!(layout.lines().count(), 2); + env.check_layout_snapshot(&layout); +} + +#[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/breaking_whitespace_does_not_wrap-0.png b/parley/tests/snapshots/breaking_whitespace_does_not_wrap-0.png new file mode 100644 index 0000000000000000000000000000000000000000..29689962c3ddef99e93a15db249faaec7bda9cd4 GIT binary patch literal 593 zcmV-X0{a>?6$P7}{N~II#qT_YR#fSJUS>i{jR4SE9rLT{F>hJmV zb}uufJ$M)0t(iFTls7Nn9e+yf3wdwIK2IL9_mg709R2Hfu^1Vm;~=4ngU(JG_j!I; zPIlotzy-+CzGxh|Wgol&sye-X`;}KG^(pVhL!Oz|H~2djpUsKPkX&Bz2Z7KBV8)mHeYrDwRs5()Y(py~L-l zTS}Am;B&~=Qj+0`v);ae&-^AM=8e3!Sg%bD#`_-Xlbrnfcrh3`gqd2ZE?k(S#QgXC z;p9J%IT1>1w&Vf&R&S*B0B8xMAfquqsHUUwC@6R+W7X(WMo-DCSG z5(c0Xd5!-=YPn@~7yi-z$)CMX8)zeD18=90vwZBuSFJ)4w-YaJedUe!2EaDHQGotF zoALC%0nou_p~~XG$st#A0X>8qr%ikc}~Sm&~QmKlQNHI z`f&Bz2Z7KBV8)mHeYrDwRs5()Y(py~L-l zTS}Am;B&~=Qj+0`v);ae&-^AM=8e3!Sg%bD#`_-Xlbrnfcrh3`gqd2ZE?k(S#QgXC z;p9J%IT1>1w&Vf&R&S*B0B8xMAfquqsHUUwC@6R+W7X(WMo-DCSG z5(c0Xd5!-=YPn@~7yi-z$)CMX8)zeD18=90vwZBuSFJ)4w-YaJedUe!2EaDHQGotF zoALC%0nou_p~~XG$st#A0X>8qr%ikc}~Sm&~QmKlQNHI z`f;Drhz#V(cyzN7SCa{(62q zO=QbQ0Q04#dDCzfYRhBNUFH3=-}6J@!^&ZKAhn^FgYstk@}u$`MaJc5qZ~7Ph5>-5 z!x1Y@1dF_-kncC;@7AV_w4%z$o4#|7*Q4@P5Fk+>lwZTnVcF;k6Z5oB!Q{M-P1))0 ziHSVzIWO;f!D?hxF#@UamoPuQc9o+BG#{?w4Uk9Ye4mw1T{GTan^rf1z5*oFXqkYP z+rp0&XeQv2m51oSegG~WfT2np01R;*7=ni8_I7v4p8YIU9N@T!iVCyWsqA^`+`OCw z6Yt=>nweML*B|pCJHL(sm0!jV2!s)Uu2J;>gzcWM9sq`%JX~`&$x=hQTf(y!)Xg2| zriI~X(yfy{ulU#8o2{a>?6$P7}{N~II#qT_YR#fSJUS>i{jR4SE9rLT{F>hJmV zb}uufJ$M)0t(iFTls7Nn9e+yf3wdwIK2IL9_mg709R2Hfu^1Vm;~=4ngU(JG_j!I; zPIlotzy-+CzGxh|Wgol&sye-X`;}KG^(pVhL!Oz|H~2djpUsKPkX{a>?6$P7}{N~II#qT_YR#fSJUS>i{jR4SE9rLT{F>hJmV zb}uufJ$M)0t(iFTls7Nn9e+yf3wdwIK2IL9_mg709R2Hfu^1Vm;~=4ngU(JG_j!I; zPIlotzy-+CzGxh|Wgol&sye-X`;}KG^(pVhL!Oz|H~2djpUsKPkXx=z`($Ne}A8!pRcd4zrVj9A0JOoPj7E;5D*X`ARtgsP@tfo(b3V6kdUyj zuyAm2U|?WCKtPX=kAQ%HFE1|-4-YUfFkfF^@9*!=&(A+UKS*RO#Q*>Sj!8s8RCwC$ z*V%6CAQXk+vlcc64?UgYf$9ApvjvhiNpDI;Zz}$aXe*LGV=N*0U#rz>wOXy)P+E~J zI$Y`muJhe{>8~zLrLv(%PSWDKaD+g<0H97&nJjo*yiwEL$RXWM0_*{-9E#*g%NZ%b z;gI)oR@vvNNM2UQ?DYq%`6By|H7m*zd7!{}LTs6!=&$8v+g|4h08e=;k;gsj$f}<1 z-r&v$2TxwZ{+-&W$d9qUPh$V43HG* z>hWGSAF>(0FGn(_Z3JQ~#iwlI{%`>JnkMyq*&ayse56Foas8A@G7JEiCI$S~iPYqu z%kF9WFOYiPHeXMcJc6Dj0AUycPqk!43&2tUX7_;SR7oB+V(c++h?89^$>$|{kDiDL zRZH%1&T8bU%OU)h+_SE{Tf{bnl`c+dt0ecy6O<5n`KP^-><-h3XR|wC;*;1k*j8R{4GZ*PK=$>8Z|QtT9<}7O7KlajL)z{%SNDjYgxm_bQAQtTVzk%|CM@^)h;(v;ZSN|!g#@kGU)TZEmOp- zZz<>8D#Z^|QXEQ+bIu`Wqyd0c1B;FK*CI^H-)y+qp;Y9LloEFUwEeO-3qY_->B|25 z0**HRuwdhX)WSOoh&HPSOt#DS)WH2CJCIT%00)UlemSr>tq9k(s@(o}qR zpM;&Ob0Ql#ca3MKJWoMd4}ewB z@KA?Ig>r%&=f2pJQX#x5RTi*r()p=+t-Brz$IW_& z+IRh_scsJtdr%HsW?)q1hB`GBr!~=aog-lQorL2apy`rmyNymwA&$(fDSsz1%V9GS unAybhkK2gK-9YUUX*3#*Mx)VqB7XoZ0wH^xdBjWr00001k*j8R{4GZ*PK=$>8Z|QtT9<}7O7KlajL)z{%SNDjYgxm_bQAQtTVzk%|CM@^)h;(v;ZSN|!g#@kGU)TZEmOp- zZz<>8D#Z^|QXEQ+bIu`Wqyd0c1B;FK*CI^H-)y+qp;Y9LloEFUwEeO-3qY_->B|25 z0**HRuwdhX)WSOoh&HPSOt#DS)WH2CJCIT%00)UlemSr>tq9k(s@(o}qR zpM;&Ob0Ql#ca3MKJWoMd4}ewB z@KA?Ig>r%&=f2pJQX#x5RTi*r()p=+t-Brz$IW_& z+IRh_scsJtdr%HsW?)q1hB`GBr!~=aog-lQorL2apy`rmyNymwA&$(fDSsz1%V9GS unAybhkK2gK-9YUUX*3#*Mx)VqB7XoZ0wH^xdBjWr0000{a>?6$P7}{N~II#qT_YR#fSJUS>i{jR4SE9rLT{F>hJmV zb}uufJ$M)0t(iFTls7Nn9e+yf3wdwIK2IL9_mg709R2Hfu^1Vm;~=4ngU(JG_j!I; zPIlotzy-+CzGxh|Wgol&sye-X`;}KG^(pVhL!Oz|H~2djpUsKPkX Date: Mon, 15 Dec 2025 05:24:24 +1030 Subject: [PATCH 2/3] . --- parley/src/tests/test_basic.rs | 16 +--------------- .../breaking_whitespace_does_not_wrap-0.png | Bin 593 -> 0 bytes 2 files changed, 1 insertion(+), 15 deletions(-) delete mode 100644 parley/tests/snapshots/breaking_whitespace_does_not_wrap-0.png diff --git a/parley/src/tests/test_basic.rs b/parley/src/tests/test_basic.rs index 559794641..a84248588 100644 --- a/parley/src/tests/test_basic.rs +++ b/parley/src/tests/test_basic.rs @@ -251,20 +251,6 @@ fn leading_whitespace() { } } -#[test] -fn breaking_whitespace_does_not_wrap() { - let mut env = TestEnv::new(test_name!(), None); - - 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::Start, AlignmentOptions::default()); - - //assert_eq!(layout.lines().count(), 2); - env.check_layout_snapshot(&layout); -} - #[test] fn hanging_whitespace_does_not_contribute_to_align() { let mut env = TestEnv::new(test_name!(), None); @@ -281,7 +267,7 @@ fn hanging_whitespace_does_not_contribute_to_align() { layout.break_all_lines(Some(50.0)); layout.align(None, alignment, AlignmentOptions::default()); - //assert_eq!(layout.lines().count(), 2); + assert_eq!(layout.lines().count(), 2); env.with_name(test_case_name).check_layout_snapshot(&layout); } } diff --git a/parley/tests/snapshots/breaking_whitespace_does_not_wrap-0.png b/parley/tests/snapshots/breaking_whitespace_does_not_wrap-0.png deleted file mode 100644 index 29689962c3ddef99e93a15db249faaec7bda9cd4..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 593 zcmV-X0{a>?6$P7}{N~II#qT_YR#fSJUS>i{jR4SE9rLT{F>hJmV zb}uufJ$M)0t(iFTls7Nn9e+yf3wdwIK2IL9_mg709R2Hfu^1Vm;~=4ngU(JG_j!I; zPIlotzy-+CzGxh|Wgol&sye-X`;}KG^(pVhL!Oz|H~2djpUsKPkX Date: Mon, 15 Dec 2025 05:27:33 +1030 Subject: [PATCH 3/3] . --- parley/src/layout/alignment.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/parley/src/layout/alignment.rs b/parley/src/layout/alignment.rs index fa79c6c09..cf30f3998 100644 --- a/parley/src/layout/alignment.rs +++ b/parley/src/layout/alignment.rs @@ -149,8 +149,8 @@ fn align_impl( continue; } - let adjustment = - free_space / line.num_non_trailing_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)