rust-x-bindings / rust-xcb

Rust bindings and wrapper for XCB.
MIT License
161 stars 63 forks source link

Add `is_from_send_event()` for all events to detect synthetic events + keyboard example #237

Closed bew closed 4 months ago

bew commented 1 year ago

Hello!

First of all, thanks for this library, it's very nice to use!

This PR adds is_from_send_event() to all events struct, to offer a nicer-to-use interface to the last bit of response_type. The documentation mentions:

Every event contains an 8-bit type code. The most significant bit in this code is set if the event was generated from a SendEvent request.

Instead of having to manually check ev.response_code() & 0x80 != 0 in an application, which feels like a magic value, having is_from_send_event() directly available is more readable, makes it easier to use and is discoverable.

I considered making another function to add a function for the event type (ev.response_code() & 0x7f), however naming-things-is-hard™: event_type would conflict for 21 structs, and I don't think making a breaking change on response_type is worth it. Moreover since the events are already separated in different structures, I don't think there's much use to have it exposed. (and TheEvent::NUMBER has it's type code anyway)

Here is a preview of the diff of generated content: is_from_send_event.diff.txt


In addition to this helper method, I updated (& renamed) the xkb_init example to xkb_keyboard_and_mouse_events and continued playing with xcb (:smiley:), making a small tool similar (but slim'ed down) to xev.

The output would be nicer if getting the keysyms from xkb, but I'm didn't find keysyms-related functions and adding a dependency on libxkbcommon just for an example seems a bit too much. What do you think?

Here is a sample output:

click to open ``` >>> XCB & XKB initialized, watching events now >>> Press `Escape` to quit Window 0x1c200000 got input focus Window 0x1c200000 received key DOWN Keycode: 37 Event modifiers: (empty) Window 0x1c200000 received key UP Keycode: 37 Event modifiers: CONTROL XKB last state: Change: MODIFIER_STATE | MODIFIER_BASE | COMPAT_STATE | GRAB_MODS | COMPAT_GRAB_MODS | LOOKUP_MODS | COMPAT_LOOKUP_MODS Modifiers: CONTROL Window 0x1c200000 received key DOWN Keycode: 50 Event modifiers: (empty) Window 0x1c200000 received key UP Keycode: 50 Event modifiers: SHIFT XKB last state: Change: MODIFIER_STATE | MODIFIER_BASE | COMPAT_STATE | GRAB_MODS | COMPAT_GRAB_MODS | LOOKUP_MODS | COMPAT_LOOKUP_MODS Modifiers: SHIFT Window 0x1c200000 lost input focus Window 0x1c200000 got input focus Window 0x1c200000 received mouse button DOWN Button: 1 Event modifiers: (empty) Window 0x1c200000 received mouse button (BUTTON1) movement: (root x=1010 y=849) | (win x=132 y=306) Window 0x1c200000 received mouse button (BUTTON1) movement: (root x=1011 y=849) | (win x=133 y=306) Window 0x1c200000 received mouse button (BUTTON1) movement: (root x=1011 y=848) | (win x=133 y=305) Window 0x1c200000 received mouse button (BUTTON1) movement: (root x=1012 y=848) | (win x=134 y=305) Window 0x1c200000 received mouse button (BUTTON1) movement: (root x=1012 y=848) | (win x=134 y=305) Window 0x1c200000 received mouse button UP Button: 1 Event modifiers: BUTTON1 XKB last state: Change: POINTER_BUTTONS Buttons: BUTTON1 Window 0x1c200000 received mouse button DOWN Button: 1 Event modifiers: (empty) Window 0x1c200000 received mouse button (BUTTON1) movement: (root x=1097 y=746) | (win x=219 y=203) Window 0x1c200000 received mouse button (BUTTON1) movement: (root x=1098 y=746) | (win x=220 y=203) Window 0x1c200000 received mouse button (BUTTON1) movement: (root x=1099 y=746) | (win x=221 y=203) Window 0x1c200000 received mouse button (BUTTON1) movement: (root x=1100 y=746) | (win x=222 y=203) Window 0x1c200000 received mouse button (BUTTON1) movement: (root x=1100 y=746) | (win x=222 y=203) Window 0x1c200000 received mouse button (BUTTON1) movement: (root x=1101 y=746) | (win x=223 y=203) Window 0x1c200000 received mouse button (BUTTON1) movement: (root x=1101 y=745) | (win x=223 y=202) Window 0x1c200000 received mouse button (BUTTON1) movement: (root x=1102 y=745) | (win x=224 y=202) Window 0x1c200000 received mouse button UP Button: 1 Event modifiers: BUTTON1 XKB last state: Change: POINTER_BUTTONS Buttons: BUTTON1 Window 0x1c200000 received key DOWN Keycode: 50 Event modifiers: (empty) Window 0x1c200000 received mouse button DOWN Button: 1 Event modifiers: SHIFT XKB last state: Change: MODIFIER_STATE | MODIFIER_BASE | COMPAT_STATE | GRAB_MODS | COMPAT_GRAB_MODS | LOOKUP_MODS | COMPAT_LOOKUP_MODS Modifiers: SHIFT Window 0x1c200000 received mouse button (SHIFT | BUTTON1) movement: (root x=1107 y=736) | (win x=229 y=193) Window 0x1c200000 received mouse button (SHIFT | BUTTON1) movement: (root x=1107 y=736) | (win x=229 y=193) Window 0x1c200000 received mouse button (SHIFT | BUTTON1) movement: (root x=1107 y=736) | (win x=229 y=193) Window 0x1c200000 received mouse button (SHIFT | BUTTON1) movement: (root x=1107 y=735) | (win x=229 y=192) Window 0x1c200000 received mouse button (SHIFT | BUTTON1) movement: (root x=1108 y=735) | (win x=230 y=192) Window 0x1c200000 received mouse button (SHIFT | BUTTON1) movement: (root x=1109 y=735) | (win x=231 y=192) Window 0x1c200000 received mouse button (SHIFT | BUTTON1) movement: (root x=1109 y=735) | (win x=231 y=192) Window 0x1c200000 received mouse button UP Button: 1 Event modifiers: SHIFT | BUTTON1 XKB last state: Change: POINTER_BUTTONS Modifiers: SHIFT Buttons: BUTTON1 Window 0x1c200000 received key UP Keycode: 50 Event modifiers: SHIFT XKB last state: Change: POINTER_BUTTONS Modifiers: SHIFT Window 0x1c200000 received mouse button DOWN Button: 3 Event modifiers: (empty) Window 0x1c200000 received mouse button UP Button: 3 Event modifiers: BUTTON3 XKB last state: Change: POINTER_BUTTONS Buttons: BUTTON3 Window 0x1c200000 received key DOWN Keycode: 37 Event modifiers: (empty) Window 0x1c200000 received key DOWN Keycode: 50 Event modifiers: CONTROL XKB last state: Change: MODIFIER_STATE | MODIFIER_BASE | COMPAT_STATE | GRAB_MODS | COMPAT_GRAB_MODS | LOOKUP_MODS | COMPAT_LOOKUP_MODS Modifiers: CONTROL Window 0x1c200000 received key DOWN Keycode: 28 Event modifiers: SHIFT | CONTROL XKB last state: Change: MODIFIER_STATE | MODIFIER_BASE | COMPAT_STATE | GRAB_MODS | COMPAT_GRAB_MODS | LOOKUP_MODS | COMPAT_LOOKUP_MODS Modifiers: SHIFT | CONTROL Window 0x1c200000 received key UP Keycode: 28 Event modifiers: SHIFT | CONTROL XKB last state: Change: MODIFIER_STATE | MODIFIER_BASE | COMPAT_STATE | GRAB_MODS | COMPAT_GRAB_MODS | LOOKUP_MODS | COMPAT_LOOKUP_MODS Modifiers: SHIFT | CONTROL Window 0x1c200000 received key UP Keycode: 50 Event modifiers: SHIFT | CONTROL XKB last state: Change: MODIFIER_STATE | MODIFIER_BASE | COMPAT_STATE | GRAB_MODS | COMPAT_GRAB_MODS | LOOKUP_MODS | COMPAT_LOOKUP_MODS Modifiers: SHIFT | CONTROL Window 0x1c200000 received key UP Keycode: 37 Event modifiers: CONTROL XKB last state: Change: MODIFIER_STATE | MODIFIER_BASE | COMPAT_STATE | GRAB_MODS | COMPAT_GRAB_MODS | LOOKUP_MODS | COMPAT_LOOKUP_MODS Modifiers: CONTROL Window 0x1c200000 received key DOWN Keycode: 9 Event modifiers: (empty) `Escape` was pressed, exiting.. ```

If you prefer, to keep this PR more focused on the new method I can move this example to another PR

rtbo commented 1 year ago

You need to run cargo fmt and the test will pass.

I regret that response_type is not in the BaseEvent trait. If it would, is_from_send_event would have a single definition. But I think moving the definitions from a struct to a trait will be a breaking change and require version 2 of the library.

adding a dependency on libxkbcommon just for an example seems a bit too much

dev-dependencies serve this purpose (I already have dependency to gl only for one example).

rtbo commented 1 year ago

In fact it is possible to implement directly in the traits. response_type must be fetched from the FFI pointer:

diff --git a/src/base.rs b/src/base.rs
index 391cf1361d..c53a0db5a4 100644
--- a/src/base.rs
+++ b/src/base.rs
@@ -81,6 +81,14 @@ pub trait BaseEvent: Raw<xcb_generic_event_t> {

     /// The number associated to this event
     const NUMBER: u32;
+
+    /// Check whether this event is synthetic (sent by the [x::SendEvent](crate::x::SendEvent) request),
+    /// or sent by the X server.
+    fn is_from_send_event(&self) -> bool {
+        let ev = self.as_raw();
+        let response_type = unsafe { (*ev).response_type };
+        (response_type & 0x80) != 0
+    }
 }

 /// A trait for GE_GENERIC events

I would only implement for BaseEvent (and not GeEvent), because the SendEvent request only accepts BaseEvent objects so far.

bew commented 1 year ago

I'll see what I can do with libxkbcommon a bit later.

I would only implement for BaseEvent (and not GeEvent), because the SendEvent request only accepts BaseEvent objects so far.

Unfortunately that doesn't work directly because the code that generates the Debug impl for structs does not have a cg::Event in scope and so can't be aware of the type of the event (xge or not).. It feels a bit overkill to add a is_xge_event param to all functions up the stack just for this.

I tried to check the doc for XGE but didn't find the same info about the special bit in response_type, probably because SendEvent can't send an event like this as you mentioned. :point_right: Would it be ok to add an implementation anyway in GeEvent that simply returns false ?

rtbo commented 1 year ago

Unfortunately that doesn't work directly because the code that generates the Debug impl for structs does not have a cg::Event in scope and so can't be aware of the type of the event (xge or not).

This would be easily solved by adding is_event and is_ge_event fields to cg::TypeInfo::Struct. But it is actually not needed.

If you only apply the diff I have posted, there is no need to modify the code generation at all. is_from_send_event is implemented in the trait once and for all and the PR is done.

The downside is that it create some API inconsistency because response_type is implemented in each struct where it could be implemented in the trait as well. But changing that could break existing code in production, so I'll leave it until version 2 exists (if it ever does).