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

Texture2D memory leak #635

Open
sloganking opened this issue Aug 19, 2023 · 10 comments
Open

Texture2D memory leak #635

sloganking opened this issue Aug 19, 2023 · 10 comments

Comments

@sloganking
Copy link
Contributor

sloganking commented Aug 19, 2023

In macroquad v4.3.23 Texture2D types are never removed from memory.

Memory steadily grows until system crash when repeatedly converting an Image into a Texture2D in a loop. Even though the Texture2D is dropped at the end of each loop.

image

Memory does not grow when the macroquad program is closed or my Images are never converted to Texture2Ds
image

Per #421 (comment), is this issue fixed but just not pushed to a crates.io release yet?

@Afrovis
Copy link

Afrovis commented Nov 5, 2023

I'm having an issue that I believe is related when calling get_screen_data() in my render loop (for image saving per frame). Not fixed in 0.4.4 yet.

@eboatwright
Copy link
Contributor

eboatwright commented Dec 22, 2023

This is a problem in my project as well, I'm making a pixel art game, but I still want the window resizable, so every frame I'm creating a render_target and a Camera2D every frame, with the current resolution.
But yeah, while running the program my memory usage slowly increases, and eventually my computer crashes (On Ubuntu 22.04, using the latest version "0.4.4")

@mandroll
Copy link
Contributor

mandroll commented Feb 2, 2024

I also have this issue. Here is a minimal reproducer:

use macroquad::prelude::*;

#[macroquad::main("Texture test")]
async fn main() {
    let texture_size: u32= 64 * 16;
    loop {
        let render_target = render_target(texture_size, texture_size);
        next_frame().await;
    }
}

Another user on Mac claims the issue does not happen for them, suggesting it may be platform-specific. In my case, I am on Windows 10.

@mandroll
Copy link
Contributor

mandroll commented Feb 2, 2024

I can confirm the bug is still present on HEAD. I have tested with:

[package]
name = "texture-memory"
version = "0.1.0"
edition = "2021"

[dependencies]
macroquad = { git = "https://github.com/not-fl3/macroquad.git", branch = "master" }

@mandroll
Copy link
Contributor

mandroll commented Feb 2, 2024

Here is a dhat heap profile of the above program: https://gist.github.com/mandroll/02b19220221bc81a1f83d2d299914a32

@ElnuDev
Copy link

ElnuDev commented Apr 11, 2024

I also have this issue. Here is a minimal reproducer:

use macroquad::prelude::*;

#[macroquad::main("Texture test")]
async fn main() {
    let texture_size: u32= 64 * 16;
    loop {
        let render_target = render_target(texture_size, texture_size);
        next_frame().await;
    }
}

Another user on Mac claims the issue does not happen for them, suggesting it may be platform-specific. In my case, I am on Windows 10.

I'm getting this exact same problem with initializing a RenderTarget every frame due to the wrapped Texture2D. This is a really serious issue because it will always cause crashes due to running out of memory. Is there any news on this? @not-fl3

@not-fl3
Copy link
Owner

not-fl3 commented Apr 11, 2024

Despite the bug, allocating textures and especially render targets each frame would never work really well. Its a really expensive procedure and there are so many things that could go wrong with that approach.

Not like this bug is not worth to be fixed or anything, just a side note :)

@ElnuDev
Copy link

ElnuDev commented Apr 11, 2024

Thanks! I made a mistake in my implementation originally that gave me the false impression that RenderTargets can't be reused, now I'm reusing the same RenderTarget and the memory is leak is of course gone. That being said, this summer when I have the time, I might look into this bug and see if I can find a fix.

@eboatwright
Copy link
Contributor

Despite the bug, allocating textures and especially render targets each frame would never work really well. Its a really expensive procedure and there are so many things that could go wrong with that approach.

Not like this bug is not worth to be fixed or anything, just a side note :)

Is there a way to resize a previously created RenderTarget?

profan added a commit to profan/macroquad that referenced this issue Apr 30, 2024
* Fix issue with Audio leaking memory when loaded by adding a drop impl, reported in issue not-fl3#628
* Fix issue with Font Atlas leaking memory by adding a drop impl, reported in issue not-fl3#627
* Fix issue with RenderTarget leaking memory by adding a drop impl, reported in issue not-fl3#635
profan added a commit to profan/macroquad that referenced this issue Apr 30, 2024
* Fix issue with Font Atlas leaking memory by adding a drop impl, reported in issue not-fl3#627
* Fix issue with RenderTarget leaking memory by adding a drop impl, reported in issue not-fl3#635
profan added a commit to profan/macroquad that referenced this issue Apr 30, 2024
* Fix issue with Font Atlas leaking memory by adding a drop impl, reported in issue not-fl3#627
* Fix issue with RenderTarget leaking memory by adding a drop impl, reported in issue not-fl3#635
profan added a commit to profan/macroquad that referenced this issue May 2, 2024
* Fix issue with RenderTarget leaking memory by adding a drop impl, reported in issue not-fl3#635
not-fl3 pushed a commit that referenced this issue May 3, 2024
* Fix issue with RenderTarget leaking memory by adding a drop impl, reported in issue #635
@InnocentusLime
Copy link
Contributor

It doesn't seem to leak from just loading the textures now. However, it seems the leaks still happens when build_texture_atlas() is involved.

I believe that is because the texture atlas ends up holding strong references to the textures and never gets garbage-collected

#[macroquad::main("Leak?")]
async fn main() {
    loop {
        let t = load_texture("texture.png").await.unwrap();

        build_textures_atlas();

        next_frame().await;
    }
}

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

No branches or pull requests

7 participants