[web] Fix compositing order when adding paragraph tags

lib/web_ui/dev/goldens_lock.yaml

@@ -1,2 +1,2 @@
 repository: https://github.com/flutter/goldens.git
-revision: 5ae87c98ad4abf882a2d312e4c4f75d7ad6d845b
+revision: 1839716cce5da0fd4c442395c7161f29dd983d8e

lib/web_ui/lib/src/engine/bitmap_canvas.dart

@@ -355,19 +355,18 @@ class BitmapCanvas extends EngineCanvas {
   void drawImage(ui.Image image, ui.Offset p, SurfacePaintData paint) {
     _drawImage(image, p, paint);
     _childOverdraw = true;
-    _canvasPool.allocateExtraCanvas();
+    _canvasPool.closeCurrentCanvas();
   }
 
-  html.ImageElement _drawImage(ui.Image image, ui.Offset p, SurfacePaintData paint) {
+  html.ImageElement _drawImage(
+      ui.Image image, ui.Offset p, SurfacePaintData paint) {
     final HtmlImage htmlImage = image;
     final html.Element imgElement = htmlImage.cloneImageElement();
     final ui.BlendMode blendMode = paint.blendMode;
     imgElement.style.mixBlendMode = _stringForBlendMode(blendMode);
     if (_canvasPool.isClipped) {
       // Reset width/height since they may have been previously set.
-      imgElement.style
-        ..removeProperty('width')
-        ..removeProperty('height');
+      imgElement.style..removeProperty('width')..removeProperty('height');
       final List<html.Element> clipElements = _clipContent(
           _canvasPool._clipStack, imgElement, p, _canvasPool.currentTransform);
       for (html.Element clipElement in clipElements) {
@@ -418,7 +417,8 @@ class BitmapCanvas extends EngineCanvas {
         }
       }
 
-      final html.ImageElement imgElement = _drawImage(image, ui.Offset(targetLeft, targetTop), paint);
+      final html.ImageElement imgElement =
+          _drawImage(image, ui.Offset(targetLeft, targetTop), paint);
       // To scale set width / height on destination image.
       // For clipping we need to scale according to
       // clipped-width/full image width and shift it according to left/top of
@@ -436,8 +436,22 @@ class BitmapCanvas extends EngineCanvas {
       if (requiresClipping) {
         restore();
       }
-      _canvasPool.allocateExtraCanvas();
     }
+    _closeCurrentCanvas();
+  }
+
+  // Should be called when we add new html elements into rootElement so that
+  // paint order is preserved.
+  //
+  // For example if we draw a path and then a paragraph and image:
+  //   - rootElement
+  //   |--- <canvas>
+  //   |--- <p>
+  //   |--- <img>
+  // Any drawing operations after these tags should allocate a new canvas,
+  // instead of drawing into earlier canvas.
+  void _closeCurrentCanvas() {
+    _canvasPool.closeCurrentCanvas();
     _childOverdraw = true;
   }
 
@@ -505,7 +519,6 @@ class BitmapCanvas extends EngineCanvas {
 
     final html.Element paragraphElement =
         _drawParagraphElement(paragraph, offset);
-
     if (_canvasPool.isClipped) {
       final List<html.Element> clipElements = _clipContent(
           _canvasPool._clipStack,
@@ -524,6 +537,7 @@ class BitmapCanvas extends EngineCanvas {
       rootElement.append(paragraphElement);
     }
     _children.add(paragraphElement);
+    _closeCurrentCanvas();
   }
 
   /// Paints the [picture] into this canvas.

lib/web_ui/lib/src/engine/canvas_pool.dart

@@ -9,7 +9,7 @@ part of engine;
 ///
 /// [BitmapCanvas] signals allocation of first canvas using allocateCanvas.
 /// When a painting command such as drawImage or drawParagraph requires
-/// multiple canvases for correct compositing, it calls [allocateExtraCanvas]
+/// multiple canvases for correct compositing, it calls [closeCurrentCanvas]
 /// and adds the canvas(s) to a [_pool] of active canvas(s).
 ///
 /// To make sure transformations and clips are preserved correctly when a new
@@ -54,9 +54,12 @@ class _CanvasPool extends _SaveStackTracking {
     return _contextHandle;
   }
 
-  // Allocating extra canvas items. Save current canvas so we can dispose
+  // Prevents active canvas to be used for rendering and prepares a new
+  // canvas allocation on next drawing request that will require one.
+  //
+  // Saves current canvas so we can dispose
   // and replay the clip/transform stack on top of new canvas.
-  void allocateExtraCanvas() {
+  void closeCurrentCanvas() {
     assert(_rootElement != null);
     // Place clean copy of current canvas with context stack restored and paint
     // reset into pool.

lib/web_ui/test/golden_tests/engine/canvas_draw_image_golden_test.dart

@@ -7,7 +7,7 @@ import 'dart:html' as html;
 import 'dart:math' as math;
 import 'dart:js_util' as js_util;
 
-import 'package:ui/ui.dart' hide TextStyle;
+import 'package:ui/ui.dart';
 import 'package:ui/src/engine.dart';
 import 'package:test/test.dart';
 
@@ -22,7 +22,8 @@ void main() async {
 
   // Commit a recording canvas to a bitmap, and compare with the expected
   Future<void> _checkScreenshot(RecordingCanvas rc, String fileName,
-      {Rect region = const Rect.fromLTWH(0, 0, 500, 500)}) async {
+      {Rect region = const Rect.fromLTWH(0, 0, 500, 500),
+        double maxDiffRatePercent = 0.0}) async {
     final EngineCanvas engineCanvas = BitmapCanvas(screenRect);
 
     rc.apply(engineCanvas);
@@ -32,7 +33,8 @@ void main() async {
     try {
       sceneElement.append(engineCanvas.rootElement);
       html.document.body.append(sceneElement);
-      await matchGoldenFile('$fileName.png', region: region, maxDiffRatePercent: 0.0);
+      await matchGoldenFile('$fileName.png',
+          region: region, maxDiffRatePercent: maxDiffRatePercent);
     } finally {
       // The page is reused across tests, so remove the element after taking the
       // Scuba screenshot.
@@ -310,6 +312,39 @@ void main() async {
     rc.restore();
     await _checkScreenshot(rc, 'draw_circle_on_image_clip_path');
   });
+
+  // Regression test for https://github.com/flutter/flutter/issues/53078
+  // Verified that Text+Image+Text+Rect+Text composites correctly.
+  // Yellow text should be behind image and rectangle.
+  // Cyan text should be above everything.
+  test('Paints text above and below image', () async {
+    final RecordingCanvas rc =
+        RecordingCanvas(const Rect.fromLTRB(0, 0, 400, 300));
+    rc.save();
+    Image testImage = createTestImage();
+    double testWidth = testImage.width.toDouble();
+    double testHeight = testImage.height.toDouble();
+    final Paragraph paragraph1 = createTestParagraph(
+        'should be below...............',
+        color: Color(0xFFFFFF40));
+    paragraph1.layout(const ParagraphConstraints(width: 400.0));
+    rc.drawParagraph(paragraph1, const Offset(20, 100));
+    rc.drawImageRect(testImage, Rect.fromLTRB(0, 0, testWidth, testHeight),
+        Rect.fromLTRB(100, 100, 200, 200), Paint());
+    rc.drawRect(
+        Rect.fromLTWH(50, 50, 100, 200),
+        Paint()
+          ..strokeWidth = 3
+          ..color = Color(0xA0000000));
+    final Paragraph paragraph2 = createTestParagraph(
+        'Should be above...............',
+        color: Color(0xFF00FFFF));
+    paragraph2.layout(const ParagraphConstraints(width: 400.0));
+    rc.drawParagraph(paragraph2, const Offset(20, 150));
+    rc.restore();
+    await _checkScreenshot(rc, 'draw_text_composite_order_below',
+        maxDiffRatePercent: 1.0);
+  });
 }
 
 HtmlImage createTestImage({int width = 100, int height = 50}) {
@@ -329,3 +364,16 @@ HtmlImage createTestImage({int width = 100, int height = 50}) {
   imageElement.src = js_util.callMethod(canvas, 'toDataURL', <dynamic>[]);
   return HtmlImage(imageElement, width, height);
 }
+
+Paragraph createTestParagraph(String text,
+    {Color color = const Color(0xFF000000)}) {
+  final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle(
+    fontFamily: 'Ahem',
+    fontStyle: FontStyle.normal,
+    fontWeight: FontWeight.normal,
+    fontSize: 14.0,
+  ));
+  builder.pushStyle(TextStyle(color: color));
+  builder.addText(text);
+  return builder.build();
+}