From d6b31d8932b669ee789363405664fb5e762d520d Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 26 Oct 2025 13:00:52 +0100 Subject: [PATCH] gpui: Fix `TextLayout::layout` producing invalid text runs (#41224) The issues is that the closure supplied to `request_measured_layout` could be run multiple times with differing `known_dimensions`. This in turn will mean we truncate the font runs once, inserting a multibyte char at the end but then in the next iteration use those truncated runs for possible the original untruncated string which has multibyte characters overlapping the now truncated run end resulting in a faulty utf8 boundary index. Solution to this is simple, truncate a clone of the runs when needed instead of modifying the original. Fixes https://github.com/zed-industries/zed/issues/36925 Fixed ZED-2FF Fixes ZED-2KM Fixes ZED-2KK Fixes ZED-1FF Fixes ZED-255 Fixes ZED-2JD Fixes ZED-2FX Fixes ZED-2K2 Fixes ZED-2JX Fixes ZED-2GE Fixes ZED-2FC Fixes ZED-2GD Fixes ZED-2HY Fixes ZED-2HR Fixes ZED-2FN Fixes ZED-2GT Fixes ZED-2FK Fixes ZED-2EY Fixes ZED-27E Fixes ZED-272 Fixes ZED-2EM Fixes ZED-2CC Fixes ZED-29V Fixes ZED-25B Fixes ZED-257 Fixes ZED-24R Fixes ZED-24Q Fixes ZED-23Z Fixes ZED-227 Release Notes: - Fixed a crash in text shaping when truncating rendered text --- crates/gpui/src/elements/text.rs | 10 ++--- crates/gpui/src/styled.rs | 1 - crates/gpui/src/text_system.rs | 50 ++++++++++++--------- crates/gpui/src/text_system/line_wrapper.rs | 37 ++++++++------- crates/gpui/src/window.rs | 11 ++--- 5 files changed, 57 insertions(+), 52 deletions(-) diff --git a/crates/gpui/src/elements/text.rs b/crates/gpui/src/elements/text.rs index 5d34ccfa5d78e09d47b36a2e061fdaa0fbbca45f..914e8a286510a2ffd833db4c4d3ef85c84db073f 100644 --- a/crates/gpui/src/elements/text.rs +++ b/crates/gpui/src/elements/text.rs @@ -8,6 +8,7 @@ use crate::{ use anyhow::Context as _; use smallvec::SmallVec; use std::{ + borrow::Cow, cell::{Cell, RefCell}, mem, ops::Range, @@ -334,12 +335,11 @@ impl TextLayout { .line_height .to_pixels(font_size.into(), window.rem_size()); - let mut runs = if let Some(runs) = runs { + let runs = if let Some(runs) = runs { runs } else { vec![text_style.to_run(text.len())] }; - window.request_measured_layout(Default::default(), { let element_state = self.clone(); @@ -378,15 +378,15 @@ impl TextLayout { } let mut line_wrapper = cx.text_system().line_wrapper(text_style.font(), font_size); - let text = if let Some(truncate_width) = truncate_width { + let (text, runs) = if let Some(truncate_width) = truncate_width { line_wrapper.truncate_line( text.clone(), truncate_width, &truncation_suffix, - &mut runs, + &runs, ) } else { - text.clone() + (text.clone(), Cow::Borrowed(&*runs)) }; let len = text.len(); diff --git a/crates/gpui/src/styled.rs b/crates/gpui/src/styled.rs index 4475718675b7feee4abcfcded814ae3cc38d5fdb..8dcb43c6ce6bb5522b3d1337390ae5436809720e 100644 --- a/crates/gpui/src/styled.rs +++ b/crates/gpui/src/styled.rs @@ -9,7 +9,6 @@ pub use gpui_macros::{ overflow_style_methods, padding_style_methods, position_style_methods, visibility_style_methods, }; - const ELLIPSIS: SharedString = SharedString::new_static("…"); /// A trait for elements that can be styled. diff --git a/crates/gpui/src/text_system.rs b/crates/gpui/src/text_system.rs index 85a3133ca6c9e559c1cae76f595426d702bfd3f3..0c8a32b16c5273bdcfd8b5380cf6e0698cffcbcb 100644 --- a/crates/gpui/src/text_system.rs +++ b/crates/gpui/src/text_system.rs @@ -418,22 +418,21 @@ impl WindowTextSystem { let mut font_runs = self.font_runs_pool.lock().pop().unwrap_or_default(); let mut lines = SmallVec::new(); - let mut line_start = 0; - let mut max_wrap_lines = line_clamp.unwrap_or(usize::MAX); + let mut max_wrap_lines = line_clamp; let mut wrapped_lines = 0; - let mut process_line = |line_text: SharedString| { + let mut process_line = |line_text: SharedString, line_start, line_end| { font_runs.clear(); - let line_end = line_start + line_text.len(); let mut decoration_runs = SmallVec::<[DecorationRun; 32]>::new(); let mut run_start = line_start; while run_start < line_end { let Some(run) = runs.peek_mut() else { + log::warn!("`TextRun`s do not cover the entire to be shaped text"); break; }; - let run_len_within_line = cmp::min(line_end, run_start + run.len) - run_start; + let run_len_within_line = cmp::min(line_end - run_start, run.len); let decoration_changed = if let Some(last_run) = decoration_runs.last_mut() && last_run.color == run.color @@ -467,11 +466,10 @@ impl WindowTextSystem { }); } - if run_len_within_line == run.len { + // Preserve the remainder of the run for the next line + run.len -= run_len_within_line; + if run.len == 0 { runs.next(); - } else { - // Preserve the remainder of the run for the next line - run.len -= run_len_within_line; } run_start += run_len_within_line; } @@ -481,7 +479,7 @@ impl WindowTextSystem { font_size, &font_runs, wrap_width, - Some(max_wrap_lines - wrapped_lines), + max_wrap_lines.map(|max| max.saturating_sub(wrapped_lines)), ); wrapped_lines += layout.wrap_boundaries.len(); @@ -492,7 +490,6 @@ impl WindowTextSystem { }); // Skip `\n` character. - line_start = line_end + 1; if let Some(run) = runs.peek_mut() { run.len -= 1; if run.len == 0 { @@ -502,21 +499,34 @@ impl WindowTextSystem { }; let mut split_lines = text.split('\n'); - let mut processed = false; + // Special case single lines to prevent allocating a sharedstring if let Some(first_line) = split_lines.next() && let Some(second_line) = split_lines.next() { - processed = true; - process_line(first_line.to_string().into()); - process_line(second_line.to_string().into()); + let mut line_start = 0; + process_line( + SharedString::new(first_line), + line_start, + line_start + first_line.len(), + ); + line_start += first_line.len() + '\n'.len_utf8(); + process_line( + SharedString::new(second_line), + line_start, + line_start + second_line.len(), + ); for line_text in split_lines { - process_line(line_text.to_string().into()); + line_start += line_text.len() + '\n'.len_utf8(); + process_line( + SharedString::new(line_text), + line_start, + line_start + line_text.len(), + ); } - } - - if !processed { - process_line(text); + } else { + let end = text.len(); + process_line(text, 0, end); } self.font_runs_pool.lock().push(font_runs); diff --git a/crates/gpui/src/text_system/line_wrapper.rs b/crates/gpui/src/text_system/line_wrapper.rs index 55599cc0535dfdd94bfd895ce6001f3a83a27cf6..0192a03a3238e8fad1f5f20e2b824755c0ecee4d 100644 --- a/crates/gpui/src/text_system/line_wrapper.rs +++ b/crates/gpui/src/text_system/line_wrapper.rs @@ -1,6 +1,6 @@ use crate::{FontId, FontRun, Pixels, PlatformTextSystem, SharedString, TextRun, px}; use collections::HashMap; -use std::{iter, sync::Arc}; +use std::{borrow::Cow, iter, sync::Arc}; /// The GPUI line wrapper, used to wrap lines of text to a given width. pub struct LineWrapper { @@ -129,13 +129,13 @@ impl LineWrapper { } /// Truncate a line of text to the given width with this wrapper's font and font size. - pub fn truncate_line( + pub fn truncate_line<'a>( &mut self, line: SharedString, truncate_width: Pixels, truncation_suffix: &str, - runs: &mut Vec, - ) -> SharedString { + runs: &'a [TextRun], + ) -> (SharedString, Cow<'a, [TextRun]>) { let mut width = px(0.); let mut suffix_width = truncation_suffix .chars() @@ -154,13 +154,14 @@ impl LineWrapper { if width.floor() > truncate_width { let result = SharedString::from(format!("{}{}", &line[..truncate_ix], truncation_suffix)); - update_runs_after_truncation(&result, truncation_suffix, runs); + let mut runs = runs.to_vec(); + update_runs_after_truncation(&result, truncation_suffix, &mut runs); - return result; + return (result, Cow::Owned(runs)); } } - line + (line, Cow::Borrowed(runs)) } /// Any character in this list should be treated as a word character, @@ -493,15 +494,14 @@ mod tests { fn perform_test( wrapper: &mut LineWrapper, text: &'static str, - result: &'static str, + expected: &'static str, ellipsis: &str, ) { let dummy_run_lens = vec![text.len()]; - let mut dummy_runs = generate_test_runs(&dummy_run_lens); - assert_eq!( - wrapper.truncate_line(text.into(), px(220.), ellipsis, &mut dummy_runs), - result - ); + let dummy_runs = generate_test_runs(&dummy_run_lens); + let (result, dummy_runs) = + wrapper.truncate_line(text.into(), px(220.), ellipsis, &dummy_runs); + assert_eq!(result, expected); assert_eq!(dummy_runs.first().unwrap().len, result.len()); } @@ -532,16 +532,15 @@ mod tests { fn perform_test( wrapper: &mut LineWrapper, text: &'static str, - result: &str, + expected: &str, run_lens: &[usize], result_run_len: &[usize], line_width: Pixels, ) { - let mut dummy_runs = generate_test_runs(run_lens); - assert_eq!( - wrapper.truncate_line(text.into(), line_width, "…", &mut dummy_runs), - result - ); + let dummy_runs = generate_test_runs(run_lens); + let (result, dummy_runs) = + wrapper.truncate_line(text.into(), line_width, "…", &dummy_runs); + assert_eq!(result, expected); for (run, result_len) in dummy_runs.iter().zip(result_run_len) { assert_eq!(run.len, *result_len); } diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs index 0610ea96cb5150cfbad72b2b70b4432df9b76ca2..17d09e67dbafbf51be604180f3ff1333cc732cfd 100644 --- a/crates/gpui/src/window.rs +++ b/crates/gpui/src/window.rs @@ -3255,14 +3255,11 @@ impl Window { /// returns a `Size`. /// /// This method should only be called as part of the request_layout or prepaint phase of element drawing. - pub fn request_measured_layout< - F: FnMut(Size>, Size, &mut Window, &mut App) -> Size + pub fn request_measured_layout(&mut self, style: Style, measure: F) -> LayoutId + where + F: Fn(Size>, Size, &mut Window, &mut App) -> Size + 'static, - >( - &mut self, - style: Style, - measure: F, - ) -> LayoutId { + { self.invalidator.debug_assert_prepaint(); let rem_size = self.rem_size();