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

Load Rime in Background Thread #1152

Merged
merged 27 commits into from
Dec 31, 2023
Merged

Load Rime in Background Thread #1152

merged 27 commits into from
Dec 31, 2023

Conversation

goofyz
Copy link
Collaborator

@goofyz goofyz commented Dec 18, 2023

Pull request

Issue tracker

Fix #849
Fix #949
Fix #1098
Fix #1146
Fix #1156
Fix #1158
Fix #1159

Feature

Wrap Rime in RimeWrapper, then load it in background thread. Display a loading screen when keyboard is used during Rime's initialization.

Code of conduct

Style lint

  • make sytle-lint

Build pass

  • make debug

Manually test

  • Done

Code Review

  1. No wildcards import
  2. Manual build and test pass
  3. GitHub action ci pass
  4. At least one contributor reviews and votes
  5. Can be merged clean without conflicts
  6. PR will be merged by rebase upstream base

Daily build

Login and download artifact at https://github.com/osfans/trime/actions

Additional Info

@Bambooin
Copy link
Collaborator

Works like charm, this will improve user experience.

The UI is very very fast with pop on Trime keyboard now in my phone from 1-2s to about 0s.

Some improvements:
All file should end with newline, xml files lack of newline.

If you have time, can you change the Loading to Deploying... in new installation.

Default Loading is misguarded in deploying large config file.

For the newline, I will add .editorconfig later.

@goofyz
Copy link
Collaborator Author

goofyz commented Dec 18, 2023

ok.

If possible, please ask more users to test this build. I think that this is a rather risky change and may have some unexpected bugs🥴.

@Bambooin
Copy link
Collaborator

We have QQ group in Trime about page, you can share the build to these groups.

Found three issue in commit 1b9eb75

  1. Random crash on Trime
  2. UI init will take full left screen and back to normal, hard to descripe and screenshot.
  3. The input focus won't work when screen lock password input randomly.

The third issue happens in the latest commit, the others two seem disappear right now.

@goofyz
Copy link
Collaborator Author

goofyz commented Dec 19, 2023

  1. Do u have logs for me to check?
  2. i know what you mean. The candidate view will extend to the whole screen for 1 second them back to normal. Need mkre time to test.
  3. i don‘t understand what this issue is. can u elaborate more?

PS:Sadly, i don't have QQ account.

@Bambooin
Copy link
Collaborator

The random crash seems fixed in latest commit.

The third issue still show up with latest commit, see #1156

The main merging blocker issue is the random full screen with keyboard, seems the size of the keyboard is wrong when setup the keyboard.

@wxyzh
Copy link

wxyzh commented Dec 20, 2023

I have more than 5 user dict that need to be synchronized.
It seems the syncing process splits into multiple tasks, causing userdb read/write failures.

When I send intent "com.osfans.trime.timing.sync," the keyboard won't pop up, and the main program freezes.

@goofyz
Copy link
Collaborator Author

goofyz commented Dec 20, 2023

Supposedly it is the same function as before, only now it run in a background thread.
do u mean 「同步使用者資料」?or just「部署」?

@wxyzh
Copy link

wxyzh commented Dec 20, 2023

Supposedly it is the same function as before, only now it run in a background thread. do u mean 「同步使用者資料」?or just「部署」?
同步使用者资料

I have tested other versions without this pr, These problems seem unrelated to this pr.

@Bambooin
Copy link
Collaborator

All three issues seem gone in latest commit.

New crash issue #1158 seems caused by this pull request, but it's not a merge blocker issue.

I will merge this later, you can rebase your branch on develop branch if you have time.

@Bambooin
Copy link
Collaborator

Found another blocking issue #1159

@shitlime
Copy link
Contributor

If this plan can be perfectly implemented, is it possible to use dual RIME engines during deployment? One for deployment and one for use.🤔️

@goofyz
Copy link
Collaborator Author

goofyz commented Dec 25, 2023

If this plan can be perfectly implemented, is it possible to use dual RIME engines during deployment? One for deployment and one for use.🤔️

I doubt it, as they will be reading and writing in the same directory.
To achieve this, I think we need to change the behaviour of the underlying Rime library.

@Bambooin
Copy link
Collaborator

With the latest commit, the #1159 blocking issue was resovled, but the #1158 issue still can be reproduced with my phone.

Some work around patch, more better ideas are welcome.

This fix the restart crash, but first keyborad pop up will lead to crash too.

Seems we can move this part to RimeWrapper and delay the setup when the media is not mounted, like permission checking.

        private fun startup(fullCheck: Boolean) {
            // work around for the media mounted issue
            if (StringUtils.isEmpty(PathUtils.getExternalStoragePath())) {
                Timber.i("Media is not mounted, and skip the setup right now")
                return
            }
            
            isHandlingRimeNotification = false

            DataManager.sync()

            val sharedDataDir = AppPrefs.defaultInstance().profile.sharedDataDir
            val userDataDir = AppPrefs.defaultInstance().profile.userDataDir

            Timber.i("Starting up Rime APIs ...")
            startupRime(sharedDataDir, userDataDir, fullCheck)

            Timber.i("Initializing schema stuffs after starting up ...")
            SchemaManager.init(getCurrentRimeSchema())
            updateStatus()
        }

@goofyz
Copy link
Collaborator Author

goofyz commented Dec 25, 2023

got it.
as i can't replicate the problem, will need you to test.

@Bambooin
Copy link
Collaborator

With the latest commit, the crash won't show up in android restart, but still show up in first keyboard pop up with restart with the exact same crash in #1158.

Seems we should add this check in first keyboard pop up event too.

@Bambooin
Copy link
Collaborator

Another new crash with NPE.

--- a/app/src/main/java/com/osfans/trime/ime/core/Trime.java
+++ b/app/src/main/java/com/osfans/trime/ime/core/Trime.java
@@ -782,7 +782,6 @@ public class Trime extends LifecycleInputMethodService {
           bindKeyboardToInputView();

           setInputView(inputRootBinding.inputRoot);
-          initialKeyboard = null;
           Timber.d("onCreateInputView - completely ended");
         });
     Timber.i("onCreateInputView() finish");

Why initalKeyboard is set to null which will lead to NPE error?

@Bambooin
Copy link
Collaborator

Remove this line will fix the NPE crash.

@goofyz
Copy link
Collaborator Author

goofyz commented Dec 26, 2023

With the latest commit, the crash won't show up in android restart, but still show up in first keyboard pop up with restart with the exact same crash in #1158.

Seems we should add this check in first keyboard pop up event too.

I have already added checking for all places (in onCreate(), onCreateInputView() and onStartupInputView()).
So will the keyboard show up eventually ? (keyboard try to show up -> crash -> keyboard load again -> ok ?)

@goofyz
Copy link
Collaborator Author

goofyz commented Dec 26, 2023

Another new crash with NPE.

--- a/app/src/main/java/com/osfans/trime/ime/core/Trime.java
+++ b/app/src/main/java/com/osfans/trime/ime/core/Trime.java
@@ -782,7 +782,6 @@ public class Trime extends LifecycleInputMethodService {
           bindKeyboardToInputView();

           setInputView(inputRootBinding.inputRoot);
-          initialKeyboard = null;
           Timber.d("onCreateInputView - completely ended");
         });
     Timber.i("onCreateInputView() finish");

Why initalKeyboard is set to null which will lead to NPE error?

Just try to save some memory. Will not set it to null then if it will lead to NPE.

@Bambooin
Copy link
Collaborator

So will the keyboard show up eventually ? (keyboard try to show up -> crash -> keyboard load again -> ok ?)

No keyborad show up instead of crash page, the second time to pop up keyboard, no crash anymore.

Seems the second time keyboard pops up, the medis is mounted.

It will crash every time when phone restart, not a serious issue but something annoying.

@goofyz
Copy link
Collaborator Author

goofyz commented Dec 26, 2023

Will the crash appear no matter how long u waited after the restart?
Then the “not-mounted” issue is only affecting the first start of the app.

But that's strange. After crash, it will always be the "first" start.

@Bambooin
Copy link
Collaborator

Will the crash appear no matter how long u waited after the restart?
Yes, I wait about 3 minutes and first pop up crash too.

Then the “not-mounted” issue is only affecting the first start of the app.
But the Amaze file manager is fine which means media is mounted.

But that's strange. After crash, it will always be the "first" start.
restart => open Amaze => waiting about 3 minutes => touch input box => show up Deploying instantly => Trime crash with the same stack of #1158

--------- Crash stacktrace
java.io.IOException: No such file or directory
	at java.io.UnixFileSystem.createFileExclusively0(Native Method)
	at java.io.UnixFileSystem.createFileExclusively(UnixFileSystem.java:317)
	at java.io.File.createNewFile(File.java:1006)
	at com.osfans.trime.data.DataManager.sync(DataManager.kt:89)
	at com.osfans.trime.core.Rime$Companion.startup(Rime.kt:67)
	at com.osfans.trime.core.Rime$Companion.access$startup(Rime.kt:42)
	at com.osfans.trime.core.Rime.<init>(Rime.kt:39)
	at com.osfans.trime.core.Rime$Companion.getInstance(Rime.kt:47)
	at com.osfans.trime.ime.core.RimeWrapper$startup$2.invokeSuspend(RimeWrapper.kt:43)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:108)
	at kotlinx.coroutines.internal.LimitedDispatcher$Worker.run(LimitedDispatcher.kt:115)
	at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:103)
	at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:584)
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:793)
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:697)
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:684)
	Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [StandaloneCoroutine{Cancelling}@29b093c, Dispatchers.IO]

--------- beginning of main
12-27 19:00:47.954 D/CompatibilityChangeReporter( 3198): Compat change id reported: 171979766; UID 10167; state: ENABLED
12-27 19:00:48.134 V/GraphicsEnvironment( 3198): ANGLE Developer option for 'com.osfans.trime' set to: 'default'
12-27 19:00:48.135 V/GraphicsEnvironment( 3198): ANGLE GameManagerService for com.osfans.trime: false
12-27 19:00:48.135 V/GraphicsEnvironment( 3198): Neither updatable production driver nor prerelease driver is supported.
12-27 19:00:48.138 D/NetworkSecurityConfig( 3198): No Network Security Config specified, using platform default
12-27 19:00:48.139 D/NetworkSecurityConfig( 3198): No Network Security Config specified, using platform default
12-27 19:00:48.619 D/WindowTokenClient( 3198): Only apply configuration update to Resources because shouldReportConfigChange is false.
12-27 19:00:48.619 D/WindowTokenClient( 3198): android.window.WindowTokenClient.attachToDisplayArea:118 android.window.WindowContextController.attachToDisplayArea:107 android.window.WindowProviderService.attachBaseContext:208 android.app.Service.attach:915 android.app.ActivityThread.handleCreateService:4484 
12-27 19:00:48.647 D/CompatibilityChangeReporter( 3198): Compat change id reported: 210923482; UID 10167; state: ENABLED
12-27 19:00:48.707 I/Choreographer( 3198): Skipped 33 frames!  The application may be doing too much work on its main thread.
12-27 19:00:48.739 D/CompatibilityChangeReporter( 3198): Compat change id reported: 237531167; UID 10167; state: DISABLED
12-27 19:00:48.760 W/Parcel  ( 3198): Expecting binder but got null!
12-27 19:00:48.776 D/WindowTokenClient( 3198): Configuration not dispatch to IME because configuration is up to date. Current config={1.0 460mcc11mnc [en_US] ldltr sw393dp w393dp h798dp 440dpi nrml long port night finger -keyb/v/h -nav/h winConfig={ mBounds=Rect(0, 0 - 1080, 2340) mAppBounds=Rect(0, 80 - 1080, 2274) mMaxBounds=Rect(0, 0 - 1080, 2340) mDisplayRotation=ROTATION_0 mWindowingMode=fullscreen mDisplayWindowingMode=fullscreen mActivityType=undefined mAlwaysOnTop=undefined mRotation=ROTATION_0} as.3 s.54 fontWeightAdjustment=0}, reported config={1.0 460mcc11mnc [en_US] ldltr sw393dp w393dp h798dp 440dpi nrml long port night finger -keyb/v/h -nav/h winConfig={ mBounds=Rect(0, 0 - 1080, 2340) mAppBounds=Rect(0, 80 - 1080, 2274) mMaxBounds=Rect(0, 0 - 1080, 2340) mDisplayRotation=ROTATION_0 mWindowingMode=fullscreen mDisplayWindowingMode=fullscreen mActivityType=undefined mAlwaysOnTop=undefined mRotation=ROTATION_0} as.3 s.54 fontWeightAdjustment=0}, updated config={1.0 460mcc11mnc [en_US] ldltr sw393dp w393dp h798dp 440dpi nrml long port night finger -keyb/v/h -nav/h winConfig={ mBounds=Rect(0, 0 - 1080, 2340) mAppBounds=Rect(0, 80 - 1080, 2274) mMaxBounds=Rect(0, 0 - 1080, 2340) mDisplayRotation=ROTATION_0 mWindowingMode=fullscreen mDisplayWindowingMode=fullscreen mActivityType=undefined mAlwaysOnTop=undefined mRotation=ROTATION_0} as.3 s.54 fontWeightAdjustment=0}
12-27 19:00:53.919 D/ProfileInstaller( 3198): Skipping profile installation for com.osfans.trime
12-27 19:02:40.657 I/[main]  ( 3198): onCreateInputView() finish
12-27 19:02:40.666 I/[main]  ( 3198): onWindowShown...
12-27 19:02:40.704 I/AdrenoGLES-0( 3198): QUALCOMM build                   : c5f0e23, I9972db7aac
12-27 19:02:40.704 I/AdrenoGLES-0( 3198): Build Date                       : 04/28/21
12-27 19:02:40.704 I/AdrenoGLES-0( 3198): OpenGL ES Shader Compiler Version: EV031.32.02.10
12-27 19:02:40.704 I/AdrenoGLES-0( 3198): Local Branch                     : mybrancheba83f1e-3815-f156-b760-08f1eafda2d8
12-27 19:02:40.704 I/AdrenoGLES-0( 3198): Remote Branch                    : quic/gfx-adreno.lnx.1.0.r99-rel
12-27 19:02:40.704 I/AdrenoGLES-0( 3198): Remote Branch                    : NONE
12-27 19:02:40.704 I/AdrenoGLES-0( 3198): Reconstruct Branch               : NOTHING
12-27 19:02:40.704 I/AdrenoGLES-0( 3198): Build Config                     : S P 10.0.7 AArch64
12-27 19:02:40.704 I/AdrenoGLES-0( 3198): Driver Path                      : /vendor/lib64/egl/libGLESv2_adreno.so
12-27 19:02:40.708 I/AdrenoGLES-0( 3198): PFP: 0x016ee190, ME: 0x00000000
12-27 19:02:40.718 E/OpenGLRenderer( 3198): Unable to match the desired swap behavior.
12-27 19:02:40.721 E/FileIOUtils( 3198): create file </rime/opencc/HKVariants.txt> failed.
12-27 19:02:40.722 E/FileIOUtils( 3198): create file </rime/opencc/HKVariantsRev.txt> failed.
12-27 19:02:40.724 E/FileIOUtils( 3198): create file </rime/opencc/HKVariantsRevPhrases.txt> failed.
12-27 19:02:40.725 E/FileIOUtils( 3198): create file </rime/opencc/JPShinjitaiCharacters.txt> failed.
12-27 19:02:40.726 E/FileIOUtils( 3198): create file </rime/opencc/JPShinjitaiPhrases.txt> failed.
12-27 19:02:40.727 I/Gralloc4( 3198): Adding additional valid usage bits: 0x8202000
12-27 19:02:40.727 E/FileIOUtils( 3198): create file </rime/opencc/JPVariants.txt> failed.
12-27 19:02:40.735 I/[main]  ( 3198): onWindowShown...
12-27 19:02:40.735 I/[main]  ( 3198): onWindowShown...
12-27 19:02:40.736 E/FileIOUtils( 3198): create file </rime/opencc/STCharacters.txt> failed.
12-27 19:02:40.738 E/FileIOUtils( 3198): create file </rime/opencc/STPhrases.txt> failed.
12-27 19:02:40.739 E/FileIOUtils( 3198): create file </rime/opencc/TSCharacters.txt> failed.
12-27 19:02:40.740 E/FileIOUtils( 3198): create file </rime/opencc/TSPhrases.txt> failed.
12-27 19:02:40.741 E/FileIOUtils( 3198): create file </rime/opencc/TWPhrases.txt> failed.
12-27 19:02:40.742 E/FileIOUtils( 3198): create file </rime/opencc/TWPhrasesRev.txt> failed.
12-27 19:02:40.743 E/FileIOUtils( 3198): create file </rime/opencc/TWVariants.txt> failed.
12-27 19:02:40.745 E/FileIOUtils( 3198): create file </rime/opencc/TWVariantsRev.txt> failed.
12-27 19:02:40.746 E/FileIOUtils( 3198): create file </rime/opencc/TWVariantsRevPhrases.txt> failed.
12-27 19:02:40.747 E/FileIOUtils( 3198): create file </rime/opencc/hk2s.json> failed.
12-27 19:02:40.748 E/FileIOUtils( 3198): create file </rime/opencc/hk2t.json> failed.
12-27 19:02:40.749 E/FileIOUtils( 3198): create file </rime/opencc/jp2t.json> failed.
12-27 19:02:40.750 E/FileIOUtils( 3198): create file </rime/opencc/s2hk.json> failed.
12-27 19:02:40.751 E/FileIOUtils( 3198): create file </rime/opencc/s2t.json> failed.
12-27 19:02:40.754 E/FileIOUtils( 3198): create file </rime/opencc/s2tw.json> failed.
12-27 19:02:40.755 E/FileIOUtils( 3198): create file </rime/opencc/s2twp.json> failed.
12-27 19:02:40.756 E/FileIOUtils( 3198): create file </rime/opencc/t2hk.json> failed.
12-27 19:02:40.757 E/FileIOUtils( 3198): create file </rime/opencc/t2jp.json> failed.
12-27 19:02:40.758 E/FileIOUtils( 3198): create file </rime/opencc/t2s.json> failed.
12-27 19:02:40.760 E/FileIOUtils( 3198): create file </rime/opencc/t2tw.json> failed.
12-27 19:02:40.761 E/FileIOUtils( 3198): create file </rime/opencc/tw2s.json> failed.
12-27 19:02:40.762 E/FileIOUtils( 3198): create file </rime/opencc/tw2sp.json> failed.
12-27 19:02:40.763 I/[main]  ( 3198): onWindowShown...
12-27 19:02:40.763 I/[main]  ( 3198): onWindowShown...
12-27 19:02:40.764 E/FileIOUtils( 3198): create file </rime/opencc/tw2t.json> failed.
12-27 19:02:40.765 E/FileIOUtils( 3198): create file </rime/tongwenfeng.trime.yaml> failed.
12-27 19:02:40.766 E/FileIOUtils( 3198): create file </rime/trime.yaml> failed.
12-27 19:02:40.820 I/om.osfans.trime( 3198): System.exit called, status: 10
12-27 19:02:40.820 I/AndroidRuntime( 3198): VM exiting with result code 10, cleanup skipped.

@Bambooin
Copy link
Collaborator

First pop up crash after restart can be reproduced in the latest commit d7e4925

@goofyz
Copy link
Collaborator Author

goofyz commented Dec 28, 2023

Same exception?
did u set a custom shared directory ?

@Bambooin
Copy link
Collaborator

No custom shared directory, same exception too.

@goofyz goofyz force-pushed the fix-1146 branch 2 times, most recently from 41b81c5 to f70e525 Compare December 28, 2023 13:03
Currently candidate view use the default style in Android OS,
which varies in different manufacturer.  Adding a style to it
so that it has the same scrollbar on different Android model.
Add `rimeActionWIthResultDialog()` to run deploy and display a
result dialog.  Cannot add to `schemaPickers` because the token
of windows maybe null if it is started by a keyboard view.
…missions

If permissions are not granted, `RimeWrapper.canStart` will be set to false, so
`RimeWrapper` will save the `Runnable` and wait to start later.  When permission are
granted, `RimeWrapper.canStart` will be set to true in `onCreate()`, `onCreateInputView()`
and `onStartInputView()`.  And rime deployment will be triggered accordingly.

For UI display:  If permissions are not granted, an error message will be displayed
in `InitialKeyboard`.  If permissions are granted, `InitialKeyboard` with deployment
progress will be displayed.  If deployment is completed, the normal keyboard will be
displayed.

Do not set `initialKeyboard` to null to prevent NPE. As the callback in `RimeWrapper`
maybe perform faster or slower, we will not be sure when `setInputView()` will run.
We should either check for null for all call to `initialKeyboard`, or do not set it
to null.

Refs osfans#1159
Set external storage path and its related directory as getter property,
so that it will return the latest value when it is called.

Refs osfans#1158
@Bambooin Bambooin merged commit fc3f59f into osfans:develop Dec 31, 2023
3 checks passed
@goofyz goofyz deleted the fix-1146 branch January 22, 2024 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants