[PATCH v6 0/7] MR5319: winemac.drv keyboard layout/dead key fixes
This is a series of commits tackling dead key state handling and scan code to vkey mapping improvements (especially for French and German keyboards). * https://gitlab.winehq.org/mzent/wine/-/commit/6c5c90b746cc341049eec216e3bb94...: Simplifies the logic a bit there and also correctly null-terminates dead keys in ToUnicodeEx * https://gitlab.winehq.org/mzent/wine/-/commit/a05e2fc5068a22cae94bc800b49727...: Windows does not do it and some applications misbehave with this, fully fixed later in https://gitlab.winehq.org/mzent/wine/-/commit/7cf1f6a00f17b0626126448f77a1cb... * https://gitlab.winehq.org/mzent/wine/-/commit/4bd96932139a22f2c3b1311e44c43f...: MSDN states single keys are always uppercased there. * https://gitlab.winehq.org/mzent/wine/-/commit/bac8e97d7e6a7494672d76f5fc5868...: There is a bug in `CFStringCompare` with the flags used there currently, resulting in the comparison of the identical buffers "ß" and "ß" to be false. In any case `UCCompareText` behaves correctly and is much faster there. * https://gitlab.winehq.org/mzent/wine/-/commit/bac5f485a09fbddd39efb8f2c0d87e...: I think this was an accidental mistake there, going through all modifiers first per symbol almost always results in the wrong match being made. The logic is adjusted there now to be similar to the other iterations involving `char_matches_string`. * https://gitlab.winehq.org/mzent/wine/-/commit/ffde55df4ae3f6d53198839c583569... https://gitlab.winehq.org/mzent/wine/-/commit/a453d0d4c89c59c3e1ba6c24d2d0f6...: Adds additional symbol vkey mappings (now completely maps a German Macintosh and PC keyboard correctly). There are still some minor defects with the French layout (around the ú region), however changing the symbol mapping there will cause issues with other layouts I think, since the wrong match happens very early there in the priority in these cases. With this change at least the number row is now behaving correctly. Additionally the French Macintosh keyboard layout is further complicated by the "=" key being next to right shift, which is in conflict with pretty much any other keyboard layout. To fix this properly would require a partial rewrite of the heuristic being used or a new approach, but most likely more of a long term project. * https://gitlab.winehq.org/mzent/wine/-/commit/7cf1f6a00f17b0626126448f77a1cb...: Gives dead keys friendly names in `GetKeyNameFriendlyText`, just like Windows does. I tried to be as exhaustive as possible, but if there are any missing the mapping can be easily extended. All in all the incorrect scan code to vkey mapping is usually not that tragic, since an application would need to be aware of the layout and have its own mapping of scan codes of each (FFXIV does this though). The French layout corrections are now "good enough" to behave correctly for all the hotbar slots being assigned there with these changes now though. -- v6: winemac.drv: Give dead keys a friendly name in GetKeyNameText. winemac.drv: Add additional German symbol vkeys. winemac.drv: Resolve symbol vkeys first without modifiers. winemac.drv: Use UCCompareText in char_matches_string. winemac.drv: Uppercase single keys in GetKeyNameText. winemac.drv: Do not append " dead" to dead keycodes in GetKeyNameText. winemac.drv: Handle length of dead keycodes in ToUnicodeEx correctly. https://gitlab.winehq.org/wine/wine/-/merge_requests/5319
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/winemac.drv/keyboard.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/dlls/winemac.drv/keyboard.c b/dlls/winemac.drv/keyboard.c index 474ca499961..7f8148bdf24 100644 --- a/dlls/winemac.drv/keyboard.c +++ b/dlls/winemac.drv/keyboard.c @@ -1622,7 +1622,7 @@ INT macdrv_ToUnicodeEx(UINT virtKey, UINT scanCode, const BYTE *lpKeyState, UInt32 modifierKeyState; OptionBits options; UInt32 deadKeyState, savedDeadKeyState; - UniCharCount len; + UniCharCount len = 0; BOOL dead = FALSE; TRACE_(key)("virtKey 0x%04x scanCode 0x%04x lpKeyState %p bufW %p bufW_size %d flags 0x%08x hkl %p\n", @@ -1758,7 +1758,7 @@ INT macdrv_ToUnicodeEx(UINT virtKey, UINT scanCode, const BYTE *lpKeyState, &savedDeadKeyState, bufW_size, &len, bufW); if (status != noErr) { - ERR_(key)("Couldn't translate keycode 0x%04x, status %d\n", keyc, status); + ERR_(key)("Couldn't translate dead keycode 0x%04x, status %d\n", keyc, status); goto done; } @@ -1768,8 +1768,7 @@ INT macdrv_ToUnicodeEx(UINT virtKey, UINT scanCode, const BYTE *lpKeyState, if (len > 0) len = strip_apple_private_chars(bufW, len); - if (dead && len > 0) ret = -1; - else ret = len; + ret = dead ? -len : len; /* Control-Return produces line feed instead of carriage return. */ if (ret > 0 && (lpKeyState[VK_CONTROL] & 0x80) && virtKey == VK_RETURN) @@ -1783,10 +1782,10 @@ INT macdrv_ToUnicodeEx(UINT virtKey, UINT scanCode, const BYTE *lpKeyState, done: /* Null-terminate the buffer, if there's room. MSDN clearly states that the caller must not assume this is done, but some programs (e.g. Audiosurf) do. */ - if (1 <= ret && ret < bufW_size) - bufW[ret] = 0; + if (len < bufW_size) + bufW[len] = 0; - TRACE_(key)("returning %d / %s\n", ret, debugstr_wn(bufW, abs(ret))); + TRACE_(key)("returning %d / %s\n", ret, debugstr_wn(bufW, len)); return ret; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5319
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/winemac.drv/keyboard.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/dlls/winemac.drv/keyboard.c b/dlls/winemac.drv/keyboard.c index 7f8148bdf24..281085ca979 100644 --- a/dlls/winemac.drv/keyboard.c +++ b/dlls/winemac.drv/keyboard.c @@ -1310,7 +1310,6 @@ INT macdrv_GetKeyNameText(LONG lparam, LPWSTR buffer, INT size) { if (thread_data->keyc2scan[keyc] == scan) { - static const WCHAR dead[] = {' ','d','e','a','d',0}; const UCKeyboardLayout *uchr; UInt32 deadKeyState = 0; UniCharCount len; @@ -1370,12 +1369,6 @@ INT macdrv_GetKeyNameText(LONG lparam, LPWSTR buffer, INT size) if (!len) break; - if (status == noErr && deadKeyState) - { - lstrcpynW(buffer + len, dead, size - len); - len = wcslen(buffer); - } - TRACE("lparam 0x%08x -> %s\n", (unsigned int)lparam, debugstr_w(buffer)); return len; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5319
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/winemac.drv/keyboard.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/dlls/winemac.drv/keyboard.c b/dlls/winemac.drv/keyboard.c index 281085ca979..804164c643a 100644 --- a/dlls/winemac.drv/keyboard.c +++ b/dlls/winemac.drv/keyboard.c @@ -1369,6 +1369,18 @@ INT macdrv_GetKeyNameText(LONG lparam, LPWSTR buffer, INT size) if (!len) break; + if (status == noErr && len == 1) + { + CFStringRef stringRef = CFStringCreateWithCharacters(kCFAllocatorDefault, buffer, len); + CFMutableStringRef mutableStringRef = CFStringCreateMutableCopy(kCFAllocatorDefault, 0, stringRef); + + CFStringUppercase(mutableStringRef, CFLocaleGetSystem()); + if (CFStringGetLength(mutableStringRef) == 1) + CFStringGetCharacters(mutableStringRef, CFRangeMake(0, 1), buffer); + CFRelease(stringRef); + CFRelease(mutableStringRef); + } + TRACE("lparam 0x%08x -> %s\n", (unsigned int)lparam, debugstr_w(buffer)); return len; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5319
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/winemac.drv/keyboard.c | 42 ++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/dlls/winemac.drv/keyboard.c b/dlls/winemac.drv/keyboard.c index 804164c643a..d999628df0d 100644 --- a/dlls/winemac.drv/keyboard.c +++ b/dlls/winemac.drv/keyboard.c @@ -398,18 +398,18 @@ static const struct { { VK_VOLUME_UP | 0x100, "Volume Up" }, }; -static BOOL char_matches_string(WCHAR wchar, UniChar *string, BOOL ignore_diacritics) +static Boolean char_matches_string(WCHAR wchar, UniChar *string, CollatorRef collatorRef) { - BOOL ret; - CFStringRef s1 = CFStringCreateWithCharactersNoCopy(NULL, (UniChar*)&wchar, 1, kCFAllocatorNull); - CFStringRef s2 = CFStringCreateWithCharactersNoCopy(NULL, string, wcslen(string), kCFAllocatorNull); - CFStringCompareFlags flags = kCFCompareCaseInsensitive | kCFCompareNonliteral | kCFCompareWidthInsensitive; - if (ignore_diacritics) - flags |= kCFCompareDiacriticInsensitive; - ret = (CFStringCompare(s1, s2, flags) == kCFCompareEqualTo); - CFRelease(s1); - CFRelease(s2); - return ret; + Boolean equivalent; + OSStatus status; + + status = UCCompareText(collatorRef, (UniChar*)&wchar, 1, string, wcslen(string), &equivalent, NULL); + if (status != noErr) + { + WARN("Failed to compare %s to %s\n", debugstr_wn(&wchar, 1), debugstr_w(string)); + return FALSE; + } + return equivalent; } @@ -657,6 +657,9 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) int keyc; WCHAR vkey; const UCKeyboardLayout *uchr; + LocaleRef localeRef; + CollatorRef collatorRef, caseInsensitiveCollatorRef, diacriticInsensitiveCollatorRef; + UCCollateOptions collateOptions = 0; const UInt32 modifier_combos[] = { 0, shiftKey >> 8, @@ -778,6 +781,13 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) uchr = (const UCKeyboardLayout*)CFDataGetBytePtr(thread_data->keyboard_layout_uchr); + LocaleRefFromLocaleString("POSIX", &localeRef); + UCCreateCollator(localeRef, 0, collateOptions, &collatorRef); + collateOptions |= kUCCollateComposeInsensitiveMask | kUCCollateWidthInsensitiveMask | kUCCollateCaseInsensitiveMask; + UCCreateCollator(localeRef, 0, collateOptions, &caseInsensitiveCollatorRef); + collateOptions |= kUCCollateDiacritInsensitiveMask; + UCCreateCollator(localeRef, 0, collateOptions, &diacriticInsensitiveCollatorRef); + /* Using the keyboard layout, build a map of key code + modifiers -> characters. */ memset(map, 0, sizeof(map)); for (keyc = 0; keyc < ARRAY_SIZE(map); keyc++) @@ -824,7 +834,7 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) if (thread_data->keyc2vkey[keyc] || !map[keyc][combo][0]) continue; - if (char_matches_string(vkey, map[keyc][combo], ignore_diacritics)) + if (char_matches_string(vkey, map[keyc][combo], ignore_diacritics ? diacriticInsensitiveCollatorRef : caseInsensitiveCollatorRef)) { thread_data->keyc2vkey[keyc] = vkey; vkey_used[vkey] = 1; @@ -850,7 +860,7 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) if (thread_data->keyc2vkey[keyc] || !map[keyc][combo][0]) continue; - if (char_matches_string(vkey, map[keyc][combo], FALSE)) + if (char_matches_string(vkey, map[keyc][combo], collatorRef)) { thread_data->keyc2vkey[keyc] = vkey; vkey_used[vkey] = 1; @@ -879,7 +889,7 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) if (thread_data->keyc2vkey[keyc] || !map[keyc][combo][0]) continue; - if (char_matches_string(symbol_vkeys[i].wchar, map[keyc][combo], FALSE)) + if (char_matches_string(symbol_vkeys[i].wchar, map[keyc][combo], collatorRef)) { thread_data->keyc2vkey[keyc] = vkey; vkey_used[vkey] = 1; @@ -982,6 +992,10 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) vkey_used[vkey] = 1; TRACE("keyc 0x%04x -> vkey 0x%04x (spare vkey)\n", keyc, vkey); } + + UCDisposeCollator(&collatorRef); + UCDisposeCollator(&caseInsensitiveCollatorRef); + UCDisposeCollator(&diacriticInsensitiveCollatorRef); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5319
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/winemac.drv/keyboard.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/dlls/winemac.drv/keyboard.c b/dlls/winemac.drv/keyboard.c index d999628df0d..f0b07d0015a 100644 --- a/dlls/winemac.drv/keyboard.c +++ b/dlls/winemac.drv/keyboard.c @@ -874,15 +874,15 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) /* Now try to match key codes for certain common punctuation characters to the most common OEM vkeys (e.g. '.' to VK_OEM_PERIOD). */ - for (i = 0; i < ARRAY_SIZE(symbol_vkeys); i++) + for (combo = 0; combo < ARRAY_SIZE(modifier_combos); combo++) { - vkey = symbol_vkeys[i].vkey; + for (i = 0; i < ARRAY_SIZE(symbol_vkeys); i++) + { + vkey = symbol_vkeys[i].vkey; - if (vkey_used[vkey]) - continue; + if (vkey_used[vkey]) + continue; - for (combo = 0; combo < ARRAY_SIZE(modifier_combos); combo++) - { for (keyc = 0; keyc < ARRAY_SIZE(map); keyc++) { if (!thread_data->keyc2scan[keyc]) continue; /* not a known Mac key code */ @@ -898,9 +898,6 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) break; } } - - if (vkey_used[vkey]) - break; } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5319
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/winemac.drv/keyboard.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dlls/winemac.drv/keyboard.c b/dlls/winemac.drv/keyboard.c index f0b07d0015a..6deb21f3a2f 100644 --- a/dlls/winemac.drv/keyboard.c +++ b/dlls/winemac.drv/keyboard.c @@ -689,6 +689,10 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) { '`', VK_OEM_3 }, { '[', VK_OEM_4 }, { '~', VK_OEM_3 }, + { 0x00DF, VK_OEM_4 }, /* 0x00DF is ESZETT */ + { 0x00FC, VK_OEM_1 }, /* 0x00FC is German U Umlaut */ + { 0x00F6, VK_OEM_3 }, /* 0x00F6 is German O Umlaut */ + { 0x00E4, VK_OEM_7 }, /* 0x00B4 is German A Umlaut */ { '?', VK_OEM_2 }, { ']', VK_OEM_6 }, { '/', VK_OEM_2 }, @@ -711,6 +715,7 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) { '^', VK_OEM_6 }, { '*', VK_OEM_2 }, { '{', VK_OEM_6 }, + { 0x00B4, VK_OEM_6 }, { '~', VK_OEM_1 }, { '?', VK_OEM_PLUS }, { '?', VK_OEM_4 }, @@ -720,6 +725,7 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) { ']', VK_OEM_4 }, { '\'', VK_OEM_3 }, { 0x00A7, VK_OEM_7 }, + { '<', VK_OEM_102 }, }; int i; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5319
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/winemac.drv/keyboard.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/dlls/winemac.drv/keyboard.c b/dlls/winemac.drv/keyboard.c index 6deb21f3a2f..0926b87f888 100644 --- a/dlls/winemac.drv/keyboard.c +++ b/dlls/winemac.drv/keyboard.c @@ -398,6 +398,27 @@ static const struct { { VK_VOLUME_UP | 0x100, "Volume Up" }, }; + +static const struct { + WCHAR wchar; + const char *name; +} dead_key_names[] = { + { '^', "CIRCUMFLEX ACCENT" }, + { '`', "GRAVE ACCENT" }, + { 0x00B4, "ACUTE ACCENT" }, + { '~', "TILDE" }, + { 0x00A8, "DIAERESIS" }, + { 0x00B8, "CEDILLA" }, + { 0x02D8, "BREVE" }, + { 0x02D9, "DOT ABOVE" }, + { 0x00AF, "MACRON" }, + { 0x02DA, "RING ABOVE" }, + { 0x02DB, "OGONEK" }, + { 0x02DC, "SMALL TILDE" }, + { 0x02DD, "DOUBLE ACUTE ACCENT" }, +}; + + static Boolean char_matches_string(WCHAR wchar, UniChar *string, CollatorRef collatorRef) { Boolean equivalent; @@ -1386,6 +1407,20 @@ INT macdrv_GetKeyNameText(LONG lparam, LPWSTR buffer, INT size) if (!len) break; + if (status == noErr && deadKeyState) + { + for (i = 0; i < ARRAY_SIZE(dead_key_names); i++) + { + if (dead_key_names[i].wchar == buffer[0]) + { + len = min(strlen(dead_key_names[i].name) + 1, size); + ascii_to_unicode(buffer, dead_key_names[i].name, len); + if (len) buffer[--len] = 0; + break; + } + } + } + if (status == noErr && len == 1) { CFStringRef stringRef = CFStringCreateWithCharacters(kCFAllocatorDefault, buffer, len); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5319
On Fri Jun 21 09:57:00 2024 +0000, Rémi Bernon wrote:
Doesn't seem to be needed. The problem is that `UCKeyTranslate` on failure does not always update its `actualStringLength` out parameter, leaving effectively `len` uninitialized when jumping to `done` this way.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5319#note_74342
On Tue Jun 25 09:41:10 2024 +0000, Marc-Aurel Zent wrote:
changed this line in [version 6 of the diff](/wine/wine/-/merge_requests/5319/diffs?diff_id=119389&start_sha=9f2294f1e4359ee671551333b2ef63ba3e6c11ef#8ad61927ca9cd33508c8ef07654574b3db25a865_1822_1822) Yep indeed, thanks for the catch
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5319#note_74343
On Tue Jun 25 09:41:10 2024 +0000, Marc-Aurel Zent wrote:
changed this line in [version 6 of the diff](/wine/wine/-/merge_requests/5319/diffs?diff_id=119389&start_sha=9f2294f1e4359ee671551333b2ef63ba3e6c11ef#8ad61927ca9cd33508c8ef07654574b3db25a865_1843_1842) Fixed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5319#note_74344
On Fri Jun 21 09:57:02 2024 +0000, Rémi Bernon wrote:
Does we need to support unicode? What about: ```suggestion:-10+0 if (status == noErr && len == 1 && buffer[0] >= 'a' && buffer[0] <= 'z') buffer[0] += 'A' - 'a'; ``` The problem here is that this also needs to support `Ä`,`Ü` etc on some layouts, so unicode is needed there I think.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5319#note_74345
On Tue Jun 25 09:41:11 2024 +0000, Marc-Aurel Zent wrote:
changed this line in [version 6 of the diff](/wine/wine/-/merge_requests/5319/diffs?diff_id=119389&start_sha=9f2294f1e4359ee671551333b2ef63ba3e6c11ef#8ad61927ca9cd33508c8ef07654574b3db25a865_1416_1416) Fixed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5319#note_74346
On Tue Jun 25 09:45:14 2024 +0000, Marc-Aurel Zent wrote:
The problem here is that this also needs to support `Ä`,`Ü` etc on some layouts, so unicode is needed there I think. I guess it is also an option to only use the unicode codepath if `buffer[0] >= 'a' && buffer[0] <= 'z'` is false, if it is a performance bottleneck...
Ideally this would just be a `CharUpperBuffW(buffer, len)` operation, if possible. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5319#note_74347
On Tue Jun 25 09:51:20 2024 +0000, Marc-Aurel Zent wrote:
I guess it is also an option to only use the unicode codepath if `buffer[0] >= 'a' && buffer[0] <= 'z'` is false, if it is a performance bottleneck... Ideally this would just be a `CharUpperBuffW(buffer, len)` operation, if possible. Well, I can easily believe that applications rely on single letter ASCII key names being uppercase, but I'm more doubtful that they do for any unicode ones, or that Windows really enforces anything (it's probably just a default table that happens to be uppercase for ASCII keys).
A quick check for instance with a German keyboard, shows that the Ü key name is lowercase ü. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5319#note_74348
participants (3)
-
Marc-Aurel Zent -
Marc-Aurel Zent (＠mzent)
-
Rémi Bernon