-
Notifications
You must be signed in to change notification settings - Fork 920
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
Rework (split) Event::{Suspended, Resumed}
to more clearly convey their meaning (on Android)
#2337
Comments
Event::{Suspended, Resumed}
to more clearly convey their meaningEvent::{Suspended, Resumed}
to more clearly convey their meaning (on Android)
My initial thoughts here are:
Overall I see that Winit is a portability abstraction and for something like this I wouldn't necessarily expect a perfect match with the underlying system events, which may be ok if they still solve a practical problem. If we broadly define the Suspended/Resumed events as a mechanism for delineating when apps are paused/suspended from rendering I think these portable events can pretty much be kept as is; I would probably consider a few minor tweaks though:
Effectively trying to expose platform specific lifecycle semantics could end up being a pretty complex can of worms where I guess it may never be possible to come up with a super set of events that will cover the nuanced differences between platforms. One idea I had the other day was potentially to expose an event like Thinking of ways to more explicitly emphasize that applications should wait until they are It could still also be reasonable to consider adding some extra events like |
This is only in light of surfaces needing to be created/destroyed, which is an explicit action users of
Does the user need to destroy their surfaces on iOS here too (and to prevent being countered again: or leak it, as long a new surface is created in
When exactly would you fire this, then? Once for Other than that I agree that we don't need to strive for every platform-specific event under the sun, but I do feel that we can do a better job at making Regarding |
As far as I know Android is the only system that effectively invalidates window surfaces automatically, so I think on iOS they can count on their surfaces remaining valid. They can maybe destroy surfaces while they aren't rendering (while inactive) to help minimize memory usage - but as far as I recall there's no requirement to do this. For slightly improved consistency though, maybe it would make sense on iOS to map |
It would be a really small tweak in the existing backed logic, that would essentially track the two things separately internally (i.e. set a flag for the lifecycle paused/resumed state and a flag for the window create/destroyed state) and then whenever either flag is updated the backend would potentially dispatch a |
Is it common to do that to save on memory though? Otherwise I'm still inclined to have a very specific event solely related to surface creation and removal just to make intent completely clear. But it muddies the waters some when thinking about surface removal versus "you can only render within these events". Not sure if those should be considered the same or not...
You mean changing from the |
For the |
to clarify here; the logic here would determine when we're "suspended" or "resumed" and the backend can also explicitly track that, e.g. via some For reference though I did also include a note in the docs for #2331 that apps should be resilient to redundant events (even though they are easy to avoid here) just to reserve wiggle room for handling awkward, unforseen corner cases in backends and also just to be clear about what guarantees are provided by winit (re: #2185 where this specific question of redundant events was also discussed). |
@rib Regarding your explicit request to have logical operators between these events:
I don't think it's a good idea to generalize surface events together with the
So a graphics surface may very well continue to exist and be visible on screen, while destroying a graphics surface/swapchain on top of it could lead to unneeded corruption or black/empty surfaces. In the Then finally, you mentioned above that Android is the only platform where surfaces are created and destroyed (iOS devs can rely on the surface to remain existing during the lifetime of the app), so there isn't much to generalize anyway. Creation and destruction only has meaning on Android, and could get separate - clearly defined - events for that. |
Looks like we're sticking with the current terminology, but at least everyone is on the same page now that this is where surface creation/destruction should happen. |
The
Suspended
andResumed
events currently don't quite carry the load of "you must create and destroy your graphics resources created fromRawWindowHandle
1 within these events".At the same time they are disjoint from Android's
onPause
andonResume
lifecycle events (see also #2118), as the framework is "free" to not destroy the backing surface for its window even when the user switches away.As far as I understand these events are used this way in iOS.
Then there exist
WindowEvent::Focused(bool)
events which correspond to https://developer.android.com/reference/android/app/Activity#onWindowFocusChanged(boolean), that itself isn't part of the holistic activity lifecycle either.Finally these events don't correspond to a window being created nor
WindowEvent::Destroyed
, as the actvity itself generally stays alive. These events purely correspond to Android's need for managing the underlyingSurface
(a producer in a queue of images, not even directly the backing buffer of the window on screen), and IIRC map directly to the callbacks in https://developer.android.com/reference/android/view/SurfaceHolder.Callback.There doesn't seem to be any similar system in the other backends, and again
Resumed
andSuspended
don't quite relay this concept of surface handling. At the same time we are not relayingRelated
onPause
/onResume
lifecycle events toSuspended
andResumed
, showing that there's clearly a naming conflict on these events;Event::{Suspended, Resumed}
is lacking #2185: Documentation lacking for these events, showing that they aren't descriptive;Resumed
event on all platforms #2331: Raises theResumed
event across all platforms, and adds complicated "portability" documentation showcasing that the status-quo isn't the right way forward;winit
s misunderstanding and lacking mapping of Android's lifecycle and various events/callbacks to winit callbacks.CC @rib
More events to consider
(https://github.com/rust-windowing/winit/pull/2118/files#r777200781)
onStart
: No mappingonResume
:Resumed
WindowCreated
2:SurfaceCreated(RawWindowHandle?)
WindowHasFocus
:Focused(true)
WindowLostFocus
:Focused(false)
onPause
:Suspended
onStop
: No mappingWindowDestroyed
2:SurfaceDestroyed
Back
button/KeyCode
on Android #2304:CloseRequested
(if there's no other back stack??)Destroy
:WindowEvent::Destroyed
?And how do we deal with multiple
Activity
instances (multiple windows, really)?Footnotes
Surface
s, native windows, oh my! ↩ndk_glue
naming, becauseNativeWindow
==Surface
in the NDK. ↩ ↩2The text was updated successfully, but these errors were encountered: