On Fri, 5 May 2023 20:04:12 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:

>> Note: the Java-side changes in this PR are also in #694 which fixes the same 
>> issue (and more) on Linux. Unfortunately the Linux Robot code is not working 
>> making it difficult to test on that platform (see #718).
>> 
>> KeyCharacterCombinations allow the specification of accelerators based on 
>> characters whose KeyCodes vary across keyboard layouts. For example, the + 
>> character is on KeyCode.EQUALS on a U.S. English layout, KeyCode.PLUS on a 
>> German layout, and KeyCode.DIGIT1 on a Mac Swiss German layout. 
>> KeyCharacterCombinations finds the correct KeyCode by calling 
>> `Toolkit.getKeyCodeForChar`.
>> 
>> `getKeyCodeForChar` can only return one KeyCode for a given character so it 
>> can't easily handle characters which appear in more than one location, like 
>> + which is on the main keyboard and the numeric keypad. It's also reliant on 
>> KeyCodes which prevents KeyCharacterCombinations from working on keys with 
>> no codes (e.g. the base character contains a diacritic). It also relies on 
>> the platform to map from a character to a key which is the reverse of how 
>> key mapping normally works making it slow and/or imprecise to implement on 
>> Mac and Linux (Windows is the only platform with a system call to do this).
>> 
>> This PR introduces a new way for a platform to pass key information to the 
>> Java core. `View.notifyKeyEx` takes an additional platform-specific 
>> `hardwareCode` which identifies the key and is tracked in a private field in 
>> the KeyEvent. This is opt-in; a platform can continue to call the old 
>> `View.notifyKey` method and allow the `hardwareCode` to default to -1.
>> 
>> On the back-end `KeyCharacterCombination.match` calls the new routine 
>> `Toolkit.getKeyCanGenerateCharacter` which unpacks the KeyEvent information 
>> and sends it on to the Application. This is also opt-in; the default 
>> implementation falls back to the Application's `getKeyCodeForChar` call. 
>> Platforms which call `View.notifyKeyEx` will be handed the `hardwareCode` 
>> for the key in addition to the Java KeyCode.
>> 
>> The new `View.notifyKeyEx` returns a boolean indicating whether the event 
>> was consumed or not. This plays no role here but will be used later to fix 
>> [JDK-8087863](https://bugs.openjdk.org/browse/JDK-8087863).
>> 
>> For testing I've included the manual KeyboardTest app that also appears in 
>> PR #425. Tests with keypad combinations should now work.
>> 
>> Note: this PR only fixes Windows. Fixes for Mac and Linux but can't be 
>> submitted until #425 and #718 are integrated.
>
> modules/javafx.graphics/src/main/native-glass/win/KeyTable.cpp line 366:
> 
>> 364:         UINT keyChar = ::MapVirtualKeyEx(UINT(hardwareCode), 2, layout);
>> 365:         // Filter out dead keys
>> 366:         BOOL isDead = (keyChar & 0x80000000) != 0;
> 
> Wouldn't `hardwareCode >= 0` already filter out dead keys since when the 
> highest bit is set the value is negative?

I just realized that I never actually said what `hardwareCode` means on this 
platform.

A negative number means the `hardwareCode` isn't known (in other words, the 
KeyEvent wasn't constructed by glass). If it's not negative it's a Windows VK 
code that identifies the key the user pressed. That code doesn't reflect 
whether the key is dead or not. Here I'm asking what character that key would 
generate if pressed without any modifiers and ignoring it if the key is dead.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1126#discussion_r1189109403

Reply via email to