RFR: 8308644: [Linux] Simplify and fix small bugs on glass key related events [v4]

[Thiago Milczarek Sayao]

On Tue, 23 May 2023 22:43:11 GMT, Thiago Milczarek Sayao <tsayao at openjdk.org> wrote:

>> modules/javafx.graphics/src/main/native-glass/gtk/glass_key.cpp line 240:
>> 
>>> 238: };
>>> 239: 
>>> 240: jint gdk_keyval_to_glass(guint keyval) {
>> 
>> It's almost always best to leave working code as-is. The Contributing document contains this line:
>> 
>>> Note that it is unlikely the project will merge refactors for the sake of refactoring.
>> 
>> I don't find a hard-coded table and custom binary search algorithm to be simpler than a hash map, particularly if contributors are responsible for keeping the table in correct order. In any case the old code was working fine and I would leave it alone.
>> 
>> I'm still glad you did this work, it's good to revisit detail-oriented code like this every now and then looking for things that were overlooked.
>
> I was not sure about that, It does generate the hash table on the first key press, but performance-wise I don't think there are a significant difference. I like the binary search better, but without further arguments/reasons I think it's hard to convince the reviewer.

Reverted bash to hash table and added the missing dead keys mapping.

>> modules/javafx.graphics/src/main/native-glass/gtk/glass_key.cpp line 396:
>> 
>>> 394: 
>>> 395:         case com_sun_glass_events_KeyEvent_VK_NUM_LOCK:
>>> 396:             return (gdk_keymap_get_num_lock_state(keymap))
>> 
>> Have you tried testing this with a Wayland backend? I was doing some testing and it seemed that unless JavaFX opened a window we didn't connect with the Wayland server so we couldn't get accurate keyboard layout information. It wouldn't surprise me if this also prevented us from getting accurate keyboard state so the manual test for this code (which does not create a window) might fail. Beyond that I agree we should be migrating away from X11 calls if possible.
>
> I have attached a `key_release.c` on the JBS bug, it works better on Wayland. On Xorg when you release the CAPS LOCK it will report as still ON until you press another key.
> 
> Compile with gcc -o key_release key_release.c `pkg-config --cflags --libs gtk+-3.0`

I reverted back for now since the CAPS LOCK key release behaviour was not ok.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1143#discussion_r1204130650
PR Review Comment: https://git.openjdk.org/jfx/pull/1143#discussion_r1204129794