RFR: 8305418: [Linux] Replace obsolete XIM as Input Method Editor [v13]

[Martin Fox]

On Tue, 28 Nov 2023 22:09:37 GMT, Thiago Milczarek Sayao <tsayao at openjdk.org> wrote:

>> This replaces obsolete XIM and uses gtk api for IME.
>> Gtk uses [ibus](https://github.com/ibus/ibus)
>> 
>> Gtk3+ uses relative positioning (as Wayland does), so I've added a Relative positioning on `InputMethodRequest`.
>> 
>> [Screencast from 17-09-2023 21:59:04.webm](https://github.com/openjdk/jfx/assets/30704286/6c398e39-55a3-4420-86a2-beff07b549d3)
>
> Thiago Milczarek Sayao has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Account the case of !filtered

This PR is a massive simplification of the code and brings JavaFX in line with the behavior of other Linux apps. The complication is the introduction of scene-relative caret positioning. I understand Wayland is forcing the issue by ditching screen coordinates but I'm not sure this is the PR to start tackling that.

modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassViewEventHandler.java line 681:

> 679:     public double[] getInputMethodCandidateRelativePos(int offset) {
> 680:         Point2D p2d = scene.inputMethodRequests.getTextLocationRelative(offset);
> 681:         return new double[] { p2d.getX(), p2d.getY() };

On my system the IM window is incorrectly positioned. This appears to be because I'm running a high-DPI display with 200% magnification. I think you need to call getPlatformScaleX and getPlatformScaleY and apply those scaling factors before passing these coordinates on to glass. You'll see other places in this file where conversions like that are being performed.

modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp line 486:

> 484:     CHECK_JNI_EXCEPTION(mainEnv)
> 485: 
> 486:     if (press && key > 0) { // TYPED events should only be sent for printable characters.

A handler for the PRESS event might close the window. In that case `jview` will be set to zero before you send out the TYPED event. You should do another check for that here.

See [JDK-8301219](https://bugs.openjdk.org/browse/JDK-8301219) for some sample code. I'll be submitting a fix for that bug just as soon as I get a test case working reliably.

modules/javafx.graphics/src/main/native-glass/gtk/glass_window_ime.cpp line 67:

> 65:     } while (pango_attr_iterator_next(iter));
> 66: 
> 67:     pango_attr_iterator_destroy (iter);

According to the docs you need to release the attribute list using pango_attr_list_unref().

modules/javafx.graphics/src/main/native-glass/gtk/glass_window_ime.cpp line 120:

> 118:     if (!filtered || (filtered && im_ctx.send_keypress)) {
> 119:         process_key(&event->key);
> 120:         im_ctx.send_keypress = false;

I'm seeing two RELEASE events on each keystroke. If you call process_key() here you need to set `filtered` to true to ensure the event isn't processed again.

modules/javafx.graphics/src/main/native-glass/gtk/glass_window_ime.cpp line 175:

> 173:     if (im_ctx.ctx != NULL) {
> 174:         g_signal_handlers_disconnect_matched(im_ctx.ctx, G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, NULL);
> 175:         g_object_unref(im_ctx.ctx);

If the IM window is visible when the window is closed disableIME() can get called twice; I'm seeing debug output being generated. Set im_ctx.ctx to NULL here or you'll unref it twice.

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

Changes requested by mfox (Committer).

PR Review: https://git.openjdk.org/jfx/pull/1080#pullrequestreview-1780301464
PR Review Comment: https://git.openjdk.org/jfx/pull/1080#discussion_r1425768173
PR Review Comment: https://git.openjdk.org/jfx/pull/1080#discussion_r1425788182
PR Review Comment: https://git.openjdk.org/jfx/pull/1080#discussion_r1425774627
PR Review Comment: https://git.openjdk.org/jfx/pull/1080#discussion_r1425775953
PR Review Comment: https://git.openjdk.org/jfx/pull/1080#discussion_r1425777541