Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add android ime support #2993

Closed

Conversation

lucasmerlin
Copy link
Contributor

@lucasmerlin lucasmerlin commented Aug 1, 2023

This adds Support for opening the Soft Keyboard on android and passing through TextInputState events to the application.
TextInputState contains the currently edited text as string, the current selection / cursor range and a optional compose region.
This enables applications to support text input on android including autocomplete and autocorrect.
While the TextInputState would currently be exclusive to android, I think it should be possible to implement the same api on iOS so there is a unified api for soft keyboards.

I implemented this as a new event because I think the current Ime event wouldn't work well for mobile text input, but it might also be possible to make it work with some tweaks. There would at least need to be some way to set the current text field content, so android has some context on what to predict for autocomplete.

I implemented this api for egui here: lucasmerlin/egui@d47677a

See it in action

Screen_Recording_20230708_144153.mp4

A disadvantage of this api is, that it'd probably be difficult if not impossible to reliably edit rich text with this. The alternative would be to implement some kind of shared abstraction over InputConnection and UITextInput but these apis are a lot more complicated and for android it'd be difficult to implement it with the game-activity approach.

I'm bad at naming things, so there is probably a better name for the event than TextInputState.

Some other things missing to allow a great keyboard experience on mobile devices:

  • Inset events / some way to query the current insets, so one can make sure the text field is not covered by the keyboard
  • Support setting keyboard flags, for e.g. number / password / email input, enabling / disabling autocorrect, toggling capitalization for sentences

Part of #1823.


  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work on that, even though I'm not familiar with android, but I do use IME with winit and would like to see it improved.

I don't like that we have a separate event for no real reason and that we have IME separate with your approach, which shouldn't be the case.

Could you look at https://wayland.app/protocols/text-input-unstable-v3#zwp_text_input_v3:request:set_surrounding_text and https://wayland.app/protocols/text-input-unstable-v3#zwp_text_input_v3:event:preedit_string ?

I think it's pretty much what you've added, except for the compose key? We could extent the Preedit API if it matters or just adjust IME API to better account for Android here.

In general, if you'd need help with other platforms I could implement some of them (Wayland/macOS/X11/and maybe Windows).

src/window.rs Outdated
Comment on lines 1115 to 1126
/// Opens the IME input (soft keyboard) if the platform supports it.
/// Currently only supported on Android.
#[inline]
pub fn begin_ime_input(&self) {
self.window.begin_ime_input();
}

/// Hides the IME input (soft keyboard).
#[inline]
pub fn end_ime_input(&self) {
self.window.end_ime_input();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks redundant. We have set_ime_allowed where true means that the soft board will be opened, because we allow ime. false will hide the IME, this works the same on Wayland btw and its softboards, so this API is redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, egui just calls set_ime_allowed once on application start, I think that confused me. I've changed this in my egui branch and removed the begin_ime_input and end_ime_input functions

src/window.rs Outdated
Comment on lines 1128 to 1130
pub fn set_text_input_state(&self, state: TextInputState) {
self.window.set_text_input_state(state);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you get a bit more details on what this particular function tries to do? In general, that's true that we'd nee a way to set an IME state, like position of the ime, surrounding text, etc.

I think what we probably should do is to have a set_ime_state(state: Option<ImeState>) where you can set that information + position of the input method, content hint, and similar.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've replaced this with a set_ime_surrounding_text function similar to the one from wayland you sent.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does android have a notion of atomic state changes? Because wayland has for example, and using some big InputState would be preferred for it, but I can refactor it myself later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean, but I'd guess not. The android game text input library this relies on is quite simple. The idea is that this is called once when a text field is first focused and when the selection gets updated by the user.

src/event.rs Outdated Show resolved Hide resolved
src/event.rs Outdated Show resolved Hide resolved
@lucasmerlin
Copy link
Contributor Author

Thanks for the review @kchibisov!
I see how having an additional api is not ideal. I'll try to get it working with the existing Ime event.

@kchibisov
Copy link
Member

I see how having an additional api is not ideal. I'll try to get it working with the existing Ime event.

I'm just saying that nothing stops us from extending the existing event, especially given that what you've added looks similar to it.

@lucasmerlin
Copy link
Contributor Author

I've updated my code and added a Ime::Replace event. It looks pretty similar to the Preedit event but works differently: When this event is called the whole text (the one that was previously set with set_surrounding_text) should be replaced with the updated text.

I can't use the Preedit and Commit events, since the android game text input has no such thing as a commit. Whenever the user enters text, we get the complete updated text and a compose and selection region. One could maybe estimate whether a commit has happened based on the existence of a compose region and by diffing the updated text but I don't think that'd work reliably.

@kchibisov
Copy link
Member

I've updated my code and added a Ime::Replace event. It looks pretty similar to the Preedit event but works differently: When this event is called the whole text (the one that was previously set with set_surrounding_text) should be replaced with the updated text.

On Wayland we have a similar concept, it's called https://wayland.app/protocols/text-input-unstable-v3#zwp_text_input_v3:event:delete_surrounding_text .

Besides, users can't rely on the replace event alone, we need Commit event when the input is done into the editor. Maybe you could adapt the technique from the Wayland protocol, which looks similar to what you're doing? Read the https://wayland.app/protocols/text-input-unstable-v3#zwp_text_input_v3:event:done it should provide an algorithm to process events, which we can adopt on Android.

The key thing here is that Commit is mandatory for the work cross platform, Preedit is not that useful, but you can mark the selection with it.

Basically you should send Commit each time the text is written into the widget, if each input implies text being written into the keyboard widget, then you should send commit for each update. The users on the other side, should reply with set_surrounding_text in response to Commit, so the Replace/delete surrounding text could work. Some clients for example, can't work with surrounding text, because they forward the input into some side channel (think terminal emulator).

However, maybe Android should Commit the text when the keyboard is being hidden or something like that?

@lucasmerlin
Copy link
Contributor Author

I cloud in theory do something like this, basically diff the current and previous TextInputState to guess whether there should be a preedit or commit event:

let events = if let Some(compose_region) = &ime_state.compose_region {
    vec![event::Ime::Preedit(ime_state.text[compose_region.start..compose_region.end].to_string(), Some((compose_region.start, compose_region.end)))]
} else {
    if let Some(previous_text_input_state) = &self.previous_text_input_state {
        if let Some(prev_compose_region) = &previous_text_input_state.compose_region {
            vec![
                event::Ime::Preedit("".to_string(), None),
                event::Ime::Commit(previous_text_input_state.text[prev_compose_region.start..prev_compose_region.end].to_string()),
            ]
        } else {
            vec![]
        }
    } else {
        vec![]
    }
};

But I don't think this would ever work reliably, no matter how many edgecases are handled.

That's why I added the Replace event, which would replace the Commit event for android. If the application doesn't handle the Replace event, it won't have keyboard input support on android. Maybe Replace is a bad name, I guess it could be called CommitReplace or something like that. Since this event would likely be android specific, maybe it could also be a android exclusive platform extension, if there is such a thing for events?

However, maybe Android should Commit the text when the keyboard is being hidden or something like that?

That would be an idea. While typing the whole text could be sent as a Preedit. This would mean the whole text is highlighted while typing though, instead of the current compose region. Also when the keyboard is hidden the text field has already lost focus, so it would probably not get the final commit event and the text would be dismissed?

Some clients for example, can't work with surrounding text, because they forward the input into some side channel (think terminal emulator).

I see how that would be a problem. I think if you don't set any keyboard flags(disable autocomplete/autocorrect), you should reliably be able to get the last typed character from the end of the string in the Ime::Replace event. By comparing the length you could see if the user pressed backspace. But that would also be a horrible hack. I have no better idea though.

@kchibisov
Copy link
Member

The issue with the Replace alone is that it's not clear what it tries to do, because this event doesn't map into any other platform directly. The idea behind Replace is likely fine, but what if don't set surrounding text? Could we just get a regular input and such?

Preedit is optional, only Commit is mandatory, because that's what is being inserted in the end. Surely you have some logic in egui to determine when the text is inserted? Do you just count the last state of the Replace as it? You should have some heuristic for that, I guess.

@kchibisov
Copy link
Member

Hm, maybe we could do that differently, could you compute the difference to replace based on the surrounding text provided by user, send replace event telling it to Delete the text from the start/end of the cursor, and then you send a Commit event resulting in effectively what the Replace does?

The only issue I think is the selection or something like that, but it probably should be transparent to the user?

It just looks like the Replace event you're proposing is basically what Wayland spec suggests (2-4 points)?

  1. Replace existing preedit string with the cursor. 2. Delete requested surrounding text. 3. Insert commit string with the cursor at its end. 4. Calculate surrounding text to send. 5. Insert new preedit text in cursor position. 6. Place cursor inside preedit text.

@kchibisov kchibisov added the C - waiting on author Waiting for a response or another PR label Aug 3, 2023
Comment on lines +922 to +928
pub fn set_ime_allowed(&self, allowed: bool) {
if allowed {
self.app.show_soft_input(true);
} else {
self.app.hide_soft_input(true);
}
}
Copy link
Member

@kchibisov kchibisov Aug 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you send this change separately? I can send it myself though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could make a separate PR with this change, it won't be very useful without the rest of these changes though, since android unfortunately doesn't send key events for the soft keyboard (See the first note here). So the keyboard would open but nothing will happen when the user starts typing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I think it does so with the game activity and recent changes merged here #3004 , no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand, those changes are only relevant to Hardware Keyboards (e.g. usb keyboards plugged in your phone), but maybe I'm missing something. @rib can you clarify?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unicode mapping support from #3004 is also for soft keyboards which also generate KeyEvents, though I do tend to think of IME vs KeyEvents handling as two entirely separate input handling modes (and that's the case for other platforms too) and I'm not sure that it'd be ideal to make this a common API for those separate modes.

Overloading set_ime_allowed for showing the keyboard in situations where you are happy to handle KeyEvents instead of IME events seems like it could lead to non-portable code because calling this (in the same app) for other platforms will enable IME handling / events which might not be desirable.

It seems like ideally there should be a separate API for being able to show/hide mobile soft keyboards which would have no effect on platforms that don't need a virtual keyboard (but they may otherwise have IME support that the app is not trying to enable).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, on Wayland it's tied to the set_ime_allowed as well, and it's tied to that on the protocol level though. It's not written in the spec, but that's the defacto how it works. I guess if you don't set surrounding text you won't get any sort of ime-ish input, no?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern isn't so much whether you'd get IME events on Android, (GameActivity doesn't differentiate whether you've set some prior text or not, so you would get IME events on Android) but that you'd be enabling IMEs on other platforms when you don't necessarily want that (if your app is cross-platform).

If you want to write portable code that currently works in terms Key events (e.g. for work our engine doesn't currently have full IME support) then you may want to be able to show a virtual keyboard at times on Android (and handle corresponding Key events) but wouldn't want to enable IMEs generally.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rib you enable IME when you write text, on Android you can't really write text without a soft keyboard. The same applies for others, if you write text you generally want an IME enabled, unless you're using the keyboard input where any sort of IME input is disallowed.

Like for example a game using soft keyboard must provide me a way to input IME, because it's likely some field and what if I want to write some script?

On desktop you just sometimes want to remove IME when you actually game, but not chat, which is not the case on Android because you're using on screen controls.

On macOS for example not enabling IME won't let you type in some cases anything meaningful.

Also, enabling IME doesn't mean that you'll receive only IME events, you could still receive regular keys, it's just you indicate that you can receive extra IME events and you can provide a feedback for IME when needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I'm just cautious about conflating the two different use cases because I'm aware that it would affect our engine currently since we don't have any IME support right now but do show/hide the virtual keyboard on Android.

It might be OK (or at least it wouldn't be too tricky to add a cfg guard for android if necessary).

If we still get Key events on other platforms while IME is enabled (that's the case on Android) that probably generally makes this less of an issue but for desktop IMEs you would normally specify the position of the overlay window and we currently don't have any support for doing that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is also for soft keyboards which also generate KeyEvents

I just tried this, and none of the soft keyboards I have on my phone triggered any key events, only the TextEvent is triggered.

@lucasmerlin
Copy link
Contributor Author

Hm, maybe we could do that differently, could you compute the difference to replace based on the surrounding text provided by user, send replace event telling it to Delete the text from the start/end of the cursor, and then you send a Commit event resulting in effectively what the Replace does?

Yeah, something like that could work. I could use the Delete Event to delete the current text and use commit to replace it with the newly received text. Then we would need a new event to update the cursor and compose position though, or else the cursor would always be shown at the end of the text. I could give this a go. In the end, this solution would still require a non standard event (update cursor position, haven't found anything like that in the wayland events) to be added though, so I'm not sure if this is better than the replace solution.

I tried to implement a diffing algorithm, but I haven't gotten it to work. Since the android keyboard could in theory randomly replace text independent of the cursor position, I feel like a solution that uses diffing would always have some edge cases and bugs.

@rib do you maybe have an idea how this could be solved? I think you originally also tried to use the existing ime events?

@kchibisov
Copy link
Member

We can commit the cursor along side the commit, it's not a big deal.

You also don't have to diff actually, you could just delete everything that was before and commit new, so it's exactly like replace, you just send 2 events.

@lucasmerlin
Copy link
Contributor Author

@kchibisov I removed the Replace event and added a DeleteSurroundingText event, this seems to work well. Is this what you had in mind?

I think it might actually also be possible to use the Preedit event instead of Surrounding Text, if I set the text to "" and the range to 0 and usize::MAX, the application should be deleting the current text, if I understand correctly? But this seems really hacky to me.

@rib
Copy link
Contributor

rib commented Aug 10, 2023

Sorry for the delay following up here.

I do generally agree with the direction of trying to work with the Winit Ime events if we can make it work (based on adding a DeleteSurroundingText event).

It's unfortunate that the GameActivity interface we get doesn't give us more fine grained events but the way Android's InputConnection works isn't totally dissimilar to other IME protocols so I feel like we should try to avoid leaking the current limitations of GameActivity if at all possible.

It would be nice too if it's not necessary for a toolkit like Egui to have to treat Android as a special case when it comes to input method handling.

is also for soft keyboards which also generate KeyEvents

I just tried this, and none of the soft keyboards I have on my phone triggered any key events, only the TextEvent is triggered.

Curious that you see no KeyEvents at all.

I certainly see KeyEvents for at least basic virtual keyboard input (essentially just ascii characters).

We currently rely on these events in our engine while we haven't hooked up IME support yet but it seems like it may really depend on the implementation of different virtual keyboards as to what keys might generate KeyEvents.

I had actually thought more keys would be supported so we could rely on KeyEvents as a stop gap for a little longer but yeah it looks like it's not really something that can be relied on - especially if some virtual keyboards in fact don't emit any KeyEvents :/

event::Ime::DeleteSurroundingText {
before_length: usize::MAX,
after_length: usize::MAX,
},
Copy link
Contributor

@rib rib Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleting surrounding text with usize::MAX might be a foot gun at some point.

IME edits are relative so the current cursor because conceptually they could be used within a large body of text (e.g. think 10k word Google Doc) so I'm a bit concerned that asking the toolkit to delete like this could potentially result in it deleting someone's larger document unless the toolkit additionally tracks the bounds of the surrounding context it has given Winit and treats that like a sandbox (but that's not currently clearly specified afik).

Maybe we can reliably track the size of the current text so we can more precisely delete just the text from the last update?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with that, doing delete all is sort of weird. Given that it's all about the feedback loop with setSurroundingText request, maybe we can base everything on it, since
IME shouldn't touch anything outside of the user set surrounding text, no?

DeleteSurroundingText {
before_length: usize,
after_length: usize,
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we're exposing a selection region it seems like we need to clarify what the before/after are relative to.

On Android the before would be relative to the start of the selection and the after is relative to the end of the selection.

The Winit docs don't currently say whether we're following the Android notion that the "cursor" is just a special case of a selection where start == end.

With that model though we also need to consider how we will delete / update the selection region?

With the InputConnection API there is a separate setSelection that would make it possible to set the selection start==end for being able to delete all the text.

I'm currently thinking we might need to add a SetSelection event for being able to just update the start/end indices of the selection.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the InputConnection API there is a separate setSelection that would make it possible to set the selection start==end for being able to delete all the text.

I'm currently thinking we might need to add a SetSelection event for being able to just update the start/end indices of the selection.

The selection should be mapped to the concept of the cursor, so it's effectively the same as in preedit. The Commit could set the cursor as well, but I guess the virtual keyboard could move the cursor on its own? Yeah, it would be fine to have a separate method to advance the selection like you suggest, but we need to make it general on what selection is and probably encapsulate it into a separate type ImeSelection or something.

Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides we should also communicate the Enabled/Disabled state somehow, maybe we should just sent Enabled initially and call it a day?

Comment on lines +902 to +903
before_length: usize,
after_length: usize,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We must specify the unit in which those are, because it's not clear whether it's a byte or unicode codepoint based indecies. Some API we have already is in bytes, not code points. So logically this API should be in bytes as well. Like for example Wayland uses bytes for that as well, and we can't really convert to code points in wayland case, because we simply don't know anything about the text around.

Maybe we should have a variant like enum Amount { Byte(usize), Codepoint(usize) }, so the client can deal with all of that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels odd to expose the offsets as bytes. I can sort of see why the wayland protocol would use bytes but forcing toolkits like Egui to have to somehow deal with byte offsets within Rust Strings sounds like it would be horrible to deal with.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wayland backend should be able to map the byte offsets to the corresponding utf8 codepoint and can somehow treat it as an error / undefined behaviour if you ever get a byte offset into the middle of a utf8 character.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rib it can't do so for delete surrounding text. Unless we stash surrounding text around. The rust string a sliced with bytes though, so it works pretty much ok.

Comment on lines +894 to +896
/// If compose_region is Some, the compose region should be updated after replacing the
/// text.
compose_region: Option<(usize, usize)>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't understand what the user should even do with that, our docs must be clear here, it's not something common on any other backend/ime stuff.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Android shows a underline under the word thats currently being edited. That is the compose region. But since no other platform has this (even iOS doesn't seem to have something like this) I think we could also remove it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Others usually have a preedit and the update is like pending to be inserted.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Android's compose region is just different terminology for the "preedit" string.

The preedit string is the transient text that would be shown that may preview a tentative completion e.g. for kanji.

I think there are essentially three high level spans of text:

  1. the "surrounding" text
  2. the "preedit" text (or "compose region" for Android)
  3. the "selection" or "cursor" (empty selection)

I think the main thing that's changing here is that we need to introduce the notion that there is surrounding text which Winit didn't previously expose - though this isn't something unique to Android either.

Android has a slightly more general decoupling of the selection/cursor and the preedit string - while other IMEs might only track a cursor (can be considered an empty selection) or the cursor/selection will always be a subset of the preedit string.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the context of the Winit public API we should probably rename "compose_region" here to "preedit_region" or "preedit_span" perhaps.

Conceptually this would be committing some surrounding text and also updating the span for the selection and the span for the preedit string in one event.

Comment on lines +891 to +893
/// If selection is Some, the selection / cursor position should be updated after
/// placing the `content`.
selection: Option<(usize, usize)>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably having a separate Selection type at this point unified with Preedit would be the way forward. Like the selection is used throughout the docs consistently, we should also decide on metrics we use and be explicit, I think on android it's unicode codepoints, but everything else we have is bytes?

@kchibisov kchibisov added this to the Version 0.29.0 milestone Aug 11, 2023
@xorgy
Copy link
Contributor

xorgy commented Jan 22, 2024

Hey @lucasmerlin just wanted to let you know that I am resurrecting this work. I'm going to split off a version without the API changes first. I'll try to make sure the commits retain your attribution. :+ )

If you're still interested in this, I'll rope you in to the review.

@lucasmerlin
Copy link
Contributor Author

Awesome, good luck @xorgy!
I'm happy to help / review, and I'm still interested in landing this. Let me know if you need access to my repo so you can push to the branch of this PR, or if there is some other way to do this. Otherwise just open a new one, and I can close this one with a notice.

@jb55
Copy link

jb55 commented Apr 10, 2024

I tried to follow the discussion, what's the next steps here? I would be happy to help since I am currently using it for my android egui app.

@lucasmerlin
Copy link
Contributor Author

@jb55 Maybe a you could try to get basic text entry working with winit's existing api. There wouldn't be autocomplete support but it would be a first step and maybe enough for some apps.

Also the people from linebender are very interested in android keyboard support, theres a long discussion here: https://xi.zulipchat.com/#narrow/stream/351333-glazier/topic/Comparisons.20against.20Winit

I think @xorgy also looked into this but I'm not sure if he made any progress

@kchibisov
Copy link
Member

The API could be extended, it's just certain aspects should be shared. Like the actual insert to widget part should be shared, etc, so so users will write some kind of cross platform code.

@ardocrat
Copy link

ardocrat commented Apr 15, 2024

FYI with current implementation on Android you can not get input from non-english languages as keycodes, at Java side you can get them at Activity::dispatchKeyEvent callback, characters field of KeyEvent. Ideally will be to provide already converted text (also with pressed shift) at input event, like Android made for non-english language.

Also I have problem with back button when keyboard appears after call of AndroidApp::show_soft_input from Rust code and press on at any character at keyboard. Some input listener keeps to receive events and blocking others.

Cyannide added a commit to Cyannide/winit that referenced this pull request Apr 23, 2024
@xorgy
Copy link
Contributor

xorgy commented May 23, 2024

I have this now rebased (I can't update @lucasmerlin 's branch myself)
master...xorgy:winit:android_ime_support

But Android 11 (API 30) and later make it more painful to get the soft keyboard to show up, as others have discovered.

@xorgy
Copy link
Contributor

xorgy commented May 29, 2024

So I have managed to get the soft keyboard to show up, but the only way I was able to do it was by calling into the WindowInsetsController with JNI. Possible good news for #1823 is that I think we are getting keyboard events from the IME;
the mixed news is that it doesn't show any complex IMEs currently (won't switch to them). Proper IMEs will likely require an InputConnection subclass or other such thing.

screen-20240529-162246.mp4

@lucasmerlin
Copy link
Contributor Author

Closing this in favor of #3787 by @xorgy, which has a better approach

@xorgy
Copy link
Contributor

xorgy commented Jul 17, 2024

And just to clarify, #3787 only opens the soft keyboard (which can send key events). Real IME on Android is looking like it will require specific support from the Activity, or at very least, the introduction of a View subclass (either when building the Activity, or possibly injected with runtime dex loading).

@MarijnS95
Copy link
Member

MarijnS95 commented Jul 17, 2024

or at very least, the introduction of a View subclass (either when building the Activity, or possibly injected with runtime dex loading).

Fyi this is actively beeing prototyped and worked on. Will be in xbuild first but can easily be ported to cargo-apk too.

(It can also be hacked into a Rust app with the InMemory dex class loader, but ehh 😅)

@kchibisov
Copy link
Member

Nothing stops winit form using the xbuild btw, it's just we had issues with it once we tried to switch the CI.

@MarijnS95
Copy link
Member

I don't think winit must absolutely migrate to xbuild right now, just saying that I might be working on some solutions to make embedding Java with otherwise-native apps a bit nicer and easier :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C - waiting on author Waiting for a response or another PR DS - android
Development

Successfully merging this pull request may close these issues.

8 participants