RFR: 8305418: [Linux] Replace obsolete XIM as Input Method Editor [v13]
Martin Fox
mfox at openjdk.org
Tue Jan 2 17:01:00 UTC 2024
On Sat, 16 Dec 2023 22:55:42 GMT, Thiago Milczarek Sayao <tsayao at openjdk.org> wrote:
>> 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.
>
> I did revert the relative positioning change as it will get in the way of merging this. Could you check if it's correctly positioned now? I don't own a fancy monitor :) Tried 200% scale here and everything looks monstrous. 125% scale looks correct.
I think the problem is deeper in the Java code since it also affects Windows. See PR #1311.
>> 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.
>
> I changed to "if not filtered", just propagate.
Looks good to me.
>> 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.
>
> Fixed.
Looks good to me.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1080#discussion_r1439634871
PR Review Comment: https://git.openjdk.org/jfx/pull/1080#discussion_r1439635767
PR Review Comment: https://git.openjdk.org/jfx/pull/1080#discussion_r1439635167
More information about the openjfx-dev
mailing list