[prev in list] [next in list] [prev in thread] [next in thread] 

List:       freedesktop-xorg
Subject:    Re: [PATCH] Cygwin/X: Fix compilation for mandated XKB
From:       Peter Hutterer <peter.hutterer () who-t ! net>
Date:       2009-01-30 22:20:22
Message-ID: 20090130222021.GB7288 () dingo
[Download RAW message or body]

Two comments, see below.

On Fri, Jan 30, 2009 at 02:23:45PM +0000, Jon TURNEY wrote:
> Fix a stray '}'
> Update to use RMLVO interface
> Remove g_winInfo.xkb.disable, can never be set since noXkbExtension has been removed
> Change to retrieve modifier key state using XkbStateFieldFromRec() from Colin Harrison
> Update to use XKB defaults from xkb-config.h
> 
> Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
> ---
>  hw/xwin/winconfig.c      |   23 +++---------
>  hw/xwin/winconfig.h      |    1 -
>  hw/xwin/winkeybd.c       |   91 ++++++++++++++++++----------------------------
>  hw/xwin/winwndproc.c     |    3 +-
>  include/xwin-config.h.in |    1 +
>  5 files changed, 42 insertions(+), 77 deletions(-)
> 
> diff --git a/hw/xwin/winconfig.c b/hw/xwin/winconfig.c
> index 5f5b482..3091bd1 100644
> --- a/hw/xwin/winconfig.c
> +++ b/hw/xwin/winconfig.c
> @@ -81,7 +81,6 @@ winInfoRec g_winInfo = {
>     }
>    ,
>    {				/* xkb */
> -   FALSE,			/* disable */
>     NULL,			/* rules */
>     NULL,			/* model */
>     NULL,			/* layout */
> @@ -222,25 +221,14 @@ winConfigKeyboard (DeviceIntPtr pDevice)
>    char				*s = NULL;
>  
>    /* Setup defaults */
> -  g_winInfo.xkb.disable = FALSE;
> -# ifdef PC98 /* japanese */	/* not implemented */
> -  g_winInfo.xkb.rules = "xfree98";
> -  g_winInfo.xkb.model = "pc98";
> -  g_winInfo.xkb.layout = "nex/jp";
> -  g_winInfo.xkb.variant = NULL;
> -  g_winInfo.xkb.options = NULL;
> -# else
> -  g_winInfo.xkb.rules = "xorg";
> -  g_winInfo.xkb.model = "pc101";
> -  g_winInfo.xkb.layout = "us";
> -  g_winInfo.xkb.variant = NULL;
> -  g_winInfo.xkb.options = NULL;
> -# endif	/* PC98 */
> +  g_winInfo.xkb.rules = XKB_DFLT_RULES;
> +  g_winInfo.xkb.model = XKB_DFLT_MODEL;
> +  g_winInfo.xkb.layout = XKB_DFLT_LAYOUT;
> +  g_winInfo.xkb.variant = XKB_DFLT_VARIANT;
> +  g_winInfo.xkb.options = XKB_DFLT_OPTIONS;

I think it'd be better to call XkbGetRulesDflts(&g_winInfo.xkb);

>  
>    /*
>     * Query the windows autorepeat settings and change the xserver defaults.   
> -   * If XKB is disabled then windows handles the autorepeat and the special 
> -   * treatment is not needed
>     */
>    {
>      int kbd_delay;
> @@ -483,7 +471,6 @@ winConfigKeyboard (DeviceIntPtr pDevice)
>  	    winMsg (from, "XKB: options: \"%s\"\n", s);
>  	  }
>  
> -	}
>  #ifdef XWIN_XF86CONFIG
>      }
>  #endif
> diff --git a/hw/xwin/winconfig.h b/hw/xwin/winconfig.h
> index 71c4582..0e22b9d 100644
> --- a/hw/xwin/winconfig.h
> +++ b/hw/xwin/winconfig.h
> @@ -307,7 +307,6 @@ typedef struct
>    keyboard;
>    struct
>    {
> -    Bool disable;
>      char *rules;
>      char *model;
>      char *layout;
> diff --git a/hw/xwin/winkeybd.c b/hw/xwin/winkeybd.c
> index 41168f3..00ce6f4 100644
> --- a/hw/xwin/winkeybd.c
> +++ b/hw/xwin/winkeybd.c
> @@ -44,10 +44,6 @@
>  
>  static Bool g_winKeyState[NUM_KEYCODES];
>  
> -/* Stored to get internal mode key states.  Must be read-only.  */
> -static unsigned short const *g_winInternalModeKeyStatesPtr = NULL;
> -
> -
>  /*
>   * Local prototypes
>   */
> @@ -204,7 +200,6 @@ winKeybdBell (int iPercent, DeviceIntPtr pDeviceInt,
>  static void
>  winKeybdCtrl (DeviceIntPtr pDevice, KeybdCtrl *pCtrl)
>  {
> -  g_winInternalModeKeyStatesPtr = &(pDevice->key->state);
>  }
>  
>  
> @@ -216,11 +211,11 @@ winKeybdCtrl (DeviceIntPtr pDevice, KeybdCtrl *pCtrl)
>  int
>  winKeybdProc (DeviceIntPtr pDeviceInt, int iState)
>  {
> -  KeySymsRec		keySyms;
>    DevicePtr		pDevice = (DevicePtr) pDeviceInt;
>    XkbComponentNamesRec names;
>    XkbSrvInfoPtr       xkbi;
>    XkbControlsPtr      ctrl;
> +  XkbRMLVOSet rmlvo;
>  
>    switch (iState)
>      {
> @@ -230,65 +225,47 @@ winKeybdProc (DeviceIntPtr pDeviceInt, int iState)
>        /* FIXME: Maybe we should use winGetKbdLeds () here? */
>        defaultKeyboardControl.leds = g_winInfo.keyboard.leds;
>  
> -      if (g_winInfo.xkb.disable) 
> -	{
> -	  InitKeyboardDeviceStruct (pDevice,
> -				    &keySyms,
> -				    winKeybdBell,
> -				    winKeybdCtrl);
> -	} 
> -      else 
> -	{
> -
> -          names.keymap = g_winInfo.xkb.keymap;
> -          names.keycodes = g_winInfo.xkb.keycodes;
> -          names.types = g_winInfo.xkb.types;
> -          names.compat = g_winInfo.xkb.compat;
> -          names.symbols = g_winInfo.xkb.symbols;
> -          names.geometry = g_winInfo.xkb.geometry;
> -
> -	  winErrorFVerb(2, "Rules = \"%s\" Model = \"%s\" Layout = \"%s\""
> -		 " Variant = \"%s\" Options = \"%s\"\n",
> -		 g_winInfo.xkb.rules ? g_winInfo.xkb.rules : "none",
> -		 g_winInfo.xkb.model ? g_winInfo.xkb.model : "none",
> -		 g_winInfo.xkb.layout ? g_winInfo.xkb.layout : "none",
> -		 g_winInfo.xkb.variant ? g_winInfo.xkb.variant : "none",
> -		 g_winInfo.xkb.options ? g_winInfo.xkb.options : "none");
> -          
> -	  XkbSetRulesDflts (g_winInfo.xkb.rules, g_winInfo.xkb.model, 
> -			    g_winInfo.xkb.layout, g_winInfo.xkb.variant, 
> -			    g_winInfo.xkb.options);
> -	  XkbInitKeyboardDeviceStruct (pDeviceInt, &names, &keySyms,
> -				       winKeybdBell, winKeybdCtrl);
> -	}
> -
> -      if (!g_winInfo.xkb.disable)
> -        {  
> -          xkbi = pDeviceInt->key->xkbInfo;
> -          if (xkbi != NULL)
> -            {  
> -              ctrl = xkbi->desc->ctrls;
> -              ctrl->repeat_delay = g_winInfo.keyboard.delay;
> -              ctrl->repeat_interval = 1000/g_winInfo.keyboard.rate;
> -            }
> -          else
> -            {  
> -              winErrorFVerb (1, "winKeybdProc - Error initializing keyboard AutoRepeat (No XKB)\n");
> -            }
> +      memset(&rmlvo, 0, sizeof(rmlvo));

arguably superfluous, since you set everything anyway.

> +      rmlvo.rules = g_winInfo.xkb.rules;
> +      rmlvo.model = g_winInfo.xkb.model;
> +      rmlvo.layout = g_winInfo.xkb.layout;
> +      rmlvo.variant = g_winInfo.xkb.variant;
> +      rmlvo.options = g_winInfo.xkb.options;
> +
> +      winErrorFVerb(2, "Rules = \"%s\" Model = \"%s\" Layout = \"%s\""
> +                    " Variant = \"%s\" Options = \"%s\"\n",
> +                    g_winInfo.xkb.rules ? g_winInfo.xkb.rules : "none",
> +                    g_winInfo.xkb.model ? g_winInfo.xkb.model : "none",
> +                    g_winInfo.xkb.layout ? g_winInfo.xkb.layout : "none",
> +                    g_winInfo.xkb.variant ? g_winInfo.xkb.variant : "none",
> +                    g_winInfo.xkb.options ? g_winInfo.xkb.options : "none");
> +
> +      InitKeyboardDeviceStruct (pDeviceInt,
> +                                &rmlvo,
> +                                winKeybdBell,
> +                                winKeybdCtrl);
> +
> +      xkbi = pDeviceInt->key->xkbInfo;
> +      if ((xkbi != NULL) && (xkbi->desc != NULL))
> +        {
> +          ctrl = xkbi->desc->ctrls;
> +          ctrl->repeat_delay = g_winInfo.keyboard.delay;
> +          ctrl->repeat_interval = 1000/g_winInfo.keyboard.rate;
> +        }
> +      else
> +        {
> +          winErrorFVerb (1, "winKeybdProc - Error initializing keyboard AutoRepeat\n");
>          }
>  
> -      g_winInternalModeKeyStatesPtr = &(pDeviceInt->key->state);
>        break;
>        
>      case DEVICE_ON: 
>        pDevice->on = TRUE;
> -      g_winInternalModeKeyStatesPtr = &(pDeviceInt->key->state);
>        break;
>  
>      case DEVICE_CLOSE:
>      case DEVICE_OFF: 
>        pDevice->on = FALSE;
> -      g_winInternalModeKeyStatesPtr = NULL;
>        break;
>      }
>  
> @@ -350,7 +327,7 @@ winRestoreModeKeyStates ()
>    unsigned short	internalKeyStates;
>  
>    /* X server is being initialized */
> -  if (!g_winInternalModeKeyStatesPtr)
> +  if (!inputInfo.keyboard)
>      return;
>  
>    /* Only process events if the rootwindow is mapped. The keyboard events
> @@ -363,7 +340,9 @@ winRestoreModeKeyStates ()
>      mieqProcessInputEvents ();
>    
>    /* Read the mode key states of our X server */
> -  internalKeyStates = *g_winInternalModeKeyStatesPtr;
> +  /* (stored in the virtual core keyboard) */
> +  internalKeyStates = XkbStateFieldFromRec(&inputInfo.keyboard->key->xkbInfo->state);
> +  winDebug("winRestoreModeKeyStates: state %d\n", internalKeyStates);
>  
>    /* 
>     * NOTE: The C XOR operator, ^, will not work here because it is
> diff --git a/hw/xwin/winwndproc.c b/hw/xwin/winwndproc.c
> index 36cde35..a942fb3 100644
> --- a/hw/xwin/winwndproc.c
> +++ b/hw/xwin/winwndproc.c
> @@ -1031,9 +1031,8 @@ winWindowProc (HWND hwnd, UINT message,
>  
>        /* 
>         * Discard presses generated from Windows auto-repeat
> -       * ago: Only discard them if XKB is not disabled 
>         */
> -      if (!g_winInfo.xkb.disable && (lParam & (1<<30)))
> +      if (lParam & (1<<30))
>        {
>          switch (wParam)
>          {
> diff --git a/include/xwin-config.h.in b/include/xwin-config.h.in
> index c8de110..45dbb00 100644
> --- a/include/xwin-config.h.in
> +++ b/include/xwin-config.h.in
> @@ -5,6 +5,7 @@
>   *
>   */
>  #include <dix-config.h>
> +#include <xkb-config.h>
>  
>  /* Winsock networking */
>  #undef HAS_WINSOCK
> -- 
> 1.6.0.4
>

Rest looks OK.

btw. do you have a fdo account or do you need someone to push it? If the
latter, please tell me (and make sure you CC me on the email).

Cheers,
  Peter
_______________________________________________
xorg mailing list
xorg@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/xorg
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic