Re: AltGr key mostly fires an additional CONTROL key

[Oliver Schmidt <oschmidt-mailinglists@gmx.de>]

Hi Jon,

On 9/5/2011 3:35 PM, Jon TURNEY wrote:
> comments, as what this code is trying to do is slightly obscure, and I
> assume that the old comments about TweakUI being the cause of this are
> just wrong (as you don't mention that you have it installed)

Yes I didn't test it with Tweak UI and I ignored the comments on Tweak
UI. I also think that the old "sleep(0)" workaround for Tweak UI is now
not necessary with the new patch. But I kept the changes as minimal as
possible and tried to keep existing code as much as it is.

>> diff --git a/hw/xwin/winkeybd.c b/hw/xwin/winkeybd.c
>> +  static Bool   hasLastControlL = FALSE;
>> +  static UINT   lastMessage;
>> +  static WPARAM lastWparam;
>> +  static LPARAM lastLparam;
>> +  static LONG   lastTime;
> 
> I was going to suggest using static  MSG lastMsg, but then I noticed
> that lastWparam, lastLparam are completely unused...

yes, they are now unused editing relicts from my testing phase...

>> +
>> +          if ((lastMessage == WM_KEYDOWN || lastMessage ==
>> WM_SYSKEYDOWN)
>> +              && lastTime == lTime)
> 
> Why is it necessary to check that the last message was WM_(SYS)KEYDOWN
> here? hasLastControlL can't get set unless it was?

yes you are right: the if condition here is a little over-determined ;-)


>> diff --git a/hw/xwin/winwndproc.c b/hw/xwin/winwndproc.c
>> +      /* Discard fake Ctrl_L presses that precede AltGR on non-US
>> keyboards */
>> +      if (winIsFakeCtrl_L (message, wParam, lParam))
>> +    return 0;
>> +
>> -      /* Discard fake Ctrl_L presses that precede AltGR on non-US
>> keyboards */
>> -      if (winIsFakeCtrl_L (message, wParam, lParam))
>> -    return 0;
>> -
> 
> Can you say why it's necessary to change the order here and why this is
> the correct ordering?  A comment here might be a good idea :-)

in the old coding there is this check for generated Windows auto-repeat
key events and this check can cause the function to exit:

      /*
       * Discard presses generated from Windows auto-repeat
       */
      if (lParam & (1<<30))
      {
        switch (wParam)
        {
          /* ago: Pressing LControl while RControl is pressed is
           * Indicated as repeat. Fix this!
           */
          case VK_CONTROL:
          case VK_SHIFT:
            if (winCheckKeyPressed(wParam, lParam))
              return 0;
            break;
          default:
            return 0;
        }
      }

I didn't change this coding and I'm not sure what the funtion
winCheckKeyPressed exactly does. However the check winIsFakeCtrl_L must
be done before leaving the function because of auto-repeated key events.
Otherwise there will be problems with autorepeated AltGr-Keys. These
must also be tracked with winIsFakeCtrl_L otherwise autorepeated AltGr
keys could produce a "hanging" Control_L key (I had this effect when I
tested the patch).

So the order has to be changed, because the function winIsFakeCtrl_L now
has an internal state due to it's static variables and this state has to
be synchronized with the actuel key events.

Best regards,
Oliver

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://x.cygwin.com/docs/
FAQ:                   http://x.cygwin.com/docs/faq/