Skip to content

Adjust advance when justifying text #397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kekelp wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Adjust advance when justifying text #397

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b09284c
Adjust advance when justifying text
Dec 1, 2025
ec8ae67
Add changelog entry
Dec 1, 2025
11af141
Store unjustified advance for unjustify calculation
Dec 1, 2025
663f59e
Apply suggestions from code review
kekelp Dec 1, 2025
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ fn bounding_box_to_rect(bb: parley::BoundingBox) -> kurbo::Rect {

#### Parley

- Adjust the advance of the whole line when justifying text. ([#397][] by [@kekelp][])
- Selection extension moves the focus to the side being extended. ([#385][] by [@kekelp][])
- Ranged builder default style not respecting `scale`. ([#368][] by [@xStrom][])
- Cluster source character not correct. ([#402][] by [@taj-p][])
Expand Down Expand Up @@ -438,6 +439,7 @@ This release has an [MSRV][] of 1.70.
[#451]: https://github.com/linebender/parley/pull/451
[#467]: https://github.com/linebender/parley/pull/467
[#468]: https://github.com/linebender/parley/pull/468
[#397]: https://github.com/linebender/parley/pull/397

[Unreleased]: https://github.com/linebender/parley/compare/v0.7.0...HEAD
[0.7.0]: https://github.com/linebender/parley/compare/v0.6.0...v0.7.0
Expand Down
3 changes: 2 additions & 1 deletion parley/src/layout/alignment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ fn align_impl<B: Brush, const UNDO_JUSTIFICATION: bool>(

// Compute free space.
let free_space =
layout.alignment_width - line.metrics.advance + line.metrics.trailing_whitespace;
layout.alignment_width - line.unjustified_advance + line.metrics.trailing_whitespace;

if !options.align_when_overflowing && free_space <= 0.0 {
if is_rtl {
Expand Down Expand Up @@ -177,6 +177,7 @@ fn align_impl<B: Brush, const UNDO_JUSTIFICATION: bool>(
}
if cluster.info.whitespace().is_space_or_nbsp() {
cluster.advance += adjustment;
line.metrics.advance += adjustment;
Copy link
Member

@tomcur tomcur Aug 16, 2025

Choose a reason for hiding this comment

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

Could this be set (outside the loop) as line.metrics.advance = layout.alignment_width, for all except the final line? For very wide lines with many clusters, this iterative calculation will probably become quite lossy.

Does the code in the PR currently lead to a line.metrics.advance larger than the alignment_width, as trailing whitespace is counted twice? (I haven't run this locally yet.)

Copy link
Contributor Author

@kekelp kekelp Aug 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Setting line.metrics.advance = layout.alignment_width, this is what happens:

image

The 2nd and 3rd lines use line.metrics.advance for the width of their selection box, so now it stops exactly at the end of the alignment_width. The first line calculates the selection box manually without using line.metrics.advance, and it shows that the trailing whitespace lives beyond layout.alignment_width, because the justification code ignores it.

We could just change the selection box code, of course, but the thing is that line.metrics.advance is documented as "Full advance of the line, including trailing whitespace". So I think it's correct that line.metrics.advance is larger than the alignment_width in this case.

Still, we can probably still avoid the iterative calculation by just setting line.metrics.advance = layout.alignment_width + line.metrics.trailing_whitespace. I'm not sure about the RTL line and the one with a trailing newline instead of a trailing space, but this seems to work as well:

image

applied += 1;
}
});
Expand Down
2 changes: 2 additions & 0 deletions parley/src/layout/data.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,8 @@ pub(crate) struct LineData {
pub(crate) max_advance: f32,
/// Number of justified clusters on the line.
pub(crate) num_spaces: usize,
/// Max advance for the line before justification.
pub(crate) unjustified_advance: f32,
Comment on lines +170 to +171
Copy link
Member

@tomcur tomcur Aug 16, 2025

Choose a reason for hiding this comment

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

It's probably fine to store the unjustified advance here, as the user likely doesn't need to know.

}

impl LineData {
Expand Down
1 change: 1 addition & 0 deletions parley/src/layout/line_break.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,7 @@ fn try_commit_line<B: Brush>(
advance: state.x,
..Default::default()
},
unjustified_advance: state.x,
..Default::default()
});

Expand Down