From d67005945106040ec23df75af4bfdc0f448e8c67 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Mon, 24 Feb 2020 14:26:03 -0800 Subject: [PATCH] [web] Respect maxLines when calculating boxes for a range (#16749) --- lib/web_ui/lib/src/engine/text/paragraph.dart | 8 +- lib/web_ui/lib/src/engine/text/ruler.dart | 16 ++ lib/web_ui/test/paragraph_test.dart | 198 +++++++++++++++--- 3 files changed, 195 insertions(+), 27 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text/paragraph.dart b/lib/web_ui/lib/src/engine/text/paragraph.dart index 60e9e7954fcbf..0b761d64e962d 100644 --- a/lib/web_ui/lib/src/engine/text/paragraph.dart +++ b/lib/web_ui/lib/src/engine/text/paragraph.dart @@ -374,6 +374,10 @@ class EngineParagraph implements ui.Paragraph { } final List lines = _measurementResult.lines; + if (start >= lines.last.endIndex) { + return []; + } + final EngineLineMetrics startLine = _getLineForIndex(start); EngineLineMetrics endLine = _getLineForIndex(end); @@ -535,8 +539,7 @@ class EngineParagraph implements ui.Paragraph { EngineLineMetrics _getLineForIndex(int index) { assert(_hasLineMetrics); final List lines = _measurementResult.lines; - assert(index >= lines.first.startIndex); - assert(index <= lines.last.endIndex); + assert(index >= 0); for (int i = 0; i < lines.length; i++) { final EngineLineMetrics line = lines[i]; @@ -545,7 +548,6 @@ class EngineParagraph implements ui.Paragraph { } } - assert(index == lines.last.endIndex); return lines.last; } diff --git a/lib/web_ui/lib/src/engine/text/ruler.dart b/lib/web_ui/lib/src/engine/text/ruler.dart index b16401fca6323..08ac9c89a0cfd 100644 --- a/lib/web_ui/lib/src/engine/text/ruler.dart +++ b/lib/web_ui/lib/src/engine/text/ruler.dart @@ -733,7 +733,22 @@ class ParagraphRuler { final List> clientRects = rangeSpan.getClientRects(); final List boxes = []; + final double maxLinesLimit = style.maxLines == null + ? double.infinity + : style.maxLines * lineHeightDimensions.height; + + html.Rectangle previousRect; for (html.Rectangle rect in clientRects) { + // If [rect] is an empty box on the same line as the previous box, don't + // include it in the result. + if (rect.top == previousRect?.top && rect.left == rect.right) { + continue; + } + // As soon as we go beyond [maxLines], stop adding boxes. + if (rect.top >= maxLinesLimit) { + break; + } + boxes.add(ui.TextBox.fromLTRBD( rect.left + alignOffset, rect.top, @@ -741,6 +756,7 @@ class ParagraphRuler { rect.bottom, textDirection, )); + previousRect = rect; } // Cleanup after measuring the boxes. diff --git a/lib/web_ui/test/paragraph_test.dart b/lib/web_ui/test/paragraph_test.dart index 5a8b06c13e702..29d52b830218f 100644 --- a/lib/web_ui/test/paragraph_test.dart +++ b/lib/web_ui/test/paragraph_test.dart @@ -441,12 +441,6 @@ void main() async { final Paragraph paragraph = builder.build(); paragraph.layout(const ParagraphConstraints(width: 100)); - // 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" // At the beginning of the first line. @@ -497,8 +491,6 @@ void main() async { paragraph.getBoxesForRange(2, 5), [ TextBox.fromLTRBD(20.0, 0.0, 40.0, 10.0, TextDirection.ltr), - if (isDiscrepancyExpected) - TextBox.fromLTRBD(40.0, 0.0, 40.0, 10.0, TextDirection.ltr), ], ); @@ -533,8 +525,6 @@ void main() async { paragraph.getBoxesForRange(10, 13), [ TextBox.fromLTRBD(50.0, 10.0, 70.0, 20.0, TextDirection.ltr), - if (isDiscrepancyExpected) - TextBox.fromLTRBD(70.0, 10.0, 70.0, 20.0, TextDirection.ltr), ], ); @@ -573,8 +563,6 @@ void main() async { paragraph.getBoxesForRange(2, 8), [ TextBox.fromLTRBD(20.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), ], ); @@ -593,11 +581,7 @@ void main() async { paragraph.getBoxesForRange(3, 14), [ TextBox.fromLTRBD(30.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 (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), ], ); @@ -607,11 +591,7 @@ void main() async { paragraph.getBoxesForRange(0, 13), [ TextBox.fromLTRBD(0.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 (isDiscrepancyExpected) - TextBox.fromLTRBD(70.0, 10.0, 70.0, 20.0, TextDirection.ltr), ], ); @@ -620,16 +600,186 @@ void main() async { paragraph.getBoxesForRange(0, 15), [ TextBox.fromLTRBD(0.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 (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), ], ); }); + testEachMeasurement('getBoxesForRange with maxLines', () { + final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle( + fontFamily: 'Ahem', + fontStyle: FontStyle.normal, + fontWeight: FontWeight.normal, + fontSize: 10, + textDirection: TextDirection.ltr, + maxLines: 2, + )); + builder.addText('abcd\n'); + builder.addText('abcdefg\n'); + builder.addText('ab'); + final Paragraph paragraph = builder.build(); + paragraph.layout(const ParagraphConstraints(width: 100)); + + // 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), + ], + ); + + // 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), + ], + ); + + // 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), + [], + ); + // The range "ab" in the last line. + expect( + paragraph.getBoxesForRange(13, 15), + [], + ); + + + // Combine multiple lines + + // The range "cd\nabc". + expect( + paragraph.getBoxesForRange(2, 8), + [ + TextBox.fromLTRBD(20.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), + TextBox.fromLTRBD(0.0, 10.0, 70.0, 20.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), + TextBox.fromLTRBD(0.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), + TextBox.fromLTRBD(0.0, 10.0, 70.0, 20.0, TextDirection.ltr), + ], + ); + }); + test('longestLine', () { // [Paragraph.longestLine] is only supported by canvas-based measurement. TextMeasurementService.enableExperimentalCanvasImplementation = true;