core: Put Character::Bitmap's data behind a Gc

This adds a new `BitmapCharacter` struct to hold the data as fields

Author: moulins

core/src/avm1/globals/bitmap_data.rs

@@ -1538,10 +1538,10 @@ fn load_bitmap<'gc>(
         .library_for_movie(movie)
         .and_then(|l| l.character_by_export_name(name));
 
-    let Some((_id, Character::Bitmap { compressed, .. })) = character else {
+    let Some((_id, Character::Bitmap(bitmap))) = character else {
         return Ok(Value::Undefined);
     };
-    let bitmap = compressed.decode().unwrap();
+    let bitmap = bitmap.compressed().decode().unwrap();
 
     let transparency = true;
     let bitmap_data = BitmapData::new_with_pixels(

core/src/avm2/globals/flash/display/bitmap.rs

@@ -44,18 +44,14 @@ pub fn bitmap_allocator<'gc>(
             .avm2_class_registry()
             .class_symbol(class)
         {
-            if let Some(Character::Bitmap {
-                compressed,
-                avm2_bitmapdata_class: _,
-                handle: _,
-            }) = activation
+            if let Some(Character::Bitmap(bitmap)) = activation
                 .context
                 .library
                 .library_for_movie_mut(movie)
                 .character_by_id(symbol)
                 .cloned()
             {
-                let new_bitmap_data = fill_bitmap_data_from_symbol(activation, &compressed);
+                let new_bitmap_data = fill_bitmap_data_from_symbol(activation, bitmap.compressed());
                 let bitmap_data_obj = BitmapDataObject::from_bitmap_data_internal(
                     activation,
                     BitmapDataWrapper::dummy(activation.gc()),

core/src/avm2/globals/flash/display/bitmap_data.rs

@@ -114,14 +114,9 @@ pub fn init<'gc>(
                 .cloned()
         });
 
-    let new_bitmap_data = if let Some(Character::Bitmap {
-        compressed,
-        avm2_bitmapdata_class: _,
-        handle: _,
-    }) = character
-    {
+    let new_bitmap_data = if let Some(Character::Bitmap(bitmap)) = character {
         // Instantiating BitmapData from an Animate-style bitmap asset
-        fill_bitmap_data_from_symbol(activation, &compressed)
+        fill_bitmap_data_from_symbol(activation, bitmap.compressed())
     } else {
         if character.is_some() {
             //TODO: Determine if mismatched symbols will still work as a

core/src/character.rs

@@ -6,8 +6,12 @@ use crate::display_object::{
     Avm1Button, Avm2Button, BitmapClass, EditText, Graphic, MorphShape, MovieClip, Text, Video,
 };
 use crate::font::Font;
-use gc_arena::{Collect, Gc, GcCell};
-use ruffle_render::bitmap::{BitmapHandle, BitmapSize};
+use gc_arena::barrier::unlock;
+use gc_arena::lock::Lock;
+use gc_arena::{Collect, Gc, Mutation};
+use ruffle_render::backend::RenderBackend;
+use ruffle_render::bitmap::{Bitmap as RenderBitmap, BitmapHandle, BitmapSize};
+use ruffle_render::error::Error as RenderError;
 use swf::DefineBitsLossless;
 
 #[derive(Clone, Collect, Debug)]
@@ -16,16 +20,7 @@ pub enum Character<'gc> {
     EditText(EditText<'gc>),
     Graphic(Graphic<'gc>),
     MovieClip(MovieClip<'gc>),
-    Bitmap {
-        #[collect(require_static)]
-        compressed: CompressedBitmap,
-        /// A lazily constructed GPU handle, used when performing fills with this bitmap
-        #[collect(require_static)]
-        handle: OnceCell<BitmapHandle>,
-        /// The bitmap class set by `SymbolClass` - this is used when we instantaite
-        /// a `Bitmap` displayobject.
-        avm2_bitmapdata_class: GcCell<'gc, BitmapClass<'gc>>,
-    },
+    Bitmap(Gc<'gc, BitmapCharacter<'gc>>),
     Avm1Button(Avm1Button<'gc>),
     Avm2Button(Avm2Button<'gc>),
     Font(Font<'gc>),
@@ -36,6 +31,56 @@ pub enum Character<'gc> {
     BinaryData(Gc<'gc, BinaryData>),
 }
 
+#[derive(Collect, Debug)]
+#[collect(no_drop)]
+pub struct BitmapCharacter<'gc> {
+    #[collect(require_static)]
+    compressed: CompressedBitmap,
+    /// A lazily constructed GPU handle, used when performing fills with this bitmap
+    #[collect(require_static)]
+    handle: OnceCell<BitmapHandle>,
+    /// The bitmap class set by `SymbolClass` - this is used when we instantaite
+    /// a `Bitmap` displayobject.
+    avm2_class: Lock<BitmapClass<'gc>>,
+}
+
+impl<'gc> BitmapCharacter<'gc> {
+    pub fn new(compressed: CompressedBitmap) -> Self {
+        Self {
+            compressed,
+            handle: OnceCell::default(),
+            avm2_class: Lock::new(BitmapClass::NoSubclass),
+        }
+    }
+
+    pub fn compressed(&self) -> &CompressedBitmap {
+        &self.compressed
+    }
+
+    pub fn avm2_class(&self) -> BitmapClass<'gc> {
+        self.avm2_class.get()
+    }
+
+    pub fn set_avm2_class(this: Gc<'gc, Self>, bitmap_class: BitmapClass<'gc>, mc: &Mutation<'gc>) {
+        unlock!(Gc::write(mc, this), Self, avm2_class).set(bitmap_class);
+    }
+
+    pub fn bitmap_handle(
+        &self,
+        backend: &mut dyn RenderBackend,
+    ) -> Result<BitmapHandle, RenderError> {
+        // FIXME - use `OnceCell::get_or_try_init` when stabilized.
+        if let Some(handle) = self.handle.get() {
+            return Ok(handle.clone());
+        }
+        let decoded = self.compressed.decode()?;
+        let new_handle = backend.register_bitmap(decoded)?;
+        // FIXME - do we ever want to release this handle, to avoid taking up GPU memory?
+        self.handle.set(new_handle.clone()).unwrap();
+        Ok(new_handle)
+    }
+}
+
 /// Holds a bitmap from an SWF tag, plus the decoded width/height.
 /// We avoid decompressing the image until it's actually needed - some pathological SWFS
 /// like 'House' have thousands of highly-compressed (mostly empty) bitmaps, which can
@@ -64,7 +109,7 @@ impl CompressedBitmap {
             },
         }
     }
-    pub fn decode(&self) -> Result<ruffle_render::bitmap::Bitmap, ruffle_render::error::Error> {
+    pub fn decode(&self) -> Result<RenderBitmap, RenderError> {
         match self {
             CompressedBitmap::Jpeg {
                 data,

core/src/display_object/movie_clip.rs

@@ -14,7 +14,7 @@ use crate::backend::audio::{AudioManager, SoundInstanceHandle};
 use crate::backend::navigator::Request;
 use crate::backend::ui::MouseCursor;
 use crate::binary_data::BinaryData;
-use crate::character::{Character, CompressedBitmap};
+use crate::character::{BitmapCharacter, Character, CompressedBitmap};
 use crate::context::{ActionType, RenderContext, UpdateContext};
 use crate::display_object::container::{dispatch_removed_event, ChildContainer};
 use crate::display_object::interactive::{
@@ -41,7 +41,7 @@ use bitflags::bitflags;
 use core::fmt;
 use gc_arena::barrier::unlock;
 use gc_arena::lock::{Lock, RefLock};
-use gc_arena::{Collect, Gc, GcCell, GcWeak, Mutation};
+use gc_arena::{Collect, Gc, GcWeak, Mutation};
 use ruffle_macros::istr;
 use ruffle_render::perspective_projection::PerspectiveProjection;
 use smallvec::SmallVec;
@@ -3167,20 +3167,19 @@ impl<'gc, 'a> MovieClipShared<'gc> {
         version: u8,
     ) -> Result<(), Error> {
         let define_bits_lossless = reader.read_define_bits_lossless(version)?;
-        let bitmap = Character::Bitmap {
-            compressed: CompressedBitmap::Lossless(DefineBitsLossless {
+        let bitmap = Gc::new(
+            context.gc(),
+            BitmapCharacter::new(CompressedBitmap::Lossless(DefineBitsLossless {
                 id: define_bits_lossless.id,
                 format: define_bits_lossless.format,
                 width: define_bits_lossless.width,
                 height: define_bits_lossless.height,
                 version: define_bits_lossless.version,
                 data: Cow::Owned(define_bits_lossless.data.into_owned()),
-            }),
-            handle: Default::default(),
-            avm2_bitmapdata_class: GcCell::new(context.gc(), BitmapClass::NoSubclass),
-        };
+            })),
+        );
         self.library_mut(context)
-            .register_character(define_bits_lossless.id, bitmap);
+            .register_character(define_bits_lossless.id, Character::Bitmap(bitmap));
         Ok(())
     }
 
@@ -3298,19 +3297,16 @@ impl<'gc, 'a> MovieClipShared<'gc> {
         let jpeg_data =
             ruffle_render::utils::glue_tables_to_jpeg(jpeg_data, jpeg_tables).into_owned();
         let (width, height) = ruffle_render::utils::decode_define_bits_jpeg_dimensions(&jpeg_data)?;
-        library.register_character(
-            id,
-            Character::Bitmap {
-                compressed: CompressedBitmap::Jpeg {
-                    data: jpeg_data,
-                    alpha: None,
-                    width,
-                    height,
-                },
-                handle: Default::default(),
-                avm2_bitmapdata_class: GcCell::new(mc, BitmapClass::NoSubclass),
-            },
-        );
+        let bitmap = Character::Bitmap(Gc::new(
+            mc,
+            BitmapCharacter::new(CompressedBitmap::Jpeg {
+                data: jpeg_data,
+                alpha: None,
+                width,
+                height,
+            }),
+        ));
+        library.register_character(id, bitmap);
         Ok(())
     }
 
@@ -3322,16 +3318,15 @@ impl<'gc, 'a> MovieClipShared<'gc> {
     ) -> Result<(), Error> {
         let (id, jpeg_data) = reader.read_define_bits_jpeg_2()?;
         let (width, height) = ruffle_render::utils::decode_define_bits_jpeg_dimensions(jpeg_data)?;
-        let bitmap = Character::Bitmap {
-            compressed: CompressedBitmap::Jpeg {
+        let bitmap = Character::Bitmap(Gc::new(
+            context.gc(),
+            BitmapCharacter::new(CompressedBitmap::Jpeg {
                 data: jpeg_data.to_owned(),
                 alpha: None,
                 width,
                 height,
-            },
-            handle: Default::default(),
-            avm2_bitmapdata_class: GcCell::new(context.gc(), BitmapClass::NoSubclass),
-        };
+            }),
+        ));
         self.library_mut(context).register_character(id, bitmap);
         Ok(())
     }
@@ -3345,16 +3340,16 @@ impl<'gc, 'a> MovieClipShared<'gc> {
     ) -> Result<(), Error> {
         let jpeg = reader.read_define_bits_jpeg_3(version)?;
         let (width, height) = ruffle_render::utils::decode_define_bits_jpeg_dimensions(jpeg.data)?;
-        let bitmap = Character::Bitmap {
-            compressed: CompressedBitmap::Jpeg {
+
+        let bitmap = Character::Bitmap(Gc::new(
+            context.gc(),
+            BitmapCharacter::new(CompressedBitmap::Jpeg {
                 data: jpeg.data.to_owned(),
                 alpha: Some(jpeg.alpha_data.to_owned()),
                 width,
                 height,
-            },
-            handle: Default::default(),
-            avm2_bitmapdata_class: GcCell::new(context.gc(), BitmapClass::NoSubclass),
-        };
+            }),
+        ));
         self.library_mut(context)
             .register_character(jpeg.id, bitmap);
         Ok(())
@@ -4136,15 +4131,16 @@ impl<'gc, 'a> MovieClip<'gc> {
                                         .library
                                         .library_for_movie_mut(movie.clone());
 
-                                    let Some(Character::Bitmap {
-                                        avm2_bitmapdata_class,
-                                        ..
-                                    }) = library.character_by_id(id)
+                                    let Some(&Character::Bitmap(bitmap)) =
+                                        library.character_by_id(id)
                                     else {
                                         unreachable!();
                                     };
-                                    *avm2_bitmapdata_class.write(activation.context.gc_context) =
-                                        bitmap_class;
+                                    BitmapCharacter::set_avm2_class(
+                                        bitmap,
+                                        bitmap_class,
+                                        activation.gc(),
+                                    );
                                 } else {
                                     tracing::error!("Associated class {:?} for symbol {} must extend flash.display.Bitmap or BitmapData, does neither", class_object.inner_class_definition().name(), id);
                                 }

core/src/library.rs

@@ -253,14 +253,11 @@ impl<'gc> MovieLibrary<'gc> {
         mc: &Mutation<'gc>,
     ) -> Option<DisplayObject<'gc>> {
         match character {
-            Character::Bitmap {
-                compressed,
-                avm2_bitmapdata_class,
-                handle: _,
-            } => {
-                let bitmap = compressed.decode().unwrap();
+            Character::Bitmap(bitmap) => {
+                let avm2_class = bitmap.avm2_class();
+                let bitmap = bitmap.compressed().decode().unwrap();
                 let bitmap = Bitmap::new(mc, id, bitmap, self.swf.clone());
-                bitmap.set_avm2_bitmapdata_class(mc, *avm2_bitmapdata_class.read());
+                bitmap.set_avm2_bitmapdata_class(mc, avm2_class);
                 Some(bitmap.instantiate(mc))
             }
             Character::EditText(edit_text) => Some(edit_text.instantiate(mc)),
@@ -373,44 +370,25 @@ pub struct MovieLibrarySource<'a, 'gc> {
 
 impl ruffle_render::bitmap::BitmapSource for MovieLibrarySource<'_, '_> {
     fn bitmap_size(&self, id: u16) -> Option<ruffle_render::bitmap::BitmapSize> {
-        if let Some(Character::Bitmap { compressed, .. }) = self.library.characters.get(&id) {
-            Some(compressed.size())
+        if let Some(Character::Bitmap(bitmap)) = self.library.characters.get(&id) {
+            Some(bitmap.compressed().size())
         } else {
             None
         }
     }
 
     fn bitmap_handle(&self, id: u16, backend: &mut dyn RenderBackend) -> Option<BitmapHandle> {
-        let Some(Character::Bitmap {
-            compressed,
-            handle,
-            avm2_bitmapdata_class: _,
-        }) = self.library.characters.get(&id)
-        else {
+        let Some(Character::Bitmap(bitmap)) = self.library.characters.get(&id) else {
             return None;
         };
 
-        // FIXME - use `OnceCell::get_or_try_init` when stabilized.
-        if let Some(handle) = handle.get() {
-            return Some(handle.clone());
-        }
-        let decoded = match compressed.decode() {
-            Ok(decoded) => decoded,
-            Err(e) => {
-                tracing::error!("Failed to decode bitmap character {id:?}: {e:?}");
-                return None;
-            }
-        };
-        let new_handle = match backend.register_bitmap(decoded) {
-            Ok(handle) => handle,
+        match bitmap.bitmap_handle(backend) {
+            Ok(handle) => Some(handle),
             Err(e) => {
-                tracing::error!("Failed to register bitmap character {id:?}: {e:?}");
-                return None;
+                tracing::error!("Failed to register bitmap character {id}: {e}");
+                None
             }
-        };
-        // FIXME - do we ever want to release this handle, to avoid taking up GPU memory?
-        handle.set(new_handle.clone()).unwrap();
-        Some(new_handle)
+        }
     }
 }