Adjust advance when justifying text #397
Conversation
Accept suggestions Co-authored-by: Daniel McNab <[email protected]>
| /// Max advance for the line before justification. | ||
| pub(crate) unjustified_advance: f32, |
There was a problem hiding this comment.
It's probably fine to store the unjustified advance here, as the user likely doesn't need to know.
| } | ||
| if cluster.info.whitespace().is_space_or_nbsp() { | ||
| cluster.advance += adjustment; | ||
| line.metrics.advance += adjustment; |
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
Setting line.metrics.advance = layout.alignment_width, this is what happens:
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:
3 participants
Resolves #396.