diff --git a/crates/gpui/src/text_system.rs b/crates/gpui/src/text_system.rs index 593c3b8668e..1eea420b3de 100644 --- a/crates/gpui/src/text_system.rs +++ b/crates/gpui/src/text_system.rs @@ -233,7 +233,7 @@ pub struct TextSystem { font_ids_by_font: RwLock>, font_metrics: RwLock>, raster_bounds: RwLock>>, - wrapper_pool: Mutex>>, + wrapper_pool: Mutex>>, fallback_font_stack: SmallVec<[Font; 2]>, font_generation: Arc, missing_glyph_reporter: Arc, @@ -515,12 +515,12 @@ impl TextSystem { pub fn line_wrapper(self: &Arc, font: Font, font_size: Pixels) -> LineWrapperHandle { let lock = &mut self.wrapper_pool.lock(); let font_id = self.resolve_font(&font); - let wrappers = lock + let char_widths = lock .entry(FontIdWithSize { font_id, font_size }) - .or_default(); - let wrapper = wrappers + .or_default() .pop() - .unwrap_or_else(|| LineWrapper::new(font_id, font_size, self.clone())); + .unwrap_or_default(); + let wrapper = LineWrapper::new(font_id, font_size, self.clone(), char_widths); LineWrapperHandle { wrapper: Some(wrapper), @@ -945,7 +945,7 @@ impl Drop for LineWrapperHandle { font_size: wrapper.font_size, }) .unwrap() - .push(wrapper); + .push(wrapper.into_char_widths()); } } @@ -1315,6 +1315,28 @@ mod font_resolution_tests { } } +#[cfg(test)] +mod line_wrapper_pool_tests { + use super::*; + use crate::NoopTextSystem; + + #[test] + fn pooled_line_wrappers_do_not_retain_text_system() { + let text_system = Arc::new(TextSystem::new(Arc::new(NoopTextSystem::new()))); + let weak_text_system = Arc::downgrade(&text_system); + + let mut wrapper = text_system.line_wrapper(font(".ZedMono"), px(16.)); + let fragments = [LineFragment::text("hello world")]; + wrapper + .wrap_line(&fragments, px(40.), IndentAdjustment::default()) + .for_each(drop); + drop(wrapper); + drop(text_system); + + assert!(weak_text_system.upgrade().is_none()); + } +} + #[cfg(test)] mod missing_glyph_tests { use super::*; diff --git a/crates/gpui/src/text_system/line_wrapper.rs b/crates/gpui/src/text_system/line_wrapper.rs index ca821bbbb43..500ed441f87 100644 --- a/crates/gpui/src/text_system/line_wrapper.rs +++ b/crates/gpui/src/text_system/line_wrapper.rs @@ -30,24 +30,46 @@ pub struct LineWrapper { text_system: Arc, pub(crate) font_id: FontId, pub(crate) font_size: Pixels, - cached_ascii_char_widths: [Option; 128], - cached_other_char_widths: HashMap, + char_widths: CharWidthCache, +} + +/// Cached character widths for one font and size. +pub(crate) struct CharWidthCache { + ascii: [Option; 128], + other: HashMap, +} + +impl Default for CharWidthCache { + fn default() -> Self { + Self { + ascii: [None; 128], + other: HashMap::default(), + } + } } impl LineWrapper { /// The maximum indent that can be applied to a line. pub const MAX_INDENT: u32 = 256; - pub(crate) fn new(font_id: FontId, font_size: Pixels, text_system: Arc) -> Self { + pub(crate) fn new( + font_id: FontId, + font_size: Pixels, + text_system: Arc, + char_widths: CharWidthCache, + ) -> Self { Self { text_system, font_id, font_size, - cached_ascii_char_widths: [None; 128], - cached_other_char_widths: HashMap::default(), + char_widths, } } + pub(crate) fn into_char_widths(self) -> CharWidthCache { + self.char_widths + } + /// Wrap a line of text to the given width with this wrapper's font and font size. pub fn wrap_line<'a>( &'a mut self, @@ -543,22 +565,22 @@ impl LineWrapper { #[inline(always)] fn width_for_char(&mut self, c: char) -> Pixels { if (c as u32) < 128 { - if let Some(cached_width) = self.cached_ascii_char_widths[c as usize] { + if let Some(cached_width) = self.char_widths.ascii[c as usize] { cached_width } else { let width = self .text_system .layout_width(self.font_id, self.font_size, c); - self.cached_ascii_char_widths[c as usize] = Some(width); + self.char_widths.ascii[c as usize] = Some(width); width } - } else if let Some(cached_width) = self.cached_other_char_widths.get(&c) { + } else if let Some(cached_width) = self.char_widths.other.get(&c) { *cached_width } else { let width = self .text_system .layout_width(self.font_id, self.font_size, c); - self.cached_other_char_widths.insert(c, width); + self.char_widths.other.insert(c, width); width } } @@ -746,7 +768,12 @@ mod tests { let dispatcher = TestDispatcher::new(0); let cx = TestAppContext::build(dispatcher, None); let id = cx.text_system().resolve_font(&font(".ZedMono")); - LineWrapper::new(id, px(16.), cx.text_system().clone()) + LineWrapper::new( + id, + px(16.), + cx.text_system().clone(), + CharWidthCache::default(), + ) } fn generate_test_runs(input_run_len: &[usize]) -> Vec {