-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
It seems there is no Into<u16> implementation for xproto::EventMask #746
Comments
More-or-less a duplicate of #543 (hence, I'll close this issue, but feel free to ask more questions here) Your "and have to use this instead" is more or less the best that we can do. The problem IMO comes from the XML description of the X11 protocol. x11rb/xcb-proto-1.14-1-g2b3559c/src/xproto.xml Lines 151 to 159 in 9020fbe
In the X11 protocol documentation, GrabPointer's Near the end of that document these sets of events are defined.
See #542 / #543 for some more details & previous reports. I doubt libxcb can fix this easily, but I'll give it a try anyway. |
Can you please show me where Into implementation is defined, because I can't find it. And why there should be 16-bit limit for EvenMask in methods like "grab_pointer"? Thank you for your help! |
Dunno why it does not show up in the docs (I guess because it is x11rb/x11rb-protocol/src/protocol/xproto.rs Lines 494 to 499 in 9020fbe
Also from https://doc.rust-lang.org/std/convert/trait.Into.html
That's just X11. Someone a long time ago decided that e.g. |
About From trait, yes, I know, but it's really weird it's not showing up in docs, only From... Anyway, thanks for your help! (And thanks for amazing library and detailed tutorial!). Have a nice day! |
I now opened https://gitlab.freedesktop.org/xorg/proto/xcbproto/-/merge_requests/35 with xcb-proto. Let's see if that is accepted and whether this actually helps much in this confusion... |
I want to know if I got right what you're proposing: you created new enums which specify what kind of events we want to grub and therefore we can avoid such cumbersome conversion and just use appropriate type of mask instead. Is that right? |
Yup, new enums which only contain the valid values for the two cases. And since the contained values all fit into |
Thank you! Hope it will be resolved! By the way, would it be a good idea to use TryInto here? Because, for example, in xcb or even in earlier versions of x11rb EvenMask::foo was just a number and we could use |
I am not quite sure what you are asking...? You can just use TryInto yourself: grab_pointer(
conn,
true,
screen.root,
u32::from(EventMask::POINTER_MOTION).try_into().expect("PointerMotion should fit into an u16 just fine"),
GrabMode::ASYNC,
GrabMode::ASYNC,
x11rb::NONE,
cursor,
Time::CURRENT_TIME,
)?
.reply()?; So I guess you mean something else...? |
I meant to put TryInto (instead of Into) for mask argument in trait bounds in definition of grab_pointer |
Uhm. Having a I haven't fully thought this through nor tried it out, but from a quick first impression: That feels like it would end up in "generic hell" and causing lots of problems down the road. |
Can't we just panic on error? I.e. in body of grab_pointer use something like |
Well, yes, of course that's possible. But that does not really feel Rust-y to me. I'd try to avoid more panics in x11rb. |
Oh, okay! I just thought it might avoid some confusion. I don't know why it's not Rusty to you, because it will panic only when you do something bad, and Rust will panic in this case anyway ... Maybe not in And I hope that xorg will accept your PR and I think it's the best idiomatically-wise solution. But I think it would be a good idea to add some notes into documentation! |
I just opened #747. This gets rid of the |
But what will happen if someone uses u32 EvenMask parameter? |
Something between "won't compile" and "they will have to add a call to |
But why it won't compile? If there is no restriction for the EventMask type ... |
Well, I don't mind giving people the tools to shoot themselves into the foot. I am just trying to make it harder. Previously, anything implementing |
You didn't get me. I meant, earlier you could put something beyond 32u size, but now you can. Isn't it bad? |
What do you mean with "beyond 32u size"? Larger than u32? Uhm, how? The only integer types implementing Or are you asking for a way to turn e.g. |
No, I mean, that, when you removed trait bound |
Ah, that part. Yeah. Let's forget about e.g. GrabButton. For other cases, this now does the right thing: Anything that implements For GrabButton, the change will only help once https://gitlab.freedesktop.org/xorg/proto/xcbproto/-/merge_requests/35 is accepted upstream and we updated our copy of xcb-proto. At that point, the Here's the diff if I manually update the xcb-proto version right now:diff --git a/x11rb-protocol/src/protocol/xproto.rs b/x11rb-protocol/src/protocol/xproto.rs
index bdb7e8b..d006629 100644
--- a/x11rb-protocol/src/protocol/xproto.rs
+++ b/x11rb-protocol/src/protocol/xproto.rs
@@ -558,6 +558,166 @@ impl core::fmt::Debug for EventMask {
}
bitmask_binop!(EventMask, u32);
+/// subset of event mask for pointer events in grabs.
+///
+/// # See
+///
+/// * `GrabPointer`: request
+/// * `GrabButton`: request
+/// * `ChangeActivePointerGrab`: request
+#[derive(Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+pub struct PointerEventMask(u16);
+impl PointerEventMask {
+ pub const NO_EVENT: Self = Self(0);
+ pub const BUTTON_PRESS: Self = Self(1 << 2);
+ pub const BUTTON_RELEASE: Self = Self(1 << 3);
+ pub const ENTER_WINDOW: Self = Self(1 << 4);
+ pub const LEAVE_WINDOW: Self = Self(1 << 5);
+ pub const POINTER_MOTION: Self = Self(1 << 6);
+ pub const POINTER_MOTION_HINT: Self = Self(1 << 7);
+ pub const BUTTON1_MOTION: Self = Self(1 << 8);
+ pub const BUTTON2_MOTION: Self = Self(1 << 9);
+ pub const BUTTON3_MOTION: Self = Self(1 << 10);
+ pub const BUTTON4_MOTION: Self = Self(1 << 11);
+ pub const BUTTON5_MOTION: Self = Self(1 << 12);
+ pub const BUTTON_MOTION: Self = Self(1 << 13);
+ pub const KEYMAP_STATE: Self = Self(1 << 14);
+}
+impl From<PointerEventMask> for u16 {
+ #[inline]
+ fn from(input: PointerEventMask) -> Self {
+ input.0
+ }
+}
+impl From<PointerEventMask> for Option<u16> {
+ #[inline]
+ fn from(input: PointerEventMask) -> Self {
+ Some(input.0)
+ }
+}
+impl From<PointerEventMask> for u32 {
+ #[inline]
+ fn from(input: PointerEventMask) -> Self {
+ u32::from(input.0)
+ }
+}
+impl From<PointerEventMask> for Option<u32> {
+ #[inline]
+ fn from(input: PointerEventMask) -> Self {
+ Some(u32::from(input.0))
+ }
+}
+impl From<u8> for PointerEventMask {
+ #[inline]
+ fn from(value: u8) -> Self {
+ Self(value.into())
+ }
+}
+impl From<u16> for PointerEventMask {
+ #[inline]
+ fn from(value: u16) -> Self {
+ Self(value)
+ }
+}
+impl core::fmt::Debug for PointerEventMask {
+ fn fmt(&self, fmt: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+ let variants = [
+ (Self::NO_EVENT.0.into(), "NO_EVENT", "NoEvent"),
+ (Self::BUTTON_PRESS.0.into(), "BUTTON_PRESS", "ButtonPress"),
+ (Self::BUTTON_RELEASE.0.into(), "BUTTON_RELEASE", "ButtonRelease"),
+ (Self::ENTER_WINDOW.0.into(), "ENTER_WINDOW", "EnterWindow"),
+ (Self::LEAVE_WINDOW.0.into(), "LEAVE_WINDOW", "LeaveWindow"),
+ (Self::POINTER_MOTION.0.into(), "POINTER_MOTION", "PointerMotion"),
+ (Self::POINTER_MOTION_HINT.0.into(), "POINTER_MOTION_HINT", "PointerMotionHint"),
+ (Self::BUTTON1_MOTION.0.into(), "BUTTON1_MOTION", "Button1Motion"),
+ (Self::BUTTON2_MOTION.0.into(), "BUTTON2_MOTION", "Button2Motion"),
+ (Self::BUTTON3_MOTION.0.into(), "BUTTON3_MOTION", "Button3Motion"),
+ (Self::BUTTON4_MOTION.0.into(), "BUTTON4_MOTION", "Button4Motion"),
+ (Self::BUTTON5_MOTION.0.into(), "BUTTON5_MOTION", "Button5Motion"),
+ (Self::BUTTON_MOTION.0.into(), "BUTTON_MOTION", "ButtonMotion"),
+ (Self::KEYMAP_STATE.0.into(), "KEYMAP_STATE", "KeymapState"),
+ ];
+ pretty_print_bitmask(fmt, self.0.into(), &variants)
+ }
+}
+bitmask_binop!(PointerEventMask, u16);
+
+/// subset of event mask for device events that is used in do_not_propagate_mask.
+///
+/// # See
+///
+/// * `CreateWindow`: request
+/// * `ChangeWindowAttributes`: request
+/// * `GetWindowAttributes`: request
+#[derive(Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+pub struct DeviceEventMask(u32);
+impl DeviceEventMask {
+ pub const NO_EVENT: Self = Self(0);
+ pub const KEY_PRESS: Self = Self(1 << 0);
+ pub const KEY_RELEASE: Self = Self(1 << 1);
+ pub const BUTTON_PRESS: Self = Self(1 << 2);
+ pub const BUTTON_RELEASE: Self = Self(1 << 3);
+ pub const POINTER_MOTION: Self = Self(1 << 6);
+ pub const BUTTON1_MOTION: Self = Self(1 << 8);
+ pub const BUTTON2_MOTION: Self = Self(1 << 9);
+ pub const BUTTON3_MOTION: Self = Self(1 << 10);
+ pub const BUTTON4_MOTION: Self = Self(1 << 11);
+ pub const BUTTON5_MOTION: Self = Self(1 << 12);
+ pub const BUTTON_MOTION: Self = Self(1 << 13);
+}
+impl From<DeviceEventMask> for u32 {
+ #[inline]
+ fn from(input: DeviceEventMask) -> Self {
+ input.0
+ }
+}
+impl From<DeviceEventMask> for Option<u32> {
+ #[inline]
+ fn from(input: DeviceEventMask) -> Self {
+ Some(input.0)
+ }
+}
+impl From<u8> for DeviceEventMask {
+ #[inline]
+ fn from(value: u8) -> Self {
+ Self(value.into())
+ }
+}
+impl From<u16> for DeviceEventMask {
+ #[inline]
+ fn from(value: u16) -> Self {
+ Self(value.into())
+ }
+}
+impl From<u32> for DeviceEventMask {
+ #[inline]
+ fn from(value: u32) -> Self {
+ Self(value)
+ }
+}
+impl core::fmt::Debug for DeviceEventMask {
+ fn fmt(&self, fmt: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+ let variants = [
+ (Self::NO_EVENT.0, "NO_EVENT", "NoEvent"),
+ (Self::KEY_PRESS.0, "KEY_PRESS", "KeyPress"),
+ (Self::KEY_RELEASE.0, "KEY_RELEASE", "KeyRelease"),
+ (Self::BUTTON_PRESS.0, "BUTTON_PRESS", "ButtonPress"),
+ (Self::BUTTON_RELEASE.0, "BUTTON_RELEASE", "ButtonRelease"),
+ (Self::POINTER_MOTION.0, "POINTER_MOTION", "PointerMotion"),
+ (Self::BUTTON1_MOTION.0, "BUTTON1_MOTION", "Button1Motion"),
+ (Self::BUTTON2_MOTION.0, "BUTTON2_MOTION", "Button2Motion"),
+ (Self::BUTTON3_MOTION.0, "BUTTON3_MOTION", "Button3Motion"),
+ (Self::BUTTON4_MOTION.0, "BUTTON4_MOTION", "Button4Motion"),
+ (Self::BUTTON5_MOTION.0, "BUTTON5_MOTION", "Button5Motion"),
+ (Self::BUTTON_MOTION.0, "BUTTON_MOTION", "ButtonMotion"),
+ ];
+ pretty_print_bitmask(fmt, self.0, &variants)
+ }
+}
+bitmask_binop!(DeviceEventMask, u32);
+
#[derive(Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct BackingStore(u32);
@@ -6779,7 +6939,7 @@ pub struct CreateWindowAux {
pub override_redirect: Option<Bool32>,
pub save_under: Option<Bool32>,
pub event_mask: Option<EventMask>,
- pub do_not_propogate_mask: Option<EventMask>,
+ pub do_not_propogate_mask: Option<DeviceEventMask>,
pub colormap: Option<Colormap>,
pub cursor: Option<Cursor>,
}
@@ -7102,7 +7262,7 @@ impl CreateWindowAux {
}
/// Set the `do_not_propogate_mask` field of this structure.
#[must_use]
- pub fn do_not_propogate_mask<I>(mut self, value: I) -> Self where I: Into<Option<EventMask>> {
+ pub fn do_not_propogate_mask<I>(mut self, value: I) -> Self where I: Into<Option<DeviceEventMask>> {
self.do_not_propogate_mask = value.into();
self
}
@@ -7332,7 +7492,7 @@ pub struct ChangeWindowAttributesAux {
pub override_redirect: Option<Bool32>,
pub save_under: Option<Bool32>,
pub event_mask: Option<EventMask>,
- pub do_not_propogate_mask: Option<EventMask>,
+ pub do_not_propogate_mask: Option<DeviceEventMask>,
pub colormap: Option<Colormap>,
pub cursor: Option<Cursor>,
}
@@ -7655,7 +7815,7 @@ impl ChangeWindowAttributesAux {
}
/// Set the `do_not_propogate_mask` field of this structure.
#[must_use]
- pub fn do_not_propogate_mask<I>(mut self, value: I) -> Self where I: Into<Option<EventMask>> {
+ pub fn do_not_propogate_mask<I>(mut self, value: I) -> Self where I: Into<Option<DeviceEventMask>> {
self.do_not_propogate_mask = value.into();
self
}
@@ -7935,7 +8095,7 @@ pub struct GetWindowAttributesReply {
pub colormap: Colormap,
pub all_event_masks: EventMask,
pub your_event_mask: EventMask,
- pub do_not_propagate_mask: EventMask,
+ pub do_not_propagate_mask: DeviceEventMask,
}
impl TryParse for GetWindowAttributesReply {
fn try_parse(initial_value: &[u8]) -> Result<(Self, &[u8]), ParseError> {
@@ -11667,7 +11827,7 @@ pub const GRAB_POINTER_REQUEST: u8 = 26;
pub struct GrabPointerRequest {
pub owner_events: bool,
pub grab_window: Window,
- pub event_mask: EventMask,
+ pub event_mask: PointerEventMask,
pub pointer_mode: GrabMode,
pub keyboard_mode: GrabMode,
pub confine_to: Window,
@@ -11680,7 +11840,7 @@ impl GrabPointerRequest {
let length_so_far = 0;
let owner_events_bytes = self.owner_events.serialize();
let grab_window_bytes = self.grab_window.serialize();
- let event_mask_bytes = (u32::from(self.event_mask) as u16).serialize();
+ let event_mask_bytes = u16::from(self.event_mask).serialize();
let pointer_mode_bytes = u8::from(self.pointer_mode).serialize();
let keyboard_mode_bytes = u8::from(self.keyboard_mode).serialize();
let confine_to_bytes = self.confine_to.serialize();
@@ -12044,7 +12204,7 @@ pub const GRAB_BUTTON_REQUEST: u8 = 28;
pub struct GrabButtonRequest {
pub owner_events: bool,
pub grab_window: Window,
- pub event_mask: EventMask,
+ pub event_mask: PointerEventMask,
pub pointer_mode: GrabMode,
pub keyboard_mode: GrabMode,
pub confine_to: Window,
@@ -12058,7 +12218,7 @@ impl GrabButtonRequest {
let length_so_far = 0;
let owner_events_bytes = self.owner_events.serialize();
let grab_window_bytes = self.grab_window.serialize();
- let event_mask_bytes = (u32::from(self.event_mask) as u16).serialize();
+ let event_mask_bytes = u16::from(self.event_mask).serialize();
let pointer_mode_bytes = u8::from(self.pointer_mode).serialize();
let keyboard_mode_bytes = u8::from(self.keyboard_mode).serialize();
let confine_to_bytes = self.confine_to.serialize();
@@ -12223,7 +12383,7 @@ pub const CHANGE_ACTIVE_POINTER_GRAB_REQUEST: u8 = 30;
pub struct ChangeActivePointerGrabRequest {
pub cursor: Cursor,
pub time: Timestamp,
- pub event_mask: EventMask,
+ pub event_mask: PointerEventMask,
}
impl ChangeActivePointerGrabRequest {
/// Serialize this request into bytes for the provided connection
@@ -12231,7 +12391,7 @@ impl ChangeActivePointerGrabRequest {
let length_so_far = 0;
let cursor_bytes = self.cursor.serialize();
let time_bytes = self.time.serialize();
- let event_mask_bytes = (u32::from(self.event_mask) as u16).serialize();
+ let event_mask_bytes = u16::from(self.event_mask).serialize();
let mut request0 = vec![
CHANGE_ACTIVE_POINTER_GRAB_REQUEST,
0,
diff --git a/x11rb/src/protocol/xproto.rs b/x11rb/src/protocol/xproto.rs
index 5cfc4bc..e46997b 100644
--- a/x11rb/src/protocol/xproto.rs
+++ b/x11rb/src/protocol/xproto.rs
@@ -1103,7 +1103,7 @@ where
/// }
/// }
/// ```
-pub fn grab_pointer<Conn, A, B, C>(conn: &Conn, owner_events: bool, grab_window: Window, event_mask: EventMask, pointer_mode: GrabMode, keyboard_mode: GrabMode, confine_to: A, cursor: B, time: C) -> Result<Cookie<'_, Conn, GrabPointerReply>, ConnectionError>
+pub fn grab_pointer<Conn, A, B, C>(conn: &Conn, owner_events: bool, grab_window: Window, event_mask: PointerEventMask, pointer_mode: GrabMode, keyboard_mode: GrabMode, confine_to: A, cursor: B, time: C) -> Result<Cookie<'_, Conn, GrabPointerReply>, ConnectionError>
where
Conn: RequestConnection + ?Sized,
A: Into<Window>,
@@ -1232,7 +1232,7 @@ where
/// * `Value` - TODO: reasons?
/// * `Cursor` - The specified `cursor` does not exist.
/// * `Window` - The specified `window` does not exist.
-pub fn grab_button<Conn, A, B>(conn: &Conn, owner_events: bool, grab_window: Window, event_mask: EventMask, pointer_mode: GrabMode, keyboard_mode: GrabMode, confine_to: A, cursor: B, button: ButtonIndex, modifiers: ModMask) -> Result<VoidCookie<'_, Conn>, ConnectionError>
+pub fn grab_button<Conn, A, B>(conn: &Conn, owner_events: bool, grab_window: Window, event_mask: PointerEventMask, pointer_mode: GrabMode, keyboard_mode: GrabMode, confine_to: A, cursor: B, button: ButtonIndex, modifiers: ModMask) -> Result<VoidCookie<'_, Conn>, ConnectionError>
where
Conn: RequestConnection + ?Sized,
A: Into<Window>,
@@ -1270,7 +1270,7 @@ where
conn.send_request_without_reply(&slices, fds)
}
-pub fn change_active_pointer_grab<Conn, A, B>(conn: &Conn, cursor: A, time: B, event_mask: EventMask) -> Result<VoidCookie<'_, Conn>, ConnectionError>
+pub fn change_active_pointer_grab<Conn, A, B>(conn: &Conn, cursor: A, time: B, event_mask: PointerEventMask) -> Result<VoidCookie<'_, Conn>, ConnectionError>
where
Conn: RequestConnection + ?Sized,
A: Into<Cursor>,
@@ -4115,7 +4115,7 @@ pub trait ConnectionExt: RequestConnection {
/// }
/// }
/// ```
- fn grab_pointer<A, B, C>(&self, owner_events: bool, grab_window: Window, event_mask: EventMask, pointer_mode: GrabMode, keyboard_mode: GrabMode, confine_to: A, cursor: B, time: C) -> Result<Cookie<'_, Self, GrabPointerReply>, ConnectionError>
+ fn grab_pointer<A, B, C>(&self, owner_events: bool, grab_window: Window, event_mask: PointerEventMask, pointer_mode: GrabMode, keyboard_mode: GrabMode, confine_to: A, cursor: B, time: C) -> Result<Cookie<'_, Self, GrabPointerReply>, ConnectionError>
where
A: Into<Window>,
B: Into<Cursor>,
@@ -4219,7 +4219,7 @@ pub trait ConnectionExt: RequestConnection {
/// * `Value` - TODO: reasons?
/// * `Cursor` - The specified `cursor` does not exist.
/// * `Window` - The specified `window` does not exist.
- fn grab_button<A, B>(&self, owner_events: bool, grab_window: Window, event_mask: EventMask, pointer_mode: GrabMode, keyboard_mode: GrabMode, confine_to: A, cursor: B, button: ButtonIndex, modifiers: ModMask) -> Result<VoidCookie<'_, Self>, ConnectionError>
+ fn grab_button<A, B>(&self, owner_events: bool, grab_window: Window, event_mask: PointerEventMask, pointer_mode: GrabMode, keyboard_mode: GrabMode, confine_to: A, cursor: B, button: ButtonIndex, modifiers: ModMask) -> Result<VoidCookie<'_, Self>, ConnectionError>
where
A: Into<Window>,
B: Into<Cursor>,
@@ -4230,7 +4230,7 @@ pub trait ConnectionExt: RequestConnection {
{
ungrab_button(self, button, grab_window, modifiers)
}
- fn change_active_pointer_grab<A, B>(&self, cursor: A, time: B, event_mask: EventMask) -> Result<VoidCookie<'_, Self>, ConnectionError>
+ fn change_active_pointer_grab<A, B>(&self, cursor: A, time: B, event_mask: PointerEventMask) -> Result<VoidCookie<'_, Self>, ConnectionError>
where
A: Into<Cursor>,
B: Into<Timestamp>, |
I want to use xproto::grab_pointer, which requires even_mask, which should implement Into. I don't know why, but for some reason I can't just use
grab_pointer(
conn,
true,
screen.root,
EventMask::POINTER_MOTION,
GrabMode::ASYNC,
GrabMode::ASYNC,
x11rb::NONE,
cursor,
Time::CURRENT_TIME,
)?
.reply()?;
And have to use this instead.
grab_pointer(
conn,
true,
screen.root,
<EventMask as Into>::into(EventMask::POINTER_MOTION) as u16,
GrabMode::ASYNC,
GrabMode::ASYNC,
x11rb::NONE,
cursor,
Time::CURRENT_TIME,
)?
.reply()?;
I can't find the implementation Into for EventMask either. I see just From and From. I am new to Rust, so maybe I am just doing something wrong. Thanks in advance for your help!
The text was updated successfully, but these errors were encountered: