Optimizes View::screen_coords_at_pos by not computing char_to_line two times

helix-core/src/lib.rs

@@ -95,7 +95,8 @@ pub use {regex, tree_sitter};
 
 pub use graphemes::RopeGraphemes;
 pub use position::{
-    coords_at_pos, pos_at_coords, pos_at_visual_coords, visual_coords_at_pos, Position,
+    coords_at_pos, pos_at_coords, pos_at_visual_coords, visual_col_position, visual_coords_at_pos,
+    Position,
 };
 pub use selection::{Range, Selection};
 pub use smallvec::{smallvec, SmallVec};

helix-core/src/position.rs

@@ -54,18 +54,15 @@ impl From<Position> for tree_sitter::Point {
         Self::new(pos.row, pos.col)
     }
 }
+
 /// Convert a character index to (line, column) coordinates.
 ///
 /// column in `char` count which can be used for row:column display in
 /// status line. See [`visual_coords_at_pos`] for a visual one.
 pub fn coords_at_pos(text: RopeSlice, pos: usize) -> Position {
-    let line = text.char_to_line(pos);
-
-    let line_start = text.line_to_char(line);
-    let pos = ensure_grapheme_boundary_prev(text, pos);
-    let col = RopeGraphemes::new(text.slice(line_start..pos)).count();
-
-    Position::new(line, col)
+    let row = text.char_to_line(pos);
+    let col = rope_graphemes(text, pos, row).count();
+    Position::new(row, col)
 }
 
 /// Convert a character index to (line, column) coordinates visually.
@@ -74,23 +71,25 @@ pub fn coords_at_pos(text: RopeSlice, pos: usize) -> Position {
 /// not in the document in the future.
 /// See [`coords_at_pos`] for an "objective" one.
 pub fn visual_coords_at_pos(text: RopeSlice, pos: usize, tab_width: usize) -> Position {
-    let line = text.char_to_line(pos);
-
-    let line_start = text.line_to_char(line);
-    let pos = ensure_grapheme_boundary_prev(text, pos);
+    let row = text.char_to_line(pos);
+    let col = visual_col_position(text, pos, row, tab_width);
+    Position { row, col }
+}
 
+/// Returns a column position of a character index in a row.
+///
+/// Accounts for tab (\t) and double-width characters (CJK).
+pub fn visual_col_position(text: RopeSlice, pos: usize, row: usize, tab_width: usize) -> usize {
     let mut col = 0;
-
-    for grapheme in RopeGraphemes::new(text.slice(line_start..pos)) {
+    for grapheme in rope_graphemes(text, pos, row) {
         if grapheme == "\t" {
             col += tab_width - (col % tab_width);
         } else {
             let grapheme = Cow::from(grapheme);
             col += grapheme_width(&grapheme);
         }
     }
-
-    Position::new(line, col)
+    col
 }
 
 /// Convert (line, column) coordinates to a character index.
@@ -169,6 +168,12 @@ pub fn pos_at_visual_coords(text: RopeSlice, coords: Position, tab_width: usize)
     line_start + col_char_offset
 }
 
+fn rope_graphemes(text: RopeSlice, pos: usize, row: usize) -> RopeGraphemes {
+    let row_start = text.line_to_char(row);
+    let pos = ensure_grapheme_boundary_prev(text, pos);
+    RopeGraphemes::new(text.slice(row_start..pos))
+}
+
 #[cfg(test)]
 mod test {
     use super::*;

helix-view/src/view.rs

@@ -1,6 +1,7 @@
 use crate::{editor::GutterType, graphics::Rect, Document, DocumentId, ViewId};
 use helix_core::{
-    pos_at_visual_coords, visual_coords_at_pos, Position, RopeSlice, Selection, Transaction,
+    pos_at_visual_coords, visual_col_position, visual_coords_at_pos, Position, RopeSlice,
+    Selection, Transaction,
 };
 
 use std::{collections::VecDeque, fmt};
@@ -242,19 +243,17 @@ impl View {
         text: RopeSlice,
         pos: usize,
     ) -> Option<Position> {
-        let line = text.char_to_line(pos);
+        let row = text.char_to_line(pos);
 
-        if line < self.offset.row || line > self.last_line(doc) {
+        if row < self.offset.row || row > self.last_line(doc) {
             // Line is not visible on screen
             return None;
         }
 
-        let tab_width = doc.tab_width();
-        // TODO: visual_coords_at_pos also does char_to_line which we ignore, can we reuse the call?
-        let Position { col, .. } = visual_coords_at_pos(text, pos, tab_width);
+        let col = visual_col_position(text, pos, row, doc.tab_width());
 
         // It is possible for underflow to occur if the buffer length is larger than the terminal width.
-        let row = line.saturating_sub(self.offset.row);
+        let row = row.saturating_sub(self.offset.row);
         let col = col.saturating_sub(self.offset.col);
 
         Some(Position::new(row, col))