Skip to content

fix ChildContainer.depth_list child removal when child's depth changed #10405

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

emgfc
Copy link
Contributor

@emgfc emgfc commented Mar 27, 2023

This is a follow up on #10362

I don't think this should actually be merged as is, just wanted to describe somewhere what's going on.

Ruffle has some sort of a leak in delayed DisplayObject child removing. When a child removing is delayed, that child object (and all of its children recursively) is assigned a negative depth and queued for removing in ChildContainer::queue_removal() func. Though there's a logic for updating that child element's position in depth_list (removing old ones, inserting new ones), it's not recursive, so nested children objects are having negative depths while being stored in depth_list with their previous positive depths as a keys. And this is the reason why we have lookup errors with that depth_list a bit later.

An easy and obvious fix for that would be implementing a recursive ChildContainer::remove_child_from_depth_list() and ChildContainer::insert_child_into_depth_list(). And yes, I did it, and everything just became worse when playing that slimy yellow sponge game.

I believe someone smarter than me could actually fix it the right way.

@danielhjacobs danielhjacobs added A-core Area: Core player, where no other category fits T-fix Type: Bug fix (in something that's supposed to work already) labels Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Core player, where no other category fits T-fix Type: Bug fix (in something that's supposed to work already)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants