diff --git a/lib/web_ui/dev/goldens_lock.yaml b/lib/web_ui/dev/goldens_lock.yaml index 137070e356b29..084f6e21c666d 100644 --- a/lib/web_ui/dev/goldens_lock.yaml +++ b/lib/web_ui/dev/goldens_lock.yaml @@ -1,2 +1,2 @@ repository: https://github.com/flutter/goldens.git -revision: 5ae87c98ad4abf882a2d312e4c4f75d7ad6d845b +revision: 1839716cce5da0fd4c442395c7161f29dd983d8e diff --git a/lib/web_ui/lib/src/engine/bitmap_canvas.dart b/lib/web_ui/lib/src/engine/bitmap_canvas.dart index 5444086f8c24c..e0ba89020de02 100644 --- a/lib/web_ui/lib/src/engine/bitmap_canvas.dart +++ b/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 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 + // |--- + // |---

+ // |--- + // 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 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. diff --git a/lib/web_ui/lib/src/engine/canvas_pool.dart b/lib/web_ui/lib/src/engine/canvas_pool.dart index b97ffede6f0ae..56d8020023359 100644 --- a/lib/web_ui/lib/src/engine/canvas_pool.dart +++ b/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. diff --git a/lib/web_ui/test/golden_tests/engine/canvas_draw_image_golden_test.dart b/lib/web_ui/test/golden_tests/engine/canvas_draw_image_golden_test.dart index 762c7b810fc5a..dcaf6c93b59bd 100644 --- a/lib/web_ui/test/golden_tests/engine/canvas_draw_image_golden_test.dart +++ b/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 _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', []); 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(); +}