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 AndroidApp::vm_as_ptr() and ::activity_as_ptr() APIs #61

Merged
merged 1 commit into from
Feb 14, 2023

Conversation

rib
Copy link
Collaborator

@rib rib commented Feb 13, 2023

This enables applications to make JNI calls without needing the ndk-context crate - which we would like to deprecate.

For now this just exposes the raw pointers, and makes no assumptions about what crates an application will use to make JNI calls - which means that some amount of unsafe code will be required when passing these to other crates.

The added _as_ptr() suffix makes this clear, and also means we can consider adding similar (suffix-free) getters later that would return jni crate types that could reduce the need for unsafe code before making JNI calls.

Fixes: #60

@rib
Copy link
Collaborator Author

rib commented Feb 13, 2023

@MarijnS95 hopefully this looks like a reasonable initial solution for exposing the JNI pointers we need without needing the ndk-context crate?

@rib rib mentioned this pull request Feb 13, 2023
android-activity/src/lib.rs Outdated Show resolved Hide resolved
android-activity/src/lib.rs Outdated Show resolved Hide resolved
android-activity/src/game_activity/mod.rs Outdated Show resolved Hide resolved
@MarijnS95
Copy link
Member

@MarijnS95 hopefully this looks like a reasonable initial solution for exposing the JNI pointers we need without needing the ndk-context crate?

I think, as long as there is a builder pattern available to explicitly pass these values from fn main() to crates (like app_dirs2) that were previously consuming ndk-context; and users are aware that they have to glue these pieces together.

This enables applications to make JNI calls without needing the
`ndk-context` crate - which we would like to deprecate.

Fixes: rust-mobile#60
@rib rib force-pushed the jni-vm-and-activity-ptrs branch from 645e5fa to dc9ca83 Compare February 14, 2023 11:52
@rib
Copy link
Collaborator Author

rib commented Feb 14, 2023

Thanks for looking over the patch. I've applied the suggested changes.

@MarijnS95 hopefully this looks like a reasonable initial solution for exposing the JNI pointers we need without needing the ndk-context crate?

I think, as long as there is a builder pattern available to explicitly pass these values from fn main() to crates (like app_dirs2) that were previously consuming ndk-context; and users are aware that they have to glue these pieces together.

Yeah - probably my main concern with this direction atm is that the ergonomics aren't going to be great. These APIs that accept raw pointers will need to be unsafe which then leads you to put all the building inside an unsafe block or else you need to split the building pattern up, which defeats the benefit of the pattern.

Regardless of exactly how the dots are joined though I think it still makes sense that there is at least some way to access the JNI pointers directly from an AndroidApp now, so we have options for figuring out how crates like app_dirs2 can be initialized without depending on ndk-context.

@rib rib merged commit 6ae2e42 into rust-mobile:main Feb 14, 2023
@MarijnS95
Copy link
Member

Regardless of exactly how the dots are joined though I think it still makes sense that there is at least some way to access the JNI pointers directly from an AndroidApp now, so we have options for figuring out how crates like app_dirs2 can be initialized without depending on ndk-context.

Agreed, we can't have the latter without at least having the former (this PR). Did we already have an issue describing how to proceed to get rid of ndk-context for multi-Activity support where we can discuss the continuation of this effort (we discussed it in many places but I'm not sure if there's already a singleton issue rehashing it all.

For starters I'd look at the few consumers of ndk-context and investigate for each of those whan an ndk-context-less API might look like, and/or if we need some shared machinery to cleanly support this?

dobo90 added a commit to dobo90/tiop01-gui that referenced this pull request Mar 13, 2024
* Since [1] android-activity exposes jvm and activity pointers. Use them
  directly instead of depending on ndk-context. Passing them via OnceLock
  isn't the prettiest way but well... it works.

[1]: rust-mobile/android-activity#61
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 this pull request may close these issues.

Expose JavaVM and Activity reference that can be used to make JNI calls
2 participants