event_mask argument to xproto's GrabButton is smaller than the corresponding bitmask

[psychon]

See #542:

xproto's GrabButton request has this field:

x11rb/xcb-proto-1.14-1-g2b3559c/src/xproto.xml

Line 2743 in a29b84d

<field type="CARD16" name="event_mask" mask="EventMask" />

This is a 16 bit field, but refers to an enum that has 24 bit values. Thus, the enum does not implement Into<u16> in x11rb and the API requires manual casts. It would be nice if we could do something about that.

Edit:
Looking at the protocol manual, it seems like all of the following are represented by EventMask in XCB, but are actually different in the protocol docs:

Type	Values
EVENT	{ KeyPress, KeyRelease, OwnerGrabButton, ButtonPress, ButtonRelease, EnterWindow, LeaveWindow, PointerMotion, PointerMotionHint, Button1Motion, Button2Motion, Button3Motion, Button4Motion, Button5Motion, ButtonMotion, Exposure, VisibilityChange, StructureNotify, ResizeRedirect, SubstructureNotify, SubstructureRedirect, FocusChange, PropertyChange, ColormapChange, KeymapState }
POINTEREVENT	{ ButtonPress, ButtonRelease, EnterWindow, LeaveWindow, PointerMotion, PointerMotionHint, Button1Motion, Button2Motion, Button3Motion, Button4Motion, Button5Motion, ButtonMotion, KeymapState }
DEVICEEVENT	{ KeyPress, KeyRelease, ButtonPress, ButtonRelease, PointerMotion, Button1Motion, Button2Motion, Button3Motion, Button4Motion, Button5Motion, ButtonMotion }

[samyak-jain]

Are there any plans to fix this? What will it take to fix this? I can try to help.

[psychon]

Well....

Are there any plans to fix this?

In general: Yes. But I do not know how to (easily) fix this.

What will it take to fix this?

I'm not sure. I guess "a good idea on how to fix this". A random idea would be to go the #369 route and deviate from upstream's XML (since I doubt that this could be fixed upstream in a backwards compatible way).

Or do "something special" in the code generator. This is basically the same idea as the previous point, just implemented in a different way.

Even ignoring the "how?" (which is what I thought about in the last two points), there is also the "what?": How should the generated code for this look like? What's the best fix that doesn't cause problems down the road. E.g. adding other event enums means that users might now expect a way to convert between these enums....

So, do sum it up: I'm not really sure

[samyak-jain]

So, I'm confused. Is the reason event mask 16 bit despite it having the ability to have more values because for GrabButton, there aren't more values required? There are many places in the xml where the event mask is used with 32 bits instead. Do you think it is possible to generate separate event masks based on what's making the request? My understanding is that GrabButton requires only a subset of EventMask so maybe the enum needed for GrabButton only needs to contain those values?

[psychon]

According to the protocol manual ( https://www.x.org/releases/X11R7.6/doc/xproto/x11protocol.html ), the one event mask that xcb-proto / xcb has really should be three separate ones:

Type	Values
EVENT	{ KeyPress, KeyRelease, OwnerGrabButton, ButtonPress, ButtonRelease, EnterWindow, LeaveWindow, PointerMotion, PointerMotionHint, Button1Motion, Button2Motion, Button3Motion, Button4Motion, Button5Motion, ButtonMotion, Exposure, VisibilityChange, StructureNotify, ResizeRedirect, SubstructureNotify, SubstructureRedirect, FocusChange, PropertyChange, ColormapChange, KeymapState }
POINTEREVENT	{ ButtonPress, ButtonRelease, EnterWindow, LeaveWindow, PointerMotion, PointerMotionHint, Button1Motion, Button2Motion, Button3Motion, Button4Motion, Button5Motion, ButtonMotion, KeymapState }
DEVICEEVENT	{ KeyPress, KeyRelease, ButtonPress, ButtonRelease, PointerMotion, Button1Motion, Button2Motion, Button3Motion, Button4Motion, Button5Motion, ButtonMotion }

"DEVICEEVENT" only appears in the do-not-propagate-mask for CreateWindow / ChangeWindow. Its largest value seems to be <bit>13</bit>.

Searching for "pointerevents", it seems like this enum is for grab-related things (e.g. GrabPointer). Its largest values is <bit>14</bit>.

So, both of these would fit into a 16 bit number.

Thus, yeah, this really should be three different enums, I guess. libxcb just didn't need this thanks to C's implicit integer conversion.

[samyak-jain]

Do you know where these enums are generated in the code? Do you think we can make a special case for EventMask and just hard code 3 different enums? Or is there a better way to do this? I don't know if there's a more general solution to deal with other places such an issue may occur.

[psychon]

Hopefully this is the only place where this occurs.

I guess the easiest way to fix this is to do what #369 suggests and just patch our XML.

Anyway, the code for an <enum> is generated by this function:

x11rb/generator/src/generator/namespace/mod.rs

Line 645 in 2fd40af

fn generate_enum_def(&self, enum_def: &xcbdefs::EnumDef, out: &mut Output) {

The Rust struct that represents the <enum> is generated by this code:

x11rb/generator/src/generator/namespace/mod.rs

Lines 673 to 711 in 2fd40af

	outln!(out, "#[derive(Clone, Copy, PartialEq, Eq)]");
	outln!(out, "pub struct {}({});", rust_name, raw_type);
	outln!(out, "impl {} {{", rust_name);
	for enum_item in enum_def.items.iter() {
	let rust_item_name = ename_to_rust(&enum_item.name);
	if global_enum_size != 1 {
	match enum_item.value {
	xcbdefs::EnumValue::Value(value) => {
	outln!(
	out.indent(),
	"pub const {}: Self = Self({});",
	rust_item_name,
	expr_to_str::format_literal_integer(value),
	);
	}
	xcbdefs::EnumValue::Bit(bit) => {
	outln!(
	out.indent(),
	"pub const {}: Self = Self(1 << {});",
	rust_item_name,
	bit,
	);
	}
	}
	} else {
	let value = match enum_item.value {
	xcbdefs::EnumValue::Value(value) => value,
	xcbdefs::EnumValue::Bit(bit) => 1 << bit,
	};
	outln!(
	out.indent(),
	"pub const {}: Self = Self({});",
	rust_item_name,
	if value == 0 { "false" } else { "true" },
	);
	}
	}
	outln!(out, "}}");

The rest of the function seems to emit From implementations and a Debug implementation.

[psychon]

I opened https://gitlab.freedesktop.org/xorg/proto/xcbproto/-/issues/22 and https://gitlab.freedesktop.org/xorg/proto/xcbproto/-/merge_requests/35 for this