[web] Refactor to do all justify alignment during layout #31937
Conversation
mdebbar
force-pushed
the
justify_layout
branch
2 times, most recently
from
610ae0c to
8093f30
Compare
|
Gold has detected about 1 new digest(s) on patchset 6. |
|
|
||
| int spaceBoxesToJustify = line.spaceBoxCount; | ||
| // If the last box is a space box, we can't use it to justify text. | ||
| if (lastBox is SpanBox && lastBox.isSpaceOnly) { |
There was a problem hiding this comment.
Is it guaranteed that there's no more than one space-only box?
There was a problem hiding this comment.
Good point! I'll account for multiple trailing spaces.
| box.startOffset = cumulativeWidth; | ||
| box.lineWidth = line.width; | ||
| if (box is SpanBox && box.isSpaceOnly) { | ||
| box._width += justifyPerSpaceBox; |
There was a problem hiding this comment.
Do we need to skip increasing the width of the last space-only box? Or maybe the last space-only box should have zero width if the text is justified? 🤔
There was a problem hiding this comment.
Good intuition! I was going by what the browser does. But now I checked Flutter, and you're right, it collapses the width of trailing spaces!
There was a problem hiding this comment.
FWIW, Flutter always collapses trailing spaces (regardless of which alignment is used).
There was a problem hiding this comment.
Actually, I (partially) lied.
During text painting/rendering, Flutter treats trailing spaces as if they were zero-width (e.g. no background).
During text editing (cursor placement & selection highlighting), Flutter treats trailing spaces the same as other spaces.
| Left | Right | Justify |
|---|---|---|
 |
 |
 |
I believe what's happening during text editing is they have no special case for trailing spaces, but they clamp the results of getBoxesForRange to the width of the paragraph.
There was a problem hiding this comment.
Done (minus the clamping part).
There was a problem hiding this comment.
Filed an issue to implement the clamping: flutter/flutter#100567
| box.startOffset = startOffset + cumulativeWidth; | ||
| box.lineWidth = line.width; | ||
| if (box is SpanBox && box.isSpaceOnly) { | ||
| box._width += justifyPerSpaceBox; |
There was a problem hiding this comment.
ditto w.r.t. last space-only box
|
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. |
|
Golden file changes are available for triage from new commit, Click here to view. |
mdebbar
added
the
waiting for tree to go green
4 participants
Before this PR: The
justifyalignment occurs during the painting of text on canvas2d.After this PR: Do all the
justifyalignment calculation during layout.Benefits:
To achieve this, we have to move the box-positioning logic out of the
LineBuilder. It has to happen after all the lines have been constructed so thatjustifyalignment can be calculated.Fixes flutter/flutter#96533