From 2fedf31fb20c7b656eb772b400f75a2876862e92 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Fri, 21 Feb 2020 10:28:35 -0800 Subject: [PATCH 1/2] [web] Consider maxLines when calculating boxes for a range --- lib/web_ui/lib/src/engine/text/paragraph.dart | 8 +- lib/web_ui/lib/src/engine/text/ruler.dart | 20 +- lib/web_ui/test/paragraph_test.dart | 198 ++++++++++++++++++ 3 files changed, 216 insertions(+), 10 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..53b1f95f09265 100644 --- a/lib/web_ui/lib/src/engine/text/ruler.dart +++ b/lib/web_ui/lib/src/engine/text/ruler.dart @@ -733,14 +733,20 @@ class ParagraphRuler { final List> clientRects = rangeSpan.getClientRects(); final List boxes = []; + final double maxLinesLimit = style.maxLines == null + ? double.infinity + : style.maxLines * lineHeightDimensions.height; + for (html.Rectangle rect in clientRects) { - boxes.add(ui.TextBox.fromLTRBD( - rect.left + alignOffset, - rect.top, - rect.right + alignOffset, - rect.bottom, - textDirection, - )); + if (rect.top < maxLinesLimit) { + boxes.add(ui.TextBox.fromLTRBD( + rect.left + alignOffset, + rect.top, + rect.right + alignOffset, + rect.bottom, + textDirection, + )); + } } // 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..504d0dab8dff7 100644 --- a/lib/web_ui/test/paragraph_test.dart +++ b/lib/web_ui/test/paragraph_test.dart @@ -630,6 +630,204 @@ void main() async { ); }); + 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)); + + // 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. + 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 (isDiscrepancyExpected) + 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 (isDiscrepancyExpected) + 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), + [], + ); + // 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), + 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), + ], + ); + + // 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 (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), + ], + ); + + // The range "abcd\nabcdefg\n". + expect( + 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), + ], + ); + + // The range "abcd\nabcdefg\nab". + expect( + 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), + ], + ); + }); + test('longestLine', () { // [Paragraph.longestLine] is only supported by canvas-based measurement. TextMeasurementService.enableExperimentalCanvasImplementation = true; From 615d089af5a3da42510359f0240e9da3d5a59911 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Mon, 24 Feb 2020 11:02:04 -0800 Subject: [PATCH 2/2] Remove the discrepancies --- lib/web_ui/lib/src/engine/text/ruler.dart | 26 ++++++++---- lib/web_ui/test/paragraph_test.dart | 48 ----------------------- 2 files changed, 18 insertions(+), 56 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text/ruler.dart b/lib/web_ui/lib/src/engine/text/ruler.dart index 53b1f95f09265..08ac9c89a0cfd 100644 --- a/lib/web_ui/lib/src/engine/text/ruler.dart +++ b/lib/web_ui/lib/src/engine/text/ruler.dart @@ -737,16 +737,26 @@ class ParagraphRuler { ? double.infinity : style.maxLines * lineHeightDimensions.height; + html.Rectangle previousRect; for (html.Rectangle rect in clientRects) { - if (rect.top < maxLinesLimit) { - boxes.add(ui.TextBox.fromLTRBD( - rect.left + alignOffset, - rect.top, - rect.right + alignOffset, - rect.bottom, - textDirection, - )); + // 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, + rect.right + alignOffset, + 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 504d0dab8dff7..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,11 +600,7 @@ 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), ], ); @@ -645,12 +621,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. @@ -701,8 +671,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), ], ); @@ -737,8 +705,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), ], ); @@ -773,8 +739,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), ], ); @@ -793,11 +757,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), ], ); @@ -806,11 +766,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), ], ); @@ -819,11 +775,7 @@ 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), ], ); });