From ad4a4e533f8d073cf8da5f560b82dabc0e9e89d3 Mon Sep 17 00:00:00 2001 From: Kirill Chibisov Date: Wed, 3 Jul 2024 22:53:17 +0300 Subject: [PATCH] wayland: ignore events for dead objects Nothing wrong will happen if we ignore events when compositor is at wrong, at least crashing because compositor is just _wrong_ probably is not a great option. Links: https://github.com/alacritty/alacritty/issues/8065 --- src/changelog/unreleased.md | 4 ++ .../linux/wayland/seat/keyboard/mod.rs | 59 +++++++++++-------- src/platform_impl/linux/wayland/seat/mod.rs | 17 +++++- .../linux/wayland/seat/pointer/mod.rs | 34 +++++++---- .../linux/wayland/seat/touch/mod.rs | 37 ++++++++++-- 5 files changed, 106 insertions(+), 45 deletions(-) diff --git a/src/changelog/unreleased.md b/src/changelog/unreleased.md index 09795e9104..9b0751f207 100644 --- a/src/changelog/unreleased.md +++ b/src/changelog/unreleased.md @@ -76,3 +76,7 @@ changelog entry. This feature was incomplete, and the equivalent functionality can be trivially achieved outside of `winit` using `objc2-ui-kit` and calling `UIDevice::currentDevice().userInterfaceIdiom()`. + +### Fixed + +- On Wayland, avoid crashing when compositor is misbehaving. diff --git a/src/platform_impl/linux/wayland/seat/keyboard/mod.rs b/src/platform_impl/linux/wayland/seat/keyboard/mod.rs index 4e731d47e7..ef4f99520b 100644 --- a/src/platform_impl/linux/wayland/seat/keyboard/mod.rs +++ b/src/platform_impl/linux/wayland/seat/keyboard/mod.rs @@ -18,7 +18,6 @@ use crate::keyboard::ModifiersState; use crate::platform_impl::common::xkb::Context; use crate::platform_impl::wayland::event_loop::sink::EventSink; -use crate::platform_impl::wayland::seat::WinitSeatState; use crate::platform_impl::wayland::state::WinitState; use crate::platform_impl::wayland::{self, DeviceId, WindowId}; @@ -33,7 +32,17 @@ impl Dispatch for WinitState { ) { let seat_state = match state.seats.get_mut(&data.seat.id()) { Some(seat_state) => seat_state, - None => return, + None => { + warn!("Received keyboard event {event:?} without seat"); + return; + }, + }; + let keyboard_state = match seat_state.keyboard_state.as_mut() { + Some(keyboard_state) => keyboard_state, + None => { + warn!("Received keyboard event {event:?} without keyboard"); + return; + }, }; match event { @@ -43,7 +52,7 @@ impl Dispatch for WinitState { warn!("non-xkb compatible keymap") }, WlKeymapFormat::XkbV1 => { - let context = &mut seat_state.keyboard_state.as_mut().unwrap().xkb_context; + let context = &mut keyboard_state.xkb_context; context.set_keymap_from_fd(fd, size as usize); }, _ => unreachable!(), @@ -67,7 +76,6 @@ impl Dispatch for WinitState { }; // Drop the repeat, if there were any. - let keyboard_state = seat_state.keyboard_state.as_mut().unwrap(); keyboard_state.current_repeat = None; if let Some(token) = keyboard_state.repeat_token.take() { keyboard_state.loop_handle.remove(token); @@ -93,7 +101,6 @@ impl Dispatch for WinitState { // NOTE: we should drop the repeat regardless whethere it was for the present // window of for the window which just went gone. - let keyboard_state = seat_state.keyboard_state.as_mut().unwrap(); keyboard_state.current_repeat = None; if let Some(token) = keyboard_state.repeat_token.take() { keyboard_state.loop_handle.remove(token); @@ -128,7 +135,7 @@ impl Dispatch for WinitState { let key = key + 8; key_input( - seat_state, + keyboard_state, &mut state.events_sink, data, key, @@ -136,7 +143,6 @@ impl Dispatch for WinitState { false, ); - let keyboard_state = seat_state.keyboard_state.as_mut().unwrap(); let delay = match keyboard_state.repeat_info { RepeatInfo::Repeat { delay, .. } => delay, RepeatInfo::Disable => return, @@ -163,18 +169,25 @@ impl Dispatch for WinitState { state.dispatched_events = true; let data = wl_keyboard.data::().unwrap(); - let seat_state = state.seats.get_mut(&data.seat.id()).unwrap(); - - // NOTE: The removed on event source is batched, but key change to - // `None` is instant. - let repeat_keycode = - match seat_state.keyboard_state.as_ref().unwrap().current_repeat { - Some(repeat_keycode) => repeat_keycode, - None => return TimeoutAction::Drop, - }; + let seat_state = match state.seats.get_mut(&data.seat.id()) { + Some(seat_state) => seat_state, + None => return TimeoutAction::Drop, + }; + + let keyboard_state = match seat_state.keyboard_state.as_mut() { + Some(keyboard_state) => keyboard_state, + None => return TimeoutAction::Drop, + }; + + // NOTE: The removed on event source is batched, but key change to `None` + // is instant. + let repeat_keycode = match keyboard_state.current_repeat { + Some(repeat_keycode) => repeat_keycode, + None => return TimeoutAction::Drop, + }; key_input( - seat_state, + keyboard_state, &mut state.events_sink, data, repeat_keycode, @@ -183,7 +196,7 @@ impl Dispatch for WinitState { ); // NOTE: the gap could change dynamically while repeat is going. - match seat_state.keyboard_state.as_ref().unwrap().repeat_info { + match keyboard_state.repeat_info { RepeatInfo::Repeat { gap, .. } => TimeoutAction::ToDuration(gap), RepeatInfo::Disable => TimeoutAction::Drop, } @@ -194,7 +207,7 @@ impl Dispatch for WinitState { let key = key + 8; key_input( - seat_state, + keyboard_state, &mut state.events_sink, data, key, @@ -202,7 +215,6 @@ impl Dispatch for WinitState { false, ); - let keyboard_state = seat_state.keyboard_state.as_mut().unwrap(); if keyboard_state.repeat_info != RepeatInfo::Disable && keyboard_state.xkb_context.keymap_mut().unwrap().key_repeats(key) && Some(key) == keyboard_state.current_repeat @@ -216,7 +228,7 @@ impl Dispatch for WinitState { WlKeyboardEvent::Modifiers { mods_depressed, mods_latched, mods_locked, group, .. } => { - let xkb_context = &mut seat_state.keyboard_state.as_mut().unwrap().xkb_context; + let xkb_context = &mut keyboard_state.xkb_context; let xkb_state = match xkb_context.state_mut() { Some(state) => state, None => return, @@ -240,7 +252,6 @@ impl Dispatch for WinitState { ); }, WlKeyboardEvent::RepeatInfo { rate, delay } => { - let keyboard_state = seat_state.keyboard_state.as_mut().unwrap(); keyboard_state.repeat_info = if rate == 0 { // Stop the repeat once we get a disable event. keyboard_state.current_repeat = None; @@ -348,7 +359,7 @@ impl KeyboardData { } fn key_input( - seat_state: &mut WinitSeatState, + keyboard_state: &mut KeyboardState, event_sink: &mut EventSink, data: &KeyboardData, keycode: u32, @@ -360,8 +371,6 @@ fn key_input( None => return, }; - let keyboard_state = seat_state.keyboard_state.as_mut().unwrap(); - let device_id = crate::event::DeviceId(crate::platform_impl::DeviceId::Wayland(DeviceId)); if let Some(mut key_context) = keyboard_state.xkb_context.key_context() { let event = key_context.process_key_event(keycode, state, repeat); diff --git a/src/platform_impl/linux/wayland/seat/mod.rs b/src/platform_impl/linux/wayland/seat/mod.rs index 103cf6fce0..82e38e5414 100644 --- a/src/platform_impl/linux/wayland/seat/mod.rs +++ b/src/platform_impl/linux/wayland/seat/mod.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use ahash::AHashMap; +use tracing::warn; use sctk::reexports::client::backend::ObjectId; use sctk::reexports::client::protocol::wl_seat::WlSeat; @@ -76,7 +77,13 @@ impl SeatHandler for WinitState { seat: WlSeat, capability: SeatCapability, ) { - let seat_state = self.seats.get_mut(&seat.id()).unwrap(); + let seat_state = match self.seats.get_mut(&seat.id()) { + Some(seat_state) => seat_state, + None => { + warn!("Received wl_seat::new_capability for unknown seat"); + return; + }, + }; match capability { SeatCapability::Touch if seat_state.touch.is_none() => { @@ -139,7 +146,13 @@ impl SeatHandler for WinitState { seat: WlSeat, capability: SeatCapability, ) { - let seat_state = self.seats.get_mut(&seat.id()).unwrap(); + let seat_state = match self.seats.get_mut(&seat.id()) { + Some(seat_state) => seat_state, + None => { + warn!("Received wl_seat::remove_capability for unknown seat"); + return; + }, + }; if let Some(text_input) = seat_state.text_input.take() { text_input.destroy(); diff --git a/src/platform_impl/linux/wayland/seat/pointer/mod.rs b/src/platform_impl/linux/wayland/seat/pointer/mod.rs index f26c326e9c..fcca59343b 100644 --- a/src/platform_impl/linux/wayland/seat/pointer/mod.rs +++ b/src/platform_impl/linux/wayland/seat/pointer/mod.rs @@ -4,6 +4,8 @@ use std::ops::Deref; use std::sync::{Arc, Mutex}; use std::time::Duration; +use tracing::warn; + use sctk::reexports::client::delegate_dispatch; use sctk::reexports::client::protocol::wl_pointer::WlPointer; use sctk::reexports::client::protocol::wl_seat::WlSeat; @@ -41,7 +43,21 @@ impl PointerHandler for WinitState { events: &[PointerEvent], ) { let seat = pointer.winit_data().seat(); - let seat_state = self.seats.get(&seat.id()).unwrap(); + let seat_state = match self.seats.get(&seat.id()) { + Some(seat_state) => seat_state, + None => { + warn!("Received pointer event without seat"); + return; + }, + }; + + let themed_pointer = match seat_state.pointer.as_ref() { + Some(pointer) => pointer, + None => { + warn!("Received pointer event without pointer"); + return; + }, + }; let device_id = crate::event::DeviceId(crate::platform_impl::DeviceId::Wayland(DeviceId)); @@ -78,9 +94,7 @@ impl PointerHandler for WinitState { event.position.0, event.position.1, ) { - if let Some(pointer) = seat_state.pointer.as_ref() { - let _ = pointer.set_cursor(connection, icon); - } + let _ = themed_pointer.set_cursor(connection, icon); } }, PointerEventKind::Leave { .. } if parent_surface != surface => { @@ -113,9 +127,7 @@ impl PointerHandler for WinitState { self.events_sink .push_window_event(WindowEvent::CursorEntered { device_id }, window_id); - if let Some(pointer) = seat_state.pointer.as_ref().map(Arc::downgrade) { - window.pointer_entered(pointer); - } + window.pointer_entered(Arc::downgrade(themed_pointer)); // Set the currently focused surface. pointer.winit_data().inner.lock().unwrap().surface = Some(window_id); @@ -126,9 +138,7 @@ impl PointerHandler for WinitState { ); }, PointerEventKind::Leave { .. } => { - if let Some(pointer) = seat_state.pointer.as_ref().map(Arc::downgrade) { - window.pointer_left(pointer); - } + window.pointer_left(Arc::downgrade(themed_pointer)); // Remove the active surface. pointer.winit_data().inner.lock().unwrap().surface = None; @@ -185,13 +195,13 @@ impl PointerHandler for WinitState { // Mice events have both pixel and discrete delta's at the same time. So prefer // the descrite values if they are present. let delta = if has_discrete_scroll { - // XXX Wayland sign convention is the inverse of winit. + // NOTE: Wayland sign convention is the inverse of winit. MouseScrollDelta::LineDelta( (-horizontal.discrete) as f32, (-vertical.discrete) as f32, ) } else { - // XXX Wayland sign convention is the inverse of winit. + // NOTE: Wayland sign convention is the inverse of winit. MouseScrollDelta::PixelDelta( LogicalPosition::new(-horizontal.absolute, -vertical.absolute) .to_physical(scale_factor), diff --git a/src/platform_impl/linux/wayland/seat/touch/mod.rs b/src/platform_impl/linux/wayland/seat/touch/mod.rs index a037ae9f37..124504feea 100644 --- a/src/platform_impl/linux/wayland/seat/touch/mod.rs +++ b/src/platform_impl/linux/wayland/seat/touch/mod.rs @@ -1,5 +1,7 @@ //! Touch handling. +use tracing::warn; + use sctk::reexports::client::protocol::wl_seat::WlSeat; use sctk::reexports::client::protocol::wl_surface::WlSurface; use sctk::reexports::client::protocol::wl_touch::WlTouch; @@ -31,11 +33,16 @@ impl TouchHandler for WinitState { None => return, }; - let location = LogicalPosition::::from(position); - - let seat_state = self.seats.get_mut(&touch.seat().id()).unwrap(); + let seat_state = match self.seats.get_mut(&touch.seat().id()) { + Some(seat_state) => seat_state, + None => { + warn!("Received wl_touch::down without seat"); + return; + }, + }; // Update the state of the point. + let location = LogicalPosition::::from(position); seat_state.touch_map.insert(id, TouchPoint { surface, location }); self.events_sink.push_window_event( @@ -61,7 +68,13 @@ impl TouchHandler for WinitState { _: u32, id: i32, ) { - let seat_state = self.seats.get_mut(&touch.seat().id()).unwrap(); + let seat_state = match self.seats.get_mut(&touch.seat().id()) { + Some(seat_state) => seat_state, + None => { + warn!("Received wl_touch::up without seat"); + return; + }, + }; // Remove the touch point. let touch_point = match seat_state.touch_map.remove(&id) { @@ -98,7 +111,13 @@ impl TouchHandler for WinitState { id: i32, position: (f64, f64), ) { - let seat_state = self.seats.get_mut(&touch.seat().id()).unwrap(); + let seat_state = match self.seats.get_mut(&touch.seat().id()) { + Some(seat_state) => seat_state, + None => { + warn!("Received wl_touch::motion without seat"); + return; + }, + }; // Remove the touch point. let touch_point = match seat_state.touch_map.get_mut(&id) { @@ -129,7 +148,13 @@ impl TouchHandler for WinitState { } fn cancel(&mut self, _: &Connection, _: &QueueHandle, touch: &WlTouch) { - let seat_state = self.seats.get_mut(&touch.seat().id()).unwrap(); + let seat_state = match self.seats.get_mut(&touch.seat().id()) { + Some(seat_state) => seat_state, + None => { + warn!("Received wl_touch::cancel without seat"); + return; + }, + }; for (id, touch_point) in seat_state.touch_map.drain() { let window_id = wayland::make_wid(&touch_point.surface);