[PATCH v6 0/5] MR10779: winex11: Keyboard layouts rework, part 2
-- v6: winex11: Store Xkb keyboard layout id. winex11: Store Xkb keyboard layout KLID. winex11: Keep a list of every known Xkb keyboard layout. win32u: Simplify layout name generation for base keyboard layouts. win32u: Tweak a couple of comments. https://gitlab.winehq.org/wine/wine/-/merge_requests/10779
From: Matteo Bruni <mbruni@codeweavers.com> --- dlls/win32u/input.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/dlls/win32u/input.c b/dlls/win32u/input.c index a8055a9bb5d..6356b01cf8a 100644 --- a/dlls/win32u/input.c +++ b/dlls/win32u/input.c @@ -872,7 +872,7 @@ DWORD WINAPI NtUserGetQueueStatus( UINT flags ) } /******************************************************************* - * NtUserGetThreadInfo (win32u.@) + * NtUserGetThreadState (win32u.@) */ ULONG_PTR WINAPI NtUserGetThreadState( USERTHREADSTATECLASS cls ) { @@ -977,9 +977,6 @@ static HKL get_locale_kbd_layout(void) /*********************************************************************** * NtUserGetKeyboardLayout (win32u.@) - * - * Device handle for keyboard layout defaulted to - * the language id. This is the way Windows default works. */ HKL WINAPI NtUserGetKeyboardLayout( DWORD thread_id ) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10779
From: Matteo Bruni <mbruni@codeweavers.com> Registry access is unnecessary, the layout name matches the high word of the HKL in those cases. --- dlls/win32u/input.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/dlls/win32u/input.c b/dlls/win32u/input.c index 6356b01cf8a..282d897d232 100644 --- a/dlls/win32u/input.c +++ b/dlls/win32u/input.c @@ -1454,11 +1454,10 @@ BOOL WINAPI NtUserGetKeyboardLayoutName( WCHAR *name ) layout = NtUserGetKeyboardLayout( 0 ); id = HandleToUlong( layout ); - if (HIWORD( id ) == LOWORD( id )) id = LOWORD( id ); + if (!(HIWORD( id ) & 0xf000)) id = HIWORD( id ); snprintf( buffer, sizeof(buffer), "%08X", id ); asciiz_to_unicode( name, buffer ); - - if ((hkey = reg_open_key( NULL, keyboard_layouts_keyW, sizeof(keyboard_layouts_keyW) ))) + if ((HIWORD( id ) & 0x1000) && (hkey = reg_open_key( NULL, keyboard_layouts_keyW, sizeof(keyboard_layouts_keyW) ))) { while (!NtEnumerateKey( hkey, i++, KeyNodeInformation, key, sizeof(buffer) - sizeof(WCHAR), &len )) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10779
From: Matteo Bruni <mbruni@codeweavers.com> Very much inspired by a patch by Rémi Bernon. Similarly for some of the following patches. --- dlls/winex11.drv/keyboard.c | 53 +++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/dlls/winex11.drv/keyboard.c b/dlls/winex11.drv/keyboard.c index 65de84678ec..8f6cbd9dfe7 100644 --- a/dlls/winex11.drv/keyboard.c +++ b/dlls/winex11.drv/keyboard.c @@ -55,6 +55,7 @@ #include "kbd.h" #include "wine/server.h" #include "wine/debug.h" +#include "wine/list.h" /* log format (add 0-padding as appropriate): keycode %u as in output from xev @@ -65,6 +66,16 @@ WINE_DEFAULT_DEBUG_CHANNEL(keyboard); WINE_DECLARE_DEBUG_CHANNEL(key); +struct layout +{ + struct list entry; + + int xkb_group; + char *xkb_layout; + + LANGID lang; +}; + static const unsigned int ControlMask = 1 << 2; static int min_keycode, max_keycode, keysyms_per_keycode; @@ -73,9 +84,41 @@ static WORD keyc2vkey[256], keyc2scan[256]; static int NumLockMask, ScrollLockMask, AltGrMask; /* mask in the XKeyEvent state */ static pthread_mutex_t kbd_mutex = PTHREAD_MUTEX_INITIALIZER; +static struct list xkb_layouts = LIST_INIT( xkb_layouts ); static char KEYBOARD_MapDeadKeysym(KeySym keysym); +static void create_layout_from_xkb( int xkb_group, const char *xkb_layout, LANGID lang ) +{ + struct layout *layout; + + TRACE( "xkb_group %u, xkb_layout %s, lang %04x\n", xkb_group, xkb_layout, lang ); + + LIST_FOR_EACH_ENTRY( layout, &xkb_layouts, struct layout, entry ) + { + if (!strcmp( layout->xkb_layout, xkb_layout )) + { + TRACE( "Found existing layout entry %p, lang %04x\n", layout, layout->lang ); + if (layout->xkb_group == -1) layout->xkb_group = xkb_group; + return; + } + } + + if (!(layout = calloc( 1, sizeof(*layout) + strlen( xkb_layout ) + 1 ))) + { + WARN( "Failed to allocate memory for Xkb layout entry\n" ); + return; + } + list_add_tail( &xkb_layouts, &layout->entry ); + + layout->xkb_group = xkb_group; + layout->xkb_layout = strcpy( (char *)(layout + 1), xkb_layout ); + + layout->lang = lang; + + TRACE( "Created layout entry %p, lang %04x\n", layout, layout->lang ); +} + /* Keyboard translation tables */ #define MAIN_LEN 49 static const WORD main_key_scan_qwerty[MAIN_LEN] = @@ -1960,6 +2003,7 @@ void init_keyboard_layouts( Display *display ) XkbStateRec xkb_state; XModifierKeymap *mmp; XkbDescRec *xkb_desc; + struct layout *entry; LANGID xkb_lang = 0; Status status; KeyCode *kcp; @@ -1990,6 +2034,10 @@ void init_keyboard_layouts( Display *display ) } } + /* Flag any previously created Xkb layout as invalid */ + LIST_FOR_EACH_ENTRY( entry, &xkb_layouts, struct layout, entry ) + entry->xkb_group = -1; + status = XkbGetState( display, XkbUseCoreKbd, &xkb_state ); xkb_group = status ? 0 : xkb_state.group; TRACE( "current group %u (status %#x)\n", xkb_group, status ); @@ -2004,9 +2052,11 @@ void init_keyboard_layouts( Display *display ) if (!xkb_desc->names->groups[count]) break; if (!XGetAtomNames( display, xkb_desc->names->groups, count, names )) count = 0; + TRACE("Found %u group names\n", count); for (int i = 0; i < count; i++) { const char *layout, *variant = NULL; + char buffer[1024]; LANGID lang; if (!names[i]) continue; @@ -2017,6 +2067,9 @@ void init_keyboard_layouts( Display *display ) TRACE( "Found group %u with name %s -> layout %s:%s, lang %04x\n", i, debugstr_a(names[i]), debugstr_a(layout), debugstr_a(variant), lang ); + + snprintf( buffer, ARRAY_SIZE(buffer), "%s:%s", layout, variant ); + create_layout_from_xkb( i, buffer, lang ); } XFree( names[i] ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10779
From: Matteo Bruni <mbruni@codeweavers.com> --- dlls/winex11.drv/keyboard.c | 58 ++++++++++++++++++++++++++++++++----- 1 file changed, 51 insertions(+), 7 deletions(-) diff --git a/dlls/winex11.drv/keyboard.c b/dlls/winex11.drv/keyboard.c index 8f6cbd9dfe7..e9ab7e4127d 100644 --- a/dlls/winex11.drv/keyboard.c +++ b/dlls/winex11.drv/keyboard.c @@ -74,6 +74,7 @@ struct layout char *xkb_layout; LANGID lang; + DWORD klid; }; static const unsigned int ControlMask = 1 << 2; @@ -88,20 +89,23 @@ static struct list xkb_layouts = LIST_INIT( xkb_layouts ); static char KEYBOARD_MapDeadKeysym(KeySym keysym); -static void create_layout_from_xkb( int xkb_group, const char *xkb_layout, LANGID lang ) +static void create_layout_from_xkb( int xkb_group, const char *xkb_layout, LANGID lang, DWORD klid ) { struct layout *layout; + WORD index = 0; - TRACE( "xkb_group %u, xkb_layout %s, lang %04x\n", xkb_group, xkb_layout, lang ); + TRACE( "xkb_group %u, xkb_layout %s, lang %04x, klid %08x\n", xkb_group, xkb_layout, lang, klid ); LIST_FOR_EACH_ENTRY( layout, &xkb_layouts, struct layout, entry ) { if (!strcmp( layout->xkb_layout, xkb_layout )) { - TRACE( "Found existing layout entry %p, lang %04x\n", layout, layout->lang ); + TRACE( "Found existing layout entry %p, lang %04x, klid %08x\n", + layout, layout->lang, layout->klid ); if (layout->xkb_group == -1) layout->xkb_group = xkb_group; return; } + if (lang == layout->lang && HIWORD(layout->klid) >= 0x20) index++; } if (!(layout = calloc( 1, sizeof(*layout) + strlen( xkb_layout ) + 1 ))) @@ -115,8 +119,9 @@ static void create_layout_from_xkb( int xkb_group, const char *xkb_layout, LANGI layout->xkb_layout = strcpy( (char *)(layout + 1), xkb_layout ); layout->lang = lang; + layout->klid = !klid ? MAKELONG(lang, 0) : (klid == ~0u) ? MAKELONG(lang, index + 0x20) : klid; - TRACE( "Created layout entry %p, lang %04x\n", layout, layout->lang ); + TRACE( "Created layout entry %p, lang %04x, klid %08x\n", layout, layout->lang, layout->klid ); } /* Keyboard translation tables */ @@ -1640,6 +1645,43 @@ static LANGID langid_from_xkb_layout( const char *layout ) return MAKELANGID(LANG_NEUTRAL, SUBLANG_CUSTOM_UNSPECIFIED); }; +static const struct klid_map_entry +{ + const char *layout; + const char *variant; + DWORD klid; +} klid_map[] = +{ + { "us", "dvorak", 0x00010409 }, + { "us", "dvorak-l", 0x00030409 }, + { "us", "dvorak-r", 0x00040409 }, +}; + +static int klid_map_cmp( const void *key, const void *element ) +{ + const struct klid_map_entry *map_key = key; + const struct klid_map_entry *entry = element; + int c; + + if ((c = strcmp( map_key->layout, entry->layout ))) return c; + return strcmp( map_key->variant, entry->variant ); +} + +static DWORD klid_from_xkb_layout( const char *layout, const char *variant ) +{ + struct klid_map_entry key = { layout, variant }; + const struct klid_map_entry *entry; + + if (!variant) + return 0; + + entry = bsearch( &key, klid_map, ARRAY_SIZE(klid_map), sizeof(*klid_map), klid_map_cmp ); + if (entry) return entry->klid; + + FIXME( "Unknown variant %s\n", debugstr_a(variant) ); + return ~0u; +} + /* fuzzy layout detection through keysym / keycode matching, kbd_section must be held */ static void detect_keyboard_layout( Display *display, XModifierKeymap *modmap, unsigned int xkb_group ) { @@ -2057,19 +2099,21 @@ void init_keyboard_layouts( Display *display ) { const char *layout, *variant = NULL; char buffer[1024]; + DWORD klid; LANGID lang; if (!names[i]) continue; if (find_xkb_layout_variant( names[i], &layout, &variant )) { lang = langid_from_xkb_layout( layout ); + klid = klid_from_xkb_layout( layout, variant ); if (i == xkb_group) xkb_lang = lang; - TRACE( "Found group %u with name %s -> layout %s:%s, lang %04x\n", i, debugstr_a(names[i]), - debugstr_a(layout), debugstr_a(variant), lang ); + TRACE( "Found group %u with name %s -> layout %s:%s, lang %04x, klid %08x\n", i, debugstr_a(names[i]), + debugstr_a(layout), debugstr_a(variant), lang, klid ); snprintf( buffer, ARRAY_SIZE(buffer), "%s:%s", layout, variant ); - create_layout_from_xkb( i, buffer, lang ); + create_layout_from_xkb( i, buffer, lang, klid ); } XFree( names[i] ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10779
From: Matteo Bruni <mbruni@codeweavers.com> --- dlls/winex11.drv/keyboard.c | 61 +++++++++++++++++++++++++++++++++++-- 1 file changed, 58 insertions(+), 3 deletions(-) diff --git a/dlls/winex11.drv/keyboard.c b/dlls/winex11.drv/keyboard.c index e9ab7e4127d..9a971d8d54e 100644 --- a/dlls/winex11.drv/keyboard.c +++ b/dlls/winex11.drv/keyboard.c @@ -75,6 +75,7 @@ struct layout LANGID lang; DWORD klid; + WORD layout_id; }; static const unsigned int ControlMask = 1 << 2; @@ -89,10 +90,62 @@ static struct list xkb_layouts = LIST_INIT( xkb_layouts ); static char KEYBOARD_MapDeadKeysym(KeySym keysym); +static const WCHAR keyboard_layouts_keyW[] = +{ + '\\','R','e','g','i','s','t','r','y', + '\\','M','a','c','h','i','n','e', + '\\','S','y','s','t','e','m', + '\\','C','u','r','r','e','n','t','C','o','n','t','r','o','l','S','e','t', + '\\','C','o','n','t','r','o','l', + '\\','K','e','y','b','o','a','r','d',' ','L','a','y','o','u','t','s' +}; + +static ULONG query_reg_ascii_value( HKEY hkey, const char *name, KEY_VALUE_PARTIAL_INFORMATION *info, ULONG size ) +{ + WCHAR nameW[64]; + + asciiz_to_unicode( nameW, name ); + return query_reg_value( hkey, nameW, info, size ); +} + +static WORD get_layout_id_from_klid( DWORD klid ) +{ + char buffer[2048]; + KEY_VALUE_PARTIAL_INFORMATION *value = (KEY_VALUE_PARTIAL_INFORMATION *)buffer; + WORD layout_id = 0; + HKEY hkey, subkey; + WCHAR name[16]; + + TRACE( "klid %08x\n", klid ); + + snprintf( buffer, sizeof(buffer), "%08X", klid ); + asciiz_to_unicode( name, buffer ); + + if (!(hkey = reg_open_key( NULL, keyboard_layouts_keyW, sizeof(keyboard_layouts_keyW) ))) + return 0; + + if (!(subkey = reg_open_key( hkey, name, 8 * sizeof(WCHAR) ))) + { + NtClose( hkey ); + return 0; + } + + if (query_reg_ascii_value( subkey, "Layout Id", value, sizeof(buffer) ) && value->Type == REG_SZ) + layout_id = wcstoul( (const WCHAR *)value->Data, NULL, 16 ); + NtClose( subkey ); + NtClose( hkey ); + + TRACE( "layout_id %04x\n", layout_id ); + return layout_id; +} + static void create_layout_from_xkb( int xkb_group, const char *xkb_layout, LANGID lang, DWORD klid ) { + static WORD next_layout_id = 0x100; + struct layout *layout; WORD index = 0; + WORD layout_id; TRACE( "xkb_group %u, xkb_layout %s, lang %04x, klid %08x\n", xkb_group, xkb_layout, lang, klid ); @@ -100,8 +153,8 @@ static void create_layout_from_xkb( int xkb_group, const char *xkb_layout, LANGI { if (!strcmp( layout->xkb_layout, xkb_layout )) { - TRACE( "Found existing layout entry %p, lang %04x, klid %08x\n", - layout, layout->lang, layout->klid ); + TRACE( "Found existing layout entry %p, lang %04x, klid %08x, layout_id %04x\n", + layout, layout->lang, layout->klid, layout->layout_id ); if (layout->xkb_group == -1) layout->xkb_group = xkb_group; return; } @@ -120,8 +173,10 @@ static void create_layout_from_xkb( int xkb_group, const char *xkb_layout, LANGI layout->lang = lang; layout->klid = !klid ? MAKELONG(lang, 0) : (klid == ~0u) ? MAKELONG(lang, index + 0x20) : klid; + layout_id = klid && klid != ~0u ? get_layout_id_from_klid( klid ) : 0; + layout->layout_id = layout_id ? layout_id : HIWORD(klid) ? next_layout_id++ : 0; - TRACE( "Created layout entry %p, lang %04x, klid %08x\n", layout, layout->lang, layout->klid ); + TRACE( "Created layout entry %p, lang %04x, klid %08x, layout_id %04x\n", layout, layout->lang, layout->klid, layout->layout_id ); } /* Keyboard translation tables */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10779
Rémi Bernon (@rbernon) commented about dlls/winex11.drv/keyboard.c:
+ return strcmp( map_key->variant, entry->variant ); +} + +static DWORD klid_from_xkb_layout( const char *layout, const char *variant ) +{ + struct klid_map_entry key = { layout, variant }; + const struct klid_map_entry *entry; + + if (!variant) + return 0; + + entry = bsearch( &key, klid_map, ARRAY_SIZE(klid_map), sizeof(*klid_map), klid_map_cmp ); + if (entry) return entry->klid; + + FIXME( "Unknown variant %s\n", debugstr_a(variant) ); + return ~0u; In general I think -1 is less error prone, if type gets ever changed to a larger size ~0u might not be extended.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10779#note_140318
What about moving this logic into `get_layout_id`? I think it avoid make these conditions easier to read. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10779#note_140319
This would be for instance: ```suggestion:-29+0 static WORD get_layout_id_from_klid( DWORD klid ) { static WORD next_layout_id = 0x100; char buffer[2048]; KEY_VALUE_PARTIAL_INFORMATION *value = (KEY_VALUE_PARTIAL_INFORMATION *)buffer; WORD layout_id = 0; HKEY hkey, subkey; WCHAR name[16]; TRACE( "klid %08x\n", klid ); snprintf( buffer, sizeof(buffer), "%08X", klid ); asciiz_to_unicode( name, buffer ); if (!(hkey = reg_open_key( NULL, keyboard_layouts_keyW, sizeof(keyboard_layouts_keyW) ))) goto done; if (!(subkey = reg_open_key( hkey, name, 8 * sizeof(WCHAR) ))) goto done; if (query_reg_ascii_value( subkey, "Layout Id", value, sizeof(buffer) ) && value->Type == REG_SZ) layout_id = wcstoul( (const WCHAR *)value->Data, NULL, 16 ); NtClose( subkey ); done: if (hkey) NtClose( hkey ); if (!layout_id && HIWORD(klid)) layout_id = next_layout_id++ TRACE( "layout_id %04x\n", layout_id ); return layout_id; } ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10779#note_140320
And this would become: ```suggestion:-1+0 layout->layout_id = get_layout_id_from_klid( klid ); ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10779#note_140321
participants (3)
-
Matteo Bruni -
Matteo Bruni (＠Mystral)
-
Rémi Bernon (＠rbernon)