[Merge] lp:~banw/compiz/compiz.a11y-shotcuts into lp:compiz
Samuel thibault
samuel.thibault at ens-lyon.org
Tue Sep 19 13:17:13 UTC 2017
Yes, normal XI2 event are very probably affected by grabs, and so AIUI raw events are the only way.
I have commented along the patch. Overall the approach seems sound to me.
Diff comments:
>
> === modified file 'src/event.cpp'
> --- src/event.cpp 2016-05-17 02:52:07 +0000
> +++ src/event.cpp 2017-03-16 17:06:36 +0000
> @@ -977,6 +1025,81 @@
> XEvent nev;
> XPeekEvent (dpy, &nev);
>
> +/* FIXME: we have extra XI2 RawEvents that will likely break this detection,
> + * but how to do this right? */
> +#if 0
> +#if 1
> + //~ translateXI2Event (&nev);
> + if (nev.type == GenericEvent && nev.xcookie.extension == xi2.get())
> + {
> + XGetEventData (dpy, &nev.xcookie);
> +
> + XIDeviceEvent *xidevev = (XIDeviceEvent *) nev.xcookie.data;
> + XIRawEvent *xiRawEv = (XIRawEvent *) nev.xcookie.data;
> +
> + if ((xidevev->evtype == XI_KeyRelease ||
> + xidevev->evtype == XI_KeyPress) &&
> + xidevev->time == event->xkey.time &&
> + (unsigned int) xidevev->detail == event->xkey.keycode)
> + {
> + if (xidevev->evtype == XI_KeyRelease)
> + {
> + /* it's the same as the Core event, we can safely drop
> + * it and have a chance getting the next Core event */
> + /* FIXME: actually no it's not safe, it'll eat XI2
> + * events that might be the only triggers for an action */
I'm not sure to understand: can't we just, when using XI2, ignore KeyPress/Release events entirely?
AIUI, when using XI2, we'd get both the press/release events through XI2, and then through core events. Since we will have triggered bindings when receiving the XI2 events, we don't need to process the core events (otherwise the binding action would happen twice!), or is there something else I'm not thinking about?
> + XFreeEventData (dpy, &nev.xcookie);
> + XNextEvent (dpy, &nev);
> + XPeekEvent (dpy, &nev);
> + //~ XGetEventData (dpy, &nev.xcookie);
> + }
> + else
> + nextKeyPressIsRepeated_ = true;
> + }
> + else if ((xiRawEv->evtype == XI_RawKeyRelease ||
> + xiRawEv->evtype == XI_RawKeyPress) &&
> + xiRawEv->time == event->xkey.time &&
> + (unsigned int) xiRawEv->detail == event->xkey.keycode)
> + {
> + if (xiRawEv->evtype == XI_RawKeyRelease)
> + {
> + /* it's the same as the Core event, we can safely drop
> + * it and have a chance getting the next Core event */
> + /* FIXME: actually no it's not safe, it'll eat XI2
> + * events that might be the only triggers for an action */
> + XFreeEventData (dpy, &nev.xcookie);
> + XNextEvent (dpy, &nev);
> + XPeekEvent (dpy, &nev);
> + //~ XGetEventData (dpy, &nev.xcookie);
> + }
> + else
> + nextKeyPressIsRepeated_ = true;
> + }
> + }
> +#else
> + /* we can safely ignore XI events here
> + * -- well no, actually we can't, it'll eat XI2 events that might
> + * be the only triggers for an action */
> + bool hasEvents = true;
> + if (nev.type == GenericEvent && nev.xcookie.extension == xi2.get())
> + {
> + XFreeEventData (dpy, &nev.xcookie);
> + hasEvents = true;
> + do
> + {
> + XNextEvent (dpy, &nev);
> + if (! XEventsQueued (dpy, QueuedAfterReading))
> + hasEvents = false;
> + else
> + XPeekEvent (dpy, &nev);
> + }
> + while (hasEvents && nev.type == GenericEvent &&
> + nev.xcookie.extension == xi2.get());
> + }
> + if (hasEvents)
> +#endif
> +#endif
> +
> if (nev.type == KeyPress && nev.xkey.time == event->xkey.time &&
> nev.xkey.keycode == event->xkey.keycode)
> {
> @@ -1164,6 +1287,196 @@
> xdndWindow = None;
> }
> break;
> +
> + case GenericEvent:
> + if (event->xcookie.extension == xi2.get())
> + {
> + XIRawEvent *xiRawEv = (XIRawEvent *) event->xcookie.data;
> +
> + switch (xiRawEv->evtype)
> + {
> + case XI_RawButtonPress:
> + {
> + // Ignore Master Devices
> + if (xiRawEv->deviceid != xiRawEv->sourceid)
> + break;
I guess this could be factorized since it's the same for all events.
> +
> + XButtonEvent fakeEvent;
Do we really need to fill a fakeevent, since we'll fill o only anyway?
> + fakeEvent.type = ButtonPress;
> + fakeEvent.window = root; // FIXME:
Yes, that's sad, we have to determine the window that will receive the event. Or perhaps we can ask the X server for which window currently has focus, with XIGetFocus perhaps.
> + fakeEvent.state = pointerMods; // FIXME:
> + fakeEvent.x_root = pointerX; // FIXME:
> + fakeEvent.y_root = pointerY; // FIXME:
pointerMods, pointerX and pointerY should be fine. Yes it means we have to update their content, even if it looks ugly, it's not so bad.
> + fakeEvent.root = root; // FIXME
> + fakeEvent.button = xiRawEv->detail;
> + fakeEvent.time = xiRawEv->time;
> +
> + /* We need to determine if we clicked on a parent frame
> + * window, if so, pass the appropriate child window as
> + * "window" and the frame as "event_window"
> + */
> +
> + xid = fakeEvent.window;
> +
> + foreach (CompWindow *w, screen->windows ())
> + {
> + if (w->priv->frame == xid)
> + xid = w->id ();
> + }
> +
> + o[0].value ().set ((int) fakeEvent.window);
> + o[1].value ().set ((int) xid);
> + o[2].value ().set ((int) fakeEvent.state);
> + o[3].value ().set ((int) fakeEvent.x_root);
> + o[4].value ().set ((int) fakeEvent.y_root);
> + o[5].value ().set ((int) fakeEvent.root);
> +
> + o[6].setName ("button", CompOption::TypeInt);
> + o[7].setName ("time", CompOption::TypeInt);
> +
> + o[6].value ().set ((int) fakeEvent.button);
> + o[7].value ().set ((int) fakeEvent.time);
> +
> + eventManager.resetPossibleTap();
> + foreach (CompPlugin *p, CompPlugin::getPlugins ())
> + {
> + CompOption::Vector &options = p->vTable->getOptions ();
> + if (triggerButtonPressBindings (options, &fakeEvent, true, o))
> + return true;
> + }
> + break;
> + }
> + case XI_RawButtonRelease:
> + {
> + // Ignore Master Devices
> + if (xiRawEv->deviceid != xiRawEv->sourceid)
> + break;
> +
> + XButtonEvent fakeEvent;
> + fakeEvent.type = ButtonRelease;
> + fakeEvent.window = root; // FIXME:
> + fakeEvent.state = pointerMods; // FIXME:
> + fakeEvent.x_root = pointerX; // FIXME:
> + fakeEvent.y_root = pointerY; // FIXME:
> + fakeEvent.root = root; // FIXME:
> + fakeEvent.button = xiRawEv->detail;
> + fakeEvent.time = xiRawEv->time;
> +
> + o[0].value ().set ((int) fakeEvent.window);
> + o[1].value ().set ((int) fakeEvent.window);
> + o[2].value ().set ((int) fakeEvent.state);
> + o[3].value ().set ((int) fakeEvent.x_root);
> + o[4].value ().set ((int) fakeEvent.y_root);
> + o[5].value ().set ((int) fakeEvent.root);
> +
> + o[6].setName ("button", CompOption::TypeInt);
> + o[7].setName ("time", CompOption::TypeInt);
> +
> + o[6].value ().set ((int) fakeEvent.button);
> + o[7].value ().set ((int) fakeEvent.time);
> +
> + foreach (CompPlugin *p, CompPlugin::getPlugins ())
> + {
> + CompOption::Vector &options = p->vTable->getOptions ();
> + if (triggerButtonReleaseBindings (options, &fakeEvent, true, o))
> + return true;
> + }
> + break;
> + }
> + case XI_RawKeyPress:
> + {
> + // Ignore Master Devices
> + if (xiRawEv->deviceid != xiRawEv->sourceid)
> + break;
> +
> + XKeyEvent fakeEvent;
> + fakeEvent.type = KeyPress;
> + fakeEvent.window = root; // FIXME:
> + fakeEvent.state = pointerMods; // FIXME:
> + fakeEvent.x_root = pointerX; // FIXME:
> + fakeEvent.y_root = pointerY; // FIXME:
> + fakeEvent.root = root; // FIXME:
> + fakeEvent.keycode = xiRawEv->detail;
> + fakeEvent.time = xiRawEv->time;
> +
> + o[0].value ().set ((int) fakeEvent.window);
> + o[1].value ().set ((int) orphanData.activeWindow);
> + o[2].value ().set ((int) fakeEvent.state);
> + o[3].value ().set ((int) fakeEvent.x_root);
> + o[4].value ().set ((int) fakeEvent.y_root);
> + o[5].value ().set ((int) fakeEvent.root);
> +
> + o[6].setName ("keycode", CompOption::TypeInt);
> + o[7].setName ("time", CompOption::TypeInt);
> + o[8].setName ("is_repeated", CompOption::TypeBool);
> +
> + o[6].value ().set ((int) fakeEvent.keycode);
> + o[7].value ().set ((int) fakeEvent.time);
> +/*
> + o[8].value ().set (nextKeyPressIsRepeated_);
> + nextKeyPressIsRepeated_ = false;
> +*/
> + // FIXME: is that really valid for RawKeyPress?
> + o[8].value ().set ((xiRawEv->flags & XIKeyRepeat) != 0);
> +
> + eventManager.resetPossibleTap();
> + foreach (CompPlugin *p, CompPlugin::getPlugins ())
> + {
> + CompOption::Vector &options = p->vTable->getOptions ();
> + CompAction::Vector &actions = p->vTable->getActions ();
> + if (triggerKeyPressBindings (options, actions, &fakeEvent, true, o))
> + return true;
> + }
> +
> + break;
> + }
> + case XI_RawKeyRelease:
> + {
> + // Ignore Master Devices
> + if (xiRawEv->deviceid != xiRawEv->sourceid)
> + break;
> +
> + XKeyEvent fakeEvent;
> + fakeEvent.type = KeyRelease;
> + fakeEvent.window = root; // FIXME:
> + fakeEvent.state = pointerMods; // FIXME:
> + fakeEvent.x_root = pointerX; // FIXME:
> + fakeEvent.y_root = pointerY; // FIXME:
> + fakeEvent.root = root; // FIXME:
> + fakeEvent.keycode = xiRawEv->detail;
> + fakeEvent.time = xiRawEv->time;
> +
> + o[0].value ().set ((int) fakeEvent.window);
> + o[1].value ().set ((int) orphanData.activeWindow);
> + o[2].value ().set ((int) fakeEvent.state);
> + o[3].value ().set ((int) fakeEvent.x_root);
> + o[4].value ().set ((int) fakeEvent.y_root);
> + o[5].value ().set ((int) fakeEvent.root);
> +
> + o[6].setName ("keycode", CompOption::TypeInt);
> + o[7].setName ("time", CompOption::TypeInt);
> +
> + o[6].value ().set ((int) fakeEvent.keycode);
> + o[7].value ().set ((int) fakeEvent.time);
> +
> + bool handled = false;
> +
> + foreach (CompPlugin *p, CompPlugin::getPlugins ())
> + {
> + CompOption::Vector &options = p->vTable->getOptions ();
> + CompAction::Vector &actions = p->vTable->getActions ();
> + handled |= triggerKeyReleaseBindings (options, actions, &fakeEvent, true, o);
> + }
> +
> + if (handled)
> + return true;
> +
> + break;
> + }
> + }
> + }
> + break;
> +
> default:
> if (event->type == xkbEvent.get())
> {
--
https://code.launchpad.net/~banw/compiz/compiz.a11y-shotcuts/+merge/320091
Your team Compiz Maintainers is subscribed to branch lp:compiz.
More information about the Ubuntu-reviews
mailing list