From ed348f231e6dee95f3a03261c849e6a0c21b287b Mon Sep 17 00:00:00 2001 From: Murarth Date: Wed, 8 Jan 2020 20:22:38 -0700 Subject: [PATCH] Move `ModifiersChanged` variant to `WindowEvent` --- CHANGELOG.md | 1 + examples/cursor_grab.rs | 2 +- src/event.rs | 26 ++-- src/platform_impl/linux/wayland/keyboard.rs | 23 +++- .../linux/x11/event_processor.rs | 119 +++++++++++------- src/platform_impl/linux/x11/mod.rs | 1 + 6 files changed, 109 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2be5dfad5..7b3ee2a56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ - On Windows, fix bug where `RedrawRequested` would only get emitted every other iteration of the event loop. - On X11, fix deadlock on window state when handling certain window events. - `WindowBuilder` now implements `Default`. +- **Breaking:** Move `ModifiersChanged` variant from `DeviceEvent` to `WindowEvent`. # 0.20.0 (2020-01-05) diff --git a/examples/cursor_grab.rs b/examples/cursor_grab.rs index 5ed193ecb..317577e3c 100644 --- a/examples/cursor_grab.rs +++ b/examples/cursor_grab.rs @@ -38,6 +38,7 @@ fn main() { _ => (), } } + WindowEvent::ModifiersChanged(m) => modifiers = m, _ => (), }, Event::DeviceEvent { event, .. } => match event { @@ -46,7 +47,6 @@ fn main() { ElementState::Pressed => println!("mouse button {} pressed", button), ElementState::Released => println!("mouse button {} released", button), }, - DeviceEvent::ModifiersChanged(m) => modifiers = m, _ => (), }, _ => (), diff --git a/src/event.rs b/src/event.rs index 9f30e8811..c6d68b0bd 100644 --- a/src/event.rs +++ b/src/event.rs @@ -235,6 +235,13 @@ pub enum WindowEvent<'a> { is_synthetic: bool, }, + /// The keyboard modifiers have changed. + /// + /// Platform-specific behavior: + /// - **Web**: This API is currently unimplemented on the web. This isn't by design - it's an + /// issue, and it should get fixed - but it's the current state of the API. + ModifiersChanged(ModifiersState), + /// The cursor has moved on the window. CursorMoved { device_id: DeviceId, @@ -243,7 +250,7 @@ pub enum WindowEvent<'a> { /// limited by the display area and it may have been transformed by the OS to implement effects such as cursor /// acceleration, it should not be used to implement non-cursor-like interactions such as 3D camera control. position: PhysicalPosition, - #[deprecated = "Deprecated in favor of DeviceEvent::ModifiersChanged"] + #[deprecated = "Deprecated in favor of WindowEvent::ModifiersChanged"] modifiers: ModifiersState, }, @@ -258,7 +265,7 @@ pub enum WindowEvent<'a> { device_id: DeviceId, delta: MouseScrollDelta, phase: TouchPhase, - #[deprecated = "Deprecated in favor of DeviceEvent::ModifiersChanged"] + #[deprecated = "Deprecated in favor of WindowEvent::ModifiersChanged"] modifiers: ModifiersState, }, @@ -267,7 +274,7 @@ pub enum WindowEvent<'a> { device_id: DeviceId, state: ElementState, button: MouseButton, - #[deprecated = "Deprecated in favor of DeviceEvent::ModifiersChanged"] + #[deprecated = "Deprecated in favor of WindowEvent::ModifiersChanged"] modifiers: ModifiersState, }, @@ -341,6 +348,7 @@ impl<'a> WindowEvent<'a> { input, is_synthetic, }), + ModifiersChanged(modifiers) => Some(ModifiersChanged(modifiers)), #[allow(deprecated)] CursorMoved { device_id, @@ -464,16 +472,6 @@ pub enum DeviceEvent { Key(KeyboardInput), - /// The keyboard modifiers have changed. - /// - /// This is tracked internally to avoid tracking errors arising from modifier key state changes when events from - /// this device are not being delivered to the application, e.g. due to keyboard focus being elsewhere. - /// - /// Platform-specific behavior: - /// - **Web**: This API is currently unimplemented on the web. This isn't by design - it's an - /// issue, and it should get fixed - but it's the current state of the API. - ModifiersChanged(ModifiersState), - Text { codepoint: char, }, @@ -502,7 +500,7 @@ pub struct KeyboardInput { /// /// This is tracked internally to avoid tracking errors arising from modifier key state changes when events from /// this device are not being delivered to the application, e.g. due to keyboard focus being elsewhere. - #[deprecated = "Deprecated in favor of DeviceEvent::ModifiersChanged"] + #[deprecated = "Deprecated in favor of WindowEvent::ModifiersChanged"] pub modifiers: ModifiersState, } diff --git a/src/platform_impl/linux/wayland/keyboard.rs b/src/platform_impl/linux/wayland/keyboard.rs index ed7aa20a1..8f911888f 100644 --- a/src/platform_impl/linux/wayland/keyboard.rs +++ b/src/platform_impl/linux/wayland/keyboard.rs @@ -8,9 +8,7 @@ use smithay_client_toolkit::{ reexports::client::protocol::{wl_keyboard, wl_seat}, }; -use crate::event::{ - DeviceEvent, ElementState, KeyboardInput, ModifiersState, VirtualKeyCode, WindowEvent, -}; +use crate::event::{ElementState, KeyboardInput, ModifiersState, VirtualKeyCode, WindowEvent}; pub fn init_keyboard( seat: &wl_seat::WlSeat, @@ -33,9 +31,24 @@ pub fn init_keyboard( let wid = make_wid(&surface); my_sink.send_window_event(WindowEvent::Focused(true), wid); *target.lock().unwrap() = Some(wid); + + let modifiers = *modifiers_tracker.lock().unwrap(); + + if !modifiers.is_empty() { + my_sink.send_window_event(WindowEvent::ModifiersChanged(modifiers), wid); + } } KbEvent::Leave { surface, .. } => { let wid = make_wid(&surface); + let modifiers = *modifiers_tracker.lock().unwrap(); + + if !modifiers.is_empty() { + my_sink.send_window_event( + WindowEvent::ModifiersChanged(ModifiersState::empty()), + wid, + ); + } + my_sink.send_window_event(WindowEvent::Focused(false), wid); *target.lock().unwrap() = None; } @@ -88,7 +101,9 @@ pub fn init_keyboard( *modifiers_tracker.lock().unwrap() = modifiers; - my_sink.send_device_event(DeviceEvent::ModifiersChanged(modifiers), DeviceId); + if let Some(wid) = *target.lock().unwrap() { + my_sink.send_window_event(WindowEvent::ModifiersChanged(modifiers), wid); + } } } }, diff --git a/src/platform_impl/linux/x11/event_processor.rs b/src/platform_impl/linux/x11/event_processor.rs index 14fc1b1ff..b4c6a82ff 100644 --- a/src/platform_impl/linux/x11/event_processor.rs +++ b/src/platform_impl/linux/x11/event_processor.rs @@ -32,6 +32,8 @@ pub(super) struct EventProcessor { // Number of touch events currently in progress pub(super) num_touch: u32, pub(super) first_touch: Option, + // Currently focused window belonging to this process + pub(super) active_window: Option, } impl EventProcessor { @@ -136,11 +138,12 @@ impl EventProcessor { if let Some(modifiers) = self.device_mod_state.update_state(&state, modifier) { - let device_id = mkdid(util::VIRTUAL_CORE_KEYBOARD); - callback(Event::DeviceEvent { - device_id, - event: DeviceEvent::ModifiersChanged(modifiers), - }); + if let Some(window_id) = self.active_window { + callback(Event::WindowEvent { + window_id: mkwid(window_id), + event: WindowEvent::ModifiersChanged(modifiers), + }); + } } } } @@ -874,45 +877,60 @@ impl EventProcessor { ffi::XI_FocusIn => { let xev: &ffi::XIFocusInEvent = unsafe { &*(xev.data as *const _) }; - let window_id = mkwid(xev.event); - wt.ime .borrow_mut() .focus(xev.event) .expect("Failed to focus input context"); - callback(Event::WindowEvent { - window_id, - event: Focused(true), - }); - let modifiers = ModifiersState::from_x11(&xev.mods); - update_modifiers!(modifiers, None); + self.device_mod_state.update_state(&modifiers, None); - // The deviceid for this event is for a keyboard instead of a pointer, - // so we have to do a little extra work. - let pointer_id = self - .devices - .borrow() - .get(&DeviceId(xev.deviceid)) - .map(|device| device.attachment) - .unwrap_or(2); + if self.active_window != Some(xev.event) { + self.active_window = Some(xev.event); - let position = - PhysicalPosition::new(xev.event_x as i32, xev.event_y as i32); + let window_id = mkwid(xev.event); - callback(Event::WindowEvent { - window_id, - event: CursorMoved { - device_id: mkdid(pointer_id), - position, - modifiers, - }, - }); + callback(Event::WindowEvent { + window_id, + event: Focused(true), + }); - // Issue key press events for all pressed keys - self.handle_pressed_keys(window_id, ElementState::Pressed, &mut callback); + if !modifiers.is_empty() { + callback(Event::WindowEvent { + window_id, + event: WindowEvent::ModifiersChanged(modifiers), + }); + } + + // The deviceid for this event is for a keyboard instead of a pointer, + // so we have to do a little extra work. + let pointer_id = self + .devices + .borrow() + .get(&DeviceId(xev.deviceid)) + .map(|device| device.attachment) + .unwrap_or(2); + + let position = + PhysicalPosition::new(xev.event_x as i32, xev.event_y as i32); + + callback(Event::WindowEvent { + window_id, + event: CursorMoved { + device_id: mkdid(pointer_id), + position, + modifiers, + }, + }); + + // Issue key press events for all pressed keys + self.handle_pressed_keys( + window_id, + ElementState::Pressed, + &mut callback, + ); + } } ffi::XI_FocusOut => { let xev: &ffi::XIFocusOutEvent = unsafe { &*(xev.data as *const _) }; @@ -924,15 +942,26 @@ impl EventProcessor { .unfocus(xev.event) .expect("Failed to unfocus input context"); - let window_id = mkwid(xev.event); + if self.active_window.take() == Some(xev.event) { + let window_id = mkwid(xev.event); - // Issue key release events for all pressed keys - self.handle_pressed_keys(window_id, ElementState::Released, &mut callback); + // Issue key release events for all pressed keys + self.handle_pressed_keys( + window_id, + ElementState::Released, + &mut callback, + ); - callback(Event::WindowEvent { - window_id, - event: Focused(false), - }) + callback(Event::WindowEvent { + window_id, + event: WindowEvent::ModifiersChanged(ModifiersState::empty()), + }); + + callback(Event::WindowEvent { + window_id, + event: Focused(false), + }) + } } ffi::XI_TouchBegin | ffi::XI_TouchUpdate | ffi::XI_TouchEnd => { @@ -1087,10 +1116,12 @@ impl EventProcessor { let new_modifiers = self.device_mod_state.modifiers(); if modifiers != new_modifiers { - callback(Event::DeviceEvent { - device_id, - event: DeviceEvent::ModifiersChanged(new_modifiers), - }); + if let Some(window_id) = self.active_window { + callback(Event::WindowEvent { + window_id: mkwid(window_id), + event: WindowEvent::ModifiersChanged(new_modifiers), + }); + } } } } diff --git a/src/platform_impl/linux/x11/mod.rs b/src/platform_impl/linux/x11/mod.rs index f0275bf93..0d548f6f6 100644 --- a/src/platform_impl/linux/x11/mod.rs +++ b/src/platform_impl/linux/x11/mod.rs @@ -206,6 +206,7 @@ impl EventLoop { device_mod_state: Default::default(), num_touch: 0, first_touch: None, + active_window: None, }; // Register for device hotplug events