Skip to content
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

Add early return when setting transparent_bg #90055

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

Sauermann
Copy link
Contributor

@Sauermann Sauermann commented Mar 30, 2024

Fix memory leak on Mac.

circumvents #90030

Since I can't reproduce the problem of the linked issue, It would be great, if someone else could test, if the memory leak on MacOs is resolved.

@Sauermann Sauermann added this to the 4.x milestone Mar 30, 2024
@Sauermann Sauermann requested a review from a team as a code owner March 30, 2024 21:39
@BastiaanOlij
Copy link
Contributor

Probably something where we want to dig a little deeper to see why it's leaking. It's still possible we're leaking when changing the background mode and this is just a stopgap.

That said, recreating our buffers just because the background mode is touched without it actually changing seems overkill so this change could be justified for that reason.

@Sauermann
Copy link
Contributor Author

Sauermann commented Mar 31, 2024

Sounds good to me. Lets keep this PR open until after the actual reason for the memory leak is identified. It is very likely, that I will not dig deeper into that topic in the near future.

Edit: There is a MRP available to reproduce the memory leak with this PR merged.

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to do for efficiency reasons.

@clayjohn
Copy link
Member

clayjohn commented Apr 2, 2024

I have a feeling that the memory leak comes from MoltenVK, since it is not leaking on non-Apple devices.

I would go ahead and merge this since it is useful anyway. And then modify the MRP to instead flip the transparent_bg mode each frame

@akien-mga akien-mga merged commit 550f5a5 into godotengine:master Apr 4, 2024
16 checks passed
@Sauermann Sauermann deleted the fix-add-tbg-check branch April 4, 2024 12:47
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants