Skip to content

Commit e707052

Browse files
murarthgoddessfreyaryechrisduerr
authored
Move ModifiersChanged variant to WindowEvent (#1381)
* Move `ModifiersChanged` variant to `WindowEvent` * macos: Fix flags_changed for ModifiersChanged variant move I haven't look too deep at what this does internally, but at least cargo-check is fully happy now. :) Signed-off-by: Kristofer Rye <kristofer.rye@gmail.com> * macos: Fire a ModifiersChanged event on window_did_resign_key From debugging, I determined that macOS' emission of a flagsChanged around window switching is inconsistent. It is fair to assume, I think, that when the user switches windows, they do not expect their former modifiers state to remain effective; so I think it's best to clear that state by sending a ModifiersChanged(ModifiersState::empty()). Signed-off-by: Kristofer Rye <kristofer.rye@gmail.com> * windows: Fix build I don't know enough about the code to implement the fix as it is done on this branch, but this commit at least fixes the build. Signed-off-by: Kristofer Rye <kristofer.rye@gmail.com> * windows: Send ModifiersChanged(ModifiersState::empty) on KILLFOCUS Very similar to the changes made in [1], as focus is lost, send an event to the window indicating that the modifiers have been released. It's unclear to me (without a Windows device to test this on) whether this is necessary, but it certainly ensures that unfocused windows will have at least received this event, which is an improvement. [1]: f79f216 Signed-off-by: Kristofer Rye <kristofer.rye@gmail.com> * macos: Add a hook to update stale modifiers Sometimes, `ViewState` and `event` might have different values for their stored `modifiers` flags. These are internally stored as a bitmask in the latter and an enum in the former. We can check to see if they differ, and if they do, automatically dispatch an event to update consumers of modifier state as well as the stored `state.modifiers`. That's what the hook does. This hook is then called in the key_down, mouse_entered, mouse_exited, mouse_click, scroll_wheel, and pressure_change_with_event callbacks, which each will contain updated modifiers. Signed-off-by: Kristofer Rye <kristofer.rye@gmail.com> * Only call event_mods once when determining whether to update state Signed-off-by: Kristofer Rye <kristofer.rye@gmail.com> * flags_changed: Memoize window_id collection Signed-off-by: Kristofer Rye <kristofer.rye@gmail.com> * window_did_resign_key: Remove synthetic ModifiersChanged event We no longer need to emit this event, since we are checking the state of our modifiers before emitting most other events. Signed-off-by: Kristofer Rye <kristofer.rye@gmail.com> * mouse_motion: Add a call to update_potentially_stale_modifiers Now, cover all events (that I can think of, at least) where stale modifiers might affect how user programs behave. Effectively, every human-interface event (keypress, mouse click, keydown, etc.) will cause a ModifiersChanged event to be fired if something has changed. Signed-off-by: Kristofer Rye <kristofer.rye@gmail.com> * key_up: Add a call to update_potentially_stale_modifiers We also want to make sure modifiers state is synchronized here, too. Signed-off-by: Kristofer Rye <kristofer.rye@gmail.com> * mouse_motion: Remove update_potentially_stale_modifiers invocation Signed-off-by: Kristofer Rye <kristofer.rye@gmail.com> * Retry CI * ViewState: Promote visibility of modifiers to the macos impl This is so that we can interact with the ViewState directly from the WindowDelegate. Signed-off-by: Kristofer Rye <kristofer.rye@gmail.com> * window_delegate: Synthetically set modifiers state to empty on resignKey This logic is implemented similarly on other platforms, so we wish to regain parity here. Originally this behavior was implemented to always fire an event with ModifiersState::empty(), but that was not the best as it was not necessarily correct and could be a duplicate event. This solution is perhaps the most elegant possible to implement the desired behavior of sending a synthetic empty modifiers event when a window loses focus, trading some safety for interoperation between the NSWindowDelegate and the NSView (as the objc runtime must now be consulted in order to acquire access to the ViewState which is "owned" by the NSView). Signed-off-by: Kristofer Rye <kristofer.rye@gmail.com> * Check for modifiers change in window events * Fix modifier changed on macOS Since the `mouse_entered` function was generating a mouse motion, which updates the modifier state, a modifiers changed event was incorrectly generated. The updating of the modifier state has also been changed to make sure it consistently happens before events that have a modifier state attached to it, without happening on any other event. This of course means that no `CursorMoved` event is generated anymore when the user enters the window without it being focused, however I'd say that is consistent with how winit should behave. * Fix unused variable warning * Move changelog entry into `Unreleased` section Co-authored-by: Freya Gentz <zegentzy@protonmail.com> Co-authored-by: Kristofer Rye <kristofer.rye@gmail.com> Co-authored-by: Christian Duerr <contact@christianduerr.com>
1 parent 71bd6e7 commit e707052

File tree

10 files changed

+229
-122
lines changed

10 files changed

+229
-122
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
- Fix `Event::to_static` returning `None` for user events.
1111
- On Wayland, Hide CSD for fullscreen windows.
1212
- On Windows, ignore spurious mouse move messages.
13+
- **Breaking:** Move `ModifiersChanged` variant from `DeviceEvent` to `WindowEvent`.
1314

1415
# 0.21.0 (2020-02-04)
1516

‎examples/cursor_grab.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ fn main() {
3838
_ => (),
3939
}
4040
}
41+
WindowEvent::ModifiersChanged(m) => modifiers = m,
4142
_ => (),
4243
},
4344
Event::DeviceEvent { event, .. } => match event {
@@ -46,7 +47,6 @@ fn main() {
4647
ElementState::Pressed => println!("mouse button {} pressed", button),
4748
ElementState::Released => println!("mouse button {} released", button),
4849
},
49-
DeviceEvent::ModifiersChanged(m) => modifiers = m,
5050
_ => (),
5151
},
5252
_ => (),

‎src/event.rs

+12-14
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,13 @@ pub enum WindowEvent<'a> {
235235
is_synthetic: bool,
236236
},
237237

238+
/// The keyboard modifiers have changed.
239+
///
240+
/// Platform-specific behavior:
241+
/// - **Web**: This API is currently unimplemented on the web. This isn't by design - it's an
242+
/// issue, and it should get fixed - but it's the current state of the API.
243+
ModifiersChanged(ModifiersState),
244+
238245
/// The cursor has moved on the window.
239246
CursorMoved {
240247
device_id: DeviceId,
@@ -243,7 +250,7 @@ pub enum WindowEvent<'a> {
243250
/// limited by the display area and it may have been transformed by the OS to implement effects such as cursor
244251
/// acceleration, it should not be used to implement non-cursor-like interactions such as 3D camera control.
245252
position: PhysicalPosition<f64>,
246-
#[deprecated = "Deprecated in favor of DeviceEvent::ModifiersChanged"]
253+
#[deprecated = "Deprecated in favor of WindowEvent::ModifiersChanged"]
247254
modifiers: ModifiersState,
248255
},
249256

@@ -258,7 +265,7 @@ pub enum WindowEvent<'a> {
258265
device_id: DeviceId,
259266
delta: MouseScrollDelta,
260267
phase: TouchPhase,
261-
#[deprecated = "Deprecated in favor of DeviceEvent::ModifiersChanged"]
268+
#[deprecated = "Deprecated in favor of WindowEvent::ModifiersChanged"]
262269
modifiers: ModifiersState,
263270
},
264271

@@ -267,7 +274,7 @@ pub enum WindowEvent<'a> {
267274
device_id: DeviceId,
268275
state: ElementState,
269276
button: MouseButton,
270-
#[deprecated = "Deprecated in favor of DeviceEvent::ModifiersChanged"]
277+
#[deprecated = "Deprecated in favor of WindowEvent::ModifiersChanged"]
271278
modifiers: ModifiersState,
272279
},
273280

@@ -341,6 +348,7 @@ impl<'a> WindowEvent<'a> {
341348
input,
342349
is_synthetic,
343350
}),
351+
ModifiersChanged(modifiers) => Some(ModifiersChanged(modifiers)),
344352
#[allow(deprecated)]
345353
CursorMoved {
346354
device_id,
@@ -464,16 +472,6 @@ pub enum DeviceEvent {
464472

465473
Key(KeyboardInput),
466474

467-
/// The keyboard modifiers have changed.
468-
///
469-
/// This is tracked internally to avoid tracking errors arising from modifier key state changes when events from
470-
/// this device are not being delivered to the application, e.g. due to keyboard focus being elsewhere.
471-
///
472-
/// Platform-specific behavior:
473-
/// - **Web**: This API is currently unimplemented on the web. This isn't by design - it's an
474-
/// issue, and it should get fixed - but it's the current state of the API.
475-
ModifiersChanged(ModifiersState),
476-
477475
Text {
478476
codepoint: char,
479477
},
@@ -502,7 +500,7 @@ pub struct KeyboardInput {
502500
///
503501
/// This is tracked internally to avoid tracking errors arising from modifier key state changes when events from
504502
/// this device are not being delivered to the application, e.g. due to keyboard focus being elsewhere.
505-
#[deprecated = "Deprecated in favor of DeviceEvent::ModifiersChanged"]
503+
#[deprecated = "Deprecated in favor of WindowEvent::ModifiersChanged"]
506504
pub modifiers: ModifiersState,
507505
}
508506

‎src/platform_impl/linux/wayland/keyboard.rs

+19-4
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@ use smithay_client_toolkit::{
88
reexports::client::protocol::{wl_keyboard, wl_seat},
99
};
1010

11-
use crate::event::{
12-
DeviceEvent, ElementState, KeyboardInput, ModifiersState, VirtualKeyCode, WindowEvent,
13-
};
11+
use crate::event::{ElementState, KeyboardInput, ModifiersState, VirtualKeyCode, WindowEvent};
1412

1513
pub fn init_keyboard(
1614
seat: &wl_seat::WlSeat,
@@ -33,9 +31,24 @@ pub fn init_keyboard(
3331
let wid = make_wid(&surface);
3432
my_sink.send_window_event(WindowEvent::Focused(true), wid);
3533
*target.lock().unwrap() = Some(wid);
34+
35+
let modifiers = *modifiers_tracker.lock().unwrap();
36+
37+
if !modifiers.is_empty() {
38+
my_sink.send_window_event(WindowEvent::ModifiersChanged(modifiers), wid);
39+
}
3640
}
3741
KbEvent::Leave { surface, .. } => {
3842
let wid = make_wid(&surface);
43+
let modifiers = *modifiers_tracker.lock().unwrap();
44+
45+
if !modifiers.is_empty() {
46+
my_sink.send_window_event(
47+
WindowEvent::ModifiersChanged(ModifiersState::empty()),
48+
wid,
49+
);
50+
}
51+
3952
my_sink.send_window_event(WindowEvent::Focused(false), wid);
4053
*target.lock().unwrap() = None;
4154
}
@@ -88,7 +101,9 @@ pub fn init_keyboard(
88101

89102
*modifiers_tracker.lock().unwrap() = modifiers;
90103

91-
my_sink.send_device_event(DeviceEvent::ModifiersChanged(modifiers), DeviceId);
104+
if let Some(wid) = *target.lock().unwrap() {
105+
my_sink.send_window_event(WindowEvent::ModifiersChanged(modifiers), wid);
106+
}
92107
}
93108
}
94109
},

‎src/platform_impl/linux/x11/event_processor.rs

+73-43
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ pub(super) struct EventProcessor<T: 'static> {
3232
// Number of touch events currently in progress
3333
pub(super) num_touch: u32,
3434
pub(super) first_touch: Option<u64>,
35+
// Currently focused window belonging to this process
36+
pub(super) active_window: Option<ffi::Window>,
3537
}
3638

3739
impl<T: 'static> EventProcessor<T> {
@@ -136,11 +138,12 @@ impl<T: 'static> EventProcessor<T> {
136138
if let Some(modifiers) =
137139
self.device_mod_state.update_state(&state, modifier)
138140
{
139-
let device_id = mkdid(util::VIRTUAL_CORE_KEYBOARD);
140-
callback(Event::DeviceEvent {
141-
device_id,
142-
event: DeviceEvent::ModifiersChanged(modifiers),
143-
});
141+
if let Some(window_id) = self.active_window {
142+
callback(Event::WindowEvent {
143+
window_id: mkwid(window_id),
144+
event: WindowEvent::ModifiersChanged(modifiers),
145+
});
146+
}
144147
}
145148
}
146149
}
@@ -872,44 +875,58 @@ impl<T: 'static> EventProcessor<T> {
872875
ffi::XI_FocusIn => {
873876
let xev: &ffi::XIFocusInEvent = unsafe { &*(xev.data as *const _) };
874877

875-
let window_id = mkwid(xev.event);
876-
877878
wt.ime
878879
.borrow_mut()
879880
.focus(xev.event)
880881
.expect("Failed to focus input context");
881882

882-
callback(Event::WindowEvent {
883-
window_id,
884-
event: Focused(true),
885-
});
886-
887883
let modifiers = ModifiersState::from_x11(&xev.mods);
888884

889-
update_modifiers!(modifiers, None);
885+
self.device_mod_state.update_state(&modifiers, None);
890886

891-
// The deviceid for this event is for a keyboard instead of a pointer,
892-
// so we have to do a little extra work.
893-
let pointer_id = self
894-
.devices
895-
.borrow()
896-
.get(&DeviceId(xev.deviceid))
897-
.map(|device| device.attachment)
898-
.unwrap_or(2);
887+
if self.active_window != Some(xev.event) {
888+
self.active_window = Some(xev.event);
899889

900-
let position = PhysicalPosition::new(xev.event_x, xev.event_y);
890+
let window_id = mkwid(xev.event);
891+
let position = PhysicalPosition::new(xev.event_x, xev.event_y);
901892

902-
callback(Event::WindowEvent {
903-
window_id,
904-
event: CursorMoved {
905-
device_id: mkdid(pointer_id),
906-
position,
907-
modifiers,
908-
},
909-
});
893+
callback(Event::WindowEvent {
894+
window_id,
895+
event: Focused(true),
896+
});
897+
898+
if !modifiers.is_empty() {
899+
callback(Event::WindowEvent {
900+
window_id,
901+
event: WindowEvent::ModifiersChanged(modifiers),
902+
});
903+
}
910904

911-
// Issue key press events for all pressed keys
912-
self.handle_pressed_keys(window_id, ElementState::Pressed, &mut callback);
905+
// The deviceid for this event is for a keyboard instead of a pointer,
906+
// so we have to do a little extra work.
907+
let pointer_id = self
908+
.devices
909+
.borrow()
910+
.get(&DeviceId(xev.deviceid))
911+
.map(|device| device.attachment)
912+
.unwrap_or(2);
913+
914+
callback(Event::WindowEvent {
915+
window_id,
916+
event: CursorMoved {
917+
device_id: mkdid(pointer_id),
918+
position,
919+
modifiers,
920+
},
921+
});
922+
923+
// Issue key press events for all pressed keys
924+
self.handle_pressed_keys(
925+
window_id,
926+
ElementState::Pressed,
927+
&mut callback,
928+
);
929+
}
913930
}
914931
ffi::XI_FocusOut => {
915932
let xev: &ffi::XIFocusOutEvent = unsafe { &*(xev.data as *const _) };
@@ -921,15 +938,26 @@ impl<T: 'static> EventProcessor<T> {
921938
.unfocus(xev.event)
922939
.expect("Failed to unfocus input context");
923940

924-
let window_id = mkwid(xev.event);
941+
if self.active_window.take() == Some(xev.event) {
942+
let window_id = mkwid(xev.event);
925943

926-
// Issue key release events for all pressed keys
927-
self.handle_pressed_keys(window_id, ElementState::Released, &mut callback);
944+
// Issue key release events for all pressed keys
945+
self.handle_pressed_keys(
946+
window_id,
947+
ElementState::Released,
948+
&mut callback,
949+
);
928950

929-
callback(Event::WindowEvent {
930-
window_id,
931-
event: Focused(false),
932-
})
951+
callback(Event::WindowEvent {
952+
window_id,
953+
event: WindowEvent::ModifiersChanged(ModifiersState::empty()),
954+
});
955+
956+
callback(Event::WindowEvent {
957+
window_id,
958+
event: Focused(false),
959+
})
960+
}
933961
}
934962

935963
ffi::XI_TouchBegin | ffi::XI_TouchUpdate | ffi::XI_TouchEnd => {
@@ -1084,10 +1112,12 @@ impl<T: 'static> EventProcessor<T> {
10841112
let new_modifiers = self.device_mod_state.modifiers();
10851113

10861114
if modifiers != new_modifiers {
1087-
callback(Event::DeviceEvent {
1088-
device_id,
1089-
event: DeviceEvent::ModifiersChanged(new_modifiers),
1090-
});
1115+
if let Some(window_id) = self.active_window {
1116+
callback(Event::WindowEvent {
1117+
window_id: mkwid(window_id),
1118+
event: WindowEvent::ModifiersChanged(new_modifiers),
1119+
});
1120+
}
10911121
}
10921122
}
10931123
}

‎src/platform_impl/linux/x11/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,7 @@ impl<T: 'static> EventLoop<T> {
225225
device_mod_state: Default::default(),
226226
num_touch: 0,
227227
first_touch: None,
228+
active_window: None,
228229
};
229230

230231
// Register for device hotplug events

0 commit comments

Comments
 (0)