From cd9cb1b2bb49e0e6db0b78e5d3bd78ed8d5a29e2 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 1 Oct 2020 21:18:41 -0700 Subject: [PATCH 1/7] Implement Image.clone for CanvasKit --- .../lib/src/engine/canvaskit/image.dart | 43 ++++++++---- .../engine/canvaskit/skia_object_cache.dart | 66 +++++++++++++++++++ lib/web_ui/test/canvaskit/image_test.dart | 26 ++++++++ .../canvaskit/skia_objects_cache_test.dart | 31 +++++++++ 4 files changed, 154 insertions(+), 12 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/image.dart b/lib/web_ui/lib/src/engine/canvaskit/image.dart index badaa8e6042e4..47c8cb43b9200 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/image.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/image.dart @@ -41,10 +41,16 @@ class CkAnimatedImage implements ui.Image { // Use a box because `SkImage` may be deleted either due to this object // being garbage-collected, or by an explicit call to [delete]. - late final SkiaObjectBox box; + late final RefCountedSkiaObjectBox box; - CkAnimatedImage(this._skAnimatedImage) { - box = SkiaObjectBox(this, _skAnimatedImage as SkDeletable); + CkAnimatedImage(SkAnimatedImage skAnimatedImage) : this._(skAnimatedImage, null); + + CkAnimatedImage._(this._skAnimatedImage, RefCountedSkiaObjectBox? boxToClone) { + if (boxToClone != null) { + box = boxToClone.clone(this); + } else { + box = RefCountedSkiaObjectBox(this, _skAnimatedImage as SkDeletable); + } } @override @@ -53,13 +59,17 @@ class CkAnimatedImage implements ui.Image { } @override - ui.Image clone() => this; + ui.Image clone() => CkAnimatedImage._(_skAnimatedImage, box); @override - bool isCloneOf(ui.Image other) => other == this; + bool isCloneOf(ui.Image other) { + return other is CkAnimatedImage + && other._skAnimatedImage == _skAnimatedImage; + } @override - List? debugGetOpenHandleStackTraces() => null; + List? debugGetOpenHandleStackTraces() => box.debugGetStackTraces(); + int get frameCount => _skAnimatedImage.getFrameCount(); /// Decodes the next frame and returns the frame duration. @@ -114,10 +124,16 @@ class CkImage implements ui.Image { // Use a box because `SkImage` may be deleted either due to this object // being garbage-collected, or by an explicit call to [delete]. - late final SkiaObjectBox box; + late final RefCountedSkiaObjectBox box; - CkImage(this.skImage) { - box = SkiaObjectBox(this, skImage as SkDeletable); + CkImage(SkImage skImage) : this._(skImage, null); + + CkImage._(this.skImage, RefCountedSkiaObjectBox? boxToClone) { + if (boxToClone != null) { + box = boxToClone.clone(this); + } else { + box = RefCountedSkiaObjectBox(this, skImage as SkDeletable); + } } @override @@ -126,13 +142,16 @@ class CkImage implements ui.Image { } @override - ui.Image clone() => this; + ui.Image clone() => CkImage._(skImage, box); @override - bool isCloneOf(ui.Image other) => other == this; + bool isCloneOf(ui.Image other) { + return other is CkImage + && other.skImage == skImage; + } @override - List? debugGetOpenHandleStackTraces() => null; + List? debugGetOpenHandleStackTraces() => box.debugGetStackTraces(); @override int get width => skImage.width(); diff --git a/lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart b/lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart index 36cdf7e7c5176..89d631ee85326 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart @@ -282,6 +282,72 @@ class SkiaObjectBox { } } +/// Uses reference counting to manage the lifecycle of a Skia object owned by a +/// wrapper object. +/// +/// When the wrapper is garbage collected, decrements the refcount (only in +/// browsers that support weak references). +/// +/// The [delete] method can be used to eagerly decrement the refcount before the +/// wrapper is garbage collected. +/// +/// The [delete] method may be called any number of times. The box +/// will only delete the object once. +class RefCountedSkiaObjectBox extends SkiaObjectBox { + RefCountedSkiaObjectBox(Object wrapper, SkDeletable skObject) + : this._(wrapper, skObject, {}); + + RefCountedSkiaObjectBox._(Object wrapper, SkDeletable skObject, this._refs) + : super(wrapper, skObject) { + _refs.add(this); + if (assertionsEnabled) { + _debugStackTrace = StackTrace.current; + } + } + + /// Reference handles to the same underlying [skObject]. + final Set _refs; + + late final StackTrace? _debugStackTrace; + /// If asserts are enabled, the [StackTrace]s representing when a reference + /// was created. + List? debugGetStackTraces() { + if (assertionsEnabled) { + return _refs + .map((RefCountedSkiaObjectBox box) => box._debugStackTrace!) + .toList(); + } + return null; + } + + RefCountedSkiaObjectBox clone(Object wrapper) { + assert(!_isDeleted, 'Cannot clone from a deleted handle.'); + assert(_refs.isNotEmpty); + return RefCountedSkiaObjectBox._(wrapper, skObject, _refs); + } + + /// Removes the reference count for the [skObject]. + /// + /// Does nothing if the object has already been deleted. + /// + /// If this causes the reference count to drop to zero, deletes the + /// [skObject]. + @override + void delete() { + if (_isDeleted) { + assert(!_refs.contains(this)); + return; + } + final bool removed = _refs.remove(this); + assert(removed); + _isDeleted = true; + if (_refs.isEmpty) { + _skObjectDeleteQueue.add(skObject); + _skObjectCollector ??= _scheduleSkObjectCollection(); + } + } +} + /// Singleton that manages the lifecycles of [SkiaObject] instances. class SkiaObjects { @visibleForTesting diff --git a/lib/web_ui/test/canvaskit/image_test.dart b/lib/web_ui/test/canvaskit/image_test.dart index 11ed6e2a4b741..b7d89dc5e6ee1 100644 --- a/lib/web_ui/test/canvaskit/image_test.dart +++ b/lib/web_ui/test/canvaskit/image_test.dart @@ -38,6 +38,19 @@ void testMain() { expect(image.box.isDeleted, true); }); + test('CkAnimatedImage can be cloned and explicitly disposed of', () { + final SkAnimatedImage skAnimatedImage = canvasKit.MakeAnimatedImageFromEncoded(kTransparentImage); + final CkAnimatedImage image = CkAnimatedImage(skAnimatedImage); + final CkAnimatedImage imageClone = image.clone(); + + expect(image.isCloneOf(imageClone), true); + expect(image.box.isDeleted, false); + image.dispose(); + expect(image.box.isDeleted, false); + imageClone.dispose(); + expect(image.box.isDeleted, true); + }); + test('CkImage toString', () { final SkImage skImage = canvasKit.MakeAnimatedImageFromEncoded(kTransparentImage).getCurrentFrame(); final CkImage image = CkImage(skImage); @@ -54,6 +67,19 @@ void testMain() { image.dispose(); expect(image.box.isDeleted, true); }); + + test('CkImage can be explicitly disposed of', () { + final SkImage skImage = canvasKit.MakeAnimatedImageFromEncoded(kTransparentImage).getCurrentFrame(); + final CkImage image = CkImage(skImage); + final CkAnimatedImage imageClone = image.clone(); + + expect(image.isCloneOf(imageClone), true); + expect(image.box.isDeleted, false); + image.dispose(); + expect(image.box.isDeleted, false); + imageClone.dispose(); + expect(image.box.isDeleted, true); + }); // TODO: https://github.com/flutter/flutter/issues/60040 }, skip: isIosSafari); } diff --git a/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart b/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart index 08e0cb3d55837..b90b31e4a3d8b 100644 --- a/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart +++ b/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart @@ -12,6 +12,7 @@ import 'package:ui/ui.dart' as ui; import 'package:ui/src/engine.dart'; import 'common.dart'; +import '../matchers.dart'; void main() { internalBootstrapBrowserTest(() => testMain); @@ -153,6 +154,36 @@ void _tests() { expect(SkiaObjects.oneShotCache.debugContains(object2), isFalse); }); }); + + group(RefCountedSkiaObjectBox, () { + test('Records stack traces and respects refcounts', () { + final TestSkiaObject skObject = TestSkiaObject(); + final Object wrapper = Object(); + final RefCountedSkiaObjectBox box = RefCountedSkiaObjectBox(wrapper, skObject as SkDeletable); + + expect(box.debugGetStackTraces().length, 1); + + final RefCountedSkiaObjectBox clone = box.clone(wrapper); + expect(clone, isNot(same(box))); + expect(clone.debugGetStackTraces().length, 2); + expect(box.debugGetStackTraces().length, 2); + + box.delete(); + + expect(() => box.clone(wrapper), throwsAssertionError); + + expect(skObject.deleteCount, 0); + expect(clone.debugGetStackTraces().length, 1); + expect(box.debugGetStackTraces().length, 1); + + clone.delete(); + expect(() => clone.clone(wrapper), throwsAssertionError); + + expect(skObject.deleteCount, 1); + expect(clone.debugGetStackTraces().length, 0); + expect(box.debugGetStackTraces().length, 0); + }); + }); } class TestOneShotSkiaObject extends OneShotSkiaObject { From 1b3ae468d0c34e2516ea43db7bf50c2740b5d4a9 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 1 Oct 2020 22:06:52 -0700 Subject: [PATCH 2/7] fix tests --- lib/web_ui/test/canvaskit/image_test.dart | 10 +++++++--- lib/web_ui/test/canvaskit/skia_objects_cache_test.dart | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/web_ui/test/canvaskit/image_test.dart b/lib/web_ui/test/canvaskit/image_test.dart index b7d89dc5e6ee1..8980586db0445 100644 --- a/lib/web_ui/test/canvaskit/image_test.dart +++ b/lib/web_ui/test/canvaskit/image_test.dart @@ -46,9 +46,11 @@ void testMain() { expect(image.isCloneOf(imageClone), true); expect(image.box.isDeleted, false); image.dispose(); - expect(image.box.isDeleted, false); + expect(image.box.isDeleted, true); + expect(imageClone.box.isDeleted, false); imageClone.dispose(); expect(image.box.isDeleted, true); + expect(imageClone.box.isDeleted, true); }); test('CkImage toString', () { @@ -71,14 +73,16 @@ void testMain() { test('CkImage can be explicitly disposed of', () { final SkImage skImage = canvasKit.MakeAnimatedImageFromEncoded(kTransparentImage).getCurrentFrame(); final CkImage image = CkImage(skImage); - final CkAnimatedImage imageClone = image.clone(); + final CkImage imageClone = image.clone(); expect(image.isCloneOf(imageClone), true); expect(image.box.isDeleted, false); image.dispose(); - expect(image.box.isDeleted, false); + expect(image.box.isDeleted, true); + expect(imageClone.box.isDeleted, false); imageClone.dispose(); expect(image.box.isDeleted, true); + expect(imageClone.box.isDeleted, true); }); // TODO: https://github.com/flutter/flutter/issues/60040 }, skip: isIosSafari); diff --git a/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart b/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart index b90b31e4a3d8b..af2a3d9c2a858 100644 --- a/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart +++ b/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart @@ -198,7 +198,7 @@ class TestOneShotSkiaObject extends OneShotSkiaObject { } } -class TestSkiaObject extends ManagedSkiaObject { +class TestSkiaObject extends ManagedSkiaObject implements SkDeletable { int createDefaultCount = 0; int resurrectCount = 0; int deleteCount = 0; From a1deae380bcc707c09d5363f7628fec6f13b6a95 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 1 Oct 2020 22:36:22 -0700 Subject: [PATCH 3/7] Update skia_objects_cache_test.dart --- lib/web_ui/test/canvaskit/skia_objects_cache_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart b/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart index af2a3d9c2a858..023299298cd97 100644 --- a/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart +++ b/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart @@ -159,7 +159,7 @@ void _tests() { test('Records stack traces and respects refcounts', () { final TestSkiaObject skObject = TestSkiaObject(); final Object wrapper = Object(); - final RefCountedSkiaObjectBox box = RefCountedSkiaObjectBox(wrapper, skObject as SkDeletable); + final RefCountedSkiaObjectBox box = RefCountedSkiaObjectBox(wrapper, skObject); expect(box.debugGetStackTraces().length, 1); From 49ed2fa8b613ec3f895b8578d58268818fe8fe52 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 2 Oct 2020 10:42:26 -0700 Subject: [PATCH 4/7] Make SkiaObjectBox refcounted, fix test --- .../lib/src/engine/canvaskit/image.dart | 12 +-- .../engine/canvaskit/skia_object_cache.dart | 84 +++++++------------ .../canvaskit/skia_objects_cache_test.dart | 28 +++++-- 3 files changed, 54 insertions(+), 70 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/image.dart b/lib/web_ui/lib/src/engine/canvaskit/image.dart index 47c8cb43b9200..7ba2de2864a01 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/image.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/image.dart @@ -41,15 +41,15 @@ class CkAnimatedImage implements ui.Image { // Use a box because `SkImage` may be deleted either due to this object // being garbage-collected, or by an explicit call to [delete]. - late final RefCountedSkiaObjectBox box; + late final SkiaObjectBox box; CkAnimatedImage(SkAnimatedImage skAnimatedImage) : this._(skAnimatedImage, null); - CkAnimatedImage._(this._skAnimatedImage, RefCountedSkiaObjectBox? boxToClone) { + CkAnimatedImage._(this._skAnimatedImage, SkiaObjectBox? boxToClone) { if (boxToClone != null) { box = boxToClone.clone(this); } else { - box = RefCountedSkiaObjectBox(this, _skAnimatedImage as SkDeletable); + box = SkiaObjectBox(this, _skAnimatedImage as SkDeletable); } } @@ -124,15 +124,15 @@ class CkImage implements ui.Image { // Use a box because `SkImage` may be deleted either due to this object // being garbage-collected, or by an explicit call to [delete]. - late final RefCountedSkiaObjectBox box; + late final SkiaObjectBox box; CkImage(SkImage skImage) : this._(skImage, null); - CkImage._(this.skImage, RefCountedSkiaObjectBox? boxToClone) { + CkImage._(this.skImage, SkiaObjectBox? boxToClone) { if (boxToClone != null) { box = boxToClone.clone(this); } else { - box = RefCountedSkiaObjectBox(this, skImage as SkDeletable); + box = SkiaObjectBox(this, skImage as SkDeletable); } } diff --git a/lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart b/lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart index 89d631ee85326..eaca5c43ca4b9 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart @@ -239,49 +239,6 @@ abstract class OneShotSkiaObject extends SkiaObject { } } -/// Manages the lifecycle of a Skia object owned by a wrapper object. -/// -/// When the wrapper is garbage collected, deletes the corresponding -/// [skObject] (only in browsers that support weak references). -/// -/// The [delete] method can be used to eagerly delete the [skObject] -/// before the wrapper is garbage collected. -/// -/// The [delete] method may be called any number of times. The box -/// will only delete the object once. -class SkiaObjectBox { - SkiaObjectBox(Object wrapper, this.skObject) { - if (browserSupportsFinalizationRegistry) { - boxRegistry.register(wrapper, this); - } - } - - /// The Skia object whose lifecycle is being managed. - final SkDeletable skObject; - - /// Whether this object has been deleted. - bool get isDeleted => _isDeleted; - bool _isDeleted = false; - - /// Deletes Skia objects when their wrappers are garbage collected. - static final SkObjectFinalizationRegistry boxRegistry = - SkObjectFinalizationRegistry(js.allowInterop((SkiaObjectBox box) { - box.delete(); - })); - - /// Deletes the [skObject]. - /// - /// Does nothing if the object has already been deleted. - void delete() { - if (_isDeleted) { - return; - } - _isDeleted = true; - _skObjectDeleteQueue.add(skObject); - _skObjectCollector ??= _scheduleSkObjectCollection(); - } -} - /// Uses reference counting to manage the lifecycle of a Skia object owned by a /// wrapper object. /// @@ -293,20 +250,22 @@ class SkiaObjectBox { /// /// The [delete] method may be called any number of times. The box /// will only delete the object once. -class RefCountedSkiaObjectBox extends SkiaObjectBox { - RefCountedSkiaObjectBox(Object wrapper, SkDeletable skObject) - : this._(wrapper, skObject, {}); +class SkiaObjectBox { + SkiaObjectBox(Object wrapper, SkDeletable skObject) + : this._(wrapper, skObject, {}); - RefCountedSkiaObjectBox._(Object wrapper, SkDeletable skObject, this._refs) - : super(wrapper, skObject) { - _refs.add(this); + SkiaObjectBox._(Object wrapper, this.skObject, this._refs) { if (assertionsEnabled) { _debugStackTrace = StackTrace.current; } + _refs.add(this); + if (browserSupportsFinalizationRegistry) { + boxRegistry.register(wrapper, this); + } } /// Reference handles to the same underlying [skObject]. - final Set _refs; + final Set _refs; late final StackTrace? _debugStackTrace; /// If asserts are enabled, the [StackTrace]s representing when a reference @@ -314,25 +273,40 @@ class RefCountedSkiaObjectBox extends SkiaObjectBox { List? debugGetStackTraces() { if (assertionsEnabled) { return _refs - .map((RefCountedSkiaObjectBox box) => box._debugStackTrace!) + .map((SkiaObjectBox box) => box._debugStackTrace!) .toList(); } return null; } - RefCountedSkiaObjectBox clone(Object wrapper) { + /// The Skia object whose lifecycle is being managed. + final SkDeletable skObject; + + /// Whether this object has been deleted. + bool get isDeleted => _isDeleted; + bool _isDeleted = false; + + /// Deletes Skia objects when their wrappers are garbage collected. + static final SkObjectFinalizationRegistry boxRegistry = + SkObjectFinalizationRegistry(js.allowInterop((SkiaObjectBox box) { + box.delete(); + })); + + /// Returns a clone of this object, which increases its reference count. + /// + /// Clones must be [dispose]d when finished. + SkiaObjectBox clone(Object wrapper) { assert(!_isDeleted, 'Cannot clone from a deleted handle.'); assert(_refs.isNotEmpty); - return RefCountedSkiaObjectBox._(wrapper, skObject, _refs); + return SkiaObjectBox._(wrapper, skObject, _refs); } - /// Removes the reference count for the [skObject]. + /// Decrements the reference count for the [skObject]. /// /// Does nothing if the object has already been deleted. /// /// If this causes the reference count to drop to zero, deletes the /// [skObject]. - @override void delete() { if (_isDeleted) { assert(!_refs.contains(this)); diff --git a/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart b/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart index 023299298cd97..a47246c24894b 100644 --- a/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart +++ b/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart @@ -155,15 +155,17 @@ void _tests() { }); }); - group(RefCountedSkiaObjectBox, () { - test('Records stack traces and respects refcounts', () { - final TestSkiaObject skObject = TestSkiaObject(); + + group(SkiaObjectBox, () { + test('Records stack traces and respects refcounts', () async { + TestOneShotSkiaObject.deleteCount = 0; + final TestOneShotSkiaObject skObject = TestOneShotSkiaObject(); final Object wrapper = Object(); - final RefCountedSkiaObjectBox box = RefCountedSkiaObjectBox(wrapper, skObject); + final SkiaObjectBox box = SkiaObjectBox(wrapper, skObject); expect(box.debugGetStackTraces().length, 1); - final RefCountedSkiaObjectBox clone = box.clone(wrapper); + final SkiaObjectBox clone = box.clone(wrapper); expect(clone, isNot(same(box))); expect(clone.debugGetStackTraces().length, 2); expect(box.debugGetStackTraces().length, 2); @@ -172,21 +174,29 @@ void _tests() { expect(() => box.clone(wrapper), throwsAssertionError); - expect(skObject.deleteCount, 0); + expect(box.isDeleted, true); + + // Let any timers elapse. + await Future.delayed(Duration.zero); + expect(TestOneShotSkiaObject.deleteCount, 0); + expect(clone.debugGetStackTraces().length, 1); expect(box.debugGetStackTraces().length, 1); clone.delete(); expect(() => clone.clone(wrapper), throwsAssertionError); - expect(skObject.deleteCount, 1); + // Let any timers elapse. + await Future.delayed(Duration.zero); + expect(TestOneShotSkiaObject.deleteCount, 1); + expect(clone.debugGetStackTraces().length, 0); expect(box.debugGetStackTraces().length, 0); }); }); } -class TestOneShotSkiaObject extends OneShotSkiaObject { +class TestOneShotSkiaObject extends OneShotSkiaObject implements SkDeletable { static int deleteCount = 0; TestOneShotSkiaObject() : super(SkPaint()); @@ -198,7 +208,7 @@ class TestOneShotSkiaObject extends OneShotSkiaObject { } } -class TestSkiaObject extends ManagedSkiaObject implements SkDeletable { +class TestSkiaObject extends ManagedSkiaObject { int createDefaultCount = 0; int resurrectCount = 0; int deleteCount = 0; From 4594551e2daf22ef8ab2d5c02d2694b376f84fe7 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 2 Oct 2020 11:19:26 -0700 Subject: [PATCH 5/7] isDeleted/isAliasOf embind --- .../lib/src/engine/canvaskit/canvaskit_api.dart | 4 ++++ lib/web_ui/lib/src/engine/canvaskit/image.dart | 4 ++-- lib/web_ui/test/canvaskit/image_test.dart | 16 ++++++++++++++-- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart index bca5a9b995472..1252cba16e436 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart @@ -683,6 +683,8 @@ class SkAnimatedImage { /// /// This object is no longer usable after calling this method. external void delete(); + external bool isAliasOf(SkAnimatedImage other); + external bool isDeleted(); } @JS() @@ -698,6 +700,8 @@ class SkImage { ); external Uint8List readPixels(SkImageInfo imageInfo, int srcX, int srcY); external SkData encodeToData(); + external bool isAliasOf(SkImage other); + external bool isDeleted(); } @JS() diff --git a/lib/web_ui/lib/src/engine/canvaskit/image.dart b/lib/web_ui/lib/src/engine/canvaskit/image.dart index 7ba2de2864a01..dbf69fadcf901 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/image.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/image.dart @@ -64,7 +64,7 @@ class CkAnimatedImage implements ui.Image { @override bool isCloneOf(ui.Image other) { return other is CkAnimatedImage - && other._skAnimatedImage == _skAnimatedImage; + && other._skAnimatedImage.isAliasOf(_skAnimatedImage); } @override @@ -147,7 +147,7 @@ class CkImage implements ui.Image { @override bool isCloneOf(ui.Image other) { return other is CkImage - && other.skImage == skImage; + && other.skImage.isAliasOf(skImage); } @override diff --git a/lib/web_ui/test/canvaskit/image_test.dart b/lib/web_ui/test/canvaskit/image_test.dart index 8980586db0445..08be25e0937aa 100644 --- a/lib/web_ui/test/canvaskit/image_test.dart +++ b/lib/web_ui/test/canvaskit/image_test.dart @@ -38,19 +38,25 @@ void testMain() { expect(image.box.isDeleted, true); }); - test('CkAnimatedImage can be cloned and explicitly disposed of', () { + test('CkAnimatedImage can be cloned and explicitly disposed of', () async { final SkAnimatedImage skAnimatedImage = canvasKit.MakeAnimatedImageFromEncoded(kTransparentImage); final CkAnimatedImage image = CkAnimatedImage(skAnimatedImage); final CkAnimatedImage imageClone = image.clone(); expect(image.isCloneOf(imageClone), true); expect(image.box.isDeleted, false); + await Future.delayed(Duration.zero); + expect(skAnimatedImage.isDeleted(), false); image.dispose(); expect(image.box.isDeleted, true); expect(imageClone.box.isDeleted, false); + await Future.delayed(Duration.zero); + expect(skAnimatedImage.isDeleted(), false); imageClone.dispose(); expect(image.box.isDeleted, true); expect(imageClone.box.isDeleted, true); + await Future.delayed(Duration.zero); + expect(skAnimatedImage.isDeleted(), true); }); test('CkImage toString', () { @@ -70,19 +76,25 @@ void testMain() { expect(image.box.isDeleted, true); }); - test('CkImage can be explicitly disposed of', () { + test('CkImage can be explicitly disposed of when cloned', () async { final SkImage skImage = canvasKit.MakeAnimatedImageFromEncoded(kTransparentImage).getCurrentFrame(); final CkImage image = CkImage(skImage); final CkImage imageClone = image.clone(); expect(image.isCloneOf(imageClone), true); expect(image.box.isDeleted, false); + await Future.delayed(Duration.zero); + expect(skImage.isDeleted(), false); image.dispose(); expect(image.box.isDeleted, true); expect(imageClone.box.isDeleted, false); + await Future.delayed(Duration.zero); + expect(skImage.isDeleted(), false); imageClone.dispose(); expect(image.box.isDeleted, true); expect(imageClone.box.isDeleted, true); + await Future.delayed(Duration.zero); + expect(skImage.isDeleted(), true); }); // TODO: https://github.com/flutter/flutter/issues/60040 }, skip: isIosSafari); From 38f0221979516c50db156ba4d36bd9870840ad01 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 2 Oct 2020 11:31:44 -0700 Subject: [PATCH 6/7] asserts --- lib/web_ui/lib/src/engine/canvaskit/image.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/web_ui/lib/src/engine/canvaskit/image.dart b/lib/web_ui/lib/src/engine/canvaskit/image.dart index dbf69fadcf901..9d3cff2575864 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/image.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/image.dart @@ -47,6 +47,7 @@ class CkAnimatedImage implements ui.Image { CkAnimatedImage._(this._skAnimatedImage, SkiaObjectBox? boxToClone) { if (boxToClone != null) { + assert(boxToClone.skObject == _skAnimatedImage); box = boxToClone.clone(this); } else { box = SkiaObjectBox(this, _skAnimatedImage as SkDeletable); @@ -130,6 +131,7 @@ class CkImage implements ui.Image { CkImage._(this.skImage, SkiaObjectBox? boxToClone) { if (boxToClone != null) { + assert(boxToClone.skObject == _skAnimatedImage); box = boxToClone.clone(this); } else { box = SkiaObjectBox(this, skImage as SkDeletable); From f1727e02bae4ccdaee4dd283f3a5f08edb82d55a Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 2 Oct 2020 11:56:18 -0700 Subject: [PATCH 7/7] copy paste error... --- lib/web_ui/lib/src/engine/canvaskit/image.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/image.dart b/lib/web_ui/lib/src/engine/canvaskit/image.dart index 9d3cff2575864..192e301c1bdcd 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/image.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/image.dart @@ -131,7 +131,7 @@ class CkImage implements ui.Image { CkImage._(this.skImage, SkiaObjectBox? boxToClone) { if (boxToClone != null) { - assert(boxToClone.skObject == _skAnimatedImage); + assert(boxToClone.skObject == skImage); box = boxToClone.clone(this); } else { box = SkiaObjectBox(this, skImage as SkDeletable);