[Xcb] xcb_event_loop renaming

[Jamey Sharp]

On Wed, Jan 09, 2008 at 09:26:29AM -0800, Barton C Massey wrote:
> Thanks also for the patch! Resubmit with xcb_event_loop()
> also renamed as Vincent suggested and we'll take it happily.
> (This latter change also means fixing Jamey's sample window
> manager and some stuff in util to reflect the renaming.)

As Bart says, thanks for this work! However, I couldn't accept the patch
as-is; I've committed a modified version.

On Wed, Jan 09, 2008 at 04:33:22PM +0100, Arnaud Fontaine wrote:
> I attached to this mail the patch which adds xcb_poll_for_event_loop()
> and also perform bitwise AND (~0x80) on the response type in order to
> get the event code (I read this in the XCB manual, Vincent tried to
> explain me what it means but I haven't fully understood yet).

The SendEvent bit is provided because there are cases where applications
need to know whether the event was generated internally by the X server,
or whether it was generated by another X client with the SendEvent
request. I thought ICCCM or EWMH actually required this somewhere, but I
could be wrong.

You do need to ignore the SendEvent bit in order to determine which
actual event you have, though.

> diff --git a/event/events.c b/event/events.c
> index 2900bd3..95fa755 100644
> --- a/event/events.c
> +++ b/event/events.c
> @@ -67,6 +67,18 @@ void xcb_event_loop(xcb_event_handlers_t *evenths)
>  	xcb_generic_event_t *event;
>  	while((event = xcb_wait_for_event(evenths->c)))
>  	{
> +		event->response_type &= ~0x80;
> +		handle_event(evenths, event);

Adding a non-blocking event loop, and renaming the blocking one, are
both good ideas, but you can't mask the response_type here.

First, xcb_(wait|poll)_for_event returns either an X event or an X
error, and you can only safely mask off the SendEvent bit for X events.
So this patch is incorrect.

Second, for X events, handle_event calls get_event_handler, which
already does this masking when finding the right handler:

	event &= 0x7f;

So the only potential benefit of masking off event->response_type is
that the callback doesn't see the SendEvent bit; but if you care, you
should either use different callback functions for different events, or
mask off the bit yourself. Don't prevent the application from using the
SendEvent bit if it needs it.

I've committed your patch without the two masking statements.

On Wed, Jan 09, 2008 at 09:22:14AM -0800, Barton C Massey wrote:
> I think all this callback-based stuff should just be ripped
> out, though, so I'm the wrong guy to ask :-).  This kind of
> problem is exactly why we made XCB thread-transparent.

I only implemented the callback interfaces (util/event and util/reply)
because people keep asking for them and I wanted to demonstrate that XCB
doesn't need new API to support them. I agree that they're a bad idea.

Jamey
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://lists.freedesktop.org/archives/xcb/attachments/20080128/cf6bebb7/attachment.pgp