Skip to content

Commit

Permalink
Refactor CanvasKit image ref counting; fix a minor memory leak (flutt…
Browse files Browse the repository at this point in the history 
…er#22549)

* Refactor SkiaObjectBox ref counting
* make CkAnimatedImage a Codec
* disallow double dispose; better assertion messages
  • Loading branch information
@yjbanov
yjbanov authored Nov 18, 2020
1 parent 9a5fd32 commit 35a0b9f
Show file tree
Hide file tree
Showing 6 changed files with 249 additions and 214 deletions.
4 changes: 1 addition & 3 deletions lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -685,14 +685,11 @@ class SkAnimatedImage {
external SkImage getCurrentFrame();
external int width();
external int height();
external Uint8List readPixels(SkImageInfo imageInfo, int srcX, int srcY);
external SkData encodeToData();

/// Deletes the C++ object.
///
/// This object is no longer usable after calling this method.
external void delete();
external bool isAliasOf(SkAnimatedImage other);
external bool isDeleted();
}

Expand Down Expand Up @@ -1820,6 +1817,7 @@ class SkData {
external int size();
external bool isEmpty();
external Uint8List bytes();
external void delete();
}

@JS()
Expand Down
216 changes: 96 additions & 120 deletions lib/web_ui/lib/src/engine/canvaskit/image.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,17 @@ part of engine;
/// Instantiates a [ui.Codec] backed by an `SkAnimatedImage` from Skia.
void skiaInstantiateImageCodec(Uint8List list, Callback<ui.Codec> callback,
[int? width, int? height, int? format, int? rowBytes]) {
final SkAnimatedImage skAnimatedImage =
canvasKit.MakeAnimatedImageFromEncoded(list);
final CkAnimatedImage animatedImage = CkAnimatedImage(skAnimatedImage);
final CkAnimatedImageCodec codec = CkAnimatedImageCodec(animatedImage);
final CkAnimatedImage codec = CkAnimatedImage.decodeFromBytes(list);
callback(codec);
}

/// Instantiates a [ui.Codec] backed by an `SkAnimatedImage` from Skia after
/// requesting from URI.
Future<ui.Codec> skiaInstantiateWebImageCodec(
String src, WebOnlyImageCodecChunkCallback? chunkCallback) {
String uri, WebOnlyImageCodecChunkCallback? chunkCallback) {
Completer<ui.Codec> completer = Completer<ui.Codec>();
//TODO: Switch to using MakeImageFromCanvasImageSource when animated images are supported.
html.HttpRequest.request(src, responseType: "arraybuffer",
html.HttpRequest.request(uri, responseType: "arraybuffer",
onProgress: (html.ProgressEvent event) {
if (event.lengthComputable) {
chunkCallback?.call(event.loaded!, event.total!);
Expand All @@ -33,134 +30,123 @@ Future<ui.Codec> skiaInstantiateWebImageCodec(
}
final Uint8List list =
new Uint8List.view((response.response as ByteBuffer));
final SkAnimatedImage skAnimatedImage =
canvasKit.MakeAnimatedImageFromEncoded(list);
final CkAnimatedImage animatedImage = CkAnimatedImage(skAnimatedImage);
final CkAnimatedImageCodec codec = CkAnimatedImageCodec(animatedImage);
final CkAnimatedImage codec = CkAnimatedImage.decodeFromBytes(list);
completer.complete(codec);
}, onError: (dynamic error) {
completer.completeError(error);
});
return completer.future;
}

/// A wrapper for `SkAnimatedImage`.
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<SkAnimatedImage> box;
/// The CanvasKit implementation of [ui.Codec].
///
/// Wraps `SkAnimatedImage`.
class CkAnimatedImage implements ui.Codec, StackTraceDebugger {
/// Decodes an image from a list of encoded bytes.
CkAnimatedImage.decodeFromBytes(Uint8List bytes) {
if (assertionsEnabled) {
_debugStackTrace = StackTrace.current;
}
final SkAnimatedImage skAnimatedImage =
canvasKit.MakeAnimatedImageFromEncoded(bytes);
box = SkiaObjectBox<CkAnimatedImage, SkAnimatedImage>(this, skAnimatedImage);
}

SkAnimatedImage get _skAnimatedImage => box.skObject;
// Use a box because `CkAnimatedImage` may be deleted either due to this
// object being garbage-collected, or by an explicit call to [dispose].
late final SkiaObjectBox<CkAnimatedImage, SkAnimatedImage> box;

CkAnimatedImage(SkAnimatedImage skAnimatedImage) {
box = SkiaObjectBox<SkAnimatedImage>(this, skAnimatedImage);
}
@override
StackTrace get debugStackTrace => _debugStackTrace!;
StackTrace? _debugStackTrace;

bool _disposed = false;
bool get debugDisposed => _disposed;

CkAnimatedImage.cloneOf(SkiaObjectBox<SkAnimatedImage> boxToClone) {
box = boxToClone.clone(this);
bool _debugCheckIsNotDisposed() {
assert(!_disposed, 'This image has been disposed.');
return true;
}

bool _disposed = false;
@override
void dispose() {
box.delete();
assert(
!_disposed,
'Cannot dispose a codec that has already been disposed.',
);
_disposed = true;

// This image is no longer usable. Bump the ref count.
box.unref(this);
}

@override
bool get debugDisposed {
if (assertionsEnabled) {
return _disposed;
}
throw StateError(
'Image.debugDisposed is only available when asserts are enabled.');
int get frameCount {
assert(_debugCheckIsNotDisposed());
return box.skiaObject.getFrameCount();
}

ui.Image clone() => CkAnimatedImage.cloneOf(box);

@override
bool isCloneOf(ui.Image other) {
return other is CkAnimatedImage &&
other._skAnimatedImage.isAliasOf(_skAnimatedImage);
int get repetitionCount {
assert(_debugCheckIsNotDisposed());
return box.skiaObject.getRepetitionCount();
}

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

int get frameCount => _skAnimatedImage.getFrameCount();

/// Decodes the next frame and returns the frame duration.
Duration decodeNextFrame() {
final int durationMillis = _skAnimatedImage.decodeNextFrame();
return Duration(milliseconds: durationMillis);
Future<ui.FrameInfo> getNextFrame() {
assert(_debugCheckIsNotDisposed());
final int durationMillis = box.skiaObject.decodeNextFrame();
final Duration duration = Duration(milliseconds: durationMillis);
final CkImage image = CkImage(box.skiaObject.getCurrentFrame());
return Future<ui.FrameInfo>.value(AnimatedImageFrameInfo(duration, image));
}
}

int get repetitionCount => _skAnimatedImage.getRepetitionCount();

CkImage get currentFrameAsImage {
return CkImage(_skAnimatedImage.getCurrentFrame());
/// A [ui.Image] backed by an `SkImage` from Skia.
class CkImage implements ui.Image, StackTraceDebugger {
CkImage(SkImage skImage) {
if (assertionsEnabled) {
_debugStackTrace = StackTrace.current;
}
box = SkiaObjectBox<CkImage, SkImage>(this, skImage);
}

@override
int get width => _skAnimatedImage.width();

@override
int get height => _skAnimatedImage.height();

@override
Future<ByteData> toByteData(
{ui.ImageByteFormat format = ui.ImageByteFormat.rawRgba}) {
Uint8List bytes;

if (format == ui.ImageByteFormat.rawRgba) {
final SkImageInfo imageInfo = SkImageInfo(
alphaType: canvasKit.AlphaType.Premul,
colorType: canvasKit.ColorType.RGBA_8888,
colorSpace: SkColorSpaceSRGB,
width: width,
height: height,
);
bytes = _skAnimatedImage.readPixels(imageInfo, 0, 0);
} else {
// Defaults to PNG 100%.
final SkData skData = _skAnimatedImage.encodeToData();
// Make a copy that we can return.
bytes = Uint8List.fromList(canvasKit.getDataBytes(skData));
CkImage.cloneOf(this.box) {
if (assertionsEnabled) {
_debugStackTrace = StackTrace.current;
}

final ByteData data = bytes.buffer.asByteData(0, bytes.length);
return Future<ByteData>.value(data);
box.ref(this);
}

@override
String toString() => '[$width\u00D7$height]';
}
StackTrace get debugStackTrace => _debugStackTrace!;
StackTrace? _debugStackTrace;

/// A [ui.Image] backed by an `SkImage` from Skia.
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<SkImage> box;
late final SkiaObjectBox<CkImage, SkImage> box;

SkImage get skImage => box.skObject;
/// The underlying Skia image object.
///
/// Do not store the returned value. It is memory-managed by [SkiaObjectBox].
/// Storing it may result in use-after-free bugs.
SkImage get skImage => box.skiaObject;

CkImage(SkImage skImage) {
box = SkiaObjectBox<SkImage>(this, skImage);
}
bool _disposed = false;

CkImage.cloneOf(SkiaObjectBox<SkImage> boxToClone) {
box = boxToClone.clone(this);
bool _debugCheckIsNotDisposed() {
assert(!_disposed, 'This image has been disposed.');
return true;
}

bool _disposed = false;
@override
void dispose() {
box.delete();
assert(() {
_disposed = true;
return true;
}());
assert(
!_disposed,
'Cannot dispose an image that has already been disposed.',
);
_disposed = true;
box.unref(this);
}

@override
Expand All @@ -173,10 +159,14 @@ class CkImage implements ui.Image {
}

@override
ui.Image clone() => CkImage.cloneOf(box);
ui.Image clone() {
assert(_debugCheckIsNotDisposed());
return CkImage.cloneOf(box);
}

@override
bool isCloneOf(ui.Image other) {
assert(_debugCheckIsNotDisposed());
return other is CkImage && other.skImage.isAliasOf(skImage);
}

Expand All @@ -185,14 +175,21 @@ class CkImage implements ui.Image {
box.debugGetStackTraces();

@override
int get width => skImage.width();
int get width {
assert(_debugCheckIsNotDisposed());
return skImage.width();
}

@override
int get height => skImage.height();
int get height {
assert(_debugCheckIsNotDisposed());
return skImage.height();
}

@override
Future<ByteData> toByteData(
{ui.ImageByteFormat format = ui.ImageByteFormat.rawRgba}) {
assert(_debugCheckIsNotDisposed());
Uint8List bytes;

if (format == ui.ImageByteFormat.rawRgba) {
Expand All @@ -208,38 +205,17 @@ class CkImage implements ui.Image {
final SkData skData = skImage.encodeToData(); //defaults to PNG 100%
// make a copy that we can return
bytes = Uint8List.fromList(canvasKit.getDataBytes(skData));
skData.delete();
}

final ByteData data = bytes.buffer.asByteData(0, bytes.length);
return Future<ByteData>.value(data);
}

@override
String toString() => '[$width\u00D7$height]';
}

/// A [Codec] that wraps an `SkAnimatedImage`.
class CkAnimatedImageCodec implements ui.Codec {
CkAnimatedImage animatedImage;

CkAnimatedImageCodec(this.animatedImage);

@override
void dispose() {
animatedImage.dispose();
}

@override
int get frameCount => animatedImage.frameCount;

@override
int get repetitionCount => animatedImage.repetitionCount;

@override
Future<ui.FrameInfo> getNextFrame() {
final Duration duration = animatedImage.decodeNextFrame();
final CkImage image = animatedImage.currentFrameAsImage;
return Future<ui.FrameInfo>.value(AnimatedImageFrameInfo(duration, image));
String toString() {
assert(_debugCheckIsNotDisposed());
return '[$width\u00D7$height]';
}
}

Expand Down
Loading

0 comments on commit 35a0b9f

Please sign in to comment.