From 082b80ec89748bf238f0068da80e12211fb3c7d6 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Fri, 7 Nov 2025 04:23:43 -0800 Subject: [PATCH] gpui: Unify the index_for_x methods (#42162) Supersedes https://github.com/zed-industries/zed/pull/39910 At some point, these two (`index_for_x` and `closest_index_for_x`) methods where separated out and some code paths used one, while other code paths took the other. That said, their behavior is almost identical: - `index_for_x` computes the index behind the pixel offset, and returns `None` if there's an overshoot - `closest_index_for_x` computes the nearest index to the pixel offset, taking into account whether the offset is over halfway through or not. If there's an overshoot, it returns the length of the line. Given these two behaviors, `closest_index_for_x` seems to be a more useful API than `index_for_x`, and indeed the display map and other core editor features use it extensively. So this PR is an experiment in simply replacing one behavior with the other. Release Notes: - Improved the accuracy of mouse selections in Markdown --- crates/editor/src/display_map.rs | 2 +- crates/editor/src/element.rs | 2 +- crates/editor/src/selections_collection.rs | 4 +- crates/gpui/examples/input.rs | 4 +- crates/gpui/src/text_system/line_layout.rs | 55 ++-------------------- crates/vim/src/visual.rs | 8 ++-- 6 files changed, 13 insertions(+), 62 deletions(-) diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index e5b9cfc93fd4784936a13c45daee48a121a70e79..850c4ca61c0c3a5a295f85b191df18177dc5f403 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -1097,7 +1097,7 @@ impl DisplaySnapshot { details: &TextLayoutDetails, ) -> u32 { let layout_line = self.layout_row(display_row, details); - layout_line.closest_index_for_x(x) as u32 + layout_line.index_for_x(x) as u32 } pub fn grapheme_at(&self, mut point: DisplayPoint) -> Option { diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 7854b89d1494cacbc8fed5eba83e2461bbf45bac..da0f61bf42d71049dbe894af86687c11c84e53b4 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -8334,7 +8334,7 @@ impl LineWithInvisibles { let fragment_end_x = fragment_start_x + shaped_line.width; if x < fragment_end_x { return Some( - fragment_start_index + shaped_line.index_for_x(x - fragment_start_x)?, + fragment_start_index + shaped_line.index_for_x(x - fragment_start_x), ); } fragment_start_x = fragment_end_x; diff --git a/crates/editor/src/selections_collection.rs b/crates/editor/src/selections_collection.rs index 15058fa0c821fe3bf7c60d1a7bd681eb98650cfe..eeecfffa70a30705174b64f698d2965f9540fe0b 100644 --- a/crates/editor/src/selections_collection.rs +++ b/crates/editor/src/selections_collection.rs @@ -372,7 +372,7 @@ impl SelectionsCollection { let is_empty = positions.start == positions.end; let line_len = display_map.line_len(row); let line = display_map.layout_row(row, text_layout_details); - let start_col = line.closest_index_for_x(positions.start) as u32; + let start_col = line.index_for_x(positions.start) as u32; let (start, end) = if is_empty { let point = DisplayPoint::new(row, std::cmp::min(start_col, line_len)); @@ -382,7 +382,7 @@ impl SelectionsCollection { return None; } let start = DisplayPoint::new(row, start_col); - let end_col = line.closest_index_for_x(positions.end) as u32; + let end_col = line.index_for_x(positions.end) as u32; let end = DisplayPoint::new(row, end_col); (start, end) }; diff --git a/crates/gpui/examples/input.rs b/crates/gpui/examples/input.rs index 37115feaa551a787562e7299c9d44bcc97b5fca3..16af30166c6ccdbd06469f4e2fd4cd3df8352127 100644 --- a/crates/gpui/examples/input.rs +++ b/crates/gpui/examples/input.rs @@ -178,7 +178,7 @@ impl TextInput { if position.y > bounds.bottom() { return self.content.len(); } - line.closest_index_for_x(position.x - bounds.left()) + line.index_for_x(position.x - bounds.left()) } fn select_to(&mut self, offset: usize, cx: &mut Context) { @@ -380,7 +380,7 @@ impl EntityInputHandler for TextInput { let last_layout = self.last_layout.as_ref()?; assert_eq!(last_layout.text, self.content); - let utf8_index = last_layout.index_for_x(point.x - line_point.x)?; + let utf8_index = last_layout.index_for_x(point.x - line_point.x); Some(self.offset_to_utf16(utf8_index)) } } diff --git a/crates/gpui/src/text_system/line_layout.rs b/crates/gpui/src/text_system/line_layout.rs index 375a9bdc7bccdddb9d34409c5ced138b2d5aebd2..61edd614d804434d414b34a9804e51b0b0148ea4 100644 --- a/crates/gpui/src/text_system/line_layout.rs +++ b/crates/gpui/src/text_system/line_layout.rs @@ -54,25 +54,9 @@ pub struct ShapedGlyph { } impl LineLayout { - /// The index for the character at the given x coordinate - pub fn index_for_x(&self, x: Pixels) -> Option { - if x >= self.width { - None - } else { - for run in self.runs.iter().rev() { - for glyph in run.glyphs.iter().rev() { - if glyph.position.x <= x { - return Some(glyph.index); - } - } - } - Some(0) - } - } - /// closest_index_for_x returns the character boundary closest to the given x coordinate /// (e.g. to handle aligning up/down arrow keys) - pub fn closest_index_for_x(&self, x: Pixels) -> usize { + pub fn index_for_x(&self, x: Pixels) -> usize { let mut prev_index = 0; let mut prev_x = px(0.); @@ -278,34 +262,10 @@ impl WrappedLineLayout { } /// The index corresponding to a given position in this layout for the given line height. - /// - /// See also [`Self::closest_index_for_position`]. pub fn index_for_position( - &self, - position: Point, - line_height: Pixels, - ) -> Result { - self._index_for_position(position, line_height, false) - } - - /// The closest index to a given position in this layout for the given line height. - /// - /// Closest means the character boundary closest to the given position. - /// - /// See also [`LineLayout::closest_index_for_x`]. - pub fn closest_index_for_position( - &self, - position: Point, - line_height: Pixels, - ) -> Result { - self._index_for_position(position, line_height, true) - } - - fn _index_for_position( &self, mut position: Point, line_height: Pixels, - closest: bool, ) -> Result { let wrapped_line_ix = (position.y / line_height) as usize; @@ -345,16 +305,9 @@ impl WrappedLineLayout { } else if position_in_unwrapped_line.x >= wrapped_line_end_x { Err(wrapped_line_end_index) } else { - if closest { - Ok(self - .unwrapped_layout - .closest_index_for_x(position_in_unwrapped_line.x)) - } else { - Ok(self - .unwrapped_layout - .index_for_x(position_in_unwrapped_line.x) - .unwrap()) - } + Ok(self + .unwrapped_layout + .index_for_x(position_in_unwrapped_line.x)) } } diff --git a/crates/vim/src/visual.rs b/crates/vim/src/visual.rs index 4172de80afdc1beacbf3ea342846de03953e1fc6..498c4b4dc6ec6ad8af4f47bb6ea5044a5fcd3c0a 100644 --- a/crates/vim/src/visual.rs +++ b/crates/vim/src/visual.rs @@ -371,12 +371,10 @@ impl Vim { loop { let laid_out_line = map.layout_row(row, &text_layout_details); - let start = DisplayPoint::new( - row, - laid_out_line.closest_index_for_x(positions.start) as u32, - ); + let start = + DisplayPoint::new(row, laid_out_line.index_for_x(positions.start) as u32); let mut end = - DisplayPoint::new(row, laid_out_line.closest_index_for_x(positions.end) as u32); + DisplayPoint::new(row, laid_out_line.index_for_x(positions.end) as u32); if end <= start { if start.column() == map.line_len(start.row()) { end = start;