[PATCH 2/3] Input: Add KeyFocusIn internal event
Peter Hutterer
peter.hutterer at who-t.net
Thu Nov 27 22:02:36 PST 2014
On Mon, Nov 24, 2014 at 10:17:33PM +0000, Daniel Stone wrote:
> KeyFocusIn is used to notify the server that it has entered focus with a
> key held down, so should be used to update the internal state (including
> modifiers), but perform any actions such as sending key events to the
> client, switching VTs, etc.
typo: "not perform any actions"
> This is intended to be used for FocusIn/KeymapNotify when running nested
> servers such as Xephyr, as well as a wl_keyboard::enter event from
> XWayland.
>
> Running the currently-pressed keys through the entire XKB event
> mechanism is quite fragile, and will produce incorrect results in some
> cases, e.g. entering with Shift+AltGr pressed when Shift→AltGr will
> produce different results to AltGr→Shift. In this case, we need to lift
> the _actual_ modifier state from the parent server, however this is a
> much larger job.
>
> Signed-off-by: Daniel Stone <daniels at collabora.com>
> Tested-by: Giulio Camuffo <giuliocamuffo at gmail.com>
> ---
> Xi/exevents.c | 8 +++++++-
> dix/events.c | 5 +++--
> dix/getevents.c | 7 ++++++-
> dix/inpututils.c | 3 ++-
> include/eventstr.h | 1 +
> xkb/xkbActions.c | 38 ++++++++++++++++++++++++++++++++++----
> xkb/xkbPrKeyEv.c | 8 +++++---
> 7 files changed, 58 insertions(+), 12 deletions(-)
>
> diff --git a/Xi/exevents.c b/Xi/exevents.c
> index b0bc47e..cd2924a 100644
> --- a/Xi/exevents.c
> +++ b/Xi/exevents.c
> @@ -810,6 +810,7 @@ UpdateDeviceState(DeviceIntPtr device, DeviceEvent *event)
> case ET_ButtonPress:
> case ET_ButtonRelease:
> case ET_KeyPress:
> + case ET_KeyFocusIn:
I would honestly prefer some different name. We do real focus in events, I'd
like to avoid potential confusion here. How about ET_ServerFocusIn?
but see my reply to 3/3, an extra field in the ET_KeyPress event should
serve us just fine here too.
> case ET_KeyRelease:
> case ET_ProximityIn:
> case ET_ProximityOut:
> @@ -854,7 +855,7 @@ UpdateDeviceState(DeviceIntPtr device, DeviceEvent *event)
> v->axisVal[i] = event->valuators.data[i];
> }
>
> - if (event->type == ET_KeyPress) {
> + if (event->type == ET_KeyPress || event->type == ET_KeyFocusIn) {
> if (!k)
> return DONT_PROCESS;
>
> @@ -1715,6 +1716,7 @@ ProcessDeviceEvent(InternalEvent *ev, DeviceIntPtr device)
> case ET_ButtonPress:
> case ET_ButtonRelease:
> case ET_KeyPress:
> + case ET_KeyFocusIn:
> case ET_KeyRelease:
> case ET_ProximityIn:
> case ET_ProximityOut:
> @@ -1749,6 +1751,10 @@ ProcessDeviceEvent(InternalEvent *ev, DeviceIntPtr device)
> if (!grab && CheckDeviceGrabs(device, event, 0))
> return;
> break;
> + case ET_KeyFocusIn:
> + /* FocusIn events are just for internal state management, and do not
> + * get posted to clients. */
> + return;
you need to run a tab vs space replace on this patch (and maybe the other
two patches as well).
> case ET_KeyRelease:
> if (grab && device->deviceGrab.fromPassiveGrab &&
> (key == device->deviceGrab.activatingKey) &&
> diff --git a/dix/events.c b/dix/events.c
> index b8c67fd..49f21d1 100644
> --- a/dix/events.c
> +++ b/dix/events.c
> @@ -4308,10 +4308,11 @@ FixKeyState(DeviceEvent *event, DeviceIntPtr keybd)
>
> if (event->type == ET_KeyPress) {
> DebugF("FixKeyState: Key %d %s\n", key,
> - ((event->type == ET_KeyPress) ? "down" : "up"));
> + ((event->type == ET_KeyPress) ? "down" :
> + ((event->type == ET_KeyFocusIn) ? "down (focus)" : "up")));
> }
>
> - if (event->type == ET_KeyPress)
> + if (event->type == ET_KeyPress || event->type == ET_KeyFocusIn)
> set_key_down(keybd, key, KEY_PROCESSED);
> else if (event->type == ET_KeyRelease)
> set_key_up(keybd, key, KEY_PROCESSED);
> diff --git a/dix/getevents.c b/dix/getevents.c
> index dd96265..c2248fb 100644
> --- a/dix/getevents.c
> +++ b/dix/getevents.c
> @@ -1120,7 +1120,8 @@ GetKeyboardEvents(InternalEvent *events, DeviceIntPtr pDev, int type,
> UpdateFromMaster(events, pDev, DEVCHANGE_KEYBOARD_EVENT, &num_events);
>
> /* Handle core repeating, via press/release/press/release. */
> - if (type == KeyPress && key_is_down(pDev, key_code, KEY_POSTED)) {
> + if ((type == KeyPress || type == KeymapNotify) &&
> + key_is_down(pDev, key_code, KEY_POSTED)) {
> /* If autorepeating is disabled either globally or just for that key,
> * or we have a modifier, don't generate a repeat event. */
> if (!pDev->kbdfeed->ctrl.autoRepeat ||
> @@ -1152,6 +1153,10 @@ GetKeyboardEvents(InternalEvent *events, DeviceIntPtr pDev, int type,
> event->type = ET_KeyPress;
> set_key_down(pDev, key_code, KEY_POSTED);
> }
> + else if (type == KeymapNotify) {
> + event->type = ET_KeyFocusIn;
> + set_key_down(pDev, key_code, KEY_POSTED);
> + }
> else if (type == KeyRelease) {
> event->type = ET_KeyRelease;
> set_key_up(pDev, key_code, KEY_POSTED);
> diff --git a/dix/inpututils.c b/dix/inpututils.c
> index e5bcc31..f6d781a 100644
> --- a/dix/inpututils.c
> +++ b/dix/inpututils.c
> @@ -694,7 +694,8 @@ event_set_state(DeviceIntPtr mouse, DeviceIntPtr kbd, DeviceEvent *event)
> XkbStatePtr state;
>
> /* we need the state before the event happens */
> - if (event->type == ET_KeyPress || event->type == ET_KeyRelease)
> + if (event->type == ET_KeyPress || event->type == ET_KeyRelease ||
> + event->type == ET_KeyFocusIn)
> state = &kbd->key->xkbInfo->prev_state;
> else
> state = &kbd->key->xkbInfo->state;
> diff --git a/include/eventstr.h b/include/eventstr.h
> index cce903d..2f85d60 100644
> --- a/include/eventstr.h
> +++ b/include/eventstr.h
> @@ -74,6 +74,7 @@ enum EventType {
> ET_XQuartz,
> ET_BarrierHit,
> ET_BarrierLeave,
> + ET_KeyFocusIn,
> ET_Internal = 0xFF /* First byte */
> };
>
> diff --git a/xkb/xkbActions.c b/xkb/xkbActions.c
> index c6cbf56..c075115 100644
> --- a/xkb/xkbActions.c
> +++ b/xkb/xkbActions.c
> @@ -1144,9 +1144,10 @@ _XkbEnsureStateChange(XkbSrvInfoPtr xkbi)
> }
>
> static void
> -_XkbApplyState(DeviceIntPtr dev, Bool genStateNotify, int evtype, int key)
> +_XkbApplyState(DeviceIntPtr dev, Bool genStateNotify, int evtype_int, int key)
.. Bool genStateNotify, enum EventType evtype, int key)
we have an enum, use it, it's much more expressive than "int evtype_int".
coincidentally, storage is cheap enough these days that we could, on
average, afford to add "internal" instead of just "int".
> {
> XkbSrvInfoPtr xkbi = dev->key->xkbInfo;
> + int evtype = (evtype_int == ET_KeyFocusIn) ? KeyPress : evtype_int;
> int changed;
>
> XkbComputeDerivedState(xkbi);
> @@ -1174,10 +1175,11 @@ _XkbApplyState(DeviceIntPtr dev, Bool genStateNotify, int evtype, int key)
> }
>
> void
> -XkbPushLockedStateToSlaves(DeviceIntPtr master, int evtype, int key)
> +XkbPushLockedStateToSlaves(DeviceIntPtr master, int evtype_int, int key)
same here
Cheers,
Peter
> {
> DeviceIntPtr dev;
> Bool genStateNotify;
> + int evtype = (evtype_int == ET_KeyFocusIn) ? KeyPress : evtype_int;
>
> nt_list_for_each_entry(dev, inputInfo.devices, next) {
> if (!dev->key || GetMaster(dev, MASTER_KEYBOARD) != master)
> @@ -1199,6 +1201,32 @@ XkbActionGetFilter(DeviceIntPtr dev, DeviceEvent *event, KeyCode key,
> XkbSrvInfoPtr xkbi = dev->key->xkbInfo;
> XkbFilterPtr filter;
>
> + /* For KeyFocusIn, we only want to run actions which update our state to
> + * (hopefully vaguely kinda) match that of the host server, rather than
> + * actually execute anything. For example, if we enter our VT with
> + * Ctrl+Alt+Backspace held down, we don't want to terminate our server
> + * immediately, but we _do_ want Ctrl+Alt to be latched down, so if
> + * Backspace is released and then pressed again, the server will terminate.
> + *
> + * This is pretty flaky, and we should in fact inherit the complete state
> + * from the host server. There are some state combinations that we cannot
> + * express by running the state machine over every key, e.g. if AltGr+Shift
> + * generates a different state to Shift+AltGr. */
> + if (event->type == ET_KeyFocusIn) {
> + switch (act->type) {
> + case XkbSA_SetMods:
> + case XkbSA_SetGroup:
> + case XkbSA_LatchMods:
> + case XkbSA_LatchGroup:
> + case XkbSA_LockMods:
> + case XkbSA_LockGroup:
> + break;
> + default:
> + *sendEvent = 1;
> + return;
> + }
> + }
> +
> switch (act->type) {
> case XkbSA_SetMods:
> case XkbSA_SetGroup:
> @@ -1291,9 +1319,11 @@ XkbHandleActions(DeviceIntPtr dev, DeviceIntPtr kbd, DeviceEvent *event)
> xkbi->groupChange = 0;
>
> sendEvent = 1;
> - keyEvent = ((event->type == ET_KeyPress) || (event->type == ET_KeyRelease));
> + keyEvent = ((event->type == ET_KeyPress) || (event->type == ET_KeyRelease) ||
> + (event->type == ET_KeyFocusIn));
> pressEvent = ((event->type == ET_KeyPress) ||
> - (event->type == ET_ButtonPress));
> + (event->type == ET_ButtonPress) ||
> + (event->type == ET_KeyFocusIn));
>
> if (pressEvent) {
> if (keyEvent)
> diff --git a/xkb/xkbPrKeyEv.c b/xkb/xkbPrKeyEv.c
> index b24fd6c..0b15f84 100644
> --- a/xkb/xkbPrKeyEv.c
> +++ b/xkb/xkbPrKeyEv.c
> @@ -56,7 +56,8 @@ XkbProcessKeyboardEvent(DeviceEvent *event, DeviceIntPtr keybd)
> key = event->detail.key;
> if (xkbDebugFlags & 0x8)
> DebugF("[xkb] XkbPKE: Key %d %s\n", key,
> - (event->type == ET_KeyPress ? "down" : "up"));
> + (event->type == ET_KeyPress ? "down" :
> + (event->type == ET_KeyFocusIn ? "down (focus)" : "up")));
>
> if (xkbi->repeatKey == key && event->type == ET_KeyRelease &&
> !(xkbi->desc->ctrls->enabled_ctrls & XkbRepeatKeysMask))
> @@ -73,14 +74,15 @@ XkbProcessKeyboardEvent(DeviceEvent *event, DeviceIntPtr keybd)
> case XkbKB_Default:
> /* Neither of these should happen in practice, but ignore them
> anyway. */
> - if (event->type == ET_KeyPress && !event->key_repeat &&
> - key_is_down(keybd, key, KEY_PROCESSED))
> + if ((event->type == ET_KeyPress || event->type == ET_KeyFocusIn) &&
> + !event->key_repeat && key_is_down(keybd, key, KEY_PROCESSED))
> return;
> else if (event->type == ET_KeyRelease &&
> !key_is_down(keybd, key, KEY_PROCESSED))
> return;
> break;
> case XkbKB_Lock:
> + /* XXX: Semantics here might be incorrect for KeyFocusIn. */
> if (event->type == ET_KeyRelease)
> return;
> else if (key_is_down(keybd, key, KEY_PROCESSED))
> --
> 2.1.0
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>
More information about the xorg-devel
mailing list