[Merge] lp:~samuel-thibault/compiz/shortcuts into lp:compiz
Marco Trevisan (Treviño)
mail at 3v1n0.net
Wed Nov 15 23:01:38 UTC 2017
Thanks for your work, it's really appreciated and something I really had in mind since long time...
However, a part few cleanups (and a bit more of C++11 features usage), we've to think as this will deal with Unity, since there we're using XI2 already for some input monitor operations (mostly for edge events and for monitoring the mouse position when menus are opened, plus few other things), and the fact is that we disable such events when we don't need them.
Now, in theory we could move all that to compiz, introducing new actions types, but the problem now is that we can't really deal with all this work now that we're focused in GNOME
Another possibility is just to make unity to save the state on load with XIGetSelectedEvents and resume that state when we disable it, but in general I'd prefer the events not to be selected unless there's anything else that need them.
Another possibility is just make unity to keep them always enabled, and never toggle them.
In any case, it would be cool if you could test it with your changes, and in case propose a fix for it too. As without that change we can't land compiz in ubuntu (and thus upstream) easily as it could be prone to regressions.
Diff comments:
>
> === modified file 'src/screen.cpp'
> --- src/screen.cpp 2017-07-04 12:14:30 +0000
> +++ src/screen.cpp 2017-11-14 19:27:00 +0000
> @@ -831,6 +892,111 @@
>
> while (getNextEvent (event))
> {
> + if (xi2.isEnabled ()) {
> + /* Using XI2 events, ignore any duplicate core events */
> + switch (event.type) {
> + case KeyPress:
> + case KeyRelease:
> + XFlush (dpy);
> + continue;
> + }
> + }
> +
> + if (event.type == GenericEvent
> + && event.xcookie.extension == xi2.get ()) {
> + /* Convert XI2 events to core events */
> +
> + XIDeviceEvent *xiDevEv = (XIDeviceEvent *) event.xcookie.data;
> +
> + if (xiDevEv->deviceid != xiDevEv->sourceid) {
> + // Ignore Master Devices
> + XFreeEventData (dpy, &event.xcookie);
> + continue;
> + }
> +
> + switch (event.xcookie.evtype) {
> + case XI_ButtonPress:
> + case XI_ButtonRelease:
> + {
> + XButtonEvent fakeEvent;
> + fakeEvent.type = event.xcookie.evtype == XI_ButtonPress ?
> + ButtonPress : XI_ButtonRelease;
> + fakeEvent.serial = event.xcookie.serial;
> + fakeEvent.send_event = event.xcookie.send_event;
> + fakeEvent.display = event.xcookie.display;
> + fakeEvent.window = xiDevEv->event;
> + fakeEvent.root = xiDevEv->root;
> + fakeEvent.subwindow = xiDevEv->child;
> + fakeEvent.time = xiDevEv->time;
> + fakeEvent.x = xiDevEv->event_x;
> + fakeEvent.y = xiDevEv->event_y;
> + fakeEvent.x_root = xiDevEv->root_x;
> + fakeEvent.y_root = xiDevEv->root_y;
> + fakeEvent.state = translateXI2State(xiDevEv);
> + fakeEvent.button = xiDevEv->detail;
> + fakeEvent.same_screen = True;
> + XFreeEventData (dpy, &event.xcookie);
> + event.xbutton = fakeEvent;
> + break;
> + }
> +
> + case XI_KeyPress:
> + case XI_KeyRelease:
> + {
> + XKeyEvent fakeEvent;
> + fakeEvent.type = event.xcookie.evtype == XI_KeyPress ?
> + KeyPress : XI_KeyRelease;
> + fakeEvent.serial = event.xcookie.serial;
> + fakeEvent.send_event = event.xcookie.send_event;
> + fakeEvent.display = event.xcookie.display;
> + fakeEvent.window = xiDevEv->event;
> + fakeEvent.root = xiDevEv->root;
> + fakeEvent.subwindow = xiDevEv->child;
> + fakeEvent.time = xiDevEv->time;
> + fakeEvent.x = xiDevEv->event_x;
> + fakeEvent.y = xiDevEv->event_y;
> + fakeEvent.x_root = xiDevEv->root_x;
> + fakeEvent.y_root = xiDevEv->root_y;
> + fakeEvent.state = translateXI2State(xiDevEv);
> + fakeEvent.keycode = xiDevEv->detail;
> + fakeEvent.same_screen = True;
> + XFreeEventData (dpy, &event.xcookie);
> + event.xkey = fakeEvent;
> + break;
> + }
> +
> + case XI_Motion:
> + {
> + /* TODO: mousepoll is probably useless with XI2. */
> + XMotionEvent fakeEvent;
> + fakeEvent.type = MotionNotify;
> + fakeEvent.serial = event.xcookie.serial;
> + fakeEvent.send_event = event.xcookie.send_event;
> + fakeEvent.display = event.xcookie.display;
> + fakeEvent.window = xiDevEv->event;
> + fakeEvent.root = xiDevEv->root;
> + fakeEvent.subwindow = xiDevEv->child;
> + fakeEvent.time = xiDevEv->time;
> + fakeEvent.x = xiDevEv->event_x;
> + fakeEvent.y = xiDevEv->event_y;
> + fakeEvent.x_root = xiDevEv->root_x;
> + fakeEvent.y_root = xiDevEv->root_y;
> + fakeEvent.state = translateXI2State(xiDevEv);
> + fakeEvent.is_hint = NotifyNormal;
> + fakeEvent.same_screen = True;
> + XFreeEventData (dpy, &event.xcookie);
> + event.xmotion = fakeEvent;
> + break;
> + }
It would have been nice to use some C++ templates to generate this for different types as we do in unity's unity-shared/InputMonitor.cpp as it's less prone to errors, but this is indeed ok.
> +
> + default:
> + /* Ignore other XI2 events */
> + XFreeEventData (dpy, &event.xcookie);
> + XFlush (dpy);
> + continue;
> + }
> + }
> +
> switch (event.type) {
> case ButtonPress:
> case ButtonRelease:
> @@ -5189,6 +5391,24 @@
> SubstructureNotifyMask);
> }
>
> + if (xi2.isEnabled ())
> + {
> + XIEventMask eventmask;
> + unsigned char mask[XIMaskLen (XI_LASTEVENT)] = { 0 };
> +
> + eventmask.deviceid = XIAllDevices;
> + eventmask.mask_len = sizeof (mask);
> + eventmask.mask = mask;
> +
> + XISetMask (mask, XI_KeyPress);
> + XISetMask (mask, XI_KeyRelease);
> + XISetMask (mask, XI_ButtonPress);
> + XISetMask (mask, XI_ButtonRelease);
> + XISetMask (mask, XI_Motion);
> +
> + XISelectEvents (dpy, root_tmp, &eventmask, 1);
As optimization, this could be enabled only if there's any action that has the ignoreGrabs flag set to true, no?
> + }
> +
> for (int i = 0; i < SCREEN_EDGE_NUM; i++)
> {
> screenEdge[i].id = None;
>
> === modified file 'src/window.cpp'
> --- src/window.cpp 2017-04-20 07:41:24 +0000
> +++ src/window.cpp 2017-11-14 19:27:00 +0000
> @@ -6946,6 +6947,17 @@
>
> /* Do not get any events from here on */
> XSelectInput (dpy, screen->root (), NoEventMask);
> + if (screen->xi2OpCode () != -1)
> + {
> + XIEventMask eventmask;
> + unsigned char mask[1] = { 0 };
> +
> + eventmask.deviceid = XIAllDevices;
> + eventmask.mask_len = sizeof (mask);
> + eventmask.mask = mask;
> +
> + XISelectEvents (dpy, screen->root (), &eventmask, 1);
> + }
Move this to privateServer I guess...
>
> /* If we have some frame extents, we should apply them here and
> * set lastFrameExtents */
> @@ -7043,13 +7063,28 @@
> PropertyChangeMask |
> LeaveWindowMask |
> EnterWindowMask |
> - KeyPressMask |
> - KeyReleaseMask |
> - ButtonPressMask |
> - ButtonReleaseMask |
> + keymask |
> FocusChangeMask |
> ExposureMask);
>
> + if (screen->xi2OpCode () != -1)
> + {
> + XIEventMask eventmask;
> + unsigned char mask[XIMaskLen (XI_LASTEVENT)] = { 0 };
> +
> + eventmask.deviceid = XIAllDevices;
> + eventmask.mask_len = sizeof (mask);
> + eventmask.mask = mask;
> +
> + XISetMask (mask, XI_KeyPress);
> + XISetMask (mask, XI_KeyRelease);
> + XISetMask (mask, XI_ButtonPress);
> + XISetMask (mask, XI_ButtonRelease);
> + XISetMask (mask, XI_Motion);
> +
> + XISelectEvents (dpy, screen->root (), &eventmask, 1);
Same of above... Avoid duplication here, and move all these functions in server.
> + }
> +
> XUngrabServer (dpy);
> XSync (dpy, false);
>
--
https://code.launchpad.net/~samuel-thibault/compiz/shortcuts/+merge/333330
Your team Compiz Maintainers is requested to review the proposed merge of lp:~samuel-thibault/compiz/shortcuts into lp:compiz.
More information about the Ubuntu-reviews
mailing list