On Sat, 16 Dec 2023 22:55:42 GMT, Thiago Milczarek Sayao <tsa...@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
