Implement bounds_for_range, so that text input windows are positioned correctly (#3278)

![Screenshot 2023-11-08 at 5 33
10 PM](https://github.com/zed-industries/zed/assets/326587/09efd785-2c43-41b2-9429-c17067497fd2)

![Screenshot 2023-11-08 at 5 33
28 PM](https://github.com/zed-industries/zed/assets/326587/14a9faee-3547-40b6-a31f-16f653cdcb36)

crates/editor2/src/display_map.rs

@@ -572,7 +572,6 @@ impl DisplaySnapshot {
     ) -> Line {
         let mut runs = Vec::new();
         let mut line = String::new();
-        let mut ended_in_newline = false;
 
         let range = display_row..display_row + 1;
         for chunk in self.highlighted_chunks(range, false, &editor_style) {
@@ -588,17 +587,18 @@ impl DisplaySnapshot {
             } else {
                 Cow::Borrowed(&editor_style.text)
             };
-            ended_in_newline = chunk.chunk.ends_with("\n");
 
             runs.push(text_style.to_run(chunk.chunk.len()))
         }
 
-        // our pixel positioning logic assumes each line ends in \n,
-        // this is almost always true except for the last line which
-        // may have no trailing newline.
-        if !ended_in_newline && display_row == self.max_point().row() {
-            line.push_str("\n");
-            runs.push(editor_style.text.to_run("\n".len()));
+        if line.ends_with('\n') {
+            line.pop();
+            if let Some(last_run) = runs.last_mut() {
+                last_run.len -= 1;
+                if last_run.len == 0 {
+                    runs.pop();
+                }
+            }
         }
 
         let font_size = editor_style.text.font_size.to_pixels(*rem_size);

crates/editor2/src/editor.rs

@@ -39,11 +39,10 @@ use futures::FutureExt;
 use fuzzy::{StringMatch, StringMatchCandidate};
 use git::diff_hunk_to_display;
 use gpui::{
-    action, actions, div, px, relative, rems, AnyElement, AppContext, BackgroundExecutor,
-    ClipboardItem, Context, DispatchContext, Div, Element, Entity, EventEmitter, FocusHandle,
-    FontFeatures, FontStyle, FontWeight, HighlightStyle, Hsla, InputHandler, Model, Pixels,
-    PlatformInputHandler, Render, Styled, Subscription, Task, TextStyle, View, ViewContext,
-    VisualContext, WeakView, WindowContext,
+    action, actions, point, px, relative, rems, size, AnyElement, AppContext, BackgroundExecutor,
+    Bounds, ClipboardItem, Context, DispatchContext, EventEmitter, FocusHandle, FontFeatures,
+    FontStyle, FontWeight, HighlightStyle, Hsla, InputHandler, Model, Pixels, Render, Subscription,
+    Task, TextStyle, View, ViewContext, VisualContext, WeakView, WindowContext,
 };
 use highlight_matching_bracket::refresh_matching_bracket_highlights;
 use hover_popover::{hide_hover, HoverState};
@@ -675,6 +674,7 @@ pub struct Editor {
     next_inlay_id: usize,
     _subscriptions: Vec<Subscription>,
     pixel_position_of_newest_cursor: Option<gpui::Point<Pixels>>,
+    gutter_width: Pixels,
     style: Option<EditorStyle>,
 }
 
@@ -1985,6 +1985,7 @@ impl Editor {
             inlay_hint_cache: InlayHintCache::new(inlay_hint_settings),
             gutter_hovered: false,
             pixel_position_of_newest_cursor: None,
+            gutter_width: Default::default(),
             style: None,
             _subscriptions: vec![
                 cx.observe(&buffer, Self::on_buffer_changed),
@@ -9775,13 +9776,35 @@ impl InputHandler for Editor {
     }
 
     fn bounds_for_range(
-        &self,
+        &mut self,
         range_utf16: Range<usize>,
+        element_bounds: gpui::Bounds<Pixels>,
         cx: &mut ViewContext<Self>,
-    ) -> Option<gpui::Bounds<f32>> {
-        // todo!()
-        // See how we did it before: `rect_for_range`
-        None
+    ) -> Option<gpui::Bounds<Pixels>> {
+        let text_layout_details = self.text_layout_details(cx);
+        let style = &text_layout_details.editor_style;
+        let font_id = cx.text_system().font_id(&style.text.font()).unwrap();
+        let font_size = style.text.font_size.to_pixels(cx.rem_size());
+        let line_height = style.text.line_height_in_pixels(cx.rem_size());
+        let em_width = cx
+            .text_system()
+            .typographic_bounds(font_id, font_size, 'm')
+            .unwrap()
+            .size
+            .width;
+
+        let snapshot = self.snapshot(cx);
+        let scroll_position = snapshot.scroll_position();
+        let scroll_left = scroll_position.x * em_width;
+
+        let start = OffsetUtf16(range_utf16.start).to_display_point(&snapshot);
+        let x = snapshot.x_for_point(start, &text_layout_details) - scroll_left + self.gutter_width;
+        let y = line_height * (start.row() as f32 - scroll_position.y);
+
+        Some(Bounds {
+            origin: element_bounds.origin + point(x, y),
+            size: size(em_width, line_height),
+        })
     }
 }
 

crates/editor2/src/element.rs

@@ -17,10 +17,11 @@ use collections::{BTreeMap, HashMap};
 use gpui::{
     black, hsla, point, px, relative, size, transparent_black, Action, AnyElement,
     BorrowAppContext, BorrowWindow, Bounds, ContentMask, Corners, DispatchContext, DispatchPhase,
-    Edges, Element, ElementId, Entity, GlobalElementId, Hsla, KeyDownEvent, KeyListener, KeyMatch,
-    Line, Modifiers, MouseButton, MouseDownEvent, MouseMoveEvent, MouseUpEvent, Pixels,
-    ScrollWheelEvent, ShapedGlyph, Size, Style, TextRun, TextStyle, TextSystem, ViewContext,
-    WindowContext,
+    Edges, Element, ElementId, Entity, FocusHandle, GlobalElementId, Hsla, InputHandler,
+    InputHandlerView, KeyDownEvent, KeyListener, KeyMatch, Line, LineLayout, Modifiers,
+    MouseButton, MouseDownEvent, MouseMoveEvent, MouseUpEvent, Pixels, ScrollWheelEvent,
+    ShapedGlyph, Size, Style, TextRun, TextStyle, TextSystem, ViewContext, WindowContext,
+    WrappedLineLayout,
 };
 use itertools::Itertools;
 use language::language_settings::ShowWhitespaceSetting;
@@ -1467,6 +1468,7 @@ impl EditorElement {
             gutter_margin = Pixels::ZERO;
         };
 
+        editor.gutter_width = gutter_width;
         let text_width = bounds.size.width - gutter_width;
         let overscroll = size(em_width, px(0.));
         let snapshot = {
@@ -2502,10 +2504,6 @@ impl Element<Editor> for EditorElement {
             size: layout.text_size,
         };
 
-        if editor.focus_handle.is_focused(cx) {
-            cx.handle_text_input();
-        }
-
         cx.with_content_mask(ContentMask { bounds }, |cx| {
             self.paint_mouse_listeners(
                 bounds,
@@ -2521,6 +2519,14 @@ impl Element<Editor> for EditorElement {
             self.paint_text(text_bounds, &layout, editor, cx);
         });
     }
+
+    fn handle_text_input<'a>(
+        &self,
+        editor: &'a mut Editor,
+        cx: &mut ViewContext<Editor>,
+    ) -> Option<(Box<dyn InputHandlerView>, &'a FocusHandle)> {
+        Some((Box::new(cx.view()), &editor.focus_handle))
+    }
 }
 
 // impl EditorElement {

crates/gpui2/src/element.rs

@@ -1,4 +1,7 @@
-use crate::{BorrowWindow, Bounds, ElementId, LayoutId, Pixels, ViewContext};
+use crate::{
+    BorrowWindow, Bounds, ElementId, FocusHandle, InputHandlerView, LayoutId, Pixels, ViewContext,
+    WindowInputHandler,
+};
 use derive_more::{Deref, DerefMut};
 pub(crate) use smallvec::SmallVec;
 use std::{any::Any, mem};
@@ -31,6 +34,14 @@ pub trait Element<V: 'static> {
         element_state: &mut Self::ElementState,
         cx: &mut ViewContext<V>,
     );
+
+    fn handle_text_input<'a>(
+        &self,
+        _view_state: &'a mut V,
+        _cx: &mut ViewContext<V>,
+    ) -> Option<(Box<dyn InputHandlerView>, &'a FocusHandle)> {
+        None
+    }
 }
 
 #[derive(Deref, DerefMut, Default, Clone, Debug, Eq, PartialEq, Hash)]
@@ -154,6 +165,18 @@ where
                 mut frame_state,
             } => {
                 let bounds = cx.layout_bounds(layout_id);
+                if let Some((input_handler, focus_handle)) =
+                    self.element.handle_text_input(view_state, cx)
+                {
+                    if focus_handle.is_focused(cx) {
+                        cx.window.requested_input_handler = Some(Box::new(WindowInputHandler {
+                            cx: cx.app.this.clone(),
+                            window: cx.window_handle(),
+                            input_handler,
+                            element_bounds: bounds,
+                        }));
+                    }
+                }
                 if let Some(id) = self.element.id() {
                     cx.with_element_state(id, |element_state, cx| {
                         let mut element_state = element_state.unwrap();

crates/gpui2/src/platform.rs

@@ -305,7 +305,7 @@ pub trait PlatformInputHandler {
         new_selected_range: Option<Range<usize>>,
     );
     fn unmark_text(&mut self);
-    fn bounds_for_range(&self, range_utf16: Range<usize>) -> Option<Bounds<f32>>;
+    fn bounds_for_range(&self, range_utf16: Range<usize>) -> Option<Bounds<Pixels>>;
 }
 
 #[derive(Debug)]

crates/gpui2/src/platform/mac/window.rs

@@ -1484,10 +1484,12 @@ extern "C" fn first_rect_for_character_range(
         |bounds| {
             NSRect::new(
                 NSPoint::new(
-                    frame.origin.x + bounds.origin.x as f64,
-                    frame.origin.y + frame.size.height - bounds.origin.y as f64,
+                    frame.origin.x + bounds.origin.x.0 as f64,
+                    frame.origin.y + frame.size.height
+                        - bounds.origin.y.0 as f64
+                        - bounds.size.height.0 as f64,
                 ),
-                NSSize::new(bounds.size.width as f64, bounds.size.height as f64),
+                NSSize::new(bounds.size.width.0 as f64, bounds.size.height.0 as f64),
             )
         },
     )

crates/gpui2/src/window.rs

@@ -2,14 +2,13 @@ use crate::{
     px, size, Action, AnyBox, AnyDrag, AnyView, AppContext, AsyncWindowContext, AvailableSpace,
     Bounds, BoxShadow, Context, Corners, CursorStyle, DevicePixels, DispatchContext, DisplayId,
     Edges, Effect, Entity, EntityId, EventEmitter, FileDropEvent, FocusEvent, FontId,
-    GlobalElementId, GlyphId, Hsla, ImageData, InputEvent, InputHandler, IsZero, KeyListener,
-    KeyMatch, KeyMatcher, Keystroke, LayoutId, Model, ModelContext, Modifiers, MonochromeSprite,
-    MouseButton, MouseDownEvent, MouseMoveEvent, MouseUpEvent, Path, Pixels, PlatformAtlas,
-    PlatformDisplay, PlatformInputHandler, PlatformWindow, Point, PolychromeSprite, PromptLevel,
-    Quad, Render, RenderGlyphParams, RenderImageParams, RenderSvgParams, ScaledPixels,
-    SceneBuilder, Shadow, SharedString, Size, Style, SubscriberSet, Subscription,
-    TaffyLayoutEngine, Task, Underline, UnderlineStyle, View, VisualContext, WeakView,
-    WindowBounds, WindowInputHandler, WindowOptions, SUBPIXEL_VARIANTS,
+    GlobalElementId, GlyphId, Hsla, ImageData, InputEvent, IsZero, KeyListener, KeyMatch,
+    KeyMatcher, Keystroke, LayoutId, Model, ModelContext, Modifiers, MonochromeSprite, MouseButton,
+    MouseDownEvent, MouseMoveEvent, MouseUpEvent, Path, Pixels, PlatformAtlas, PlatformDisplay,
+    PlatformInputHandler, PlatformWindow, Point, PolychromeSprite, PromptLevel, Quad, Render,
+    RenderGlyphParams, RenderImageParams, RenderSvgParams, ScaledPixels, SceneBuilder, Shadow,
+    SharedString, Size, Style, SubscriberSet, Subscription, TaffyLayoutEngine, Task, Underline,
+    UnderlineStyle, View, VisualContext, WeakView, WindowBounds, WindowOptions, SUBPIXEL_VARIANTS,
 };
 use anyhow::{anyhow, Result};
 use collections::HashMap;
@@ -212,7 +211,7 @@ pub struct Window {
     default_prevented: bool,
     mouse_position: Point<Pixels>,
     requested_cursor_style: Option<CursorStyle>,
-    requested_input_handler: Option<Box<dyn PlatformInputHandler>>,
+    pub(crate) requested_input_handler: Option<Box<dyn PlatformInputHandler>>,
     scale_factor: f32,
     bounds: WindowBounds,
     bounds_observers: SubscriberSet<(), AnyObserver>,
@@ -2177,19 +2176,6 @@ impl<'a, V: 'static> ViewContext<'a, V> {
     }
 }
 
-impl<V> ViewContext<'_, V>
-where
-    V: InputHandler + 'static,
-{
-    pub fn handle_text_input(&mut self) {
-        self.window.requested_input_handler = Some(Box::new(WindowInputHandler {
-            cx: self.app.this.clone(),
-            window: self.window_handle(),
-            handler: self.view().downgrade(),
-        }));
-    }
-}
-
 impl<V> ViewContext<'_, V>
 where
     V: EventEmitter,

crates/gpui2/src/window_input_handler.rs

@@ -1,66 +1,40 @@
-use crate::{AnyWindowHandle, AppCell, Context, PlatformInputHandler, ViewContext, WeakView};
+use crate::{
+    AnyWindowHandle, AppCell, Bounds, Context, Pixels, PlatformInputHandler, View, ViewContext,
+    WindowContext,
+};
 use std::{ops::Range, rc::Weak};
 
-pub struct WindowInputHandler<V>
-where
-    V: InputHandler,
-{
+pub struct WindowInputHandler {
     pub cx: Weak<AppCell>,
+    pub input_handler: Box<dyn InputHandlerView>,
     pub window: AnyWindowHandle,
-    pub handler: WeakView<V>,
+    pub element_bounds: Bounds<Pixels>,
 }
 
-impl<V: InputHandler + 'static> PlatformInputHandler for WindowInputHandler<V> {
-    fn selected_text_range(&self) -> Option<std::ops::Range<usize>> {
-        self.update(|view, cx| view.selected_text_range(cx))
-            .flatten()
-    }
-
-    fn marked_text_range(&self) -> Option<std::ops::Range<usize>> {
-        self.update(|view, cx| view.marked_text_range(cx)).flatten()
-    }
-
-    fn text_for_range(&self, range_utf16: std::ops::Range<usize>) -> Option<String> {
-        self.update(|view, cx| view.text_for_range(range_utf16, cx))
-            .flatten()
-    }
-
+pub trait InputHandlerView {
+    fn text_for_range(&self, range: Range<usize>, cx: &mut WindowContext) -> Option<String>;
+    fn selected_text_range(&self, cx: &mut WindowContext) -> Option<Range<usize>>;
+    fn marked_text_range(&self, cx: &mut WindowContext) -> Option<Range<usize>>;
+    fn unmark_text(&self, cx: &mut WindowContext);
     fn replace_text_in_range(
-        &mut self,
-        replacement_range: Option<std::ops::Range<usize>>,
+        &self,
+        range: Option<Range<usize>>,
         text: &str,
-    ) {
-        self.update(|view, cx| view.replace_text_in_range(replacement_range, text, cx));
-    }
-
+        cx: &mut WindowContext,
+    );
     fn replace_and_mark_text_in_range(
-        &mut self,
-        range_utf16: Option<std::ops::Range<usize>>,
+        &self,
+        range: Option<Range<usize>>,
         new_text: &str,
-        new_selected_range: Option<std::ops::Range<usize>>,
-    ) {
-        self.update(|view, cx| {
-            view.replace_and_mark_text_in_range(range_utf16, new_text, new_selected_range, cx)
-        });
-    }
-
-    fn unmark_text(&mut self) {
-        self.update(|view, cx| view.unmark_text(cx));
-    }
-
-    fn bounds_for_range(&self, range_utf16: std::ops::Range<usize>) -> Option<crate::Bounds<f32>> {
-        self.update(|view, cx| view.bounds_for_range(range_utf16, cx))
-            .flatten()
-    }
-}
-
-impl<V: InputHandler + 'static> WindowInputHandler<V> {
-    fn update<T>(&self, f: impl FnOnce(&mut V, &mut ViewContext<V>) -> T) -> Option<T> {
-        let cx = self.cx.upgrade()?;
-        let mut cx = cx.borrow_mut();
-        cx.update_window(self.window, |_, cx| self.handler.update(cx, f).ok())
-            .ok()?
-    }
+        new_selected_range: Option<Range<usize>>,
+        cx: &mut WindowContext,
+    );
+    fn bounds_for_range(
+        &self,
+        range_utf16: std::ops::Range<usize>,
+        element_bounds: crate::Bounds<Pixels>,
+        cx: &mut WindowContext,
+    ) -> Option<crate::Bounds<Pixels>>;
 }
 
 pub trait InputHandler: Sized {
@@ -81,9 +55,113 @@ pub trait InputHandler: Sized {
         new_selected_range: Option<Range<usize>>,
         cx: &mut ViewContext<Self>,
     );
+    fn bounds_for_range(
+        &mut self,
+        range_utf16: std::ops::Range<usize>,
+        element_bounds: crate::Bounds<Pixels>,
+        cx: &mut ViewContext<Self>,
+    ) -> Option<crate::Bounds<Pixels>>;
+}
+
+impl<V: InputHandler + 'static> InputHandlerView for View<V> {
+    fn text_for_range(&self, range: Range<usize>, cx: &mut WindowContext) -> Option<String> {
+        self.update(cx, |this, cx| this.text_for_range(range, cx))
+    }
+
+    fn selected_text_range(&self, cx: &mut WindowContext) -> Option<Range<usize>> {
+        self.update(cx, |this, cx| this.selected_text_range(cx))
+    }
+
+    fn marked_text_range(&self, cx: &mut WindowContext) -> Option<Range<usize>> {
+        self.update(cx, |this, cx| this.marked_text_range(cx))
+    }
+
+    fn unmark_text(&self, cx: &mut WindowContext) {
+        self.update(cx, |this, cx| this.unmark_text(cx))
+    }
+
+    fn replace_text_in_range(
+        &self,
+        range: Option<Range<usize>>,
+        text: &str,
+        cx: &mut WindowContext,
+    ) {
+        self.update(cx, |this, cx| this.replace_text_in_range(range, text, cx))
+    }
+
+    fn replace_and_mark_text_in_range(
+        &self,
+        range: Option<Range<usize>>,
+        new_text: &str,
+        new_selected_range: Option<Range<usize>>,
+        cx: &mut WindowContext,
+    ) {
+        self.update(cx, |this, cx| {
+            this.replace_and_mark_text_in_range(range, new_text, new_selected_range, cx)
+        })
+    }
+
     fn bounds_for_range(
         &self,
         range_utf16: std::ops::Range<usize>,
-        cx: &mut ViewContext<Self>,
-    ) -> Option<crate::Bounds<f32>>;
+        element_bounds: crate::Bounds<Pixels>,
+        cx: &mut WindowContext,
+    ) -> Option<crate::Bounds<Pixels>> {
+        self.update(cx, |this, cx| {
+            this.bounds_for_range(range_utf16, element_bounds, cx)
+        })
+    }
+}
+
+impl PlatformInputHandler for WindowInputHandler {
+    fn selected_text_range(&self) -> Option<Range<usize>> {
+        self.update(|handler, cx| handler.selected_text_range(cx))
+            .flatten()
+    }
+
+    fn marked_text_range(&self) -> Option<Range<usize>> {
+        self.update(|handler, cx| handler.marked_text_range(cx))
+            .flatten()
+    }
+
+    fn text_for_range(&self, range_utf16: Range<usize>) -> Option<String> {
+        self.update(|handler, cx| handler.text_for_range(range_utf16, cx))
+            .flatten()
+    }
+
+    fn replace_text_in_range(&mut self, replacement_range: Option<Range<usize>>, text: &str) {
+        self.update(|handler, cx| handler.replace_text_in_range(replacement_range, text, cx));
+    }
+
+    fn replace_and_mark_text_in_range(
+        &mut self,
+        range_utf16: Option<Range<usize>>,
+        new_text: &str,
+        new_selected_range: Option<Range<usize>>,
+    ) {
+        self.update(|handler, cx| {
+            handler.replace_and_mark_text_in_range(range_utf16, new_text, new_selected_range, cx)
+        });
+    }
+
+    fn unmark_text(&mut self) {
+        self.update(|handler, cx| handler.unmark_text(cx));
+    }
+
+    fn bounds_for_range(&self, range_utf16: Range<usize>) -> Option<Bounds<Pixels>> {
+        self.update(|handler, cx| handler.bounds_for_range(range_utf16, self.element_bounds, cx))
+            .flatten()
+    }
+}
+
+impl WindowInputHandler {
+    fn update<R>(
+        &self,
+        f: impl FnOnce(&dyn InputHandlerView, &mut WindowContext) -> R,
+    ) -> Option<R> {
+        let cx = self.cx.upgrade()?;
+        let mut cx = cx.borrow_mut();
+        cx.update_window(self.window, |_, cx| f(&*self.input_handler, cx))
+            .ok()
+    }
 }

test.rs

@@ -10,49 +10,49 @@ use element::Element;
 use frame::frame;
 use gpui::{
     geometry::{rect::RectF, vector::vec2f},
-    platform::WindowOptions,
+    platform::WindowOptions,aa
 };
-use log::LevelFilter;
+use log::LevelFilter;a
 use simplelog::SimpleLogger;
 use themes::{rose_pine, ThemeColors};
-use view::view;
+use view::view;a
 mod adapter {
     use crate::element::AnyElement;
     use crate::element::{LayoutContext, PaintContext};
-    use gpui::{geometry::rect::RectF, LayoutEngine};
+    use gpui::{geometry::rect::RectF, LayoutEngine};aaaa
     use util::ResultExt;
     pub struct Adapter<V>(pub(crate) AnyElement<V>);
-    impl<V: 'static> gpui::Element<V> for Adapter<V> {
-        type LayoutState = Option<LayoutEngine>;
+    impl<V: 'static> gpui::Element<V> for Adapter<V> {aa
+        type LayoutState = Option<LayaoutEngine>;
         type PaintState = ();
         fn layout(
             &mut self,
             constraint: gpui::SizeConstraint,
             view: &mut V,
-            cx: &mut LayoutContext<V>,
+            cx: &mut LayoutContext<V>,aa
         ) -> (gpui::geometry::vector::Vector2F, Self::LayoutState) {
             cx.push_layout_engine(LayoutEngine::new());
-            let node = self.0.layout(view, cx).log_err();
+            let node = self.0.layout(view, cx).log_err();a
             if let Some(node) = node {
                 let layout_engine = cx.layout_engine().unwrap();
                 layout_engine.compute_layout(node, constraint.max).log_err();
             }
             let layout_engine = cx.pop_layout_engine();
-            if true {
+            if true {a
                 if !layout_engine.is_some() {
                     ::core::panicking::panic("assertion failed: layout_engine.is_some()")
                 }
             }
-            (constraint.max, layout_engine)
+            (constraint.max, layout_engine)a
         }
-        fn paint(
+        fn paint(a
             &mut self,
             scene: &mut gpui::SceneBuilder,
             bounds: RectF,
             visible_bounds: RectF,
             layout_engine: &mut Option<LayoutEngine>,
             view: &mut V,
-            legacy_cx: &mut gpui::PaintContext<V>,
+            legacy_cx: &mut gpui::PaintContext<V>,aaa
         ) -> Self::PaintState {
             legacy_cx.push_layout_engine(layout_engine.take().unwrap());
             let mut cx = PaintContext::new(legacy_cx, scene);