Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit d62e1ad

Browse files
author
Casey Hillers
authored
Revert "[web] remove obsolete object caches; simplify native object management" (#40861)
Reverts #40617 See b/276167870. This is causing a build breakage to Google testing for all web projects.
1 parent 1645578 commit d62e1ad

File tree

12 files changed

+767
-539
lines changed

12 files changed

+767
-539
lines changed

ci/licenses_golden/licenses_flutter

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1874,7 +1874,6 @@ ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/layer_scene_builder
18741874
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/layer_tree.dart + ../../../flutter/LICENSE
18751875
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/mask_filter.dart + ../../../flutter/LICENSE
18761876
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/n_way_canvas.dart + ../../../flutter/LICENSE
1877-
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart + ../../../flutter/LICENSE
18781877
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/noto_font.dart + ../../../flutter/LICENSE
18791878
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/painting.dart + ../../../flutter/LICENSE
18801879
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/path.dart + ../../../flutter/LICENSE
@@ -4455,7 +4454,6 @@ FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/layer_scene_builder.d
44554454
FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/layer_tree.dart
44564455
FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/mask_filter.dart
44574456
FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/n_way_canvas.dart
4458-
FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart
44594457
FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/noto_font.dart
44604458
FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/painting.dart
44614459
FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/path.dart

lib/web_ui/lib/src/engine.dart

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ export 'engine/canvaskit/layer_scene_builder.dart';
3939
export 'engine/canvaskit/layer_tree.dart';
4040
export 'engine/canvaskit/mask_filter.dart';
4141
export 'engine/canvaskit/n_way_canvas.dart';
42-
export 'engine/canvaskit/native_memory.dart';
4342
export 'engine/canvaskit/noto_font.dart';
4443
export 'engine/canvaskit/painting.dart';
4544
export 'engine/canvaskit/path.dart';

lib/web_ui/lib/src/engine/canvaskit/canvas.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ class CkCanvas {
215215
offset.dx,
216216
offset.dy,
217217
);
218+
paragraph.markUsed();
218219
}
219220

220221
void drawPath(CkPath path, CkPaint paint) {
@@ -1111,6 +1112,7 @@ class CkDrawParagraphCommand extends CkPaintCommand {
11111112
offset.dx,
11121113
offset.dy,
11131114
);
1115+
paragraph.markUsed();
11141116
}
11151117
}
11161118

lib/web_ui/lib/src/engine/canvaskit/image.dart

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@ import 'canvas.dart';
1515
import 'canvaskit_api.dart';
1616
import 'image_wasm_codecs.dart';
1717
import 'image_web_codecs.dart';
18-
import 'native_memory.dart';
1918
import 'painting.dart';
2019
import 'picture.dart';
2120
import 'picture_recorder.dart';
21+
import 'skia_object_cache.dart';
2222

2323
/// Instantiates a [ui.Codec] backed by an `SkAnimatedImage` from Skia.
2424
FutureOr<ui.Codec> skiaInstantiateImageCodec(Uint8List list,
@@ -227,8 +227,52 @@ Future<Uint8List> readChunked(HttpFetchPayload payload, int contentLength, WebOn
227227
/// A [ui.Image] backed by an `SkImage` from Skia.
228228
class CkImage implements ui.Image, StackTraceDebugger {
229229
CkImage(SkImage skImage, { this.videoFrame }) {
230-
box = CountedRef<CkImage, SkImage>(skImage, this, 'SkImage');
231230
_init();
231+
if (browserSupportsFinalizationRegistry) {
232+
box = SkiaObjectBox<CkImage, SkImage>(this, skImage);
233+
} else {
234+
// If finalizers are not supported we need to be able to resurrect the
235+
// image if it was temporarily deleted. To do that, we keep the original
236+
// pixels and ask the SkiaObjectBox to make an image from them when
237+
// resurrecting.
238+
//
239+
// IMPORTANT: the alphaType, colorType, and colorSpace passed to
240+
// _encodeImage and to canvasKit.MakeImage must be the same. Otherwise
241+
// Skia will misinterpret the pixels and corrupt the image.
242+
final ByteData? originalBytes = _encodeImage(
243+
skImage: skImage,
244+
format: ui.ImageByteFormat.rawRgba,
245+
alphaType: canvasKit.AlphaType.Premul,
246+
colorType: canvasKit.ColorType.RGBA_8888,
247+
colorSpace: SkColorSpaceSRGB,
248+
);
249+
if (originalBytes == null) {
250+
printWarning('Unable to encode image to bytes. We will not '
251+
'be able to resurrect it once it has been garbage collected.');
252+
return;
253+
}
254+
final int originalWidth = skImage.width().toInt();
255+
final int originalHeight = skImage.height().toInt();
256+
box = SkiaObjectBox<CkImage, SkImage>.resurrectable(this, skImage, () {
257+
final SkImage? skImage = canvasKit.MakeImage(
258+
SkImageInfo(
259+
alphaType: canvasKit.AlphaType.Premul,
260+
colorType: canvasKit.ColorType.RGBA_8888,
261+
colorSpace: SkColorSpaceSRGB,
262+
width: originalWidth.toDouble(),
263+
height: originalHeight.toDouble(),
264+
),
265+
originalBytes.buffer.asUint8List(),
266+
(4 * originalWidth).toDouble(),
267+
);
268+
if (skImage == null) {
269+
throw ImageCodecException(
270+
'Failed to resurrect image from pixels.'
271+
);
272+
}
273+
return skImage;
274+
});
275+
}
232276
}
233277

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

250-
// Use ref counting because `SkImage` may be deleted either due to this object
294+
// Use a box because `SkImage` may be deleted either due to this object
251295
// being garbage-collected, or by an explicit call to [delete].
252-
late final CountedRef<CkImage, SkImage> box;
296+
late final SkiaObjectBox<CkImage, SkImage> box;
253297

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

262306
/// The underlying Skia image object.
263307
///
264-
/// Do not store the returned value. It is memory-managed by [CountedRef].
308+
/// Do not store the returned value. It is memory-managed by [SkiaObjectBox].
265309
/// Storing it may result in use-after-free bugs.
266-
SkImage get skImage => box.nativeObject;
310+
SkImage get skImage => box.skiaObject;
267311

268312
bool _disposed = false;
269313

lib/web_ui/lib/src/engine/canvaskit/native_memory.dart

Lines changed: 0 additions & 214 deletions
This file was deleted.

lib/web_ui/lib/src/engine/canvaskit/picture.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ class CkPicture extends ManagedSkiaObject<SkPicture> implements ui.Picture {
4646
/// false.
4747
///
4848
/// This extra flag is necessary on top of [rawSkiaObject] because
49-
/// [rawSkiaObject] being null does not indicate permanent deletion.
49+
/// [rawSkiaObject] being null does not indicate permanent deletion. See
50+
/// similar flag [SkiaObjectBox.isDeletedPermanently].
5051
bool _isDisposed = false;
5152

5253
/// The stack trace taken when [dispose] was called.

0 commit comments

Comments
 (0)