fix: Dispose TextBoxComponent image cache properly (#1579)

flame-engine · Apr 27, 2022 · c0e3257 · c0e3257
1 parent 2cdf386
commit c0e3257
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 13 deletions.
diff --git a/packages/flame/lib/src/components/text_box_component.dart b/packages/flame/lib/src/components/text_box_component.dart
@@ -55,9 +55,11 @@ class TextBoxComponent<T extends TextRenderer> extends TextComponent {
   late int _totalLines;
 
   double _lifeTime = 0.0;
-  Image? _cache;
   int? _previousChar;
 
+  @visibleForTesting
+  Image? cache;
+
   TextBoxConfig get boxConfig => _boxConfig;
 
   TextBoxComponent({
@@ -100,6 +102,14 @@ class TextBoxComponent<T extends TextRenderer> extends TextComponent {
     await redraw();
   }
 
+  @override
+  @mustCallSuper
+  void onMount() {
+    if (cache == null) {
+      redraw();
+    }
+  }
+
   @override
   @internal
   void updateBounds() {
@@ -190,12 +200,12 @@ class TextBoxComponent<T extends TextRenderer> extends TextComponent {
 
   @override
   void render(Canvas c) {
-    if (_cache == null) {
+    if (cache == null) {
       return;
     }
     c.save();
     c.scale(1 / pixelRatio);
-    c.drawImage(_cache!, Offset.zero, _imagePaint);
+    c.drawImage(cache!, Offset.zero, _imagePaint);
     c.restore();
   }
 
@@ -234,7 +244,8 @@ class TextBoxComponent<T extends TextRenderer> extends TextComponent {
 
   Future<void> redraw() async {
     final newSize = _recomputeSize();
-    _cache = await _fullRenderAsImage(newSize);
+    cache?.dispose();
+    cache = await _fullRenderAsImage(newSize);
     size = newSize;
   }
 
@@ -246,4 +257,12 @@ class TextBoxComponent<T extends TextRenderer> extends TextComponent {
     }
     _previousChar = currentChar;
   }
+
+  @override
+  @mustCallSuper
+  void onRemove() {
+    super.onRemove();
+    cache?.dispose();
+    cache = null;
+  }
 }
diff --git a/packages/flame/lib/src/sprite.dart b/packages/flame/lib/src/sprite.dart
@@ -111,7 +111,8 @@ class Sprite {
   /// Return a new Image based on the [src] of the Sprite.
   ///
   /// **Note:** This is a heavy async operation and should not be called inside
-  /// the game loop.
+  /// the game loop. Remember to call dispose on the [Image] object once you
+  /// aren't going to use it anymore.
   Future<Image> toImage() async {
     final composition = ImageComposition()
       ..add(image, Vector2.zero(), source: src);

diff --git a/packages/flame/lib/src/widgets/nine_tile_box.dart b/packages/flame/lib/src/widgets/nine_tile_box.dart
@@ -1,7 +1,6 @@
-import 'dart:ui' as ui;
+import 'dart:ui';
 
-import 'package:flutter/material.dart';
-import 'package:flutter/widgets.dart';
+import 'package:flutter/material.dart' hide Image;
 
 import '../../assets.dart';
 import '../../flame.dart';
@@ -13,7 +12,7 @@ export '../nine_tile_box.dart';
 export '../sprite.dart';
 
 class _Painter extends CustomPainter {
-  final ui.Image image;
+  final Image image;
   final double tileSize;
   final double destTileSize;
   late final non_widget.NineTileBox _nineTileBox;
@@ -42,7 +41,7 @@ typedef NineTileBox = NineTileBoxWidget;
 
 /// A [StatelessWidget] that renders NineTileBox
 class NineTileBoxWidget extends StatelessWidget {
-  final Future<ui.Image> Function() _imageFuture;
+  final Future<Image> Function() _imageFuture;
 
   /// The size of the tile on the image
   final double tileSize;
@@ -61,7 +60,7 @@ class NineTileBoxWidget extends StatelessWidget {
   final WidgetBuilder? loadingBuilder;
 
   NineTileBoxWidget({
-    required ui.Image image,
+    required Image image,
     required this.tileSize,
     required this.destTileSize,
     this.width,
@@ -89,7 +88,7 @@ class NineTileBoxWidget extends StatelessWidget {
 
   @override
   Widget build(BuildContext context) {
-    return BaseFutureBuilder<ui.Image>(
+    return BaseFutureBuilder<Image>(
       futureBuilder: _imageFuture,
       builder: (_, image) {
         return _NineTileBox(
@@ -108,7 +107,7 @@ class NineTileBoxWidget extends StatelessWidget {
 }
 
 class _NineTileBox extends StatelessWidget {
-  final ui.Image image;
+  final Image image;
   final double tileSize;
   final double destTileSize;
   final double? width;

diff --git a/packages/flame/test/components/text_box_component_test.dart b/packages/flame/test/components/text_box_component_test.dart
@@ -37,5 +37,40 @@ void main() {
           ),
       );
     });
+
+    testWithFlameGame(
+      'internal image is disposed when component is removed',
+      (game) async {
+        final c = TextBoxComponent(text: 'foo bar');
+
+        await game.ensureAdd(c);
+        final imageCache = c.cache;
+
+        final canvas = MockCanvas();
+        game.render(canvas);
+        game.remove(c);
+        game.update(0);
+        expect(imageCache, isNotNull);
+        expect(imageCache!.debugDisposed, isTrue);
+        expect(c.cache, null);
+      },
+    );
+
+    testWithFlameGame(
+      'internal image is redrawn when component is re-added',
+      (game) async {
+        final c = TextBoxComponent(text: 'foo bar');
+
+        await game.ensureAdd(c);
+        game.remove(c);
+        game.update(0);
+        await game.ensureAdd(c);
+        expect(c.isMounted, true);
+
+        await null;
+        expect(c.cache, isNotNull);
+        expect(c.cache!.debugDisposed, isFalse);
+      },
+    );
   });
 }