[web] Fix painting of last placeholder in paragraph (flutter#24716)

Author: mdebbar

lib/web_ui/dev/goldens_lock.yaml

@@ -1,2 +1,2 @@
 repository: https://github.com/flutter/goldens.git
-revision: bb55871d3803337053f7200b8690a4c1322e82ea
+revision: d4599aade933fe5c9c74d4c4acc08649ab6146bc

lib/web_ui/lib/src/engine/text/layout_service.dart

@@ -78,36 +78,40 @@ class TextLayoutService {
     final Spanometer spanometer = Spanometer(paragraph, context);
 
     int spanIndex = 0;
-    ParagraphSpan span = paragraph.spans[0];
     LineBuilder currentLine =
         LineBuilder.first(paragraph, spanometer, maxWidth: constraints.width);
 
     // The only way to exit this while loop is by hitting one of the `break;`
     // statements (e.g. when we reach `endOfText`, when ellipsis has been
     // appended).
     while (true) {
-      // *********************************************** //
-      // *** HANDLE HARD LINE BREAKS AND END OF TEXT *** //
-      // *********************************************** //
-
-      if (currentLine.end.isHard) {
-        if (currentLine.isNotEmpty) {
+      // ************************** //
+      // *** HANDLE END OF TEXT *** //
+      // ************************** //
+
+      // All spans have been consumed.
+      final bool reachedEnd = spanIndex == spanCount;
+      if (reachedEnd) {
+        // In some cases, we need to extend the line to the end of text and
+        // build it:
+        //
+        // 1. Line is not empty. This could happen when the last span is a
+        //    placeholder.
+        //
+        // 2. We haven't reached `LineBreakType.endOfText` yet. This could
+        //    happen when the last character is a new line.
+        if (currentLine.isNotEmpty || currentLine.end.type != LineBreakType.endOfText) {
+          currentLine.extendToEndOfText();
           lines.add(currentLine.build());
-          if (currentLine.end.type != LineBreakType.endOfText) {
-            currentLine = currentLine.nextLine();
-          }
-        }
-
-        if (currentLine.end.type == LineBreakType.endOfText) {
-          break;
         }
+        break;
       }
 
       // ********************************* //
       // *** THE MAIN MEASUREMENT PART *** //
       // ********************************* //
 
-      final isLastSpan = spanIndex == spanCount - 1;
+      final ParagraphSpan span = paragraph.spans[spanIndex];
 
       if (span is PlaceholderSpan) {
         if (currentLine.widthIncludingSpace + span.width <= constraints.width) {
@@ -121,11 +125,7 @@ class TextLayoutService {
           }
           currentLine.addPlaceholder(span);
         }
-
-        if (isLastSpan) {
-          lines.add(currentLine.build());
-          break;
-        }
+        spanIndex++;
       } else if (span is FlatTextSpan) {
         spanometer.currentSpan = span;
         final LineBreakResult nextBreak = currentLine.findNextBreak(span.end);
@@ -138,6 +138,10 @@ class TextLayoutService {
 
           // The line can extend to `nextBreak` without overflowing.
           currentLine.extendTo(nextBreak);
+          if (nextBreak.type == LineBreakType.mandatory) {
+            lines.add(currentLine.build());
+            currentLine = currentLine.nextLine();
+          }
         } else {
           // The chunk of text can't fit into the current line.
           final bool isLastLine =
@@ -165,23 +169,19 @@ class TextLayoutService {
             currentLine = currentLine.nextLine();
           }
         }
+
+        // Only go to the next span if we've reached the end of this span.
+        if (currentLine.end.index >= span.end) {
+          currentLine.createBox();
+          ++spanIndex;
+        }
       } else {
         throw UnimplementedError('Unknown span type: ${span.runtimeType}');
       }
 
       if (lines.length == maxLines) {
         break;
       }
-
-      // ********************************************* //
-      // *** ADVANCE TO THE NEXT SPAN IF NECESSARY *** //
-      // ********************************************* //
-
-      // Only go to the next span if we've reached the end of this span.
-      if (currentLine.end.index >= span.end && spanIndex < spanCount - 1) {
-        currentLine.createBox();
-        span = paragraph.spans[++spanIndex];
-      }
     }
 
     // ************************************************** //
@@ -205,20 +205,27 @@ class TextLayoutService {
     // ******************************** //
 
     spanIndex = 0;
-    span = paragraph.spans[0];
     currentLine =
         LineBuilder.first(paragraph, spanometer, maxWidth: constraints.width);
 
-    while (currentLine.end.type != LineBreakType.endOfText) {
+    while (spanIndex < spanCount) {
+      final ParagraphSpan span = paragraph.spans[spanIndex];
+
       if (span is PlaceholderSpan) {
         currentLine.addPlaceholder(span);
+        spanIndex++;
       } else if (span is FlatTextSpan) {
         spanometer.currentSpan = span;
         final LineBreakResult nextBreak = currentLine.findNextBreak(span.end);
 
         // For the purpose of max intrinsic width, we don't care if the line
         // fits within the constraints or not. So we always extend it.
         currentLine.extendTo(nextBreak);
+
+        // Only go to the next span if we've reached the end of this span.
+        if (currentLine.end.index >= span.end) {
+          spanIndex++;
+        }
       }
 
       final double widthOfLastSegment = currentLine.lastSegment.width;
@@ -231,19 +238,9 @@ class TextLayoutService {
         maxIntrinsicWidth = currentLine.widthIncludingSpace;
       }
 
-      if (currentLine.end.isHard) {
+      if (currentLine.end.type == LineBreakType.mandatory) {
         currentLine = currentLine.nextLine();
       }
-
-      // Only go to the next span if we've reached the end of this span.
-      if (currentLine.end.index >= span.end) {
-        if (spanIndex < spanCount - 1) {
-          span = paragraph.spans[++spanIndex];
-        } else {
-          // We reached the end of the last span in the paragraph.
-          break;
-        }
-      }
     }
   }
 
@@ -776,6 +773,23 @@ class LineBuilder {
     _addSegment(_createSegment(newEnd));
   }
 
+  void extendToEndOfText() {
+    final LineBreakResult endOfText = LineBreakResult.sameIndex(
+      paragraph.toPlainText().length,
+      LineBreakType.endOfText,
+    );
+
+    // The spanometer may not be ready in some cases. E.g. when the paragraph
+    // is made up of only placeholders and no text.
+    if (spanometer.isReady) {
+      ascent = math.max(ascent, spanometer.ascent);
+      descent = math.max(descent, spanometer.descent);
+      _addSegment(_createSegment(endOfText));
+    } else {
+      end = endOfText;
+    }
+  }
+
   void addPlaceholder(PlaceholderSpan placeholder) {
     // Increase the line's height to fit the placeholder, if necessary.
     final double ascent, descent;
@@ -1024,7 +1038,7 @@ class LineBuilder {
     final LineBreakResult boxEnd = end;
     // Avoid creating empty boxes. This could happen when the end of a span
     // coincides with the end of a line. In this case, `createBox` is called twice.
-    if (boxStart == boxEnd) {
+    if (boxStart.index == boxEnd.index) {
       return;
     }
 
@@ -1150,6 +1164,9 @@ class Spanometer {
     }
   }
 
+  /// Whether the spanometer is ready to take measurements.
+  bool get isReady => _currentSpan != null;
+
   /// The distance from the top of the current span to the alphabetic baseline.
   double get ascent => _currentRuler!.alphabeticBaseline;
 

lib/web_ui/test/golden_tests/engine/canvas_paragraph/placeholders_test.dart

@@ -50,9 +50,7 @@ void testMain() async {
       canvas.drawParagraph(paragraph, offset);
 
       // Then fill the placeholders.
-      final TextBox placeholderBox = paragraph.getBoxesForPlaceholders().single;
-      final SurfacePaint redPaint = Paint()..color = red;
-      canvas.drawRect(placeholderBox.toRect().shift(offset), redPaint.paintData);
+      fillPlaceholder(canvas, offset, paragraph);
 
       offset = offset.translate(0.0, paragraph.height + 30.0);
     }
@@ -86,9 +84,7 @@ void testMain() async {
       canvas.drawParagraph(paragraph, offset);
 
       // Then fill the placeholders.
-      final TextBox placeholderBox = paragraph.getBoxesForPlaceholders().single;
-      final SurfacePaint redPaint = Paint()..color = red;
-      canvas.drawRect(placeholderBox.toRect().shift(offset), redPaint.paintData);
+      fillPlaceholder(canvas, offset, paragraph);
 
       offset = offset.translate(0.0, paragraph.height + 30.0);
     }
@@ -122,13 +118,89 @@ void testMain() async {
       canvas.drawParagraph(paragraph, offset);
 
       // Then fill the placeholders.
-      final TextBox placeholderBox = paragraph.getBoxesForPlaceholders().single;
-      final SurfacePaint redPaint = Paint()..color = red;
-      canvas.drawRect(placeholderBox.toRect().shift(offset), redPaint.paintData);
+      fillPlaceholder(canvas, offset, paragraph);
 
       offset = offset.translate(0.0, paragraph.height + 30.0);
     }
 
     return takeScreenshot(canvas, bounds, 'canvas_paragraph_placeholders_align_dom');
   });
+
+  test('draws paragraphs starting or ending with a placeholder', () {
+    const Rect bounds = Rect.fromLTWH(0, 0, 420, 300);
+    final canvas = BitmapCanvas(bounds, RenderStrategy());
+
+    Offset offset = Offset(10, 10);
+
+    // First paragraph with a placeholder at the beginning.
+    final CanvasParagraph paragraph1 = rich(
+      ParagraphStyle(fontFamily: 'Roboto', fontSize: 24.0, textAlign: TextAlign.center),
+      (builder) {
+        builder.addPlaceholder(80.0, 50.0, PlaceholderAlignment.baseline, baseline: TextBaseline.alphabetic);
+        builder.pushStyle(TextStyle(color: black));
+        builder.addText(' Lorem ipsum.');
+      },
+    )..layout(constrain(400.0));
+
+    // Draw the paragraph.
+    canvas.drawParagraph(paragraph1, offset);
+    fillPlaceholder(canvas, offset, paragraph1);
+    surroundParagraph(canvas, offset, paragraph1);
+
+    offset = offset.translate(0.0, paragraph1.height + 30.0);
+
+    // Second paragraph with a placeholder at the end.
+    final CanvasParagraph paragraph2 = rich(
+      ParagraphStyle(fontFamily: 'Roboto', fontSize: 24.0, textAlign: TextAlign.center),
+      (builder) {
+        builder.pushStyle(TextStyle(color: black));
+        builder.addText('Lorem ipsum ');
+        builder.addPlaceholder(80.0, 50.0, PlaceholderAlignment.baseline, baseline: TextBaseline.alphabetic);
+      },
+    )..layout(constrain(400.0));
+
+    // Draw the paragraph.
+    canvas.drawParagraph(paragraph2, offset);
+    fillPlaceholder(canvas, offset, paragraph2);
+    surroundParagraph(canvas, offset, paragraph2);
+
+    offset = offset.translate(0.0, paragraph2.height + 30.0);
+
+    // Third paragraph with a placeholder alone in the second line.
+    final CanvasParagraph paragraph3 = rich(
+      ParagraphStyle(fontFamily: 'Roboto', fontSize: 24.0, textAlign: TextAlign.center),
+      (builder) {
+        builder.pushStyle(TextStyle(color: black));
+        builder.addText('Lorem ipsum ');
+        builder.addPlaceholder(80.0, 50.0, PlaceholderAlignment.baseline, baseline: TextBaseline.alphabetic);
+      },
+    )..layout(constrain(200.0));
+
+    // Draw the paragraph.
+    canvas.drawParagraph(paragraph3, offset);
+    fillPlaceholder(canvas, offset, paragraph3);
+    surroundParagraph(canvas, offset, paragraph3);
+
+    return takeScreenshot(canvas, bounds, 'canvas_paragraph_placeholders_start_and_end');
+  });
+}
+
+void surroundParagraph(
+  EngineCanvas canvas,
+  Offset offset,
+  CanvasParagraph paragraph,
+) {
+  final Rect rect = offset & Size(paragraph.width, paragraph.height);
+  final SurfacePaint paint = Paint()..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 = Paint()..color = red;
+  canvas.drawRect(placeholderBox.toRect().shift(offset), paint.paintData);
 }

lib/web_ui/test/text/layout_service_rich_test.dart

@@ -164,7 +164,26 @@ void testMain() async {
     expect(paragraph.minIntrinsicWidth, 300.0);
     expect(paragraph.height, 50.0);
     expectLines(paragraph, [
-      l('', 0, 0, hardBreak: false, width: 300.0, left: 100.0),
+      l('', 0, 0, hardBreak: true, width: 300.0, left: 100.0),
+    ]);
+  });
+
+  test('correct maxIntrinsicWidth when paragraph ends with placeholder', () {
+    final EngineParagraphStyle paragraphStyle = EngineParagraphStyle(
+      fontFamily: 'ahem',
+      fontSize: 10,
+      textAlign: ui.TextAlign.center,
+    );
+    final CanvasParagraph paragraph = rich(paragraphStyle, (builder) {
+      builder.addText('abcd');
+      builder.addPlaceholder(300.0, 50.0, ui.PlaceholderAlignment.bottom);
+    })..layout(constrain(400.0));
+
+    expect(paragraph.maxIntrinsicWidth, 340.0);
+    expect(paragraph.minIntrinsicWidth, 300.0);
+    expect(paragraph.height, 50.0);
+    expectLines(paragraph, [
+      l('abcd', 0, 4, hardBreak: true, width: 340.0, left: 30.0),
     ]);
   });
 }