Fix memory leak when forget_image is called while loading#7380
Fix memory leak when forget_image is called while loading#7380emilk merged 2 commits intoemilk:mainfrom
forget_image is called while loading#7380Conversation
|
Preview available at https://egui-pr-preview.github.io/pr/7380-image-loader-assertion |
…ing it This is the same issue that was fixed for the http bytes loader in 239ade9. However the mentioned fix was wrong as it kept the loaded data in the cache even in the case where the Pending cache entry was removed while loading (i.e. the image was forgotten). Instead, use the same code as ab9f55a (file loader) but for the ehttp and image loaders.
|
Actually I suspect that what 239ade9 (#3750) wanted to achieve is kind of wrong in the case where we In the case of my application, it means that instead of crashing on the assertion the memory just looks looks like it's getting leaked a lot (it's trivial for me to reach 5GB RAM by just scrolling sufficiently fast that few images have even time to load fully before I suspect that the WebP/SVG/GIF loaders do not have any of those issues as they do not use a background thread for loading (I'm not quite sure why that's something each ImageLoader implementation should care about themselves but that's another topic). |
7c2ff9f to
7878153
Compare
|
Now the ehttp and image loaders have the same correct behavior in case of |
forget_image is called while loading
This is the same issue that was fixed for the http bytes loader in 239ade9 * [x] I have followed the instructions in the PR template ---------------- Funnily, [this comment](#3747 (comment)) describes exactly how I encountered this issue: > That assert is wrong if something calls forget between the start of the request and the end of it. I'm displaying lots of images in a scrolling grid (20 or so visible at a time). It seems like image textures are never freed up automatically so it stacks up a lot meaning I have to unload the image textures manually with `egui::Context::forget_image()` in each `eframe::App::update()` call for the images that are no longer shown when scrolling. --------- Co-authored-by: Emil Ernerfeldt <[email protected]>
This is the same issue that was fixed for the http bytes loader in 239ade9 * [x] I have followed the instructions in the PR template ---------------- Funnily, [this comment](emilk#3747 (comment)) describes exactly how I encountered this issue: > That assert is wrong if something calls forget between the start of the request and the end of it. I'm displaying lots of images in a scrolling grid (20 or so visible at a time). It seems like image textures are never freed up automatically so it stacks up a lot meaning I have to unload the image textures manually with `egui::Context::forget_image()` in each `eframe::App::update()` call for the images that are no longer shown when scrolling. --------- Co-authored-by: Emil Ernerfeldt <[email protected]>
This is the same issue that was fixed for the http bytes loader in 239ade9
Funnily, this comment describes exactly how I encountered this issue:
I'm displaying lots of images in a scrolling grid (20 or so visible at a time). It seems like image textures are never freed up automatically so it stacks up a lot meaning I have to unload the image textures manually with
egui::Context::forget_image()in eacheframe::App::update()call for the images that are no longer shown when scrolling.