-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Save state on suspend on Android #5601
base: master
Are you sure you want to change the base?
Conversation
Same as in the wgpu integration
Just to make sure CI passes
When doing a self-review I noticed it was me who added it. I am sure it was not added when implementing the saving logic for Android, seems like it was a remnant of the debugging stage. Saving still works without it.
Preview available at https://egui-pr-preview.github.io/pr/5601-save-app-on-suspend-on-android |
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.
Looks good! Maybe remove or lower some of the logging
Ask to save on suspend on all platforms, always save even if the window does not exist
Looked at the comments and implemented the requested things. |
It is now used on all platforms
This pull request fixes a subset of #5492 by saving the application state when the
suspended
event is received on Android. This way, even if the user exits the app and closes it manually right after changing some state, it will be saved sincesuspended
gets fired when the app is exited. It does not fix theon_exit
function not being fired - this seems to be a winit bug (theexiting
function in the winit application handler trait is not called on exit). Once it gets fixed, it may be possible to remove logic introduced by this PR (however, I am not sure how it would handle the app being killed by the system when in the background, that would have to be tested).I've tested the logic by:
In all of these instances, the state was saved (the last one being a pleasant surprise). It was tested on the repository mentioned in #5492 with my forked repository as the source for eframe (I unfortunately am not able to test it in a larger project of mine due to dependence on "3rd party" egui libraries (like egui_notify) which do not compile along with the master branch of eframe (different versions of egui), but I believe it should work in the same manner in all scenarios). Tests were conducted on a Galaxy Tab S8 running Android 14, One UI 6.1.1.
CI passed on my fork.