From ff0060aa36c76a02e696acacde25af1095277a0a Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Fri, 16 May 2025 23:48:36 +0200 Subject: [PATCH] Remove unnecessary result in line shaping (#30721) Updates #29879 Release Notes: - N/A --- crates/editor/src/display_map.rs | 4 +- crates/editor/src/element.rs | 180 ++++++++----------- crates/gpui/examples/input.rs | 3 +- crates/gpui/src/text_system.rs | 12 +- crates/repl/src/outputs/table.rs | 8 +- crates/terminal_view/src/terminal_element.rs | 66 +++---- 6 files changed, 118 insertions(+), 155 deletions(-) diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index b3742620da..4d49099f16 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -1026,9 +1026,7 @@ impl DisplaySnapshot { } let font_size = editor_style.text.font_size.to_pixels(*rem_size); - text_system - .layout_line(&line, font_size, &runs) - .expect("we expect the font to be loaded because it's rendered by the editor") + text_system.layout_line(&line, font_size, &runs) } pub fn x_for_display_point( diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index b601984ea7..cca91c2df0 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -1343,7 +1343,7 @@ impl EditorElement { None } }) - .and_then(|text| { + .map(|text| { let len = text.len(); let font = cursor_row_layout @@ -1369,21 +1369,18 @@ impl EditorElement { cx.theme().colors().editor_background }; - window - .text_system() - .shape_line( - text, - cursor_row_layout.font_size, - &[TextRun { - len, - font, - color, - background_color: None, - strikethrough: None, - underline: None, - }], - ) - .log_err() + window.text_system().shape_line( + text, + cursor_row_layout.font_size, + &[TextRun { + len, + font, + color, + background_color: None, + strikethrough: None, + underline: None, + }], + ) }) } else { None @@ -2690,9 +2687,8 @@ impl EditorElement { } }) .unwrap_or_else(|| cx.theme().colors().editor_line_number); - let shaped_line = self - .shape_line_number(SharedString::from(&line_number), color, window) - .log_err()?; + let shaped_line = + self.shape_line_number(SharedString::from(&line_number), color, window); let scroll_top = scroll_position.y * line_height; let line_origin = gutter_hitbox.map(|hitbox| { hitbox.origin @@ -2808,7 +2804,7 @@ impl EditorElement { .chain(iter::repeat("")) .take(rows.len()); placeholder_lines - .filter_map(move |line| { + .map(move |line| { let run = TextRun { len: line.len(), font: style.text.font(), @@ -2817,17 +2813,17 @@ impl EditorElement { underline: None, strikethrough: None, }; - window - .text_system() - .shape_line(line.to_string().into(), font_size, &[run]) - .log_err() - }) - .map(|line| LineWithInvisibles { - width: line.width, - len: line.len, - fragments: smallvec![LineFragment::Text(line)], - invisibles: Vec::new(), - font_size, + let line = + window + .text_system() + .shape_line(line.to_string().into(), font_size, &[run]); + LineWithInvisibles { + width: line.width, + len: line.len, + fragments: smallvec![LineFragment::Text(line)], + invisibles: Vec::new(), + font_size, + } }) .collect() } else { @@ -4764,13 +4760,7 @@ impl EditorElement { let Some(()) = (if !is_singleton && hitbox.is_hovered(window) { let color = cx.theme().colors().editor_hover_line_number; - let Some(line) = self - .shape_line_number(shaped_line.text.clone(), color, window) - .log_err() - else { - continue; - }; - + let line = self.shape_line_number(shaped_line.text.clone(), color, window); line.paint(hitbox.origin, line_height, window, cx).log_err() } else { shaped_line @@ -6137,21 +6127,18 @@ impl EditorElement { fn column_pixels(&self, column: usize, window: &mut Window, _: &mut App) -> Pixels { let style = &self.style; let font_size = style.text.font_size.to_pixels(window.rem_size()); - let layout = window - .text_system() - .shape_line( - SharedString::from(" ".repeat(column)), - font_size, - &[TextRun { - len: column, - font: style.text.font(), - color: Hsla::default(), - background_color: None, - underline: None, - strikethrough: None, - }], - ) - .unwrap(); + let layout = window.text_system().shape_line( + SharedString::from(" ".repeat(column)), + font_size, + &[TextRun { + len: column, + font: style.text.font(), + color: Hsla::default(), + background_color: None, + underline: None, + strikethrough: None, + }], + ); layout.width } @@ -6171,7 +6158,7 @@ impl EditorElement { text: SharedString, color: Hsla, window: &mut Window, - ) -> anyhow::Result { + ) -> ShapedLine { let run = TextRun { len: text.len(), font: self.style.text.font(), @@ -6451,10 +6438,10 @@ impl LineWithInvisibles { }]) { if let Some(replacement) = highlighted_chunk.replacement { if !line.is_empty() { - let shaped_line = window - .text_system() - .shape_line(line.clone().into(), font_size, &styles) - .unwrap(); + let shaped_line = + window + .text_system() + .shape_line(line.clone().into(), font_size, &styles); width += shaped_line.width; len += shaped_line.len; fragments.push(LineFragment::Text(shaped_line)); @@ -6470,14 +6457,11 @@ impl LineWithInvisibles { } else { SharedString::from(Arc::from(highlighted_chunk.text)) }; - let shaped_line = window - .text_system() - .shape_line( - chunk, - font_size, - &[text_style.to_run(highlighted_chunk.text.len())], - ) - .unwrap(); + let shaped_line = window.text_system().shape_line( + chunk, + font_size, + &[text_style.to_run(highlighted_chunk.text.len())], + ); AvailableSpace::Definite(shaped_line.width) } else { AvailableSpace::MinContent @@ -6522,7 +6506,6 @@ impl LineWithInvisibles { let line_layout = window .text_system() .shape_line(x, font_size, &[run]) - .unwrap() .with_len(highlighted_chunk.text.len()); width += line_layout.width; @@ -6533,10 +6516,11 @@ impl LineWithInvisibles { } else { for (ix, mut line_chunk) in highlighted_chunk.text.split('\n').enumerate() { if ix > 0 { - let shaped_line = window - .text_system() - .shape_line(line.clone().into(), font_size, &styles) - .unwrap(); + let shaped_line = window.text_system().shape_line( + line.clone().into(), + font_size, + &styles, + ); width += shaped_line.width; len += shaped_line.len; fragments.push(LineFragment::Text(shaped_line)); @@ -8038,36 +8022,30 @@ impl Element for EditorElement { }); let invisible_symbol_font_size = font_size / 2.; - let tab_invisible = window - .text_system() - .shape_line( - "→".into(), - invisible_symbol_font_size, - &[TextRun { - len: "→".len(), - font: self.style.text.font(), - color: cx.theme().colors().editor_invisible, - background_color: None, - underline: None, - strikethrough: None, - }], - ) - .unwrap(); - let space_invisible = window - .text_system() - .shape_line( - "•".into(), - invisible_symbol_font_size, - &[TextRun { - len: "•".len(), - font: self.style.text.font(), - color: cx.theme().colors().editor_invisible, - background_color: None, - underline: None, - strikethrough: None, - }], - ) - .unwrap(); + let tab_invisible = window.text_system().shape_line( + "→".into(), + invisible_symbol_font_size, + &[TextRun { + len: "→".len(), + font: self.style.text.font(), + color: cx.theme().colors().editor_invisible, + background_color: None, + underline: None, + strikethrough: None, + }], + ); + let space_invisible = window.text_system().shape_line( + "•".into(), + invisible_symbol_font_size, + &[TextRun { + len: "•".len(), + font: self.style.text.font(), + color: cx.theme().colors().editor_invisible, + background_color: None, + underline: None, + strikethrough: None, + }], + ); let mode = snapshot.mode.clone(); diff --git a/crates/gpui/examples/input.rs b/crates/gpui/examples/input.rs index 2d01e3d749..5d28a8a8a9 100644 --- a/crates/gpui/examples/input.rs +++ b/crates/gpui/examples/input.rs @@ -481,8 +481,7 @@ impl Element for TextElement { let font_size = style.font_size.to_pixels(window.rem_size()); let line = window .text_system() - .shape_line(display_text, font_size, &runs) - .unwrap(); + .shape_line(display_text, font_size, &runs); let cursor_pos = line.x_for_index(cursor); let (selection, cursor) = if selected_range.is_empty() { diff --git a/crates/gpui/src/text_system.rs b/crates/gpui/src/text_system.rs index 216e14b6da..3aa78491eb 100644 --- a/crates/gpui/src/text_system.rs +++ b/crates/gpui/src/text_system.rs @@ -343,7 +343,7 @@ impl WindowTextSystem { text: SharedString, font_size: Pixels, runs: &[TextRun], - ) -> Result { + ) -> ShapedLine { debug_assert!( text.find('\n').is_none(), "text argument should not contain newlines" @@ -370,13 +370,13 @@ impl WindowTextSystem { }); } - let layout = self.layout_line(&text, font_size, runs)?; + let layout = self.layout_line(&text, font_size, runs); - Ok(ShapedLine { + ShapedLine { layout, text, decoration_runs, - }) + } } /// Shape a multi line string of text, at the given font_size, for painting to the screen. @@ -510,7 +510,7 @@ impl WindowTextSystem { text: Text, font_size: Pixels, runs: &[TextRun], - ) -> Result> + ) -> Arc where Text: AsRef, SharedString: From, @@ -537,7 +537,7 @@ impl WindowTextSystem { font_runs.clear(); self.font_runs_pool.lock().push(font_runs); - Ok(layout) + layout } } diff --git a/crates/repl/src/outputs/table.rs b/crates/repl/src/outputs/table.rs index 5165168e9b..0606b421aa 100644 --- a/crates/repl/src/outputs/table.rs +++ b/crates/repl/src/outputs/table.rs @@ -106,10 +106,7 @@ impl TableView { for field in table.schema.fields.iter() { runs[0].len = field.name.len(); - let mut width = text_system - .layout_line(&field.name, font_size, &runs) - .map(|layout| layout.width) - .unwrap_or(px(0.)); + let mut width = text_system.layout_line(&field.name, font_size, &runs).width; let Some(data) = table.data.as_ref() else { widths.push(width); @@ -122,8 +119,7 @@ impl TableView { let cell_width = window .text_system() .layout_line(&content, font_size, &runs) - .map(|layout| layout.width) - .unwrap_or(px(0.)); + .width; width = width.max(cell_width) } diff --git a/crates/terminal_view/src/terminal_element.rs b/crates/terminal_view/src/terminal_element.rs index 8c75f8e079..2014f91602 100644 --- a/crates/terminal_view/src/terminal_element.rs +++ b/crates/terminal_view/src/terminal_element.rs @@ -288,13 +288,11 @@ impl TerminalElement { let cell_style = TerminalElement::cell_style(&cell, fg, theme, text_style, hyperlink); - let layout_cell = text_system - .shape_line( - cell_text.into(), - text_style.font_size.to_pixels(window.rem_size()), - &[cell_style], - ) - .unwrap(); + let layout_cell = text_system.shape_line( + cell_text.into(), + text_style.font_size.to_pixels(window.rem_size()), + &[cell_style], + ); cells.push(LayoutCell::new( AlacPoint::new(line_index as i32, cell.point.column.0 as i32), @@ -813,21 +811,18 @@ impl Element for TerminalElement { let cursor_text = { let str_trxt = cursor_char.to_string(); let len = str_trxt.len(); - window - .text_system() - .shape_line( - str_trxt.into(), - text_style.font_size.to_pixels(window.rem_size()), - &[TextRun { - len, - font: text_style.font(), - color: theme.colors().terminal_ansi_background, - background_color: None, - underline: Default::default(), - strikethrough: None, - }], - ) - .unwrap() + window.text_system().shape_line( + str_trxt.into(), + text_style.font_size.to_pixels(window.rem_size()), + &[TextRun { + len, + font: text_style.font(), + color: theme.colors().terminal_ansi_background, + background_color: None, + underline: Default::default(), + strikethrough: None, + }], + ) }; let focused = self.focused; @@ -1008,21 +1003,18 @@ impl Element for TerminalElement { wavy: false, }); - let shaped_line = window - .text_system() - .shape_line( - text_to_mark.clone().into(), - ime_style.font_size.to_pixels(window.rem_size()), - &[TextRun { - len: text_to_mark.len(), - font: ime_style.font(), - color: ime_style.color, - background_color: None, - underline: ime_style.underline, - strikethrough: None, - }], - ) - .unwrap(); + let shaped_line = window.text_system().shape_line( + text_to_mark.clone().into(), + ime_style.font_size.to_pixels(window.rem_size()), + &[TextRun { + len: text_to_mark.len(), + font: ime_style.font(), + color: ime_style.color, + background_color: None, + underline: ime_style.underline, + strikethrough: None, + }], + ); shaped_line .paint(ime_position, layout.dimensions.line_height, window, cx) .log_err();