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

Thiago Milczarek Sayao tsayao at openjdk.org
Tue May 23 22:46:01 UTC 2023


On Tue, 23 May 2023 16:59:10 GMT, Martin Fox <duke at openjdk.org> wrote:

>> While working on IME I noticed this code could be simplified, so I removed the `g_hash_table` usage for a simple struct ordered for binary search (I'm sure it's ordered because I did a simple app to generate it ordered).
>> 
>> This also fixes:
>> 
>> - UNKNOWN key codes being sent on KEY_PRESS;
>> - Alt Gr is now working;
>> - Replaced X calls with GDK calls for key locked check for as a small step towards supporting Wayland in the future
>> - Added missing mappings (for F13 - F24 and dead keys) - Linux does not send dead keys as keypress but sends as key release, and they might be used on a robot test, so it's now covered.
>
> 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.

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

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


More information about the openjfx-dev mailing list