Kirill Bulatov created
This reverts commit 94faaebfec5e46afa6d3aee5372cbbdb33b97c33.
The logic changed in the [original
PR](https://github.com/zed-industries/zed/pull/45969) is either
misplaced or lacks a counterpart that reacts on `gpui::Image` drop one
way or another.
The change was dropping the texture out of the global rendering atlas
when an image preview tab was disposed of, while in reality, another
instance (agent panel or another image preview tab) could require the
very same atlas entry to be rendered meanwhile.
Currently, only `RetainAllImageCache` in Zed does any kind of image
cleanup, and any other image usages will leak memory.
What makes it harder is that the atlas entry needs to live as long as a
particular `Arc<Image>` lives, and some public `gpui` API expects this
type to stay:
https://github.com/zed-industries/zed/blob/e747cfc955e8cfd9327d8d6b8d617cf1d3a6bcaa/crates/gpui/src/platform.rs#L1851-L1866
For image viewer in particular, we need to consider why
`RetainAllImageCache` is not used there; for the global, normal fix, we
need to consider ways to have `cx` and `window` and a way to react on
`Image` drop.
As an option, we could break the `gpui` API (as it [happens
periodically](https://github.com/zed-industries/zed/issues/46183)
already) and use `Entity<Image>` instead of `Arc`, then react with
`cx.on_release_in` for each such image.
Closes https://github.com/zed-industries/zed/issues/46755
Closes https://github.com/zed-industries/zed/issues/46435
Release Notes:
- Fixed panics on concurrent image handlingcrates/image_viewer/src/image_viewer.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -44,7 +44,7 @@ impl ImageView {
cx.on_release_in(window, |this, window, cx| {
let image_data = this.image_item.read(cx).image.clone();
if let Some(image) = image_data.clone().get_render_image(window, cx) {
- cx.drop_image(image, Some(window));
+ cx.drop_image(image, None);
}
image_data.remove_asset(cx);
})