sokol_app.h Android: key_up/down, show_keyboard, clipboard get/set, quit_requested

[ghost]

Updated some parts of sokol_app for Android:

• The back key will send the "SAPP_EVENTTYPE_QUIT_REQUESTED" instead of just shutting down
• _sapp_android_key_event send all SAPP_EVENTTYPE_KEY_DOWN/SAPP_EVENTTYPE_KEY_UP
• The clipboard is supported using JNI, the get/set functions handle the conversion to MUTF8
• show_keyboard should work properly now
• keyboard_shown added, even if it finds if the keyboard is visible in a hackish way since there is no standard way to get this information

[floooh]

Nice :) I thought that virtual keyboard support from plain C was a hopeless case, but this doesn't look too bad actually ;)

Just FYI, I'll delay all sokol_app.h PRs until I'm tackling the multiwindow PR, hopefully this won't require too many changes in backend code, especially not on mobile platforms which won't have multiwindow-support anyway. But just as a heads up that other sokol_app.h PRs will be delayed a bit more.

[ghost]

It still feels so hackish to manage, I hope it will works on all devices.
No problems for the delay, I'll update my PR once the multiwindow is integrated in master.

[Wertzui123]

Any idea when this will be merged? Looks like you stopped working on the multiwindow PR.

[floooh]

It's back on my todo list for the next round of PRs (after I'm done with sokol_spine.h).

[floooh]

Ok, I'll start looking into this PR now (low intensity though, because I'll also need to look into a couple of other smaller things, also sokol_spine.h is still not done, but I'll push that back a little more).

[floooh]

@olivierbuisard I'm getting build errors like this when compiling as C++:

/Users/floh/projects/sokol/sokol_app.h:9305:14: error: member reference type 'JavaVM' (aka '_JavaVM') is not a pointer; did you mean to use '.'?
    if ((*vm)->GetEnv(vm, (void**)env, JNI_VERSION_1_6) == JNI_OK) {

I don't quite understand the (*vm)->GetEnv() pointer indirection, because vm is a simple pointer to a struct, not a pointer to a pointer:

    JavaVM *vm = _sapp.android.activity->vm;
    *env = NULL;

    if ((*vm)->GetEnv(vm, (void**)env, JNI_VERSION_1_6) == JNI_OK) {
        return false;
    }

The _JavaVM struct declaration looks like this (with the embedded JNIInvokeInterface struct):

/*
 * JNI invocation interface.
 */
struct JNIInvokeInterface {
    void*       reserved0;
    void*       reserved1;
    void*       reserved2;

    jint        (*DestroyJavaVM)(JavaVM*);
    jint        (*AttachCurrentThread)(JavaVM*, JNIEnv**, void*);
    jint        (*DetachCurrentThread)(JavaVM*);
    jint        (*GetEnv)(JavaVM*, void**, jint);
    jint        (*AttachCurrentThreadAsDaemon)(JavaVM*, JNIEnv**, void*);
};

/*
 * C++ version.
 */
struct _JavaVM {
    const struct JNIInvokeInterface* functions;

#if defined(__cplusplus)
    jint DestroyJavaVM()
    { return functions->DestroyJavaVM(this); }
    jint AttachCurrentThread(JNIEnv** p_env, void* thr_args)
    { return functions->AttachCurrentThread(this, p_env, thr_args); }
    jint DetachCurrentThread()
    { return functions->DetachCurrentThread(this); }
    jint GetEnv(void** env, jint version)
    { return functions->GetEnv(this, env, version); }
    jint AttachCurrentThreadAsDaemon(JNIEnv** p_env, void* thr_args)
    { return functions->AttachCurrentThreadAsDaemon(this, p_env, thr_args); }
#endif /*__cplusplus*/
};

...should the invocation not look like this? (I'll try that)

    vm->functions->GetEnv(...)

PS: hmm no it's not that easy, now I get errors in C mode:

error: member reference base type 'JavaVM' (aka 'const struct JNIInvokeInterface *') is not a structure or union
    if (vm->functions->GetEnv(vm, (void**)env, JNI_VERSION_1_6) == JNI_OK) {
        ~~^ ~~~~~~~~~

...hrmpf... I wished the NDK headers wouldn't try to be so 'clever'...

[floooh]

@olivierbuisard another question: the UTF-8 <=> MUTF-8 conversion functions look a bit scary (in terms of buffer overflows). Are those copied from some "reference implementation" where I could take a look at the "original", and maybe also leave a link to the original source in sokol_app.h?

Btw, I'm doing those changes in my own merge-branch, no need for you to update the PR.

[floooh]

Ugh, this whole UTF-8 vs MUTF-8 topic is a rabbit hole of epic proportions. Why is every little thing on Android such a mess. It's like a shit fractal ;)

[floooh]

...another thing I'll fix, this C99-ism:

    JavaVMAttachArgs args = {
        .version  = JNI_VERSION_1_6,
        .name = "NativeThread",
        .group = NULL
    };

[floooh]

...I'll fix the C vs C++ issue with JNI method wrappers like this (same thing I did for DirectX calls):

_SOKOL_PRIVATE jclass _sapp_android_JNIEnv_FindClass(JNIEnv* self, const char* name) {
    #if defined(__cplusplus)
        return self->FindClass(name);
    #else
        return (*self)->FindClass(self, name);
    #endif
}

[billzez]

Maybe add a macro?

#if defined(__cplusplus)
  #define DEREF_IF_C(obj) (obj)
#else
  #define DEREF_IF_C(obj) (*obj)
#endif

return DEREF_IF_C(self)->FindClass(self, name);

Why is every little thing on Android such a mess. It's like a shit fractal ;)

This might be my new favorite phrase.

[Manuzor]

return DEREF_IF_C(self)->FindClass(self, name);

That won't work because the C++ version does not need self passed as first parameter.

Why is every little thing on Android such a mess. It's like a shit fractal ;)

This might be my new favorite phrase.

Agreed.

[floooh]

...it's probably possible to solve this with variadic macros, but I didn't want to go down that road because it would be the first usage in the sokol headers, and I'd need to check first if it works across all compilers both in C and C++ mode.

[floooh]

I'm going to add a new mobile-input sample to the sokol-samples repo to better test the features in this PR and also protect it from regressions. It'll be a couple more days until the merge.

[billzez]

_sapp_android_get_jni_env is useful for accessing other android APIs, can it be made public?

[billzez]

Also, the screen keyboard row in the feature matrix needs updating.

[floooh]

Also, the screen keyboard row in the feature matrix needs updating.

Fixed in my branch just now.

_sapp_android_get_jni_env is useful for accessing other android APIs, can it be made public?

There's sapp_android_get_native_activity() already, and getting the JNIEnv from ANativeActivity is relatively trivial. Would that be enough?

PS: what I mean is: do we really need that extra function since there's already a way to get the native activity pointer, which then can be used by 'user code' to get the JNIEnv pointer.

The current implementation of _sapp_android_get_jni_env() also calls AttachCurrentThread, which is a bit ugly if the function would simply be made public, I'd rather keep the platform specific public functions to a minimum.

[billzez]

Yeah, that should work. I can get the env from ANativeActivity and call AttachCurrentThread myself.

[floooh]

hmmm... the detection if the keyboard is open doesn't appear to work on my Google Pixel 4 because apparently there's no status bar (view_visible_height is identical to the display_height, which means the expression (display_height - status_bar_height) != view_visible_height is always true (so the code thinks the keyboard is open, even when it is closed).

PS: fixed with a hack by assuming that keyboards are at least 256 pixels high.

[floooh]

Hrrcchhh... the PR doesn't provide SAPP_EVENTTYPE_CHAR events unfortunately, and it looks like that there's no way to get the UNICODE code point from a key event through the NDK... which means even more ugly JNI code (if it's even possible at all). I'll abandon the PR for now. If anybody wants to pick up the pieces and work on this, my WIP branch is here: https://github.com/floooh/sokol/tree/olivierbuisard-master

(without UNICODE code point support the whole endevaour to support the Android soft keyboard is pretty much useless, because one could just as well render a custom keyboard instead - that's what I'll do for my emulators at least).

[Wertzui123]

Hm, that's really sad to hear. I get your point that Unicode support is important, but wouldn't it be better to have a "normal" keyboard at least than nothing? If you maybe accept ASCII input only anyway or e.g. just want to use arrow keys on a physical keyboard for scrolling, this PR would be very useful, don't you think so?
Yes, you could implement your own virtual keyboard, but why when there's already this implementation here?

[floooh]

There's also this hack to detect if the keyboard is open (the original approach didn't work on my Pixel 4), and the JNI code is just too much hassle (basically 3..5 lines of ugly C code for every line of Java code). And properly handling UNICODE input would increase the "JNI surface" even more.

Mobile keyboard support is actually a problem on all platforms, full of weird workarounds (also on iOS and especially on Web). I'm currently thinking about removing the mobile keyboard functions completely, and instead allow to inject sokol-app events from the outside. This would allow to solve all sorts of scenarios without bloating sokol_app.h further, for instance IME support on Windows and Mac, gesture recognition, support for other input devices like gamepads, etc... these things could then be handled by 3rd-party-libs with a bit of glue code, or by specialized sokol headers where the coding mess would be better isolated.

[billzez]

That's disappointing to hear. I was excited because it seemed sokol was going to succeed where others (glut/glfw/glfm) have failed. The native virtual keyboards are so advanced these days, I just don't see anyone coming up with a custom one that would be anywhere close in features.

[billzez]

For reference, here is how glfm gets unicode key char events:

https://github.com/brackeen/glfm/blob/master/src/glfm_platform_android.c#L389

[floooh]

That's looks actually quite simple! (I still think it's better to put mobile keyboard support into a separate header, and just provide the necessary hooks in sokol_app.h)

[billzez]

I agree with the sentiment, platform abstraction can be a rabbit hole, and sokol-app should probably define some sort of line, or what "Tier 1" support means, and I would argue that IME might not make the cut. And I agree that having virtual keyboards be "pluggable" via separate headers makes sense. Some apps will want a custom game oriented keyboard, and some will want the full-blown native experience.

But however you decide to structure the code, I think including support for the native virtual keyboards as a "Tier 1" feature in the main repo will be greatly appreciated by many, and really I hope this work can be merged before the code is restructured.

I also wonder if it would make sense to go a step further and split sokol-app out for each platform, so there is an easier way to use the wayland backend.

[kerrhome]

That's disappointing to hear. I was excited because it seemed sokol was going to succeed where others (glut/glfw/glfm) have failed. The native virtual keyboards are so advanced these days, I just don't see anyone coming up with a custom one that would be anywhere close in features.

Completely agree. Can't wait to see this branch merged!