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 <[email protected]> > Tested-by: Giulio Camuffo <[email protected]> > --- > 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 > > _______________________________________________ > [email protected]: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel > _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
