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

Android IME support #475

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

miscvariables
Copy link

@miscvariables miscvariables commented Feb 11, 2021

2021/02/13
update
util/sokol_imgui.h call functions in imgui_internal.h, so all imgui examples in sokol-samples should include "imgui_internal.h". I found a coding style of sokol: sokol_imgui.h doen't include some necessary headers, such as imgui.h, It depends on examples to include these headers. So any extra headers which sokol_imgui.h needs will cause modification of sokol-samples.

I will submit the modifications in sokol-samples soon

2021/02/13

  • Adjust some behaviors of the input state, to make it have better interaction with the sugguestions of IME
  • Clean up debug code, and no compiling warnings now
  • Using macro SOKOL_MALLOC SOKOL_CALLOC SOKOL_FREE to allocate memory in heap. Malloc/free are carefully paired
  • Don't need to merge https://github.com/miscvariables/sokol-samples , It just have an example imgui-highdpi-sapp.cc to show CJK input

I think it's ready to merge ;)

2021/02/11
based on checkout of sokol and sokol-samples on 2021/02/11
Android ImGui IME support. Simple Information:

No Java. Pure c/c++ android native activity.

Using JNI to call JavaVM functions, show/hide soft keyboard, create Android EditText widget in top view DecorView

Update the content between EditText and ImGui::InputText

I use 4 states to update the content: previous text in EditText(atext_prev), current text in EditText(atext), previous text in ImGui::InputText(vtext_prev), current text in ImGui::InputText(vtext). "atext" means Android-Text, "vtext" means Virtual-Text. The state machine is in sokol_app.h

Android main UI thread and sokol gl&imgui thread use Android looper to communicate. All JNI calls are done in Android main UI thread. It can avoid crash

sokol_imgui.h should pass ImGui::InputText state of current focus to sokol_app.h

sokol android seems only using c interface(no c++), so I only implement JNI c interface in sokol_app.h, JNI c++ interface has only empty functions for compiling complains.

Pure c/c++ Android app must use many tricks, the the code looks a bit dirty. There are many commented out codes in sokol_app.h, show the sucessful and unsucessful tries I ever made.

Patched files:
sokol/sokol_app.h
sokol/util/sokol_imgui.h
sokol-samples/sapp/imgui-highdpi-sapp.cc  The demo uses /system/fonts/NotoSansCJK-Regular.ttc default to support chinese, you can edit line 46 to use default in memory font
sokol-samples/sapp/CMakeLists.txt (stripped. Contains only imgui-highdpi-sapp.cc Not necessary)
sokol-samples/libs/dbgui/dbgui.cc (only add #include imgui_internal.h, for compiling complains)
sokol-samples/tests/cpp-compile-test.cc (only add #include imgui_internal.h, for compiling complains)

@miscvariables
Copy link
Author

f21bce2a97fb790e023ae24d1b8ee2c

7859e4798e72763da44eb95eb1ce17d

@miscvariables
Copy link
Author

Bugfix and better performance in sokol_app.h, no known bugs now

@floooh
Copy link
Owner

floooh commented Feb 11, 2021

Thanks for the PR, but TBH, currently it looks too messy to be considered. There are big code blocks inside#if 0 / #endif and #if 1 / #endif, an strdup() call which most likely leads to a memory leak (in any case it circumvents the SOKOL_MALLOC allocation function), large blocks of code are commented out, and function names like UnicodeStrFromUtf8 which don't follow the naming convention and might collide with external symbols.

(also there's a block of text in the README.md file which probably shouldn't be there).

@miscvariables
Copy link
Author

miscvariables commented Feb 11, 2021

I am cleaning up the code, so many tricks when only using c, some comments are reserved for alternative options. It looks immature to merge now. Tested on Android 7.0, I published here for further evaluation, coming soon.
Is it better to open an issue for test before merge? thanx

@floooh floooh marked this pull request as draft February 11, 2021 17:00
@floooh
Copy link
Owner

floooh commented Feb 11, 2021

I am cleaning up the code

Ah ok I see. You can set the Pull Request to "Draft" state as long as you're working on it:

https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/changing-the-stage-of-a-pull-request#converting-a-pull-request-to-a-draft

That way I also can't accidentally hit the "Merge" button ;)

PS: turns out I can do this too, so I set the PR to "draft".

@floooh
Copy link
Owner

floooh commented Feb 11, 2021

PS:

Is it better to open an issue for test before merge?

No, I think it's better to discuss the PR right here.

@miscvariables miscvariables marked this pull request as ready for review February 13, 2021 22:26
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.

None yet

2 participants