android: Allow redraws and resizes when a Surface is present#3897
Draft
MarijnS95 wants to merge 1 commit intorust-windowing:masterfrom
Draft
android: Allow redraws and resizes when a Surface is present#3897MarijnS95 wants to merge 1 commit intorust-windowing:masterfrom
Surface is present#3897MarijnS95 wants to merge 1 commit intorust-windowing:masterfrom
Conversation
MarijnS95
commented
Sep 2, 2024
Comment on lines
+297
to
298
| if self.window_has_surface { | ||
| if resized { |
Member
Author
There was a problem hiding this comment.
Given the asserts above I want to get rid of this double checking, except for handling a user-triggered redraw request.
Member
There was a problem hiding this comment.
Shouldn't we also assert if the user triggers request_redraw while there is no surface?
Member
Author
There was a problem hiding this comment.
Since it could be async, I don't want to be that harsh on them - for now.
Perhaps that changes once we wire redraws up to Choreographer (and perhaps have a default "keep automatically requesting redraws every vsync" mode).
MarijnS95
commented
Sep 2, 2024
Comment on lines
+500
to
502
| // TODO: Theoretically the wake-up should also be handled when there's no surface | ||
| timeout = if self.window_has_surface | ||
| && (self.pending_redraw || self.window_target.proxy_wake_up.load(Ordering::Relaxed)) |
Member
Author
There was a problem hiding this comment.
Specifically, a non-pending_redraw arbitrary wakeup.
MarijnS95
commented
Sep 2, 2024
Comment on lines
+106
to
+112
| // /// `onStart()` - `onStop()` lifecycle, denoting when the app is visible | ||
| // TODO: Might use activity_visible for allowing proxy wakeup events? | ||
| // activity_visible: bool, | ||
| // /// `onResume()` - `onPause()` lifecycle, denoting when the app is actively interacted with | ||
| // activity_active: bool, | ||
| // TODO: This should be per-Activity (likely analogous to per-Window) state. | ||
| window_has_surface: bool, |
Member
Author
There was a problem hiding this comment.
I started with these states just to see what would happen, but it's quite common to have:
onStart()onResume()NativeWindowCreatedNativeWindowResizedNativeWindowRedrawNeededWindowFocusChangedto1- ...
WindowFocusChangedto0onPause()NativeWindowDestroyedonStop()- (
SaveInstanceState...)
So keying off of NativeWindowCreated/Destroyed is our only safe bet.
madsmtm
approved these changes
Sep 2, 2024
Member
madsmtm
left a comment
There was a problem hiding this comment.
Looks like an improvement so far
abaca14 to
048c9c2
Compare
... and not _just when the activity is active_. [Android's activity lifecycle] denotes that `onResume()` and `onPause()` are called when the activity changes the "active" state (which is separate from the "focused" state) and may _or may not_ be followed or preceded by the `onStart()` - `onStop()` lifecycle which denotes Activity _visibility_. The latter more closely relates to when a surface is present (since when the activity is not active, but still visible redraws and resizes are still possible and expected) but also doesn't exactly represent it. Note that Android fires its `WindowResized` and `RedrawNeeded` callbacks at least once, directly after `InitWindow`, and _should_ (but this is not explicitly documented) never fire again after `TerminateWindow`. Theoretically this isn't the right state to key off of for arbitrary `EventLoop` wakeups via the `Proxy` though. [Android's activity lifecycle]: https://developer.android.com/guide/components/activities/activity-lifecycle
048c9c2 to
f242da9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
... and not just when the activity is active. Android's activity lifecycle denotes that
onResume()andonPause()are called when the activity changes the "active" state (which is separate from the "focused" state) and may or may not be followed or preceded by theonStart()-onStop()lifecycle which denotes Activity visibility. The latter more closely relates to when a surface is present (since when the activity is not active, but still visible redraws and resizes are still possible and expected) but also doesn't exactly represent it.Note that Android fires its
WindowResizedandRedrawNeededcallbacks at least once, directly afterInitWindow, and should (but this is not explicitly documented) never fire again afterTerminateWindow.Theoretically this isn't the right state to key off of for arbitrary
EventLoopwakeups via theProxythough.(PR spun out from #3786)
changelogmodule if knowledge of this change could be valuable to users