From ee623f77c1332d2bb779cce16faf8d64c1097457 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 10 Jul 2024 19:54:26 +0200 Subject: [PATCH] linux/x11: Restore differentiation of mouse/keyboard focus (#13995) This restores https://github.com/zed-industries/zed/pull/13943 which was reverted in #13974 because it was possible to get in a state where focus could not be restored on a window. In this PR there's an additional change: `FocusIn` and `FocusOut` events are always handled, even if the `event.mode` is not "NORMAL". In my testing, `alt-tabbing` between windows didn't produce `FocusIn` and `FocusOut` events when we had that check. Now, with the check removed, it's possible to switch focus between two windows again with `alt-tab`. Release Notes: - N/A --------- Co-authored-by: Conrad Co-authored-by: Conrad Irwin --- crates/gpui/src/platform.rs | 2 + .../gpui/src/platform/linux/wayland/client.rs | 5 ++- .../gpui/src/platform/linux/wayland/window.rs | 17 ++++++++ crates/gpui/src/platform/linux/x11/client.rs | 39 +++++++++++++------ crates/gpui/src/platform/linux/x11/window.rs | 29 +++++++++++++- crates/gpui/src/platform/mac/window.rs | 7 ++++ crates/gpui/src/platform/test/window.rs | 10 +++++ crates/gpui/src/platform/windows/window.rs | 7 ++++ crates/gpui/src/window.rs | 28 ++++++++++++- 9 files changed, 127 insertions(+), 17 deletions(-) diff --git a/crates/gpui/src/platform.rs b/crates/gpui/src/platform.rs index 3d0472026a..b998325e72 100644 --- a/crates/gpui/src/platform.rs +++ b/crates/gpui/src/platform.rs @@ -318,6 +318,7 @@ pub(crate) trait PlatformWindow: HasWindowHandle + HasDisplayHandle { ) -> Option>; fn activate(&self); fn is_active(&self) -> bool; + fn is_hovered(&self) -> bool; fn set_title(&mut self, title: &str); fn set_background_appearance(&self, background_appearance: WindowBackgroundAppearance); fn minimize(&self); @@ -327,6 +328,7 @@ pub(crate) trait PlatformWindow: HasWindowHandle + HasDisplayHandle { fn on_request_frame(&self, callback: Box); fn on_input(&self, callback: Box DispatchEventResult>); fn on_active_status_change(&self, callback: Box); + fn on_hover_status_change(&self, callback: Box); fn on_resize(&self, callback: Box, f32)>); fn on_moved(&self, callback: Box); fn on_should_close(&self, callback: Box bool>); diff --git a/crates/gpui/src/platform/linux/wayland/client.rs b/crates/gpui/src/platform/linux/wayland/client.rs index 0dbe05d1d8..c514d3e94e 100644 --- a/crates/gpui/src/platform/linux/wayland/client.rs +++ b/crates/gpui/src/platform/linux/wayland/client.rs @@ -1403,6 +1403,7 @@ impl Dispatch for WaylandClientStatePtr { if let Some(window) = get_window(&mut state, &surface.id()) { state.mouse_focused_window = Some(window.clone()); + if state.enter_token.is_some() { state.enter_token = None; } @@ -1416,7 +1417,7 @@ impl Dispatch for WaylandClientStatePtr { } } drop(state); - window.set_focused(true); + window.set_hovered(true); } } wl_pointer::Event::Leave { .. } => { @@ -1432,7 +1433,7 @@ impl Dispatch for WaylandClientStatePtr { drop(state); focused_window.handle_input(input); - focused_window.set_focused(false); + focused_window.set_hovered(false); } } wl_pointer::Event::Motion { diff --git a/crates/gpui/src/platform/linux/wayland/window.rs b/crates/gpui/src/platform/linux/wayland/window.rs index 6acb63815a..ba97f202fc 100644 --- a/crates/gpui/src/platform/linux/wayland/window.rs +++ b/crates/gpui/src/platform/linux/wayland/window.rs @@ -36,6 +36,7 @@ pub(crate) struct Callbacks { request_frame: Option>, input: Option crate::DispatchEventResult>>, active_status_change: Option>, + hover_status_change: Option>, resize: Option, f32)>>, moved: Option>, should_close: Option bool>>, @@ -97,6 +98,7 @@ pub struct WaylandWindowState { client: WaylandClientStatePtr, handle: AnyWindowHandle, active: bool, + hovered: bool, in_progress_configure: Option, in_progress_window_controls: Option, window_controls: WindowControls, @@ -181,6 +183,7 @@ impl WaylandWindowState { appearance, handle, active: false, + hovered: false, in_progress_window_controls: None, // Assume that we can do anything, unless told otherwise window_controls: WindowControls { @@ -700,6 +703,12 @@ impl WaylandWindowStatePtr { } } + pub fn set_hovered(&self, focus: bool) { + if let Some(ref mut fun) = self.callbacks.borrow_mut().hover_status_change { + fun(focus); + } + } + pub fn set_appearance(&mut self, appearance: WindowAppearance) { self.state.borrow_mut().appearance = appearance; @@ -845,6 +854,10 @@ impl PlatformWindow for WaylandWindow { self.borrow().active } + fn is_hovered(&self) -> bool { + self.borrow().hovered + } + fn set_title(&mut self, title: &str) { self.borrow().toplevel.set_title(title.to_string()); } @@ -899,6 +912,10 @@ impl PlatformWindow for WaylandWindow { self.0.callbacks.borrow_mut().active_status_change = Some(callback); } + fn on_hover_status_change(&self, callback: Box) { + self.0.callbacks.borrow_mut().hover_status_change = Some(callback); + } + fn on_resize(&self, callback: Box, f32)>) { self.0.callbacks.borrow_mut().resize = Some(callback); } diff --git a/crates/gpui/src/platform/linux/x11/client.rs b/crates/gpui/src/platform/linux/x11/client.rs index 2345ac59b2..38e17778c8 100644 --- a/crates/gpui/src/platform/linux/x11/client.rs +++ b/crates/gpui/src/platform/linux/x11/client.rs @@ -110,7 +110,8 @@ pub struct X11ClientState { pub(crate) _resource_database: Database, pub(crate) atoms: XcbAtoms, pub(crate) windows: HashMap, - pub(crate) focused_window: Option, + pub(crate) mouse_focused_window: Option, + pub(crate) keyboard_focused_window: Option, pub(crate) xkb: xkbc::State, pub(crate) ximc: Option>>, pub(crate) xim_handler: Option, @@ -144,7 +145,12 @@ impl X11ClientStatePtr { if let Some(window_ref) = state.windows.remove(&x_window) { state.loop_handle.remove(window_ref.refresh_event_token); } - + if state.mouse_focused_window == Some(x_window) { + state.mouse_focused_window = None; + } + if state.keyboard_focused_window == Some(x_window) { + state.keyboard_focused_window = None; + } state.cursor_styles.remove(&x_window); if state.windows.is_empty() { @@ -341,7 +347,8 @@ impl X11Client { _resource_database: resource_database, atoms, windows: HashMap::default(), - focused_window: None, + mouse_focused_window: None, + keyboard_focused_window: None, xkb: xkb_state, ximc, xim_handler, @@ -502,7 +509,7 @@ impl X11Client { .push(AttributeName::ClientWindow, xim_handler.window) .push(AttributeName::FocusWindow, xim_handler.window); - let window_id = state.focused_window; + let window_id = state.keyboard_focused_window; drop(state); if let Some(window_id) = window_id { let window = self.get_window(window_id).unwrap(); @@ -586,17 +593,17 @@ impl X11Client { } Event::FocusIn(event) => { let window = self.get_window(event.event)?; - window.set_focused(true); + window.set_active(true); let mut state = self.0.borrow_mut(); - state.focused_window = Some(event.event); + state.keyboard_focused_window = Some(event.event); drop(state); self.enable_ime(); } Event::FocusOut(event) => { let window = self.get_window(event.event)?; - window.set_focused(false); + window.set_active(false); let mut state = self.0.borrow_mut(); - state.focused_window = None; + state.keyboard_focused_window = None; if let Some(compose_state) = state.compose_state.as_mut() { compose_state.reset(); } @@ -620,7 +627,7 @@ impl X11Client { if state.modifiers == modifiers { drop(state); } else { - let focused_window_id = state.focused_window?; + let focused_window_id = state.keyboard_focused_window?; state.modifiers = modifiers; drop(state); @@ -871,12 +878,18 @@ impl X11Client { valuator_idx += 1; } } + Event::XinputEnter(event) if event.mode == xinput::NotifyMode::NORMAL => { + let window = self.get_window(event.event)?; + window.set_hovered(true); + let mut state = self.0.borrow_mut(); + state.mouse_focused_window = Some(event.event); + } Event::XinputLeave(event) if event.mode == xinput::NotifyMode::NORMAL => { self.0.borrow_mut().scroll_x = None; // Set last scroll to `None` so that a large delta isn't created if scrolling is done outside the window (the valuator is global) self.0.borrow_mut().scroll_y = None; - let window = self.get_window(event.event)?; let mut state = self.0.borrow_mut(); + state.mouse_focused_window = None; let pressed_button = pressed_button_from_mask(event.buttons[0]); let position = point( px(event.event_x as f32 / u16::MAX as f32 / state.scale_factor), @@ -886,11 +899,13 @@ impl X11Client { state.modifiers = modifiers; drop(state); + let window = self.get_window(event.event)?; window.handle_input(PlatformInput::MouseExited(crate::MouseExitEvent { pressed_button, position, modifiers, })); + window.set_hovered(false); } _ => {} }; @@ -1140,7 +1155,7 @@ impl LinuxClient for X11Client { fn set_cursor_style(&self, style: CursorStyle) { let mut state = self.0.borrow_mut(); - let Some(focused_window) = state.focused_window else { + let Some(focused_window) = state.mouse_focused_window else { return; }; let current_style = state @@ -1272,7 +1287,7 @@ impl LinuxClient for X11Client { fn active_window(&self) -> Option { let state = self.0.borrow(); - state.focused_window.and_then(|focused_window| { + state.keyboard_focused_window.and_then(|focused_window| { state .windows .get(&focused_window) diff --git a/crates/gpui/src/platform/linux/x11/window.rs b/crates/gpui/src/platform/linux/x11/window.rs index 8e439e9e3a..fb077634a9 100644 --- a/crates/gpui/src/platform/linux/x11/window.rs +++ b/crates/gpui/src/platform/linux/x11/window.rs @@ -211,6 +211,7 @@ pub struct Callbacks { request_frame: Option>, input: Option crate::DispatchEventResult>>, active_status_change: Option>, + hovered_status_change: Option>, resize: Option, f32)>>, moved: Option>, should_close: Option bool>>, @@ -238,6 +239,7 @@ pub struct X11WindowState { maximized_horizontal: bool, hidden: bool, active: bool, + hovered: bool, fullscreen: bool, client_side_decorations_supported: bool, decorations: WindowDecorations, @@ -451,6 +453,7 @@ impl X11WindowState { xinput::XIEventMask::MOTION | xinput::XIEventMask::BUTTON_PRESS | xinput::XIEventMask::BUTTON_RELEASE + | xinput::XIEventMask::ENTER | xinput::XIEventMask::LEAVE, ], }], @@ -507,6 +510,7 @@ impl X11WindowState { atoms: *atoms, input_handler: None, active: false, + hovered: false, fullscreen: false, maximized_vertical: false, maximized_horizontal: false, @@ -777,6 +781,15 @@ impl X11WindowStatePtr { state.hidden = true; } } + + let hovered_window = self + .xcb_connection + .query_pointer(state.x_root_window) + .unwrap() + .reply() + .unwrap() + .child; + self.set_hovered(hovered_window == self.x_window); } pub fn close(&self) { @@ -912,12 +925,18 @@ impl X11WindowStatePtr { } } - pub fn set_focused(&self, focus: bool) { + pub fn set_active(&self, focus: bool) { if let Some(ref mut fun) = self.callbacks.borrow_mut().active_status_change { fun(focus); } } + pub fn set_hovered(&self, focus: bool) { + if let Some(ref mut fun) = self.callbacks.borrow_mut().hovered_status_change { + fun(focus); + } + } + pub fn set_appearance(&mut self, appearance: WindowAppearance) { let mut state = self.state.borrow_mut(); state.appearance = appearance; @@ -1046,6 +1065,10 @@ impl PlatformWindow for X11Window { self.0.state.borrow().active } + fn is_hovered(&self) -> bool { + self.0.state.borrow().hovered + } + fn set_title(&mut self, title: &str) { self.0 .xcb_connection @@ -1162,6 +1185,10 @@ impl PlatformWindow for X11Window { self.0.callbacks.borrow_mut().active_status_change = Some(callback); } + fn on_hover_status_change(&self, callback: Box) { + self.0.callbacks.borrow_mut().hovered_status_change = Some(callback); + } + fn on_resize(&self, callback: Box, f32)>) { self.0.callbacks.borrow_mut().resize = Some(callback); } diff --git a/crates/gpui/src/platform/mac/window.rs b/crates/gpui/src/platform/mac/window.rs index 3ed4f22f14..b5ce6719ee 100644 --- a/crates/gpui/src/platform/mac/window.rs +++ b/crates/gpui/src/platform/mac/window.rs @@ -940,6 +940,11 @@ impl PlatformWindow for MacWindow { unsafe { self.0.lock().native_window.isKeyWindow() == YES } } + // is_hovered is unused on macOS. See WindowContext::is_window_hovered. + fn is_hovered(&self) -> bool { + false + } + fn set_title(&mut self, title: &str) { unsafe { let app = NSApplication::sharedApplication(nil); @@ -1061,6 +1066,8 @@ impl PlatformWindow for MacWindow { self.0.as_ref().lock().activate_callback = Some(callback); } + fn on_hover_status_change(&self, _: Box) {} + fn on_resize(&self, callback: Box, f32)>) { self.0.as_ref().lock().resize_callback = Some(callback); } diff --git a/crates/gpui/src/platform/test/window.rs b/crates/gpui/src/platform/test/window.rs index 7ffaf77250..8c7ee5507a 100644 --- a/crates/gpui/src/platform/test/window.rs +++ b/crates/gpui/src/platform/test/window.rs @@ -23,6 +23,7 @@ pub(crate) struct TestWindowState { pub(crate) should_close_handler: Option bool>>, input_callback: Option DispatchEventResult>>, active_status_change_callback: Option>, + hover_status_change_callback: Option>, resize_callback: Option, f32)>>, moved_callback: Option>, input_handler: Option, @@ -66,6 +67,7 @@ impl TestWindow { should_close_handler: None, input_callback: None, active_status_change_callback: None, + hover_status_change_callback: None, resize_callback: None, moved_callback: None, input_handler: None, @@ -182,6 +184,10 @@ impl PlatformWindow for TestWindow { false } + fn is_hovered(&self) -> bool { + false + } + fn set_title(&mut self, title: &str) { self.0.lock().title = Some(title.to_owned()); } @@ -225,6 +231,10 @@ impl PlatformWindow for TestWindow { self.0.lock().active_status_change_callback = Some(callback) } + fn on_hover_status_change(&self, callback: Box) { + self.0.lock().hover_status_change_callback = Some(callback) + } + fn on_resize(&self, callback: Box, f32)>) { self.0.lock().resize_callback = Some(callback) } diff --git a/crates/gpui/src/platform/windows/window.rs b/crates/gpui/src/platform/windows/window.rs index a44787a72d..1cb469bd0e 100644 --- a/crates/gpui/src/platform/windows/window.rs +++ b/crates/gpui/src/platform/windows/window.rs @@ -503,6 +503,11 @@ impl PlatformWindow for WindowsWindow { self.0.hwnd == unsafe { GetActiveWindow() } } + // is_hovered is unused on Windows. See WindowContext::is_window_hovered. + fn is_hovered(&self) -> bool { + false + } + fn set_title(&mut self, title: &str) { unsafe { SetWindowTextW(self.0.hwnd, &HSTRING::from(title)) } .inspect_err(|e| log::error!("Set title failed: {e}")) @@ -604,6 +609,8 @@ impl PlatformWindow for WindowsWindow { self.0.state.borrow_mut().callbacks.active_status_change = Some(callback); } + fn on_hover_status_change(&self, _: Box) {} + fn on_resize(&self, callback: Box, f32)>) { self.0.state.borrow_mut().callbacks.resize = Some(callback); } diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs index a5f2af6035..7665940513 100644 --- a/crates/gpui/src/window.rs +++ b/crates/gpui/src/window.rs @@ -541,6 +541,7 @@ pub struct Window { appearance: WindowAppearance, appearance_observers: SubscriberSet<(), AnyObserver>, active: Rc>, + hovered: Rc>, pub(crate) dirty: Rc>, pub(crate) needs_present: Rc>, pub(crate) last_input_timestamp: Rc>, @@ -672,6 +673,7 @@ impl Window { let text_system = Arc::new(WindowTextSystem::new(cx.text_system().clone())); let dirty = Rc::new(Cell::new(true)); let active = Rc::new(Cell::new(platform_window.is_active())); + let hovered = Rc::new(Cell::new(platform_window.is_hovered())); let needs_present = Rc::new(Cell::new(false)); let next_frame_callbacks: Rc>> = Default::default(); let last_input_timestamp = Rc::new(Cell::new(Instant::now())); @@ -778,7 +780,17 @@ impl Window { .log_err(); } })); - + platform_window.on_hover_status_change(Box::new({ + let mut cx = cx.to_async(); + move |active| { + handle + .update(&mut cx, |_, cx| { + cx.window.hovered.set(active); + cx.refresh(); + }) + .log_err(); + } + })); platform_window.on_input({ let mut cx = cx.to_async(); Box::new(move |event| { @@ -829,6 +841,7 @@ impl Window { appearance, appearance_observers: SubscriberSet::new(), active, + hovered, dirty, needs_present, last_input_timestamp, @@ -1222,6 +1235,17 @@ impl<'a> WindowContext<'a> { self.window.active.get() } + /// Returns whether this window is considered to be the window + /// that currently owns the mouse cursor. + /// On mac, this is equivalent to `is_window_active`. + pub fn is_window_hovered(&self) -> bool { + if cfg!(target_os = "linux") { + self.window.hovered.get() + } else { + self.is_window_active() + } + } + /// Toggle zoom on the window. pub fn zoom_window(&self) { self.window.platform_window.zoom(); @@ -2980,7 +3004,7 @@ impl<'a> WindowContext<'a> { fn reset_cursor_style(&self) { // Set the cursor only if we're the active window. - if self.is_window_active() { + if self.is_window_hovered() { let style = self .window .rendered_frame