AltGr key mostly fires an additional CONTROL key
Jon TURNEY
jon.turney@dronecode.org.uk
Mon Sep 5 13:35:00 GMT 2011
On 15/08/2011 13:53, Oliver Schmidt wrote:
> I also had problems with the AltGr key. These could reliably
> be reproduced by holding the AltGr for some seconds (causing
> Windows generating auto repeat events).
>
> Unfortunately the test version at
>
> ftp://cygwin.com/pub/cygwinx/XWin.20110801-git-2d9f9305cb559907.exe.bz2
>
> doesn't fix this problem for me.
>
> I discovered that the mechanism in winkeybd.c function
> winIsFakeCtrl_L had a problem if PeekMessage cannot obtain
> the next Alt_R message because it is not there.
>
> I prepared a patch that remembers the last Ctrl_L event and
> reacts on a later following Alt_R. It was also necessary to
> alter the order in winWindowProc in winwndproc.c: the invocation
> of winIsFakeCtrl_L had to be done before discarding auto-repeated
> key presses.
>
> The attached patch is against the sources of xserver-cygwin-1.10.3-1.
Thanks for the patch. The approach of remembering if the last keyevent was a
Ctrl-L and reversing it if it was fake because it's followed by a AltGr is a
good one, which hadn't occurred to me, and it clearly better than assuming the
AltGr keyevent message will be available some fixed time after the Ctrl-L one.
(I had considered buffering the Ctrl-L keyevent until we could determine a
AltGr wasn't going to be coming after it, but that would be more complex to
implement)
A few comments on the patch follow. It could also do with some better
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)
> diff --git a/hw/xwin/winkeybd.c b/hw/xwin/winkeybd.c
> index e807fc5..460c9d6 100644
> --- a/hw/xwin/winkeybd.c
> +++ b/hw/xwin/winkeybd.c
> @@ -356,6 +356,12 @@ winIsFakeCtrl_L (UINT message, WPARAM wParam, LPARAM lParam)
> MSG msgNext;
> LONG lTime;
> Bool fReturn;
> +
> + 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...
> /*
> * Fake Ctrl_L presses will be followed by an Alt_R keypress
> @@ -389,9 +395,22 @@ winIsFakeCtrl_L (UINT message, WPARAM wParam, LPARAM lParam)
> WM_KEYDOWN, WM_SYSKEYDOWN,
> PM_NOREMOVE);
> }
> - if (msgNext.message != WM_KEYDOWN && msgNext.message != WM_SYSKEYDOWN)
> + if (fReturn && msgNext.message != WM_KEYDOWN && msgNext.message != WM_SYSKEYDOWN)
> fReturn = 0;
> + if (!fReturn)
> + {
> + hasLastControlL = TRUE;
> + lastMessage = message;
> + lastWparam = wParam;
> + lastLparam = lParam;
> + lastTime = lTime;
> + }
> + else
> + {
> + hasLastControlL = FALSE;
> + }
> +
> /* Is next press an Alt_R with the same timestamp? */
> if (fReturn && msgNext.wParam == VK_MENU
> && msgNext.time == lTime
> @@ -406,11 +425,33 @@ winIsFakeCtrl_L (UINT message, WPARAM wParam, LPARAM lParam)
> }
> }
>
> + /*
> + * Check for Alt_R keypress, that was not ready when the
> + * last Ctrl_L appeared.
> + */
> + else if ((message == WM_KEYDOWN || message == WM_SYSKEYDOWN)
> + && wParam == VK_MENU
> + && (HIWORD (lParam) & KF_EXTENDED))
> + {
> + if (hasLastControlL)
> + {
> + lTime = GetMessageTime ();
> +
> + 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?
> + {
> + /* take back the fake ctrl_L key */
> + winSendKeyEvent (KEY_LCtrl, FALSE);
> + }
> + hasLastControlL = FALSE;
> + }
> + }
> +
> /*
> * Fake Ctrl_L releases will be followed by an Alt_R release
> * with the same timestamp as the Ctrl_L release.
> */
> - if ((message == WM_KEYUP || message == WM_SYSKEYUP)
> + else if ((message == WM_KEYUP || message == WM_SYSKEYUP)
> && wParam == VK_CONTROL
> && (HIWORD (lParam) & KF_EXTENDED) == 0)
> {
> @@ -439,9 +480,11 @@ winIsFakeCtrl_L (UINT message, WPARAM wParam, LPARAM lParam)
> PM_NOREMOVE);
> }
>
> - if (msgNext.message != WM_KEYUP && msgNext.message != WM_SYSKEYUP)
> + if (fReturn && msgNext.message != WM_KEYUP && msgNext.message != WM_SYSKEYUP)
> fReturn = 0;
>
> + hasLastControlL = FALSE;
> +
> /* Is next press an Alt_R with the same timestamp? */
> if (fReturn
> && (msgNext.message == WM_KEYUP
> @@ -458,6 +501,10 @@ winIsFakeCtrl_L (UINT message, WPARAM wParam, LPARAM lParam)
> return TRUE;
> }
> }
> + else
> + {
> + hasLastControlL = FALSE;
> + }
>
> /* Not a fake control left press/release */
> return FALSE;
> diff --git a/hw/xwin/winwndproc.c b/hw/xwin/winwndproc.c
> index 316cf08..7de5a5d 100644
> --- a/hw/xwin/winwndproc.c
> +++ b/hw/xwin/winwndproc.c
> @@ -1060,6 +1060,10 @@ winWindowProc (HWND hwnd, UINT message,
> if ((wParam == VK_LWIN || wParam == VK_RWIN) && !g_fKeyboardHookLL)
> break;
>
> + /* Discard fake Ctrl_L presses that precede AltGR on non-US keyboards */
> + if (winIsFakeCtrl_L (message, wParam, lParam))
> + return 0;
> +
> /*
> * Discard presses generated from Windows auto-repeat
> */
> @@ -1080,10 +1084,6 @@ winWindowProc (HWND hwnd, UINT message,
> }
> }
>
> - /* 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 :-)
> /* Translate Windows key code to X scan code */
> winTranslateKey (wParam, lParam, &iScanCode);
>
>
--
Jon TURNEY
Volunteer Cygwin/X X Server maintainer
--
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/
More information about the Cygwin-xfree
mailing list