Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand Down
41 changes: 31 additions & 10 deletions lib/web_ui/lib/src/engine/canvaskit/image.dart
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,15 @@ class CkAnimatedImage implements ui.Image {
// being garbage-collected, or by an explicit call to [delete].
late final SkiaObjectBox box;

CkAnimatedImage(this._skAnimatedImage) {
box = SkiaObjectBox(this, _skAnimatedImage as SkDeletable);
CkAnimatedImage(SkAnimatedImage skAnimatedImage) : this._(skAnimatedImage, null);

CkAnimatedImage._(this._skAnimatedImage, SkiaObjectBox? boxToClone) {
if (boxToClone != null) {
assert(boxToClone.skObject == _skAnimatedImage);
box = boxToClone.clone(this);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we assert that the image in boxToClone and _skAnimatedImage are the same?

Alternatively, we could split this into two specialized constructors.

} else {
box = SkiaObjectBox(this, _skAnimatedImage as SkDeletable);
}
}

@override
Expand All @@ -53,13 +60,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.isAliasOf(_skAnimatedImage);
}

@override
List<StackTrace>? debugGetOpenHandleStackTraces() => null;
List<StackTrace>? debugGetOpenHandleStackTraces() => box.debugGetStackTraces();

int get frameCount => _skAnimatedImage.getFrameCount();

/// Decodes the next frame and returns the frame duration.
Expand Down Expand Up @@ -116,8 +127,15 @@ class CkImage implements ui.Image {
// being garbage-collected, or by an explicit call to [delete].
late final SkiaObjectBox box;

CkImage(this.skImage) {
box = SkiaObjectBox(this, skImage as SkDeletable);
CkImage(SkImage skImage) : this._(skImage, null);

CkImage._(this.skImage, SkiaObjectBox? boxToClone) {
if (boxToClone != null) {
assert(boxToClone.skObject == skImage);
box = boxToClone.clone(this);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment as in CkAnimatedImage.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added these asserts

} else {
box = SkiaObjectBox(this, skImage as SkDeletable);
}
}

@override
Expand All @@ -126,13 +144,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.isAliasOf(skImage);
}

@override
List<StackTrace>? debugGetOpenHandleStackTraces() => null;
List<StackTrace>? debugGetOpenHandleStackTraces() => box.debugGetStackTraces();

@override
int get width => skImage.width();
Expand Down
58 changes: 49 additions & 9 deletions lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart
Original file line number Diff line number Diff line change
Expand Up @@ -239,23 +239,46 @@ abstract class OneShotSkiaObject<T extends Object> extends SkiaObject<T> {
}
}

/// Manages the lifecycle of a Skia object owned by a wrapper object.
/// Uses reference counting to manage 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).
/// When the wrapper is garbage collected, decrements the refcount (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 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 SkiaObjectBox {
SkiaObjectBox(Object wrapper, this.skObject) {
SkiaObjectBox(Object wrapper, SkDeletable skObject)
: this._(wrapper, skObject, <SkiaObjectBox>{});

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<SkiaObjectBox> _refs;

late final StackTrace? _debugStackTrace;
/// If asserts are enabled, the [StackTrace]s representing when a reference
/// was created.
List<StackTrace>? debugGetStackTraces() {
if (assertionsEnabled) {
return _refs
.map<StackTrace>((SkiaObjectBox box) => box._debugStackTrace!)
.toList();
}
return null;
}

/// The Skia object whose lifecycle is being managed.
final SkDeletable skObject;

Expand All @@ -269,16 +292,33 @@ class SkiaObjectBox {
box.delete();
}));

/// Deletes the [skObject].
/// 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 SkiaObjectBox._(wrapper, skObject, _refs);
}

/// 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].
void delete() {
if (_isDeleted) {
assert(!_refs.contains(this));
return;
}
final bool removed = _refs.remove(this);
assert(removed);
_isDeleted = true;
_skObjectDeleteQueue.add(skObject);
_skObjectCollector ??= _scheduleSkObjectCollection();
if (_refs.isEmpty) {
_skObjectDeleteQueue.add(skObject);
_skObjectCollector ??= _scheduleSkObjectCollection();
}
}
}

Expand Down
42 changes: 42 additions & 0 deletions lib/web_ui/test/canvaskit/image_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,27 @@ void testMain() {
expect(image.box.isDeleted, true);
});

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<void>.delayed(Duration.zero);
expect(skAnimatedImage.isDeleted(), false);
image.dispose();
expect(image.box.isDeleted, true);
expect(imageClone.box.isDeleted, false);
await Future<void>.delayed(Duration.zero);
expect(skAnimatedImage.isDeleted(), false);
imageClone.dispose();
expect(image.box.isDeleted, true);
expect(imageClone.box.isDeleted, true);
await Future<void>.delayed(Duration.zero);
expect(skAnimatedImage.isDeleted(), true);
});

test('CkImage toString', () {
final SkImage skImage = canvasKit.MakeAnimatedImageFromEncoded(kTransparentImage).getCurrentFrame();
final CkImage image = CkImage(skImage);
Expand All @@ -54,6 +75,27 @@ void testMain() {
image.dispose();
expect(image.box.isDeleted, true);
});

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<void>.delayed(Duration.zero);
expect(skImage.isDeleted(), false);
image.dispose();
expect(image.box.isDeleted, true);
expect(imageClone.box.isDeleted, false);
await Future<void>.delayed(Duration.zero);
expect(skImage.isDeleted(), false);
imageClone.dispose();
expect(image.box.isDeleted, true);
expect(imageClone.box.isDeleted, true);
await Future<void>.delayed(Duration.zero);
expect(skImage.isDeleted(), true);
});
// TODO: https://github.com/flutter/flutter/issues/60040
}, skip: isIosSafari);
}
43 changes: 42 additions & 1 deletion lib/web_ui/test/canvaskit/skia_objects_cache_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -153,9 +154,49 @@ void _tests() {
expect(SkiaObjects.oneShotCache.debugContains(object2), isFalse);
});
});


group(SkiaObjectBox, () {
test('Records stack traces and respects refcounts', () async {
TestOneShotSkiaObject.deleteCount = 0;
final TestOneShotSkiaObject skObject = TestOneShotSkiaObject();
final Object wrapper = Object();
final SkiaObjectBox box = SkiaObjectBox(wrapper, skObject);

expect(box.debugGetStackTraces().length, 1);

final SkiaObjectBox 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(box.isDeleted, true);

// Let any timers elapse.
await Future<void>.delayed(Duration.zero);
expect(TestOneShotSkiaObject.deleteCount, 0);

expect(clone.debugGetStackTraces().length, 1);
expect(box.debugGetStackTraces().length, 1);

clone.delete();
expect(() => clone.clone(wrapper), throwsAssertionError);

// Let any timers elapse.
await Future<void>.delayed(Duration.zero);
expect(TestOneShotSkiaObject.deleteCount, 1);

expect(clone.debugGetStackTraces().length, 0);
expect(box.debugGetStackTraces().length, 0);
});
});
}

class TestOneShotSkiaObject extends OneShotSkiaObject<SkPaint> {
class TestOneShotSkiaObject extends OneShotSkiaObject<SkPaint> implements SkDeletable {
static int deleteCount = 0;

TestOneShotSkiaObject() : super(SkPaint());
Expand Down