input method #1735

duarm · 2022-10-27T00:05:45Z

input method draft, it works, but it was hacked together since I'm not familiar with rofi's code base, nor an ime expert

We just create an XIM handler with a new dependency xcb-imdkit, and forward each PRESS event to it.
only tested with fcitx5.

fix warning
window position is slightly off
unresponsive typing (because of rofi_set_im_window_pos)
first character is delayed

sardemff7

Not an IM expert, but a few remarks across the board

Looks good from here, with some tweaks it’ll be mergeable in my book

source/xcb.c

source/view.c

duarm · 2022-10-27T17:10:54Z

related libgwater PR sardemff7/libgwater#4

duarm · 2022-10-27T17:16:14Z

fixed the event forwarding, we only forward the event if we have a xim server in place (when xcb->ic is created), so typing works without an xim server.

added a disconnection callback too, so if the xim server is down, we properly revert to not forwarding events.

both warnings are related to event type 31 (XCB_SELECTION_NOTIFY), for some reason we receive one as soon as we get to the x11 main loop, and rofi returns a "Failed to convert selection" if the xim server is down, or a "Failed" if the server is up. Not sure why. this isn't affecting anything as far as I know.

XIM server window position is slightly off, the position can be offsetted changing the open_xim_callback spot, not sure how to get the necessary information to reposition it in rofi, may be you can help with this one and the warnings?

DaveDavenport · 2022-10-27T17:27:08Z

~~I currently do not keep a relative (or absolute) x,y position for widgets (that I can remember). This is something I could look at adding. (never really had a use for that information before).~~

Ugh, my brain is foggy. I already do this (for the mouse handling). We just need to recurse back to sum up all relative positions.

Do you need position relative to rofi window or absolute position?

There is a widget_get_absolute_xpos/ypos function you can call on state->text that gives you the top left position of the entry box. Adding widget_get_height(state->text) to y offset should allow you to find the bottom of the widget.

duarm · 2022-10-27T17:42:58Z

Do you need position relative to rofi window or absolute position?

not sure, I'm passing the CacheState.mainWindow to the server, we define an offset, the window by default (offset 0,0) appears here

on my launcher, it appears here

There is a widget_get_absolute_xpos/ypos function you can call on state->text that gives you the top right position of the entry box. Adding widget_get_height(state->text) to y offset should allow you to find the bottom of the widget.

I'll take a look later

sardemff7 · 2022-10-27T17:56:57Z

As noted in sardemff7/libgwater#4 the xcb_xim_t should be created on the rofi side

DaveDavenport · 2022-10-27T18:01:38Z

I wanted to say:

top left..

sardemff7

We’re getting closer!

source/xcb.c

duarm · 2022-10-27T20:00:18Z

how can I get the text width? the IM window is already at the correct height for most widgets ([thanks @DaveDavenport), but now I need the current text width so the window follows the text.

DaveDavenport · 2022-10-27T20:02:06Z

how can I get the text width? the IM window is already at the correct height for most widgets ([thanks @DaveDavenport), but now I need the current text width so the window follows the text.

That is currently not stored. I will try to add a function that gets the cursor position (I think this is what we want).

duarm · 2022-10-27T20:11:17Z

xim context creation was moved to rofi, a little easier to test now

duarm · 2022-10-27T20:15:44Z

sorry for the next two commits, forgot to unstage the changes

DaveDavenport · 2022-10-27T20:24:27Z

I've added a get_cursor_x_pos method to textbox

textbox_get_cursor_x_pos(state->text);

sardemff7

Although it’s some hard-to-read code (as any code related to input, apparently), I’m good with this version on the plumbing side 🙂

Guess it just need some polishing on the UI side

I’ll let @DaveDavenport handle the rest 😄 (but don’t hesitate to ping again if needed)

duarm · 2022-10-27T21:43:56Z

tried the new get_cursor_x_pos() and everything's almost working, the problem I found is that get_cursor_x_pos() is returning the cached cursor_pos which is calculated on the previous draw, so just placing the update on rofi_view_handle_text() doesn't work, where should I put the window position update?

duarm · 2022-10-27T21:46:44Z

actually I think the expected behaviour is for the window to follow as you type, before commiting the text. Think I can do that through the callbacks, but that will complicate some things, I would need to preview typed text without actually triggering the fuzzy finder, we might want to do that later, on another PR.

DaveDavenport · 2022-10-27T21:48:33Z

the new position is available once drawn.. So after 'widget_draw' so in rofi_view_update

duarm · 2022-10-27T22:22:20Z

removed the positioning queue, tested without and had no problems, added a new rofi_set_im_window_pos(x,y)
here's the current behaviour

the kitty one for reference, we might want to do something like that on the future.

duarm · 2022-10-28T20:24:47Z

hello, I won't be able to work on this for a few days, couldn't make the preedit callbacks work, but aside from this small problem, it's quite ready, feel free to finish or just hack for now. The problem is that if you're typing normal character (not composing a character, like with japanese), typing really fast is slightly less responsive because of rofi_set_im_window_pos.
Once I get back I might try again. Thanks for the good work.

DaveDavenport · 2022-10-28T20:54:42Z

I noticed that with fcitx running, esp the first character takes very long to appear on my test machine. (i5-6500t, 8gb ddr4, nvme disk, not fast, but not a slow machine either).
This is kinda not-ok, so might make it a compile time flag.

duarm · 2022-10-29T05:18:43Z

This doesn't happen on my machine, are you on fcitx5 (latest version)? also, when you're using an input method, it sometimes messes your autorepeat (xset r rate) when changing between servers, try running xset r rate 300 50 or something like that again and see if that was the problem. If it was, we can't do nothing about it.

DaveDavenport · 2022-10-29T12:29:06Z

Installed the fcitx5 (deb) on my pop-os test machine. Its up2date but no idea what version.
But there was a clear 'delay' on the first character on first run.

DaveDavenport · 2022-11-08T17:42:57Z

@sardemff7 is that 'changes requested' still open or can this be merged?

DaveDavenport · 2022-11-18T07:25:56Z

Users report (and shown) extreme slowdown by this patch making rofi near unusable.
(see the discussion I tagged @duarm in)

I will revert unless we can find a solution.

duarm · 2022-11-23T02:35:31Z

revert this for now, tested yersterday under heavy load and it's quite unusuable indeed. There's something wrong with the event forwarding, sometime the keys are swalloed without the string being commited, only to both be commited once the next event is forwarded. This becomes apparent if the code cant run fast enough, which is caused by the window position being updated every draw as I've noted before, we're paying to much round-trip with the server.

Ideally this shouldn't happen, even under heavy loads, this pr requires some more careful handling with the server (only communicate when necessary, throttling, etc.), I'll try to fix when I get the time again, or someone with more knowledge can step in.

github-actions · 2022-12-25T00:02:01Z