stack-scope all SkImageFilter instances

Author: yjbanov

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

@@ -333,7 +333,7 @@ class CkCanvas {
     } else {
       convertible = filter as CkManagedSkImageFilterConvertible;
     }
-    convertible.imageFilter((SkImageFilter filter) {
+    convertible.withSkImageFilter((SkImageFilter filter) {
       final skPaint = paint?.toSkPaint();
       skCanvas.saveLayer(
         skPaint,

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

@@ -1516,6 +1516,9 @@ class SkImageFilter {}
 extension SkImageFilterExtension on SkImageFilter {
   external JSVoid delete();
 
+  @JS('isDeleted')
+  external JSBoolean _isDeleted();
+  bool isDeleted() => _isDeleted().toDart;
 
   @JS('getOutputBounds')
   external JSInt32Array _getOutputBounds(JSFloat32Array bounds);

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

@@ -71,7 +71,7 @@ abstract class CkColorFilter implements CkManagedSkImageFilterConvertible {
   SkColorFilter _initRawColorFilter();
 
   @override
-  void imageFilter(SkImageFilterBorrow borrow) {
+  void withSkImageFilter(SkImageFilterBorrow borrow) {
     // Since ColorFilter has a const constructor it cannot store dynamically
     // created Skia objects. Therefore a new SkImageFilter is created every time
     // it's used. However, once used it's no longer needed, so it's deleted

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

@@ -10,7 +10,6 @@ import 'package:ui/ui.dart' as ui;
 import '../util.dart';
 import 'canvaskit_api.dart';
 import 'color_filter.dart';
-import 'native_memory.dart';
 
 typedef SkImageFilterBorrow = void Function(SkImageFilter);
 
@@ -22,7 +21,12 @@ typedef SkImageFilterBorrow = void Function(SkImageFilter);
 ///
 /// Currently implemented by [CkImageFilter] and [CkColorFilter].
 abstract class CkManagedSkImageFilterConvertible implements ui.ImageFilter {
-  void imageFilter(SkImageFilterBorrow borrow);
+  /// Creates a temporary [SkImageFilter], passes it to [borrow], and then
+  /// immediately deletes it.
+  ///
+  /// [SkImageFilter] objects are not kept around so that their memory is
+  /// reclaimed immediately, rather than waiting for the GC cycle.
+  void withSkImageFilter(SkImageFilterBorrow borrow);
 
   Matrix4 get transform;
 }
@@ -56,22 +60,15 @@ abstract class CkImageFilter implements CkManagedSkImageFilterConvertible {
 }
 
 class CkColorFilterImageFilter extends CkImageFilter {
-  CkColorFilterImageFilter({required this.colorFilter}) : super._() {
-    final SkImageFilter skImageFilter = colorFilter.initRawImageFilter();
-    _ref = UniqueRef<SkImageFilter>(this, skImageFilter, 'ImageFilter.color');
-  }
+  CkColorFilterImageFilter({required this.colorFilter}) : super._();
 
   final CkColorFilter colorFilter;
 
-  late final UniqueRef<SkImageFilter> _ref;
-
   @override
-  void imageFilter(SkImageFilterBorrow borrow) {
-    borrow(_ref.nativeObject);
-  }
-
-  void dispose() {
-    _ref.dispose();
+  void withSkImageFilter(SkImageFilterBorrow borrow) {
+    final skImageFilter = colorFilter.initRawImageFilter();
+    borrow(skImageFilter);
+    skImageFilter.delete();
   }
 
   @override
@@ -93,7 +90,14 @@ class CkColorFilterImageFilter extends CkImageFilter {
 class _CkBlurImageFilter extends CkImageFilter {
   _CkBlurImageFilter(
       {required this.sigmaX, required this.sigmaY, required this.tileMode})
-      : super._() {
+      : super._();
+
+  final double sigmaX;
+  final double sigmaY;
+  final ui.TileMode tileMode;
+
+  @override
+  void withSkImageFilter(SkImageFilterBorrow borrow) {
     /// Return the identity matrix when both sigmaX and sigmaY are 0. Replicates
     /// effect of applying no filter
     final SkImageFilter skImageFilter;
@@ -110,18 +114,9 @@ class _CkBlurImageFilter extends CkImageFilter {
         null,
       );
     }
-    _ref = UniqueRef<SkImageFilter>(this, skImageFilter, 'ImageFilter.blur');
-  }
-
-  final double sigmaX;
-  final double sigmaY;
-  final ui.TileMode tileMode;
 
-  late final UniqueRef<SkImageFilter> _ref;
-
-  @override
-  void imageFilter(SkImageFilterBorrow borrow) {
-    borrow(_ref.nativeObject);
+    borrow(skImageFilter);
+    skImageFilter.delete();
   }
 
   @override
@@ -149,25 +144,22 @@ class _CkMatrixImageFilter extends CkImageFilter {
       {required Float64List matrix, required this.filterQuality})
       : matrix = Float64List.fromList(matrix),
         _transform = Matrix4.fromFloat32List(toMatrix32(matrix)),
-        super._() {
-    final SkImageFilter skImageFilter =
-        canvasKit.ImageFilter.MakeMatrixTransform(
-      toSkMatrixFromFloat64(matrix),
-      toSkFilterOptions(filterQuality),
-      null,
-    );
-    _ref = UniqueRef<SkImageFilter>(this, skImageFilter, 'ImageFilter.matrix');
-  }
+        super._();
 
   final Float64List matrix;
   final ui.FilterQuality filterQuality;
   final Matrix4 _transform;
 
-  late final UniqueRef<SkImageFilter> _ref;
-
   @override
-  void imageFilter(SkImageFilterBorrow borrow) {
-    borrow(_ref.nativeObject);
+  void withSkImageFilter(SkImageFilterBorrow borrow) {
+    final skImageFilter =
+        canvasKit.ImageFilter.MakeMatrixTransform(
+      toSkMatrixFromFloat64(matrix),
+      toSkFilterOptions(filterQuality),
+      null,
+    );
+    borrow(skImageFilter);
+    skImageFilter.delete();
   }
 
   @override
@@ -192,23 +184,20 @@ class _CkMatrixImageFilter extends CkImageFilter {
 
 class _CkDilateImageFilter extends CkImageFilter {
   _CkDilateImageFilter({required this.radiusX, required this.radiusY})
-      : super._() {
-    final SkImageFilter skImageFilter = canvasKit.ImageFilter.MakeDilate(
-      radiusX,
-      radiusY,
-      null,
-    );
-    _ref = UniqueRef<SkImageFilter>(this, skImageFilter, 'ImageFilter.dilate');
-  }
+      : super._();
 
   final double radiusX;
   final double radiusY;
 
-  late final UniqueRef<SkImageFilter> _ref;
-
   @override
-  void imageFilter(SkImageFilterBorrow borrow) {
-    borrow(_ref.nativeObject);
+  void withSkImageFilter(SkImageFilterBorrow borrow) {
+    final skImageFilter = canvasKit.ImageFilter.MakeDilate(
+      radiusX,
+      radiusY,
+      null,
+    );
+    borrow(skImageFilter);
+    skImageFilter.delete();
   }
 
   @override
@@ -232,23 +221,20 @@ class _CkDilateImageFilter extends CkImageFilter {
 
 class _CkErodeImageFilter extends CkImageFilter {
   _CkErodeImageFilter({required this.radiusX, required this.radiusY})
-      : super._() {
-    final SkImageFilter skImageFilter = canvasKit.ImageFilter.MakeErode(
-      radiusX,
-      radiusY,
-      null,
-    );
-    _ref = UniqueRef<SkImageFilter>(this, skImageFilter, 'ImageFilter.erode');
-  }
+      : super._();
 
   final double radiusX;
   final double radiusY;
 
-  late final UniqueRef<SkImageFilter> _ref;
-
   @override
-  void imageFilter(SkImageFilterBorrow borrow) {
-    borrow(_ref.nativeObject);
+  void withSkImageFilter(SkImageFilterBorrow borrow) {
+    final skImageFilter = canvasKit.ImageFilter.MakeErode(
+      radiusX,
+      radiusY,
+      null,
+    );
+    borrow(skImageFilter);
+    skImageFilter.delete();
   }
 
   @override
@@ -272,27 +258,23 @@ class _CkErodeImageFilter extends CkImageFilter {
 
 class _CkComposeImageFilter extends CkImageFilter {
   _CkComposeImageFilter({required this.outer, required this.inner})
-      : super._() {
-    outer.imageFilter((SkImageFilter outerFilter) {
-      inner.imageFilter((SkImageFilter innerFilter) {
-        final SkImageFilter skImageFilter = canvasKit.ImageFilter.MakeCompose(
-          outerFilter,
-          innerFilter,
-        );
-        _ref = UniqueRef<SkImageFilter>(
-            this, skImageFilter, 'ImageFilter.compose');
-      });
-    });
-  }
+      : super._();
 
   final CkImageFilter outer;
   final CkImageFilter inner;
 
-  late final UniqueRef<SkImageFilter> _ref;
-
   @override
-  void imageFilter(SkImageFilterBorrow borrow) {
-    borrow(_ref.nativeObject);
+  void withSkImageFilter(SkImageFilterBorrow borrow) {
+    outer.withSkImageFilter((skOuter) {
+      inner.withSkImageFilter((skInner) {
+        final skImageFilter = canvasKit.ImageFilter.MakeCompose(
+          skOuter,
+          skInner,
+        );
+        borrow(skImageFilter);
+        skImageFilter.delete();
+      });
+    });
   }
 
   @override

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

@@ -417,16 +417,17 @@ class ImageFilterEngineLayer extends ContainerLayer
     }
     final ui.Rect childPaintBounds =
         prerollChildren(prerollContext, childMatrix);
-    convertible.imageFilter((SkImageFilter filter) {
-      // If the filter is a ColorFilter, the extended paint bounds will be the
-      // entire screen, which is not what we want.
       if (_filter is ui.ColorFilter) {
+        // If the filter is a ColorFilter, the extended paint bounds will be the
+        // entire screen, which is not what we want.
         paintBounds = childPaintBounds;
       } else {
-        paintBounds =
-            rectFromSkIRect(filter.getOutputBounds(toSkRect(childPaintBounds)));
+        convertible.withSkImageFilter((skFilter) {
+          paintBounds = rectFromSkIRect(
+            skFilter.getOutputBounds(toSkRect(childPaintBounds)),
+          );
+        });
       }
-    });
     prerollContext.mutatorsStack.pop();
   }
 

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

@@ -21,6 +21,7 @@ import 'shader.dart';
 ///
 /// This class is backed by a Skia object that must be explicitly
 /// deleted to avoid a memory leak. This is done by extending [SkiaObject].
+// TODO(154281): try to unify with SkwasmPaint
 class CkPaint implements ui.Paint {
   CkPaint();
 
@@ -63,7 +64,7 @@ class CkPaint implements ui.Paint {
 
     final localImageFilter = _imageFilter;
     if (localImageFilter != null) {
-      localImageFilter.imageFilter((SkImageFilter skImageFilter) {
+      localImageFilter.withSkImageFilter((skImageFilter) {
         skPaint.setImageFilter(skImageFilter);
       });
     }

lib/web_ui/test/canvaskit/filter_test.dart

@@ -54,16 +54,25 @@ void testMain() {
   setUpCanvasKitTest(withImplicitView: true);
 
   group('ImageFilters', () {
-    test('can be constructed', () {
-      final CkImageFilter imageFilter = CkImageFilter.blur(sigmaX: 5, sigmaY: 10, tileMode: ui.TileMode.clamp);
-      expect(imageFilter, isA<CkImageFilter>());
-      SkImageFilter? skFilter;
-      imageFilter.imageFilter((SkImageFilter value) {
-        skFilter = value;
-      });
-      expect(skFilter, isNotNull);
-    });
-
+    {
+      final testFilters = createImageFilters();
+      for (final imageFilter in testFilters) {
+        test('${imageFilter.runtimeType}.withSkImageFilter creates temp SkImageFilter', () {
+          expect(imageFilter, isA<CkImageFilter>());
+          SkImageFilter? skFilter;
+          imageFilter.withSkImageFilter((value) {
+            expect(value.isDeleted(), isFalse);
+            skFilter = value;
+          });
+          expect(skFilter, isNotNull);
+          expect(
+            reason: 'Because the SkImageFilter instance is temporary',
+            skFilter!.isDeleted(),
+            isTrue,
+          );
+        });
+      }
+    }
 
     test('== operator', () {
       final List<ui.ImageFilter> filters1 = <ui.ImageFilter>[