Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[web] Refactor to do all justify alignment during layout #31937

Merged
fluttergithubbot merged 5 commits into from
Mar 22, 2022
Merged

[web] Refactor to do all justify alignment during layout #31937

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
170decf
[web] Refactor to do all justify alignment during layout
mdebbar Mar 7, 2022
942aeb1
Merge branch 'main' into justify_layout
mdebbar Mar 17, 2022
59ed753
Merge branch 'main' into justify_layout
mdebbar Mar 21, 2022
6017c52
handle trailing spaces correctly
mdebbar Mar 21, 2022
f902654
fix test on safari
mdebbar Mar 22, 2022
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
274 changes: 170 additions & 104 deletions lib/web_ui/lib/src/engine/text/layout_service.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,23 @@ class TextLayoutService {
}
}

// ********************** //
// *** POSITION BOXES *** //
// ********************** //

if (lines.isNotEmpty) {
final EngineLineMetrics lastLine = lines.last;
final bool shouldJustifyParagraph =
width.isFinite &&
paragraph.paragraphStyle.textAlign == ui.TextAlign.justify;

for (final EngineLineMetrics line in lines) {
// Don't apply justification to the last line.
final bool shouldJustifyLine = shouldJustifyParagraph && line != lastLine;
_positionLineBoxes(line, withJustification: shouldJustifyLine);
}
}

// ******************************** //
// *** MAX/MIN INTRINSIC WIDTHS *** //
// ******************************** //
Expand Down Expand Up @@ -270,6 +287,145 @@ class TextLayoutService {
}
}

ui.TextDirection get _paragraphDirection =>
paragraph.paragraphStyle.effectiveTextDirection;

/// Positions the boxes in the given [line] and takes into account their
/// directions, the paragraph's direction, and alignment justification.
void _positionLineBoxes(EngineLineMetrics line, {
required bool withJustification,
}) {
final List<RangeBox> boxes = line.boxes;
final double justifyPerSpaceBox = withJustification
? _calculateJustifyPerSpaceBox(line)
: 0.0;

int i = 0;
double cumulativeWidth = 0.0;
while (i < boxes.length) {
final RangeBox box = boxes[i];
if (box.boxDirection == _paragraphDirection) {
// The box is in the same direction as the paragraph.
box.startOffset = cumulativeWidth;
box.lineWidth = line.width;
if (box is SpanBox && box.isSpaceOnly) {
box._width += justifyPerSpaceBox;
Copy link
Contributor

@yjbanov yjbanov Mar 16, 2022

Choose a reason for hiding this comment

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

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? 🤔

Copy link
Contributor Author

@mdebbar mdebbar Mar 21, 2022

Choose a reason for hiding this comment

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

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!

Copy link
Contributor Author

@mdebbar mdebbar Mar 21, 2022

Choose a reason for hiding this comment

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

FWIW, Flutter always collapses trailing spaces (regardless of which alignment is used).

Copy link
Contributor Author

@mdebbar mdebbar Mar 21, 2022

Choose a reason for hiding this comment

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

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
image image image

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.

Copy link
Contributor Author

@mdebbar mdebbar Mar 21, 2022

Choose a reason for hiding this comment

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

Done (minus the clamping part).

Copy link
Contributor Author

@mdebbar mdebbar Mar 22, 2022

Choose a reason for hiding this comment

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

Filed an issue to implement the clamping: flutter/flutter#100567

}

cumulativeWidth += box.width;
i++;
continue;
}

// At this point, we found a box that has the opposite direction to the
// paragraph. This could be a sequence of one or more boxes.
//
// These boxes should flow in the opposite direction. So we need to
// position them in reverse order.
//
// If the last box in the sequence is a space-only box (contains only
// whitespace characters), it should be excluded from the sequence.
//
// Example: an LTR paragraph with the contents:
//
// "ABC rtl1 rtl2 rtl3 XYZ"
// ^ ^ ^ ^
// SP1 SP2 SP3 SP4
//
//
// box direction: LTR RTL LTR
// |------>|<-----------------------|------>
// +----------------------------------------+
// | ABC | | rtl3 | | rtl2 | | rtl1 | | XYZ |
// +----------------------------------------+
// ^ ^ ^ ^
// SP1 SP3 SP2 SP4
//
// Notice how SP2 and SP3 are flowing in the RTL direction because of the
// surrounding RTL words. SP4 is also preceded by an RTL word, but it marks
// the end of the RTL sequence, so it goes back to flowing in the paragraph
// direction (LTR).

final int first = i;
int lastNonSpaceBox = first;
i++;
while (i < boxes.length && boxes[i].boxDirection != _paragraphDirection) {
final RangeBox box = boxes[i];
if (box is SpanBox && box.isSpaceOnly) {
// Do nothing.
} else {
lastNonSpaceBox = i;
}
i++;
}
final int last = lastNonSpaceBox;
i = lastNonSpaceBox + 1;

// The range (first:last) is the entire sequence of boxes that have the
// opposite direction to the paragraph.
final double sequenceWidth = _positionLineBoxesInReverse(
line,
first,
last,
startOffset: cumulativeWidth,
justifyPerSpaceBox: justifyPerSpaceBox,
);
cumulativeWidth += sequenceWidth;
}
}

/// Positions a sequence of boxes in the direction opposite to the paragraph
/// text direction.
///
/// This is needed when a right-to-left sequence appears in the middle of a
/// left-to-right paragraph, or vice versa.
///
/// Returns the total width of all the positioned boxes in the sequence.
///
/// [first] and [last] are expected to be inclusive.
double _positionLineBoxesInReverse(
EngineLineMetrics line,
int first,
int last, {
required double startOffset,
required double justifyPerSpaceBox,
}) {
final List<RangeBox> boxes = line.boxes;
double cumulativeWidth = 0.0;
for (int i = last; i >= first; i--) {
// Update the visual position of each box.
final RangeBox box = boxes[i];
assert(box.boxDirection != _paragraphDirection);
box.startOffset = startOffset + cumulativeWidth;
box.lineWidth = line.width;
if (box is SpanBox && box.isSpaceOnly) {
box._width += justifyPerSpaceBox;
Copy link
Contributor

@yjbanov yjbanov Mar 16, 2022

Choose a reason for hiding this comment

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

ditto w.r.t. last space-only box

Copy link
Contributor Author

@mdebbar mdebbar Mar 21, 2022

Choose a reason for hiding this comment

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

Done.

}

cumulativeWidth += box.width;
}
return cumulativeWidth;
}

/// Calculates for the given [line], the amount of extra width that needs to be
/// added to each space box in order to align the line with the rest of the
/// paragraph.
double _calculateJustifyPerSpaceBox(EngineLineMetrics line) {
final double justifyTotal = paragraph.width - line.width;
final RangeBox lastBox = line.boxes.last;

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) {
Copy link
Contributor

@yjbanov yjbanov Mar 16, 2022

Choose a reason for hiding this comment

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

Is it guaranteed that there's no more than one space-only box?

Copy link
Contributor Author

@mdebbar mdebbar Mar 21, 2022

Choose a reason for hiding this comment

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

Good point! I'll account for multiple trailing spaces.

Copy link
Contributor Author

@mdebbar mdebbar Mar 21, 2022

Choose a reason for hiding this comment

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

Done.

spaceBoxesToJustify--;
}
if (spaceBoxesToJustify > 0) {
return justifyTotal / spaceBoxesToJustify;
}

return 0.0;
}

List<ui.TextBox> getBoxesForPlaceholders() {
final List<ui.TextBox> boxes = <ui.TextBox>[];
for (final EngineLineMetrics line in lines) {
Expand Down Expand Up @@ -396,7 +552,6 @@ abstract class RangeBox {
RangeBox(
this.start,
this.end,
this.width,
this.paragraphDirection,
this.boxDirection,
);
Expand All @@ -421,7 +576,7 @@ abstract class RangeBox {
: lineWidth - startOffset;

/// The distance from the left edge of the box to the right edge of the box.
final double width;
double get width;

/// The width of the line that this box belongs to.
late final double lineWidth;
Expand Down Expand Up @@ -464,10 +619,13 @@ class PlaceholderBox extends RangeBox {
required LineBreakResult index,
required ui.TextDirection paragraphDirection,
required ui.TextDirection boxDirection,
}) : super(index, index, placeholder.width, paragraphDirection, boxDirection);
}) : super(index, index, paragraphDirection, boxDirection);

final PlaceholderSpan placeholder;

@override
double get width => placeholder.width;

@override
ui.TextBox toTextBox(EngineLineMetrics line) {
final double left = line.left + this.left;
Expand Down Expand Up @@ -536,7 +694,8 @@ class SpanBox extends RangeBox {
}) : span = spanometer.currentSpan,
height = spanometer.height,
baseline = spanometer.ascent,
super(start, end, width, paragraphDirection, boxDirection);
_width = width,
super(start, end, paragraphDirection, boxDirection);


final Spanometer spanometer;
Expand Down Expand Up @@ -566,6 +725,13 @@ class SpanBox extends RangeBox {
/// Whether this box is made of only white space.
final bool isSpaceOnly;

/// This is made mutable so it can be updated later in the layout process for
/// the purpose of aligning the lines of a paragraph with [ui.TextAlign.justify].
double _width;

@override
double get width => _width;

/// Whether the contents of this box flow in the left-to-right direction.
bool get isContentLtr => contentDirection == ui.TextDirection.ltr;

Expand Down Expand Up @@ -1290,7 +1456,6 @@ class LineBuilder {
EngineLineMetrics build({String? ellipsis}) {
// At the end of each line, we cut the last box of the line.
createBox();
_positionBoxes();

final double ellipsisWidth =
ellipsis == null ? 0.0 : spanometer.measureText(ellipsis);
Expand Down Expand Up @@ -1321,105 +1486,6 @@ class LineBuilder {
);
}

/// Positions the boxes and takes into account their directions, and the
/// paragraph's direction.
void _positionBoxes() {
final List<RangeBox> boxes = _boxes;

int i = 0;
double cumulativeWidth = 0.0;
while (i < boxes.length) {
final RangeBox box = boxes[i];
if (box.boxDirection == _paragraphDirection) {
// The box is in the same direction as the paragraph.
box.startOffset = cumulativeWidth;
box.lineWidth = width;

cumulativeWidth += box.width;
i++;
continue;
}

// At this point, we found a box that has the opposite direction to the
// paragraph. This could be a sequence of one or more boxes.
//
// These boxes should flow in the opposite direction. So we need to
// position them in reverse order.
//
// If the last box in the sequence is a space-only box (contains only
// whitespace characters), it should be excluded from the sequence.
//
// Example: an LTR paragraph with the contents:
//
// "ABC rtl1 rtl2 rtl3 XYZ"
// ^ ^ ^ ^
// SP1 SP2 SP3 SP4
//
//
// box direction: LTR RTL LTR
// |------>|<-----------------------|------>
// +----------------------------------------+
// | ABC | | rtl3 | | rtl2 | | rtl1 | | XYZ |
// +----------------------------------------+
// ^ ^ ^ ^
// SP1 SP3 SP2 SP4
//
// Notice how SP2 and SP3 are flowing in the RTL direction because of the
// surrounding RTL words. SP4 is also preceded by an RTL word, but it marks
// the end of the RTL sequence, so it goes back to flowing in the paragraph
// direction (LTR).

final int first = i;
int lastNonSpaceBox = first;
i++;
while (i < boxes.length && boxes[i].boxDirection != _paragraphDirection) {
final RangeBox box = boxes[i];
if (box is SpanBox && box.isSpaceOnly) {
// Do nothing.
} else {
lastNonSpaceBox = i;
}
i++;
}
final int last = lastNonSpaceBox;
i = lastNonSpaceBox + 1;

// The range (first:last) is the entire sequence of boxes that have the
// opposite direction to the paragraph.
final double sequenceWidth =
_positionBoxesInReverse(boxes, first, last, startOffset: cumulativeWidth);
cumulativeWidth += sequenceWidth;
}
}

/// Positions a sequence of boxes in the direction opposite to the paragraph
/// text direction.
///
/// This is needed when a right-to-left sequence appears in the middle of a
/// left-to-right paragraph, or vice versa.
///
/// Returns the total width of all the positioned boxes in the sequence.
///
/// [first] and [last] are expected to be inclusive.
double _positionBoxesInReverse(
List<RangeBox> boxes,
int first,
int last, {
required double startOffset,
}) {
double cumulativeWidth = 0.0;
for (int i = last; i >= first; i--) {
// Update the visual position of each box.
final RangeBox box = boxes[i];
assert(box.boxDirection != _paragraphDirection);
box.startOffset = startOffset + cumulativeWidth;
box.lineWidth = width;

cumulativeWidth += box.width;
}
return cumulativeWidth;
}

LineBreakResult? _cachedNextBreak;

/// Finds the next line break after the end of this line.
Expand Down
Loading