From 41b3eb8b3693b54c26b632727f252576a2e484de Mon Sep 17 00:00:00 2001 From: jarca0123 <11705208+jarca0123@users.noreply.github.com> Date: Mon, 8 Sep 2025 09:20:47 +0200 Subject: [PATCH 1/2] core: Adjust clip removal logic for rewinds Co-authored-by: Mike Welsh Co-authored-by: Kamil Jarosz --- core/src/display_object/movie_clip.rs | 78 ++++++++++++++++++- tests/tests/swfs/avm1/rewind_depth/test.toml | 2 - .../duplicateMovieClip/dontremove/test.toml | 1 - .../timeline/nav/morphShape/test.toml | 1 - .../timeline/nav/ratio2/test.toml | 1 - .../timeline/nav/ratio3/test.toml | 1 - 6 files changed, 77 insertions(+), 7 deletions(-) diff --git a/core/src/display_object/movie_clip.rs b/core/src/display_object/movie_clip.rs index c96c20dc96df..e88d756168bc 100644 --- a/core/src/display_object/movie_clip.rs +++ b/core/src/display_object/movie_clip.rs @@ -1,4 +1,5 @@ //! `MovieClip` display object and support code. +use crate::avm1::globals::AVM_DEPTH_BIAS; use crate::avm1::Avm1; use crate::avm1::{Activation as Avm1Activation, ActivationIdentifier}; use crate::avm1::{NativeObject as Avm1NativeObject, Object as Avm1Object, Value as Avm1Value}; @@ -1562,10 +1563,30 @@ impl<'gc> MovieClip<'gc> { // TODO: We want to do something like self.children.retain here, // but BTreeMap::retain does not exist. // TODO: Should AS3 children ignore GOTOs? + + let final_placements: HashMap> = + goto_commands.iter().map(|cmd| (cmd.depth(), cmd)).collect(); + let children: SmallVec<[_; 16]> = self .iter_render_list() - .filter(|clip| clip.place_frame() > frame) + .filter(|child| { + let is_candidate_for_removal = if self.movie().is_action_script_3() { + child.place_frame() > frame || child.placed_by_script() + } else { + child.depth() < AVM_DEPTH_BIAS + }; + + if !is_candidate_for_removal && child.as_morph_shape().is_none() { + return false; + } + if let Some(final_placement) = final_placements.get(&child.depth()) { + !self.survives_rewind(*child, &final_placement.place_object) + } else { + true + } + }) .collect(); + for child in children { if !child.placed_by_script() { self.remove_child(context, child); @@ -1690,6 +1711,61 @@ impl<'gc> MovieClip<'gc> { self.assert_expected_tag_end(hit_target_frame); } + fn survives_rewind(self, old_object: DisplayObject<'_>, new_params: &swf::PlaceObject) -> bool { + // TODO [KJ] This logic is not 100% tested. It's possible it's a bit + // different in reality, but the spirit is there :) + + if !old_object.movie().is_action_script_3() + && old_object.placed_by_avm1_script() + && old_object.depth() < AVM_DEPTH_BIAS + { + return false; + } + let id_equals = match new_params.action { + swf::PlaceObjectAction::Place(id) | swf::PlaceObjectAction::Replace(id) => { + old_object.id() == id + } + _ => false, + }; + let ratio_equals = match new_params.ratio { + Some(ratio) => old_object.ratio() == ratio, + None => true, + }; + + let clip_depth_equals = match new_params.clip_depth { + Some(clip_depth) => old_object.clip_depth() == clip_depth as Depth, + None => true, + }; + + let color_transform_equals = match new_params.color_transform { + Some(color_transform) => old_object.base().color_transform() == color_transform, + None => true, + }; + + let base_matrix_equals = match new_params.matrix { + Some(matrix) => old_object.base().matrix() == matrix.into(), + None => true, + }; + + match old_object { + DisplayObject::MorphShape(_) | DisplayObject::Graphic(_) | DisplayObject::Text(_) => { + ratio_equals + && id_equals + && clip_depth_equals + && base_matrix_equals + && color_transform_equals + } + DisplayObject::Avm1Button(_) + | DisplayObject::Avm2Button(_) + | DisplayObject::EditText(_) + | DisplayObject::Bitmap(_) + | DisplayObject::Video(_) => ratio_equals && id_equals && clip_depth_equals, + DisplayObject::MovieClip(_) + | DisplayObject::Stage(_) + | DisplayObject::LoaderDisplay(_) => ratio_equals, + } + } + fn construct_as_avm1_object( self, context: &mut UpdateContext<'gc>, diff --git a/tests/tests/swfs/avm1/rewind_depth/test.toml b/tests/tests/swfs/avm1/rewind_depth/test.toml index 32af32d9f0e7..c9072e537432 100644 --- a/tests/tests/swfs/avm1/rewind_depth/test.toml +++ b/tests/tests/swfs/avm1/rewind_depth/test.toml @@ -1,3 +1 @@ num_frames = 6 -# FIXME - Ruffle doesn't handle AVM1 'goto's correctly -known_failure = true diff --git a/tests/tests/swfs/from_shumway/avm1/duplicateMovieClip/dontremove/test.toml b/tests/tests/swfs/from_shumway/avm1/duplicateMovieClip/dontremove/test.toml index 31395f49f345..9c39512ae230 100644 --- a/tests/tests/swfs/from_shumway/avm1/duplicateMovieClip/dontremove/test.toml +++ b/tests/tests/swfs/from_shumway/avm1/duplicateMovieClip/dontremove/test.toml @@ -1,4 +1,3 @@ # Test adapted from Shumway at https://github.com/mozilla/shumway/tree/master/test/swfs/avm1/duplicatemovieclip num_frames = 4 -known_failure = true # https://github.com/ruffle-rs/ruffle/issues/12270 diff --git a/tests/tests/swfs/from_shumway/timeline/nav/morphShape/test.toml b/tests/tests/swfs/from_shumway/timeline/nav/morphShape/test.toml index 04cb8a2f2618..8fc5d1e9a933 100644 --- a/tests/tests/swfs/from_shumway/timeline/nav/morphShape/test.toml +++ b/tests/tests/swfs/from_shumway/timeline/nav/morphShape/test.toml @@ -1,4 +1,3 @@ # Test adapted from Shumway at https://github.com/mozilla/shumway/tree/master/test/swfs/timeline/nav num_frames = 3 -known_failure = true # https://github.com/ruffle-rs/ruffle/issues/12143 diff --git a/tests/tests/swfs/from_shumway/timeline/nav/ratio2/test.toml b/tests/tests/swfs/from_shumway/timeline/nav/ratio2/test.toml index 41a870d0bdb7..8fc5d1e9a933 100644 --- a/tests/tests/swfs/from_shumway/timeline/nav/ratio2/test.toml +++ b/tests/tests/swfs/from_shumway/timeline/nav/ratio2/test.toml @@ -1,4 +1,3 @@ # Test adapted from Shumway at https://github.com/mozilla/shumway/tree/master/test/swfs/timeline/nav num_frames = 3 -known_failure = true # https://github.com/ruffle-rs/ruffle/issues/12144 diff --git a/tests/tests/swfs/from_shumway/timeline/nav/ratio3/test.toml b/tests/tests/swfs/from_shumway/timeline/nav/ratio3/test.toml index fb0caca6f2b4..8fc5d1e9a933 100644 --- a/tests/tests/swfs/from_shumway/timeline/nav/ratio3/test.toml +++ b/tests/tests/swfs/from_shumway/timeline/nav/ratio3/test.toml @@ -1,4 +1,3 @@ # Test adapted from Shumway at https://github.com/mozilla/shumway/tree/master/test/swfs/timeline/nav num_frames = 3 -known_failure = true # https://github.com/ruffle-rs/ruffle/issues/12145 From 00dd269ab81e0b6f674ddf99bdc13c6d19412376 Mon Sep 17 00:00:00 2001 From: Kamil Jarosz Date: Sat, 11 Oct 2025 18:59:26 +0200 Subject: [PATCH 2/2] core: Move rewind-related logic to survives_rewind --- core/src/display_object/movie_clip.rs | 41 ++++++++++++++++----------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/core/src/display_object/movie_clip.rs b/core/src/display_object/movie_clip.rs index e88d756168bc..48c85c47677b 100644 --- a/core/src/display_object/movie_clip.rs +++ b/core/src/display_object/movie_clip.rs @@ -1569,22 +1569,7 @@ impl<'gc> MovieClip<'gc> { let children: SmallVec<[_; 16]> = self .iter_render_list() - .filter(|child| { - let is_candidate_for_removal = if self.movie().is_action_script_3() { - child.place_frame() > frame || child.placed_by_script() - } else { - child.depth() < AVM_DEPTH_BIAS - }; - - if !is_candidate_for_removal && child.as_morph_shape().is_none() { - return false; - } - if let Some(final_placement) = final_placements.get(&child.depth()) { - !self.survives_rewind(*child, &final_placement.place_object) - } else { - true - } - }) + .filter(|child| !self.survives_rewind(*child, &final_placements, frame)) .collect(); for child in children { @@ -1711,22 +1696,44 @@ impl<'gc> MovieClip<'gc> { self.assert_expected_tag_end(hit_target_frame); } - fn survives_rewind(self, old_object: DisplayObject<'_>, new_params: &swf::PlaceObject) -> bool { + fn survives_rewind( + self, + old_object: DisplayObject<'_>, + final_placements: &HashMap>, + frame: FrameNumber, + ) -> bool { // TODO [KJ] This logic is not 100% tested. It's possible it's a bit // different in reality, but the spirit is there :) + let is_candidate_for_removal = if self.movie().is_action_script_3() { + old_object.place_frame() > frame || old_object.placed_by_script() + } else { + old_object.depth() < AVM_DEPTH_BIAS + }; + + if !is_candidate_for_removal && old_object.as_morph_shape().is_none() { + return true; + } + let Some(final_placement) = final_placements.get(&old_object.depth()) else { + return false; + }; + + let new_params = &final_placement.place_object; + if !old_object.movie().is_action_script_3() && old_object.placed_by_avm1_script() && old_object.depth() < AVM_DEPTH_BIAS { return false; } + let id_equals = match new_params.action { swf::PlaceObjectAction::Place(id) | swf::PlaceObjectAction::Replace(id) => { old_object.id() == id } _ => false, }; + let ratio_equals = match new_params.ratio { Some(ratio) => old_object.ratio() == ratio, None => true,