manage sprites better

agb/src/display/object/managed.rs

@@ -27,7 +27,6 @@ struct ObjectItem {
 
 struct Store {
     store: UnsafeCell<slotmap::SlotMap<ObjectKey, ObjectItem>>,
-    removal_list: UnsafeCell<Vec<ObjectKey>>,
     first_z: Cell<Option<ObjectKey>>,
 }
 
@@ -113,45 +112,32 @@ impl Store {
     }
 
     fn remove_object(&self, object: ObjectKey) {
+        remove_from_linked_list(self, object);
+
         let data = unsafe { &mut *self.store.get() };
         data.remove(object);
     }
 
-    fn remove_all_in_removal_list(&self) {
-        let removal_list = unsafe { &mut *self.removal_list.get() };
-        for object in removal_list.drain(..) {
-            self.remove_object(object);
-        }
-    }
-
-    fn mark_for_removal(&self, object: ObjectKey) {
-        let removal_list = unsafe { &mut *self.removal_list.get() };
-        removal_list.push(object);
-
-        remove_from_linked_list(self, object);
-    }
-
     fn get_object(&self, key: ObjectKey) -> &ObjectItem {
         &(unsafe { &*self.store.get() }[key])
     }
 }
 
 pub struct OAMManager<'gba> {
-    phantom: PhantomData<&'gba ()>,
     object_store: Store,
     sprite_loader: UnsafeCell<StaticSpriteLoader>,
+    unmanaged: UnsafeCell<UnmanagedOAM<'gba>>,
 }
 
 impl OAMManager<'_> {
     pub(crate) fn new() -> Self {
         Self {
-            phantom: PhantomData,
             object_store: Store {
                 store: UnsafeCell::new(SlotMap::with_key()),
-                removal_list: UnsafeCell::new(Vec::new()),
                 first_z: Cell::new(None),
             },
             sprite_loader: UnsafeCell::new(StaticSpriteLoader::new()),
+            unmanaged: UnsafeCell::new(UnmanagedOAM::new()),
         }
     }
 
@@ -167,9 +153,8 @@ impl OAMManager<'_> {
     }
 
     pub fn commit(&self) {
-        let mut unmanaged = UnmanagedOAM::new();
+        // safety: commit is not reentrant
-
+        let unmanaged = unsafe { &mut *self.unmanaged.get() };
-        // do interactions with OAM
 
         for (object, mut slot) in unsafe { self.object_store.iter() }
             .map(|item| unsafe { &*item.object.get() })
@@ -179,9 +164,6 @@ impl OAMManager<'_> {
             slot.set(object);
         }
 
-        // finished OAM interactions
-
-        self.object_store.remove_all_in_removal_list();
         // safety: not reentrant
         unsafe {
             self.do_work_with_sprite_loader(StaticSpriteLoader::garbage_collect);
@@ -212,7 +194,7 @@ pub struct Object<'controller> {
 
 impl Drop for Object<'_> {
     fn drop(&mut self) {
-        self.store.mark_for_removal(self.me);
+        self.store.remove_object(self.me);
     }
 }
 

agb/src/display/object/unmanaged/object.rs

@@ -1,9 +1,7 @@
-use core::{
+use core::{cell::UnsafeCell, marker::PhantomData};
-    cell::{Cell, UnsafeCell},
-    marker::PhantomData,
-};
 
 use agb_fixnum::Vector2D;
+use alloc::vec::Vec;
 
 use crate::display::{
     object::{sprites::SpriteVram, OBJECT_ATTRIBUTE_MEMORY},
@@ -12,31 +10,41 @@ use crate::display::{
 
 use super::attributes::{AffineMode, Attributes};
 
+#[derive(Default, Debug)]
+struct OamFrameModifyables {
+    up_to: i32,
+    this_frame_sprites: Vec<SpriteVram>,
+}
+
 pub struct UnmanagedOAM<'gba> {
     phantom: PhantomData<&'gba ()>,
-    up_to: Cell<i32>,
+    frame_data: UnsafeCell<OamFrameModifyables>,
+    previous_frame_sprites: Vec<SpriteVram>,
 }
 
 pub struct OAMIterator<'oam> {
     index: usize,
-    up_to: &'oam Cell<i32>,
+    frame_data: &'oam UnsafeCell<OamFrameModifyables>,
 }
 
 pub struct OAMSlot<'oam> {
     slot: usize,
-    up_to: &'oam Cell<i32>,
+    frame_data: &'oam UnsafeCell<OamFrameModifyables>,
 }
 
 impl OAMSlot<'_> {
     pub fn set(&mut self, object: &UnmanagedObject) {
         self.set_bytes(object.attributes.bytes());
 
+        // SAFETY: This function is not reentrant and we currently hold a mutable borrow of the [UnmanagedOAM].
+        let frame_data = unsafe { &mut *self.frame_data.get() };
+
         // SAFETY: This is called here and in set_sprite, neither of which call the other.
-        let sprites = unsafe { &mut *object.sprites.get() };
+        let sprite = unsafe { &mut *object.sprites.get() };
 
-        sprites.previous_sprite = Some(sprites.sprite.clone());
+        frame_data.this_frame_sprites.push(sprite.clone());
 
-        self.up_to.set(self.slot as i32);
+        frame_data.up_to = self.slot as i32;
     }
 
     fn set_bytes(&mut self, bytes: [u8; 6]) {
@@ -59,7 +67,7 @@ impl<'oam> Iterator for OAMIterator<'oam> {
         } else {
             Some(OAMSlot {
                 slot: idx,
-                up_to: self.up_to,
+                frame_data: self.frame_data,
             })
         }
     }
@@ -67,7 +75,7 @@ impl<'oam> Iterator for OAMIterator<'oam> {
 
 impl Drop for OAMIterator<'_> {
     fn drop(&mut self) {
-        let last_written = self.up_to.get();
+        let last_written = unsafe { &*self.frame_data.get() }.up_to;
 
         let number_writen = (last_written + 1) as usize;
 
@@ -82,31 +90,36 @@ impl Drop for OAMIterator<'_> {
 
 impl UnmanagedOAM<'_> {
     pub fn iter(&mut self) -> OAMIterator<'_> {
-        self.up_to.set(-1);
+        let frame_data = self.frame_data.get_mut();
+        frame_data.up_to = -1;
+
+        // We drain the previous frame sprites here to reuse the Vecs allocation and remove the now unused sprites.
+        // Any sprites currently being shown will now be put in the new Vec.
+        self.previous_frame_sprites.drain(..);
+        core::mem::swap(
+            &mut frame_data.this_frame_sprites,
+            &mut self.previous_frame_sprites,
+        );
+
         OAMIterator {
             index: 0,
-            up_to: &self.up_to,
+            frame_data: &self.frame_data,
         }
     }
 
     pub(crate) fn new() -> Self {
         Self {
-            up_to: Cell::new(-1),
+            frame_data: Default::default(),
             phantom: PhantomData,
+            previous_frame_sprites: Default::default(),
         }
     }
 }
 
-#[derive(Debug)]
-struct VramSprites {
-    sprite: SpriteVram,
-    previous_sprite: Option<SpriteVram>,
-}
-
 #[derive(Debug)]
 pub struct UnmanagedObject {
     attributes: Attributes,
-    sprites: UnsafeCell<VramSprites>,
+    sprites: UnsafeCell<SpriteVram>,
 }
 
 impl UnmanagedObject {
@@ -118,10 +131,7 @@ impl UnmanagedObject {
 
         let mut sprite = Self {
             attributes: Attributes::default(),
-            sprites: UnsafeCell::new(VramSprites {
+            sprites: UnsafeCell::new(sprite),
-                sprite,
-                previous_sprite: None,
-            }),
         };
 
         sprite.attributes.set_sprite(sprite_location, shape, size);
@@ -204,7 +214,7 @@ impl UnmanagedObject {
 
         // SAFETY: This is called here and in OAMSlot set, neither of which call the other.
         let sprites = unsafe { &mut *self.sprites.get() };
-        sprites.sprite = sprite;
+        *sprites = sprite;
 
         self
     }