From 170decfa14df39930dfa1918a8dcc227e3f5f0c2 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Mon, 7 Mar 2022 14:12:00 -0500 Subject: [PATCH 1/3] [web] Refactor to do all justify alignment during layout --- .../lib/src/engine/text/layout_service.dart | 274 +++++++++++------- .../lib/src/engine/text/paint_service.dart | 50 +--- .../test/html/paragraph/bidi_golden_test.dart | 13 +- lib/web_ui/test/html/paragraph/helper.dart | 26 ++ .../html/paragraph/justify_golden_test.dart | 193 +++++++++++- .../paragraph/placeholders_golden_test.dart | 10 - 6 files changed, 383 insertions(+), 183 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text/layout_service.dart b/lib/web_ui/lib/src/engine/text/layout_service.dart index ae9ebce657bc5..f6f1106215a6b 100644 --- a/lib/web_ui/lib/src/engine/text/layout_service.dart +++ b/lib/web_ui/lib/src/engine/text/layout_service.dart @@ -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 *** // // ******************************** // @@ -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 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; + } + + 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 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; + } + + 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) { + spaceBoxesToJustify--; + } + if (spaceBoxesToJustify > 0) { + return justifyTotal / spaceBoxesToJustify; + } + + return 0.0; + } + List getBoxesForPlaceholders() { final List boxes = []; for (final EngineLineMetrics line in lines) { @@ -396,7 +552,6 @@ abstract class RangeBox { RangeBox( this.start, this.end, - this.width, this.paragraphDirection, this.boxDirection, ); @@ -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; @@ -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; @@ -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; @@ -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; @@ -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); @@ -1321,105 +1486,6 @@ class LineBuilder { ); } - /// Positions the boxes and takes into account their directions, and the - /// paragraph's direction. - void _positionBoxes() { - final List 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 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. diff --git a/lib/web_ui/lib/src/engine/text/paint_service.dart b/lib/web_ui/lib/src/engine/text/paint_service.dart index 524c6a12db4ce..4f5b8fadcf56c 100644 --- a/lib/web_ui/lib/src/engine/text/paint_service.dart +++ b/lib/web_ui/lib/src/engine/text/paint_service.dart @@ -26,17 +26,12 @@ class TextPaintService { return; } - final EngineLineMetrics lastLine = lines.last; for (final EngineLineMetrics line in lines) { if (line.boxes.isEmpty) { continue; } final RangeBox lastBox = line.boxes.last; - final double justifyPerSpaceBox = - _calculateJustifyPerSpaceBox(paragraph, line, lastLine, lastBox); - - ui.Offset justifiedOffset = offset; for (final RangeBox box in line.boxes) { final bool isTrailingSpaceBox = @@ -44,13 +39,9 @@ class TextPaintService { // Don't paint background for the trailing space in the line. if (!isTrailingSpaceBox) { - _paintBackground(canvas, justifiedOffset, line, box, justifyPerSpaceBox); - } - _paintText(canvas, justifiedOffset, line, box); - - if (box is SpanBox && box.isSpaceOnly && justifyPerSpaceBox != 0.0) { - justifiedOffset = justifiedOffset.translate(justifyPerSpaceBox, 0.0); + _paintBackground(canvas, offset, line, box); } + _paintText(canvas, offset, line, box); } } } @@ -60,7 +51,6 @@ class TextPaintService { ui.Offset offset, EngineLineMetrics line, RangeBox box, - double justifyPerSpaceBox, ) { if (box is SpanBox) { final FlatTextSpan span = box.span; @@ -68,13 +58,7 @@ class TextPaintService { // Paint the background of the box, if the span has a background. final SurfacePaint? background = span.style.background as SurfacePaint?; if (background != null) { - ui.Rect rect = box.toTextBox(line).toRect().shift(offset); - if (box.isSpaceOnly) { - rect = ui.Rect.fromPoints( - rect.topLeft, - rect.bottomRight.translate(justifyPerSpaceBox, 0.0), - ); - } + final ui.Rect rect = box.toTextBox(line).toRect().shift(offset); canvas.drawRect(rect, background.paintData); } } @@ -144,31 +128,3 @@ class TextPaintService { canvas.setUpPaint(paint.paintData, null); } } - -/// 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( - CanvasParagraph paragraph, - EngineLineMetrics line, - EngineLineMetrics lastLine, - RangeBox lastBox, -) { - // Don't apply any justification on the last line. - if (line != lastLine && - paragraph.width.isFinite && - paragraph.paragraphStyle.textAlign == ui.TextAlign.justify) { - final double justifyTotal = paragraph.width - line.width; - - 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) { - spaceBoxesToJustify--; - } - if (spaceBoxesToJustify > 0) { - return justifyTotal / spaceBoxesToJustify; - } - } - - return 0.0; -} diff --git a/lib/web_ui/test/html/paragraph/bidi_golden_test.dart b/lib/web_ui/test/html/paragraph/bidi_golden_test.dart index 1132b39e56928..c9a459fc31e61 100644 --- a/lib/web_ui/test/html/paragraph/bidi_golden_test.dart +++ b/lib/web_ui/test/html/paragraph/bidi_golden_test.dart @@ -487,11 +487,11 @@ Future testMain() async { final Offset offset = Offset(bounds.left + 5, bounds.top + y + 5); // Range for "em 12 " and the first character of `_rtlWord1`. - paintBoxes(canvas, offset, paragraph.getBoxesForRange(3, 10), lightBlue); + fillBoxes(canvas, offset, paragraph.getBoxesForRange(3, 10), lightBlue); // Range for the second half of `_rtlWord1` and all of `_rtlWord2` and " 3". - paintBoxes(canvas, offset, paragraph.getBoxesForRange(11, 21), lightPurple); + fillBoxes(canvas, offset, paragraph.getBoxesForRange(11, 21), lightPurple); // Range for "psum dolo". - paintBoxes(canvas, offset, paragraph.getBoxesForRange(24, 33), green); + fillBoxes(canvas, offset, paragraph.getBoxesForRange(24, 33), green); canvas.drawParagraph(paragraph, offset); } @@ -574,10 +574,3 @@ Future testMain() async { return takeScreenshot(canvas, bounds, 'canvas_paragraph_bidi_selection_dom'); }); } - -void paintBoxes(EngineCanvas canvas, Offset offset, List boxes, Color color) { - for (final TextBox box in boxes) { - final Rect rect = box.toRect().shift(offset); - canvas.drawRect(rect, SurfacePaintData()..color = color); - } -} diff --git a/lib/web_ui/test/html/paragraph/helper.dart b/lib/web_ui/test/html/paragraph/helper.dart index 7c913d3bab902..419aba346b777 100644 --- a/lib/web_ui/test/html/paragraph/helper.dart +++ b/lib/web_ui/test/html/paragraph/helper.dart @@ -67,3 +67,29 @@ Future takeScreenshot( sceneElement.remove(); } } + +/// Fills the single placeholder in the given [paragraph] with a red rectangle. +/// +/// The placeholder is filled relative to [offset]. +/// +/// Throws if the paragraph contains more than one placeholder. +void fillPlaceholder( + EngineCanvas canvas, + Offset offset, + CanvasParagraph paragraph, +) { + final TextBox placeholderBox = paragraph.getBoxesForPlaceholders().single; + final SurfacePaint paint = SurfacePaint()..color = red; + canvas.drawRect(placeholderBox.toRect().shift(offset), paint.paintData); +} + + +/// Fill the given [boxes] with rectangles of the given [color]. +/// +/// All rectangles are filled relative to [offset]. +void fillBoxes(EngineCanvas canvas, Offset offset, List boxes, Color color) { + for (final TextBox box in boxes) { + final Rect rect = box.toRect().shift(offset); + canvas.drawRect(rect, SurfacePaintData()..color = color); + } +} diff --git a/lib/web_ui/test/html/paragraph/justify_golden_test.dart b/lib/web_ui/test/html/paragraph/justify_golden_test.dart index b64470741541b..f68725f484c42 100644 --- a/lib/web_ui/test/html/paragraph/justify_golden_test.dart +++ b/lib/web_ui/test/html/paragraph/justify_golden_test.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'dart:async'; +import 'dart:html' as html; import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; @@ -19,10 +20,7 @@ void main() { Future testMain() async { setUpStableTestFonts(); - test('TextAlign.justify with multiple spans', () { - const Rect bounds = Rect.fromLTWH(0, 0, 400, 400); - final BitmapCanvas canvas = BitmapCanvas(bounds, RenderStrategy()); - + void testJustifyWithMultipleSpans(EngineCanvas canvas) { void build(CanvasParagraphBuilder builder) { builder.pushStyle(EngineTextStyle.only(color: black)); builder.addText('Lorem ipsum dolor sit '); @@ -49,14 +47,23 @@ Future testMain() async { ); paragraph.layout(constrain(250.0)); canvas.drawParagraph(paragraph, Offset.zero); + } + test('TextAlign.justify with multiple spans', () { + const Rect bounds = Rect.fromLTWH(0, 0, 400, 400); + final BitmapCanvas canvas = BitmapCanvas(bounds, RenderStrategy()); + testJustifyWithMultipleSpans(canvas); return takeScreenshot(canvas, bounds, 'canvas_paragraph_justify'); }); - test('TextAlign.justify with single space and empty line', () { + test('TextAlign.justify with multiple spans (DOM)', () { const Rect bounds = Rect.fromLTWH(0, 0, 400, 400); - final BitmapCanvas canvas = BitmapCanvas(bounds, RenderStrategy()); + final DomCanvas canvas = DomCanvas(html.document.createElement('flt-picture')); + testJustifyWithMultipleSpans(canvas); + return takeScreenshot(canvas, bounds, 'canvas_paragraph_justify_dom'); + }); + void testJustifyWithEmptyLine(EngineCanvas canvas) { void build(CanvasParagraphBuilder builder) { builder.pushStyle(bg(yellow)); builder.pushStyle(EngineTextStyle.only(color: black)); @@ -79,15 +86,25 @@ Future testMain() async { ); paragraph.layout(constrain(250.0)); canvas.drawParagraph(paragraph, Offset.zero); + } + test('TextAlign.justify with single space and empty line', () { + const Rect bounds = Rect.fromLTWH(0, 0, 400, 400); + final BitmapCanvas canvas = BitmapCanvas(bounds, RenderStrategy()); + testJustifyWithEmptyLine(canvas); return takeScreenshot( canvas, bounds, 'canvas_paragraph_justify_empty_line'); }); - test('TextAlign.justify with ellipsis', () { - const Rect bounds = Rect.fromLTWH(0, 0, 400, 300); - final BitmapCanvas canvas = BitmapCanvas(bounds, RenderStrategy()); + test('TextAlign.justify with single space and empty line (DOM)', () { + const Rect bounds = Rect.fromLTWH(0, 0, 400, 400); + final DomCanvas canvas = DomCanvas(html.document.createElement('flt-picture')); + testJustifyWithEmptyLine(canvas); + return takeScreenshot( + canvas, bounds, 'canvas_paragraph_justify_empty_line_dom'); + }); + void testJustifyWithEllipsis(EngineCanvas canvas) { final EngineParagraphStyle paragraphStyle = EngineParagraphStyle( fontFamily: 'Roboto', fontSize: 20.0, @@ -116,14 +133,23 @@ Future testMain() async { ); paragraph.layout(constrain(250)); canvas.drawParagraph(paragraph, Offset.zero); + } + test('TextAlign.justify with ellipsis', () { + const Rect bounds = Rect.fromLTWH(0, 0, 400, 300); + final BitmapCanvas canvas = BitmapCanvas(bounds, RenderStrategy()); + testJustifyWithEllipsis(canvas); return takeScreenshot(canvas, bounds, 'canvas_paragraph_justify_ellipsis'); }); - test('TextAlign.justify with background', () { - const Rect bounds = Rect.fromLTWH(0, 0, 400, 400); - final BitmapCanvas canvas = BitmapCanvas(bounds, RenderStrategy()); + test('TextAlign.justify with ellipsis (DOM)', () { + const Rect bounds = Rect.fromLTWH(0, 0, 400, 300); + final DomCanvas canvas = DomCanvas(html.document.createElement('flt-picture')); + testJustifyWithEllipsis(canvas); + return takeScreenshot(canvas, bounds, 'canvas_paragraph_justify_ellipsis_dom'); + }); + void testJustifyWithBackground(EngineCanvas canvas) { final EngineParagraphStyle paragraphStyle = EngineParagraphStyle( fontFamily: 'Roboto', fontSize: 20.0, @@ -153,10 +179,153 @@ Future testMain() async { ); paragraph.layout(constrain(250)); canvas.drawParagraph(paragraph, Offset.zero); + } + test('TextAlign.justify with background', () { + const Rect bounds = Rect.fromLTWH(0, 0, 400, 400); + final BitmapCanvas canvas = BitmapCanvas(bounds, RenderStrategy()); + testJustifyWithBackground(canvas); return takeScreenshot( canvas, bounds, 'canvas_paragraph_justify_background'); }); + + test('TextAlign.justify with background (DOM)', () { + const Rect bounds = Rect.fromLTWH(0, 0, 400, 400); + final DomCanvas canvas = DomCanvas(html.document.createElement('flt-picture')); + testJustifyWithBackground(canvas); + return takeScreenshot( + canvas, bounds, 'canvas_paragraph_justify_background_dom'); + }); + + void testJustifyWithPlaceholder(EngineCanvas canvas) { + void build(CanvasParagraphBuilder builder) { + builder.pushStyle(EngineTextStyle.only(color: black)); + builder.addText('Lorem ipsum dolor sit '); + builder.pushStyle(EngineTextStyle.only(color: blue)); + builder.addText('amet, consectetur '); + builder.pushStyle(EngineTextStyle.only(color: green)); + builder.addText('adipiscing elit, sed do '); + builder.addPlaceholder(40, 40, PlaceholderAlignment.bottom); + builder.addText(' eiusmod tempor incididunt ut '); + builder.pushStyle(EngineTextStyle.only(color: red)); + builder.addText('labore et dolore magna aliqua.'); + } + + final CanvasParagraph paragraph = rich( + EngineParagraphStyle( + fontFamily: 'Roboto', + fontSize: 20.0, + textAlign: TextAlign.justify, + ), + build, + ); + paragraph.layout(constrain(250.0)); + canvas.drawParagraph(paragraph, Offset.zero); + fillPlaceholder(canvas, Offset.zero, paragraph); + } + + test('TextAlign.justify with placeholder', () { + const Rect bounds = Rect.fromLTWH(0, 0, 400, 400); + final BitmapCanvas canvas = BitmapCanvas(bounds, RenderStrategy()); + testJustifyWithPlaceholder(canvas); + return takeScreenshot(canvas, bounds, 'canvas_paragraph_justify_placeholder'); + }); + + test('TextAlign.justify with placeholder (DOM)', () { + const Rect bounds = Rect.fromLTWH(0, 0, 400, 400); + final DomCanvas canvas = DomCanvas(html.document.createElement('flt-picture')); + testJustifyWithPlaceholder(canvas); + return takeScreenshot(canvas, bounds, 'canvas_paragraph_justify_placeholder_dom'); + }); + + void testJustifyWithSelection(EngineCanvas canvas) { + void build(CanvasParagraphBuilder builder) { + builder.pushStyle(EngineTextStyle.only(color: black)); + builder.addText('Lorem ipsum dolor sit '); + builder.pushStyle(EngineTextStyle.only(color: blue)); + builder.addText('amet, consectetur '); // 40 + builder.pushStyle(EngineTextStyle.only(color: green)); + builder.addText('adipiscing elit, sed do eiusmod tempor incididunt ut '); + builder.pushStyle(EngineTextStyle.only(color: red)); + builder.addText('labore et dolore magna aliqua.'); + } + + final CanvasParagraph paragraph = rich( + EngineParagraphStyle( + fontFamily: 'Roboto', + fontSize: 20.0, + textAlign: TextAlign.justify, + ), + build, + ); + paragraph.layout(constrain(250.0)); + + // Draw selection for "em ipsum d". + fillBoxes(canvas, Offset.zero, paragraph.getBoxesForRange(3, 13), lightBlue); + // Draw selection for " ut labore et dolore mag". + fillBoxes(canvas, Offset.zero, paragraph.getBoxesForRange(89, 113), lightPurple); + + canvas.drawParagraph(paragraph, Offset.zero); + } + + test('TextAlign.justify with selection', () { + const Rect bounds = Rect.fromLTWH(0, 0, 400, 400); + final BitmapCanvas canvas = BitmapCanvas(bounds, RenderStrategy()); + testJustifyWithSelection(canvas); + return takeScreenshot(canvas, bounds, 'canvas_paragraph_justify_selection'); + }); + + test('TextAlign.justify with selection (DOM)', () { + const Rect bounds = Rect.fromLTWH(0, 0, 400, 400); + final DomCanvas canvas = DomCanvas(html.document.createElement('flt-picture')); + testJustifyWithSelection(canvas); + return takeScreenshot(canvas, bounds, 'canvas_paragraph_justify_selection_dom'); + }); + + void testJustifyRtl(EngineCanvas canvas) { + const String rtlWord = 'مرحبا'; + void build(CanvasParagraphBuilder builder) { + builder.pushStyle(EngineTextStyle.only(color: black)); + builder.addText('Lorem $rtlWord dolor sit '); + builder.pushStyle(EngineTextStyle.only(color: blue)); + builder.addText('amet, consectetur '); // 40 + builder.pushStyle(EngineTextStyle.only(color: green)); + builder.addText('adipiscing elit, sed do eiusmod $rtlWord incididunt ut '); + builder.pushStyle(EngineTextStyle.only(color: red)); + builder.addText('labore et dolore magna aliqua.'); + } + + final CanvasParagraph paragraph = rich( + EngineParagraphStyle( + fontFamily: 'Roboto', + fontSize: 20.0, + textAlign: TextAlign.justify, + ), + build, + ); + paragraph.layout(constrain(250.0)); + + // Draw selection for "em $rtlWord d". + fillBoxes(canvas, Offset.zero, paragraph.getBoxesForRange(3, 13), lightBlue); + // Draw selection for " ut labore et dolore mag". + fillBoxes(canvas, Offset.zero, paragraph.getBoxesForRange(89, 113), lightPurple); + + canvas.drawParagraph(paragraph, Offset.zero); + } + + test('TextAlign.justify rtl', () { + const Rect bounds = Rect.fromLTWH(0, 0, 400, 400); + final BitmapCanvas canvas = BitmapCanvas(bounds, RenderStrategy()); + testJustifyRtl(canvas); + return takeScreenshot(canvas, bounds, 'canvas_paragraph_justify_rtl'); + }); + + test('TextAlign.justify rtl (DOM)', () { + const Rect bounds = Rect.fromLTWH(0, 0, 400, 400); + final DomCanvas canvas = DomCanvas(html.document.createElement('flt-picture')); + testJustifyRtl(canvas); + return takeScreenshot(canvas, bounds, 'canvas_paragraph_justify_rtl_dom'); + }); } EngineTextStyle bg(Color color) { diff --git a/lib/web_ui/test/html/paragraph/placeholders_golden_test.dart b/lib/web_ui/test/html/paragraph/placeholders_golden_test.dart index 4118621a6ec6a..2e6ff80333a9d 100644 --- a/lib/web_ui/test/html/paragraph/placeholders_golden_test.dart +++ b/lib/web_ui/test/html/paragraph/placeholders_golden_test.dart @@ -194,13 +194,3 @@ void surroundParagraph( final SurfacePaint paint = SurfacePaint()..color = blue..style = PaintingStyle.stroke; canvas.drawRect(rect, paint.paintData); } - -void fillPlaceholder( - EngineCanvas canvas, - Offset offset, - CanvasParagraph paragraph, -) { - final TextBox placeholderBox = paragraph.getBoxesForPlaceholders().single; - final SurfacePaint paint = SurfacePaint()..color = red; - canvas.drawRect(placeholderBox.toRect().shift(offset), paint.paintData); -} From 6017c52672b47a74191d222fc420273a9598e4e1 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Mon, 21 Mar 2022 16:01:50 -0400 Subject: [PATCH 2/3] handle trailing spaces correctly --- .../lib/src/engine/text/canvas_paragraph.dart | 8 +- .../lib/src/engine/text/layout_service.dart | 75 +++++++++++++------ .../lib/src/engine/text/paint_service.dart | 2 +- lib/web_ui/lib/src/engine/text/paragraph.dart | 10 ++- .../text/canvas_paragraph_builder_test.dart | 69 +++++++++-------- 5 files changed, 106 insertions(+), 58 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart b/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart index 07a150db343b5..9d86fd49dda1d 100644 --- a/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart +++ b/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart @@ -315,11 +315,13 @@ void _applySpanStylesToParagraph({ } void _positionSpanElement(html.Element element, EngineLineMetrics line, RangeBox box) { - final ui.TextBox textBox = box.toTextBox(line); + final ui.Rect boxRect = box.toTextBox(line, forPainting: true).toRect(); element.style ..position = 'absolute' - ..top = '${textBox.top}px' - ..left = '${textBox.left}px'; + ..top = '${boxRect.top}px' + ..left = '${boxRect.left}px' + // This is needed for space-only spans that are used to justify the paragraph. + ..width = '${boxRect.width}px'; } /// A common interface for all types of spans that make up a paragraph. diff --git a/lib/web_ui/lib/src/engine/text/layout_service.dart b/lib/web_ui/lib/src/engine/text/layout_service.dart index f6f1106215a6b..ab3cc21ecad20 100644 --- a/lib/web_ui/lib/src/engine/text/layout_service.dart +++ b/lib/web_ui/lib/src/engine/text/layout_service.dart @@ -308,7 +308,7 @@ class TextLayoutService { // The box is in the same direction as the paragraph. box.startOffset = cumulativeWidth; box.lineWidth = line.width; - if (box is SpanBox && box.isSpaceOnly) { + if (box is SpanBox && box.isSpaceOnly && !box.isTrailingSpace) { box._width += justifyPerSpaceBox; } @@ -398,7 +398,7 @@ class TextLayoutService { assert(box.boxDirection != _paragraphDirection); box.startOffset = startOffset + cumulativeWidth; box.lineWidth = line.width; - if (box is SpanBox && box.isSpaceOnly) { + if (box is SpanBox && box.isSpaceOnly && !box.isTrailingSpace) { box._width += justifyPerSpaceBox; } @@ -411,14 +411,9 @@ class TextLayoutService { /// 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; + final double justifyTotal = width - line.width; - 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) { - spaceBoxesToJustify--; - } + final int spaceBoxesToJustify = line.nonTrailingSpaceBoxCount; if (spaceBoxesToJustify > 0) { return justifyTotal / spaceBoxesToJustify; } @@ -431,7 +426,7 @@ class TextLayoutService { for (final EngineLineMetrics line in lines) { for (final RangeBox box in line.boxes) { if (box is PlaceholderBox) { - boxes.add(box.toTextBox(line)); + boxes.add(box.toTextBox(line, forPainting: false)); } } } @@ -461,7 +456,7 @@ class TextLayoutService { if (line.overlapsWith(start, end)) { for (final RangeBox box in line.boxes) { if (box is SpanBox && box.overlapsWith(start, end)) { - boxes.add(box.intersect(line, start, end)); + boxes.add(box.intersect(line, start, end, forPainting: false)); } } } @@ -602,7 +597,12 @@ abstract class RangeBox { /// /// The coordinates of the resulting [ui.TextBox] are relative to the /// paragraph, not to the line. - ui.TextBox toTextBox(EngineLineMetrics line); + /// + /// The [forPainting] parameter specifies whether the text box is wanted for + /// painting purposes or not. The difference is observed in the handling of + /// trailing spaces. Trailing spaces aren't painted on the screen, but their + /// dimensions are still useful for other cases like highlighting selection. + ui.TextBox toTextBox(EngineLineMetrics line, {required bool forPainting}); /// Returns the text position within this box's range that's closest to the /// given [x] offset. @@ -627,7 +627,7 @@ class PlaceholderBox extends RangeBox { double get width => placeholder.width; @override - ui.TextBox toTextBox(EngineLineMetrics line) { + ui.TextBox toTextBox(EngineLineMetrics line, {required bool forPainting}) { final double left = line.left + this.left; final double right = line.left + this.right; @@ -725,6 +725,10 @@ class SpanBox extends RangeBox { /// Whether this box is made of only white space. final bool isSpaceOnly; + /// Whether this box is a trailing space box at the end of a line. + bool get isTrailingSpace => _isTrailingSpace; + bool _isTrailingSpace = false; + /// 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; @@ -758,13 +762,9 @@ class SpanBox extends RangeBox { return spanometer.paragraph.toPlainText().substring(start.index, end.indexWithoutTrailingNewlines); } - /// Returns a [ui.TextBox] representing this range box in the given [line]. - /// - /// The coordinates of the resulting [ui.TextBox] are relative to the - /// paragraph, not to the line. @override - ui.TextBox toTextBox(EngineLineMetrics line) { - return intersect(line, start.index, end.index); + ui.TextBox toTextBox(EngineLineMetrics line, {required bool forPainting}) { + return intersect(line, start.index, end.index, forPainting: forPainting); } /// Performs the intersection of this box with the range given by [start] and @@ -772,7 +772,7 @@ class SpanBox extends RangeBox { /// /// The coordinates of the resulting [ui.TextBox] are relative to the /// paragraph, not to the line. - ui.TextBox intersect(EngineLineMetrics line, int start, int end) { + ui.TextBox intersect(EngineLineMetrics line, int start, int end, {required bool forPainting}) { final double top = line.baseline - baseline; final double before; @@ -791,7 +791,7 @@ class SpanBox extends RangeBox { after = spanometer._measure(end, this.end.indexWithoutTrailingNewlines); } - final double left, right; + double left, right; if (isContentLtr) { // Example: let's say the text is "Loremipsum" and we want to get the box // for "rem". In this case, `before` is the width of "Lo", and `after` @@ -825,6 +825,18 @@ class SpanBox extends RangeBox { right = this.right - before; } + // When painting a paragraph, trailing spaces should have a zero width. + final bool isZeroWidth = forPainting && isTrailingSpace; + if (isZeroWidth) { + // Collapse the box to the left or to the right depending on the paragraph + // direction. + if (paragraphDirection == ui.TextDirection.ltr) { + right = left; + } else { + left = right; + } + } + // The [RangeBox]'s left and right edges are relative to the line. In order // to make them relative to the paragraph, we need to add the left edge of // the line. @@ -1467,6 +1479,9 @@ class LineBuilder { } else { hardBreak = end.isHard; } + + _processTrailingSpaces(); + return EngineLineMetrics.rich( lineNumber, ellipsis: ellipsis, @@ -1483,9 +1498,27 @@ class LineBuilder { descent: descent, boxes: _boxes, spaceBoxCount: _spaceBoxCount, + trailingSpaceBoxCount: _trailingSpaceBoxCount, ); } + int _trailingSpaceBoxCount = 0; + + void _processTrailingSpaces() { + _trailingSpaceBoxCount = 0; + for (int i = _boxes.length - 1; i >= 0; i--) { + final RangeBox box = _boxes[i]; + final bool isSpaceBox = box is SpanBox && box.isSpaceOnly; + if (!isSpaceBox) { + // We traversed all trailing space boxes. + break; + } + + box._isTrailingSpace = true; + _trailingSpaceBoxCount++; + } + } + LineBreakResult? _cachedNextBreak; /// Finds the next line break after the end of this line. diff --git a/lib/web_ui/lib/src/engine/text/paint_service.dart b/lib/web_ui/lib/src/engine/text/paint_service.dart index 4f5b8fadcf56c..59f8a0bf2a80c 100644 --- a/lib/web_ui/lib/src/engine/text/paint_service.dart +++ b/lib/web_ui/lib/src/engine/text/paint_service.dart @@ -58,7 +58,7 @@ class TextPaintService { // Paint the background of the box, if the span has a background. final SurfacePaint? background = span.style.background as SurfacePaint?; if (background != null) { - final ui.Rect rect = box.toTextBox(line).toRect().shift(offset); + final ui.Rect rect = box.toTextBox(line, forPainting: true).toRect().shift(offset); canvas.drawRect(rect, background.paintData); } } diff --git a/lib/web_ui/lib/src/engine/text/paragraph.dart b/lib/web_ui/lib/src/engine/text/paragraph.dart index 58f1e97487e44..590e126603e43 100644 --- a/lib/web_ui/lib/src/engine/text/paragraph.dart +++ b/lib/web_ui/lib/src/engine/text/paragraph.dart @@ -31,7 +31,8 @@ class EngineLineMetrics implements ui.LineMetrics { endIndexWithoutNewlines = -1, widthWithTrailingSpaces = width, boxes = [], - spaceBoxCount = 0; + spaceBoxCount = 0, + trailingSpaceBoxCount = 0; EngineLineMetrics.rich( this.lineNumber, { @@ -49,6 +50,7 @@ class EngineLineMetrics implements ui.LineMetrics { required this.descent, required this.boxes, required this.spaceBoxCount, + required this.trailingSpaceBoxCount, }) : displayText = null, unscaledAscent = double.infinity; @@ -81,6 +83,12 @@ class EngineLineMetrics implements ui.LineMetrics { /// The number of boxes that are space-only. final int spaceBoxCount; + /// The number of trailing boxes that are space-only. + final int trailingSpaceBoxCount; + + /// The number of space-only boxes excluding trailing spaces. + int get nonTrailingSpaceBoxCount => spaceBoxCount - trailingSpaceBoxCount; + @override final bool hardBreak; diff --git a/lib/web_ui/test/text/canvas_paragraph_builder_test.dart b/lib/web_ui/test/text/canvas_paragraph_builder_test.dart index 2997f99d6f427..5ef9182342ebf 100644 --- a/lib/web_ui/test/text/canvas_paragraph_builder_test.dart +++ b/lib/web_ui/test/text/canvas_paragraph_builder_test.dart @@ -51,7 +51,7 @@ Future testMain() async { expectOuterHtml( paragraph, '

' - '' + '' 'Hello' '' '

', @@ -63,10 +63,10 @@ Future testMain() async { expectOuterHtml( paragraph, '

' - '' + '' 'Hel' '' - '' + '' 'lo' '' '

', @@ -95,7 +95,7 @@ Future testMain() async { expect( paragraph.toDomElement().outerHtml, '

' - '' + '' 'Hello' '' '

', @@ -119,7 +119,7 @@ Future testMain() async { expect( paragraph.toDomElement().outerHtml, '

' - '' + '' 'Hello' '' '

', @@ -140,7 +140,7 @@ Future testMain() async { expect( paragraph.toDomElement().outerHtml, '

' - '' + '' 'Hell...' '' '

', @@ -168,7 +168,7 @@ Future testMain() async { expect( paragraph.toDomElement().outerHtml, '

' - '' + '' 'Hello' '' '

', @@ -206,31 +206,32 @@ Future testMain() async { expectOuterHtml( paragraph, '

' - '' + '' 'Hello' '' - '' + '' ' ' '' - '' + '' 'world' '' '

', ignorePositions: !isBlink, ); - // Should break "Hello world" into 2 lines: "Hello" and " world". + // Should break "Hello world" into 2 lines: "Hello " and "world". paragraph.layout(const ParagraphConstraints(width: 75.0)); expectOuterHtml( paragraph, '

' - '' + '' 'Hello' '' - '' + // Trailing space. + '' ' ' '' - '' + '' 'world' '' '

', @@ -279,16 +280,16 @@ Future testMain() async { expectOuterHtml( paragraph, '

' - '' + '' 'Hello' '' - '' + '' ' ' '' - '' + '' 'world' '' - '' + '' '!' '' '

', @@ -347,37 +348,38 @@ Future testMain() async { expectOuterHtml( paragraph, '

' - '' + '' 'First' '' - '' + '' 'Second' '' - '' + '' ' ' '' - '' + '' 'ThirdLongLine' '' '

', ignorePositions: !isBlink, ); - // Should break the paragraph into "First", "Second" and "ThirdLongLine". + // Should break the paragraph into "First", "Second " and "ThirdLongLine". paragraph.layout(const ParagraphConstraints(width: 180.0)); expectOuterHtml( paragraph, '

' - '' + '' 'First' '' - '' + '' 'Second' '' - '' + // Trailing space. + '' ' ' '' - '' + '' 'ThirdLongLine' '' '

', @@ -409,19 +411,19 @@ Future testMain() async { expectOuterHtml( paragraph, '

' - '' + '' 'First' '' - '' + '' ' ' '' - '' + '' 'Second' '' - '' + '' ' ' '' - '' + '' 'Third' '' '

', @@ -453,6 +455,7 @@ String paragraphStyle({ String spanStyle({ required num? top, required num? left, + required num? width, String fontFamily = defaultFontFamily, num fontSize = defaultFontSize, String? fontWeight, @@ -471,6 +474,7 @@ String spanStyle({ 'position: absolute;', if (top != null) 'top: ${top}px;', if (left != null) 'left: ${left}px;', + if (width != null) 'width: ${width}px;', ].join(' '); } @@ -510,6 +514,7 @@ void expectOuterHtml(CanvasParagraph paragraph, String expected, {required bool /// unknown and could be different depending on browser and environment. String removePositionInfo(String outerHtml) { return outerHtml + .replaceAll(RegExp(r'\s*width:\s*[\d\.]+px\s*;\s*'), '') .replaceAll(RegExp(r'\s*top:\s*[\d\.]+px\s*;\s*'), '') .replaceAll(RegExp(r'\s*left:\s*[\d\.]+px\s*;\s*'), ''); } From f902654e11b5b463469351c81ae2458a12d6ca8d Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Tue, 22 Mar 2022 11:34:53 -0400 Subject: [PATCH 3/3] fix test on safari --- lib/web_ui/test/text/canvas_paragraph_builder_test.dart | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/web_ui/test/text/canvas_paragraph_builder_test.dart b/lib/web_ui/test/text/canvas_paragraph_builder_test.dart index 5ef9182342ebf..bd7a526d4125e 100644 --- a/lib/web_ui/test/text/canvas_paragraph_builder_test.dart +++ b/lib/web_ui/test/text/canvas_paragraph_builder_test.dart @@ -165,13 +165,14 @@ Future testMain() async { expect(paragraph.spans, hasLength(1)); paragraph.layout(const ParagraphConstraints(width: double.infinity)); - expect( - paragraph.toDomElement().outerHtml, + expectOuterHtml( + paragraph, '

' '' 'Hello' '' '

', + ignorePositions: !isBlink, ); final FlatTextSpan span = paragraph.spans.single as FlatTextSpan;