From a98d0489053ca2053c5a6e3b0673f79e3bedd053 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Thu, 11 Jan 2024 18:52:00 +0100 Subject: [PATCH] gpui: Make TextSystem::line_wrapper non-fallible. (#4022) Editors WrapMap could become desynchronised if user had an invalid font specified in their config. Compared to Zed1, WrapMap ignored the resolution failure instead of panicking. Now, if there's an invalid font in the user config, we just fall back to an arbitrary default. Release Notes: - Fixed the editor panic in presence of invalid font name in the config (fixes https://github.com/zed-industries/community/issues/2397) --------- Co-authored-by: Conrad Co-authored-by: Conrad Irwin --- crates/editor/src/display_map/wrap_map.rs | 52 ++++++++++------------- crates/gpui/src/text_system.rs | 22 +++------- 2 files changed, 30 insertions(+), 44 deletions(-) diff --git a/crates/editor/src/display_map/wrap_map.rs b/crates/editor/src/display_map/wrap_map.rs index dbd58b0accd89c4054d266f4867659add671a03b..ce2e5ee3d9eb66c1e4a769dacdb9ff8c357a1ff3 100644 --- a/crates/editor/src/display_map/wrap_map.rs +++ b/crates/editor/src/display_map/wrap_map.rs @@ -11,7 +11,6 @@ use smol::future::yield_now; use std::{cmp, collections::VecDeque, mem, ops::Range, time::Duration}; use sum_tree::{Bias, Cursor, SumTree}; use text::Patch; -use util::ResultExt; pub use super::tab_map::TextSummary; pub type WrapEdit = text::Edit; @@ -154,26 +153,24 @@ impl WrapMap { if let Some(wrap_width) = self.wrap_width { let mut new_snapshot = self.snapshot.clone(); - let mut edits = Patch::default(); + let text_system = cx.text_system().clone(); let (font, font_size) = self.font_with_size.clone(); let task = cx.background_executor().spawn(async move { - if let Some(mut line_wrapper) = text_system.line_wrapper(font, font_size).log_err() - { - let tab_snapshot = new_snapshot.tab_snapshot.clone(); - let range = TabPoint::zero()..tab_snapshot.max_point(); - edits = new_snapshot - .update( - tab_snapshot, - &[TabEdit { - old: range.clone(), - new: range.clone(), - }], - wrap_width, - &mut line_wrapper, - ) - .await; - } + let mut line_wrapper = text_system.line_wrapper(font, font_size); + let tab_snapshot = new_snapshot.tab_snapshot.clone(); + let range = TabPoint::zero()..tab_snapshot.max_point(); + let edits = new_snapshot + .update( + tab_snapshot, + &[TabEdit { + old: range.clone(), + new: range.clone(), + }], + wrap_width, + &mut line_wrapper, + ) + .await; (new_snapshot, edits) }); @@ -245,15 +242,12 @@ impl WrapMap { let (font, font_size) = self.font_with_size.clone(); let update_task = cx.background_executor().spawn(async move { let mut edits = Patch::default(); - if let Some(mut line_wrapper) = - text_system.line_wrapper(font, font_size).log_err() - { - for (tab_snapshot, tab_edits) in pending_edits { - let wrap_edits = snapshot - .update(tab_snapshot, &tab_edits, wrap_width, &mut line_wrapper) - .await; - edits = edits.compose(&wrap_edits); - } + let mut line_wrapper = text_system.line_wrapper(font, font_size); + for (tab_snapshot, tab_edits) in pending_edits { + let wrap_edits = snapshot + .update(tab_snapshot, &tab_edits, wrap_width, &mut line_wrapper) + .await; + edits = edits.compose(&wrap_edits); } (snapshot, edits) }); @@ -1059,7 +1053,7 @@ mod tests { }; let tab_size = NonZeroU32::new(rng.gen_range(1..=4)).unwrap(); let font = font("Helvetica"); - let _font_id = text_system.font_id(&font).unwrap(); + let _font_id = text_system.font_id(&font); let font_size = px(14.0); log::info!("Tab size: {}", tab_size); @@ -1086,7 +1080,7 @@ mod tests { let tabs_snapshot = tab_map.set_max_expansion_column(32); log::info!("TabMap text: {:?}", tabs_snapshot.text()); - let mut line_wrapper = text_system.line_wrapper(font.clone(), font_size).unwrap(); + let mut line_wrapper = text_system.line_wrapper(font.clone(), font_size); let unwrapped_text = tabs_snapshot.text(); let expected_text = wrap_text(&unwrapped_text, wrap_width, &mut line_wrapper); diff --git a/crates/gpui/src/text_system.rs b/crates/gpui/src/text_system.rs index 47073bcde0ef77b7b6674d0c0a24b7dfa107e948..2d3cc34f3fc09e6402ece055f9aa6ba45fe5434e 100644 --- a/crates/gpui/src/text_system.rs +++ b/crates/gpui/src/text_system.rs @@ -364,28 +364,20 @@ impl TextSystem { self.line_layout_cache.start_frame() } - pub fn line_wrapper( - self: &Arc, - font: Font, - font_size: Pixels, - ) -> Result { + pub fn line_wrapper(self: &Arc, font: Font, font_size: Pixels) -> LineWrapperHandle { let lock = &mut self.wrapper_pool.lock(); - let font_id = self.font_id(&font)?; + let font_id = self.resolve_font(&font); let wrappers = lock .entry(FontIdWithSize { font_id, font_size }) .or_default(); - let wrapper = wrappers.pop().map(anyhow::Ok).unwrap_or_else(|| { - Ok(LineWrapper::new( - font_id, - font_size, - self.platform_text_system.clone(), - )) - })?; + let wrapper = wrappers.pop().unwrap_or_else(|| { + LineWrapper::new(font_id, font_size, self.platform_text_system.clone()) + }); - Ok(LineWrapperHandle { + LineWrapperHandle { wrapper: Some(wrapper), text_system: self.clone(), - }) + } } pub fn raster_bounds(&self, params: &RenderGlyphParams) -> Result> {