Skip to content

GameActivity onTextInputEvent getting called twice #186

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

Open
jb55 opened this issue Mar 12, 2025 · 4 comments · May be fixed by #191
Open

GameActivity onTextInputEvent getting called twice #186

jb55 opened this issue Mar 12, 2025 · 4 comments · May be fixed by #191

Comments

@jb55
Copy link

jb55 commented Mar 12, 2025

When I press a on the virtual keyboard

static void onTextInputEvent(GameActivity* activity,
                             const GameTextInputState* state) {
    struct android_app* android_app = ToApp(activity);
    LOGV("got text input event: '%.*s'", state->text_length, state->text_UTF8);
    pthread_mutex_lock(&android_app->mutex);
    if (!android_app->destroyed) {
        android_app->textInputState = 1;
        notifyInput(android_app);
    }
    pthread_mutex_unlock(&android_app->mutex);
}
03-11 17:00:55.721 28026 28026 V android_activity: got text input event: 'a'
03-11 17:00:55.721 28026 28086 I RustStdoutStderr: Notifying Input Available    
03-11 17:00:55.722 28026 28026 V android_activity: got text input event: 'aa'

only seems to happen on one of my devices. bizarre.

jb55 referenced this issue Mar 12, 2025
With the way events are delivered via an `InputQueue` with
`NativeActivity` there is no direct access to the underlying KeyEvent
and MotionEvent Java objects and no `ndk` API that supports the
equivalent of `KeyEvent.getUnicodeChar()`

What `getUnicodeChar` does under the hood though is to do lookups into a
`KeyCharacterMap` for the corresponding `InputDevice` based on the
event's `key_code` and `meta_state` - which are things we can do via
some JNI bindings for `KeyCharacterMap`.

Although it's still awkward to expose an API like
`key_event.get_unicode_char()` we can instead provide an API that
lets you look up a `KeyCharacterMap` for any `device_id` and
applications can then use that for character mapping.

This approach is also more general than the `getUnicodeChar` utility
since it exposes other useful state, such as being able to check what
kind of keyboard input events are coming from (such as a full physical
keyboard vs a virtual / 'predictive' keyboard)

For consistency this exposes the same API through the game-activity
backend, even though the game-activity backend is technically able to
support unicode lookups via `getUnicodeChar` (since it has access to the
Java `KeyEvent` object).

This highlighted a need to be able to use other `AndroidApp` APIs while
processing input, which wasn't possible with the `.input_events()` API
design because the `AndroidApp` held a lock over the backend while
iterating events.

This changes `input_events()` to `input_events_iter()` which now returns
a form of lending iterator and instead of taking a callback that gets
called repeatedly by `input_events()` a similar callback is now passed
to `iter.next(callback)`.

The API isn't as ergonomic as I would have liked, considering that
lending iterators aren't a standard feature for Rust yet but also since
we still want to have the handling for each individual event go via a
callback that can report whether an event was "handled". I think the
slightly awkward ergonomics are acceptable though considering that
the API will generally be used as an implementation detail within
middleware frameworks like Winit.

Since this is the first example where we're creating non-trivial Java
bindings for an Android SDK API this adds some JNI utilities and
establishes a pattern for how we can implement a class binding.

It's an implementation detail but with how I wrote the binding I tried
to keep in mind the possibility of creating a procmacro later that would
generate some of the JNI boilerplate involved.
@jb55
Copy link
Author

jb55 commented Mar 12, 2025

I thought it might be a double-callback register or something but this didn't fix it (still get an a and then aa event)

diff --git a/android-activity/game-activity-csrc/game-activity/GameActivity.cpp b/android-activity/game-activity-csrc/game-activity/GameActivity.cpp
index 4dbc9e83ecac..cda7e6b717e5 100644
--- a/android-activity/game-activity-csrc/game-activity/GameActivity.cpp
+++ b/android-activity/game-activity-csrc/game-activity/GameActivity.cpp
@@ -405,6 +405,10 @@ static int mainWorkCallback(int fd, int events, void *data) {
 // ------------------------------------------------------------------------
 static thread_local std::string g_error_msg;
 
+extern "C" {
+	void glue_onTextInputEvent(GameActivity* activity, const GameTextInputState* state);
+};
+
 static jlong initializeNativeCode_native(
     JNIEnv *env, jobject javaGameActivity, jstring internalDataDir,
     jstring obbDir, jstring externalDataDir, jobject jAssetMgr,
@@ -488,9 +492,9 @@ static jlong initializeNativeCode_native(
     GameActivity_onCreate_C(code, rawSavedState, rawSavedSize);
 
     code->gameTextInput = GameTextInput_init(env, 0);
+
     GameTextInput_setEventCallback(code->gameTextInput,
-                                   reinterpret_cast<GameTextInputEventCallback>(
-                                       code->callbacks.onTextInputEvent),
+                                   reinterpret_cast<GameTextInputEventCallback>(glue_onTextInputEvent),
                                    code);
 
     if (rawSavedState != NULL) {
diff --git a/android-activity/game-activity-csrc/game-activity/native_app_glue/android_native_app_glue.c b/android-activity/game-activity-csrc/game-activity/native_app_glue/android_native_app_glue.c
index 6b70cfee904f..f6a4221bdff1 100644
--- a/android-activity/game-activity-csrc/game-activity/native_app_glue/android_native_app_glue.c
+++ b/android-activity/game-activity-csrc/game-activity/native_app_glue/android_native_app_glue.c
@@ -668,17 +668,6 @@ void android_app_clear_key_events(struct android_input_buffer* inputBuffer) {
     inputBuffer->keyEventsCount = 0;
 }
 
-static void onTextInputEvent(GameActivity* activity,
-                             const GameTextInputState* state) {
-    struct android_app* android_app = ToApp(activity);
-    pthread_mutex_lock(&android_app->mutex);
-    if (!android_app->destroyed) {
-        android_app->textInputState = 1;
-        notifyInput(android_app);
-    }
-    pthread_mutex_unlock(&android_app->mutex);
-}
-
 static void onWindowInsetsChanged(GameActivity* activity) {
     LOGV("WindowInsetsChanged: %p", activity);
     android_app_write_cmd(ToApp(activity), APP_CMD_WINDOW_INSETS_CHANGED);
@@ -697,6 +686,18 @@ static void onContentRectChanged(GameActivity* activity, const ARect *rect) {
     pthread_mutex_unlock(&android_app->mutex);
 }
 
+void glue_onTextInputEvent(GameActivity* activity, const GameTextInputState* state) {
+    struct android_app* android_app = (struct android_app*)activity->instance;
+    LOGV("got text input event: '%.*s'. composingRegion: %d %d", state->text_length, state->text_UTF8,
+		    state->composingRegion.start, state->composingRegion.end);
+    pthread_mutex_lock(&android_app->mutex);
+    if (!android_app->destroyed) {
+        android_app->textInputState = 1;
+        notifyInput(android_app);
+    }
+    pthread_mutex_unlock(&android_app->mutex);
+}
+
 // XXX: This symbol is renamed with a _C suffix and then re-exported from
 // Rust because Rust/Cargo don't give us a way to directly export symbols
 // from C/C++ code: https://github.com/rust-lang/rfcs/issues/2771
@@ -714,7 +715,7 @@ void GameActivity_onCreate_C(GameActivity* activity, void* savedState,
     activity->callbacks->onTouchEvent = onTouchEvent;
     activity->callbacks->onKeyDown = onKey;
     activity->callbacks->onKeyUp = onKey;
-    activity->callbacks->onTextInputEvent = onTextInputEvent;
+    activity->callbacks->onTextInputEvent = NULL;
     activity->callbacks->onConfigurationChanged = onConfigurationChanged;
     activity->callbacks->onTrimMemory = onTrimMemory;
     activity->callbacks->onWindowFocusChanged = onWindowFocusChanged;
diff --git a/android-activity/src/game_activity/mod.rs b/android-activity/src/game_activity/mod.rs
index 7cf38949a34d..d460f69e2104 100644
--- a/android-activity/src/game_activity/mod.rs
+++ b/android-activity/src/game_activity/mod.rs
@@ -832,6 +832,7 @@ impl<'a> InputIteratorInner<'a> {
                 // before the java main thread really updates the state.
                 if (*app_ptr).textInputState != 0 {
                     let state = self.native_app.text_input_state(); // Will clear .textInputState
+                    log::debug!("sending InputEvent::TextEvent({state:?})");
                     let _ = callback(&InputEvent::TextEvent(state));
                     return true;
                 }

@jb55
Copy link
Author

jb55 commented Mar 13, 2025

Hoping:

will fix this

@jb55
Copy link
Author

jb55 commented Mar 13, 2025

Could this be why? "GameActivity loading native library twice" -> double callbacks? 🤔 spotted at:

by @znakeeye

@jb55
Copy link
Author

jb55 commented Mar 15, 2025

Just confirmed that 4.0.0 fixes this...

@jb55 jb55 linked a pull request Mar 15, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant