[PATCH] xkb: Fixes to LatchMods/LatchGroup
Peter Hutterer
peter.hutterer at who-t.net
Tue Mar 5 17:24:22 PST 2013
On Sun, Mar 03, 2013 at 08:25:44PM +0100, Andreas Wettstein wrote:
> The main problem this patch addresses is that if a latch is put on
> multi-level key with a Latch/Lock/Set, it is possible that after all
> keys are released, still base modifiers are set, which typically will
> make the keyboard unusable. To see how it happens (without the patch),
> assume that key AltGr sets Mod5 when pressed by itself, and latches Mod3
> when pressed together with Shift. Now press Shift, then AltGr and
> release both keys in reverse order. Mod3 is now latched, and the
> LatchMods filter remains active as the second filter. Now press AltGr;
> Mod5 base modifier gets set, and the SetMods filter will become active
> as the first filter. Release AltGr: First, the SetMods filter will set
> clearMods to Mod5, then the LatchMods filter will overwrite clearMods
> with Mod3. Result: the Mod5 base modifier will remain set. This
> example becomes practically relevant for the revised German standard
> layout (DIN 2137-1:2012-06).
>
> Other changes implement the latch behaviour more accurately according to
> the specification. For example, releasing a modifier latching key can
> at the same time clear a locked modifier, promote another modifier that
> is latched to locked, and latch a third modifier. Overall, what the
> code does should be straightforward to compare what the XKB protocol
> specification demands, see the table in section 6.3.
>
> Finally, releasing a key no longer cancels a latch that has not become
> pending yet. In my opinion, the specification is not clear; it speaks
> of "operating" a key, which the patch effectivly interprets as "press"
> rather than "press or release". From my experience, using the latter
> interpretation makes latches on higher levels practically unusable. In
> the example given above, one would have to release AltGr always before
> Shift to get the Mod3-Latch. The practical relevance of latches on
> higher levels is once more given by the revised German standard layout.
>
> Signed-off-by: Andreas Wettstein <wettstein509 at solnet.ch>
whoah, that patch made my head spin a bit (like all of xkb). thanks, merged
into next.
fwiw, please set your editor to use spaces, not tabs on the xserver code. I
re-indented this patch.
Also two minor amendments moving declarations up to fix:
xkbActions.c:272:6: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
xkbActions.c:284:3: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
Cheers,
Peter
> ---
> xkb/xkbActions.c | 175 +++++++++++++++++++++++++++++--------------------------
> 1 file changed, 91 insertions(+), 84 deletions(-)
>
> diff --git a/xkb/xkbActions.c b/xkb/xkbActions.c
> index da727e6..56cddd3 100644
> --- a/xkb/xkbActions.c
> +++ b/xkb/xkbActions.c
> @@ -222,14 +222,14 @@ _XkbFilterSetState(XkbSrvInfoPtr xkbi,
>
> #define LATCH_KEY_DOWN 1
> #define LATCH_PENDING 2
> -#define NO_LATCH 3
>
> static int
> _XkbFilterLatchState(XkbSrvInfoPtr xkbi,
> XkbFilterPtr filter, unsigned keycode, XkbAction *pAction)
> {
>
> - if (filter->keycode == 0) { /* initial press */
> + if (filter->keycode == 0) { /* initial press */
> + AccessXCancelRepeatKey(xkbi,keycode);
> filter->keycode = keycode;
> filter->active = 1;
> filter->filterOthers = 1;
> @@ -250,91 +250,98 @@ _XkbFilterLatchState(XkbSrvInfoPtr xkbi,
> else if (pAction && (filter->priv == LATCH_PENDING)) {
> if (((1 << pAction->type) & XkbSA_BreakLatch) != 0) {
> filter->active = 0;
> - if (filter->upAction.type == XkbSA_LatchMods)
> - xkbi->state.latched_mods &= ~filter->upAction.mods.mask;
> - else
> - xkbi->state.latched_group -=
> - XkbSAGroup(&filter->upAction.group);
> - }
> - else if ((pAction->type == filter->upAction.type) &&
> - (pAction->mods.flags == filter->upAction.mods.flags) &&
> - (pAction->mods.mask == filter->upAction.mods.mask)) {
> - if (filter->upAction.mods.flags & XkbSA_LatchToLock) {
> - XkbControlsPtr ctrls = xkbi->desc->ctrls;
> -
> - if (filter->upAction.type == XkbSA_LatchMods)
> - pAction->mods.type = XkbSA_LockMods;
> - else
> - pAction->group.type = XkbSA_LockGroup;
> - if (XkbAX_NeedFeedback(ctrls, XkbAX_StickyKeysFBMask) &&
> - (ctrls->enabled_ctrls & XkbStickyKeysMask)) {
> - XkbDDXAccessXBeep(xkbi->device, _BEEP_STICKY_LOCK,
> - XkbStickyKeysMask);
> - }
> - }
> - else {
> - if (filter->upAction.type == XkbSA_LatchMods)
> - pAction->mods.type = XkbSA_SetMods;
> - else
> - pAction->group.type = XkbSA_SetGroup;
> - }
> - if (filter->upAction.type == XkbSA_LatchMods)
> - xkbi->state.latched_mods &= ~filter->upAction.mods.mask;
> - else
> - xkbi->state.latched_group -=
> - XkbSAGroup(&filter->upAction.group);
> - filter->active = 0;
> + /* If one latch is broken, all latches are broken, so it's no use
> + to find out which particular latch this filter tracks. */
> + xkbi->state.latched_mods = 0;
> + xkbi->state.latched_group = 0;
> }
> }
> - else if (filter->keycode == keycode) { /* release */
> - XkbControlsPtr ctrls = xkbi->desc->ctrls;
> - int needBeep;
> - int beepType = _BEEP_NONE;
> -
> - needBeep = ((ctrls->enabled_ctrls & XkbStickyKeysMask) &&
> - XkbAX_NeedFeedback(ctrls, XkbAX_StickyKeysFBMask));
> - if (filter->upAction.type == XkbSA_LatchMods) {
> - xkbi->clearMods = filter->upAction.mods.mask;
> - if ((filter->upAction.mods.flags & XkbSA_ClearLocks) &&
> - (xkbi->clearMods & xkbi->state.locked_mods) ==
> - xkbi->clearMods) {
> - xkbi->state.locked_mods &= ~xkbi->clearMods;
> - filter->priv = NO_LATCH;
> - beepType = _BEEP_STICKY_UNLOCK;
> - }
> - }
> - else {
> - xkbi->groupChange = -XkbSAGroup(&filter->upAction.group);
> - if ((filter->upAction.group.flags & XkbSA_ClearLocks) &&
> - (xkbi->state.locked_group)) {
> - xkbi->state.locked_group = 0;
> - filter->priv = NO_LATCH;
> - beepType = _BEEP_STICKY_UNLOCK;
> - }
> - }
> - if (filter->priv == NO_LATCH) {
> - filter->active = 0;
> - }
> - else {
> - filter->priv = LATCH_PENDING;
> - if (filter->upAction.type == XkbSA_LatchMods) {
> - xkbi->state.latched_mods |= filter->upAction.mods.mask;
> - needBeep = xkbi->state.latched_mods ? needBeep : 0;
> - xkbi->state.latched_mods |= filter->upAction.mods.mask;
> - }
> - else {
> - xkbi->state.latched_group +=
> - XkbSAGroup(&filter->upAction.group);
> - }
> - if (needBeep && (beepType == _BEEP_NONE))
> - beepType = _BEEP_STICKY_LATCH;
> - }
> - if (needBeep && (beepType != _BEEP_NONE))
> - XkbDDXAccessXBeep(xkbi->device, beepType, XkbStickyKeysMask);
> + else if (filter->keycode == keycode && filter->priv != LATCH_PENDING){
> + /* The test above for LATCH_PENDING skips subsequent releases of the
> + key after it has been released first time and the latch became
> + pending. */
> + XkbControlsPtr ctrls = xkbi->desc->ctrls;
> + int needBeep = ((ctrls->enabled_ctrls & XkbStickyKeysMask) &&
> + XkbAX_NeedFeedback(ctrls, XkbAX_StickyKeysFBMask));
> +
> + if (filter->upAction.type == XkbSA_LatchMods) {
> + unsigned char mask = filter->upAction.mods.mask;
> + xkbi->clearMods = mask;
> +
> + /* ClearLocks */
> + unsigned char common = mask & xkbi->state.locked_mods;
> + if ((filter->upAction.mods.flags & XkbSA_ClearLocks) && common) {
> + mask &= ~common;
> + xkbi->state.locked_mods &= ~common;
> + if (needBeep)
> + XkbDDXAccessXBeep(xkbi->device, _BEEP_STICKY_UNLOCK,
> + XkbStickyKeysMask);
> + }
> + /* LatchToLock */
> + common = mask & xkbi->state.latched_mods;
> + if ((filter->upAction.mods.flags & XkbSA_LatchToLock) && common) {
> + mask &= ~common;
> + unsigned char newlocked = common & ~xkbi->state.locked_mods;
> + if(newlocked){
> + xkbi->state.locked_mods |= newlocked;
> + if (needBeep)
> + XkbDDXAccessXBeep(xkbi->device, _BEEP_STICKY_LOCK,
> + XkbStickyKeysMask);
> +
> + }
> + xkbi->state.latched_mods &= ~common;
> + }
> + /* Latch remaining modifiers, if any. */
> + if (mask) {
> + xkbi->state.latched_mods |= mask;
> + filter->priv = LATCH_PENDING;
> + if (needBeep)
> + XkbDDXAccessXBeep(xkbi->device, _BEEP_STICKY_LATCH,
> + XkbStickyKeysMask);
> + }
> + }
> + else {
> + xkbi->groupChange = -XkbSAGroup(&filter->upAction.group);
> + /* ClearLocks */
> + if ((filter->upAction.group.flags & XkbSA_ClearLocks) &&
> + (xkbi->state.locked_group)) {
> + xkbi->state.locked_group = 0;
> + if (needBeep)
> + XkbDDXAccessXBeep(xkbi->device, _BEEP_STICKY_UNLOCK,
> + XkbStickyKeysMask);
> + }
> + /* LatchToLock */
> + else if ((filter->upAction.group.flags & XkbSA_LatchToLock)
> + && (xkbi->state.latched_group)) {
> + xkbi->state.locked_group += XkbSAGroup(&filter->upAction.group);
> + xkbi->state.latched_group -= XkbSAGroup(&filter->upAction.group);
> + if(XkbSAGroup(&filter->upAction.group) && needBeep)
> + XkbDDXAccessXBeep(xkbi->device, _BEEP_STICKY_LOCK,
> + XkbStickyKeysMask);
> + }
> + /* Latch group */
> + else if(XkbSAGroup(&filter->upAction.group)){
> + xkbi->state.latched_group += XkbSAGroup(&filter->upAction.group);
> + filter->priv = LATCH_PENDING;
> + if (needBeep)
> + XkbDDXAccessXBeep(xkbi->device, _BEEP_STICKY_LATCH,
> + XkbStickyKeysMask);
> + }
> + }
> +
> + if (filter->priv != LATCH_PENDING)
> + filter->active = 0;
> }
> - else if (filter->priv == LATCH_KEY_DOWN) {
> - filter->priv = NO_LATCH;
> - filter->filterOthers = 0;
> + 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)
> + filter->upAction.type = XkbSA_SetMods;
> + else
> + filter->upAction.type = XkbSA_SetGroup;
> + filter->filter = _XkbFilterSetState;
> + filter->priv = 0;
> + return filter->filter(xkbi, filter, keycode, pAction);
> }
> return 1;
> }
> --
> 1.8.1.2
>
> _______________________________________________
> 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