From de38b85deb4e920982c70194d77d9fdccdb97f6b Mon Sep 17 00:00:00 2001 From: Ferhat Date: Mon, 30 Mar 2020 16:42:36 -0700 Subject: [PATCH] [web] Fix HtmlImage state pollution due to image.clone optimization (flutter/engine#17370) * fix drawImage style when destination width==source * Add regression test * dartfmt * update golden lock * Addressed review comment * remove transform reset since _drawImage is called --- .../web_ui/lib/src/engine/bitmap_canvas.dart | 9 +- .../engine/canvas_draw_picture_test.dart | 122 ++++++++++++++++++ 2 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 engine/src/flutter/lib/web_ui/test/golden_tests/engine/canvas_draw_picture_test.dart diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/bitmap_canvas.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/bitmap_canvas.dart index f14fedb652a..5444086f8c2 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/bitmap_canvas.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/bitmap_canvas.dart @@ -364,6 +364,10 @@ class BitmapCanvas extends EngineCanvas { 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'); final List clipElements = _clipContent( _canvasPool._clipStack, imgElement, p, _canvasPool.currentTransform); for (html.Element clipElement in clipElements) { @@ -375,7 +379,10 @@ class BitmapCanvas extends EngineCanvas { transformWithOffset(_canvasPool.currentTransform, p).storage); imgElement.style ..transformOrigin = '0 0 0' - ..transform = cssTransform; + ..transform = cssTransform + // Reset width/height since they may have been previously set. + ..removeProperty('width') + ..removeProperty('height'); rootElement.append(imgElement); _children.add(imgElement); } diff --git a/engine/src/flutter/lib/web_ui/test/golden_tests/engine/canvas_draw_picture_test.dart b/engine/src/flutter/lib/web_ui/test/golden_tests/engine/canvas_draw_picture_test.dart new file mode 100644 index 00000000000..de4c52cebb1 --- /dev/null +++ b/engine/src/flutter/lib/web_ui/test/golden_tests/engine/canvas_draw_picture_test.dart @@ -0,0 +1,122 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// @dart = 2.6 +import 'dart:html' as html; + +import 'package:ui/ui.dart'; +import 'package:ui/src/engine.dart'; +import 'package:test/test.dart'; + +import 'package:web_engine_tester/golden_tester.dart'; + +final Rect region = Rect.fromLTWH(0, 0, 500, 100); + +void main() async { + setUp(() async { + debugShowClipLayers = true; + SurfaceSceneBuilder.debugForgetFrameScene(); + for (html.Node scene in html.document.querySelectorAll('flt-scene')) { + scene.remove(); + } + + await webOnlyInitializePlatform(); + webOnlyFontCollection.debugRegisterTestFonts(); + await webOnlyFontCollection.ensureFontsLoaded(); + }); + + test('draw growing picture across frames', () async { + final SurfaceSceneBuilder builder = SurfaceSceneBuilder(); + builder.pushClipRect( + const Rect.fromLTRB(0, 0, 100, 100), + ); + + _drawTestPicture(builder, 100, false); + builder.pop(); + + html.Element elm1 = builder.build().webOnlyRootElement; + html.document.body.append(elm1); + + // Now draw picture again but at larger size. + final SurfaceSceneBuilder builder2 = SurfaceSceneBuilder(); + builder2.pushClipRect( + const Rect.fromLTRB(0, 0, 100, 100), + ); + // Now draw the picture at original target size, which will use a + // different code path that should normally not have width/height set + // on image element. + _drawTestPicture(builder2, 20, false); + builder2.pop(); + + elm1.remove(); + html.document.body.append(builder2.build().webOnlyRootElement); + + await matchGoldenFile('canvas_draw_picture_acrossframes.png', + region: region); + }); + + test('draw growing picture across frames clipped', () async { + final SurfaceSceneBuilder builder = SurfaceSceneBuilder(); + builder.pushClipRect( + const Rect.fromLTRB(0, 0, 100, 100), + ); + + _drawTestPicture(builder, 100, true); + builder.pop(); + + html.Element elm1 = builder.build().webOnlyRootElement; + html.document.body.append(elm1); + + // Now draw picture again but at larger size. + final SurfaceSceneBuilder builder2 = SurfaceSceneBuilder(); + builder2.pushClipRect( + const Rect.fromLTRB(0, 0, 100, 100), + ); + _drawTestPicture(builder2, 20, true); + builder2.pop(); + + elm1.remove(); + html.document.body.append(builder2.build().webOnlyRootElement); + + await matchGoldenFile('canvas_draw_picture_acrossframes_clipped.png', + region: region); + }); +} + +HtmlImage sharedImage; + +void _drawTestPicture(SceneBuilder builder, double targetSize, bool clipped) { + sharedImage ??= _createRealTestImage(); + final EnginePictureRecorder recorder = PictureRecorder(); + final RecordingCanvas canvas = + recorder.beginRecording(const Rect.fromLTRB(0, 0, 100, 100)); + canvas.debugEnforceArbitraryPaint(); + if (clipped) { + canvas.clipRRect( + RRect.fromLTRBR(0, 0, targetSize, targetSize, Radius.circular(4))); + } + canvas.drawImageRect(sharedImage, Rect.fromLTWH(0, 0, 20, 20), + Rect.fromLTWH(0, 0, targetSize, targetSize), Paint()); + final Picture picture = recorder.endRecording(); + builder.addPicture( + Offset.zero, + picture, + ); +} + +typedef PaintCallback = void Function(RecordingCanvas canvas); + +const String _base64Encoded20x20TestImage = + 'iVBORw0KGgoAAAANSUhEUgAAABQAAAAUCAIAAAAC64paAAAACXBIWXMAAC4jAAAuIwF4pT92AAAA' + 'B3RJTUUH5AMFFBksg4i3gQAAABl0RVh0Q29tbWVudABDcmVhdGVkIHdpdGggR0lNUFeBDhcAAAAj' + 'SURBVDjLY2TAC/7jlWVioACMah4ZmhnxpyHG0QAb1UyZZgBjWAIm/clP0AAAAABJRU5ErkJggg=='; + +HtmlImage _createRealTestImage() { + return HtmlImage( + html.ImageElement() + ..src = 'data:text/plain;base64,$_base64Encoded20x20TestImage', + 20, + 20, + ); +}