feat: Use a Free List Strategy on BatchItem indexes within SpriteBatch and return index from .add()

Author: gnarhard

packages/flame/lib/src/sprite_batch.dart

@@ -13,8 +13,8 @@ extension SpriteBatchExtension on Game {
   /// its options.
   Future<SpriteBatch> loadSpriteBatch(
     String path, {
-    Color? defaultColor,
-    BlendMode? defaultBlendMode,
+    Color defaultColor = const Color(0x00000000),
+    BlendMode defaultBlendMode = BlendMode.srcOver,
     RSTransform? defaultTransform,
     Images? imageCache,
     bool useAtlas = true,
@@ -122,10 +122,10 @@ enum FlippedAtlasStatus {
 class SpriteBatch {
   SpriteBatch(
     this.atlas, {
+    this.defaultColor = const Color(0x00000000),
+    this.defaultBlendMode = BlendMode.srcOver,
     this.defaultTransform,
     this.useAtlas = true,
-    this.defaultColor,
-    this.defaultBlendMode,
     Images? imageCache,
     String? imageKey,
   }) : _imageCache = imageCache,
@@ -136,10 +136,10 @@ class SpriteBatch {
   /// When the [images] is omitted, the global [Flame.images] is used.
   static Future<SpriteBatch> load(
     String path, {
+    Color defaultColor = const Color(0x00000000),
+    BlendMode defaultBlendMode = BlendMode.srcOver,
     RSTransform? defaultTransform,
     Images? images,
-    Color? defaultColor,
-    BlendMode? defaultBlendMode,
     bool useAtlas = true,
   }) async {
     final imagesCache = images ?? Flame.images;
@@ -156,37 +156,38 @@ class SpriteBatch {
 
   FlippedAtlasStatus _flippedAtlasStatus = FlippedAtlasStatus.none;
 
-  /// List of all the existing batch items.
-  final _batchItems = <BatchItem>[];
+  /// Stack of available (freed) indices using ListQueue as a stack.
+  final Queue<int> _freeIndices = Queue<int>();
 
-  /// The sources to use on the [atlas].
-  final _sources = <Rect>[];
+  /// Returns the total number of indices that have been allocated.
+  int get allocatedCount => _nextIndex;
 
-  /// The sources list shouldn't be modified directly, that is why an
-  /// [UnmodifiableListView] is used. If you want to add sources use the
-  /// [add] or [addTransform] method.
-  UnmodifiableListView<Rect> get sources {
-    return UnmodifiableListView<Rect>(_sources);
-  }
+  /// Returns the number of currently free indices.
+  int get freeCount => _freeIndices.length;
 
-  /// The transforms that should be applied on the [_sources].
-  final _transforms = <RSTransform>[];
+  /// The next index to allocate if no free indices are available.
+  int _nextIndex = 0;
 
-  /// The transforms list shouldn't be modified directly, that is why an
-  /// [UnmodifiableListView] is used. If you want to add transforms use the
-  /// [add] or [addTransform] method.
-  UnmodifiableListView<RSTransform> get transforms {
-    return UnmodifiableListView<RSTransform>(_transforms);
-  }
+  /// Sparse array of batch items, indexed by allocated indices.
+  final Map<int, BatchItem> _batchItems = {};
+
+  /// Returns the number of active batch items.
+  int get length => _batchItems.length;
 
-  /// The background color for the [_sources].
-  final _colors = <Color>[];
+  /// Returns the number of indices currently in use.
+  int get usedCount => _nextIndex - _freeIndices.length;
+
+  /// Allocates a new index, reusing freed indices when possible.
+  int _allocateIndex() {
+    if (_freeIndices.isNotEmpty) {
+      return _freeIndices.removeFirst();
+    }
+    return _nextIndex++;
+  }
 
-  /// The colors list shouldn't be modified directly, that is why an
-  /// [UnmodifiableListView] is used. If you want to add colors use the
-  /// [add] or [addTransform] method.
-  UnmodifiableListView<Color> get colors {
-    return UnmodifiableListView<Color>(_colors);
+  /// Frees an index to be reused later.
+  void _freeIndex(int index) {
+    _freeIndices.addFirst(index);
   }
 
   /// The atlas used by the [SpriteBatch].
@@ -234,6 +235,12 @@ class SpriteBatch {
   /// Does this batch contain any operations?
   bool get isEmpty => _batchItems.isEmpty;
 
+  // Used to not create new Paint objects in [render] and
+  // [generateFlippedAtlas].
+  final _emptyPaint = Paint();
+
+  static const _defaultColor = Color(0x00000000);
+
   Future<void> _makeFlippedAtlas() async {
     _flippedAtlasStatus = FlippedAtlasStatus.generating;
     final key = '$imageKey#with-flips';
@@ -255,12 +262,10 @@ class SpriteBatch {
     return picture.toImageSafe(image.width * 2, image.height);
   }
 
-  int get length => _sources.length;
-
   /// Replace provided values of a batch item at the [index], when a parameter
   /// is not provided, the original value of the batch item will be used.
   ///
-  /// Throws an [ArgumentError] if the [index] is out of bounds.
+  /// Throws an [ArgumentError] if the [index] doesn't exist.
   /// At least one of the parameters must be different from null.
   void replace(
     int index, {
@@ -273,11 +278,11 @@ class SpriteBatch {
       'At least one of the parameters must be different from null.',
     );
 
-    if (index < 0 || index >= length) {
-      throw ArgumentError('Index out of bounds: $index');
+    if (!_batchItems.containsKey(index)) {
+      throw ArgumentError('Index does not exist: $index');
     }
 
-    final currentBatchItem = _batchItems[index];
+    final currentBatchItem = _batchItems[index]!;
     final newBatchItem = BatchItem(
       source: source ?? currentBatchItem.source,
       transform: transform ?? currentBatchItem.transform,
@@ -286,10 +291,14 @@ class SpriteBatch {
     );
 
     _batchItems[index] = newBatchItem;
+  }
 
-    _sources[index] = newBatchItem.source;
-    _transforms[index] = newBatchItem.transform;
-    _colors[index] = color ?? _defaultColor;
+  /// Returns the [BatchItem] at the given [index].
+  BatchItem getBatchItem(int index) {
+    if (!_batchItems.containsKey(index)) {
+      throw ArgumentError('Index does not exist: $index');
+    }
+    return _batchItems[index]!;
   }
 
   /// Add a new batch item using a RSTransform.
@@ -307,26 +316,15 @@ class SpriteBatch {
   /// cosine of the rotation so that they can be reused over multiple calls to
   /// this constructor, it may be more efficient to directly use this method
   /// instead.
-  void addTransform({
+  int addTransform({
     required Rect source,
     RSTransform? transform,
     bool flip = false,
     Color? color,
   }) {
+    final index = _allocateIndex();
     final batchItem = BatchItem(
-      source: source,
-      transform: transform ??= defaultTransform ?? RSTransform(1, 0, 0, 0),
-      flip: flip,
-      color: color ?? defaultColor,
-    );
-
-    if (flip && useAtlas && _flippedAtlasStatus.isNone) {
-      _makeFlippedAtlas();
-    }
-
-    _batchItems.add(batchItem);
-    _sources.add(
-      flip
+      source: flip
           ? Rect.fromLTWH(
               // The atlas is twice as wide when the flipped atlas is generated.
               (atlas.width * (_flippedAtlasStatus.isGenerated ? 1 : 2)) -
@@ -335,10 +333,19 @@ class SpriteBatch {
               source.width,
               source.height,
             )
-          : batchItem.source,
+          : source,
+      transform: transform ??= defaultTransform ?? RSTransform(1, 0, 0, 0),
+      flip: flip,
+      color: color ?? defaultColor,
     );
-    _transforms.add(batchItem.transform);
-    _colors.add(color ?? _defaultColor);
+
+    if (flip && useAtlas && _flippedAtlasStatus.isNone) {
+      _makeFlippedAtlas();
+    }
+
+    _batchItems[index] = batchItem;
+
+    return index;
   }
 
   /// Add a new batch item.
@@ -359,7 +366,7 @@ class SpriteBatch {
   /// multiple [RSTransform] objects,
   /// it may be more efficient to directly use the more direct [addTransform]
   /// method instead.
-  void add({
+  int add({
     required Rect source,
     double scale = 1.0,
     Vector2? anchor,
@@ -389,26 +396,31 @@ class SpriteBatch {
       );
     }
 
-    addTransform(
+    return addTransform(
       source: source,
       transform: transform,
       flip: flip,
       color: color,
     );
   }
 
+  /// Removes a batch item at the given [index].
+  void removeAt(int index) {
+    if (!_batchItems.containsKey(index)) {
+      throw ArgumentError('Index does not exist: $index');
+    }
+
+    _batchItems.remove(index);
+    _freeIndex(index);
+  }
+
   /// Clear the SpriteBatch so it can be reused.
   void clear() {
-    _sources.clear();
-    _transforms.clear();
-    _colors.clear();
     _batchItems.clear();
+    _freeIndices.clear();
+    _nextIndex = 0;
   }
 
-  // Used to not create new Paint objects in [render] and
-  // [generateFlippedAtlas].
-  final _emptyPaint = Paint();
-
   void render(
     Canvas canvas, {
     BlendMode? blendMode,
@@ -419,27 +431,38 @@ class SpriteBatch {
       return;
     }
 
-    final renderPaint = paint ?? _emptyPaint;
-
-    final hasNoColors = _colors.every((c) => c == _defaultColor);
-    final actualBlendMode = blendMode ?? defaultBlendMode;
-    if (!hasNoColors && actualBlendMode == null) {
-      throw 'When setting any colors, a blend mode must be provided.';
-    }
+    paint ??= _emptyPaint;
 
     if (useAtlas && !_flippedAtlasStatus.isGenerating) {
+      final transforms = _batchItems.values
+          .map((e) => e.transform)
+          .toList(growable: false);
+      final sources = _batchItems.values
+          .map((e) => e.source)
+          .toList(growable: false);
+      final colors = _batchItems.values
+          .map((e) => e.paint.color)
+          .toList(growable: false);
+
+      final hasNoColors = colors.every((c) => c == _defaultColor);
+      final actualBlendMode = blendMode ?? defaultBlendMode;
+      if (!hasNoColors && actualBlendMode == null) {
+        throw 'When setting any colors, a blend mode must be provided.';
+      }
+
       canvas.drawAtlas(
         atlas,
-        _transforms,
-        _sources,
-        hasNoColors ? null : _colors,
+        transforms,
+        sources,
+        hasNoColors ? null : colors,
         actualBlendMode,
         cullRect,
-        renderPaint,
+        paint,
       );
     } else {
-      for (final batchItem in _batchItems) {
-        renderPaint.blendMode = blendMode ?? renderPaint.blendMode;
+      for (final index in _batchItems.keys) {
+        final batchItem = _batchItems[index]!;
+        paint.blendMode = blendMode ?? paint.blendMode;
 
         canvas
           ..save()
@@ -449,12 +472,10 @@ class SpriteBatch {
             atlas,
             batchItem.source,
             batchItem.destination,
-            renderPaint,
+            paint,
           )
           ..restore();
       }
     }
   }
-
-  static const _defaultColor = Color(0x00000000);
 }

packages/flame/test/sprite_batch_test.dart

@@ -16,9 +16,9 @@ void main() {
     test('can add to the batch', () {
       final image = _MockImage();
       final spriteBatch = SpriteBatch(image);
-      spriteBatch.add(source: Rect.zero);
+      final index = spriteBatch.add(source: Rect.zero);
 
-      expect(spriteBatch.transforms, hasLength(1));
+      expect(spriteBatch.getBatchItem(index), isNotNull);
     });
 
     test('can replace the color of a batch', () {
@@ -28,8 +28,13 @@ void main() {
 
       spriteBatch.replace(0, color: Colors.red);
 
-      expect(spriteBatch.colors, hasLength(1));
-      expect(spriteBatch.colors.first, Colors.red);
+      final batchItem = spriteBatch.getBatchItem(0);
+
+      /// Use .closeTo() to avoid floating point rounding errors.
+      expect(batchItem.paint.color.a, closeTo(Colors.red.a, 0.001));
+      expect(batchItem.paint.color.r, closeTo(Colors.red.r, 0.001));
+      expect(batchItem.paint.color.g, closeTo(Colors.red.g, 0.001));
+      expect(batchItem.paint.color.b, closeTo(Colors.red.b, 0.001));
     });
 
     test('can replace the source of a batch', () {
@@ -38,9 +43,9 @@ void main() {
       spriteBatch.add(source: Rect.zero);
 
       spriteBatch.replace(0, source: const Rect.fromLTWH(1, 1, 1, 1));
+      final batchItem = spriteBatch.getBatchItem(0);
 
-      expect(spriteBatch.sources, hasLength(1));
-      expect(spriteBatch.sources.first, const Rect.fromLTWH(1, 1, 1, 1));
+      expect(batchItem.source, const Rect.fromLTWH(1, 1, 1, 1));
     });
 
     test('can replace the transform of a batch', () {
@@ -49,10 +54,10 @@ void main() {
       spriteBatch.add(source: Rect.zero);
 
       spriteBatch.replace(0, transform: RSTransform(1, 1, 1, 1));
+      final batchItem = spriteBatch.getBatchItem(0);
 
-      expect(spriteBatch.transforms, hasLength(1));
       expect(
-        spriteBatch.transforms.first,
+        batchItem.transform,
         isA<RSTransform>()
             .having((t) => t.scos, 'scos', 1)
             .having((t) => t.ssin, 'ssin', 1)