From cfc050e3fe2cd795941e50a246006834af372fc3 Mon Sep 17 00:00:00 2001 From: Julia Date: Wed, 13 Dec 2023 12:16:39 -0500 Subject: [PATCH] Fire focus handlers on draw to avoid timing with newly created item Co-Authored-By: Antonio Scandurra --- crates/gpui2/src/app.rs | 68 ++------------- crates/gpui2/src/elements/div.rs | 13 +-- crates/gpui2/src/interactive.rs | 8 +- crates/gpui2/src/key_dispatch.rs | 20 ++++- crates/gpui2/src/window.rs | 137 ++++++++++++------------------- 5 files changed, 84 insertions(+), 162 deletions(-) diff --git a/crates/gpui2/src/app.rs b/crates/gpui2/src/app.rs index 62ce6305ea7303362a5b6fb8889e63e18ca3fd68..9b9a5921e17dca7fa3c22571ca6f6f0fdc9e5c55 100644 --- a/crates/gpui2/src/app.rs +++ b/crates/gpui2/src/app.rs @@ -17,11 +17,10 @@ use time::UtcOffset; use crate::{ current_platform, image_cache::ImageCache, init_app_menus, Action, ActionRegistry, Any, AnyView, AnyWindowHandle, AppMetadata, AssetSource, BackgroundExecutor, ClipboardItem, Context, - DispatchPhase, DisplayId, Entity, EventEmitter, FocusEvent, FocusHandle, FocusId, - ForegroundExecutor, KeyBinding, Keymap, Keystroke, LayoutId, Menu, PathPromptOptions, Pixels, - Platform, PlatformDisplay, Point, Render, SharedString, SubscriberSet, Subscription, - SvgRenderer, Task, TextStyle, TextStyleRefinement, TextSystem, View, ViewContext, Window, - WindowContext, WindowHandle, WindowId, + DispatchPhase, DisplayId, Entity, EventEmitter, ForegroundExecutor, KeyBinding, Keymap, + Keystroke, LayoutId, Menu, PathPromptOptions, Pixels, Platform, PlatformDisplay, Point, Render, + SharedString, SubscriberSet, Subscription, SvgRenderer, Task, TextStyle, TextStyleRefinement, + TextSystem, View, ViewContext, Window, WindowContext, WindowHandle, WindowId, }; use anyhow::{anyhow, Result}; use collections::{FxHashMap, FxHashSet, VecDeque}; @@ -577,23 +576,21 @@ impl AppContext { Effect::Notify { emitter } => { self.apply_notify_effect(emitter); } + Effect::Emit { emitter, event_type, event, } => self.apply_emit_effect(emitter, event_type, event), - Effect::FocusChanged { - window_handle, - focused, - } => { - self.apply_focus_changed_effect(window_handle, focused); - } + Effect::Refresh => { self.apply_refresh_effect(); } + Effect::NotifyGlobalObservers { global_type } => { self.apply_notify_global_observers_effect(global_type); } + Effect::Defer { callback } => { self.apply_defer_effect(callback); } @@ -693,51 +690,6 @@ impl AppContext { }); } - fn apply_focus_changed_effect( - &mut self, - window_handle: AnyWindowHandle, - focused: Option, - ) { - window_handle - .update(self, |_, cx| { - // The window might change focus multiple times in an effect cycle. - // We only honor effects for the most recently focused handle. - if cx.window.focus == focused { - // if someone calls focus multiple times in one frame with the same handle - // the first apply_focus_changed_effect will have taken the last blur already - // and run the rest of this, so we can return. - let Some(last_blur) = cx.window.last_blur.take() else { - return; - }; - - let focused = focused - .map(|id| FocusHandle::for_id(id, &cx.window.focus_handles).unwrap()); - - let blurred = - last_blur.and_then(|id| FocusHandle::for_id(id, &cx.window.focus_handles)); - - let focus_changed = focused.is_some() || blurred.is_some(); - let event = FocusEvent { focused, blurred }; - - let mut listeners = mem::take(&mut cx.window.rendered_frame.focus_listeners); - if focus_changed { - for listener in &mut listeners { - listener(&event, cx); - } - } - cx.window.rendered_frame.focus_listeners = listeners; - - if focus_changed { - cx.window - .focus_listeners - .clone() - .retain(&(), |listener| listener(&event, cx)); - } - } - }) - .ok(); - } - fn apply_refresh_effect(&mut self) { for window in self.windows.values_mut() { if let Some(window) = window.as_mut() { @@ -1246,10 +1198,6 @@ pub(crate) enum Effect { event_type: TypeId, event: Box, }, - FocusChanged { - window_handle: AnyWindowHandle, - focused: Option, - }, Refresh, NotifyGlobalObservers { global_type: TypeId, diff --git a/crates/gpui2/src/elements/div.rs b/crates/gpui2/src/elements/div.rs index a102c71a6fbc1d8d0dbc0b2d984efcbdbc677276..3ec0a46fcbfbe9caf0bd371741026a18a1345017 100644 --- a/crates/gpui2/src/elements/div.rs +++ b/crates/gpui2/src/elements/div.rs @@ -1,10 +1,9 @@ use crate::{ point, px, Action, AnyDrag, AnyElement, AnyTooltip, AnyView, AppContext, BorrowAppContext, - BorrowWindow, Bounds, ClickEvent, DispatchPhase, Element, ElementId, FocusEvent, FocusHandle, - IntoElement, KeyContext, KeyDownEvent, KeyUpEvent, LayoutId, MouseButton, MouseDownEvent, - MouseMoveEvent, MouseUpEvent, ParentElement, Pixels, Point, Render, ScrollWheelEvent, - SharedString, Size, StackingOrder, Style, StyleRefinement, Styled, Task, View, Visibility, - WindowContext, + BorrowWindow, Bounds, ClickEvent, DispatchPhase, Element, ElementId, FocusHandle, IntoElement, + KeyContext, KeyDownEvent, KeyUpEvent, LayoutId, MouseButton, MouseDownEvent, MouseMoveEvent, + MouseUpEvent, ParentElement, Pixels, Point, Render, ScrollWheelEvent, SharedString, Size, + StackingOrder, Style, StyleRefinement, Styled, Task, View, Visibility, WindowContext, }; use collections::HashMap; use refineable::Refineable; @@ -462,10 +461,6 @@ pub trait FocusableElement: InteractiveElement { } } -pub type FocusListeners = Vec; - -pub type FocusListener = Box; - pub type MouseDownListener = Box; pub type MouseUpListener = diff --git a/crates/gpui2/src/interactive.rs b/crates/gpui2/src/interactive.rs index 1c6955111cd2f31870ff92fb2dfeff11db4b02ef..58ee849aad4a6d665c6ec14176f72eb457c9a82e 100644 --- a/crates/gpui2/src/interactive.rs +++ b/crates/gpui2/src/interactive.rs @@ -1,6 +1,5 @@ use crate::{ - div, point, Div, Element, FocusHandle, IntoElement, Keystroke, Modifiers, Pixels, Point, - Render, ViewContext, + div, point, Div, Element, IntoElement, Keystroke, Modifiers, Pixels, Point, Render, ViewContext, }; use smallvec::SmallVec; use std::{any::Any, fmt::Debug, marker::PhantomData, ops::Deref, path::PathBuf}; @@ -290,11 +289,6 @@ impl InputEvent { } } -pub struct FocusEvent { - pub blurred: Option, - pub focused: Option, -} - #[cfg(test)] mod test { use crate::{ diff --git a/crates/gpui2/src/key_dispatch.rs b/crates/gpui2/src/key_dispatch.rs index a44987aa39e65ecbabacbca40868fc27d61a3b49..ddb1f1e6ca8b4fd4db5d9c317144ca302519aa87 100644 --- a/crates/gpui2/src/key_dispatch.rs +++ b/crates/gpui2/src/key_dispatch.rs @@ -29,6 +29,7 @@ pub(crate) struct DispatchNode { pub key_listeners: Vec, pub action_listeners: Vec, pub context: Option, + focus_id: Option, parent: Option, } @@ -127,8 +128,9 @@ impl DispatchTree { } pub fn make_focusable(&mut self, focus_id: FocusId) { - self.focusable_node_ids - .insert(focus_id, self.active_node_id()); + let node_id = self.active_node_id(); + self.active_node().focus_id = Some(focus_id); + self.focusable_node_ids.insert(focus_id, node_id); } pub fn focus_contains(&self, parent: FocusId, child: FocusId) -> bool { @@ -247,6 +249,20 @@ impl DispatchTree { dispatch_path } + pub fn focus_path(&self, focus_id: FocusId) -> SmallVec<[FocusId; 8]> { + let mut focus_path: SmallVec<[FocusId; 8]> = SmallVec::new(); + let mut current_node_id = self.focusable_node_ids.get(&focus_id).copied(); + while let Some(node_id) = current_node_id { + let node = self.node(node_id); + if let Some(focus_id) = node.focus_id { + focus_path.push(focus_id); + } + current_node_id = node.parent; + } + focus_path.reverse(); // Reverse the path so it goes from the root to the focused node. + focus_path + } + pub fn node(&self, node_id: DispatchNodeId) -> &DispatchNode { &self.nodes[node_id.0] } diff --git a/crates/gpui2/src/window.rs b/crates/gpui2/src/window.rs index 77eb4e27be20bd734c27258a00fd05eaeff35caf..d2f48917112f5eda16c8a9a8a684bf47b036da09 100644 --- a/crates/gpui2/src/window.rs +++ b/crates/gpui2/src/window.rs @@ -2,10 +2,10 @@ use crate::{ key_dispatch::DispatchActionListener, px, size, Action, AnyDrag, AnyView, AppContext, AsyncWindowContext, AvailableSpace, Bounds, BoxShadow, Context, Corners, CursorStyle, DevicePixels, DispatchNodeId, DispatchTree, DisplayId, Edges, Effect, Entity, EntityId, - EventEmitter, FileDropEvent, Flatten, FocusEvent, FontId, GlobalElementId, GlyphId, Hsla, - ImageData, InputEvent, IsZero, KeyBinding, KeyContext, KeyDownEvent, KeystrokeEvent, LayoutId, - Model, ModelContext, Modifiers, MonochromeSprite, MouseButton, MouseMoveEvent, MouseUpEvent, - Path, Pixels, PlatformAtlas, PlatformDisplay, PlatformInputHandler, PlatformWindow, Point, + EventEmitter, FileDropEvent, Flatten, FontId, GlobalElementId, GlyphId, Hsla, ImageData, + InputEvent, IsZero, KeyBinding, KeyContext, KeyDownEvent, KeystrokeEvent, LayoutId, Model, + ModelContext, Modifiers, MonochromeSprite, MouseButton, MouseMoveEvent, MouseUpEvent, Path, + Pixels, PlatformAtlas, PlatformDisplay, PlatformInputHandler, PlatformWindow, Point, PolychromeSprite, PromptLevel, Quad, Render, RenderGlyphParams, RenderImageParams, RenderSvgParams, ScaledPixels, Scene, SceneBuilder, Shadow, SharedString, Size, Style, SubscriberSet, Subscription, Surface, TaffyLayoutEngine, Task, Underline, UnderlineStyle, View, @@ -74,9 +74,13 @@ impl DispatchPhase { type AnyObserver = Box bool + 'static>; type AnyMouseListener = Box; -type AnyFocusListener = Box; type AnyWindowFocusListener = Box bool + 'static>; +struct FocusEvent { + previous_focus_path: SmallVec<[FocusId; 8]>, + current_focus_path: SmallVec<[FocusId; 8]>, +} + slotmap::new_key_type! { pub struct FocusId; } impl FocusId { @@ -227,8 +231,8 @@ pub struct Window { pub(crate) rendered_frame: Frame, pub(crate) next_frame: Frame, pub(crate) focus_handles: Arc>>, - pub(crate) focus_listeners: SubscriberSet<(), AnyWindowFocusListener>, - pub(crate) blur_listeners: SubscriberSet<(), AnyObserver>, + focus_listeners: SubscriberSet<(), AnyWindowFocusListener>, + blur_listeners: SubscriberSet<(), AnyObserver>, default_prevented: bool, mouse_position: Point, requested_cursor_style: Option, @@ -238,7 +242,6 @@ pub struct Window { active: bool, pub(crate) dirty: bool, activation_observers: SubscriberSet<(), AnyObserver>, - pub(crate) last_blur: Option>, pub(crate) focus: Option, } @@ -250,10 +253,10 @@ pub(crate) struct ElementStateBox { // #[derive(Default)] pub(crate) struct Frame { + focus: Option, pub(crate) element_states: HashMap, mouse_listeners: HashMap>, pub(crate) dispatch_tree: DispatchTree, - pub(crate) focus_listeners: Vec, pub(crate) scene_builder: SceneBuilder, pub(crate) depth_map: Vec<(StackingOrder, Bounds)>, pub(crate) z_index_stack: StackingOrder, @@ -264,10 +267,10 @@ pub(crate) struct Frame { impl Frame { fn new(dispatch_tree: DispatchTree) -> Self { Frame { + focus: None, element_states: HashMap::default(), mouse_listeners: HashMap::default(), dispatch_tree, - focus_listeners: Vec::new(), scene_builder: SceneBuilder::default(), z_index_stack: StackingOrder::default(), depth_map: Default::default(), @@ -279,10 +282,15 @@ impl Frame { fn clear(&mut self) { self.element_states.clear(); self.mouse_listeners.values_mut().for_each(Vec::clear); - self.focus_listeners.clear(); self.dispatch_tree.clear(); self.depth_map.clear(); } + + fn focus_path(&self) -> SmallVec<[FocusId; 8]> { + self.focus + .map(|focus_id| self.dispatch_tree.focus_path(focus_id)) + .unwrap_or_default() + } } impl Window { @@ -372,7 +380,6 @@ impl Window { active: false, dirty: false, activation_observers: SubscriberSet::new(), - last_blur: None, focus: None, } } @@ -449,35 +456,17 @@ impl<'a> WindowContext<'a> { return; } - let focus_id = handle.id; - - if self.window.last_blur.is_none() { - self.window.last_blur = Some(self.window.focus); - } - - self.window.focus = Some(focus_id); + self.window.focus = Some(handle.id); self.window .rendered_frame .dispatch_tree .clear_pending_keystrokes(); - self.app.push_effect(Effect::FocusChanged { - window_handle: self.window.handle, - focused: Some(focus_id), - }); self.notify(); } /// Remove focus from all elements within this context's window. pub fn blur(&mut self) { - if self.window.last_blur.is_none() { - self.window.last_blur = Some(self.window.focus); - } - self.window.focus = None; - self.app.push_effect(Effect::FocusChanged { - window_handle: self.window.handle, - focused: None, - }); self.notify(); } @@ -1236,16 +1225,6 @@ impl<'a> WindowContext<'a> { /// Draw pixels to the display for this window based on the contents of its scene. pub(crate) fn draw(&mut self) -> Scene { - let window_was_focused = self - .window - .focus - .and_then(|focus_id| { - self.window - .rendered_frame - .dispatch_tree - .focusable_node_id(focus_id) - }) - .is_some(); self.text_system().start_frame(); self.window.platform_window.clear_input_handler(); self.window.layout_engine.as_mut().unwrap().clear(); @@ -1284,23 +1263,6 @@ impl<'a> WindowContext<'a> { }); } - let window_is_focused = self - .window - .focus - .and_then(|focus_id| { - self.window - .next_frame - .dispatch_tree - .focusable_node_id(focus_id) - }) - .is_some(); - if window_was_focused && !window_is_focused { - self.window - .blur_listeners - .clone() - .retain(&(), |listener| listener(self)); - } - self.window .next_frame .dispatch_tree @@ -1308,11 +1270,34 @@ impl<'a> WindowContext<'a> { &mut self.window.rendered_frame.dispatch_tree, self.window.focus, ); + self.window.next_frame.focus = self.window.focus; self.window.root_view = Some(root_view); + let previous_focus_path = self.window.rendered_frame.focus_path(); + let window = &mut self.window; mem::swap(&mut window.rendered_frame, &mut window.next_frame); + let current_focus_path = self.window.rendered_frame.focus_path(); + + if previous_focus_path != current_focus_path { + if !previous_focus_path.is_empty() && current_focus_path.is_empty() { + self.window + .blur_listeners + .clone() + .retain(&(), |listener| listener(self)); + } + + let event = FocusEvent { + previous_focus_path, + current_focus_path, + }; + self.window + .focus_listeners + .clone() + .retain(&(), |listener| listener(&event, self)); + } + let scene = self.window.rendered_frame.scene_builder.build(); // Set the cursor only if we're the active window. @@ -1699,22 +1684,6 @@ impl<'a> WindowContext<'a> { result } - /// Register a focus listener for the next frame only. It will be cleared - /// on the next frame render. You should use this method only from within elements, - /// and we may want to enforce that better via a different context type. - // todo!() Move this to `FrameContext` to emphasize its individuality? - pub fn on_focus_changed( - &mut self, - listener: impl Fn(&FocusEvent, &mut WindowContext) + 'static, - ) { - self.window - .next_frame - .focus_listeners - .push(Box::new(move |event, cx| { - listener(event, cx); - })); - } - /// Set an input handler, such as [ElementInputHandler], which interfaces with the /// platform to receive textual input with proper integration with concerns such /// as IME interactions. @@ -2389,7 +2358,9 @@ impl<'a, V: 'static> ViewContext<'a, V> { (), Box::new(move |event, cx| { view.update(cx, |view, cx| { - if event.focused.as_ref().map(|focused| focused.id) == Some(focus_id) { + if event.previous_focus_path.last() != Some(&focus_id) + && event.current_focus_path.last() == Some(&focus_id) + { listener(view, cx) } }) @@ -2414,10 +2385,8 @@ impl<'a, V: 'static> ViewContext<'a, V> { (), Box::new(move |event, cx| { view.update(cx, |view, cx| { - if event - .focused - .as_ref() - .map_or(false, |focused| focus_id.contains(focused.id, cx)) + if !event.previous_focus_path.contains(&focus_id) + && event.current_focus_path.contains(&focus_id) { listener(view, cx) } @@ -2443,7 +2412,9 @@ impl<'a, V: 'static> ViewContext<'a, V> { (), Box::new(move |event, cx| { view.update(cx, |view, cx| { - if event.blurred.as_ref().map(|blurred| blurred.id) == Some(focus_id) { + if event.previous_focus_path.last() == Some(&focus_id) + && event.current_focus_path.last() != Some(&focus_id) + { listener(view, cx) } }) @@ -2484,10 +2455,8 @@ impl<'a, V: 'static> ViewContext<'a, V> { (), Box::new(move |event, cx| { view.update(cx, |view, cx| { - if event - .blurred - .as_ref() - .map_or(false, |blurred| focus_id.contains(blurred.id, cx)) + if event.previous_focus_path.contains(&focus_id) + && !event.current_focus_path.contains(&focus_id) { listener(view, cx) }