gpui: Unify the index_for_x methods (#42162)

Mikayla Maki created

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

Change summary

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(-)

Detailed changes

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<SharedString> {

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;

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)
         };

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<Self>) {
@@ -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))
     }
 }

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<usize> {
-        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<Pixels>,
-        line_height: Pixels,
-    ) -> Result<usize, usize> {
-        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<Pixels>,
-        line_height: Pixels,
-    ) -> Result<usize, usize> {
-        self._index_for_position(position, line_height, true)
-    }
-
-    fn _index_for_position(
         &self,
         mut position: Point<Pixels>,
         line_height: Pixels,
-        closest: bool,
     ) -> Result<usize, usize> {
         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))
         }
     }
 

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;