-
-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
WIP: Native Android backend + example #3446
Conversation
Hello @duddel, by the way sorry I haven't answered to this yet. I'm not familiar with Android but the whole PR looks nicely done and sane and up to dear imgui style/standards so I hope you get to keep working on that. I'll see if I can get some Android users to test this. Do you know if there's a way we could avoid some of the gradle-related cruft (e.g. grapple-wrapper.jar) binary? |
Hi @ocornut, thanks alot and no problem! I figured already, by reading related issues, that you are not an Android user. No need to hurry. Concerning Gradle: I also find this a little bit disturbing, but providing the Gradle wrapper binary (gradle-wrapper.jar) and the gradlew scripts (gradlew, gradlew.bat) is the recommended way of executing Gradle builds: https://docs.gradle.org/current/userguide/gradle_wrapper.html At least these files are auto-generated, and our If things change in the future, and there is a simpler way of executing Gradle, I am happy to delete these files. |
return 0; | ||
} | ||
|
||
// Unfortunately, the native KeyEvent implementation has no getUnicodeChar() function. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can have if you try hard enough:
uint32_t Android_AInputEvent_GetUnicodeChar(JNIEnv* env, struct AInputEvent* event)
{
if (event == NULL || (AInputEvent_getType(event) != AINPUT_EVENT_TYPE_KEY))
return 0;
//KeyEvent(long downTime, long eventTime, int action, int code, int repeat, int metaState, int deviceId, int scancode, int flags, int source)
int64_t downTime = AKeyEvent_getDownTime(event);
int64_t eventTime = AKeyEvent_getEventTime(event);
int32_t action = AKeyEvent_getAction(event);
int32_t code = AKeyEvent_getKeyCode(event);
int32_t repeat = AKeyEvent_getRepeatCount(event);
int32_t metaState = AKeyEvent_getMetaState(event);
int32_t deviceId = AInputEvent_getDeviceId(event);
int32_t scancode = AKeyEvent_getScanCode(event);
int32_t flags = AKeyEvent_getFlags(event);
int32_t source = AInputEvent_getSource(event);
// Create an instance of android.view.KeyEvent.
jclass keyEventClass = env->FindClass("android/view/KeyEvent");
jmethodID keyEventClassConstructor = env->GetMethodID(keyEventClass, "<init>", "(JJIIIIIIII)V");
jobject keyEventObject = env->NewObject(keyEventClass, keyEventClassConstructor,
downTime, eventTime, action, code, repeat, metaState, deviceId, scancode, flags, source);
// Get unicode char.
jmethodID keyEventGetUnicodeChar = env->GetMethodID(keyEventClass, "getUnicodeChar", "()I");
int32_t unicodeChar = env->CallIntMethod(keyEventObject, keyEventGetUnicodeChar);
// Release object
env->DeleteLocalRef(keyEventObject);
env->DeleteLocalRef(keyEventClass);
return unicodeChar;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing that out. But still, this is not a native implementation for the KeyEvent.
I used this method before, but changed it in f8a0f03 to capturing KeyEvents in non-native code and let the characters be polled from the native side. This is less error prone, imo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sometimes native functions in NDK are plainly broken. Showing soft keyboard for example was not working. Maybe now it is, but on android-9 it was not. Often they're often not on par with Android SDK. Like choreographer was missing from NDK for a few releases. Using SDK API to translate key event into Unicode codepoint is safe operation. One may not like dealing with JNI, but this is the nature of the beast.
Out of curiosity, can you elaborate more on why do you think pooling key events from Java is less error prone? It is bypassing native input queue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My approach of reconstructing the KeyEvent via JNI and calling getUnicodeChar()
was only able to capture ASCII characters. I was doubtful about the reconstructed KeyEvent holding enough information to safely get Unicode characters. I figured that calling it on the non-native KeyEvent (Java/Kotlin side) is the best shot.
Concerning the input queue: You are right, it is based on the assumption, that all KeyEvents reach our Activity via dispatchKeyEvent()
. I did not find any information if this is guaranteed. Your Android_AInputEvent_GetUnicodeChar()
solution looks more promising than mine (I constructed the KeyEvent with event_type and key_code only). I will have a look.
About showing the soft keyboard: Yes, no reliable way to do it in native code. That was the reason to extend the NativeActivity in the first place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this isn't that big of a deal here. Key events would be useful when physical keyboard is connected to device. Text input will probably need to be implemented using IME.
What does Android do if your keyboard is set to a foreign language? |
I used a physical keyboard and switched the keyboard layout in Android around. |
You mentioned it only handed ASCII so I was trying to understand how internationalization might work... You answered me I think... |
von s in b@€ nur uh u8h.mi99u am nun mit 9 ggf zu b die voll bz viel zkw
rgi nur g TX krue Thun ne gute
…On Wed, 9 Sep 2020, 21:26 omar ***@***.***> wrote:
Hello @duddel <https://github.com/duddel>, by the way sorry I haven't
answered to this yet. I'm not familiar with Android but the whole PR looks
nicely done and sane and up to dear imgui style/standards so I hope you get
to keep working on that. I'll see if I can get some Android users to test
this.
Do you know if there's a way we could avoid some of the gradle-related
cruft (e.g. grapple-wrapper.jar) binary?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#3446 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIWN3SM5TT2OIQH5EFY6KTSE7JGZANCNFSM4QRAM5UQ>
.
|
Hello @duddel, Sorry for late answer. Going to try merging this soon. Some feedback:
Small stuff
Thank you! |
b652a5d
to
3a3d642
Compare
Hi @ocornut, I fixed up a squashed rebase to master, hopefully correct. Everything works fine (concerning Android). The Windows build seems to fail for the same reason as on master. I incorporated your remarks. Just let me know if anything is still unfortunate.
Concerning Gradle: the committed gradle-wrapper.jar is responsible for downloading the actual Gradle binaries. Your idea of not providing the wrapper, but downloading it manually, is good. I checked it again. Unfortunately, they let you download the sha256 but not he wrapper itself. The wrapper must be generated with an actual Gradle installation, and then should be committed (this is what I initially did). As I said, I am also not pleased with this procedure, but it is still the recommended way. If you like, we can try to find another solution before merge. Maybe it is possible to use a provided Gradle installation on the GitHub Actions worker (but then losing the benefit of a fixed Gradle version via the wrapper, and complicating things for users). Thanks again! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a few comments, which I think will improve this PR and help it blend with other ImGui examples.
Overall native application is very basic. Yet is is enough to show ImGui running on Android.
Thanks for making the effort! : )
examples/example_android_opengl3/android/app/src/main/java/MainActivity.kt
Outdated
Show resolved
Hide resolved
// Therefore, we call showSoftInput() of the main activity implemented in MainActivity.kt via JNI. | ||
static int showSoftInput() | ||
{ | ||
JavaVM *jVM = g_App->activity->vm; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JNIEnv*
for native thread can be fetched, attached and detached in android_main()
function.
This will speed up JNI calls. Local references will need to be explicitly released in this case.
It is helpful if functions making JNI calls accept JNIEnv*
as a first argument. This will make them reentrant and possible to call from Java callbacks.
Functions in current example will work, but need to be reworked every time someone decide to take this example as a starting point.
No change is necessary. I wanted everyone to be aware of this potential pitfall.
ImGui::CreateContext(); | ||
ImGuiIO &io = ImGui::GetIO(); | ||
io.IniFilename = NULL; | ||
ImGui::StyleColorsDark(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you consider importing fonts to be on pair with other examples?
assets
could be utilized for that purpose. This can greatly improve readability of the UI. Default one is tiny and very hard to see on modern phone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fonts can now be loaded from assets/. I prepared a function add_font_from_assets_ttf()
for that and copied/adjusted the default comment block from other examples.
case APP_CMD_SAVE_STATE: | ||
break; | ||
case APP_CMD_INIT_WINDOW: | ||
init(app); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please consider adding ANativeWindow_acquire
and ANativeWindow_release
to avoid situations where Java thread release app->window
object but native app sill use it a valid object, because APP_CMD_TERM_WINDOW was not yet processed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added ANativeWindow_acquire()
in init()
and ANativeWindow_release()
in shutdown()
.
I am not entirely sure if it has an effect here, but it shouldn't harm either. This is rarely used in any NDK examples I have seen, but I keep my eyes open.
Thanks alot, I will have a look at your remarks! After all, I also plan to remove the Gradle wrapper to unclutter the repo. |
78bdf2a
to
83fc403
Compare
Incorporated most of the remarks. Tested on multiple devices including font loading. |
Thank you very much for the update. Some extra feedback, sorry there were missing from previous batch of feedbacks: (1) In the header of the imgui_impl_android.cpp and imgui_impl_android.h I would suggest listing the missing features( (by looking at e.g. win32, glfw backend). It's fine if they are not all implemented in native Android backend today but we'd want that list as explicit documentation. I would add:
I would suggest NOT adding:
But only add it if/when mouse support gets added later. (2) Headers (3) Headers using Skimming at it, it seems like your queuing mechanism has an issue, (4) (5) (6) Example: wouldn't the (7) Could the (8) Same question for the unicode char machinery. Could part of it be handled by the backend? Both 7 and 8 may be naive and I presume tricky, but this is also what the backend value should be (in theory). Thank you again! |
Hi, thanks for the comments. I made some changes
Synchronized (missing) features with other examples. I left "Mouse cursor shape and visibility" in place because mouse input is now added too, and maybe it is possible to even change the cursor shape on Android in the future.
ok.
I added a few comment and look forward of using the ImGui input queue. You are right about the maybe-changed order of key presses ImGui recieves. But I didn't come across a case where it interfered the input. Overall, this thing is cumbersome and I will delete it with joy, once the input queue is ready :)
ok. changed todos to FIXME
ok. I left the "trivial" fallthroughs (merging multiple cases, identical code) in place.
Yes, I changed the helper function. It is cleaner this way, although a few more lines for the user.
Initially, I had it that way, but put these functions into the user code (main.cpp) to make a cleaner separation. Overall, I prefer to leave it like that, for now. Thanks again! |
Thanks for the update.
OK for now, I guess we can always revisit if needed. |
Great! Thank you very much. One small thing: The Changelog says
But the example is actually set up for GL ES3. If one desires ES2 compatibility, this should be possible with a couple of small tweaks. |
commit ee643b2 Author: ocornut <omarcornut@gmail.com> Date: Thu Mar 4 19:59:59 2021 +0100 IsItemHovered(): fixed return value false positive when used after EndChild(), EndGroup() or widgets using either... (ocornut#3851, ocornut#1370) ...when the hovered location is located within a child window, e.g. InputTextMultiline(). This is intended to have no side effects, but brace yourself for the possible comeback.. This essentially makes IsItemHovered() not accept hover from child windows, but EndChild/EndGroup are forwarded. More or less should fix/revert c76f014 which was a revert of 344d48b commit b53b8f5 Author: Rokas Kupstys <rokups@zoho.com> Date: Thu Mar 4 16:27:43 2021 +0200 Demo: Use correct string formats on non-windows platforms. (amended) commit 3e6dfd3 Author: ocornut <omarcornut@gmail.com> Date: Thu Mar 4 13:37:14 2021 +0100 ImDrawList: AddImageRounded() compare texid from cmdheader as with other functions. + Made the ImGuiMemAllocFunc / ImGuiMemFreeFunc consistent with our other typedefs (ocornut#3836) commit 8dd692c Author: ocornut <omarcornut@gmail.com> Date: Thu Mar 4 11:03:40 2021 +0100 Android: Amend backend and examples with minor consistency tweaks. (ocornut#3446) commit fb85c03 Author: duddel <duddel@users.noreply.github.com> Date: Thu Mar 4 10:35:44 2021 +0100 Add Android backend and example (ocornut#3446) commit d8c88bd Author: ocornut <omarcornut@gmail.com> Date: Thu Mar 4 09:52:00 2021 +0100 Tables: Fixed unaligned accesses when using TableSetBgColor(ImGuiTableBgTarget_CellBg). (ocornut#3872) ImSpanAllocator: Support for alignment. commit 1ddaff8 Author: ocornut <omarcornut@gmail.com> Date: Wed Mar 3 18:45:52 2021 +0100 Demo: Tweak inputs display.
Android build started failing today, seemingly because of a Gradle update? May be:
|
Yes, they updated to GitHub Actions worker ubuntu-18.04 to Gradle 7.0 (was Gradle 6.8.3). I made the Android build compatible with Gradle 7.0 in a branch: duddel@dc74b00 I updated the Android Gradle Plugin (AGP) to version 4.1.0 (current stable), which is compatible with Gradle 7.0. The AGP version must stay in sync with Gradle, so future changes on the Actions workers might require changes in the Gradle config in our project. see: |
Thank you @duddel, I confirm that merging the commit you linked fixed CI for us. |
When I clone it and run it after compilation, the following error will be reported
2021-06-21 12:52:06.245 5801-5816/imgui.example.android A/libc: Fatal signal 11 (SIGSEGV), code 1, fault addr 0x0 in tid 5816 (example.android) |
Basically, it is what it says: "no ES 3 support". The example app requires GL ES 3 (not 2), which your device (or emulator) does not support. |
Hlo bro can u give me imgui android studio project |
FYI CI for this started failing a few days ago since Gradle 8.0: |
I will have a look asap, will fix it by next tuesday. |
Hello, |
Looks great, it could be useful (with some minor stylistic tweaks), but license is not MIT and you may not be owner, so I don't know how that would work? |
I am owner and OK to add MIT license to this file. |
Then I think it would work and could be helpful to improve the Android backend. Thanks! This Android backend in general has been a little bit neglected :( |
|
Hi @BrunoLevy, this sounds interesting! I love to see a pure-native implementation for the key input and unicode mess. The current detour to the Kotlin code, and back, is unfortunate. |
Hi, I'm working on that, and will submit a PR, before then I'm adapting everything to the new event-queue-based system, I tried with your backend. Everything mostly works, except a little problem: when a menu is open and you click somewhere else then everything gets stuck, probably because fingers do not send hovering events, I'm investigating. I'm testing with my own app here, and will also test with the android example bundled with Dear ImGui. How do you build it ? (it has no |
We decided to go without the Gradle wrapper und use the stock Gradle, that comes with the GitHub Actions runner, instead (recently changed to Gradle 8.x, see #6229). You can do the same thing locally (just call Curious how your adaptions will work out. |
I'm mostly there: I could replace my old backend with the new one, and insert calls the the JNI functions to show/hide the soft keyboard and translate the keys. I tested it with the soft keyboard and a BT keyboard, with different layouts, and it works well. Now there are two options:
What do you think ? Two other questions:
|
I'm not too familiar with cpp or android , not sure if its the right place to ask but can anyone share a Really keen to get imgui working in native vr build , already created world space render and interaction bindings , works fine in the editor right now : https://imgur.com/fAW7WFc |
In case it helps, explanations to compile geogram (that uses ImGui) with Android are here |
I am working on a native Android backend with the goals of using the available OpenGL 3 renderer, and not having other dependencies (such as SDL).
The PR branch contains the native Android backend, an example app and CI integration (GitHub Actions). The Android app project itself is (I believe) as tiny as possible. It is a "modern" Android project, based on CMake and Gradle with small wrapper code written in Kotlin.
Progress: see backends/imgui_impl_android.cpp
Maybe this is useful to others.