From 0584dd2f1e82417bdabcc0d8cb20fddc2e8cc5e7 Mon Sep 17 00:00:00 2001 From: sodiboo <37938646+sodiboo@users.noreply.github.com> Date: Sat, 18 Jan 2025 15:26:42 +0100 Subject: implement `keyboard-shortcuts-inhibit` and `wlr-virtual-pointer` (#630) * stub keyboard-shortcuts-inhibit and virtual-pointer impls * implement keyboard-shortcuts-inhibit * implement virtual-pointer * deal with supressed key release edge-case; add allow-inhibiting property * add toggle-keyboard-shortcuts-inhibit bind * add InputBackend extensions; use Device::output() for absolute pos events * add a `State` parameter to the backend exts and better document future intent * Add some tests for is_inhibiting_shortcuts --------- Co-authored-by: Ivan Molodetskikh --- src/input/backend_ext.rs | 51 ++++++++++++++++ src/input/mod.rs | 151 +++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 189 insertions(+), 13 deletions(-) create mode 100644 src/input/backend_ext.rs (limited to 'src/input') diff --git a/src/input/backend_ext.rs b/src/input/backend_ext.rs new file mode 100644 index 00000000..99f4a904 --- /dev/null +++ b/src/input/backend_ext.rs @@ -0,0 +1,51 @@ +use ::input as libinput; +use smithay::backend::input; +use smithay::backend::winit::WinitVirtualDevice; +use smithay::output::Output; + +use crate::niri::State; +use crate::protocols::virtual_pointer::VirtualPointer; + +pub trait NiriInputBackend: input::InputBackend { + type NiriDevice: NiriInputDevice; +} +impl NiriInputBackend for T +where + Self::Device: NiriInputDevice, +{ + type NiriDevice = Self::Device; +} + +pub trait NiriInputDevice: input::Device { + // FIXME: this should maybe be per-event, not per-device, + // but it's not clear that this matters in practice? + // it might be more obvious once we implement it for libinput + fn output(&self, state: &State) -> Option; +} + +impl NiriInputDevice for libinput::Device { + fn output(&self, _state: &State) -> Option { + // FIXME: Allow specifying the output per-device? + None + } +} + +impl NiriInputDevice for WinitVirtualDevice { + fn output(&self, _state: &State) -> Option { + // FIXME: we should be returning the single output that the winit backend creates, + // but for now, that will cause issues because the output is normally upside down, + // so we apply Transform::Flipped180 to it and that would also cause + // the cursor position to be flipped, which is not what we want. + // + // instead, we just return None and rely on the fact that it has only one output. + // doing so causes the cursor to be placed in *global* output coordinates, + // which are not flipped, and happen to be what we want. + None + } +} + +impl NiriInputDevice for VirtualPointer { + fn output(&self, _: &State) -> Option { + self.output().cloned() + } +} diff --git a/src/input/mod.rs b/src/input/mod.rs index b08fe380..8872df78 100644 --- a/src/input/mod.rs +++ b/src/input/mod.rs @@ -11,7 +11,7 @@ use niri_ipc::LayoutSwitchTarget; use smithay::backend::input::{ AbsolutePositionEvent, Axis, AxisSource, ButtonState, Device, DeviceCapability, Event, GestureBeginEvent, GestureEndEvent, GesturePinchUpdateEvent as _, GestureSwipeUpdateEvent as _, - InputBackend, InputEvent, KeyState, KeyboardKeyEvent, Keycode, MouseButton, PointerAxisEvent, + InputEvent, KeyState, KeyboardKeyEvent, Keycode, MouseButton, PointerAxisEvent, PointerButtonEvent, PointerMotionEvent, ProximityState, Switch, SwitchState, SwitchToggleEvent, TabletToolButtonEvent, TabletToolEvent, TabletToolProximityEvent, TabletToolTipEvent, TabletToolTipState, TouchEvent, @@ -28,7 +28,9 @@ use smithay::input::touch::{ DownEvent, GrabStartData as TouchGrabStartData, MotionEvent as TouchMotionEvent, UpEvent, }; use smithay::input::SeatHandler; +use smithay::output::Output; use smithay::utils::{Logical, Point, Rectangle, Transform, SERIAL_COUNTER}; +use smithay::wayland::keyboard_shortcuts_inhibit::KeyboardShortcutsInhibitor; use smithay::wayland::pointer_constraints::{with_pointer_constraint, PointerConstraint}; use smithay::wayland::tablet_manager::{TabletDescriptor, TabletSeatTrait}; use touch_move_grab::TouchMoveGrab; @@ -42,6 +44,7 @@ use crate::ui::screenshot_ui::ScreenshotUi; use crate::utils::spawning::spawn; use crate::utils::{center, get_monotonic_time, ResizeEdge}; +pub mod backend_ext; pub mod move_grab; pub mod resize_grab; pub mod scroll_tracker; @@ -50,6 +53,8 @@ pub mod swipe_tracker; pub mod touch_move_grab; pub mod touch_resize_grab; +use backend_ext::{NiriInputBackend as InputBackend, NiriInputDevice as _}; + pub const DOUBLE_CLICK_TIME: Duration = Duration::from_millis(400); #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -264,8 +269,10 @@ impl State { where I::Device: 'static, { + let device_output = event.device().output(self); + let device_output = device_output.as_ref(); let (target_geo, keep_ratio, px, transform) = - if let Some(output) = self.niri.output_for_tablet() { + if let Some(output) = device_output.or_else(|| self.niri.output_for_tablet()) { ( self.niri.global_space.output_geometry(output).unwrap(), true, @@ -318,6 +325,18 @@ impl State { Some(pos + target_geo.loc.to_f64()) } + fn is_inhibiting_shortcuts(&self) -> bool { + self.niri + .keyboard_focus + .surface() + .and_then(|surface| { + self.niri + .keyboard_shortcuts_inhibiting_surfaces + .get(surface) + }) + .is_some_and(KeyboardShortcutsInhibitor::is_active) + } + fn on_keyboard(&mut self, event: I::KeyboardKeyEvent) { let comp_mod = self.backend.mod_key(); @@ -342,6 +361,8 @@ impl State { self.hide_cursor_if_needed(); } + let is_inhibiting_shortcuts = self.is_inhibiting_shortcuts(); + let Some(Some(bind)) = self.niri.seat.get_keyboard().unwrap().input( self, event.key_code(), @@ -372,6 +393,7 @@ impl State { *mods, &this.niri.screenshot_ui, this.niri.config.borrow().input.disable_power_key_handling, + is_inhibiting_shortcuts, ) }, ) else { @@ -610,6 +632,19 @@ impl State { }); } } + Action::ToggleKeyboardShortcutsInhibit => { + if let Some(inhibitor) = self.niri.keyboard_focus.surface().and_then(|surface| { + self.niri + .keyboard_shortcuts_inhibiting_surfaces + .get(surface) + }) { + if inhibitor.is_active() { + inhibitor.inactivate(); + } else { + inhibitor.activate(); + } + } + } Action::CloseWindow => { if let Some(mapped) = self.niri.layout.focus() { mapped.toplevel().send_close(); @@ -1731,12 +1766,14 @@ impl State { &mut self, event: I::PointerMotionAbsoluteEvent, ) { - let Some(output_geo) = self.global_bounding_rectangle() else { + let Some(pos) = self.compute_absolute_location(&event, None).or_else(|| { + self.global_bounding_rectangle().map(|output_geo| { + event.position_transformed(output_geo.size) + output_geo.loc.to_f64() + }) + }) else { return; }; - let pos = event.position_transformed(output_geo.size) + output_geo.loc.to_f64(); - let serial = SERIAL_COUNTER.next_serial(); let pointer = self.niri.seat.get_pointer().unwrap(); @@ -2613,14 +2650,13 @@ impl State { ); } - /// Computes the cursor position for the touch event. - /// - /// This function handles the touch output mapping, as well as coordinate transform - fn compute_touch_location>( + fn compute_absolute_location( &self, - evt: &E, + evt: &impl AbsolutePositionEvent, + fallback_output: Option<&Output>, ) -> Option> { - let output = self.niri.output_for_touch()?; + let output = evt.device().output(self); + let output = output.as_ref().or(fallback_output)?; let output_geo = self.niri.global_space.output_geometry(output).unwrap(); let transform = output.current_transform(); let size = transform.invert().transform_size(output_geo.size); @@ -2630,6 +2666,16 @@ impl State { ) } + /// Computes the cursor position for the touch event. + /// + /// This function handles the touch output mapping, as well as coordinate transform + fn compute_touch_location( + &self, + evt: &impl AbsolutePositionEvent, + ) -> Option> { + self.compute_absolute_location(evt, self.niri.output_for_touch()) + } + fn on_touch_down(&mut self, evt: I::TouchDownEvent) { let Some(handle) = self.niri.seat.get_touch() else { return; @@ -2780,6 +2826,7 @@ fn should_intercept_key( mods: ModifiersState, screenshot_ui: &ScreenshotUi, disable_power_key_handling: bool, + is_inhibiting_shortcuts: bool, ) -> FilterResult> { // Actions are only triggered on presses, release of the key // shouldn't try to intercept anything unless we have marked @@ -2820,6 +2867,10 @@ fn should_intercept_key( repeat: true, cooldown: None, allow_when_locked: false, + // The screenshot UI owns the focus anyway, so this doesn't really matter. + // But logically, nothing can inhibit its actions. Only opening it can be + // inhibited. + allow_inhibiting: false, }); } } @@ -2827,10 +2878,19 @@ fn should_intercept_key( match (final_bind, pressed) { (Some(bind), true) => { - suppressed_keys.insert(key_code); - FilterResult::Intercept(Some(bind)) + if is_inhibiting_shortcuts && bind.allow_inhibiting { + FilterResult::Forward + } else { + suppressed_keys.insert(key_code); + FilterResult::Intercept(Some(bind)) + } } (_, false) => { + // By this point, we know that the key was supressed on press. Even if we're inhibiting + // shortcuts, we should still suppress the release. + // But we don't need to check for shortcuts inhibition here, because + // if it was inhibited on press (forwarded to the client), it wouldn't be suppressed, + // so the release would already have been forwarded at the start of this function. suppressed_keys.remove(&key_code); FilterResult::Intercept(None) } @@ -2870,6 +2930,12 @@ fn find_bind( repeat: true, cooldown: None, allow_when_locked: false, + // In a worst-case scenario, the user has no way to unlock the compositor and a + // misbehaving client has a keyboard shortcuts inhibitor, "jailing" the user. + // The user must always be able to change VTs to recover from such a situation. + // It also makes no sense to inhibit the default power key handling. + // Hardcoded binds must never be inhibited. + allow_inhibiting: false, }); } @@ -3035,6 +3101,7 @@ fn allowed_when_locked(action: &Action) -> bool { | Action::PowerOffMonitors | Action::PowerOnMonitors | Action::SwitchLayout(_) + | Action::ToggleKeyboardShortcutsInhibit ) } @@ -3317,6 +3384,8 @@ pub fn mods_with_finger_scroll_binds(comp_mod: CompositorMod, binds: &Binds) -> #[cfg(test)] mod tests { + use std::cell::Cell; + use super::*; use crate::animation::Clock; @@ -3332,6 +3401,7 @@ mod tests { repeat: true, cooldown: None, allow_when_locked: false, + allow_inhibiting: true, }]); let comp_mod = CompositorMod::Super; @@ -3339,6 +3409,7 @@ mod tests { let screenshot_ui = ScreenshotUi::new(Clock::default(), Default::default()); let disable_power_key_handling = false; + let is_inhibiting_shortcuts = Cell::new(false); // The key_code we pick is arbitrary, the only thing // that matters is that they are different between cases. @@ -3356,6 +3427,7 @@ mod tests { mods, &screenshot_ui, disable_power_key_handling, + is_inhibiting_shortcuts.get(), ) }; @@ -3372,6 +3444,7 @@ mod tests { mods, &screenshot_ui, disable_power_key_handling, + is_inhibiting_shortcuts.get(), ) }; @@ -3452,6 +3525,53 @@ mod tests { // Ensure that no keys are being suppressed. assert!(suppressed_keys.is_empty()); + + // Now test shortcut inhibiting. + + // With inhibited shortcuts, we don't intercept our shortcut. + is_inhibiting_shortcuts.set(true); + + mods = ModifiersState { + logo: true, + ctrl: true, + ..Default::default() + }; + + let filter = close_key_event(&mut suppressed_keys, mods, true); + assert!(matches!(filter, FilterResult::Forward)); + assert!(suppressed_keys.is_empty()); + + let filter = close_key_event(&mut suppressed_keys, mods, false); + assert!(matches!(filter, FilterResult::Forward)); + assert!(suppressed_keys.is_empty()); + + // Toggle it off after pressing the shortcut. + let filter = close_key_event(&mut suppressed_keys, mods, true); + assert!(matches!(filter, FilterResult::Forward)); + assert!(suppressed_keys.is_empty()); + + is_inhibiting_shortcuts.set(false); + + let filter = close_key_event(&mut suppressed_keys, mods, false); + assert!(matches!(filter, FilterResult::Forward)); + assert!(suppressed_keys.is_empty()); + + // Toggle it on after pressing the shortcut. + let filter = close_key_event(&mut suppressed_keys, mods, true); + assert!(matches!( + filter, + FilterResult::Intercept(Some(Bind { + action: Action::CloseWindow, + .. + })) + )); + assert!(suppressed_keys.contains(&close_key_code)); + + is_inhibiting_shortcuts.set(true); + + let filter = close_key_event(&mut suppressed_keys, mods, false); + assert!(matches!(filter, FilterResult::Intercept(None))); + assert!(suppressed_keys.is_empty()); } #[test] @@ -3466,6 +3586,7 @@ mod tests { repeat: true, cooldown: None, allow_when_locked: false, + allow_inhibiting: true, }, Bind { key: Key { @@ -3476,6 +3597,7 @@ mod tests { repeat: true, cooldown: None, allow_when_locked: false, + allow_inhibiting: true, }, Bind { key: Key { @@ -3486,6 +3608,7 @@ mod tests { repeat: true, cooldown: None, allow_when_locked: false, + allow_inhibiting: true, }, Bind { key: Key { @@ -3496,6 +3619,7 @@ mod tests { repeat: true, cooldown: None, allow_when_locked: false, + allow_inhibiting: true, }, Bind { key: Key { @@ -3506,6 +3630,7 @@ mod tests { repeat: true, cooldown: None, allow_when_locked: false, + allow_inhibiting: true, }, ]); -- cgit