mirror of
https://github.com/zed-industries/zed.git
synced 2026-07-10 00:13:29 +00:00
gpui: Free atlas tile space when removing tiles (#58874)
Partially addresses #54659 `PlatformAtlas::remove` removed the tile from `tiles_by_key` and decremented the texture's refcount, but never returned the tile's rectangle to the etagere allocator. Removed tiles' space could only be reclaimed if the entire texture was dropped, which doesn't happen if glyph/emoji share the texture. Because we don't have correct glyph/emoji removal I added regression tests on all three platforms to prevent this from occurring as well. Note: this doesn't fix glyph eviction or the nil-texture panic from #54659; those are follow ups. Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Closes #ISSUE Release Notes: - gpui: Fix GPUI memory growing unboundedly when images are repeatedly loaded and released
This commit is contained in:
parent
dd3521df2b
commit
0d8a4d4292
3 changed files with 161 additions and 3 deletions
|
|
@ -61,9 +61,10 @@ impl PlatformAtlas for MetalAtlas {
|
|||
|
||||
fn remove(&self, key: &AtlasKey) {
|
||||
let mut lock = self.0.lock();
|
||||
let Some(id) = lock.tiles_by_key.remove(key).map(|v| v.texture_id) else {
|
||||
let Some(tile) = lock.tiles_by_key.remove(key) else {
|
||||
return;
|
||||
};
|
||||
let id = tile.texture_id;
|
||||
|
||||
let textures = match id.kind {
|
||||
AtlasTextureKind::Monochrome => &mut lock.monochrome_textures,
|
||||
|
|
@ -80,6 +81,7 @@ impl PlatformAtlas for MetalAtlas {
|
|||
};
|
||||
|
||||
if let Some(mut texture) = texture_slot.take() {
|
||||
texture.allocator.deallocate(tile.tile_id.into());
|
||||
texture.decrement_ref_count();
|
||||
if texture.is_unreferenced() {
|
||||
textures.free_list.push(id.index as usize);
|
||||
|
|
@ -338,6 +340,34 @@ mod tests {
|
|||
let _texture = atlas.metal_texture(tile_a2.texture_id);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_remove_deallocates_tile_space_for_reuse() {
|
||||
let Some(atlas) = create_atlas() else {
|
||||
return;
|
||||
};
|
||||
|
||||
let small = Size {
|
||||
width: DevicePixels(64),
|
||||
height: DevicePixels(64),
|
||||
};
|
||||
let big = Size {
|
||||
width: DevicePixels(700),
|
||||
height: DevicePixels(700),
|
||||
};
|
||||
|
||||
let keeper_key = make_image_key(1, 0);
|
||||
let big_key_a = make_image_key(2, 0);
|
||||
let big_key_b = make_image_key(3, 0);
|
||||
|
||||
let keeper_tile = insert_tile(&atlas, &keeper_key, small);
|
||||
let tile_a = insert_tile(&atlas, &big_key_a, big);
|
||||
assert_eq!(keeper_tile.texture_id, tile_a.texture_id);
|
||||
|
||||
atlas.remove(&big_key_a);
|
||||
let tile_b = insert_tile(&atlas, &big_key_b, big);
|
||||
assert_eq!(tile_b.texture_id, keeper_tile.texture_id);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_remove_nonexistent_key_is_noop() {
|
||||
let Some(atlas) = create_atlas() else {
|
||||
|
|
|
|||
|
|
@ -130,15 +130,17 @@ impl PlatformAtlas for WgpuAtlas {
|
|||
fn remove(&self, key: &AtlasKey) {
|
||||
let mut lock = self.0.lock();
|
||||
|
||||
let Some(id) = lock.tiles_by_key.remove(key).map(|tile| tile.texture_id) else {
|
||||
let Some(tile) = lock.tiles_by_key.remove(key) else {
|
||||
return;
|
||||
};
|
||||
let id = tile.texture_id;
|
||||
|
||||
let Some(texture_slot) = lock.storage[id.kind].textures.get_mut(id.index as usize) else {
|
||||
return;
|
||||
};
|
||||
|
||||
if let Some(mut texture) = texture_slot.take() {
|
||||
texture.allocator.deallocate(tile.tile_id.into());
|
||||
texture.decrement_ref_count();
|
||||
if texture.is_unreferenced() {
|
||||
lock.pending_uploads
|
||||
|
|
@ -461,6 +463,50 @@ mod tests {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remove_deallocates_tile_space_for_reuse() -> anyhow::Result<()> {
|
||||
let (device, queue) = test_device_and_queue()?;
|
||||
let atlas = WgpuAtlas::new(device, queue, wgpu::TextureFormat::Bgra8Unorm);
|
||||
|
||||
let small = Size {
|
||||
width: DevicePixels(64),
|
||||
height: DevicePixels(64),
|
||||
};
|
||||
let big = Size {
|
||||
width: DevicePixels(700),
|
||||
height: DevicePixels(700),
|
||||
};
|
||||
|
||||
let make_key = |image_id: usize| {
|
||||
AtlasKey::Image(RenderImageParams {
|
||||
image_id: ImageId(image_id),
|
||||
frame_index: 0,
|
||||
})
|
||||
};
|
||||
let insert = |key: &AtlasKey, size: Size<DevicePixels>| {
|
||||
let byte_count = (size.width.0 as usize) * (size.height.0 as usize) * 4;
|
||||
atlas
|
||||
.get_or_insert_with(key, &mut || {
|
||||
Ok(Some((size, Cow::Owned(vec![0u8; byte_count]))))
|
||||
})
|
||||
.expect("allocation should succeed")
|
||||
.expect("callback returns Some")
|
||||
};
|
||||
|
||||
let keeper_key = make_key(1);
|
||||
let big_key_a = make_key(2);
|
||||
let big_key_b = make_key(3);
|
||||
|
||||
let keeper_tile = insert(&keeper_key, small);
|
||||
let tile_a = insert(&big_key_a, big);
|
||||
assert_eq!(keeper_tile.texture_id, tile_a.texture_id);
|
||||
|
||||
atlas.remove(&big_key_a);
|
||||
let tile_b = insert(&big_key_b, big);
|
||||
assert_eq!(tile_b.texture_id, keeper_tile.texture_id);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn swizzle_upload_data_preserves_bgra_uploads() {
|
||||
let input = vec![0x10, 0x20, 0x30, 0x40];
|
||||
|
|
|
|||
|
|
@ -98,9 +98,10 @@ impl PlatformAtlas for DirectXAtlas {
|
|||
fn remove(&self, key: &AtlasKey) {
|
||||
let mut lock = self.0.lock();
|
||||
|
||||
let Some(id) = lock.tiles_by_key.remove(key).map(|tile| tile.texture_id) else {
|
||||
let Some(tile) = lock.tiles_by_key.remove(key) else {
|
||||
return;
|
||||
};
|
||||
let id = tile.texture_id;
|
||||
|
||||
let textures = match id.kind {
|
||||
AtlasTextureKind::Monochrome => &mut lock.monochrome_textures,
|
||||
|
|
@ -113,6 +114,7 @@ impl PlatformAtlas for DirectXAtlas {
|
|||
};
|
||||
|
||||
if let Some(mut texture) = texture_slot.take() {
|
||||
texture.allocator.deallocate(tile.tile_id.into());
|
||||
texture.decrement_ref_count();
|
||||
if texture.is_unreferenced() {
|
||||
textures.free_list.push(texture.id.index as usize);
|
||||
|
|
@ -318,3 +320,83 @@ fn etagere_point_to_device(value: etagere::Point) -> Point<DevicePixels> {
|
|||
y: DevicePixels::from(value.y),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use gpui::{ImageId, RenderImageParams};
|
||||
use std::borrow::Cow;
|
||||
use windows::Win32::{
|
||||
Foundation::HMODULE,
|
||||
Graphics::{
|
||||
Direct3D::D3D_DRIVER_TYPE_WARP,
|
||||
Direct3D11::{D3D11_CREATE_DEVICE_BGRA_SUPPORT, D3D11_SDK_VERSION, D3D11CreateDevice},
|
||||
},
|
||||
};
|
||||
|
||||
fn create_atlas() -> Option<DirectXAtlas> {
|
||||
let mut device: Option<ID3D11Device> = None;
|
||||
let mut device_context: Option<ID3D11DeviceContext> = None;
|
||||
unsafe {
|
||||
D3D11CreateDevice(
|
||||
None,
|
||||
D3D_DRIVER_TYPE_WARP,
|
||||
HMODULE::default(),
|
||||
D3D11_CREATE_DEVICE_BGRA_SUPPORT,
|
||||
None,
|
||||
D3D11_SDK_VERSION,
|
||||
Some(&mut device),
|
||||
None,
|
||||
Some(&mut device_context),
|
||||
)
|
||||
}
|
||||
.ok()?;
|
||||
Some(DirectXAtlas::new(&device?, &device_context?))
|
||||
}
|
||||
|
||||
fn make_image_key(image_id: usize) -> AtlasKey {
|
||||
AtlasKey::Image(RenderImageParams {
|
||||
image_id: ImageId(image_id),
|
||||
frame_index: 0,
|
||||
})
|
||||
}
|
||||
|
||||
fn insert_tile(atlas: &DirectXAtlas, key: &AtlasKey, size: Size<DevicePixels>) -> AtlasTile {
|
||||
atlas
|
||||
.get_or_insert_with(key, &mut || {
|
||||
let byte_count = (size.width.0 as usize) * (size.height.0 as usize) * 4;
|
||||
Ok(Some((size, Cow::Owned(vec![0u8; byte_count]))))
|
||||
})
|
||||
.expect("allocation should succeed")
|
||||
.expect("callback returns Some")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_remove_deallocates_tile_space_for_reuse() {
|
||||
let Some(atlas) = create_atlas() else {
|
||||
return;
|
||||
};
|
||||
|
||||
let small = Size {
|
||||
width: DevicePixels(64),
|
||||
height: DevicePixels(64),
|
||||
};
|
||||
let big = Size {
|
||||
width: DevicePixels(700),
|
||||
height: DevicePixels(700),
|
||||
};
|
||||
|
||||
let keeper_key = make_image_key(1);
|
||||
let big_key_a = make_image_key(2);
|
||||
let big_key_b = make_image_key(3);
|
||||
|
||||
let keeper_tile = insert_tile(&atlas, &keeper_key, small);
|
||||
let tile_a = insert_tile(&atlas, &big_key_a, big);
|
||||
assert_eq!(keeper_tile.texture_id, tile_a.texture_id);
|
||||
|
||||
atlas.remove(&big_key_a);
|
||||
|
||||
let tile_b = insert_tile(&atlas, &big_key_b, big);
|
||||
assert_eq!(tile_b.texture_id, keeper_tile.texture_id);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue