From dd13ccda4ce3cb44df0505298de5233d0a2d3776 Mon Sep 17 00:00:00 2001 From: Kirill Chibisov Date: Tue, 30 Jan 2024 18:28:13 +0400 Subject: [PATCH] On Wayland, send `Focused(false)` once seats left Given that we merge all the seats, we should consider that window is not focused once all seats wl_keyboards are no longer present. We use seats instead of keyboards to track focus to protect against wl_keyboard::leave not being delivered when removing the seat (usually it's not the case though). Fixes: #3376 --- CHANGELOG.md | 1 + .../linux/wayland/seat/keyboard/mod.rs | 49 ++++++++++++------- src/platform_impl/linux/wayland/seat/mod.rs | 26 ++++++++-- .../linux/wayland/window/state.rs | 28 +++++++---- 4 files changed, 72 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fcd86c6..a9e52b02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ Unreleased` header. - Add `Window::builder`, which is intended to replace the (now deprecated) `WindowBuilder::new`. - On X11, reload dpi on `_XSETTINGS_SETTINGS` update. - On X11, fix deadlock when adjusting DPI and resizing at the same time. +- On Wayland, fix `Focused(false)` being send when other seats still have window focused. # 0.29.10 diff --git a/src/platform_impl/linux/wayland/seat/keyboard/mod.rs b/src/platform_impl/linux/wayland/seat/keyboard/mod.rs index 975e81d1..cf1ef218 100644 --- a/src/platform_impl/linux/wayland/seat/keyboard/mod.rs +++ b/src/platform_impl/linux/wayland/seat/keyboard/mod.rs @@ -61,8 +61,13 @@ impl Dispatch for WinitState { let window_id = wayland::make_wid(&surface); // Mark the window as focused. - match state.windows.get_mut().get(&window_id) { - Some(window) => window.lock().unwrap().set_has_focus(true), + let was_unfocused = match state.windows.get_mut().get(&window_id) { + Some(window) => { + let mut window = window.lock().unwrap(); + let was_unfocused = !window.has_focus(); + window.add_seat_focus(data.seat.id()); + was_unfocused + } None => return, }; @@ -73,13 +78,15 @@ impl Dispatch for WinitState { keyboard_state.loop_handle.remove(token); } - // The keyboard focus is considered as general focus. - state - .events_sink - .push_window_event(WindowEvent::Focused(true), window_id); - *data.window_id.lock().unwrap() = Some(window_id); + // The keyboard focus is considered as general focus. + if was_unfocused { + state + .events_sink + .push_window_event(WindowEvent::Focused(true), window_id); + } + // HACK: this is just for GNOME not fixing their ordering issue of modifiers. if std::mem::take(&mut seat_state.modifiers_pending) { state.events_sink.push_window_event( @@ -101,24 +108,30 @@ impl Dispatch for WinitState { // NOTE: The check whether the window exists is essential as we might get a // nil surface, regardless of what protocol says. - match state.windows.get_mut().get(&window_id) { - Some(window) => window.lock().unwrap().set_has_focus(false), + let focused = match state.windows.get_mut().get(&window_id) { + Some(window) => { + let mut window = window.lock().unwrap(); + window.remove_seat_focus(&data.seat.id()); + window.has_focus() + } None => return, }; - // Notify that no modifiers are being pressed. - state.events_sink.push_window_event( - WindowEvent::ModifiersChanged(ModifiersState::empty().into()), - window_id, - ); - // We don't need to update it above, because the next `Enter` will overwrite // anyway. *data.window_id.lock().unwrap() = None; - state - .events_sink - .push_window_event(WindowEvent::Focused(false), window_id); + if !focused { + // Notify that no modifiers are being pressed. + state.events_sink.push_window_event( + WindowEvent::ModifiersChanged(ModifiersState::empty().into()), + window_id, + ); + + state + .events_sink + .push_window_event(WindowEvent::Focused(false), window_id); + } } WlKeyboardEvent::Key { key, diff --git a/src/platform_impl/linux/wayland/seat/mod.rs b/src/platform_impl/linux/wayland/seat/mod.rs index f3562421..2e2fa438 100644 --- a/src/platform_impl/linux/wayland/seat/mod.rs +++ b/src/platform_impl/linux/wayland/seat/mod.rs @@ -4,6 +4,7 @@ use std::sync::Arc; use ahash::AHashMap; +use sctk::reexports::client::backend::ObjectId; use sctk::reexports::client::protocol::wl_seat::WlSeat; use sctk::reexports::client::protocol::wl_touch::WlTouch; use sctk::reexports::client::{Connection, Proxy, QueueHandle}; @@ -13,6 +14,7 @@ use sctk::reexports::protocols::wp::text_input::zv3::client::zwp_text_input_v3:: use sctk::seat::pointer::{ThemeSpec, ThemedPointer}; use sctk::seat::{Capability as SeatCapability, SeatHandler, SeatState}; +use crate::event::WindowEvent; use crate::keyboard::ModifiersState; use crate::platform_impl::wayland::state::WinitState; @@ -143,6 +145,10 @@ impl SeatHandler for WinitState { ) { let seat_state = self.seats.get_mut(&seat.id()).unwrap(); + if let Some(text_input) = seat_state.text_input.take() { + text_input.destroy(); + } + match capability { SeatCapability::Touch => { if let Some(touch) = seat_state.touch.take() { @@ -174,13 +180,10 @@ impl SeatHandler for WinitState { } SeatCapability::Keyboard => { seat_state.keyboard_state = None; + self.on_keyboard_destroy(&seat.id()); } _ => (), } - - if let Some(text_input) = seat_state.text_input.take() { - text_input.destroy(); - } } fn new_seat( @@ -199,6 +202,21 @@ impl SeatHandler for WinitState { seat: WlSeat, ) { let _ = self.seats.remove(&seat.id()); + self.on_keyboard_destroy(&seat.id()); + } +} + +impl WinitState { + fn on_keyboard_destroy(&mut self, seat: &ObjectId) { + for (window_id, window) in self.windows.get_mut() { + let mut window = window.lock().unwrap(); + let had_focus = window.has_focus(); + window.remove_seat_focus(seat); + if had_focus != window.has_focus() { + self.events_sink + .push_window_event(WindowEvent::Focused(false), *window_id); + } + } } } diff --git a/src/platform_impl/linux/wayland/window/state.rs b/src/platform_impl/linux/wayland/window/state.rs index ac141a93..65092c64 100644 --- a/src/platform_impl/linux/wayland/window/state.rs +++ b/src/platform_impl/linux/wayland/window/state.rs @@ -4,8 +4,10 @@ use std::num::NonZeroU32; use std::sync::{Arc, Mutex, Weak}; use std::time::Duration; +use ahash::HashSet; use log::{info, warn}; +use sctk::reexports::client::backend::ObjectId; use sctk::reexports::client::protocol::wl_seat::WlSeat; use sctk::reexports::client::protocol::wl_shm::WlShm; use sctk::reexports::client::protocol::wl_surface::WlSurface; @@ -90,8 +92,10 @@ pub struct WindowState { /// Whether the frame is resizable. resizable: bool, - /// Whether the window has focus. - has_focus: bool, + // NOTE: we can't use simple counter, since it's racy when seat getting destroyed and new + // is created, since add/removed stuff could be delivered a bit out of order. + /// Seats that has keyboard focus on that window. + seat_focus: HashSet, /// The scale factor of the window. scale_factor: f64, @@ -187,7 +191,7 @@ impl WindowState { fractional_scale, frame: None, frame_callback_state: FrameCallbackState::None, - has_focus: false, + seat_focus: Default::default(), has_pending_move: None, ime_allowed: false, ime_purpose: ImePurpose::Normal, @@ -521,10 +525,10 @@ impl WindowState { } } - /// Whether the window is focused. + /// Whether the window is focused by any seat. #[inline] pub fn has_focus(&self) -> bool { - self.has_focus + !self.seat_focus.is_empty() } /// Whether the IME is allowed. @@ -951,12 +955,16 @@ impl WindowState { } } - /// Mark that the window has focus. - /// - /// Should be used from routine that sends focused event. + /// Add seat focus for the window. #[inline] - pub fn set_has_focus(&mut self, has_focus: bool) { - self.has_focus = has_focus; + pub fn add_seat_focus(&mut self, seat: ObjectId) { + self.seat_focus.insert(seat); + } + + /// Remove seat focus from the window. + #[inline] + pub fn remove_seat_focus(&mut self, seat: &ObjectId) { + self.seat_focus.remove(seat); } /// Returns `true` if the requested state was applied.