Fix IM and virtual keyboard handling #1169
Conversation
| pub(crate) popup_geometry_callback: fn(&D, &WlSurface) -> Rectangle<i32, Logical>, | ||
| pub(crate) new_popup: fn(&mut D, PopupSurface), | ||
| pub(crate) dismiss_popup: fn(&mut D, PopupSurface), |
There was a problem hiding this comment.
Yes. Otherwise you force IME into the users.
|
The other question is that the The issue is that parent geometry could change without client losing focus and the popup needs a way for it to be updated. The most reliable solution for popup to actually ask geometry during space renderer each time from the compositor, so it ensures that everything is up to date. How something like that should be approached? I'm pretty sure it's not the first time something like that happens and some other elements may also need to query for the parent geometry all the time. |
|
Converting to draft since it needs more tuning to actually work, and probably will have more to come. I'd still like to know what to do with |
kchibisov
force-pushed
the
fix-ime-popup
branch
from
e3d2f4f to
ff50a93
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #1169 +/- ##
==========================================
- Coverage 22.02% 21.94% -0.08%
==========================================
Files 154 154
Lines 23972 24053 +81
==========================================
Hits 5279 5279
- Misses 18693 18774 +81
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
kchibisov
force-pushed
the
fix-ime-popup
branch
from
ff50a93 to
b6e2bc2
Compare
kchibisov
force-pushed
the
fix-ime-popup
branch
2 times, most recently
from
aa67f5e to
9e86a1d
Compare
kchibisov
changed the title
|
Sorry I am a bit late to the party, it can probably be merged the way it is for now I guess? |
|
Do we really need to have an implementation of dismiss_popup? Can we not just call "PopupManager::dismiss_popup" in the ime code? |
I expose all the state needed to handle collisions, there's a
It could be a
Given that we just pass things around it could be your IME doing so for whatever reason. I don't have that issue with my IME, for example. This is also has nothing to do with the way things work on this PR in general, since it just uses the smithay infra for that and what I did is basically moved the whole logic to bring up/down IME into the activate stuff. |
Yes, because |
|
I used fcitx5, which IME are you using? |
but would it not be sufficient to check if we have a popup, then just call it? |
Call what? We don't have |
kchibisov
force-pushed
the
fix-ime-popup
branch
from
9e86a1d to
ff3e3b1
Compare
Could we at least make a correct example in anvil then? Like just the 90% correct one or something?
Well, I guess I should just create an issue for this then |
Alright, in that case, could we just not check if the popup manager is used? Like we do not really have an alternative for it right now anyway? |
The virtual keyboard was always sending modifiers resulting in erronious client breakages. The keymap was also read via regular file routines instead of mmap'ing.
This commit fixes multiple issues identified during using the IM:
1. Make IM activation/deactivation based on text-input::{enable/disable}
and keyboard focus leave, instead of just keyboard focus changes.
2. Send `text_input::enter` on text-input creation, since some bind
global after getting focus.
3. Don't activate IME for clients which don't have text_input
instance, otherwise keyboard input will be disabled in general due to
grabbing.
4. Add `new_popup` and `dismiss_popup` to update IM popup in response
to IM text_input::{enable/disable} events.
5. Use text-input coordinates for input-method popup positioning
IM popup positioning. The width and height mark the region you
shouldn't obscure.
6. Fix serial handling to match input-method-v2 and text-input-v3
specifications. input-method-v2 also requires counting amount
of done events and using it to discard commit when it doesn't
match text-input. text-input-v3 just wants serial with value
equal to the number of commits being sent for the client.
kchibisov
force-pushed
the
fix-ime-popup
branch
from
ff3e3b1 to
3c33822
Compare
I don't want to do that myself, but feel free. Only cosmic can probably handle popup re constaints logic correctly.
I just tell downstream that popup should be dissmissed, how it's done is up to the downstream. Anvil uses |
|
|
Alright I will try and see if I can figure something out on my own then. Anyway great work 🙂 , focus seems to work a lot better now at least. |
The series is split into 2 commits, the first one addresses the virtual keyboard issues I've faced and the second tries to solve IM issues, which were left on the table. Each commit has a description on what it tried to fix.
--
This is by far the ugliest thing I ever wrote, but to make it nice I'd need to delete all the input handling in smithay, which I don't have time to do.