core: Replace GcCell with Gc in Bitmap

This refactor replaces GcCell with Gc and uses interior mutability
instead.

Author: kjarosh

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

@@ -110,8 +110,8 @@ pub fn init<'gc>(
         if let Some(bitmap_data) = bitmap_data {
             bitmap.set_bitmap_data(activation.context, bitmap_data);
         }
-        bitmap.set_smoothing(activation.gc(), smoothing);
-        bitmap.set_pixel_snapping(activation.gc(), pixel_snapping);
+        bitmap.set_smoothing(smoothing);
+        bitmap.set_pixel_snapping(pixel_snapping);
     } else {
         unreachable!();
     }
@@ -205,7 +205,7 @@ pub fn set_pixel_snapping<'gc>(
             return Err(make_error_2008(activation, "pixelSnapping"));
         };
 
-        bitmap.set_pixel_snapping(activation.gc(), pixel_snapping);
+        bitmap.set_pixel_snapping(pixel_snapping);
     }
     Ok(Value::Undefined)
 }
@@ -227,15 +227,15 @@ pub fn get_smoothing<'gc>(
 
 /// Implement `Bitmap.smoothing`'s setter
 pub fn set_smoothing<'gc>(
-    activation: &mut Activation<'_, 'gc>,
+    _activation: &mut Activation<'_, 'gc>,
     this: Value<'gc>,
     args: &[Value<'gc>],
 ) -> Result<Value<'gc>, Error<'gc>> {
     let this = this.as_object().unwrap();
 
     if let Some(bitmap) = this.as_display_object().and_then(|dobj| dobj.as_bitmap()) {
         let smoothing = args.get_bool(0);
-        bitmap.set_smoothing(activation.gc(), smoothing);
+        bitmap.set_smoothing(smoothing);
     }
 
     Ok(Value::Undefined)

core/src/display_object/bitmap.rs

@@ -13,15 +13,17 @@ use crate::prelude::*;
 use crate::tag_utils::SwfMovie;
 use crate::vminterface::Instantiator;
 use core::fmt;
-use gc_arena::{Collect, GcCell, GcWeakCell, Mutation};
+use gc_arena::barrier::unlock;
+use gc_arena::lock::{Lock, RefLock};
+use gc_arena::{Collect, Gc, GcCell, GcWeak, Mutation};
 use ruffle_render::backend::RenderBackend;
 use ruffle_render::bitmap::{BitmapFormat, PixelSnapping};
-use std::cell::{Ref, RefMut};
+use std::cell::{Cell, Ref, RefMut};
 use std::sync::Arc;
 
 #[derive(Clone, Debug, Collect, Copy)]
 #[collect(no_drop)]
-pub struct BitmapWeak<'gc>(GcWeakCell<'gc, BitmapGraphicData<'gc>>);
+pub struct BitmapWeak<'gc>(GcWeak<'gc, BitmapGraphicData<'gc>>);
 
 impl<'gc> BitmapWeak<'gc> {
     pub fn upgrade(self, mc: &Mutation<'gc>) -> Option<Bitmap<'gc>> {
@@ -84,47 +86,46 @@ impl<'gc> BitmapClass<'gc> {
 /// It can also be created in ActionScript using the `Bitmap` class.
 #[derive(Clone, Collect, Copy)]
 #[collect(no_drop)]
-pub struct Bitmap<'gc>(GcCell<'gc, BitmapGraphicData<'gc>>);
+pub struct Bitmap<'gc>(Gc<'gc, BitmapGraphicData<'gc>>);
 
 impl fmt::Debug for Bitmap<'_> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_struct("Bitmap")
-            .field("ptr", &self.0.as_ptr())
+            .field("ptr", &Gc::as_ptr(self.0))
             .finish()
     }
 }
 
 #[derive(Clone, Collect)]
 #[collect(no_drop)]
 pub struct BitmapGraphicData<'gc> {
-    base: DisplayObjectBase<'gc>,
+    base: RefLock<DisplayObjectBase<'gc>>,
     id: CharacterId,
     movie: Arc<SwfMovie>,
 
     /// The current bitmap data object.
-    bitmap_data: BitmapDataWrapper<'gc>,
+    bitmap_data: Lock<BitmapDataWrapper<'gc>>,
 
     /// The width and height values are cached from the BitmapDataWrapper
     /// when this Bitmap instance is first created,
     /// and continue to be reported even if the BitmapData is disposed.
-    width: u32,
-    height: u32,
+    width: Cell<u32>,
+    height: Cell<u32>,
 
     /// Whether or not bitmap smoothing is enabled.
-    smoothing: bool,
+    smoothing: Cell<bool>,
 
     /// How to snap this bitmap to the pixel grid
-    #[collect(require_static)]
-    pixel_snapping: PixelSnapping,
+    pixel_snapping: Cell<PixelSnapping>,
 
     /// The AVM2 side of this object.
     ///
     /// AVM1 code cannot directly reference `Bitmap`s, so this does not support
     /// storing an AVM1 object.
-    avm2_object: Option<Avm2Object<'gc>>,
+    avm2_object: Lock<Option<Avm2Object<'gc>>>,
 
     /// The class associated with this Bitmap.
-    avm2_bitmap_class: BitmapClass<'gc>,
+    avm2_bitmap_class: Lock<BitmapClass<'gc>>,
 }
 
 impl<'gc> Bitmap<'gc> {
@@ -148,18 +149,18 @@ impl<'gc> Bitmap<'gc> {
         let width = bitmap_data.width();
         let height = bitmap_data.height();
 
-        let bitmap = Bitmap(GcCell::new(
+        let bitmap = Bitmap(Gc::new(
             mc,
             BitmapGraphicData {
                 base: Default::default(),
                 id,
-                bitmap_data,
-                width,
-                height,
-                smoothing,
-                pixel_snapping: PixelSnapping::Auto,
-                avm2_object: None,
-                avm2_bitmap_class: BitmapClass::NoSubclass,
+                bitmap_data: Lock::new(bitmap_data),
+                width: Cell::new(width),
+                height: Cell::new(height),
+                smoothing: Cell::new(smoothing),
+                pixel_snapping: Cell::new(PixelSnapping::Auto),
+                avm2_object: Lock::new(None),
+                avm2_bitmap_class: Lock::new(BitmapClass::NoSubclass),
                 movie: movie.clone(),
             },
         ));
@@ -205,28 +206,28 @@ impl<'gc> Bitmap<'gc> {
     // values on this object. See the definition of these fields
     // for more information
     pub fn width(self) -> u16 {
-        self.0.read().width as u16
+        self.0.width.get() as u16
     }
 
     pub fn height(self) -> u16 {
-        self.0.read().height as u16
+        self.0.height.get() as u16
     }
 
     pub fn pixel_snapping(self) -> PixelSnapping {
-        self.0.read().pixel_snapping
+        self.0.pixel_snapping.get()
     }
 
-    pub fn set_pixel_snapping(self, mc: &Mutation<'gc>, value: PixelSnapping) {
-        self.0.write(mc).pixel_snapping = value;
+    pub fn set_pixel_snapping(self, value: PixelSnapping) {
+        self.0.pixel_snapping.set(value);
     }
 
     pub fn bitmap_data_wrapper(self) -> BitmapDataWrapper<'gc> {
-        self.0.read().bitmap_data
+        self.0.bitmap_data.get()
     }
 
     /// Retrieve the bitmap data associated with this `Bitmap`.
     pub fn bitmap_data(self, renderer: &mut dyn RenderBackend) -> GcCell<'gc, BitmapData<'gc>> {
-        self.0.read().bitmap_data.sync(renderer)
+        self.0.bitmap_data.get().sync(renderer)
     }
 
     /// Associate this `Bitmap` with new `BitmapData`.
@@ -243,72 +244,81 @@ impl<'gc> Bitmap<'gc> {
         bitmap_data: BitmapDataWrapper<'gc>,
     ) {
         let weak_self = DisplayObjectWeak::Bitmap(self.downgrade());
-        let mut write = self.0.write(context.gc());
 
-        write
+        self.0
             .bitmap_data
+            .get()
             .remove_display_object(context.gc(), weak_self);
 
         // Refresh our cached values, even if we're writing the same BitmapData
         // that we currently have stored. This will update them to '0' if the
         // BitmapData has been disposed since it was originally set.
-        write.width = bitmap_data.width();
-        write.height = bitmap_data.height();
-        write.bitmap_data = bitmap_data;
+        self.0.width.set(bitmap_data.width());
+        self.0.height.set(bitmap_data.height());
+        unlock!(
+            Gc::write(context.gc(), self.0),
+            BitmapGraphicData,
+            bitmap_data
+        )
+        .set(bitmap_data);
 
         bitmap_data.add_display_object(context.gc(), weak_self);
     }
 
     pub fn avm2_bitmapdata_class(self) -> Option<Avm2ClassObject<'gc>> {
-        match self.0.read().avm2_bitmap_class {
+        match self.0.avm2_bitmap_class.get() {
             BitmapClass::BitmapData(c) => Some(c),
             _ => None,
         }
     }
 
     pub fn avm2_bitmap_class(self) -> Option<Avm2ClassObject<'gc>> {
-        match self.0.read().avm2_bitmap_class {
+        match self.0.avm2_bitmap_class.get() {
             BitmapClass::Bitmap(c) => Some(c),
             _ => None,
         }
     }
 
     pub fn set_avm2_bitmapdata_class(self, mc: &Mutation<'gc>, class: BitmapClass<'gc>) {
-        self.0.write(mc).avm2_bitmap_class = class;
+        unlock!(Gc::write(mc, self.0), BitmapGraphicData, avm2_bitmap_class).set(class);
+    }
+
+    fn set_avm2_object(self, mc: &Mutation<'gc>, object: Option<Avm2Object<'gc>>) {
+        unlock!(Gc::write(mc, self.0), BitmapGraphicData, avm2_object).set(object);
     }
 
     pub fn smoothing(self) -> bool {
-        self.0.read().smoothing
+        self.0.smoothing.get()
     }
 
-    pub fn set_smoothing(self, mc: &Mutation<'gc>, smoothing: bool) {
-        self.0.write(mc).smoothing = smoothing;
+    pub fn set_smoothing(self, smoothing: bool) {
+        self.0.smoothing.set(smoothing);
     }
 
     pub fn downgrade(self) -> BitmapWeak<'gc> {
-        BitmapWeak(GcCell::downgrade(self.0))
+        BitmapWeak(Gc::downgrade(self.0))
     }
 }
 
 impl<'gc> TDisplayObject<'gc> for Bitmap<'gc> {
     fn base(&self) -> Ref<DisplayObjectBase<'gc>> {
-        Ref::map(self.0.read(), |r| &r.base)
+        self.0.base.borrow()
     }
 
     fn base_mut<'a>(&'a self, mc: &Mutation<'gc>) -> RefMut<'a, DisplayObjectBase<'gc>> {
-        RefMut::map(self.0.write(mc), |w| &mut w.base)
+        unlock!(Gc::write(mc, self.0), BitmapGraphicData, base).borrow_mut()
     }
 
     fn instantiate(&self, gc_context: &Mutation<'gc>) -> DisplayObject<'gc> {
-        Self(GcCell::new(gc_context, self.0.read().clone())).into()
+        Self(Gc::new(gc_context, self.0.as_ref().clone())).into()
     }
 
     fn as_ptr(&self) -> *const DisplayObjectPtr {
-        self.0.as_ptr() as *const DisplayObjectPtr
+        Gc::as_ptr(self.0) as *const DisplayObjectPtr
     }
 
     fn id(&self) -> CharacterId {
-        self.0.read().id
+        self.0.id
     }
 
     fn self_bounds(&self) -> Rectangle<Twips> {
@@ -345,7 +355,7 @@ impl<'gc> TDisplayObject<'gc> for Bitmap<'gc> {
                     bitmap_cls,
                 )
                 .expect("can't throw from post_instantiation -_-");
-                self.0.write(mc).avm2_object = Some(bitmap.into());
+                self.set_avm2_object(activation.gc(), Some(bitmap.into()));
 
                 // Use a dummy BitmapData when calling the constructor on the user subclass
                 // - the constructor should see an invalid BitmapData before calling 'super',
@@ -379,29 +389,30 @@ impl<'gc> TDisplayObject<'gc> for Bitmap<'gc> {
             return;
         }
 
-        let bitmap_data = self.0.read();
-        bitmap_data
-            .bitmap_data
-            .render(bitmap_data.smoothing, context, bitmap_data.pixel_snapping);
+        self.0.bitmap_data.get().render(
+            self.0.smoothing.get(),
+            context,
+            self.0.pixel_snapping.get(),
+        );
     }
 
     fn object2(&self) -> Avm2Value<'gc> {
         self.0
-            .read()
             .avm2_object
+            .get()
             .map(|o| o.into())
             .unwrap_or(Avm2Value::Null)
     }
 
     fn set_object2(&self, context: &mut UpdateContext<'gc>, to: Avm2Object<'gc>) {
-        self.0.write(context.gc()).avm2_object = Some(to);
+        self.set_avm2_object(context.gc(), Some(to));
     }
 
     fn as_bitmap(self) -> Option<Bitmap<'gc>> {
         Some(self)
     }
 
     fn movie(&self) -> Arc<SwfMovie> {
-        self.0.read().movie.clone()
+        self.0.movie.clone()
     }
 }