Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "[web] remove obsolete object caches; simplify native object management" #40937

Merged
merged 1 commit into from
Apr 5, 2023
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
2 changes: 0 additions & 2 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -1876,7 +1876,6 @@ ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/layer_scene_builder
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/layer_tree.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/mask_filter.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/n_way_canvas.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/noto_font.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/painting.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/path.dart + ../../../flutter/LICENSE
Expand Down Expand Up @@ -4461,7 +4460,6 @@ FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/layer_scene_builder.d
FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/layer_tree.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/mask_filter.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/n_way_canvas.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/noto_font.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/painting.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/path.dart
Expand Down
1 change: 0 additions & 1 deletion lib/web_ui/lib/src/engine.dart
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ export 'engine/canvaskit/layer_scene_builder.dart';
export 'engine/canvaskit/layer_tree.dart';
export 'engine/canvaskit/mask_filter.dart';
export 'engine/canvaskit/n_way_canvas.dart';
export 'engine/canvaskit/native_memory.dart';
export 'engine/canvaskit/noto_font.dart';
export 'engine/canvaskit/painting.dart';
export 'engine/canvaskit/path.dart';
Expand Down
2 changes: 2 additions & 0 deletions lib/web_ui/lib/src/engine/canvaskit/canvas.dart
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ class CkCanvas {
offset.dx,
offset.dy,
);
paragraph.markUsed();
}

void drawPath(CkPath path, CkPaint paint) {
Expand Down Expand Up @@ -1111,6 +1112,7 @@ class CkDrawParagraphCommand extends CkPaintCommand {
offset.dx,
offset.dy,
);
paragraph.markUsed();
}
}

Expand Down
13 changes: 5 additions & 8 deletions lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3392,7 +3392,7 @@ abstract class Collector {
class ProductionCollector implements Collector {
ProductionCollector() {
_skObjectFinalizationRegistry =
createSkObjectFinalizationRegistry((SkDeletable deletable) {
SkObjectFinalizationRegistry((SkDeletable deletable) {
// This is called when GC decides to collect the wrapper object and
// notify us, which may happen after the object is already deleted
// explicitly, e.g. when its ref count drops to zero. When that happens
Expand Down Expand Up @@ -3568,13 +3568,10 @@ extension JsConstructorExtension on JsConstructor {
/// 6. We call `delete` on SkPaint.
@JS('window.FinalizationRegistry')
@staticInterop
class SkObjectFinalizationRegistry {}

SkObjectFinalizationRegistry createSkObjectFinalizationRegistry(JSFunction cleanup) {
return js_util.callConstructor(
_finalizationRegistryConstructor!.toObjectShallow,
<Object>[cleanup],
);
class SkObjectFinalizationRegistry {
// TODO(hterkelsen): Add a type for the `cleanup` function when
// native constructors support type parameters.
external factory SkObjectFinalizationRegistry(JSFunction cleanup);
}

extension SkObjectFinalizationRegistryExtension on SkObjectFinalizationRegistry {
Expand Down
56 changes: 50 additions & 6 deletions lib/web_ui/lib/src/engine/canvaskit/image.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ import 'canvas.dart';
import 'canvaskit_api.dart';
import 'image_wasm_codecs.dart';
import 'image_web_codecs.dart';
import 'native_memory.dart';
import 'painting.dart';
import 'picture.dart';
import 'picture_recorder.dart';
import 'skia_object_cache.dart';

/// Instantiates a [ui.Codec] backed by an `SkAnimatedImage` from Skia.
FutureOr<ui.Codec> skiaInstantiateImageCodec(Uint8List list,
Expand Down Expand Up @@ -227,8 +227,52 @@ Future<Uint8List> readChunked(HttpFetchPayload payload, int contentLength, WebOn
/// A [ui.Image] backed by an `SkImage` from Skia.
class CkImage implements ui.Image, StackTraceDebugger {
CkImage(SkImage skImage, { this.videoFrame }) {
box = CountedRef<CkImage, SkImage>(skImage, this, 'SkImage');
_init();
if (browserSupportsFinalizationRegistry) {
box = SkiaObjectBox<CkImage, SkImage>(this, skImage);
} else {
// If finalizers are not supported we need to be able to resurrect the
// image if it was temporarily deleted. To do that, we keep the original
// pixels and ask the SkiaObjectBox to make an image from them when
// resurrecting.
//
// IMPORTANT: the alphaType, colorType, and colorSpace passed to
// _encodeImage and to canvasKit.MakeImage must be the same. Otherwise
// Skia will misinterpret the pixels and corrupt the image.
final ByteData? originalBytes = _encodeImage(
skImage: skImage,
format: ui.ImageByteFormat.rawRgba,
alphaType: canvasKit.AlphaType.Premul,
colorType: canvasKit.ColorType.RGBA_8888,
colorSpace: SkColorSpaceSRGB,
);
if (originalBytes == null) {
printWarning('Unable to encode image to bytes. We will not '
'be able to resurrect it once it has been garbage collected.');
return;
}
final int originalWidth = skImage.width().toInt();
final int originalHeight = skImage.height().toInt();
box = SkiaObjectBox<CkImage, SkImage>.resurrectable(this, skImage, () {
final SkImage? skImage = canvasKit.MakeImage(
SkImageInfo(
alphaType: canvasKit.AlphaType.Premul,
colorType: canvasKit.ColorType.RGBA_8888,
colorSpace: SkColorSpaceSRGB,
width: originalWidth.toDouble(),
height: originalHeight.toDouble(),
),
originalBytes.buffer.asUint8List(),
(4 * originalWidth).toDouble(),
);
if (skImage == null) {
throw ImageCodecException(
'Failed to resurrect image from pixels.'
);
}
return skImage;
});
}
}

CkImage.cloneOf(this.box, {this.videoFrame}) {
Expand All @@ -247,9 +291,9 @@ class CkImage implements ui.Image, StackTraceDebugger {
StackTrace get debugStackTrace => _debugStackTrace;
late StackTrace _debugStackTrace;

// Use ref counting because `SkImage` may be deleted either due to this object
// 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 CountedRef<CkImage, SkImage> box;
late final SkiaObjectBox<CkImage, SkImage> box;

/// For browsers that support `ImageDecoder` this field holds the video frame
/// from which this image was created.
Expand All @@ -261,9 +305,9 @@ class CkImage implements ui.Image, StackTraceDebugger {

/// The underlying Skia image object.
///
/// Do not store the returned value. It is memory-managed by [CountedRef].
/// 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.nativeObject;
SkImage get skImage => box.skiaObject;

bool _disposed = false;

Expand Down
216 changes: 0 additions & 216 deletions lib/web_ui/lib/src/engine/canvaskit/native_memory.dart

This file was deleted.

3 changes: 2 additions & 1 deletion lib/web_ui/lib/src/engine/canvaskit/picture.dart
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ class CkPicture extends ManagedSkiaObject<SkPicture> implements ui.Picture {
/// false.
///
/// This extra flag is necessary on top of [rawSkiaObject] because
/// [rawSkiaObject] being null does not indicate permanent deletion.
/// [rawSkiaObject] being null does not indicate permanent deletion. See
/// similar flag [SkiaObjectBox.isDeletedPermanently].
bool _isDisposed = false;

/// The stack trace taken when [dispose] was called.
Expand Down
Loading