Skip to content
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.

Sign up for GitHub

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

Drop repeat key events #98

Closed
wants to merge 21 commits into from
Closed

Drop repeat key events #98

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
86eb904
WIP: use XInput extension
iacore Feb 23, 2022
60ee007
Process xinput raw events too
iacore Feb 23, 2022
dc60cd4
WIP check for XInput events
iacore Feb 24, 2022
6bddd0e
Merge branch 'xinput' into drop-repeat
iacore Feb 24, 2022
0611392
Use XInput Key events instead of X basic events
iacore Feb 24, 2022
48323cd
Add libxi in Makefile
iacore Feb 24, 2022
cb824aa
Free X resource as well
iacore Feb 24, 2022
1bf52ea
Remove duplicate #include
iacore Feb 25, 2022
70da7d1
Update .gitignore
iacore Feb 25, 2022
5219d2a
Add XInput/libXi as build dependencies for debian & rpm
iacore Feb 25, 2022
576fa94
Don't send evnts to unfocused window
iacore Mar 5, 2022
a4a1488
Don't ignore grab events
iacore Mar 5, 2022
035be7f
Use base Focus events
iacore Mar 5, 2022
ee70785
Pass Focus type
iacore Mar 5, 2022
954fb64
Merge branch 'master' into drop-repeat
iacore Jun 17, 2022
8ec6f01
Use base Focus events
iacore Mar 5, 2022
2f91894
Merge branch 'drop-repeat' of github.com:locriacyber/qubes-gui-daemon…
iacore Mar 5, 2022
cabf231
Use Focus events from XInput, dropping less keytrokes
iacore Mar 5, 2022
110a6a9
Handle clipboard hotkeys properly
iacore Mar 8, 2022
403a952
Fix double keypresses
iacore Mar 13, 2022
8fd4e7f
Add more error handling
iacore Jun 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions debian/control
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Build-Depends:
libxrandr-dev,
libxcb1-dev,
libx11-xcb-dev,
libxi-dev,
libconfig-dev,
libpng-dev,
libnotify-dev,
Expand Down
2 changes: 2 additions & 0 deletions gui-daemon/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
qubes_guid
qubes-guid
qubes-guid.1
2 changes: 1 addition & 1 deletion gui-daemon/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
MAKEFLAGS := -rR
VCHAN_PKG = $(if $(BACKEND_VMM),vchan-$(BACKEND_VMM),vchan)
CC=gcc
pkgs := x11 xext x11-xcb xcb glib-2.0 $(VCHAN_PKG) libpng libnotify libconfig
pkgs := x11 xext x11-xcb xcb glib-2.0 xi $(VCHAN_PKG) libpng libnotify libconfig
objs := xside.o png.o trayicon.o ../gui-common/double-buffer.o ../gui-common/txrx-vchan.o \
../gui-common/error.o list.o
extra_cflags := -I../include/ -g -O2 -Wall -Wextra -Werror -pie -fPIC \
Expand Down
250 changes: 145 additions & 105 deletions gui-daemon/xside.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@
#include <X11/Xutil.h>
#include <X11/extensions/XShm.h>
#include <X11/extensions/shmproto.h>
#include <X11/extensions/XI2.h>
#include <X11/extensions/XInput2.h>
#include <X11/Xatom.h>
#include <X11/cursorfont.h>
#include <X11/Xlib-xcb.h>
Expand Down Expand Up @@ -272,6 +274,14 @@ int x11_error_handler(Display * dpy, XErrorEvent * ev)
* handled in handle_mfndump/handle_window_dump */
return 0;
}

char error_msg[1024];
XGetErrorText(ev->display, ev->error_code, error_msg, sizeof(error_msg));
int now = (int) time(NULL); // truncate
fprintf(stderr, "[%d] Encountered X Error:\n", now);
fprintf(stderr, error_msg);


#ifdef MAKE_X11_ERRORS_FATAL
/* The exit(1) below will call release_all_mapped_mfns (registerd with
* atexit(3)), which would try to release window images with XShmDetach. We
Expand Down Expand Up @@ -374,10 +384,33 @@ static Window mkwindow(Ghandles * g, struct windowdata *vm_window)
(const unsigned char *)&g->time_win,
1);
(void) XSelectInput(g->display, child_win,
ExposureMask | KeyPressMask | KeyReleaseMask |
ExposureMask |
ButtonPressMask | ButtonReleaseMask |
PointerMotionMask | EnterWindowMask | LeaveWindowMask |
FocusChangeMask | StructureNotifyMask | PropertyChangeMask);
StructureNotifyMask | PropertyChangeMask);

// select xinput events
XIEventMask xi_mask;
xi_mask.deviceid = XIAllMasterDevices; // https://stackoverflow.com/questions/44095001/getting-double-rawkeypress-events-using-xinput2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
xi_mask.deviceid = XIAllMasterDevices; // https://stackoverflow.com/questions/44095001/getting-double-rawkeypress-events-using-xinput2
// https://stackoverflow.com/questions/44095001/getting-double-rawkeypress-events-using-xinput2
xi_mask.deviceid = XIAllMasterDevices;

xi_mask.mask_len = XIMaskLen(XI_LASTEVENT);
if (!(xi_mask.mask = calloc(xi_mask.mask_len, sizeof(char)))) {
fputs("Out of memory!\n", stderr);
exit(1);
}
XISetMask(xi_mask.mask, XI_KeyPress);
XISetMask(xi_mask.mask, XI_KeyRelease);
XISetMask(xi_mask.mask, XI_FocusIn);
XISetMask(xi_mask.mask, XI_FocusOut);

int err = XISelectEvents(g->display, child_win, &xi_mask, 1);
if (err) {
fprintf(stderr, "Failed to subscribe to XI events. ErrCode: %d\n", err);
exit(1);
}
free(xi_mask.mask);
XSync(g->display, False);


XSetWMProtocols(g->display, child_win, &g->wmDeleteMessage, 1);
if (g->icon_data) {
XChangeProperty(g->display, child_win, g->net_wm_icon, XA_CARDINAL, 32,
Expand Down Expand Up @@ -647,6 +680,10 @@ static void mkghandles(Ghandles * g)
if (!XQueryExtension(g->display, "MIT-SHM",
&g->shm_major_opcode, &ev_base, &err_base))
fprintf(stderr, "MIT-SHM X extension missing!\n");
if (!XQueryExtension(g->display, "XInputExtension", &g->xi_opcode, &ev_base, &err_base)) {
fprintf(stderr, "X Input extension not available. Key press events not available. Upgrade your X11 server now.\n");
iacore marked this conversation as resolved.
Show resolved Hide resolved
exit(1);
}
/* get the work area */
XSelectInput(g->display, g->root_win, PropertyChangeMask);
update_work_area(g);
Expand Down Expand Up @@ -1240,17 +1277,17 @@ static void handle_cursor(Ghandles *g, struct windowdata *vm_window)
/* check and handle guid-special keys
* currently only for inter-vm clipboard copy
*/
static int is_special_keypress(Ghandles * g, const XKeyEvent * ev, XID remote_winid)
static int is_special_keypress(Ghandles * g, const XIDeviceEvent * ev, XID remote_winid)
{
struct msg_hdr hdr;
char *data;
int len;
Time clipboard_file_xevent_time;

/* copy */
if (((int)ev->state & SPECIAL_KEYS_MASK) == g->copy_seq_mask
&& ev->keycode == XKeysymToKeycode(g->display, g->copy_seq_key)) {
if (ev->type != KeyPress)
if (((int)ev->mods.effective & SPECIAL_KEYS_MASK) == g->copy_seq_mask
&& ev->detail == XKeysymToKeycode(g->display, g->copy_seq_key)) {
if (ev->evtype != KeyPress)
return 1;
g->clipboard_xevent_time = ev->time;
if (g->qrexec_clipboard) {
Expand All @@ -1270,9 +1307,9 @@ static int is_special_keypress(Ghandles * g, const XKeyEvent * ev, XID remote_wi
}

/* paste */
if (((int)ev->state & SPECIAL_KEYS_MASK) == g->paste_seq_mask
&& ev->keycode == XKeysymToKeycode(g->display, g->paste_seq_key)) {
if (ev->type != KeyPress)
if (((int)ev->mods.effective & SPECIAL_KEYS_MASK) == g->paste_seq_mask
&& ev->detail == XKeysymToKeycode(g->display, g->paste_seq_key)) {
if (ev->evtype != KeyPress)
return 1;
inter_appviewer_lock(g, 1);
clipboard_file_xevent_time = get_clipboard_file_xevent_timestamp();
Expand Down Expand Up @@ -1329,22 +1366,25 @@ static void update_wm_user_time(Ghandles *const g, const Window window,
1);
}

/* handle local Xserver event: XKeyEvent
/* handle local XInput event
* send it to relevant window in VM
*/
static void process_xevent_keypress(Ghandles * g, const XKeyEvent * ev)
static void process_xievent_keypress(Ghandles * g, const XIDeviceEvent * ev)
{
struct msg_hdr hdr;
struct msg_keypress k;
CHECK_NONMANAGED_WINDOW(g, ev->window);
update_wm_user_time(g, ev->window, ev->time);
CHECK_NONMANAGED_WINDOW(g, ev->event);
// yes, ev->event is the window number
update_wm_user_time(g, ev->event, ev->time);
iacore marked this conversation as resolved.
Show resolved Hide resolved
if (ev->flags & XIKeyRepeat)
return; // don't send key repeat events
if (is_special_keypress(g, ev, vm_window->remote_winid))
return;
k.type = ev->type;
k.x = ev->x;
k.y = ev->y;
k.state = ev->state;
k.keycode = ev->keycode;
k.type = ev->evtype; // ev->type is always Generic Event
k.x = ev->event_x;
k.y = ev->event_y;
k.state = ev->mods.effective;
k.keycode = ev->detail;
hdr.type = MSG_KEYPRESS;
hdr.window = vm_window->remote_winid;
write_message(g->vchan, hdr, k);
Expand Down Expand Up @@ -1382,7 +1422,6 @@ static void process_xevent_button(Ghandles * g, const XButtonEvent * ev)
update_wm_user_time(g, ev->window, ev->time);

k.type = ev->type;

k.x = ev->x;
k.y = ev->y;
k.state = ev->state;
Expand Down Expand Up @@ -1870,6 +1909,20 @@ static void handle_configure_from_vm(Ghandles * g, struct windowdata *vm_window)
}
}

static void send_keymap_notify(Ghandles * g)
{
struct msg_hdr hdr;
char keys[32];
int err = XQueryKeymap(g->display, keys);
if (err) {
fprintf(stderr, "XQueryKeymap failed: %d.\n", err);
return; // non fatal
}
hdr.type = MSG_KEYMAP_NOTIFY;
hdr.window = 0;
write_message(g->vchan, hdr, keys);
}

/* handle local Xserver event: EnterNotify, LeaveNotify
* send it to VM, but alwo we use it to fix docked
* window position */
Expand All @@ -1880,11 +1933,7 @@ static void process_xevent_crossing(Ghandles * g, const XCrossingEvent * ev)
CHECK_NONMANAGED_WINDOW(g, ev->window);

if (ev->type == EnterNotify) {
char keys[32];
XQueryKeymap(g->display, keys);
hdr.type = MSG_KEYMAP_NOTIFY;
hdr.window = 0;
write_message(g->vchan, hdr, keys);
send_keymap_notify(g);
}
/* move tray to correct position in VM */
if (vm_window->is_docked &&
Expand Down Expand Up @@ -1925,35 +1974,20 @@ static void process_xevent_motion(Ghandles * g, const XMotionEvent * ev)

/* handle local Xserver event: FocusIn, FocusOut
* send to relevant window in VM */
static void process_xevent_focus(Ghandles * g, const XFocusChangeEvent * ev)
static void process_xievent_focus(Ghandles * g, const XILeaveEvent * ev)
{
struct msg_hdr hdr;
struct msg_focus k;
CHECK_NONMANAGED_WINDOW(g, ev->window);
CHECK_NONMANAGED_WINDOW(g, ev->event);
update_wm_user_time(g, ev->event, ev->time);

/* Ignore everything other than normal, non-temporary focus change. In
* practice it ignores NotifyGrab and NotifyUngrab. VM does not have any
* way to grab focus in dom0, so it shouldn't care about those events. Grab
* is used by window managers during task switching (either classic task
* switcher, or KDE "present windows" feature).
*/
if (ev->mode != NotifyNormal && ev->mode != NotifyWhileGrabbed)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out this is the reason for extraneous keystrokes when cycling windows.
The window manager use NotifyGrab to signal temponany focus out.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept having problems with this when writing the Wayland compositor. I had no idea what the cause was, though. I wound up having to try different approaches until I found one that worked. Hopefully this PR will fix the actual problem.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't know how to send correct focus events to windows. I tried:

  • XSendEvent
  • XGrabKeyboard
    I don't know about Wayland though, I only tested with X server in VM.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know about Wayland though, I only tested with X server in VM.

Don’t worry about that. Something that works properly under X is hopefully going to be easier to handle with Wayland as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#104

return;

if (ev->type == FocusIn) {
char keys[32];
XQueryKeymap(g->display, keys);
hdr.type = MSG_KEYMAP_NOTIFY;
hdr.window = 0;
write_message(g->vchan, hdr, keys);
if (ev->type == XI_FocusIn) {
send_keymap_notify(g);
}
hdr.type = MSG_FOCUS;
hdr.window = vm_window->remote_winid;
k.type = ev->type;
/* override NotifyWhileGrabbed with NotifyNormal b/c VM shouldn't care
* about window manager details during focus switching
*/
k.mode = NotifyNormal;
k.type = ev->evtype;
k.mode = ev->mode;
k.detail = ev->detail;
write_message(g->vchan, hdr, k);
}
Expand Down Expand Up @@ -2303,11 +2337,7 @@ static void process_xevent_xembed(Ghandles * g, const XClientMessageEvent * ev)
} else if (ev->data.l[1] == XEMBED_FOCUS_IN) {
struct msg_hdr hdr;
struct msg_focus k;
char keys[32];
XQueryKeymap(g->display, keys);
hdr.type = MSG_KEYMAP_NOTIFY;
hdr.window = 0;
write_message(g->vchan, hdr, keys);
send_keymap_notify(g);
hdr.type = MSG_FOCUS;
hdr.window = vm_window->remote_winid;
k.type = FocusIn;
Expand All @@ -2322,62 +2352,72 @@ static void process_xevent_xembed(Ghandles * g, const XClientMessageEvent * ev)
static void process_xevent(Ghandles * g)
{
XEvent event_buffer;
XGenericEventCookie *cookie = &event_buffer.xcookie;
XNextEvent(g->display, &event_buffer);
switch (event_buffer.type) {
case KeyPress:
case KeyRelease:
process_xevent_keypress(g, (XKeyEvent *) & event_buffer);
break;
case ReparentNotify:
process_xevent_reparent(g, (XReparentEvent *) &event_buffer);
break;
case ConfigureNotify:
process_xevent_configure(g, (XConfigureEvent *) &
event_buffer);
break;
case ButtonPress:
case ButtonRelease:
process_xevent_button(g, (XButtonEvent *) & event_buffer);
break;
case MotionNotify:
process_xevent_motion(g, (XMotionEvent *) & event_buffer);
break;
case EnterNotify:
case LeaveNotify:
process_xevent_crossing(g,
(XCrossingEvent *) & event_buffer);
break;
case FocusIn:
case FocusOut:
process_xevent_focus(g,
(XFocusChangeEvent *) & event_buffer);
break;
case Expose:
process_xevent_expose(g, (XExposeEvent *) & event_buffer);
break;
case MapNotify:
process_xevent_mapnotify(g, (XMapEvent *) & event_buffer);
break;
case PropertyNotify:
process_xevent_propertynotify(g, (XPropertyEvent *) & event_buffer);
break;
case ClientMessage:
// fprintf(stderr, "xclient, atom=%s\n",
// XGetAtomName(g->display,
// event_buffer.xclient.message_type));
if (event_buffer.xclient.message_type == g->xembed_message) {
process_xevent_xembed(g, (XClientMessageEvent *) &
event_buffer);
} else if ((Atom)event_buffer.xclient.data.l[0] ==
g->wmDeleteMessage) {
if (g->log_level > 0)
fprintf(stderr, "close for 0x%x\n",
(int) event_buffer.xclient.window);
process_xevent_close(g,
event_buffer.xclient.window);
if (XGetEventData(g->display, cookie) &&
cookie->type == GenericEvent &&
cookie->extension == g->xi_opcode) {
XIEvent* xi_event = cookie->data; // from test_xi2.c in xinput cli utility

switch (xi_event->evtype) {
// ideally raw input events are better, but I'm relying on X server's built-in event filtering and routing feature here
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this to avoid information leaks if the GUI daemon thinks it has focus when it does not?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raw input events are sent to the root window, not individual windows. If I want to use raw events, I need to track window focus myself. Also, raw key events don't have modifiers key state (Ctrl, Alt), so I'll have to track that too.

case XI_KeyPress:
case XI_KeyRelease:
process_xievent_keypress(g, (XIDeviceEvent *)xi_event);
break;
case XI_FocusIn:
case XI_FocusOut:
process_xievent_focus(g, (XILeaveEvent *)xi_event);
break;
}
XFreeEventData(g->display, cookie);
} else {
switch (event_buffer.type) {
case ReparentNotify:
process_xevent_reparent(g, (XReparentEvent *) &event_buffer);
break;
case ConfigureNotify:
process_xevent_configure(g, (XConfigureEvent *) &
event_buffer);
break;
case ButtonPress:
case ButtonRelease:
process_xevent_button(g, (XButtonEvent *) & event_buffer);
break;
case MotionNotify:
process_xevent_motion(g, (XMotionEvent *) & event_buffer);
break;
case EnterNotify:
case LeaveNotify:
process_xevent_crossing(g,
(XCrossingEvent *) & event_buffer);
break;
case Expose:
process_xevent_expose(g, (XExposeEvent *) & event_buffer);
break;
case MapNotify:
process_xevent_mapnotify(g, (XMapEvent *) & event_buffer);
break;
case PropertyNotify:
process_xevent_propertynotify(g, (XPropertyEvent *) & event_buffer);
break;
case ClientMessage:
// fprintf(stderr, "xclient, atom=%s\n",
// XGetAtomName(g->display,
// event_buffer.xclient.message_type));
if (event_buffer.xclient.message_type == g->xembed_message) {
process_xevent_xembed(g, (XClientMessageEvent *) &
event_buffer);
} else if ((Atom)event_buffer.xclient.data.l[0] ==
g->wmDeleteMessage) {
if (g->log_level > 0)
fprintf(stderr, "close for 0x%x\n",
(int) event_buffer.xclient.window);
process_xevent_close(g,
event_buffer.xclient.window);
}
break;
}
break;
default:;
}
}

Expand Down
Loading