diff --git a/crates/gpui_macos/src/metal_atlas.rs b/crates/gpui_macos/src/metal_atlas.rs index 5bedf9df8cd..ce5664d778a 100644 --- a/crates/gpui_macos/src/metal_atlas.rs +++ b/crates/gpui_macos/src/metal_atlas.rs @@ -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 { diff --git a/crates/gpui_wgpu/src/wgpu_atlas.rs b/crates/gpui_wgpu/src/wgpu_atlas.rs index 94b4b561c00..3cf71ac98cf 100644 --- a/crates/gpui_wgpu/src/wgpu_atlas.rs +++ b/crates/gpui_wgpu/src/wgpu_atlas.rs @@ -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| { + 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]; diff --git a/crates/gpui_windows/src/directx_atlas.rs b/crates/gpui_windows/src/directx_atlas.rs index a6642dc7dc6..d5b1a674309 100644 --- a/crates/gpui_windows/src/directx_atlas.rs +++ b/crates/gpui_windows/src/directx_atlas.rs @@ -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 { 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 { + let mut device: Option = None; + let mut device_context: Option = 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) -> 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); + } +}