Introduce a short-term solution for flickering (zed-industries#8542)

This uses bounds checking alone to determine hover state to avoid flicker. It's a short-term solution because the rendering is incorrect. We think this is better than flickering though and buys us some time as we work on a more robust solution overall. Release Notes: - Fixed flickering when hovering. --------- Co-authored-by: Nathan <[email protected]>
RemcoSmitsDev · Feb 28, 2024 · 7aba9eb · 7aba9eb
1 parent 517ea73
commit 7aba9eb
Show file tree

Hide file tree

Showing 7 changed files with 72 additions and 45 deletions.
diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs
@@ -923,9 +923,15 @@ impl EditorElement {
                         .as_ref()
                         .is_some_and(|hovered_link_state| !hovered_link_state.links.is_empty())
                     {
-                        cx.set_cursor_style(CursorStyle::PointingHand);
+                        cx.set_cursor_style(
+                            CursorStyle::PointingHand,
+                            interactive_text_bounds.stacking_order.clone(),
+                        );
                     } else {
-                        cx.set_cursor_style(CursorStyle::IBeam);
+                        cx.set_cursor_style(
+                            CursorStyle::IBeam,
+                            interactive_text_bounds.stacking_order.clone(),
+                        );
                     }
                 }
 
@@ -1552,7 +1558,10 @@ impl EditorElement {
         };
         let mut mouse_position = cx.mouse_position();
         if interactive_track_bounds.visibly_contains(&mouse_position, cx) {
-            cx.set_cursor_style(CursorStyle::Arrow);
+            cx.set_cursor_style(
+                CursorStyle::Arrow,
+                interactive_track_bounds.stacking_order.clone(),
+            );
         }
 
         cx.on_mouse_event({

diff --git a/crates/gpui/src/elements/div.rs b/crates/gpui/src/elements/div.rs
@@ -1221,7 +1221,8 @@ pub struct InteractiveBounds {
 }
 
 impl InteractiveBounds {
-    /// Checks whether this point was inside these bounds, and that these bounds where the topmost layer
+    /// Checks whether this point was inside these bounds in the rendered frame, and that these bounds where the topmost layer
+    /// Never call this during paint to perform hover calculations. It will reference the previous frame and could cause flicker.
     pub fn visibly_contains(&self, point: &Point<Pixels>, cx: &WindowContext) -> bool {
         self.bounds.contains(point) && cx.was_top_layer(point, &self.stacking_order)
     }
@@ -1449,11 +1450,12 @@ impl Interactivity {
 
                         if !cx.has_active_drag() {
                             if let Some(mouse_cursor) = style.mouse_cursor {
-                                let mouse_position = &cx.mouse_position();
-                                let hovered =
-                                    interactive_bounds.visibly_contains(mouse_position, cx);
+                                let hovered = bounds.contains(&cx.mouse_position());
                                 if hovered {
-                                    cx.set_cursor_style(mouse_cursor);
+                                    cx.set_cursor_style(
+                                        mouse_cursor,
+                                        interactive_bounds.stacking_order.clone(),
+                                    );
                                 }
                             }
                         }
@@ -1955,9 +1957,7 @@ impl Interactivity {
                         if let Some(group_bounds) =
                             GroupBounds::get(&group_hover.group, cx.deref_mut())
                         {
-                            if group_bounds.contains(&mouse_position)
-                                && cx.was_top_layer(&mouse_position, cx.stacking_order())
-                            {
+                            if group_bounds.contains(&mouse_position) {
                                 style.refine(&group_hover.style);
                             }
                         }
@@ -1967,7 +1967,6 @@ impl Interactivity {
                         if bounds
                             .intersect(&cx.content_mask().bounds)
                             .contains(&mouse_position)
-                            && cx.was_top_layer(&mouse_position, cx.stacking_order())
                         {
                             style.refine(hover_style);
                         }

diff --git a/crates/gpui/src/elements/text.rs b/crates/gpui/src/elements/text.rs
@@ -427,9 +427,9 @@ impl Element for InteractiveText {
                     .clickable_ranges
                     .iter()
                     .any(|range| range.contains(&ix))
-                    && cx.was_top_layer(&mouse_position, cx.stacking_order())
                 {
-                    cx.set_cursor_style(crate::CursorStyle::PointingHand)
+                    let stacking_order = cx.stacking_order().clone();
+                    cx.set_cursor_style(crate::CursorStyle::PointingHand, stacking_order);
                 }
             }
 

diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs
@@ -1023,13 +1023,10 @@ impl<'a> WindowContext<'a> {
         self.window.root_view = Some(root_view);
 
         // Set the cursor only if we're the active window.
-        let cursor_style = self
-            .window
-            .next_frame
-            .requested_cursor_style
-            .take()
-            .unwrap_or(CursorStyle::Arrow);
+        let cursor_style_request = self.window.next_frame.requested_cursor_style.take();
         if self.is_window_active() {
+            let cursor_style =
+                cursor_style_request.map_or(CursorStyle::Arrow, |request| request.style);
             self.platform.set_cursor_style(cursor_style);
         }
 

diff --git a/crates/gpui/src/window/element_cx.rs b/crates/gpui/src/window/element_cx.rs
@@ -51,6 +51,12 @@ pub(crate) struct TooltipRequest {
     pub(crate) tooltip: AnyTooltip,
 }
 
+#[derive(Clone)]
+pub(crate) struct CursorStyleRequest {
+    pub(crate) style: CursorStyle,
+    stacking_order: StackingOrder,
+}
+
 pub(crate) struct Frame {
     pub(crate) focus: Option<FocusId>,
     pub(crate) window_active: bool,
@@ -66,8 +72,8 @@ pub(crate) struct Frame {
     pub(crate) element_offset_stack: Vec<Point<Pixels>>,
     pub(crate) requested_input_handler: Option<RequestedInputHandler>,
     pub(crate) tooltip_request: Option<TooltipRequest>,
-    pub(crate) cursor_styles: FxHashMap<EntityId, CursorStyle>,
-    pub(crate) requested_cursor_style: Option<CursorStyle>,
+    pub(crate) cursor_styles: FxHashMap<EntityId, CursorStyleRequest>,
+    pub(crate) requested_cursor_style: Option<CursorStyleRequest>,
     pub(crate) view_stack: Vec<EntityId>,
     pub(crate) reused_views: FxHashSet<EntityId>,
 
@@ -346,9 +352,13 @@ impl<'a> ElementContext<'a> {
             }
 
             // Reuse the cursor styles previously requested during painting of the reused view.
-            if let Some(style) = self.window.rendered_frame.cursor_styles.remove(&view_id) {
-                self.window.next_frame.cursor_styles.insert(view_id, style);
-                self.window.next_frame.requested_cursor_style = Some(style);
+            if let Some(cursor_style_request) =
+                self.window.rendered_frame.cursor_styles.remove(&view_id)
+            {
+                self.set_cursor_style(
+                    cursor_style_request.style,
+                    cursor_style_request.stacking_order,
+                );
             }
         }
 
@@ -387,10 +397,27 @@ impl<'a> ElementContext<'a> {
     }
 
     /// Updates the cursor style at the platform level.
-    pub fn set_cursor_style(&mut self, style: CursorStyle) {
+    pub fn set_cursor_style(&mut self, style: CursorStyle, stacking_order: StackingOrder) {
         let view_id = self.parent_view_id();
-        self.window.next_frame.cursor_styles.insert(view_id, style);
-        self.window.next_frame.requested_cursor_style = Some(style);
+        let style_request = CursorStyleRequest {
+            style,
+            stacking_order,
+        };
+        if self
+            .window
+            .next_frame
+            .requested_cursor_style
+            .as_ref()
+            .map_or(true, |prev_style_request| {
+                style_request.stacking_order > prev_style_request.stacking_order
+            })
+        {
+            self.window.next_frame.requested_cursor_style = Some(style_request.clone());
+        }
+        self.window
+            .next_frame
+            .cursor_styles
+            .insert(view_id, style_request);
     }
 
     /// Sets a tooltip to be rendered for the upcoming frame

diff --git a/crates/terminal_view/src/terminal_element.rs b/crates/terminal_view/src/terminal_element.rs
@@ -489,15 +489,12 @@ impl TerminalElement {
             }
         });
 
-        let interactive_text_bounds = InteractiveBounds {
-            bounds,
-            stacking_order: cx.stacking_order().clone(),
-        };
-        if interactive_text_bounds.visibly_contains(&cx.mouse_position(), cx) {
+        if bounds.contains(&cx.mouse_position()) {
+            let stacking_order = cx.stacking_order().clone();
             if self.can_navigate_to_selected_word && last_hovered_word.is_some() {
-                cx.set_cursor_style(gpui::CursorStyle::PointingHand)
+                cx.set_cursor_style(gpui::CursorStyle::PointingHand, stacking_order);
             } else {
-                cx.set_cursor_style(gpui::CursorStyle::IBeam)
+                cx.set_cursor_style(gpui::CursorStyle::IBeam, stacking_order);
             }
         }
 

diff --git a/crates/workspace/src/pane_group.rs b/crates/workspace/src/pane_group.rs
@@ -588,9 +588,9 @@ mod element {
     use std::{cell::RefCell, iter, rc::Rc, sync::Arc};
 
     use gpui::{
-        px, relative, Along, AnyElement, Axis, Bounds, CursorStyle, Element, InteractiveBounds,
-        IntoElement, MouseDownEvent, MouseMoveEvent, MouseUpEvent, ParentElement, Pixels, Point,
-        Size, Style, WeakView, WindowContext,
+        px, relative, Along, AnyElement, Axis, Bounds, CursorStyle, Element, IntoElement,
+        MouseDownEvent, MouseMoveEvent, MouseUpEvent, ParentElement, Pixels, Point, Size, Style,
+        WeakView, WindowContext,
     };
     use parking_lot::Mutex;
     use settings::Settings;
@@ -754,15 +754,13 @@ mod element {
             };
 
             cx.with_z_index(3, |cx| {
-                let interactive_handle_bounds = InteractiveBounds {
-                    bounds: handle_bounds,
-                    stacking_order: cx.stacking_order().clone(),
-                };
-                if interactive_handle_bounds.visibly_contains(&cx.mouse_position(), cx) {
-                    cx.set_cursor_style(match axis {
+                if handle_bounds.contains(&cx.mouse_position()) {
+                    let stacking_order = cx.stacking_order().clone();
+                    let cursor_style = match axis {
                         Axis::Vertical => CursorStyle::ResizeUpDown,
                         Axis::Horizontal => CursorStyle::ResizeLeftRight,
-                    })
+                    };
+                    cx.set_cursor_style(cursor_style, stacking_order);
                 }
 
                 cx.add_opaque_layer(handle_bounds);