Use proper, limited excerpt ranges and manage inlay cache properly

Kirill Bulatov created

Change summary

crates/editor/src/display_map.rs |   4 
crates/editor/src/editor.rs      | 146 +++++++++++++++++++++++++--------
crates/editor/src/inlay_cache.rs |  53 ++++++++---
crates/editor/src/scroll.rs      |   7 +
4 files changed, 157 insertions(+), 53 deletions(-)

Detailed changes

crates/editor/src/display_map.rs 🔗

@@ -249,6 +249,10 @@ impl DisplayMap {
         to_insert: Vec<(InlayId, InlayProperties<T>)>,
         cx: &mut ModelContext<Self>,
     ) {
+        if to_remove.is_empty() && to_insert.is_empty() {
+            return;
+        }
+
         let buffer_snapshot = self.buffer.read(cx).snapshot(cx);
         let edits = self.buffer_subscription.consume().into_inner();
         let (snapshot, edits) = self.inlay_map.sync(buffer_snapshot, edits);

crates/editor/src/editor.rs 🔗

@@ -55,7 +55,7 @@ use gpui::{
 use highlight_matching_bracket::refresh_matching_bracket_highlights;
 use hover_popover::{hide_hover, HoverState};
 use inlay_cache::{
-    Inlay, InlayCache, InlayId, InlayProperties, InlayRefreshReason, InlaySplice, QueryInlaysRange,
+    Inlay, InlayCache, InlayFetchRange, InlayId, InlayProperties, InlayRefreshReason, InlaySplice,
 };
 pub use items::MAX_TAB_TITLE_LEN;
 use itertools::Itertools;
@@ -1302,7 +1302,7 @@ impl Editor {
                 }
                 project_subscriptions.push(cx.subscribe(project, |editor, _, event, cx| {
                     if let project::Event::RefreshInlays = event {
-                        editor.refresh_inlays(InlayRefreshReason::Regular, cx);
+                        editor.refresh_inlays(InlayRefreshReason::OpenExcerptsChange, cx);
                     };
                 }));
             }
@@ -1357,7 +1357,6 @@ impl Editor {
             hover_state: Default::default(),
             link_go_to_definition_state: Default::default(),
             copilot_state: Default::default(),
-            // TODO kb has to live between editor reopens
             inlay_cache: InlayCache::new(settings::get::<EditorSettings>(cx).inlay_hints),
             gutter_hovered: false,
             _subscriptions: vec![
@@ -1383,7 +1382,7 @@ impl Editor {
         }
 
         this.report_editor_event("open", None, cx);
-        this.refresh_inlays(InlayRefreshReason::Regular, cx);
+        this.refresh_inlays(InlayRefreshReason::OpenExcerptsChange, cx);
         this
     }
 
@@ -2606,36 +2605,35 @@ impl Editor {
             return;
         }
 
+        let multi_buffer_handle = self.buffer().clone();
         match reason {
-            InlayRefreshReason::Settings(new_settings) => {
+            InlayRefreshReason::SettingsChange(new_settings) => {
                 let InlaySplice {
                     to_remove,
                     to_insert,
                 } = self.inlay_cache.apply_settings(new_settings);
                 self.splice_inlay_hints(to_remove, to_insert, cx);
             }
-            InlayRefreshReason::Regular => {
-                let buffer_handle = self.buffer().clone();
-                let inlay_fetch_ranges = buffer_handle
-                    .read(cx)
-                    .snapshot(cx)
-                    .excerpts()
-                    .filter_map(|(excerpt_id, buffer_snapshot, excerpt_range)| {
-                        let buffer_path = buffer_snapshot.resolve_file_path(cx, true)?;
-                        let buffer_id = buffer_snapshot.remote_id();
-                        let buffer_version = buffer_snapshot.version().clone();
-                        let max_buffer_offset = buffer_snapshot.len();
-                        let excerpt_range = excerpt_range.context;
-                        Some(QueryInlaysRange {
-                            buffer_path,
-                            buffer_id,
-                            buffer_version,
-                            excerpt_id,
-                            excerpt_offset_range: excerpt_range.start.offset
-                                ..excerpt_range.end.offset.min(max_buffer_offset),
-                        })
+            InlayRefreshReason::Scroll(scrolled_to) => {
+                let ranges_to_add = self
+                    .excerpt_visible_offsets(&multi_buffer_handle, cx)
+                    .into_iter()
+                    .find_map(|(buffer, excerpt_visible_offset_range, excerpt_id)| {
+                        let buffer_id = scrolled_to.anchor.buffer_id?;
+                        if buffer_id == buffer.read(cx).remote_id()
+                            && scrolled_to.anchor.excerpt_id == excerpt_id
+                        {
+                            get_inlay_fetch_range(
+                                &buffer,
+                                excerpt_id,
+                                excerpt_visible_offset_range,
+                                cx,
+                            )
+                        } else {
+                            None
+                        }
                     })
-                    .collect::<Vec<_>>();
+                    .into_iter();
 
                 cx.spawn(|editor, mut cx| async move {
                     let InlaySplice {
@@ -2643,9 +2641,36 @@ impl Editor {
                         to_insert,
                     } = editor
                         .update(&mut cx, |editor, cx| {
-                            editor.inlay_cache.fetch_inlays(
-                                buffer_handle,
-                                inlay_fetch_ranges.into_iter(),
+                            editor
+                                .inlay_cache
+                                .append_inlays(multi_buffer_handle, ranges_to_add, cx)
+                        })?
+                        .await
+                        .context("inlay cache hint fetch")?;
+
+                    editor.update(&mut cx, |editor, cx| {
+                        editor.splice_inlay_hints(to_remove, to_insert, cx)
+                    })
+                })
+                .detach_and_log_err(cx);
+            }
+            InlayRefreshReason::OpenExcerptsChange => {
+                let new_ranges = self
+                    .excerpt_visible_offsets(&multi_buffer_handle, cx)
+                    .into_iter()
+                    .filter_map(|(buffer, excerpt_visible_offset_range, excerpt_id)| {
+                        get_inlay_fetch_range(&buffer, excerpt_id, excerpt_visible_offset_range, cx)
+                    })
+                    .collect::<Vec<_>>();
+                cx.spawn(|editor, mut cx| async move {
+                    let InlaySplice {
+                        to_remove,
+                        to_insert,
+                    } = editor
+                        .update(&mut cx, |editor, cx| {
+                            editor.inlay_cache.replace_inlays(
+                                multi_buffer_handle,
+                                new_ranges.into_iter(),
                                 cx,
                             )
                         })?
@@ -2656,9 +2681,32 @@ impl Editor {
                         editor.splice_inlay_hints(to_remove, to_insert, cx)
                     })
                 })
+                // TODO kb needs cancellation for many excerpts cases like `project search "test"`
                 .detach_and_log_err(cx);
             }
-        }
+        };
+    }
+
+    fn excerpt_visible_offsets(
+        &self,
+        multi_buffer: &ModelHandle<MultiBuffer>,
+        cx: &mut ViewContext<'_, '_, Editor>,
+    ) -> Vec<(ModelHandle<Buffer>, Range<usize>, ExcerptId)> {
+        let multi_buffer = multi_buffer.read(cx);
+        let multi_buffer_snapshot = multi_buffer.snapshot(cx);
+        let multi_buffer_visible_start = self
+            .scroll_manager
+            .anchor()
+            .anchor
+            .to_point(&multi_buffer_snapshot);
+        let multi_buffer_visible_end = multi_buffer_snapshot.clip_point(
+            multi_buffer_visible_start
+                + Point::new(self.visible_line_count().unwrap_or(0.).ceil() as u32, 0),
+            Bias::Left,
+        );
+        let multi_buffer_visible_range = multi_buffer_visible_start..multi_buffer_visible_end;
+
+        multi_buffer.range_to_buffer_ranges(multi_buffer_visible_range, cx)
     }
 
     fn splice_inlay_hints(
@@ -7245,7 +7293,7 @@ impl Editor {
         event: &multi_buffer::Event,
         cx: &mut ViewContext<Self>,
     ) {
-        let refresh_inlay_hints = match event {
+        let refresh_inlays = match event {
             multi_buffer::Event::Edited => {
                 self.refresh_active_diagnostics(cx);
                 self.refresh_code_actions(cx);
@@ -7293,7 +7341,7 @@ impl Editor {
             }
             multi_buffer::Event::DiffBaseChanged => {
                 cx.emit(Event::DiffBaseChanged);
-                true
+                false
             }
             multi_buffer::Event::Closed => {
                 cx.emit(Event::Closed);
@@ -7306,8 +7354,8 @@ impl Editor {
             _ => false,
         };
 
-        if refresh_inlay_hints {
-            self.refresh_inlays(InlayRefreshReason::Regular, cx);
+        if refresh_inlays {
+            self.refresh_inlays(InlayRefreshReason::OpenExcerptsChange, cx);
         }
     }
 
@@ -7318,7 +7366,7 @@ impl Editor {
     fn settings_changed(&mut self, cx: &mut ViewContext<Self>) {
         self.refresh_copilot_suggestions(true, cx);
         self.refresh_inlays(
-            InlayRefreshReason::Settings(settings::get::<EditorSettings>(cx).inlay_hints),
+            InlayRefreshReason::SettingsChange(settings::get::<EditorSettings>(cx).inlay_hints),
             cx,
         );
     }
@@ -7612,6 +7660,34 @@ impl Editor {
     }
 }
 
+fn get_inlay_fetch_range(
+    buffer: &ModelHandle<Buffer>,
+    excerpt_id: ExcerptId,
+    excerpt_visible_offset_range: Range<usize>,
+    cx: &mut ViewContext<'_, '_, Editor>,
+) -> Option<InlayFetchRange> {
+    let buffer = buffer.read(cx);
+    let buffer_snapshot = buffer.snapshot();
+    let max_buffer_len = buffer.len();
+    let visible_offset_range_len = excerpt_visible_offset_range.len();
+
+    let query_range_start = excerpt_visible_offset_range
+        .start
+        .saturating_sub(visible_offset_range_len);
+    let query_range_end = max_buffer_len.min(
+        excerpt_visible_offset_range
+            .end
+            .saturating_add(visible_offset_range_len),
+    );
+    Some(InlayFetchRange {
+        buffer_path: buffer_snapshot.resolve_file_path(cx, true)?,
+        buffer_id: buffer.remote_id(),
+        buffer_version: buffer.version().clone(),
+        excerpt_id,
+        excerpt_offset_query_range: query_range_start..query_range_end,
+    })
+}
+
 fn consume_contiguous_rows(
     contiguous_row_selections: &mut Vec<Selection<Point>>,
     selection: &Selection<Point>,

crates/editor/src/inlay_cache.rs 🔗

@@ -4,7 +4,7 @@ use std::{
     path::{Path, PathBuf},
 };
 
-use crate::{editor_settings, Anchor, Editor, ExcerptId, MultiBuffer};
+use crate::{editor_settings, scroll::ScrollAnchor, Anchor, Editor, ExcerptId, MultiBuffer};
 use anyhow::Context;
 use clock::{Global, Local};
 use gpui::{ModelHandle, Task, ViewContext};
@@ -29,8 +29,9 @@ pub struct InlayProperties<T> {
 
 #[derive(Debug, Copy, Clone)]
 pub enum InlayRefreshReason {
-    Settings(editor_settings::InlayHints),
-    Regular,
+    SettingsChange(editor_settings::InlayHints),
+    Scroll(ScrollAnchor),
+    OpenExcerptsChange,
 }
 
 #[derive(Debug, Clone, Default)]
@@ -88,12 +89,12 @@ pub struct InlaySplice {
     pub to_insert: Vec<(InlayId, Anchor, InlayHint)>,
 }
 
-pub struct QueryInlaysRange {
+pub struct InlayFetchRange {
     pub buffer_id: u64,
     pub buffer_path: PathBuf,
     pub buffer_version: Global,
     pub excerpt_id: ExcerptId,
-    pub excerpt_offset_range: Range<usize>,
+    pub excerpt_offset_query_range: Range<usize>,
 }
 
 impl InlayCache {
@@ -105,10 +106,29 @@ impl InlayCache {
         }
     }
 
-    pub fn fetch_inlays(
+    pub fn append_inlays(
         &mut self,
         multi_buffer: ModelHandle<MultiBuffer>,
-        inlay_fetch_ranges: impl Iterator<Item = QueryInlaysRange>,
+        ranges_to_add: impl Iterator<Item = InlayFetchRange>,
+        cx: &mut ViewContext<Editor>,
+    ) -> Task<anyhow::Result<InlaySplice>> {
+        self.fetch_inlays(multi_buffer, ranges_to_add, false, cx)
+    }
+
+    pub fn replace_inlays(
+        &mut self,
+        multi_buffer: ModelHandle<MultiBuffer>,
+        new_ranges: impl Iterator<Item = InlayFetchRange>,
+        cx: &mut ViewContext<Editor>,
+    ) -> Task<anyhow::Result<InlaySplice>> {
+        self.fetch_inlays(multi_buffer, new_ranges, true, cx)
+    }
+
+    fn fetch_inlays(
+        &mut self,
+        multi_buffer: ModelHandle<MultiBuffer>,
+        inlay_fetch_ranges: impl Iterator<Item = InlayFetchRange>,
+        replace_old: bool,
         cx: &mut ViewContext<Editor>,
     ) -> Task<anyhow::Result<InlaySplice>> {
         let mut inlay_fetch_tasks = Vec::new();
@@ -127,13 +147,11 @@ impl InlayCache {
                         else { return Ok((inlay_fetch_range, Some(Vec::new()))) };
                     let task = editor
                         .update(&mut cx, |editor, cx| {
-                            let max_buffer_offset = buffer_handle.read(cx).len();
-                            let excerpt_offset_range = &inlay_fetch_range.excerpt_offset_range;
                             editor.project.as_ref().map(|project| {
                                 project.update(cx, |project, cx| {
                                     project.query_inlay_hints_for_buffer(
                                         buffer_handle,
-                                        excerpt_offset_range.start..excerpt_offset_range.end.min(max_buffer_offset),
+                                        inlay_fetch_range.excerpt_offset_query_range.clone(),
                                         cx,
                                     )
                                 })
@@ -163,16 +181,17 @@ impl InlayCache {
 
             for task_result in futures::future::join_all(inlay_fetch_tasks).await {
                 match task_result {
-                    Ok((request_key, response_inlays)) => {
+                    Ok((inlay_fetch_range, response_inlays)) => {
+                        // TODO kb different caching now
                         let inlays_per_excerpt = HashMap::from_iter([(
-                            request_key.excerpt_id,
+                            inlay_fetch_range.excerpt_id,
                             response_inlays
                                 .map(|excerpt_inlays| {
                                     excerpt_inlays.into_iter().fold(
                                         OrderedByAnchorOffset::default(),
                                         |mut ordered_inlays, inlay| {
                                             let anchor = multi_buffer_snapshot.anchor_in_excerpt(
-                                                request_key.excerpt_id,
+                                                inlay_fetch_range.excerpt_id,
                                                 inlay.position,
                                             );
                                             ordered_inlays.add(anchor, inlay);
@@ -180,14 +199,16 @@ impl InlayCache {
                                         },
                                     )
                                 })
-                                .map(|inlays| (request_key.excerpt_offset_range, inlays)),
+                                .map(|inlays| {
+                                    (inlay_fetch_range.excerpt_offset_query_range, inlays)
+                                }),
                         )]);
-                        match inlay_updates.entry(request_key.buffer_path) {
+                        match inlay_updates.entry(inlay_fetch_range.buffer_path) {
                             hash_map::Entry::Occupied(mut o) => {
                                 o.get_mut().1.extend(inlays_per_excerpt);
                             }
                             hash_map::Entry::Vacant(v) => {
-                                v.insert((request_key.buffer_version, inlays_per_excerpt));
+                                v.insert((inlay_fetch_range.buffer_version, inlays_per_excerpt));
                             }
                         }
                     }

crates/editor/src/scroll.rs 🔗

@@ -18,6 +18,7 @@ use workspace::WorkspaceId;
 use crate::{
     display_map::{DisplaySnapshot, ToDisplayPoint},
     hover_popover::hide_hover,
+    inlay_cache::InlayRefreshReason,
     persistence::DB,
     Anchor, DisplayPoint, Editor, EditorMode, Event, MultiBufferSnapshot, ToPoint,
 };
@@ -176,7 +177,7 @@ impl ScrollManager {
         autoscroll: bool,
         workspace_id: Option<i64>,
         cx: &mut ViewContext<Editor>,
-    ) {
+    ) -> ScrollAnchor {
         let (new_anchor, top_row) = if scroll_position.y() <= 0. {
             (
                 ScrollAnchor {
@@ -205,6 +206,7 @@ impl ScrollManager {
         };
 
         self.set_anchor(new_anchor, top_row, local, autoscroll, workspace_id, cx);
+        new_anchor
     }
 
     fn set_anchor(
@@ -312,7 +314,7 @@ impl Editor {
 
         hide_hover(self, cx);
         let workspace_id = self.workspace.as_ref().map(|workspace| workspace.1);
-        self.scroll_manager.set_scroll_position(
+        let scroll_anchor = self.scroll_manager.set_scroll_position(
             scroll_position,
             &map,
             local,
@@ -320,6 +322,7 @@ impl Editor {
             workspace_id,
             cx,
         );
+        self.refresh_inlays(InlayRefreshReason::Scroll(scroll_anchor), cx);
     }
 
     pub fn scroll_position(&self, cx: &mut ViewContext<Self>) -> Vector2F {