From c05c3317bea1bc01b521df7b82e572c8ea773679 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Tue, 11 Feb 2020 15:38:13 -0800 Subject: [PATCH 1/3] [web] Paragraph.getBoxesForRange uses LineMetrics --- lib/web_ui/lib/src/engine/text/paragraph.dart | 88 ++++++-- lib/web_ui/test/paragraph_test.dart | 192 ++++++++++++++++++ 2 files changed, 261 insertions(+), 19 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text/paragraph.dart b/lib/web_ui/lib/src/engine/text/paragraph.dart index bf526f1c98884..3c6362aa74fe2 100644 --- a/lib/web_ui/lib/src/engine/text/paragraph.dart +++ b/lib/web_ui/lib/src/engine/text/paragraph.dart @@ -176,6 +176,8 @@ class EngineParagraph implements ui.Paragraph { /// The measurement result of the last layout operation. MeasurementResult _measurementResult; + bool get _hasLineMetrics => _measurementResult?.lines != null; + @override double get width => _measurementResult?.width ?? -1; @@ -197,7 +199,7 @@ class EngineParagraph implements ui.Paragraph { @override double get longestLine { - if (_measurementResult.lines != null) { + if (_hasLineMetrics) { double maxWidth = 0.0; for (ui.LineMetrics metrics in _measurementResult.lines) { if (maxWidth < metrics.width) { @@ -308,7 +310,7 @@ class EngineParagraph implements ui.Paragraph { /// - Paragraphs that have a non-null word-spacing. /// - Paragraphs with a background. bool get _drawOnCanvas { - if (_measurementResult.lines == null) { + if (!_hasLineMetrics) { return false; } @@ -363,13 +365,51 @@ class EngineParagraph implements ui.Paragraph { return []; } - return _measurementService.measureBoxesForRange( - this, - _lastUsedConstraints, - start: start, - end: end, - alignOffset: _alignOffset, - textDirection: _textDirection, + // Fallback to the old, DOM-based box measurements when there's no line + // metrics. + if (!_hasLineMetrics) { + return _measurementService.measureBoxesForRange( + this, + _lastUsedConstraints, + start: start, + end: end, + alignOffset: _alignOffset, + textDirection: _textDirection, + ); + } + + final List lines = _measurementResult.lines; + final EngineLineMetrics startLine = _getLineForIndex(start); + EngineLineMetrics endLine = _getLineForIndex(end); + + // If the range end is exactly at the beginning of a line, we shouldn't + // include any boxes from that line. + if (end == endLine.startIndex) { + endLine = lines[endLine.lineNumber - 1]; + } + + final List boxes = []; + for (int i = startLine.lineNumber; i <= endLine.lineNumber; i++) { + boxes.add(_getBoxForLine(lines[i], start, end)); + } + return boxes; + } + + ui.TextBox _getBoxForLine(EngineLineMetrics line, int start, int end) { + final double left = start <= line.startIndex + ? 0.0 + : _measurementService.measureSubstringWidth(this, line.startIndex, start); + final double right = end >= line.endIndexWithoutNewlines + ? 0.0 + : _measurementService.measureSubstringWidth(this, end, line.endIndexWithoutNewlines); + + final double top = line.lineNumber * _lineHeight; + return ui.TextBox.fromLTRBD( + left + line.left, + top, + line.width + line.left - right, + top + _lineHeight, + _textDirection, ); } @@ -388,7 +428,7 @@ class EngineParagraph implements ui.Paragraph { @override ui.TextPosition getPositionForOffset(ui.Offset offset) { final List lines = _measurementResult.lines; - if (lines == null) { + if (!_hasLineMetrics) { return getPositionForMultiSpanOffset(offset); } @@ -489,19 +529,29 @@ class EngineParagraph implements ui.Paragraph { return ui.TextRange(start: start, end: end); } - @override - ui.TextRange getLineBoundary(ui.TextPosition position) { + EngineLineMetrics _getLineForIndex(int index) { + assert(_hasLineMetrics); final List lines = _measurementResult.lines; - if (lines != null) { - final int offset = position.offset; + assert(index >= lines.first.startIndex); + assert(index <= lines.last.endIndex); - for (int i = 0; i < lines.length; i++) { - final EngineLineMetrics line = lines[i]; - if (offset >= line.startIndex && offset < line.endIndex) { - return ui.TextRange(start: line.startIndex, end: line.endIndex); - } + for (int i = 0; i < lines.length; i++) { + final EngineLineMetrics line = lines[i]; + if (index >= line.startIndex && index < line.endIndex) { + return line; } } + + assert(index == lines.last.endIndex); + return lines.last; + } + + @override + ui.TextRange getLineBoundary(ui.TextPosition position) { + if (_hasLineMetrics) { + final EngineLineMetrics line = _getLineForIndex(position.offset); + return ui.TextRange(start: line.startIndex, end: line.endIndex); + } return ui.TextRange.empty; } diff --git a/lib/web_ui/test/paragraph_test.dart b/lib/web_ui/test/paragraph_test.dart index a41e741e9ccc3..18ac7e03f0171 100644 --- a/lib/web_ui/test/paragraph_test.dart +++ b/lib/web_ui/test/paragraph_test.dart @@ -426,6 +426,198 @@ void main() async { expect(paragraph.getBoxesForRange(0, 0), isEmpty); }); + testEachMeasurement('getBoxesForRange multi-line', () { + final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle( + fontFamily: 'Ahem', + fontStyle: FontStyle.normal, + fontWeight: FontWeight.normal, + fontSize: 10, + textDirection: TextDirection.ltr, + )); + builder.addText('abcd\n'); + builder.addText('abcdefg\n'); + builder.addText('ab'); + final Paragraph paragraph = builder.build(); + paragraph.layout(const ParagraphConstraints(width: 100)); + + // In the dom-based measurement, there will be some discrepancies around + // line ends. + final isDom = !TextMeasurementService.enableExperimentalCanvasImplementation; + + // First line: "abcd\n" + + // At the beginning of the first line. + expect( + paragraph.getBoxesForRange(0, 0), + [], + ); + // At the end of the first line. + expect( + paragraph.getBoxesForRange(4, 4), + [], + ); + // Between "b" and "c" in the first line. + expect( + paragraph.getBoxesForRange(2, 2), + [], + ); + // The range "ab" in the first line. + expect( + paragraph.getBoxesForRange(0, 2), + [ + TextBox.fromLTRBD(0.0, 0.0, 20.0, 10.0, TextDirection.ltr), + ], + ); + // The range "bc" in the first line. + expect( + paragraph.getBoxesForRange(1, 3), + [ + TextBox.fromLTRBD(10.0, 0.0, 30.0, 10.0, TextDirection.ltr), + ], + ); + // The range "d" in the first line. + expect( + paragraph.getBoxesForRange(3, 4), + [ + TextBox.fromLTRBD(30.0, 0.0, 40.0, 10.0, TextDirection.ltr), + ], + ); + // The range "\n" in the first line. + expect( + paragraph.getBoxesForRange(4, 5), + [ + TextBox.fromLTRBD(40.0, 0.0, 40.0, 10.0, TextDirection.ltr), + ], + ); + // The range "cd\n" in the first line. + expect( + paragraph.getBoxesForRange(2, 5), + [ + TextBox.fromLTRBD(20.0, 0.0, 40.0, 10.0, TextDirection.ltr), + if (isDom) TextBox.fromLTRBD(40.0, 0.0, 40.0, 10.0, TextDirection.ltr), + ], + ); + + // Second line: "abcdefg\n" + + // At the beginning of the second line. + expect( + paragraph.getBoxesForRange(5, 5), + [], + ); + // At the end of the second line. + expect( + paragraph.getBoxesForRange(12, 12), + [], + ); + // The range "efg" in the second line. + expect( + paragraph.getBoxesForRange(9, 12), + [ + TextBox.fromLTRBD(40.0, 10.0, 70.0, 20.0, TextDirection.ltr), + ], + ); + // The range "bcde" in the second line. + expect( + paragraph.getBoxesForRange(6, 10), + [ + TextBox.fromLTRBD(10.0, 10.0, 50.0, 20.0, TextDirection.ltr), + ], + ); + // The range "fg\n" in the second line. + expect( + paragraph.getBoxesForRange(10, 13), + [ + TextBox.fromLTRBD(50.0, 10.0, 70.0, 20.0, TextDirection.ltr), + if (isDom) TextBox.fromLTRBD(70.0, 10.0, 70.0, 20.0, TextDirection.ltr), + ], + ); + + // Last (third) line: "ab" + + // At the beginning of the last line. + expect( + paragraph.getBoxesForRange(13, 13), + [], + ); + // At the end of the last line. + expect( + paragraph.getBoxesForRange(15, 15), + [], + ); + // The range "a" in the last line. + expect( + paragraph.getBoxesForRange(14, 15), + [ + TextBox.fromLTRBD(10.0, 20.0, 20.0, 30.0, TextDirection.ltr), + ], + ); + // The range "ab" in the last line. + expect( + paragraph.getBoxesForRange(13, 15), + [ + TextBox.fromLTRBD(0.0, 20.0, 20.0, 30.0, TextDirection.ltr), + ], + ); + + + // Combine multiple lines + + // The range "cd\nabc". + expect( + paragraph.getBoxesForRange(2, 8), + [ + TextBox.fromLTRBD(20.0, 0.0, 40.0, 10.0, TextDirection.ltr), + if (isDom) TextBox.fromLTRBD(40.0, 0.0, 40.0, 10.0, TextDirection.ltr), + TextBox.fromLTRBD(0.0, 10.0, 30.0, 20.0, TextDirection.ltr), + ], + ); + + // The range "\nabcd". + expect( + paragraph.getBoxesForRange(4, 9), + [ + TextBox.fromLTRBD(40.0, 0.0, 40.0, 10.0, TextDirection.ltr), + TextBox.fromLTRBD(0.0, 10.0, 40.0, 20.0, TextDirection.ltr), + ], + ); + + // The range "d\nabcdefg\na". + expect( + paragraph.getBoxesForRange(3, 14), + [ + TextBox.fromLTRBD(30.0, 0.0, 40.0, 10.0, TextDirection.ltr), + if (isDom) TextBox.fromLTRBD(40.0, 0.0, 40.0, 10.0, TextDirection.ltr), + TextBox.fromLTRBD(0.0, 10.0, 70.0, 20.0, TextDirection.ltr), + if (isDom) TextBox.fromLTRBD(70.0, 10.0, 70.0, 20.0, TextDirection.ltr), + TextBox.fromLTRBD(0.0, 20.0, 10.0, 30.0, TextDirection.ltr), + ], + ); + + // The range "abcd\nabcdefg\n". + expect( + paragraph.getBoxesForRange(0, 13), + [ + TextBox.fromLTRBD(0.0, 0.0, 40.0, 10.0, TextDirection.ltr), + if (isDom) TextBox.fromLTRBD(40.0, 0.0, 40.0, 10.0, TextDirection.ltr), + TextBox.fromLTRBD(0.0, 10.0, 70.0, 20.0, TextDirection.ltr), + if (isDom) TextBox.fromLTRBD(70.0, 10.0, 70.0, 20.0, TextDirection.ltr), + ], + ); + + // The range "abcd\nabcdefg\nab". + expect( + paragraph.getBoxesForRange(0, 15), + [ + TextBox.fromLTRBD(0.0, 0.0, 40.0, 10.0, TextDirection.ltr), + if (isDom) TextBox.fromLTRBD(40.0, 0.0, 40.0, 10.0, TextDirection.ltr), + TextBox.fromLTRBD(0.0, 10.0, 70.0, 20.0, TextDirection.ltr), + if (isDom) TextBox.fromLTRBD(70.0, 10.0, 70.0, 20.0, TextDirection.ltr), + TextBox.fromLTRBD(0.0, 20.0, 20.0, 30.0, TextDirection.ltr), + ], + ); + }); + test('longestLine', () { // [Paragraph.longestLine] is only supported by canvas-based measurement. TextMeasurementService.enableExperimentalCanvasImplementation = true; From 2bf6f6b6de1bbfec9a1d30b81756903fd55dc12a Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Tue, 18 Feb 2020 12:42:25 -0800 Subject: [PATCH 2/3] Clarify box measurements better --- lib/web_ui/lib/src/engine/text/paragraph.dart | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text/paragraph.dart b/lib/web_ui/lib/src/engine/text/paragraph.dart index 3c6362aa74fe2..949a64ff6dac7 100644 --- a/lib/web_ui/lib/src/engine/text/paragraph.dart +++ b/lib/web_ui/lib/src/engine/text/paragraph.dart @@ -396,18 +396,26 @@ class EngineParagraph implements ui.Paragraph { } ui.TextBox _getBoxForLine(EngineLineMetrics line, int start, int end) { - final double left = start <= line.startIndex + final double widthBeforeBox = start <= line.startIndex ? 0.0 : _measurementService.measureSubstringWidth(this, line.startIndex, start); - final double right = end >= line.endIndexWithoutNewlines + final double widthAfterBox = end >= line.endIndexWithoutNewlines ? 0.0 : _measurementService.measureSubstringWidth(this, end, line.endIndexWithoutNewlines); final double top = line.lineNumber * _lineHeight; + + // |<------------------ line.width ------------------>| + // |-------------|------------------|-------------|-----------------| + // |<-line.left->|<-widthBeforeBox->|<-box width->|<-widthAfterBox->| + // |-------------|------------------|-------------|-----------------| + // + // ^^^^^^^^^^^^^ + // This is the box we want to return. return ui.TextBox.fromLTRBD( - left + line.left, + line.left + widthBeforeBox, top, - line.width + line.left - right, + line.left + line.width - widthAfterBox, top + _lineHeight, _textDirection, ); From 0e47c4dd170698c27342091eaecafbf16d12769d Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Tue, 18 Feb 2020 15:21:47 -0800 Subject: [PATCH 3/3] Fix tests on firefox --- lib/web_ui/test/paragraph_test.dart | 35 +++++++++++++++++++---------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/lib/web_ui/test/paragraph_test.dart b/lib/web_ui/test/paragraph_test.dart index 18ac7e03f0171..8b673955f67ef 100644 --- a/lib/web_ui/test/paragraph_test.dart +++ b/lib/web_ui/test/paragraph_test.dart @@ -440,9 +440,11 @@ void main() async { final Paragraph paragraph = builder.build(); paragraph.layout(const ParagraphConstraints(width: 100)); - // In the dom-based measurement, there will be some discrepancies around - // line ends. - final isDom = !TextMeasurementService.enableExperimentalCanvasImplementation; + // In the dom-based measurement (except Firefox), there will be some + // discrepancies around line ends. + final isDiscrepancyExpected = + !TextMeasurementService.enableExperimentalCanvasImplementation && + browserEngine != BrowserEngine.firefox; // First line: "abcd\n" @@ -494,7 +496,8 @@ void main() async { paragraph.getBoxesForRange(2, 5), [ TextBox.fromLTRBD(20.0, 0.0, 40.0, 10.0, TextDirection.ltr), - if (isDom) TextBox.fromLTRBD(40.0, 0.0, 40.0, 10.0, TextDirection.ltr), + if (isDiscrepancyExpected) + TextBox.fromLTRBD(40.0, 0.0, 40.0, 10.0, TextDirection.ltr), ], ); @@ -529,7 +532,8 @@ void main() async { paragraph.getBoxesForRange(10, 13), [ TextBox.fromLTRBD(50.0, 10.0, 70.0, 20.0, TextDirection.ltr), - if (isDom) TextBox.fromLTRBD(70.0, 10.0, 70.0, 20.0, TextDirection.ltr), + if (isDiscrepancyExpected) + TextBox.fromLTRBD(70.0, 10.0, 70.0, 20.0, TextDirection.ltr), ], ); @@ -568,7 +572,8 @@ void main() async { paragraph.getBoxesForRange(2, 8), [ TextBox.fromLTRBD(20.0, 0.0, 40.0, 10.0, TextDirection.ltr), - if (isDom) TextBox.fromLTRBD(40.0, 0.0, 40.0, 10.0, TextDirection.ltr), + if (isDiscrepancyExpected) + TextBox.fromLTRBD(40.0, 0.0, 40.0, 10.0, TextDirection.ltr), TextBox.fromLTRBD(0.0, 10.0, 30.0, 20.0, TextDirection.ltr), ], ); @@ -587,9 +592,11 @@ void main() async { paragraph.getBoxesForRange(3, 14), [ TextBox.fromLTRBD(30.0, 0.0, 40.0, 10.0, TextDirection.ltr), - if (isDom) TextBox.fromLTRBD(40.0, 0.0, 40.0, 10.0, TextDirection.ltr), + if (isDiscrepancyExpected) + TextBox.fromLTRBD(40.0, 0.0, 40.0, 10.0, TextDirection.ltr), TextBox.fromLTRBD(0.0, 10.0, 70.0, 20.0, TextDirection.ltr), - if (isDom) TextBox.fromLTRBD(70.0, 10.0, 70.0, 20.0, TextDirection.ltr), + if (isDiscrepancyExpected) + TextBox.fromLTRBD(70.0, 10.0, 70.0, 20.0, TextDirection.ltr), TextBox.fromLTRBD(0.0, 20.0, 10.0, 30.0, TextDirection.ltr), ], ); @@ -599,9 +606,11 @@ void main() async { paragraph.getBoxesForRange(0, 13), [ TextBox.fromLTRBD(0.0, 0.0, 40.0, 10.0, TextDirection.ltr), - if (isDom) TextBox.fromLTRBD(40.0, 0.0, 40.0, 10.0, TextDirection.ltr), + if (isDiscrepancyExpected) + TextBox.fromLTRBD(40.0, 0.0, 40.0, 10.0, TextDirection.ltr), TextBox.fromLTRBD(0.0, 10.0, 70.0, 20.0, TextDirection.ltr), - if (isDom) TextBox.fromLTRBD(70.0, 10.0, 70.0, 20.0, TextDirection.ltr), + if (isDiscrepancyExpected) + TextBox.fromLTRBD(70.0, 10.0, 70.0, 20.0, TextDirection.ltr), ], ); @@ -610,9 +619,11 @@ void main() async { paragraph.getBoxesForRange(0, 15), [ TextBox.fromLTRBD(0.0, 0.0, 40.0, 10.0, TextDirection.ltr), - if (isDom) TextBox.fromLTRBD(40.0, 0.0, 40.0, 10.0, TextDirection.ltr), + if (isDiscrepancyExpected) + TextBox.fromLTRBD(40.0, 0.0, 40.0, 10.0, TextDirection.ltr), TextBox.fromLTRBD(0.0, 10.0, 70.0, 20.0, TextDirection.ltr), - if (isDom) TextBox.fromLTRBD(70.0, 10.0, 70.0, 20.0, TextDirection.ltr), + if (isDiscrepancyExpected) + TextBox.fromLTRBD(70.0, 10.0, 70.0, 20.0, TextDirection.ltr), TextBox.fromLTRBD(0.0, 20.0, 20.0, 30.0, TextDirection.ltr), ], );