[PATCH 1/3] xkb: Several fixes to ISOLock and a few related changes.
Ran Benita
ran234 at gmail.com
Fri Feb 14 02:40:47 PST 2014
On Thu, Jan 23, 2014 at 08:39:49PM +0100, Andreas Wettstein wrote:
> Several fixes to ISOLock:
> - Use the proper byte to access the "affect" flags.
> - When changing a Set/Latch action to a Lock action, also change the
> flags for the action.
> - Respect NoLock/NoUnlock.
> - Add the missing transformation of actions that occurred before the
> ISOLock was pressed.
>
> Support NoLock/NoUnlock for "Controls".
I've yet to look at the xserver changes, but may I ask to split all
those fixes up? It's much easier to go through that way, and as far as
I'm concerned the more the better (as long as they are logically
separate).
> Output changes:
> - support flag "genKeyEvent" flag for message actions.
> - Output "affect" flags for control locks, modifier locks, and ISOLock.
(Same comments as the libxkbfile patch on these changes).
Ran
> Furthermore, prevent autorepeat for Set/Lock actions. This serves
> mainly to reduce the number of cases that can occur in connection with
> ISOLock. Also note that the code did not properly check whether a key
> is released, so autorepeat handling was incorrect anyway.
>
> Signed-off-by: Andreas Wettstein <wettstein509 at solnet.ch>
> ---
> xkb/xkbActions.c | 177 ++++++++++++++++++++++++++++++++++++++++---------------
> xkb/xkbtext.c | 50 +++++++++++++++-
> 2 files changed, 179 insertions(+), 48 deletions(-)
>
> diff --git a/xkb/xkbActions.c b/xkb/xkbActions.c
> index e32005c..f726775 100644
> --- a/xkb/xkbActions.c
> +++ b/xkb/xkbActions.c
> @@ -181,6 +181,7 @@ _XkbFilterSetState(XkbSrvInfoPtr xkbi,
> XkbFilterPtr filter, unsigned keycode, XkbAction *pAction)
> {
> if (filter->keycode == 0) { /* initial press */
> + AccessXCancelRepeatKey(xkbi, keycode);
> filter->keycode = keycode;
> filter->active = 1;
> filter->filterOthers = ((pAction->mods.mask & XkbSA_ClearLocks) != 0);
> @@ -189,6 +190,8 @@ _XkbFilterSetState(XkbSrvInfoPtr xkbi,
> if (pAction->type == XkbSA_SetMods) {
> filter->upAction = *pAction;
> xkbi->setMods = pAction->mods.mask;
> + /* This is used in case an ISOLock comes up. */
> + filter->priv = xkbi->state.locked_mods & pAction->mods.mask;
> }
> else {
> xkbi->groupChange = XkbSAGroup(&pAction->group);
> @@ -339,12 +342,14 @@ _XkbFilterLatchState(XkbSrvInfoPtr xkbi,
> else if (pAction && (filter->priv == LATCH_KEY_DOWN)) {
> /* Latch was broken before it became pending: degrade to a
> SetMods/SetGroup. */
> - if (filter->upAction.type == XkbSA_LatchMods)
> + if (filter->upAction.type == XkbSA_LatchMods) {
> filter->upAction.type = XkbSA_SetMods;
> - else
> + filter->priv = xkbi->state.locked_mods & filter->upAction.mods.mask;
> + } else {
> filter->upAction.type = XkbSA_SetGroup;
> + filter->priv = 0;
> + }
> filter->filter = _XkbFilterSetState;
> - filter->priv = 0;
> return filter->filter(xkbi, filter, keycode, pAction);
> }
> return 1;
> @@ -354,6 +359,9 @@ static int
> _XkbFilterLockState(XkbSrvInfoPtr xkbi,
> XkbFilterPtr filter, unsigned keycode, XkbAction *pAction)
> {
> + if (filter->keycode == 0) /* initial press */
> + AccessXCancelRepeatKey(xkbi, keycode);
> +
> if (pAction && (pAction->type == XkbSA_LockGroup)) {
> if (pAction->group.flags & XkbSA_GroupAbsolute)
> xkbi->state.locked_group = XkbSAGroup(&pAction->group);
> @@ -381,6 +389,47 @@ _XkbFilterLockState(XkbSrvInfoPtr xkbi,
> return 1;
> }
>
> +
> +static Bool
> +_XkbTransformActionForISOLock(XkbAction* pAction,
> + CARD8 flags, CARD8 affect)
> +{
> + switch (pAction->type) {
> + case XkbSA_SetMods:
> + case XkbSA_LatchMods:
> + if (!(affect & XkbSA_ISONoAffectMods)) {
> + pAction->type = XkbSA_LockMods;
> + pAction->mods.flags &= ~(XkbSA_ClearLocks | XkbSA_LatchToLock);
> + pAction->mods.flags |= flags & (XkbSA_LockNoLock | XkbSA_LockNoUnlock);
> + return 1;
> + }
> + break;
> + case XkbSA_SetGroup:
> + case XkbSA_LatchGroup:
> + if (!(affect & XkbSA_ISONoAffectGroup)) {
> + pAction->type = XkbSA_LockGroup;
> + pAction->group.flags &= ~(XkbSA_ClearLocks | XkbSA_LatchToLock);
> + return 1;
> + }
> + break;
> + case XkbSA_PtrBtn:
> + if (!(affect & XkbSA_ISONoAffectPtr)) {
> + pAction->type = XkbSA_LockPtrBtn;
> + pAction->btn.flags |= flags & (XkbSA_LockNoLock | XkbSA_LockNoUnlock);
> + return 1;
> + }
> + break;
> + case XkbSA_SetControls:
> + if (!(affect & XkbSA_ISONoAffectCtrls)) {
> + pAction->type = XkbSA_LockControls;
> + pAction->ctrls.flags |= flags & (XkbSA_LockNoLock | XkbSA_LockNoUnlock);
> + return 1;
> + }
> + break;
> + }
> + return 0;
> +}
> +
> #define ISO_KEY_DOWN 0
> #define NO_ISO_LOCK 1
>
> @@ -388,9 +437,12 @@ static int
> _XkbFilterISOLock(XkbSrvInfoPtr xkbi,
> XkbFilterPtr filter, unsigned keycode, XkbAction *pAction)
> {
> + int i;
>
> if (filter->keycode == 0) { /* initial press */
> + AccessXCancelRepeatKey(xkbi, keycode);
> CARD8 flags = pAction->iso.flags;
> + CARD8 affect = pAction->iso.affect;
>
> filter->keycode = keycode;
> filter->active = 1;
> @@ -406,15 +458,56 @@ _XkbFilterISOLock(XkbSrvInfoPtr xkbi,
> xkbi->setMods = pAction->iso.mask;
> xkbi->groupChange = 0;
> }
> - if ((!(flags & XkbSA_ISONoAffectMods)) && (xkbi->state.base_mods)) {
> - filter->priv = NO_ISO_LOCK;
> - xkbi->state.locked_mods ^= xkbi->state.base_mods;
> - }
> - if ((!(flags & XkbSA_ISONoAffectGroup)) && (xkbi->state.base_group)) {
> -/* 6/22/93 (ef) -- lock groups if group key is down first */
> - }
> - if (!(flags & XkbSA_ISONoAffectPtr)) {
> -/* 6/22/93 (ef) -- lock mouse buttons if they're down */
> +
> + for (i = 0; i < xkbi->szFilters; i++) {
> + if ((xkbi->filters[i].active) && (xkbi->filters[i].filter)) {
> + XkbAction *upAction = &xkbi->filters[i].upAction;
> + unsigned char oldtype = upAction->type;
> +
> + if (_XkbTransformActionForISOLock(upAction, flags, affect)) {
> + switch (oldtype) {
> + /* Latches cannot occur, as they have been tranformed by their own filter */
> + case XkbSA_SetMods:
> + xkbi->filters[i].filter = _XkbFilterLockState;
> + if (!(flags & XkbSA_LockNoLock))
> + xkbi->state.locked_mods |= upAction->mods.mask;
> + break;
> + case XkbSA_SetGroup:
> + xkbi->filters[i].active = 0;
> + xkbi->groupChange -= XkbSAGroup(&upAction->group);
> + xkbi->state.locked_group += XkbSAGroup(&upAction->group);
> + break;
> + case XkbSA_PtrBtn:
> + /* We miss actions with nonzero click count. */
> + if (!(xkbi->lockedPtrButtons & (1 << upAction->btn.button)) &&
> + !(flags & XkbSA_LockNoLock)) {
> + xkbi->lockedPtrButtons |= (1 << upAction->btn.button);
> + upAction->type = XkbSA_NoAction;
> + }
> + /* Unlocking is handled by the already transformed filter. */
> + break;
> + case XkbSA_SetControls:
> + if (flags & XkbSA_LockNoUnlock) {
> + if (!(flags & XkbSA_LockNoLock))
> + xkbi->filters[i].priv = 0;
> + }
> + else {
> + /* priv contains the controls that have been
> + enabled by the SetControl action. From those,
> + we can obtain the controls that had already been
> + enabled before. */
> + int setByAction = xkbi->filters[i].priv;
> + int setBefore = XkbActionCtrls(&upAction->ctrls) & ~setByAction;
> + if (!flags & XkbSA_LockNoLock)
> + xkbi->filters[i].priv = XkbActionCtrls(&upAction->ctrls);
> + else
> + xkbi->filters[i].priv = setBefore;
> + }
> + break;
> + }
> + filter->priv = NO_ISO_LOCK;
> + }
> + }
> }
> }
> else if (filter->keycode == keycode) {
> @@ -429,42 +522,23 @@ _XkbFilterISOLock(XkbSrvInfoPtr xkbi,
> else {
> xkbi->clearMods = filter->upAction.iso.mask;
> xkbi->groupChange = 0;
> - if (filter->priv == ISO_KEY_DOWN)
> - xkbi->state.locked_mods ^= filter->upAction.iso.mask;
> + if (filter->priv == ISO_KEY_DOWN) {
> + unsigned char common =
> + xkbi->state.locked_mods & filter->upAction.iso.mask;
> + if (!(flags & XkbSA_LockNoLock))
> + xkbi->state.locked_mods |= filter->upAction.iso.mask;
> + if (!(flags & XkbSA_LockNoUnlock))
> + xkbi->state.locked_mods &= ~common;
> + }
> }
> filter->active = 0;
> }
> else if (pAction) {
> CARD8 flags = filter->upAction.iso.flags;
> + CARD8 affect = filter->upAction.iso.affect;
>
> - switch (pAction->type) {
> - case XkbSA_SetMods:
> - case XkbSA_LatchMods:
> - if (!(flags & XkbSA_ISONoAffectMods)) {
> - pAction->type = XkbSA_LockMods;
> - filter->priv = NO_ISO_LOCK;
> - }
> - break;
> - case XkbSA_SetGroup:
> - case XkbSA_LatchGroup:
> - if (!(flags & XkbSA_ISONoAffectGroup)) {
> - pAction->type = XkbSA_LockGroup;
> - filter->priv = NO_ISO_LOCK;
> - }
> - break;
> - case XkbSA_PtrBtn:
> - if (!(flags & XkbSA_ISONoAffectPtr)) {
> - pAction->type = XkbSA_LockPtrBtn;
> - filter->priv = NO_ISO_LOCK;
> - }
> - break;
> - case XkbSA_SetControls:
> - if (!(flags & XkbSA_ISONoAffectCtrls)) {
> - pAction->type = XkbSA_LockControls;
> - filter->priv = NO_ISO_LOCK;
> - }
> - break;
> - }
> + if (_XkbTransformActionForISOLock(pAction, flags, affect))
> + filter->priv = NO_ISO_LOCK;
> }
> return 1;
> }
> @@ -635,6 +709,7 @@ _XkbFilterPointerBtn(XkbSrvInfoPtr xkbi,
> }
> break;
> }
> + return 0;
> }
> else if (filter->keycode == keycode) {
> int button = filter->upAction.btn.button;
> @@ -660,8 +735,9 @@ _XkbFilterPointerBtn(XkbSrvInfoPtr xkbi,
> break;
> }
> filter->active = 0;
> + return 0;
> }
> - return 0;
> + return 1;
> }
>
> static int
> @@ -678,17 +754,26 @@ _XkbFilterControls(XkbSrvInfoPtr xkbi,
> ctrls = xkbi->desc->ctrls;
> old = *ctrls;
> if (filter->keycode == 0) { /* initial press */
> + AccessXCancelRepeatKey(xkbi, keycode);
> filter->keycode = keycode;
> filter->active = 1;
> filter->filterOthers = 0;
> - change = XkbActionCtrls(&pAction->ctrls);
> + change = (XkbActionCtrls(&pAction->ctrls) & ~ctrls->enabled_ctrls);
> filter->priv = change;
> filter->filter = _XkbFilterControls;
> filter->upAction = *pAction;
>
> if (pAction->type == XkbSA_LockControls) {
> - filter->priv = (ctrls->enabled_ctrls & change);
> - change &= ~ctrls->enabled_ctrls;
> + if (pAction->ctrls.flags & XkbSA_LockNoUnlock)
> + filter->priv = 0;
> + else
> + /* The protocol specification says we should unlock controls
> + are NOT currently enabled. As this does not make any sense,
> + let's assume this is a typo and do the opposite. */
> + filter->priv = (XkbActionCtrls(&pAction->ctrls) & ctrls->enabled_ctrls);
> +
> + if (pAction->ctrls.flags & XkbSA_LockNoLock)
> + change = 0;
> }
>
> if (change) {
> diff --git a/xkb/xkbtext.c b/xkb/xkbtext.c
> index fdf1d17..fd323bc 100644
> --- a/xkb/xkbtext.c
> +++ b/xkb/xkbtext.c
> @@ -749,8 +749,22 @@ CopyModActionArgs(XkbDescPtr xkb, XkbAction *action, char *buf, int *sz)
> }
> else
> TryCopyStr(buf, "none", sz);
> - if (act->type == XkbSA_LockMods)
> + if (act->type == XkbSA_LockMods) {
> + switch (act->flags & (XkbSA_LockNoUnlock | XkbSA_LockNoLock)) {
> + case XkbSA_LockNoLock:
> + TryCopyStr(buf, ",affect=unlock", sz);
> + break;
> + case XkbSA_LockNoUnlock:
> + TryCopyStr(buf, ",affect=lock", sz);
> + break;
> + case XkbSA_LockNoUnlock|XkbSA_LockNoLock:
> + TryCopyStr(buf, ",affect=neither", sz);
> + break;
> + default:
> + break;
> + }
> return TRUE;
> + }
> if (act->flags & XkbSA_ClearLocks)
> TryCopyStr(buf, ",clearLocks", sz);
> if (act->flags & XkbSA_LatchToLock)
> @@ -901,8 +915,12 @@ CopyISOLockArgs(XkbDescPtr xkb, XkbAction *action, char *buf, int *sz)
> TryCopyStr(buf, "none", sz);
> }
> TryCopyStr(buf, ",affect=", sz);
> - if ((act->affect & XkbSA_ISOAffectMask) == 0)
> + if ((act->affect & XkbSA_ISOAffectMask) == 0) {
> TryCopyStr(buf, "all", sz);
> + }
> + else if ((act->affect & XkbSA_ISOAffectMask) == XkbSA_ISOAffectMask) {
> + TryCopyStr(buf, "none", sz);
> + }
> else {
> int nOut = 0;
>
> @@ -926,6 +944,18 @@ CopyISOLockArgs(XkbDescPtr xkb, XkbAction *action, char *buf, int *sz)
> nOut++;
> }
> }
> + switch (act->flags & (XkbSA_LockNoUnlock | XkbSA_LockNoLock)) {
> + case XkbSA_LockNoLock:
> + TryCopyStr(buf, "+unlock", sz);
> + break;
> + case XkbSA_LockNoUnlock:
> + TryCopyStr(buf, "+lock", sz);
> + break;
> + case XkbSA_LockNoUnlock | XkbSA_LockNoLock:
> + TryCopyStr(buf, "+neither", sz);
> + break;
> + default: ;
> + }
> return TRUE;
> }
>
> @@ -1037,6 +1067,20 @@ CopySetLockControlsArgs(XkbDescPtr xkb, XkbAction *action, char *buf, int *sz)
> nOut++;
> }
> }
> + if (action->type == XkbSA_LockControls) {
> + switch (act->flags & (XkbSA_LockNoUnlock | XkbSA_LockNoLock)) {
> + case XkbSA_LockNoLock:
> + TryCopyStr(buf, ",affect=unlock", sz);
> + break;
> + case XkbSA_LockNoUnlock:
> + TryCopyStr(buf, ",affect=lock", sz);
> + break;
> + case XkbSA_LockNoUnlock | XkbSA_LockNoLock:
> + TryCopyStr(buf, ",affect=neither", sz);
> + break;
> + default: ;
> + }
> + }
> return TRUE;
> }
>
> @@ -1070,6 +1114,8 @@ CopyActionMessageArgs(XkbDescPtr xkb, XkbAction *action, char *buf, int *sz)
> TryCopyStr(buf, tbuf, sz);
> snprintf(tbuf, sizeof(tbuf), ",data[5]=0x%02x", act->message[5]);
> TryCopyStr(buf, tbuf, sz);
> + if (act->flags & XkbSA_MessageGenKeyEvent)
> + TryCopyStr(buf, ",genKeyEvent", sz);
> return TRUE;
> }
>
> --
> 1.8.3.1
>
> _______________________________________________
> 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