From d3317d1d3d8cfb7fe2acebdd1f73643282e8b087 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Fri, 10 Jun 2022 11:43:05 -0400 Subject: [PATCH] [web] Cleaner types for EngineLineMetrics (flutter/engine#33911) --- .../lib/src/engine/text/canvas_paragraph.dart | 15 +- .../lib/src/engine/text/layout_service.dart | 48 ++-- .../lib/src/engine/text/paint_service.dart | 8 +- .../web_ui/lib/src/engine/text/paragraph.dart | 231 +++++++++++------- engine/src/flutter/lib/web_ui/pubspec.yaml | 2 +- .../test/text/canvas_paragraph_test.dart | 4 +- .../test/text/layout_service_helper.dart | 4 +- 7 files changed, 180 insertions(+), 132 deletions(-) diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart index f71cb7f042f..99c04a35562 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart @@ -75,6 +75,8 @@ class CanvasParagraph implements ui.Paragraph { @override bool get didExceedMaxLines => _layoutService.didExceedMaxLines; + List get lines => _layoutService.lines; + /// The bounds that contain the text painted inside this paragraph. ui.Rect get paintBounds => _layoutService.paintBounds; @@ -166,10 +168,8 @@ class CanvasParagraph implements ui.Paragraph { // 2. Append all spans to the paragraph. DomHTMLElement? lastSpanElement; - final List lines = computeLineMetrics(); - for (int i = 0; i < lines.length; i++) { - final EngineLineMetrics line = lines[i]; + final ParagraphLine line = lines[i]; final List boxes = line.boxes; final StringBuffer buffer = StringBuffer(); @@ -237,27 +237,26 @@ class CanvasParagraph implements ui.Paragraph { @override ui.TextRange getLineBoundary(ui.TextPosition position) { final int index = position.offset; - final List lines = computeLineMetrics(); int i; for (i = 0; i < lines.length - 1; i++) { - final EngineLineMetrics line = lines[i]; + final ParagraphLine line = lines[i]; if (index >= line.startIndex && index < line.endIndex) { break; } } - final EngineLineMetrics line = lines[i]; + final ParagraphLine line = lines[i]; return ui.TextRange(start: line.startIndex, end: line.endIndex); } @override List computeLineMetrics() { - return _layoutService.lines; + return lines.map((ParagraphLine line) => line.lineMetrics).toList(); } } -void _positionSpanElement(DomElement element, EngineLineMetrics line, RangeBox box) { +void _positionSpanElement(DomElement element, ParagraphLine line, RangeBox box) { final ui.Rect boxRect = box.toTextBox(line, forPainting: true).toRect(); element.style ..position = 'absolute' diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/text/layout_service.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/text/layout_service.dart index 71274024819..76bfe955cdc 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/text/layout_service.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/text/layout_service.dart @@ -33,7 +33,7 @@ class TextLayoutService { double height = 0.0; - EngineLineMetrics? longestLine; + ParagraphLine? longestLine; double minIntrinsicWidth = 0.0; @@ -45,7 +45,7 @@ class TextLayoutService { bool didExceedMaxLines = false; - final List lines = []; + final List lines = []; /// The bounds that contain the text painted inside this paragraph. ui.Rect get paintBounds => _paintBounds; @@ -65,7 +65,7 @@ class TextLayoutService { /// starts looping through the paragraph to calculate all layout metrics. /// /// It uses a [Spanometer] to perform measurements within spans of the - /// paragraph. It also uses [LineBuilders] to generate [EngineLineMetrics] as + /// paragraph. It also uses [LineBuilders] to generate [ParagraphLine]s as /// it iterates through the paragraph. /// /// The main loop keeps going until: @@ -214,7 +214,7 @@ class TextLayoutService { double boundsLeft = double.infinity; double boundsRight = double.negativeInfinity; - for (final EngineLineMetrics line in lines) { + for (final ParagraphLine line in lines) { height += line.height; if (alphabeticBaseline == -1.0) { alphabeticBaseline = line.baseline; @@ -246,12 +246,12 @@ class TextLayoutService { // ********************** // if (lines.isNotEmpty) { - final EngineLineMetrics lastLine = lines.last; + final ParagraphLine lastLine = lines.last; final bool shouldJustifyParagraph = width.isFinite && paragraph.paragraphStyle.textAlign == ui.TextAlign.justify; - for (final EngineLineMetrics line in lines) { + for (final ParagraphLine line in lines) { // Don't apply justification to the last line. final bool shouldJustifyLine = shouldJustifyParagraph && line != lastLine; _positionLineBoxes(line, withJustification: shouldJustifyLine); @@ -313,7 +313,7 @@ class TextLayoutService { /// Positions the boxes in the given [line] and takes into account their /// directions, the paragraph's direction, and alignment justification. - void _positionLineBoxes(EngineLineMetrics line, { + void _positionLineBoxes(ParagraphLine line, { required bool withJustification, }) { final List boxes = line.boxes; @@ -405,7 +405,7 @@ class TextLayoutService { /// /// [first] and [last] are expected to be inclusive. double _positionLineBoxesInReverse( - EngineLineMetrics line, + ParagraphLine line, int first, int last, { required double startOffset, @@ -431,7 +431,7 @@ class TextLayoutService { /// 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) { + double _calculateJustifyPerSpaceBox(ParagraphLine line) { final double justifyTotal = width - line.width; final int spaceBoxesToJustify = line.nonTrailingSpaceBoxCount; @@ -444,7 +444,7 @@ class TextLayoutService { List getBoxesForPlaceholders() { final List boxes = []; - for (final EngineLineMetrics line in lines) { + for (final ParagraphLine line in lines) { for (final RangeBox box in line.boxes) { if (box is PlaceholderBox) { boxes.add(box.toTextBox(line, forPainting: false)); @@ -473,7 +473,7 @@ class TextLayoutService { final List boxes = []; - for (final EngineLineMetrics line in lines) { + for (final ParagraphLine line in lines) { if (line.overlapsWith(start, end)) { for (final RangeBox box in line.boxes) { if (box is SpanBox && box.overlapsWith(start, end)) { @@ -490,7 +490,7 @@ class TextLayoutService { // it possible to do hit testing. Once we find the box, we look inside that // box to find where exactly the `offset` is located. - final EngineLineMetrics line = _findLineForY(offset.dy); + final ParagraphLine line = _findLineForY(offset.dy); // [offset] is to the left of the line. if (offset.dx <= line.left) { return ui.TextPosition( @@ -517,11 +517,11 @@ class TextLayoutService { return ui.TextPosition(offset: line.startIndex); } - EngineLineMetrics _findLineForY(double y) { + ParagraphLine _findLineForY(double y) { // We could do a binary search here but it's not worth it because the number // of line is typically low, and each iteration is a cheap comparison of // doubles. - for (final EngineLineMetrics line in lines) { + for (final ParagraphLine line in lines) { if (y <= line.height) { return line; } @@ -623,7 +623,7 @@ abstract class RangeBox { /// 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}); + ui.TextBox toTextBox(ParagraphLine line, {required bool forPainting}); /// Returns the text position within this box's range that's closest to the /// given [x] offset. @@ -648,7 +648,7 @@ class PlaceholderBox extends RangeBox { double get width => placeholder.width; @override - ui.TextBox toTextBox(EngineLineMetrics line, {required bool forPainting}) { + ui.TextBox toTextBox(ParagraphLine line, {required bool forPainting}) { final double left = line.left + this.left; final double right = line.left + this.right; @@ -784,7 +784,7 @@ class SpanBox extends RangeBox { } @override - ui.TextBox toTextBox(EngineLineMetrics line, {required bool forPainting}) { + ui.TextBox toTextBox(ParagraphLine line, {required bool forPainting}) { return intersect(line, start.index, end.index, forPainting: forPainting); } @@ -793,7 +793,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, {required bool forPainting}) { + ui.TextBox intersect(ParagraphLine line, int start, int end, {required bool forPainting}) { final double top = line.baseline - baseline; final double before; @@ -1001,7 +1001,7 @@ class LineSegment { bool get isSpaceOnly => width == 0; } -/// Builds instances of [EngineLineMetrics] for the given [paragraph]. +/// Builds instances of [ParagraphLine] for the given [paragraph]. /// /// Usage of this class starts by calling [LineBuilder.first] to start building /// the first line of the paragraph. @@ -1009,7 +1009,7 @@ class LineSegment { /// Then new line breaks can be found by calling [LineBuilder.findNextBreak]. /// /// The line can be extended one or more times before it's built by calling -/// [LineBuilder.build] which generates the [EngineLineMetrics] instace. +/// [LineBuilder.build] which generates the [ParagraphLine] instance. /// /// To start building the next line, simply call [LineBuilder.nextLine] which /// creates a new [LineBuilder] that can be extended and built and so on. @@ -1485,8 +1485,8 @@ class LineBuilder { _currentBoxStartOffset = widthIncludingSpace; } - /// Builds the [EngineLineMetrics] instance that represents this line. - EngineLineMetrics build({String? ellipsis}) { + /// Builds the [ParagraphLine] instance that represents this line. + ParagraphLine build({String? ellipsis}) { // At the end of each line, we cut the last box of the line. createBox(); @@ -1503,8 +1503,8 @@ class LineBuilder { _processTrailingSpaces(); - return EngineLineMetrics.rich( - lineNumber, + return ParagraphLine( + lineNumber: lineNumber, ellipsis: ellipsis, startIndex: start.index, endIndex: end.index, diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/text/paint_service.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/text/paint_service.dart index 59f8a0bf2a8..7d0ed293774 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/text/paint_service.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/text/paint_service.dart @@ -20,13 +20,13 @@ class TextPaintService { // Loop through all the lines, for each line, loop through all the boxes and // paint them. The boxes have enough information so they can be painted // individually. - final List lines = paragraph.computeLineMetrics(); + final List lines = paragraph.lines; if (lines.isEmpty) { return; } - for (final EngineLineMetrics line in lines) { + for (final ParagraphLine line in lines) { if (line.boxes.isEmpty) { continue; } @@ -49,7 +49,7 @@ class TextPaintService { void _paintBackground( BitmapCanvas canvas, ui.Offset offset, - EngineLineMetrics line, + ParagraphLine line, RangeBox box, ) { if (box is SpanBox) { @@ -67,7 +67,7 @@ class TextPaintService { void _paintText( BitmapCanvas canvas, ui.Offset offset, - EngineLineMetrics line, + ParagraphLine line, RangeBox box, ) { // There's no text to paint in placeholder spans. diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/text/paragraph.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/text/paragraph.dart index da13cfa0e86..a3568dd8147 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/text/paragraph.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/text/paragraph.dart @@ -14,7 +14,7 @@ import 'layout_service.dart'; import 'ruler.dart'; class EngineLineMetrics implements ui.LineMetrics { - EngineLineMetrics({ + const EngineLineMetrics({ required this.hardBreak, required this.ascent, required this.descent, @@ -24,70 +24,7 @@ class EngineLineMetrics implements ui.LineMetrics { required this.left, required this.baseline, required this.lineNumber, - }) : displayText = null, - ellipsis = null, - startIndex = -1, - endIndex = -1, - endIndexWithoutNewlines = -1, - widthWithTrailingSpaces = width, - boxes = [], - spaceBoxCount = 0, - trailingSpaceBoxCount = 0; - - EngineLineMetrics.rich( - this.lineNumber, { - required this.ellipsis, - required this.startIndex, - required this.endIndex, - required this.endIndexWithoutNewlines, - required this.hardBreak, - required this.width, - required this.widthWithTrailingSpaces, - required this.left, - required this.height, - required this.baseline, - required this.ascent, - required this.descent, - required this.boxes, - required this.spaceBoxCount, - required this.trailingSpaceBoxCount, - }) : displayText = null, - unscaledAscent = double.infinity; - - /// The text to be rendered on the screen representing this line. - final String? displayText; - - /// The string to be displayed as an overflow indicator. - /// - /// When the value is non-null, it means this line is overflowing and the - /// [ellipsis] needs to be displayed at the end of it. - final String? ellipsis; - - /// The index (inclusive) in the text where this line begins. - final int startIndex; - - /// The index (exclusive) in the text where this line ends. - /// - /// When the line contains an overflow, then [endIndex] goes until the end of - /// the text and doesn't stop at the overflow cutoff. - final int endIndex; - - /// The index (exclusive) in the text where this line ends, ignoring newline - /// characters. - final int endIndexWithoutNewlines; - - /// The list of boxes representing the entire line, possibly across multiple - /// spans. - final List boxes; - - /// 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; @@ -107,18 +44,6 @@ class EngineLineMetrics implements ui.LineMetrics { @override final double width; - /// The full width of the line including all trailing space but not new lines. - /// - /// The difference between [width] and [widthWithTrailingSpaces] is that - /// [widthWithTrailingSpaces] includes trailing spaces in the width - /// calculation while [width] doesn't. - /// - /// For alignment purposes for example, the [width] property is the right one - /// to use because trailing spaces shouldn't affect the centering of text. - /// But for placing cursors in text fields, we do care about trailing - /// spaces so [widthWithTrailingSpaces] is more suitable. - final double widthWithTrailingSpaces; - @override final double left; @@ -128,15 +53,8 @@ class EngineLineMetrics implements ui.LineMetrics { @override final int lineNumber; - bool overlapsWith(int startIndex, int endIndex) { - return startIndex < this.endIndex && this.startIndex < endIndex; - } - @override int get hashCode => Object.hash( - displayText, - startIndex, - endIndex, hardBreak, ascent, descent, @@ -157,9 +75,6 @@ class EngineLineMetrics implements ui.LineMetrics { return false; } return other is EngineLineMetrics && - other.displayText == displayText && - other.startIndex == startIndex && - other.endIndex == endIndex && other.hardBreak == hardBreak && other.ascent == ascent && other.descent == descent && @@ -189,6 +104,139 @@ class EngineLineMetrics implements ui.LineMetrics { } } +class ParagraphLine { + ParagraphLine({ + required bool hardBreak, + required double ascent, + required double descent, + required double height, + required double width, + required double left, + required double baseline, + required int lineNumber, + required this.ellipsis, + required this.startIndex, + required this.endIndex, + required this.endIndexWithoutNewlines, + required this.widthWithTrailingSpaces, + required this.boxes, + required this.spaceBoxCount, + required this.trailingSpaceBoxCount, + this.displayText, + }) : lineMetrics = EngineLineMetrics( + hardBreak: hardBreak, + ascent: ascent, + descent: descent, + unscaledAscent: ascent, + height: height, + width: width, + left: left, + baseline: baseline, + lineNumber: lineNumber, + ); + + /// Metrics for this line of the paragraph. + final EngineLineMetrics lineMetrics; + + /// The string to be displayed as an overflow indicator. + /// + /// When the value is non-null, it means this line is overflowing and the + /// [ellipsis] needs to be displayed at the end of it. + final String? ellipsis; + + /// The index (inclusive) in the text where this line begins. + final int startIndex; + + /// The index (exclusive) in the text where this line ends. + /// + /// When the line contains an overflow, then [endIndex] goes until the end of + /// the text and doesn't stop at the overflow cutoff. + final int endIndex; + + /// The index (exclusive) in the text where this line ends, ignoring newline + /// characters. + final int endIndexWithoutNewlines; + + /// The full width of the line including all trailing space but not new lines. + /// + /// The difference between [width] and [widthWithTrailingSpaces] is that + /// [widthWithTrailingSpaces] includes trailing spaces in the width + /// calculation while [width] doesn't. + /// + /// For alignment purposes for example, the [width] property is the right one + /// to use because trailing spaces shouldn't affect the centering of text. + /// But for placing cursors in text fields, we do care about trailing + /// spaces so [widthWithTrailingSpaces] is more suitable. + final double widthWithTrailingSpaces; + + /// The list of boxes representing the entire line, possibly across multiple + /// spans. + final List boxes; + + /// The number of boxes that are space-only. + final int spaceBoxCount; + + /// The number of trailing boxes that are space-only. + final int trailingSpaceBoxCount; + + /// The text to be rendered on the screen representing this line. + final String? displayText; + + /// The number of space-only boxes excluding trailing spaces. + int get nonTrailingSpaceBoxCount => spaceBoxCount - trailingSpaceBoxCount; + + // Convenient getters for line metrics properties. + + bool get hardBreak => lineMetrics.hardBreak; + double get ascent => lineMetrics.ascent; + double get descent => lineMetrics.descent; + double get unscaledAscent => lineMetrics.unscaledAscent; + double get height => lineMetrics.height; + double get width => lineMetrics.width; + double get left => lineMetrics.left; + double get baseline => lineMetrics.baseline; + int get lineNumber => lineMetrics.lineNumber; + + bool overlapsWith(int startIndex, int endIndex) { + return startIndex < this.endIndex && this.startIndex < endIndex; + } + + @override + int get hashCode => Object.hash( + lineMetrics, + ellipsis, + startIndex, + endIndex, + endIndexWithoutNewlines, + widthWithTrailingSpaces, + boxes, + spaceBoxCount, + trailingSpaceBoxCount, + displayText, + ); + + @override + bool operator ==(Object other) { + if (identical(this, other)) { + return true; + } + if (other.runtimeType != runtimeType) { + return false; + } + return other is ParagraphLine && + other.lineMetrics == lineMetrics && + other.ellipsis == ellipsis && + other.startIndex == startIndex && + other.endIndex == endIndex && + other.endIndexWithoutNewlines == endIndexWithoutNewlines && + other.widthWithTrailingSpaces == widthWithTrailingSpaces && + other.boxes == boxes && + other.spaceBoxCount == spaceBoxCount && + other.trailingSpaceBoxCount == trailingSpaceBoxCount && + other.displayText == displayText; + } +} + /// The web implementation of [ui.ParagraphStyle]. class EngineParagraphStyle implements ui.ParagraphStyle { /// Creates a new instance of [EngineParagraphStyle]. @@ -224,7 +272,8 @@ class EngineParagraphStyle implements ui.ParagraphStyle { // The effective style attributes should be consistent with paragraph_style.h. ui.TextAlign get effectiveTextAlign => textAlign ?? ui.TextAlign.start; - ui.TextDirection get effectiveTextDirection => textDirection ?? ui.TextDirection.ltr; + ui.TextDirection get effectiveTextDirection => + textDirection ?? ui.TextDirection.ltr; double? get lineHeight { // TODO(mdebbar): Implement proper support for strut styles. @@ -742,8 +791,7 @@ void applyTextStyleToElement({ _textDecorationToCssString(style.decoration, style.decorationStyle); if (textDecoration != null) { if (browserEngine == BrowserEngine.webkit) { - setElementStyle( - element, '-webkit-text-decoration', textDecoration); + setElementStyle(element, '-webkit-text-decoration', textDecoration); } else { cssStyle.textDecoration = textDecoration; } @@ -762,7 +810,8 @@ void applyTextStyleToElement({ final List? fontVariations = style.fontVariations; if (fontVariations != null && fontVariations.isNotEmpty) { - cssStyle.setProperty('font-variation-settings', _fontVariationListToCss(fontVariations)); + cssStyle.setProperty( + 'font-variation-settings', _fontVariationListToCss(fontVariations)); } } diff --git a/engine/src/flutter/lib/web_ui/pubspec.yaml b/engine/src/flutter/lib/web_ui/pubspec.yaml index 842d48a3d6d..d1fd0e88f74 100644 --- a/engine/src/flutter/lib/web_ui/pubspec.yaml +++ b/engine/src/flutter/lib/web_ui/pubspec.yaml @@ -3,7 +3,7 @@ publish_to: none # Keep the SDK version range in sync with pubspecs under web_sdk environment: - sdk: ">=2.12.0-0 <3.0.0" + sdk: ">=2.17.0-0 <3.0.0" dependencies: js: 0.6.4 diff --git a/engine/src/flutter/lib/web_ui/test/text/canvas_paragraph_test.dart b/engine/src/flutter/lib/web_ui/test/text/canvas_paragraph_test.dart index 33a74ec71ca..b8a6c3ccbba 100644 --- a/engine/src/flutter/lib/web_ui/test/text/canvas_paragraph_test.dart +++ b/engine/src/flutter/lib/web_ui/test/text/canvas_paragraph_test.dart @@ -395,8 +395,8 @@ Future testMain() async { // we want to make sure that the "B" box is also popped. paragraph.layout(constrain(60)); - final EngineLineMetrics firstLine = paragraph.computeLineMetrics()[0]; - final EngineLineMetrics secondLine = paragraph.computeLineMetrics()[1]; + final ParagraphLine firstLine = paragraph.lines[0]; + final ParagraphLine secondLine = paragraph.lines[1]; // There should be no "B" in the first line's boxes. expect(firstLine.boxes, hasLength(2)); diff --git a/engine/src/flutter/lib/web_ui/test/text/layout_service_helper.dart b/engine/src/flutter/lib/web_ui/test/text/layout_service_helper.dart index 632fd3b90b3..9fef792b034 100644 --- a/engine/src/flutter/lib/web_ui/test/text/layout_service_helper.dart +++ b/engine/src/flutter/lib/web_ui/test/text/layout_service_helper.dart @@ -34,10 +34,10 @@ TestLine l( void expectLines(CanvasParagraph paragraph, List expectedLines) { final String text = paragraph.toPlainText(); - final List lines = paragraph.computeLineMetrics(); + final List lines = paragraph.lines; expect(lines, hasLength(expectedLines.length)); for (int i = 0; i < lines.length; i++) { - final EngineLineMetrics line = lines[i]; + final ParagraphLine line = lines[i]; final TestLine expectedLine = expectedLines[i]; expect(