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

[walter harms]

Am 05.11.2016 20:03, schrieb Mihail Konev:
> The overlay_keys array could be avoided.
> 
> Testcase:
> 
> // In the exact order:
> // - press Insert
> // - press Tilde
> // - release Insert
> // - wait
> // - release Tilde
> //
> // Keyboard input would be locked.
> 
> 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)" };
> };
> 
> Signed-off-by: Mihail Konev <k.mvc at ya.ru>
> ---
> 
> v2: remove debug print
> 
>  xkb/xkbPrKeyEv.c | 49 ++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 38 insertions(+), 11 deletions(-)
> 
> diff --git a/xkb/xkbPrKeyEv.c b/xkb/xkbPrKeyEv.c
> index f7a6b4b14306..92a10bdc6fd2 100644
> --- a/xkb/xkbPrKeyEv.c
> +++ b/xkb/xkbPrKeyEv.c
> @@ -43,6 +43,9 @@ THE USE OR PERFORMANCE OF THIS SOFTWARE.
>  
>  /***====================================================================***/
>  
> +static unsigned char overlay_in_effect[256];
> +static unsigned int overlay_keys[2][256];
> +
>  void
>  XkbProcessKeyboardEvent(DeviceEvent *event, DeviceIntPtr keybd)
>  {
> @@ -51,6 +54,7 @@ XkbProcessKeyboardEvent(DeviceEvent *event, DeviceIntPtr keybd)
>      int key;
>      XkbBehavior behavior;
>      unsigned ndx;
> +    unsigned effective_overlay = 0;
>  
>      xkbi = keyc->xkbInfo;
>      key = event->detail.key;
> @@ -68,6 +72,16 @@ XkbProcessKeyboardEvent(DeviceEvent *event, DeviceIntPtr keybd)
>      /* do anything to implement the behavior, but it *does* report that */
>      /* key is hardwired */
>  
> +    {
> +        if (key >= 0 && key < 256) {
> +            effective_overlay = overlay_in_effect[key];
> +            switch (effective_overlay) {
> +                case 1: behavior.type = XkbKB_Overlay1; break;
> +                case 2: behavior.type = XkbKB_Overlay1; break;

	this looks strange if fall truh is intended then do it.

> +            }
> +        }
> +    }

> +
>      if (!(behavior.type & XkbKB_Permanent)) {
>          switch (behavior.type) {
>          case XkbKB_Default:
> @@ -121,20 +135,33 @@ XkbProcessKeyboardEvent(DeviceEvent *event, DeviceIntPtr keybd)
>          case XkbKB_Overlay2:
>          {
>              unsigned which;
> +            unsigned which_overlay;
> +            unsigned overlay_enabled;
> +            unsigned behavior_data = behavior.data;
>  
> -            if (behavior.type == XkbKB_Overlay1)
> +            if (behavior.type == XkbKB_Overlay1) {
>                  which = XkbOverlay1Mask;
> -            else
> +                which_overlay = 1;
> +            } else {
>                  which = XkbOverlay2Mask;
> -            if ((xkbi->desc->ctrls->enabled_ctrls & which) == 0)
> -                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   */
> +                which_overlay = 2;
> +            }
> +
> +            overlay_enabled = (xkbi->desc->ctrls->enabled_ctrls & which);
> +
> +            if (!overlay_enabled && event->type == ET_KeyRelease)
> +                overlay_in_effect[(unsigned char)key] = 0;
> +            else
> +                overlay_in_effect[(unsigned char)key] = which_overlay;
> +
> +            if (overlay_enabled)
> +                overlay_keys[which_overlay][(unsigned char)key] = behavior.data;
> +            else
> +                behavior_data = overlay_keys[which_overlay][(unsigned char)key];
> +
> +            if ((behavior_data >= xkbi->desc->min_key_code) &&
> +                (behavior_data <= xkbi->desc->max_key_code)) {
> +                event->detail.key = behavior_data;
>              }
>          }

	the whole block is huge, is it possible to make it a function ?

	just my 2 cents,

	re,
 	 wh
>              break;