core: Respect ratio field for non-morph shapes (fix #1291)

core/src/display_object.rs

@@ -36,10 +36,10 @@ pub use text::Text;
 #[derive(Clone, Debug)]
 pub struct DisplayObjectBase<'gc> {
     parent: Option<DisplayObject<'gc>>,
-    place_frame: u16,
     depth: Depth,
     transform: Transform,
     name: String,
+    ratio: u16,
     clip_depth: Depth,
 
     // Cached transform properties `_xscale`, `_yscale`, `_rotation`.
@@ -72,11 +72,11 @@ impl<'gc> Default for DisplayObjectBase<'gc> {
     fn default() -> Self {
         Self {
             parent: Default::default(),
-            place_frame: Default::default(),
             depth: Default::default(),
             transform: Default::default(),
             name: Default::default(),
             clip_depth: Default::default(),
+            ratio: Default::default(),
             rotation: Degrees::from_radians(0.0),
             scale_x: Percent::from_unit(1.0),
             scale_y: Percent::from_unit(1.0),
@@ -116,12 +116,7 @@ impl<'gc> DisplayObjectBase<'gc> {
     fn set_depth(&mut self, depth: Depth) {
         self.depth = depth;
     }
-    fn place_frame(&self) -> u16 {
-        self.place_frame
-    }
-    fn set_place_frame(&mut self, _context: MutationContext<'gc, '_>, frame: u16) {
-        self.place_frame = frame;
-    }
+
     fn transform(&self) -> &Transform {
         &self.transform
     }
@@ -282,6 +277,12 @@ impl<'gc> DisplayObjectBase<'gc> {
     fn set_clip_depth(&mut self, _context: MutationContext<'gc, '_>, depth: Depth) {
         self.clip_depth = depth;
     }
+    fn ratio(&self) -> u16 {
+        self.ratio
+    }
+    fn set_ratio(&mut self, _context: MutationContext<'gc, '_>, ratio: u16) {
+        self.ratio = ratio;
+    }
     fn parent(&self) -> Option<DisplayObject<'gc>> {
         self.parent
     }
@@ -357,6 +358,18 @@ impl<'gc> DisplayObjectBase<'gc> {
         }
     }
 
+    fn placed_during_goto(&self) -> bool {
+        self.flags.contains(DisplayObjectFlags::PlacedDuringGoto)
+    }
+
+    fn set_placed_during_goto(&mut self, value: bool) {
+        if value {
+            self.flags.insert(DisplayObjectFlags::PlacedDuringGoto);
+        } else {
+            self.flags.remove(DisplayObjectFlags::PlacedDuringGoto);
+        }
+    }
+
     fn swf_version(&self) -> u8 {
         self.parent
             .map(|p| p.swf_version())
@@ -425,9 +438,6 @@ pub trait TDisplayObject<'gc>:
         bounds
     }
 
-    fn place_frame(&self) -> u16;
-    fn set_place_frame(&self, context: MutationContext<'gc, '_>, frame: u16);
-
     fn transform(&self) -> Ref<Transform>;
     fn matrix(&self) -> Ref<Matrix>;
     fn matrix_mut(&self, context: MutationContext<'gc, '_>) -> RefMut<Matrix>;
@@ -651,6 +661,8 @@ pub trait TDisplayObject<'gc>:
 
     fn clip_depth(&self) -> Depth;
     fn set_clip_depth(&self, context: MutationContext<'gc, '_>, depth: Depth);
+    fn ratio(&self) -> u16;
+    fn set_ratio(&self, context: MutationContext<'gc, '_>, ratio: u16);
     fn parent(&self) -> Option<DisplayObject<'gc>>;
     fn set_parent(&self, context: MutationContext<'gc, '_>, parent: Option<DisplayObject<'gc>>);
     fn first_child(&self) -> Option<DisplayObject<'gc>>;
@@ -720,6 +732,9 @@ pub trait TDisplayObject<'gc>:
     /// When this flag is set, changes from SWF `PlaceObject` tags are ignored.
     fn set_transformed_by_script(&self, context: MutationContext<'gc, '_>, value: bool);
 
+    fn placed_during_goto(&self) -> bool;
+    fn set_placed_during_goto(&self, context: MutationContext<'gc, '_>, value: bool);
+
     /// Executes and propagates the given clip event.
     /// Events execute inside-out; the deepest child will react first, followed by its parent, and
     /// so forth.
@@ -782,9 +797,7 @@ pub trait TDisplayObject<'gc>:
                 self.set_clip_depth(gc_context, clip_depth.into());
             }
             if let Some(ratio) = place_object.ratio {
-                if let Some(mut morph_shape) = self.as_morph_shape() {
-                    morph_shape.set_ratio(gc_context, ratio);
-                }
+                self.set_ratio(gc_context, ratio);
             }
             // Clip events only apply to movie clips.
             if let (Some(clip_actions), Some(clip)) =
@@ -815,9 +828,7 @@ pub trait TDisplayObject<'gc>:
         self.set_color_transform(gc_context, &*other.color_transform());
         self.set_clip_depth(gc_context, other.clip_depth());
         self.set_name(gc_context, &*other.name());
-        if let (Some(mut me), Some(other)) = (self.as_morph_shape(), other.as_morph_shape()) {
-            me.set_ratio(gc_context, other.ratio());
-        }
+        self.set_ratio(gc_context, other.ratio());
         // onEnterFrame actions only apply to movie clips.
         if let (Some(me), Some(other)) = (self.as_movie_clip(), other.as_movie_clip()) {
             me.set_clip_actions(gc_context, other.clip_actions().iter().cloned().collect());
@@ -960,12 +971,6 @@ macro_rules! impl_display_object_sansbounds {
         fn set_depth(&self, gc_context: gc_arena::MutationContext<'gc, '_>, depth: Depth) {
             self.0.write(gc_context).$field.set_depth(depth)
         }
-        fn place_frame(&self) -> u16 {
-            self.0.read().$field.place_frame()
-        }
-        fn set_place_frame(&self, context: gc_arena::MutationContext<'gc, '_>, frame: u16) {
-            self.0.write(context).$field.set_place_frame(context, frame)
-        }
         fn transform(&self) -> std::cell::Ref<crate::transform::Transform> {
             std::cell::Ref::map(self.0.read(), |o| o.$field.transform())
         }
@@ -1037,6 +1042,12 @@ macro_rules! impl_display_object_sansbounds {
         ) {
             self.0.write(context).$field.set_clip_depth(context, depth)
         }
+        fn ratio(&self) -> u16 {
+            self.0.read().$field.ratio()
+        }
+        fn set_ratio(&self, context: gc_arena::MutationContext<'gc, '_>, depth: u16) {
+            self.0.write(context).$field.set_ratio(context, depth)
+        }
         fn parent(&self) -> Option<crate::display_object::DisplayObject<'gc>> {
             self.0.read().$field.parent()
         }
@@ -1102,6 +1113,12 @@ macro_rules! impl_display_object_sansbounds {
                 .$field
                 .set_transformed_by_script(value)
         }
+        fn placed_during_goto(&self) -> bool {
+            self.0.read().$field.placed_during_goto()
+        }
+        fn set_placed_during_goto(&self, context: gc_arena::MutationContext<'gc, '_>, value: bool) {
+            self.0.write(context).$field.set_placed_during_goto(value)
+        }
         fn instantiate(
             &self,
             gc_context: gc_arena::MutationContext<'gc, '_>,
@@ -1222,6 +1239,9 @@ enum DisplayObjectFlags {
     /// Whether this object has been transformed by ActionScript.
     /// When this flag is set, changes from SWF `PlaceObject` tags are ignored.
     TransformedByScript,
+
+    /// Used during a rewinding goto.
+    PlacedDuringGoto,
 }
 
 pub struct ChildIter<'gc> {

core/src/display_object/morph_shape.rs

@@ -3,7 +3,7 @@ use crate::context::{RenderContext, UpdateContext};
 use crate::display_object::{DisplayObjectBase, TDisplayObject};
 use crate::prelude::*;
 use crate::types::{Degrees, Percent};
-use gc_arena::{Collect, Gc, GcCell, MutationContext};
+use gc_arena::{Collect, Gc, GcCell};
 use swf::Twips;
 
 #[derive(Clone, Debug, Collect, Copy)]
@@ -14,7 +14,6 @@ pub struct MorphShape<'gc>(GcCell<'gc, MorphShapeData<'gc>>);
 pub struct MorphShapeData<'gc> {
     base: DisplayObjectBase<'gc>,
     static_data: Gc<'gc, MorphShapeStatic>,
-    ratio: u16,
 }
 
 impl<'gc> MorphShape<'gc> {
@@ -27,18 +26,9 @@ impl<'gc> MorphShape<'gc> {
             MorphShapeData {
                 base: Default::default(),
                 static_data: Gc::allocate(gc_context, static_data),
-                ratio: 0,
             },
         ))
     }
-
-    pub fn ratio(self) -> u16 {
-        self.0.read().ratio
-    }
-
-    pub fn set_ratio(&mut self, gc_context: MutationContext<'gc, '_>, ratio: u16) {
-        self.0.write(gc_context).ratio = ratio;
-    }
 }
 
 impl<'gc> TDisplayObject<'gc> for MorphShape<'gc> {

core/src/display_object/movie_clip.rs

@@ -942,7 +942,6 @@ impl<'gc> MovieClip<'gc> {
         }
         parent.add_child_to_exec_list(context.gc_context, child);
         child.set_parent(context.gc_context, Some((*self).into()));
-        child.set_place_frame(context.gc_context, 0);
         child.set_depth(context.gc_context, depth);
     }
 
@@ -1180,7 +1179,6 @@ impl<'gc> MovieClip<'gc> {
                 // Set initial properties for child.
                 child.set_depth(context.gc_context, depth);
                 child.set_parent(context.gc_context, Some(self_display_object));
-                child.set_place_frame(context.gc_context, self.current_frame());
                 if copy_previous_properties {
                     if let Some(prev_child) = prev_child {
                         child.copy_display_properties_from(context.gc_context, prev_child);
@@ -1212,7 +1210,11 @@ impl<'gc> MovieClip<'gc> {
         // 3) Objects that would persist over the goto conceptually should not be
         //    destroyed and recreated; they should keep their properties.
         //    Particularly for rewinds, the object should persist if it was created
-        //      *before* the frame we are going to. (DisplayObject::place_frame).
+        //      *before* the frame we are going to. This is handled by the `ratio`
+        //      field in PlaceObject; this field will indicate the frame on which the
+        //      object was placed (this is not documented in the SWF specs).
+        //      After a rewind, an object should be recreated if the place object ID
+        //      or ratio fields differ; otherwise it's the same clip, and should be reused.
         // 4) We want to avoid creating objects just to destroy them if they aren't on
         //    the goto frame, so we should instead aggregate the deltas into a final list
         //    of commands, and THEN modify the children as necessary.
@@ -1229,27 +1231,6 @@ impl<'gc> MovieClip<'gc> {
             self.0.write(context.gc_context).tag_stream_pos = 0;
             self.0.write(context.gc_context).current_frame = 0;
 
-            // Remove all display objects that were created after the destination frame.
-            // TODO: We want to do something like self.children.retain here,
-            // but BTreeMap::retain does not exist.
-            let children: SmallVec<[_; 16]> = self
-                .0
-                .read()
-                .children
-                .iter()
-                .filter_map(|(depth, clip)| {
-                    if clip.place_frame() > frame {
-                        Some((*depth, *clip))
-                    } else {
-                        None
-                    }
-                })
-                .collect();
-            for (depth, child) in children {
-                let mut mc = self.0.write(context.gc_context);
-                mc.children.remove(&depth);
-                mc.remove_child_from_exec_list(context, child);
-            }
             true
         } else {
             false
@@ -1353,10 +1334,17 @@ impl<'gc> MovieClip<'gc> {
                 // it will exist on the final frame as well. Re-use this object
                 // instead of recreating.
                 // If the ID is 0, we are modifying a previous child. Otherwise, we're replacing it.
-                // If it's a rewind, we removed any dead children above, so we always
-                // modify the previous child.
-                Some(prev_child) if params.id() == 0 || is_rewind => {
+                // If it's a rewind, we use the id and ratio field to determine if the object is the same instance.
+                // For non-morph shapes, the ratio field indicates the frame that the object was placed on.
+                // (See #1291)
+                Some(prev_child)
+                    if params.id() == 0
+                        || (is_rewind
+                            && params.id() == prev_child.id()
+                            && params.ratio() == prev_child.ratio()) =>
+                {
                     prev_child.apply_place_object(context.gc_context, &params.place_object);
+                    prev_child.set_placed_during_goto(context.gc_context, true);
                 }
                 _ => {
                     if let Some(child) = clip.instantiate_child(
@@ -1367,8 +1355,10 @@ impl<'gc> MovieClip<'gc> {
                         &params.place_object,
                         params.modifies_original_item(),
                     ) {
-                        // Set the place frame to the frame where the object *would* have been placed.
-                        child.set_place_frame(context.gc_context, params.frame);
+                        // Flag that this object was touched by a goto during a rewind.
+                        if is_rewind {
+                            child.set_placed_during_goto(context.gc_context, true);
+                        }
                     }
                 }
             }
@@ -1405,6 +1395,19 @@ impl<'gc> MovieClip<'gc> {
             .iter()
             .filter(|params| params.frame >= frame)
             .for_each(|goto| run_goto_command(self, context, goto));
+
+        if is_rewind {
+            // Remove any objects that were not touched by this goto; they do not exist on this frame.
+            for child in self.children() {
+                if child.placed_during_goto() {
+                    child.set_placed_during_goto(context.gc_context, false);
+                } else {
+                    let mut mc = self.0.write(context.gc_context);
+                    mc.children.remove(&child.depth());
+                    mc.remove_child_from_exec_list(context, child);
+                }
+            }
+        }
     }
 
     fn construct_as_avm1_object(
@@ -2017,17 +2020,18 @@ impl<'gc> MovieClipData<'gc> {
         if let Some(i) = goto_commands.iter().position(|o| o.depth() == depth) {
             goto_commands.swap_remove(i);
         }
+        // For fast-forwards, if this tag were to remove an object
+        // that existed before the goto, then we can remove that child right away.
+        // Don't do this for rewinds, because they conceptually
+        // start from an empty display list, so we need to examine the ratio field
+        // to determine if they get removed/re-created later on.
         if !is_rewind {
-            // For fast-forwards, if this tag were to remove an object
-            // that existed before the goto, then we can remove that child right away.
-            // Don't do this for rewinds, because they conceptually
-            // start from an empty display list, and we also want to examine
-            // the old children to decide if they persist (place_frame <= goto_frame).
             let child = self.children.remove(&depth);
             if let Some(child) = child {
                 self.remove_child_from_exec_list(context, child);
             }
         }
+
         Ok(())
     }
 
@@ -3080,6 +3084,11 @@ impl GotoPlaceObject {
         self.place_object.depth.into()
     }
 
+    #[inline]
+    fn ratio(&self) -> u16 {
+        self.place_object.ratio.unwrap_or(0)
+    }
+
     fn merge(&mut self, next: &mut GotoPlaceObject) {
         use swf::PlaceObjectAction;
         let cur_place = &mut self.place_object;