Introduce a short-term solution for flickering (#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 <nathan@zed.dev>
This commit is contained in:
Antonio Scandurra 2024-02-28 17:57:20 +01:00 committed by GitHub
parent 517ea734ee
commit 7aba9eb4b7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 72 additions and 45 deletions

View file

@ -923,9 +923,15 @@ impl EditorElement {
.as_ref() .as_ref()
.is_some_and(|hovered_link_state| !hovered_link_state.links.is_empty()) .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 { } 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(); let mut mouse_position = cx.mouse_position();
if interactive_track_bounds.visibly_contains(&mouse_position, cx) { 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({ cx.on_mouse_event({

View file

@ -1221,7 +1221,8 @@ pub struct InteractiveBounds {
} }
impl 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 { pub fn visibly_contains(&self, point: &Point<Pixels>, cx: &WindowContext) -> bool {
self.bounds.contains(point) && cx.was_top_layer(point, &self.stacking_order) self.bounds.contains(point) && cx.was_top_layer(point, &self.stacking_order)
} }
@ -1449,11 +1450,12 @@ impl Interactivity {
if !cx.has_active_drag() { if !cx.has_active_drag() {
if let Some(mouse_cursor) = style.mouse_cursor { if let Some(mouse_cursor) = style.mouse_cursor {
let mouse_position = &cx.mouse_position(); let hovered = bounds.contains(&cx.mouse_position());
let hovered =
interactive_bounds.visibly_contains(mouse_position, cx);
if hovered { 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) = if let Some(group_bounds) =
GroupBounds::get(&group_hover.group, cx.deref_mut()) GroupBounds::get(&group_hover.group, cx.deref_mut())
{ {
if group_bounds.contains(&mouse_position) if group_bounds.contains(&mouse_position) {
&& cx.was_top_layer(&mouse_position, cx.stacking_order())
{
style.refine(&group_hover.style); style.refine(&group_hover.style);
} }
} }
@ -1967,7 +1967,6 @@ impl Interactivity {
if bounds if bounds
.intersect(&cx.content_mask().bounds) .intersect(&cx.content_mask().bounds)
.contains(&mouse_position) .contains(&mouse_position)
&& cx.was_top_layer(&mouse_position, cx.stacking_order())
{ {
style.refine(hover_style); style.refine(hover_style);
} }

View file

@ -427,9 +427,9 @@ impl Element for InteractiveText {
.clickable_ranges .clickable_ranges
.iter() .iter()
.any(|range| range.contains(&ix)) .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);
} }
} }

View file

@ -1023,13 +1023,10 @@ impl<'a> WindowContext<'a> {
self.window.root_view = Some(root_view); self.window.root_view = Some(root_view);
// Set the cursor only if we're the active window. // Set the cursor only if we're the active window.
let cursor_style = self let cursor_style_request = self.window.next_frame.requested_cursor_style.take();
.window
.next_frame
.requested_cursor_style
.take()
.unwrap_or(CursorStyle::Arrow);
if self.is_window_active() { 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); self.platform.set_cursor_style(cursor_style);
} }

View file

@ -51,6 +51,12 @@ pub(crate) struct TooltipRequest {
pub(crate) tooltip: AnyTooltip, pub(crate) tooltip: AnyTooltip,
} }
#[derive(Clone)]
pub(crate) struct CursorStyleRequest {
pub(crate) style: CursorStyle,
stacking_order: StackingOrder,
}
pub(crate) struct Frame { pub(crate) struct Frame {
pub(crate) focus: Option<FocusId>, pub(crate) focus: Option<FocusId>,
pub(crate) window_active: bool, pub(crate) window_active: bool,
@ -66,8 +72,8 @@ pub(crate) struct Frame {
pub(crate) element_offset_stack: Vec<Point<Pixels>>, pub(crate) element_offset_stack: Vec<Point<Pixels>>,
pub(crate) requested_input_handler: Option<RequestedInputHandler>, pub(crate) requested_input_handler: Option<RequestedInputHandler>,
pub(crate) tooltip_request: Option<TooltipRequest>, pub(crate) tooltip_request: Option<TooltipRequest>,
pub(crate) cursor_styles: FxHashMap<EntityId, CursorStyle>, pub(crate) cursor_styles: FxHashMap<EntityId, CursorStyleRequest>,
pub(crate) requested_cursor_style: Option<CursorStyle>, pub(crate) requested_cursor_style: Option<CursorStyleRequest>,
pub(crate) view_stack: Vec<EntityId>, pub(crate) view_stack: Vec<EntityId>,
pub(crate) reused_views: FxHashSet<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. // 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) { if let Some(cursor_style_request) =
self.window.next_frame.cursor_styles.insert(view_id, style); self.window.rendered_frame.cursor_styles.remove(&view_id)
self.window.next_frame.requested_cursor_style = Some(style); {
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. /// 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(); let view_id = self.parent_view_id();
self.window.next_frame.cursor_styles.insert(view_id, style); let style_request = CursorStyleRequest {
self.window.next_frame.requested_cursor_style = Some(style); 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 /// Sets a tooltip to be rendered for the upcoming frame

View file

@ -489,15 +489,12 @@ impl TerminalElement {
} }
}); });
let interactive_text_bounds = InteractiveBounds { if bounds.contains(&cx.mouse_position()) {
bounds, let stacking_order = cx.stacking_order().clone();
stacking_order: cx.stacking_order().clone(),
};
if interactive_text_bounds.visibly_contains(&cx.mouse_position(), cx) {
if self.can_navigate_to_selected_word && last_hovered_word.is_some() { 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 { } else {
cx.set_cursor_style(gpui::CursorStyle::IBeam) cx.set_cursor_style(gpui::CursorStyle::IBeam, stacking_order);
} }
} }

View file

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