From 2d11a250da4113720268878a6367597044ec62d9 Mon Sep 17 00:00:00 2001 From: CUB3D Date: Tue, 20 Dec 2022 19:45:41 +0000 Subject: [PATCH] avm1: Retrieving the child of a container should prioritise lowest depth --- core/src/avm1/runtime.rs | 5 ++ core/src/display_object/container.rs | 94 +++++++++++++++++++++----- tests/tests/swfs/avm1/unload/test.swf | Bin 1138 -> 1136 bytes 3 files changed, 83 insertions(+), 16 deletions(-) diff --git a/core/src/avm1/runtime.rs b/core/src/avm1/runtime.rs index 8ec8cffa5..624ade991 100644 --- a/core/src/avm1/runtime.rs +++ b/core/src/avm1/runtime.rs @@ -430,6 +430,11 @@ impl<'gc> Avm1<'gc> { // Remove it parent_container.remove_child_directly(context, child); + + // Update pending removal state + parent_container + .raw_container_mut(context.gc_context) + .update_pending_removals(); } } diff --git a/core/src/display_object/container.rs b/core/src/display_object/container.rs index 35e439148..867f5339a 100644 --- a/core/src/display_object/container.rs +++ b/core/src/display_object/container.rs @@ -321,7 +321,20 @@ pub trait TDisplayObjectContainer<'gc>: }; if should_delay_removal { + let mut raw_container = self.raw_container_mut(context.gc_context); + + // Remove the child from the depth list, before moving it to a negative depth + raw_container.remove_child_from_depth_list(child); + + // Enqueue for removal ChildContainer::queue_removal(child, context); + + // Mark that we have a pending removal + raw_container.set_pending_removals(true); + + // Re-Insert the child at the new depth + raw_container.insert_child_into_depth_list(child.depth(), child); + return; } } @@ -524,6 +537,16 @@ pub struct ChildContainer<'gc> { /// exclusively with the depth list. However, AS3 instead references clips /// by render list indexes and does not manipulate the depth list. depth_list: BTreeMap>, + + /// Does this container have any AVM1 objects that are pending removal + /// + /// Objects that are pending removal are placed at a negative depth in the depth list, + /// because accessing children exclusively interacts with the render list, which cannot handle + /// negative render depths, we need to check both lists when we have something pending removal. + /// + /// This should be more efficient than switching the render list to a `BTreeMap`, + /// as it will usually be false + has_pending_removals: bool, } impl<'gc> Default for ChildContainer<'gc> { @@ -537,6 +560,7 @@ impl<'gc> ChildContainer<'gc> { Self { render_list: Vec::new(), depth_list: BTreeMap::new(), + has_pending_removals: false, } } @@ -623,13 +647,17 @@ impl<'gc> ChildContainer<'gc> { .next(); if let Some(above_child) = above { - let position = self + if let Some(position) = self .render_list .iter() .position(|x| DisplayObject::ptr_eq(*x, above_child)) - .unwrap(); - self.insert_id(position, child); - None + { + self.insert_id(position, child); + None + } else { + self.push_id(child); + None + } } else { self.push_id(child); None @@ -660,18 +688,42 @@ impl<'gc> ChildContainer<'gc> { /// If multiple children with the same name exist, the one that occurs /// first in the render list wins. fn get_name(&self, name: &WStr, case_sensitive: bool) -> Option> { - // TODO: Make a HashMap from name -> child? - // But need to handle conflicting names (lowest in depth order takes priority). - if case_sensitive { - self.render_list - .iter() - .copied() - .find(|child| child.name() == name) + if self.has_pending_removals { + // Find matching children by searching the depth list + let mut matching_render_children = if case_sensitive { + self.depth_list + .iter() + .filter(|(_, child)| child.name() == name) + .collect::>() + } else { + self.depth_list + .iter() + .filter(|(_, child)| child.name().eq_ignore_case(name)) + .collect::>() + }; + + // Sort so we can get the lowest depth child + matching_render_children.sort_by_key(|&(depth, _child)| *depth); + + // First child will have the lowest depth + return matching_render_children + .first() + .map(|&(_depth, child)| child) + .copied(); } else { - self.render_list - .iter() - .copied() - .find(|child| child.name().eq_ignore_case(name)) + // TODO: Make a HashMap from name -> child? + // But need to handle conflicting names (lowest in depth order takes priority). + if case_sensitive { + self.render_list + .iter() + .copied() + .find(|child| child.name() == name) + } else { + self.render_list + .iter() + .copied() + .find(|child| child.name().eq_ignore_case(name)) + } } } @@ -808,6 +860,16 @@ impl<'gc> ChildContainer<'gc> { self.render_list.iter().copied() } + /// Check for pending removals and update the pending removals flag + pub fn update_pending_removals(&mut self) { + self.has_pending_removals = self.depth_list.values().any(|c| c.pending_removal()); + } + + /// Set the pending_removals flag + pub fn set_pending_removals(&mut self, pending: bool) { + self.has_pending_removals = pending; + } + /// Should the removal of this clip be delayed to the start of the next frame /// /// Checks recursively for unload handlers @@ -821,7 +883,7 @@ impl<'gc> ChildContainer<'gc> { if mc.has_unload_handler() { return true; // If we were created via timeline and we have a dynamic unload handler, we need the delay - } else if !child.placed_by_script() { + } else if child.instantiated_by_timeline() { let obj = child.object().coerce_to_object(activation); if obj.has_property(activation, "onUnload".into()) { return true; diff --git a/tests/tests/swfs/avm1/unload/test.swf b/tests/tests/swfs/avm1/unload/test.swf index b154161c3f18bb145edda1d080a0718741be8130..3571e4d5e9c2b15ebfc1c340e5d8b846f8c94c72 100644 GIT binary patch literal 1136 zcmV-$1dsbeS5prK3jhFkoV8U;ZyQw_Ua&vG^`t65wjQB0+&xD95*vFgZm zF}%`bN)Iwjg4u29O^p$Ae1Scic?t{3_$dmUz$JJ-!R0r3UP|Yr6n@W19Cs?$kMvWk z$Wt`MQ+mllf)^43x5?)vQQY>EhGK#=OVjHImnID&m;!rdaxXl~YW4DI8V1{ocD)*^ zUmopl9@vPjA=hd-D#GCl!D&>h(wgadim9SCxrhx(UDqUjHJvGC72}!vMU);5X!4O zzp|PuuaruH$a5tzlj)0oF%++;*mhG_6#~e15^lp%AGVc!l&F*2qQDLU&n<}!F1;cHrEs^~eFxu% zi~Qoo_G0Vamiqqod!C%!*xFLuceRyTyUoJ)Q{OHAU4P@@C+lDQ{LR|Qx7UBWGd39p zc>E*GLd8)G#PAZRDexM)3wy|0M;-5;zSlsmCnMWyfZEjU6sYu~DI2zTP__1TRKiwJ zMd}s7GPlg8rD)(UjBh0}ecF_!CT5ADD;3F|4$*fyR1XSyIC-fYyK(<$e6EK%DOK zJW4R*V>o;6pOX(i^2rcbBgFdUpNGpnnU*0O!-25~+!;H;{>KK~2skagEYniMd=m$( z^Y81!YB`OvZ6*A>4WaxAqko=i7%L zSigqO4)jej(3#PJ;sX7VE>oP55v8tvM5(JE(K2rAhvCM=t8Yxa^2TEHTlyC(ROcwl Ce=pVm literal 1138 zcmV-&1daPcS5prQ3jhFkoV8U?ZyQAvf3{=00n#*WAOc0ON+RFv-C5gfd*!%E{HHC# zY2~D-st{qbJB}CZ-OcXWaSo`6L)1erz2pK1E~Fm-2@XZ-SD;875Em{8gv2*MGBdMl zJC2i3ielMs_Wk?KdvAU_?m~DAVEQY7zyy?Lt^mM;YmqQO-qz}JtyJNZZX_UD-C< z?()Kd58G{716adJD4MyA6RO$byNbo5guqiA5+oOGthlDVX`0Q#DvjbQO~q+&cWkqc z9f!axO|G~hadB?Dtv59;l|^~}Wacp}Tq3`MD2PHFp}3ISL`Y6&ykQWR=TFSksjCVFo*24+BpviTAI*0tdxluh)Br z+=@$RsbehN+_=BHn?u-Hzr)|t-)(G2>aLrBAErYee)s##(@(44eE;!#`|sbqcRe%} z1bFxp%t6IgT9`vJs7Y`ex&u4dUB_MbroPj_j$6W(+W@txTS-vabyIFx?tazm=(tF% zpr)8N1*UP=Xqt)!-oklSD$~bJX=dVV!tIIU8?-RXk`)aEHkfskdZkZiC zETe2_T-|F04ufzBVpIkWCm{mR^BNUX^EX(zmpz5b>2=R^g^C&9f0dQ(E7w>tzB23WLX;DgPcXFC zYjYjdX?KjEyLH@gquJ{D@XaV?T~Imb&=HQWxG>l)X#;0BdsQ E^