[PATCH v4 xserver] xkb: fix releasing overlay while keydown

Mariusz Mazur mariusz.g.mazur at gmail.com
Sun Jan 1 10:50:02 UTC 2017


I've been using this heavily (at least a few hundred overlay1
keystrokes per day) for the past almost two months and had zero
issues. Any chance this could get merged?

2016-11-15 10:31 GMT+01:00 Mariusz Mazur <mariusz.g.mazur at gmail.com>:
> A week has passed and everything's working fine (and I'm using the
> overlay keycombos very very heavily). Seems the patch does not have
> any unforeseen side effects.
>
> 2016-11-07 22:25 GMT+01:00 Mariusz Mazur <mariusz.g.mazur at gmail.com>:
>> Applied to my work env. If something starts acting funny I'll let you know.
>>
>> 2016-11-06 23:42 GMT+01:00 Mihail Konev <k.mvc at ya.ru>:
>>> Testcase:
>>>
>>> In ~/.xbindkeysrc:
>>>
>>>   "xterm &"
>>>        XF86LaunchA
>>>
>>> In ~/ov.xkb:
>>>
>>>   xkb_keymap {
>>>           xkb_keycodes { include "evdev" };
>>>           xkb_types    { include "complete" };
>>>
>>>           xkb_compat   { include "complete"
>>>                interpret Overlay1_Enable+AnyOfOrNone(all) {
>>>                   action= SetControls(controls=Overlay1);
>>>                };
>>>           };
>>>
>>>           xkb_symbols  { include "pc+inet(evdev)+us"
>>>                   key <INS> { [ Overlay1_Enable ] };
>>>                   key <AE01> { overlay1 = <AE02> }; // Insert+1 => 2
>>>                   key <TLDE> { overlay1 = <I128> }; // Insert+~ => XF86LaunchA
>>>           };
>>>
>>>           xkb_geometry { include "pc(pc104)" };
>>>   };
>>>
>>> Apply this layout: 'xkbcomp ~/ov.xkb $DISPLAY'.
>>> Run "xbindkeys -n -v"
>>> In the exact order:
>>> - press Insert
>>> - press Tilde
>>> - release Insert
>>> - wait
>>> - release Tilde
>>> Keyboard input in the new terminal window(s) would be locked
>>> until another Insert+Tilde .
>>>
>>> Reported-by: Mariusz Mazur <mariusz.g.mazur at gmail.com>
>>> Signed-off-by: Mihail Konev <k.mvc at ya.ru>
>>> ---
>>> v3 was still incorrect and did not done what it was supposed to.
>>> This version is specifically tested to properly enable and disable
>>> overlay, i.e. allow "`"-s to be sent both before and after Insert being
>>> down.
>>> Debugging version attached.
>>>
>>> Without (keywas_overlaid - 1) trickery it does not address the issue
>>> (i.e. input stays locked until Insert+Tilde)
>>> (but does not happen without open-new-window being triggered by xbindkeys,
>>> i.e. when the latter is not running).
>>> Maybe overlay_perkey_state description comment should better reflect this.
>>>
>>> Also commit description missed Reported-by.
>>>
>>> The "where-overlay1,2-is-in-xkb" is resolved in this patch.
>>>
>>> As for "applicability of overlays", they are per-keycode, and layout-independent.
>>> This differs from RedirectKey that are per-keysym, and, therefore,
>>> also per-shift-level and per-layout (per-group).
>>>
>>> There should be no need to use overlays instead of RedirectKey,
>>> especially given that overlay is a "behavior", which
>>> could be only one per keycode.
>>>
>>>  xkb/xkbPrKeyEv.c | 36 +++++++++++++++++++++++++++++++-----
>>>  1 file changed, 31 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/xkb/xkbPrKeyEv.c b/xkb/xkbPrKeyEv.c
>>> index f7a6b4b14306..35bb1e9f405a 100644
>>> --- a/xkb/xkbPrKeyEv.c
>>> +++ b/xkb/xkbPrKeyEv.c
>>> @@ -43,6 +43,14 @@ THE USE OR PERFORMANCE OF THIS SOFTWARE.
>>>
>>>  /***====================================================================***/
>>>
>>> +/* Keeps track of overlay in effect for a given key,
>>> + * so that if an overlay is released while key is down,
>>> + * the key retains overlaid until its release.
>>> + * Cannot be a bitmask, as needs at least three values
>>> + * (as overlaid keys need to generate two releases).
>>> + * */
>>> +static unsigned char overlay_perkey_state[256];
>>> +
>>>  void
>>>  XkbProcessKeyboardEvent(DeviceEvent *event, DeviceIntPtr keybd)
>>>  {
>>> @@ -121,20 +129,38 @@ XkbProcessKeyboardEvent(DeviceEvent *event, DeviceIntPtr keybd)
>>>          case XkbKB_Overlay2:
>>>          {
>>>              unsigned which;
>>> +            unsigned overlay_active_now;
>>> +            unsigned is_keyrelease = (event->type == ET_KeyRelease) ? 1 : 0;
>>> +            unsigned key_was_overlaid = 0;
>>>
>>>              if (behavior.type == XkbKB_Overlay1)
>>>                  which = XkbOverlay1Mask;
>>>              else
>>>                  which = XkbOverlay2Mask;
>>> -            if ((xkbi->desc->ctrls->enabled_ctrls & which) == 0)
>>> +            overlay_active_now = (xkbi->desc->ctrls->enabled_ctrls & which) ? 1 : 0;
>>> +
>>> +            if ((unsigned char)key == key) {
>>> +                key_was_overlaid = overlay_perkey_state[key];
>>> +                if (overlay_active_now && !is_keyrelease)
>>> +                    overlay_perkey_state[key] = 2;
>>> +                else if (is_keyrelease && (overlay_active_now || key_was_overlaid))
>>> +                    overlay_perkey_state[key] = key_was_overlaid - 1;
>>> +                else if (key_was_overlaid && !overlay_active_now && !is_keyrelease) {
>>> +                    /* ignore key presses after overlay is released,
>>> +                     * as their release would have been overridden in prev branch,
>>> +                     * and key would need another key-and-release to recover from overlay
>>> +                     * */
>>> +                    return;
>>> +                } else {
>>> +                    break;
>>> +                }
>>> +            }
>>> +
>>> +            if (!overlay_active_now && !key_was_overlaid)
>>>                  break;
>>>              if ((behavior.data >= xkbi->desc->min_key_code) &&
>>>                  (behavior.data <= xkbi->desc->max_key_code)) {
>>>                  event->detail.key = behavior.data;
>>> -                /* 9/11/94 (ef) -- XXX! need to match release with */
>>> -                /*                 press even if the state of the  */
>>> -                /*                 corresponding overlay control   */
>>> -                /*                 changes while the key is down   */
>>>              }
>>>          }
>>>              break;
>>> --
>>> 2.9.2
>>>
>>>
>>> _______________________________________________
>>> xorg-devel at lists.x.org: X.Org development
>>> Archives: http://lists.x.org/archives/xorg-devel
>>> Info: https://lists.x.org/mailman/listinfo/xorg-devel


More information about the xorg-devel mailing list