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.

Sign up for GitHub

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

Jump to bottom

Fix mobile renderer RID leaks #89531

Merged
1 commit merged into from
Mar 24, 2024
Merged

Fix mobile renderer RID leaks #89531

1 commit merged into from
Mar 24, 2024

Conversation

sullyscience
Copy link
Contributor

Update render_forward_mobile destructor to include instance buffers.
Update render_scene_buffers_rd cleanup to include weight buffers / blur textures.

Fixes: #89182

@sullyscience sullyscience requested a review from a team as a code owner March 15, 2024 18:34
@AThousandShips AThousandShips changed the title Fix: Address mobile resource leaks in render Fix mobile renderer RID leaks Mar 15, 2024
@AThousandShips AThousandShips added this to the 4.3 milestone Mar 15, 2024
@AThousandShips AThousandShips added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Mar 15, 2024
@sullyscience
Fix: Address mobile resource leaks in render
dfa326e 
Update render_forward_mobile destructor to include instance buffers.
Update render_scene_buffers_rd cleanup to include weight buffers / blur
textures.
clayjohn
clayjohn approved these changes Mar 15, 2024
View reviewed changes
Copy link
Member

@clayjohn clayjohn left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great work! Thank you for fixing this

Code changes look good to me, given the nature of the changes, I trust that they are good given that they are passing CI

This should be an easy cherrypick too

@BalmundSM BalmundSM mentioned this pull request Mar 18, 2024
Closed
akien-mga added a commit that referenced this pull request Mar 24, 2024
@akien-mga
Merge pull request #89531 from sullyscience/fix-resource-leaks
70c05f6 
Fix mobile renderer RID leaks
@akien-mga akien-mga closed this pull request by merging all changes into godotengine:master in 06abc86 Mar 24, 2024
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@akien-mga
Copy link
Member

Cherry-picked for 4.2.2.

@akien-mga akien-mga removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Apr 8, 2024
@chrisl8 chrisl8 mentioned this pull request Apr 9, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BalmundSM BalmundSM BalmundSM left review comments

@clayjohn clayjohn clayjohn approved these changes

@Geometror Geometror Geometror approved these changes

@AThousandShips AThousandShips Awaiting requested review from AThousandShips

Assignees
No one assigned
Labels
bug topic:rendering
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.

Vulkan Mobile leaks RIDs on exit even in empty project (StorageBuffer, Framebuffer, Texture)
6 participants
@sullyscience @akien-mga @clayjohn @Geometror @AThousandShips @BalmundSM