Patch for keyboard handling

Patch for keyboard handling

[Takuma Murakami]

[-- Attachment #1: Type: text/plain, Size: 647 bytes --]

I have made a patch to improve keyboard handling.
Any comments would be appreciated.

The changes are:

1) win.h, winkeybd.c, winwndproc.c - Improve the
synchronization of mode key states between XWin
and Windows.

2) winmultiwindowwndproc.c - Enable mode key
synchronization in -multiwindow mode.

3) winwndproc.c - Make clean termination on
logoff or shutdown.

4) winconfig.c - Fix the lacks of KEYUP messages in
Japanese environments.  The solution was proposed
by Kensuke Matsuzaki.

5) winwndproc.c - Ignore Windows keyboard auto-repeats
so that XWin controls auto-repeats instead of Windows.

Takuma Murakami (murakami@ipl.t.u-tokyo.ac.jp)

[-- Attachment #2: xwin.kbd.patch --]
[-- Type: application/octet-stream, Size: 10352 bytes --]

Index: win.h
===================================================================
RCS file: /cvsroot/xoncygwin/xc/programs/Xserver/hw/xwin/win.h,v
retrieving revision 1.5
diff -c -r1.5 win.h
*** win.h	4 Oct 2003 18:44:30 -0000	1.5
--- win.h	3 Nov 2003 07:26:57 -0000
***************
*** 435,442 ****
    DWORD			dwBlueMask;
    DWORD			dwBitsPerRGB;
  
-   DWORD			dwModeKeyStates;
- 
    /* Handle to icons that must be freed */
    HICON			hiconNotifyIcon;
  
--- 435,440 ----
***************
*** 944,953 ****
  winInitializeModeKeyStates (void);
  
  void
! winStoreModeKeyStates (ScreenPtr pScreen);
! 
! void
! winRestoreModeKeyStates (ScreenPtr pScreen);
  
  Bool
  winIsFakeCtrl_L (UINT message, WPARAM wParam, LPARAM lParam);
--- 942,948 ----
  winInitializeModeKeyStates (void);
  
  void
! winRestoreModeKeyStates (void);
  
  Bool
  winIsFakeCtrl_L (UINT message, WPARAM wParam, LPARAM lParam);
Index: winconfig.c
===================================================================
RCS file: /cvsroot/xoncygwin/xc/programs/Xserver/hw/xwin/winconfig.c,v
retrieving revision 1.12
diff -c -r1.12 winconfig.c
*** winconfig.c	8 Oct 2003 10:49:15 -0000	1.12
--- winconfig.c	3 Nov 2003 07:26:58 -0000
***************
*** 309,314 ****
--- 309,326 ----
  	g_winInfo.xkb.options = pLayout->xkboptions; 
  	break;
        }
+ 
+     if ((layoutNum == 0x411) && keyboardType == 7)
+       {
+ 	/* Japanese layouts have problems with key event messages
+ 	   such as the lack of WM_KEYUP for Caps Lock key.
+ 	   Loading US layout fixes this problem. */
+ 	if (LoadKeyboardLayout("00000409", KLF_ACTIVATE) != NULL)
+ 	  winMsg (X_INFO, "Loading US keyboard layout.\n");
+ 	else
+ 	  winMsg (X_ERROR, "LoadKeyboardLaout failed.\n");
+       }
+ 	   
    }  
    
    g_winInfo.xkb.initialMap = NULL;
Index: winkeybd.c
===================================================================
RCS file: /cvsroot/xoncygwin/xc/programs/Xserver/hw/xwin/winkeybd.c,v
retrieving revision 1.2
diff -c -r1.2 winkeybd.c
*** winkeybd.c	15 Sep 2003 18:14:05 -0000	1.2
--- winkeybd.c	3 Nov 2003 07:26:59 -0000
***************
*** 44,49 ****
--- 44,51 ----
  
  static Bool g_winKeyState[NUM_KEYCODES];
  
+ /* Stored to get internal mode key states.  Must be read-only.  */
+ static unsigned short *g_winInternalModeKeyStatesPtr = NULL;
  
  #if WIN_NEW_KEYBOARD_SUPPORT
  
***************
*** 415,421 ****
--- 417,425 ----
  				       modMap, winKeybdBell, winKeybdCtrl);
  	}
  #endif
+       g_winInternalModeKeyStatesPtr = &(pDeviceInt->key->state);
        break;
+       
      case DEVICE_ON: 
        pDevice->on = TRUE;
        break;
***************
*** 473,519 ****
  
  
  /*
-  * We have to store the last state of each mode
-  * key before we lose the keyboard focus.
-  */
- 
- void
- winStoreModeKeyStates (ScreenPtr pScreen)
- {
- #if !WIN_NEW_KEYBOARD_SUPPORT
-   winScreenPriv(pScreen);
- 
-   /* Initialize all mode key states to off */
-   pScreenPriv->dwModeKeyStates = 0x0L;
- 
-   pScreenPriv->dwModeKeyStates |= 
-     (GetKeyState (VK_NUMLOCK) & 0x0001) << NumLockMapIndex;
- 
-   pScreenPriv->dwModeKeyStates |=
-     (GetKeyState (VK_SCROLL) & 0x0001) << ScrollLockMapIndex;
- 
-   pScreenPriv->dwModeKeyStates |=
-     (GetKeyState (VK_CAPITAL) & 0x0001) << LockMapIndex;
- 
-   pScreenPriv->dwModeKeyStates |=
-     (GetKeyState (VK_KANA) & 0x0001) << KanaMapIndex;
- #endif
- }
- 
- 
- /*
   * Upon regaining the keyboard focus we must
   * resynchronize our internal mode key states
   * with the actual state of the keys.
   */
  
  void
! winRestoreModeKeyStates (ScreenPtr pScreen)
  {
  #if !WIN_NEW_KEYBOARD_SUPPORT
-   winScreenPriv(pScreen);
    DWORD			dwKeyState;
  
    /* 
     * NOTE: The C XOR operator, ^, will not work here because it is
     * a bitwise operator, not a logical operator.  C does not
--- 477,501 ----
  
  
  /*
   * Upon regaining the keyboard focus we must
   * resynchronize our internal mode key states
   * with the actual state of the keys.
   */
  
  void
! winRestoreModeKeyStates ()
  {
  #if !WIN_NEW_KEYBOARD_SUPPORT
    DWORD			dwKeyState;
+   unsigned short internalKeyStates;
  
+   /* X server is being initialized */
+   if (!g_winInternalModeKeyStatesPtr)
+     return;
+   
+   /* Read the mode key states of our X server */
+   internalKeyStates = *g_winInternalModeKeyStatesPtr;
+   
    /* 
     * NOTE: The C XOR operator, ^, will not work here because it is
     * a bitwise operator, not a logical operator.  C does not
***************
*** 522,528 ****
  
    /* Has the key state changed? */
    dwKeyState = GetKeyState (VK_NUMLOCK) & 0x0001;
!   if (WIN_XOR (pScreenPriv->dwModeKeyStates & NumLockMask, dwKeyState))
      {
        winSendKeyEvent (KEY_NumLock, TRUE);
        winSendKeyEvent (KEY_NumLock, FALSE);
--- 504,510 ----
  
    /* Has the key state changed? */
    dwKeyState = GetKeyState (VK_NUMLOCK) & 0x0001;
!   if (WIN_XOR (internalKeyStates & NumLockMask, dwKeyState))
      {
        winSendKeyEvent (KEY_NumLock, TRUE);
        winSendKeyEvent (KEY_NumLock, FALSE);
***************
*** 530,536 ****
  
    /* Has the key state changed? */
    dwKeyState = GetKeyState (VK_CAPITAL) & 0x0001;
!   if (WIN_XOR (pScreenPriv->dwModeKeyStates & LockMask, dwKeyState))
      {
        winSendKeyEvent (KEY_CapsLock, TRUE);
        winSendKeyEvent (KEY_CapsLock, FALSE);
--- 512,518 ----
  
    /* Has the key state changed? */
    dwKeyState = GetKeyState (VK_CAPITAL) & 0x0001;
!   if (WIN_XOR (internalKeyStates & LockMask, dwKeyState))
      {
        winSendKeyEvent (KEY_CapsLock, TRUE);
        winSendKeyEvent (KEY_CapsLock, FALSE);
***************
*** 538,544 ****
  
    /* Has the key state changed? */
    dwKeyState = GetKeyState (VK_SCROLL) & 0x0001;
!   if (WIN_XOR (pScreenPriv->dwModeKeyStates & ScrollLockMask, dwKeyState))
      {
        winSendKeyEvent (KEY_ScrollLock, TRUE);
        winSendKeyEvent (KEY_ScrollLock, FALSE);
--- 520,526 ----
  
    /* Has the key state changed? */
    dwKeyState = GetKeyState (VK_SCROLL) & 0x0001;
!   if (WIN_XOR (internalKeyStates & ScrollLockMask, dwKeyState))
      {
        winSendKeyEvent (KEY_ScrollLock, TRUE);
        winSendKeyEvent (KEY_ScrollLock, FALSE);
***************
*** 546,552 ****
  
    /* Has the key state changed? */
    dwKeyState = GetKeyState (VK_KANA) & 0x0001;
!   if (WIN_XOR (pScreenPriv->dwModeKeyStates & KanaMask, dwKeyState))
      {
        winSendKeyEvent (KEY_HKTG, TRUE);
        winSendKeyEvent (KEY_HKTG, FALSE);
--- 528,534 ----
  
    /* Has the key state changed? */
    dwKeyState = GetKeyState (VK_KANA) & 0x0001;
!   if (WIN_XOR (internalKeyStates & KanaMask, dwKeyState))
      {
        winSendKeyEvent (KEY_HKTG, TRUE);
        winSendKeyEvent (KEY_HKTG, FALSE);
Index: winmouse.c
===================================================================
RCS file: /cvsroot/xoncygwin/xc/programs/Xserver/hw/xwin/winmouse.c,v
retrieving revision 1.1.1.2
diff -c -r1.1.1.2 winmouse.c
*** winmouse.c	3 Jun 2003 11:12:55 -0000	1.1.1.2
--- winmouse.c	3 Nov 2003 07:26:59 -0000
***************
*** 37,42 ****
--- 37,43 ----
  void
  winMouseCtrl (DeviceIntPtr pDevice, PtrCtrl *pCtrl)
  {
+ 
  }
  
  
Index: winmultiwindowwndproc.c
===================================================================
RCS file: /cvsroot/xoncygwin/xc/programs/Xserver/hw/xwin/winmultiwindowwndproc.c,v
retrieving revision 1.3
diff -c -r1.3 winmultiwindowwndproc.c
*** winmultiwindowwndproc.c	4 Oct 2003 18:44:30 -0000	1.3
--- winmultiwindowwndproc.c	3 Nov 2003 07:27:00 -0000
***************
*** 599,609 ****
        SendMessage (hwndScreen, message, wParam, lParam);
        return 0;
  
      case WM_KILLFOCUS:
        /* Pop any pressed keys since we are losing keyboard focus */
        winKeybdReleaseKeys ();
        return 0;
! 
      case WM_SYSDEADCHAR:      
      case WM_DEADCHAR:
        /*
--- 599,616 ----
        SendMessage (hwndScreen, message, wParam, lParam);
        return 0;
  
+     case WM_SETFOCUS:
+       if (s_pScreenPriv == NULL || s_pScreenInfo->fIgnoreInput)
+ 	break;
+ 
+       winRestoreModeKeyStates ();
+       return 0;
+       
      case WM_KILLFOCUS:
        /* Pop any pressed keys since we are losing keyboard focus */
        winKeybdReleaseKeys ();
        return 0;
!       
      case WM_SYSDEADCHAR:      
      case WM_DEADCHAR:
        /*
Index: winwndproc.c
===================================================================
RCS file: /cvsroot/xoncygwin/xc/programs/Xserver/hw/xwin/winwndproc.c,v
retrieving revision 1.2
diff -c -r1.2 winwndproc.c
*** winwndproc.c	15 Sep 2003 18:14:05 -0000	1.2
--- winwndproc.c	3 Nov 2003 07:27:01 -0000
***************
*** 118,126 ****
        s_hwndLastPrivates = hwnd;
        SetProp (hwnd, WIN_SCR_PROP, s_pScreenPriv);
  
-       /* Store the mode key states so restore doesn't try to restore them */
-       winStoreModeKeyStates (s_pScreen);
- 
        /* Setup tray icon */
        if (!s_pScreenInfo->fNoTrayIcon)
  	{
--- 118,123 ----
***************
*** 837,852 ****
  	break;
  
        /* Restore the state of all mode keys */
!       winRestoreModeKeyStates (s_pScreen);
        return 0;
  
      case WM_KILLFOCUS:
        if (s_pScreenPriv == NULL || s_pScreenInfo->fIgnoreInput)
  	break;
  
-       /* Store the state of all mode keys */
-       winStoreModeKeyStates (s_pScreen);
- 
        /* Release any pressed keys */
        winKeybdReleaseKeys ();
        return 0;
--- 834,846 ----
  	break;
  
        /* Restore the state of all mode keys */
!       winRestoreModeKeyStates ();
        return 0;
  
      case WM_KILLFOCUS:
        if (s_pScreenPriv == NULL || s_pScreenInfo->fIgnoreInput)
  	break;
  
        /* Release any pressed keys */
        winKeybdReleaseKeys ();
        return 0;
***************
*** 911,916 ****
--- 905,914 ----
        if (wParam == VK_LWIN || wParam == VK_RWIN)
  	break;
  
+       /* Discard presses generated from Windows auto-repeat */
+       if (lParam & (1<<30))
+ 	return 0;
+       
        /* Discard fake Ctrl_L presses that precede AltGR on non-US keyboards */
        if (winIsFakeCtrl_L (message, wParam, lParam))
  	return 0;
***************
*** 1055,1060 ****
--- 1053,1059 ----
  	}
        break;
  
+     case WM_ENDSESSION:
      case WM_GIVEUP:
         /* Tell X that we are giving up */
        winDeinitClipboard ();

Re: Patch for keyboard handling

[Harold L Hunt II]

Takuma Murakami wrote:

> I have made a patch to improve keyboard handling.
> Any comments would be appreciated.
> 
> The changes are:
> 
> 1) win.h, winkeybd.c, winwndproc.c - Improve the
> synchronization of mode key states between XWin
> and Windows.

+ /* Stored to get internal mode key states.  Must be read-only.  */
+ static unsigned short *g_winInternalModeKeyStatesPtr = NULL;

Shouldn't this be a pointer to constant data?  Isn't that:

static unsigned short const * g_winInternalModeKeyStatesPtr = NULL;

???

> 2) winmultiwindowwndproc.c - Enable mode key
> synchronization in -multiwindow mode.

+       g_winInternalModeKeyStatesPtr = &(pDeviceInt->key->state);

Wow!  That is a really good idea.  I should have been doing that all 
along, but I didn't realize that I could access the internal mode-key 
states.  Great idea.

> 3) winwndproc.c - Make clean termination on
> logoff or shutdown.

Good catch for WM_ENDSESSION.  That should have been there all along...

> 4) winconfig.c - Fix the lacks of KEYUP messages in
> Japanese environments.  The solution was proposed
> by Kensuke Matsuzaki.

Looks good to me.  It all depends on if it works for you guys.

> 5) winwndproc.c - Ignore Windows keyboard auto-repeats
> so that XWin controls auto-repeats instead of Windows.

Okay.

I will try to apply this patch tomorrow.

Thanks for contributing,

Harold

Re: Patch for keyboard handling

[Takuma Murakami]

> + /* Stored to get internal mode key states.  Must be read-only.  */
> + static unsigned short *g_winInternalModeKeyStatesPtr = NULL;
> 
> Shouldn't this be a pointer to constant data?  Isn't that:
> 
> static unsigned short const * g_winInternalModeKeyStatesPtr = NULL;
> 
> ???

Exactly.  That's what I wanted to do.
Thank you for pointing out.

Takuma Murakami (murakami@ipl.t.u-tokyo.ac.jp)

Re: Patch for keyboard handling

[Harold L Hunt II]

Takuma Murakami wrote:
> I have made a patch to improve keyboard handling.
> Any comments would be appreciated.
> 
> The changes are:
> 
> 1) win.h, winkeybd.c, winwndproc.c - Improve the
> synchronization of mode key states between XWin
> and Windows.
> 
> 2) winmultiwindowwndproc.c - Enable mode key
> synchronization in -multiwindow mode.

Actually, now I am a little doubtful that this patch is complete.  If I 
recall correctly, we are merely enqueueing input events into a queue 
that the mi layer processes later.  Checking the mode key states within 
the X server will only indicate what the mi layer currently knows about 
the mode key states.  The my layer would not know that there are 
messages in its queue that change the state of the mode keys.

I also recall that there is a way to force the mi layer to process all 
input events (miProcessInputEvents ?) and that this could be called 
before querying the state of the mode key states in order to get a 
consistent result.

Please respond to this either with a rebuttal or a different patch.  I 
don't think I can commit the existing patch until then.

Harold

Re: Patch for keyboard handling

[Takuma Murakami]

[-- Attachment #1: Type: text/plain, Size: 1350 bytes --]

Harold,

> Actually, now I am a little doubtful that this patch is complete.  If I 
> recall correctly, we are merely enqueueing input events into a queue 
> that the mi layer processes later.  Checking the mode key states within 
> the X server will only indicate what the mi layer currently knows about 
> the mode key states.  The my layer would not know that there are 
> messages in its queue that change the state of the mode keys.

I agree with you.  If the mi event queue has pending
events when winRestoreModeKeyStates is called, it may
fail to synchronize mode key states between XWin and
Windows.  However, the new code can re-sync them at
the next time winRestoreModeKeyStates is called while
it is hard to do for the old code.

> I also recall that there is a way to force the mi layer to process all 
> input events (miProcessInputEvents ?) and that this could be called 
> before querying the state of the mode key states in order to get a 
> consistent result.

Thank you for your suggestion.  mieqProcessInputEvents
seems to do the work.  I inserted a code to call it
and it looks working well for now, though I don't sure
if it is legal to call it from the place other than
ProcessInputEvents.

The attached is a revised patch to call the function.
I deeply appreciate your feedback.

Takuma Murakami (murakami@ipl.t.u-tokyo.ac.jp)

[-- Attachment #2: xwin.kbd2.patch --]
[-- Type: application/octet-stream, Size: 9965 bytes --]

Index: win.h
===================================================================
RCS file: /cvsroot/xoncygwin/xc/programs/Xserver/hw/xwin/win.h,v
retrieving revision 1.5
diff -c -r1.5 win.h
*** win.h	4 Oct 2003 18:44:30 -0000	1.5
--- win.h	4 Nov 2003 08:24:55 -0000
***************
*** 435,442 ****
    DWORD			dwBlueMask;
    DWORD			dwBitsPerRGB;
  
-   DWORD			dwModeKeyStates;
- 
    /* Handle to icons that must be freed */
    HICON			hiconNotifyIcon;
  
--- 435,440 ----
***************
*** 944,953 ****
  winInitializeModeKeyStates (void);
  
  void
! winStoreModeKeyStates (ScreenPtr pScreen);
! 
! void
! winRestoreModeKeyStates (ScreenPtr pScreen);
  
  Bool
  winIsFakeCtrl_L (UINT message, WPARAM wParam, LPARAM lParam);
--- 942,948 ----
  winInitializeModeKeyStates (void);
  
  void
! winRestoreModeKeyStates (void);
  
  Bool
  winIsFakeCtrl_L (UINT message, WPARAM wParam, LPARAM lParam);
Index: winconfig.c
===================================================================
RCS file: /cvsroot/xoncygwin/xc/programs/Xserver/hw/xwin/winconfig.c,v
retrieving revision 1.12
diff -c -r1.12 winconfig.c
*** winconfig.c	8 Oct 2003 10:49:15 -0000	1.12
--- winconfig.c	4 Nov 2003 08:24:56 -0000
***************
*** 309,314 ****
--- 309,326 ----
  	g_winInfo.xkb.options = pLayout->xkboptions; 
  	break;
        }
+ 
+     if ((layoutNum == 0x411) && keyboardType == 7)
+       {
+ 	/* Japanese layouts have problems with key event messages
+ 	   such as the lack of WM_KEYUP for Caps Lock key.
+ 	   Loading US layout fixes this problem. */
+ 	if (LoadKeyboardLayout("00000409", KLF_ACTIVATE) != NULL)
+ 	  winMsg (X_INFO, "Loading US keyboard layout.\n");
+ 	else
+ 	  winMsg (X_ERROR, "LoadKeyboardLaout failed.\n");
+       }
+ 	   
    }  
    
    g_winInfo.xkb.initialMap = NULL;
Index: winkeybd.c
===================================================================
RCS file: /cvsroot/xoncygwin/xc/programs/Xserver/hw/xwin/winkeybd.c,v
retrieving revision 1.2
diff -c -r1.2 winkeybd.c
*** winkeybd.c	15 Sep 2003 18:14:05 -0000	1.2
--- winkeybd.c	4 Nov 2003 08:24:56 -0000
***************
*** 44,49 ****
--- 44,51 ----
  
  static Bool g_winKeyState[NUM_KEYCODES];
  
+ /* Stored to get internal mode key states.  Must be read-only.  */
+ static unsigned short const *g_winInternalModeKeyStatesPtr = NULL;
  
  #if WIN_NEW_KEYBOARD_SUPPORT
  
***************
*** 415,421 ****
--- 417,425 ----
  				       modMap, winKeybdBell, winKeybdCtrl);
  	}
  #endif
+       g_winInternalModeKeyStatesPtr = &(pDeviceInt->key->state);
        break;
+       
      case DEVICE_ON: 
        pDevice->on = TRUE;
        break;
***************
*** 473,519 ****
  
  
  /*
-  * We have to store the last state of each mode
-  * key before we lose the keyboard focus.
-  */
- 
- void
- winStoreModeKeyStates (ScreenPtr pScreen)
- {
- #if !WIN_NEW_KEYBOARD_SUPPORT
-   winScreenPriv(pScreen);
- 
-   /* Initialize all mode key states to off */
-   pScreenPriv->dwModeKeyStates = 0x0L;
- 
-   pScreenPriv->dwModeKeyStates |= 
-     (GetKeyState (VK_NUMLOCK) & 0x0001) << NumLockMapIndex;
- 
-   pScreenPriv->dwModeKeyStates |=
-     (GetKeyState (VK_SCROLL) & 0x0001) << ScrollLockMapIndex;
- 
-   pScreenPriv->dwModeKeyStates |=
-     (GetKeyState (VK_CAPITAL) & 0x0001) << LockMapIndex;
- 
-   pScreenPriv->dwModeKeyStates |=
-     (GetKeyState (VK_KANA) & 0x0001) << KanaMapIndex;
- #endif
- }
- 
- 
- /*
   * Upon regaining the keyboard focus we must
   * resynchronize our internal mode key states
   * with the actual state of the keys.
   */
  
  void
! winRestoreModeKeyStates (ScreenPtr pScreen)
  {
  #if !WIN_NEW_KEYBOARD_SUPPORT
-   winScreenPriv(pScreen);
    DWORD			dwKeyState;
  
    /* 
     * NOTE: The C XOR operator, ^, will not work here because it is
     * a bitwise operator, not a logical operator.  C does not
--- 477,503 ----
  
  
  /*
   * Upon regaining the keyboard focus we must
   * resynchronize our internal mode key states
   * with the actual state of the keys.
   */
  
  void
! winRestoreModeKeyStates ()
  {
  #if !WIN_NEW_KEYBOARD_SUPPORT
    DWORD			dwKeyState;
+   unsigned short internalKeyStates;
+ 
+   /* X server is being initialized */
+   if (!g_winInternalModeKeyStatesPtr)
+     return;
+   
+   mieqProcessInputEvents ();
  
+   /* Read the mode key states of our X server */
+   internalKeyStates = *g_winInternalModeKeyStatesPtr;
+   
    /* 
     * NOTE: The C XOR operator, ^, will not work here because it is
     * a bitwise operator, not a logical operator.  C does not
***************
*** 522,528 ****
  
    /* Has the key state changed? */
    dwKeyState = GetKeyState (VK_NUMLOCK) & 0x0001;
!   if (WIN_XOR (pScreenPriv->dwModeKeyStates & NumLockMask, dwKeyState))
      {
        winSendKeyEvent (KEY_NumLock, TRUE);
        winSendKeyEvent (KEY_NumLock, FALSE);
--- 506,512 ----
  
    /* Has the key state changed? */
    dwKeyState = GetKeyState (VK_NUMLOCK) & 0x0001;
!   if (WIN_XOR (internalKeyStates & NumLockMask, dwKeyState))
      {
        winSendKeyEvent (KEY_NumLock, TRUE);
        winSendKeyEvent (KEY_NumLock, FALSE);
***************
*** 530,536 ****
  
    /* Has the key state changed? */
    dwKeyState = GetKeyState (VK_CAPITAL) & 0x0001;
!   if (WIN_XOR (pScreenPriv->dwModeKeyStates & LockMask, dwKeyState))
      {
        winSendKeyEvent (KEY_CapsLock, TRUE);
        winSendKeyEvent (KEY_CapsLock, FALSE);
--- 514,520 ----
  
    /* Has the key state changed? */
    dwKeyState = GetKeyState (VK_CAPITAL) & 0x0001;
!   if (WIN_XOR (internalKeyStates & LockMask, dwKeyState))
      {
        winSendKeyEvent (KEY_CapsLock, TRUE);
        winSendKeyEvent (KEY_CapsLock, FALSE);
***************
*** 538,544 ****
  
    /* Has the key state changed? */
    dwKeyState = GetKeyState (VK_SCROLL) & 0x0001;
!   if (WIN_XOR (pScreenPriv->dwModeKeyStates & ScrollLockMask, dwKeyState))
      {
        winSendKeyEvent (KEY_ScrollLock, TRUE);
        winSendKeyEvent (KEY_ScrollLock, FALSE);
--- 522,528 ----
  
    /* Has the key state changed? */
    dwKeyState = GetKeyState (VK_SCROLL) & 0x0001;
!   if (WIN_XOR (internalKeyStates & ScrollLockMask, dwKeyState))
      {
        winSendKeyEvent (KEY_ScrollLock, TRUE);
        winSendKeyEvent (KEY_ScrollLock, FALSE);
***************
*** 546,552 ****
  
    /* Has the key state changed? */
    dwKeyState = GetKeyState (VK_KANA) & 0x0001;
!   if (WIN_XOR (pScreenPriv->dwModeKeyStates & KanaMask, dwKeyState))
      {
        winSendKeyEvent (KEY_HKTG, TRUE);
        winSendKeyEvent (KEY_HKTG, FALSE);
--- 530,536 ----
  
    /* Has the key state changed? */
    dwKeyState = GetKeyState (VK_KANA) & 0x0001;
!   if (WIN_XOR (internalKeyStates & KanaMask, dwKeyState))
      {
        winSendKeyEvent (KEY_HKTG, TRUE);
        winSendKeyEvent (KEY_HKTG, FALSE);
Index: winmultiwindowwndproc.c
===================================================================
RCS file: /cvsroot/xoncygwin/xc/programs/Xserver/hw/xwin/winmultiwindowwndproc.c,v
retrieving revision 1.3
diff -c -r1.3 winmultiwindowwndproc.c
*** winmultiwindowwndproc.c	4 Oct 2003 18:44:30 -0000	1.3
--- winmultiwindowwndproc.c	4 Nov 2003 08:24:57 -0000
***************
*** 599,609 ****
        SendMessage (hwndScreen, message, wParam, lParam);
        return 0;
  
      case WM_KILLFOCUS:
        /* Pop any pressed keys since we are losing keyboard focus */
        winKeybdReleaseKeys ();
        return 0;
! 
      case WM_SYSDEADCHAR:      
      case WM_DEADCHAR:
        /*
--- 599,616 ----
        SendMessage (hwndScreen, message, wParam, lParam);
        return 0;
  
+     case WM_SETFOCUS:
+       if (s_pScreenPriv == NULL || s_pScreenInfo->fIgnoreInput)
+ 	break;
+ 
+       winRestoreModeKeyStates ();
+       return 0;
+       
      case WM_KILLFOCUS:
        /* Pop any pressed keys since we are losing keyboard focus */
        winKeybdReleaseKeys ();
        return 0;
!       
      case WM_SYSDEADCHAR:      
      case WM_DEADCHAR:
        /*
Index: winwndproc.c
===================================================================
RCS file: /cvsroot/xoncygwin/xc/programs/Xserver/hw/xwin/winwndproc.c,v
retrieving revision 1.2
diff -c -r1.2 winwndproc.c
*** winwndproc.c	15 Sep 2003 18:14:05 -0000	1.2
--- winwndproc.c	4 Nov 2003 08:24:58 -0000
***************
*** 118,126 ****
        s_hwndLastPrivates = hwnd;
        SetProp (hwnd, WIN_SCR_PROP, s_pScreenPriv);
  
-       /* Store the mode key states so restore doesn't try to restore them */
-       winStoreModeKeyStates (s_pScreen);
- 
        /* Setup tray icon */
        if (!s_pScreenInfo->fNoTrayIcon)
  	{
--- 118,123 ----
***************
*** 837,852 ****
  	break;
  
        /* Restore the state of all mode keys */
!       winRestoreModeKeyStates (s_pScreen);
        return 0;
  
      case WM_KILLFOCUS:
        if (s_pScreenPriv == NULL || s_pScreenInfo->fIgnoreInput)
  	break;
  
-       /* Store the state of all mode keys */
-       winStoreModeKeyStates (s_pScreen);
- 
        /* Release any pressed keys */
        winKeybdReleaseKeys ();
        return 0;
--- 834,846 ----
  	break;
  
        /* Restore the state of all mode keys */
!       winRestoreModeKeyStates ();
        return 0;
  
      case WM_KILLFOCUS:
        if (s_pScreenPriv == NULL || s_pScreenInfo->fIgnoreInput)
  	break;
  
        /* Release any pressed keys */
        winKeybdReleaseKeys ();
        return 0;
***************
*** 911,916 ****
--- 905,914 ----
        if (wParam == VK_LWIN || wParam == VK_RWIN)
  	break;
  
+       /* Discard presses generated from Windows auto-repeat */
+       if (lParam & (1<<30))
+ 	return 0;
+       
        /* Discard fake Ctrl_L presses that precede AltGR on non-US keyboards */
        if (winIsFakeCtrl_L (message, wParam, lParam))
  	return 0;
***************
*** 1055,1060 ****
--- 1053,1059 ----
  	}
        break;
  
+     case WM_ENDSESSION:
      case WM_GIVEUP:
         /* Tell X that we are giving up */
        winDeinitClipboard ();

Re: Patch for keyboard handling

[Harold L Hunt II]

Takuma,

Takuma Murakami wrote:

> I agree with you.  If the mi event queue has pending
> events when winRestoreModeKeyStates is called, it may
> fail to synchronize mode key states between XWin and
> Windows.  However, the new code can re-sync them at
> the next time winRestoreModeKeyStates is called while
> it is hard to do for the old code.

Okay.

> Thank you for your suggestion.  mieqProcessInputEvents
> seems to do the work.  I inserted a code to call it
> and it looks working well for now, though I don't sure
> if it is legal to call it from the place other than
> ProcessInputEvents.
> 
> The attached is a revised patch to call the function.
> I deeply appreciate your feedback.

Looks good, but I think you are right about some danger in calling 
mieqProcessInputEvents.

Hmm... the problem with calling ProcessInputEvents or 
mieqProcessInputEvents from our keyboard processing is that it causes 
input events to be processed one at a time, instead of in a queue.  It 
completely negates the purpose of having an input queue in the X layer.


I think I understand your original patch better now and I think that you 
were probably doing it correctly, but I can't verify that right now.  If 
this is what you were trying to do, then it probably is correct:

1) Assume that no keyboard input is in the mi queue when winWindowProc 
is called.

2) If we are getting the keyboard focus, grab the Windows mode key state 
and X mode key state, compare them, and send fake key presses to X to 
get the two states in synch.

3) Do not synch the key states anywhere else.

That would probably work because it would enqueue key messages that will 
synch the mode key states before placing normal key messages in the 
queue.  Thus, when we ask X for the mode key states we should get a 
consistent result since the input queue in X is empty.

Does that sound like what you were trying to do initially?  I got 
confused because I couldn't keep track of where all the calls to 
winRestoreModeKeyStates would up.

Let me know,

Harold

Re: Patch for keyboard handling

[Takuma Murakami]

Harold,

> I think I understand your original patch better now and I think that you 
> were probably doing it correctly, but I can't verify that right now.  If 
> this is what you were trying to do, then it probably is correct:
> 
> 1) Assume that no keyboard input is in the mi queue when winWindowProc 
> is called.
> 
> 2) If we are getting the keyboard focus, grab the Windows mode key state 
> and X mode key state, compare them, and send fake key presses to X to 
> get the two states in synch.
> 
> 3) Do not synch the key states anywhere else.
> 
> That would probably work because it would enqueue key messages that will 
> synch the mode key states before placing normal key messages in the 
> queue.  Thus, when we ask X for the mode key states we should get a 
> consistent result since the input queue in X is empty.

The all points are right.  I think you completely
understand this patch.  The essential difference between
the old code and this patch is to remember states by
ourselves or to peek in the internal states if necessary.
Moreover, the old code refers the states of Windows
which is not guaranteed to same as the XWin's states.

Hence I believe the patch is, at least, not worse than
the old code, though there remains some incompleteness.
One is the event queue you told, another is cooperation
with customized keyboards.

> Does that sound like what you were trying to do initially?  I got 
> confused because I couldn't keep track of where all the calls to 
> winRestoreModeKeyStates would up.

winRestoreModeKeyStates is called from WM_SETFOCUS
handlers in winTopLevelWindowProc (in -multiwindow
case) and winWindowProc (in other cases).  Only 2 places.

I hope this helps your review.

Takuma Murakami (murakami@ipl.t.u-tokyo.ac.jp)