Revert "gpui: Unify the index_for_x methods (#42162)" (#42505)

Kirill Bulatov created

This reverts commit 082b80ec89748bf238f0068da80e12211fb3c7d6.

This broke clicking, e.g. in snippets like

```rs
let x = vec![
    1, 2, //
    3,
];
```

clicking between `2` and `,` is quite off now.

Release Notes:

- N/A

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, 62 insertions(+), 13 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.index_for_x(x) as u32
+        layout_line.closest_index_for_x(x) as u32
     }
 
     pub fn grapheme_at(&self, mut point: DisplayPoint) -> Option<SharedString> {

crates/editor/src/element.rs 🔗

@@ -8668,7 +8668,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.index_for_x(positions.start) as u32;
+        let start_col = line.closest_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.index_for_x(positions.end) as u32;
+            let end_col = line.closest_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.index_for_x(position.x - bounds.left())
+        line.closest_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,9 +54,25 @@ 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 index_for_x(&self, x: Pixels) -> usize {
+    pub fn closest_index_for_x(&self, x: Pixels) -> usize {
         let mut prev_index = 0;
         let mut prev_x = px(0.);
 
@@ -262,10 +278,34 @@ 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;
 
@@ -305,9 +345,16 @@ impl WrappedLineLayout {
         } else if position_in_unwrapped_line.x >= wrapped_line_end_x {
             Err(wrapped_line_end_index)
         } else {
-            Ok(self
-                .unwrapped_layout
-                .index_for_x(position_in_unwrapped_line.x))
+            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())
+            }
         }
     }
 

crates/vim/src/visual.rs 🔗

@@ -371,10 +371,12 @@ impl Vim {
 
             loop {
                 let laid_out_line = map.layout_row(row, &text_layout_details);
-                let start =
-                    DisplayPoint::new(row, laid_out_line.index_for_x(positions.start) as u32);
+                let start = DisplayPoint::new(
+                    row,
+                    laid_out_line.closest_index_for_x(positions.start) as u32,
+                );
                 let mut end =
-                    DisplayPoint::new(row, laid_out_line.index_for_x(positions.end) as u32);
+                    DisplayPoint::new(row, laid_out_line.closest_index_for_x(positions.end) as u32);
                 if end <= start {
                     if start.column() == map.line_len(start.row()) {
                         end = start;