Skip to content

Commit

Permalink
fix paint memory management
Browse files Browse the repository at this point in the history
  • Loading branch information
yjbanov committed Apr 15, 2023
1 parent d920744 commit 4c22713
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 35 deletions.
4 changes: 3 additions & 1 deletion lib/web_ui/lib/src/engine/canvaskit/image.dart
Original file line number Diff line number Diff line change
Expand Up @@ -137,12 +137,14 @@ CkImage scaleImage(SkImage image, int? targetWidth, int? targetHeight) {
final CkPictureRecorder recorder = CkPictureRecorder();
final CkCanvas canvas = recorder.beginRecording(ui.Rect.largest);

final CkPaint paint = CkPaint();
canvas.drawImageRect(
CkImage(image),
ui.Rect.fromLTWH(0, 0, image.width(), image.height()),
ui.Rect.fromLTWH(0, 0, targetWidth!.toDouble(), targetHeight!.toDouble()),
CkPaint()
paint,
);
paint.dispose();

final CkPicture picture = recorder.endRecording();
final ui.Image finalImage = picture.toImageSync(
Expand Down
6 changes: 6 additions & 0 deletions lib/web_ui/lib/src/engine/canvaskit/layer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ class BackdropFilterEngineLayer extends ContainerLayer
final CkPaint paint = CkPaint()..blendMode = _blendMode;
paintContext.internalNodesCanvas
.saveLayerWithFilter(paintBounds, _filter, paint);
paint.dispose();
paintChildren(paintContext);
paintContext.internalNodesCanvas.restore();
}
Expand Down Expand Up @@ -340,6 +341,7 @@ class OpacityEngineLayer extends ContainerLayer
final ui.Rect saveLayerBounds = paintBounds.shift(-_offset);

paintContext.internalNodesCanvas.saveLayer(saveLayerBounds, paint);
paint.dispose();
paintChildren(paintContext);
// Restore twice: once for the translate and once for the saveLayer.
paintContext.internalNodesCanvas.restore();
Expand Down Expand Up @@ -403,6 +405,7 @@ class ImageFilterEngineLayer extends ContainerLayer
final CkPaint paint = CkPaint();
paint.imageFilter = _filter;
paintContext.internalNodesCanvas.saveLayer(paintBounds, paint);
paint.dispose();
paintChildren(paintContext);
paintContext.internalNodesCanvas.restore();
paintContext.internalNodesCanvas.restore();
Expand Down Expand Up @@ -439,6 +442,7 @@ class ShaderMaskEngineLayer extends ContainerLayer

paintContext.leafNodesCanvas!.drawRect(
ui.Rect.fromLTWH(0, 0, maskRect.width, maskRect.height), paint);
paint.dispose();
paintContext.leafNodesCanvas!.restore();

paintContext.internalNodesCanvas.restore();
Expand Down Expand Up @@ -539,6 +543,7 @@ class PhysicalShapeEngineLayer extends ContainerLayer
// anti-aliased drawPath will always have such artifacts.
paintContext.leafNodesCanvas!.drawPaint(paint);
}
paint.dispose();

paintChildren(paintContext);

Expand Down Expand Up @@ -570,6 +575,7 @@ class ColorFilterEngineLayer extends ContainerLayer
paint.colorFilter = filter;

paintContext.internalNodesCanvas.saveLayer(paintBounds, paint);
paint.dispose();
paintChildren(paintContext);
paintContext.internalNodesCanvas.restore();
}
Expand Down
55 changes: 21 additions & 34 deletions lib/web_ui/lib/src/engine/canvaskit/painting.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,25 @@ import 'skia_object_cache.dart';
///
/// This class is backed by a Skia object that must be explicitly
/// deleted to avoid a memory leak. This is done by extending [SkiaObject].
class CkPaint extends ManagedSkiaObject<SkPaint> implements ui.Paint {
CkPaint();
class CkPaint implements ui.Paint {
CkPaint() : skiaObject = SkPaint() {
skiaObject.setAntiAlias(_isAntiAlias);
skiaObject.setColorInt(_defaultPaintColor.toDouble());
_ref = UniqueRef<SkPaint>(this, skiaObject, 'Paint');
}

final SkPaint skiaObject;
late final UniqueRef<SkPaint> _ref;
CkManagedSkImageFilterConvertible? _imageFilter;

static const int _defaultPaintColor = 0xFF000000;

/// Returns the native reference to the underlying [SkPaint] object.
///
/// This should only be used in tests.
@visibleForTesting
UniqueRef<SkPaint> get debugRef => _ref;

@override
ui.BlendMode get blendMode => _blendMode;
@override
Expand Down Expand Up @@ -277,38 +291,11 @@ class CkPaint extends ManagedSkiaObject<SkPaint> implements ui.Paint {
_imageFilter = filter;
}

CkManagedSkImageFilterConvertible? _imageFilter;

@override
SkPaint createDefault() {
final SkPaint paint = SkPaint();
paint.setAntiAlias(_isAntiAlias);
paint.setColorInt(_color.toDouble());
return paint;
}

@override
SkPaint resurrect() {
final SkPaint paint = SkPaint();
// No need to do anything for `invertColors`. If it was set, then it
// updated `_managedColorFilter`.
paint.setBlendMode(toSkBlendMode(_blendMode));
paint.setStyle(toSkPaintStyle(_style));
paint.setStrokeWidth(_strokeWidth);
paint.setAntiAlias(_isAntiAlias);
paint.setColorInt(_color.toDouble());
paint.setShader(_shader?.getSkShader(_filterQuality));
paint.setMaskFilter(_ckMaskFilter?.skiaObject);
paint.setColorFilter(_effectiveColorFilter?.skiaObject);
paint.setStrokeCap(toSkStrokeCap(_strokeCap));
paint.setStrokeJoin(toSkStrokeJoin(_strokeJoin));
paint.setStrokeMiter(_strokeMiterLimit);
return paint;
}

@override
void delete() {
rawSkiaObject?.delete();
/// Disposes of this paint object.
///
/// This object cannot be used again after calling this method.
void dispose() {
_ref.dispose();
}
}

Expand Down
34 changes: 34 additions & 0 deletions lib/web_ui/test/canvaskit/painting_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// 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.

import 'package:test/bootstrap/browser.dart';
import 'package:test/test.dart';

import 'package:ui/src/engine.dart';

import '../common/matchers.dart';
import 'common.dart';

void main() {
internalBootstrapBrowserTest(() => testMain);
}

void testMain() {
group('CkPaint', () {
setUpCanvasKitTest();

test('lifecycle', () {
final CkPaint paint = CkPaint();
expect(paint.skiaObject, isNotNull);
expect(paint.debugRef.isDisposed, isFalse);
paint.dispose();
expect(paint.debugRef.isDisposed, isTrue);
expect(
reason: 'Cannot dispose more than once',
() => paint.dispose(),
throwsA(isAssertionError),
);
});
});
}

0 comments on commit 4c22713

Please sign in to comment.