avm1: Retrieving the child of a container should prioritise lowest depth

This commit is contained in:
CUB3D 2022-12-20 19:45:41 +00:00 committed by kmeisthax
parent a356be15fe
commit 2d11a250da
3 changed files with 83 additions and 16 deletions

View File

@ -430,6 +430,11 @@ impl<'gc> Avm1<'gc> {
// Remove it // Remove it
parent_container.remove_child_directly(context, child); parent_container.remove_child_directly(context, child);
// Update pending removal state
parent_container
.raw_container_mut(context.gc_context)
.update_pending_removals();
} }
} }

View File

@ -321,7 +321,20 @@ pub trait TDisplayObjectContainer<'gc>:
}; };
if should_delay_removal { 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); 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; return;
} }
} }
@ -524,6 +537,16 @@ pub struct ChildContainer<'gc> {
/// exclusively with the depth list. However, AS3 instead references clips /// exclusively with the depth list. However, AS3 instead references clips
/// by render list indexes and does not manipulate the depth list. /// by render list indexes and does not manipulate the depth list.
depth_list: BTreeMap<Depth, DisplayObject<'gc>>, depth_list: BTreeMap<Depth, DisplayObject<'gc>>,
/// 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> { impl<'gc> Default for ChildContainer<'gc> {
@ -537,6 +560,7 @@ impl<'gc> ChildContainer<'gc> {
Self { Self {
render_list: Vec::new(), render_list: Vec::new(),
depth_list: BTreeMap::new(), depth_list: BTreeMap::new(),
has_pending_removals: false,
} }
} }
@ -623,17 +647,21 @@ impl<'gc> ChildContainer<'gc> {
.next(); .next();
if let Some(above_child) = above { if let Some(above_child) = above {
let position = self if let Some(position) = self
.render_list .render_list
.iter() .iter()
.position(|x| DisplayObject::ptr_eq(*x, above_child)) .position(|x| DisplayObject::ptr_eq(*x, above_child))
.unwrap(); {
self.insert_id(position, child); self.insert_id(position, child);
None None
} else { } else {
self.push_id(child); self.push_id(child);
None None
} }
} else {
self.push_id(child);
None
}
} }
} }
@ -660,6 +688,29 @@ impl<'gc> ChildContainer<'gc> {
/// If multiple children with the same name exist, the one that occurs /// If multiple children with the same name exist, the one that occurs
/// first in the render list wins. /// first in the render list wins.
fn get_name(&self, name: &WStr, case_sensitive: bool) -> Option<DisplayObject<'gc>> { fn get_name(&self, name: &WStr, case_sensitive: bool) -> Option<DisplayObject<'gc>> {
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::<Vec<_>>()
} else {
self.depth_list
.iter()
.filter(|(_, child)| child.name().eq_ignore_case(name))
.collect::<Vec<_>>()
};
// 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 {
// TODO: Make a HashMap from name -> child? // TODO: Make a HashMap from name -> child?
// But need to handle conflicting names (lowest in depth order takes priority). // But need to handle conflicting names (lowest in depth order takes priority).
if case_sensitive { if case_sensitive {
@ -674,6 +725,7 @@ impl<'gc> ChildContainer<'gc> {
.find(|child| child.name().eq_ignore_case(name)) .find(|child| child.name().eq_ignore_case(name))
} }
} }
}
/// Get a child by it's render list position (ID). /// Get a child by it's render list position (ID).
fn get_id(&self, id: usize) -> Option<DisplayObject<'gc>> { fn get_id(&self, id: usize) -> Option<DisplayObject<'gc>> {
@ -808,6 +860,16 @@ impl<'gc> ChildContainer<'gc> {
self.render_list.iter().copied() 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 /// Should the removal of this clip be delayed to the start of the next frame
/// ///
/// Checks recursively for unload handlers /// Checks recursively for unload handlers
@ -821,7 +883,7 @@ impl<'gc> ChildContainer<'gc> {
if mc.has_unload_handler() { if mc.has_unload_handler() {
return true; return true;
// If we were created via timeline and we have a dynamic unload handler, we need the delay // 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); let obj = child.object().coerce_to_object(activation);
if obj.has_property(activation, "onUnload".into()) { if obj.has_property(activation, "onUnload".into()) {
return true; return true;