search

memononen / nanovg

Antialiased 2D vector drawing library on top of OpenGL for UI and visualizations.
zlib License
5.15k stars 770 forks source link

Fixes memory leaks caused by full font images. #626

Closed olliwang closed 2 years ago

olliwang commented 2 years ago

This commit fixes the memory leak issue caused by not handling the situation where the number of allocated font images meets NVG_MAX_FONTIMAGES.

In this implementation of handling such situation. All existing font images will be released and a new one with the NVG_MAX_FONTIMAGE_SIZE will be created.

olliwang commented 2 years ago

@memononen Thanks for your hint. After examining the code again. I found that the memory leaks issue is actually caused by not releasing font images properly in nvgEndFrame().

For example, when the NVG_MAX_FONTIMAGES number of font images are allocated all at NVG_MAX_FONTIMAGE_SIZE. If the next frame uses only 2 font images. nvgEndFrame() will set the last two font images to 0 directly without calling nvgDeleteImage().

I've updated to code to initialize all font images to 0 in nvgCreateInternal() as well. Currently this is done only in nvgDeleteInternal(). Also, whenever a font image is deleted in nvgEndFrame(), the image handle resets to 0 immediately. That way, we can make sure the clear all images after j loop not reset image handle without releasing the corresponded image.

olliwang commented 2 years ago

@memononen Yeah, your solution is much better and cleaner. Just updated the code. :)

memononen commented 2 years ago

one more update, then we're good to go :)

olliwang commented 2 years ago

Oops! How can I forget that. Just updated the code. :D

memononen commented 2 years ago

Thank you for the fix!

olliwang commented 2 years ago

Cheers!

nidefawl commented 2 years ago

In my local version I implemented a handler for fonsSetErrorCallback that reallocates the image on error code FONS_ATLAS_FULL. This way I got rid of the fontImages array entirely. And I was also able to remove these checks: https://github.com/memononen/nanovg/blob/397f3300bc14bdb856ec429c71c345ab0340b0a6/src/nanovg.c#L2494-L2505

There was a bit more code that got a lot simpler that way

memononen commented 2 years ago

The reason for the complexity is that some of the backends cache the data for a whole frame. So if you fiddle with the textures there, then you might render garbage.