Skip to content

Commit

Permalink
[web] Fix canvas z-index leaking across repaints when element is reus…
Browse files Browse the repository at this point in the history
…ed. (flutter#17378)

* Fix z-index leak. Add test for canvas reuse
* add regression test
* update golden locks
* fix analysis errors
  • Loading branch information
ferhatb committed Mar 30, 2020
1 parent 7338f50 commit c779894
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/web_ui/dev/goldens_lock.yaml
@@ -1,2 +1,2 @@
repository: https://github.com/flutter/goldens.git
revision: ae6003206eb721137c20cd56d8d1d8e2a76d6dd1
revision: 5ae87c98ad4abf882a2d312e4c4f75d7ad6d845b
4 changes: 4 additions & 0 deletions lib/web_ui/lib/src/engine/canvas_pool.dart
Expand Up @@ -79,6 +79,10 @@ class _CanvasPool extends _SaveStackTracking {
bool requiresClearRect = false;
if (_reusablePool != null && _reusablePool.isNotEmpty) {
_canvas = _reusablePool.removeAt(0);
// If a canvas is the first element we set z-index = -1 to workaround
// blink compositing bug. To make sure this does not leak when reused
// reset z-index.
_canvas.style.removeProperty('z-index');
requiresClearRect = true;
} else {
// Compute the final CSS canvas size given the actual pixel count we
Expand Down
98 changes: 98 additions & 0 deletions lib/web_ui/test/golden_tests/engine/canvas_reuse_test.dart
@@ -0,0 +1,98 @@
// 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' hide TextStyle;
import 'package:ui/src/engine.dart';
import 'package:test/test.dart';

import 'package:web_engine_tester/golden_tester.dart';

void main() async {
const double screenWidth = 600.0;
const double screenHeight = 800.0;
const Rect screenRect = Rect.fromLTWH(0, 0, screenWidth, screenHeight);
final Paint testPaint = Paint()
..style = PaintingStyle.stroke
..strokeWidth = 2.0
..color = const Color(0xFFFF00FF);

setUp(() async {
debugEmulateFlutterTesterEnvironment = true;
await webOnlyInitializePlatform();
webOnlyFontCollection.debugRegisterTestFonts();
await webOnlyFontCollection.ensureFontsLoaded();
});

// Regression test for https://github.com/flutter/flutter/issues/51514
test('Canvas is reused and z-index doesn\'t leak across paints', () async {
final EngineCanvas engineCanvas = BitmapCanvas(screenRect);
const Rect region = Rect.fromLTWH(0, 0, 500, 500);

// Draw first frame into engine canvas.
final RecordingCanvas rc =
RecordingCanvas(const Rect.fromLTWH(1, 2, 300, 400));
final Path path = Path()
..moveTo(3, 0)
..lineTo(100, 97);
rc.drawPath(path, testPaint);
rc.apply(engineCanvas);
engineCanvas.endOfPaint();

html.Element sceneElement = html.Element.tag('flt-scene');
sceneElement.append(engineCanvas.rootElement);
html.document.body.append(sceneElement);

final html.CanvasElement canvas = html.document.querySelector('canvas');
// ! Since canvas is first element, it should have zIndex = -1 for correct
// paint order.
expect(canvas.style.zIndex , '-1');

// Add id to canvas element to test for reuse.
const String kTestId = 'test-id-5698';
canvas.id = kTestId;

sceneElement.remove();
// Clear so resources are marked for reuse.

engineCanvas.clear();

// Now paint a second scene to same [BitmapCanvas] but paint an image
// before the path to move canvas element into second position.
final RecordingCanvas rc2 =
RecordingCanvas(const Rect.fromLTWH(1, 2, 300, 400));
final Path path2 = Path()
..moveTo(3, 0)
..quadraticBezierTo(100, 0, 100, 100);
rc2.drawImage(_createRealTestImage(), Offset(0, 0), Paint());
rc2.drawPath(path2, testPaint);
rc2.apply(engineCanvas);

sceneElement = html.Element.tag('flt-scene');
sceneElement.append(engineCanvas.rootElement);
html.document.body.append(sceneElement);

final html.CanvasElement canvas2 = html.document.querySelector('canvas');
// ZIndex should have been cleared since we have image element preceding
// canvas.
expect(canvas.style.zIndex != '-1', true);
expect(canvas2.id, kTestId);
await matchGoldenFile('bitmap_canvas_reuse_zindex.png', region: region);
});
}

const String _base64Encoded20x20TestImage = 'iVBORw0KGgoAAAANSUhEUgAAABQAAAAUCAIAAAAC64paAAAACXBIWXMAAC4jAAAuIwF4pT92AAAA'
'B3RJTUUH5AMFFBksg4i3gQAAABl0RVh0Q29tbWVudABDcmVhdGVkIHdpdGggR0lNUFeBDhcAAAAj'
'SURBVDjLY2TAC/7jlWVioACMah4ZmhnxpyHG0QAb1UyZZgBjWAIm/clP0AAAAABJRU5ErkJggg==';

HtmlImage _createRealTestImage() {
return HtmlImage(
html.ImageElement()
..src = 'data:text/plain;base64,$_base64Encoded20x20TestImage',
20,
20,
);
}

0 comments on commit c779894

Please sign in to comment.