android-rs-glue
android-rs-glue copied to clipboard
rust-mobile
Initial JNI support
Example code using these JNI bindings can be seen in https://github.com/tomaka/glutin/pull/822
Overall I think this needs some more high-level functions as well. But I'm not sure what the right border should be between the low-level bindings and a high-level wrapper, so this could eventually be done in a later PR.
Here's a gist @sfackler made demonstrating a high-level API for JNI stuff: https://gist.github.com/sfackler/5c036006f8d58a1d38887ba014759f74 It is licensed under MIT/Apache2 so we can copy some stuff if we want.
As I said in the previous comment, this might be out of scope of this PR but I thought I'd leave it somewhere for reference.
Might also be worth depending on https://crates.io/crates/jni-sys to avoid needing to have a second copy in here. Happy to make any changes necessary on that end.
@tomaka, what is your view on using @sfackler's jni-sys crate? I'd prefer not to re-implement anything that we can readily reuse, but I'm not sure if you're happy to be adding another dependency.
Before I start addressing other feedback, it would be nice to decide if using jni-sys is on the table, and if we should add higher level wrappers to android-rs-glue, jni-sys, or some intermediate crate that wraps raw jni-sys in a nice, safe, Rust-y API.
I agree that adding a higher level API (like JNIEnv's that automatically detach_thread(), or jobject's that automatically DeleteGlobalRef/DeleteLocalRef when dropped). Most of all, I'd like to get rid of the various explicit method calls based on types.
But I'm also not sure this should be part of android-rs-glue.
@sfackler I don't think any explicit changes are required from looking at your code for a brief moment. There are some differences in the JNI implementation of Android, but those mostly (as far as I've read - I'm no expert) relate to how the JNI API is used, not the API itself.
An alternative approach would be to remove the JNI API from the glue module, and simply using the four methods provided by this patch in injected-glue from jni-sys, guarded by some #![cfg(target_os = "android")].
This would mean that android-glue-rs would not provide JNI itself, but JNI support can optionally be obtained when also including the jni-sys crate.
In my opinion this is the ideal design:
- The internal and the external glues both depend on jni-sys and communicate with each other to share the low-level objects (like you have done in your PR).
- The external glue also depends on a new crate (named
jnifor example) that doesn't exist yet and that provides a high-level wrapper around the JNI. - The high-level wrappers are reexported in the external glue's API, but the low-level objects are not exposed.
I can agree with that. I'll look into making the adaptations over the weekend, probably.
The higher level JNI wrapper will probably take a longer time to complete.
@sfackler Are you interested in creating a repo for the higher level wrapper, or should I?
I probably don't have the time to build out the high level wrapper right now unfortunately, so you might want to get it started.
Just a note that I have started on reworking this, but haven't found the time to finish it yet (because life).
So ... what is the current state? The jni_sys crate is pretty mature now and it's been two years ...
Why can't the ffi module simply be exposed / exported to the library user? This way the user can hook up the jni crate and from there on it's rather easy to interact with the JNI. I know about "API concerns", but .. something is better than nothing.
I'm ok with exposing the JNI-related functions of the FFI, as long as they are contained within their own scope and don't require an external crate.
@tomaka ... why? What's wrong about the jni crate? I mean ... do you want to re-do all of the work they've done just to not depend on them - how would you then do this function, if not using jni?
Sorry, what I mean is that we should simply reexport the content of the jni-sys crate from our own crate, instead of exposing it in the API.