rust-windowing / winit

Window handling library in pure Rust
https://docs.rs/winit/
Apache License 2.0
4.86k stars 909 forks source link

Documentation on `Event::{Suspended, Resumed}` is lacking #2185

Closed teryror closed 2 years ago

teryror commented 2 years ago

I'm in the early stages of building a desktop application, and trying to figure out which events I need to handle, and how. In general, I'd like the documentation to include an overview of "events to handle to achieve basic behavior end users would expect", but as it is, I'm just looking at the page for the winit::event::Event type. While not ideal, that would be fine, however:

The single-sentence descriptions of the Suspended and Resumed events aren't very helpful. I tried logging when receiving them, but I couldn't get them to fire at all (on Ubuntu 21.10 running X11). I initially expected them in response to minimizing and re-opening the window, respectively, but evidently that's not how they work. Multiple google searches didn't help, so I ended up looking at the winit source code. Based on that, it seems these events are only emitted on iOS and Android, but I would like confirmation that this is correct.

There are more questions left open by the documentation - even if I'm right, and these event types don't matter to my use case. Here's what I would assume these events behave like, based on pretty much nothing but their names:

It's also unclear when they can even fire relative to the other event types - maybe include them in the "traditional event-handling loop" reference code sample in the documentation for the winit::event module? Since I don't care about mobile platforms, I can't be bothered to reverse engineer how their respective platform layers work, but either way, I'd expect all of this to be clearly communicated somewhere. (Maybe it is, just in a very non-obvious place?)

Thanks in advance!

rib commented 2 years ago

I've recently been working on using winit on Android, (in particular experimenting with an alternative "glue" layer based on the GameActivity class that's part of Google's Android Game Development Kit) and have found there are a few particular details that make application portability to Android tricky right now.

The main inconsistency I've seen with Android is that applications must be prepared to re-create their surface state each time the application is Resumed and drop their surface state when it's Suspended. This is required due to how the OS actually destroys/creates the native window for the application whenever it's paused.

Although it's not necessary to recreate surfaces like this for other window systems so far, it still has made me wonder whether it would be helpful for portability if all OSs / window systems would report Resumed events, so that this could become the standard place for render state and surfaces to be created and initialized. (currently they are just emitted on Android + iOS)

There would still be the inconsistency of needing to drop/recreate surface state on Android but getting all platforms to defer their initialization of graphics state and surface state until they are Resumed instead of being done immediately when creating their event loop would already be a big step forwards for ensuring that code would be easier to adapt to also run on Android.

For all platforms besides Android then we could basically just emit a Resumed event immediately after the NewEvents(StartCause::Init) that all platforms send.

(sorry, that this doesn't answer the question about documentation, but I figured that atm the lack of documentation probably just reflects that this hasn't been considered in a lot of detail yet. I wanted to share this so we can maybe consider changing the current behaviour before it gets set in stone via more explicit docs)

rib commented 2 years ago

To try and reply to some specifics, from what I've seen from poking around recently...

  • They strictly alternate, i.e. ignoring all other events, an Event::Suspended must be followed by an Event::Resumed and vice-versa.

Yeah, hopefully it's the case but I guess I would er on the side of caution in case of awkward corner cases / races / bugs, where back-to-back suspends or resumes should be squashed/ignored if they somehow slip through to the app (there's currently no common code that automatically guarantees backends can't emit redundant events like that).

Back-to-back redundant events could e.g. be conceivable if there are multiple triggers for a suspend that might not strictly be mutually exclusive at a low level. E.g. on Android the backend suspends based on the native window being destroyed. Potentially in the future the backed should also directly respond to the 'paused' lifecycle notification and a naive / buggy backend could easily end up emitting a Suspended event for both OS events back to back. In the iOS backend it's not clear if the termination path will guarantee a suspend will be delivered before termination. I guess there might be other corner cases.

These are the lifecycle states for Android / iOS:

On iOS there are notably two routes to becoming 'foreground' - but I don't know if there's any cause for concern with potentially muddling up the state tracking here.

On Android there are two directions in the state machine that you could return to 'resumed' from being paused (either directly back, or it could get 'stopped', then 'started' before being 'resumed' (so not onion skin layering - the app can apparently get resumed from a higher 'started' state instead of a lower 'paused' state) - I can also imagine mistakes being possible with the state tracking here.

The fact that the Android backend suspends based on window terminate/created events means it's not strictly driven by the above life cycle events and there might be more gotchas with that approach that haven't been considered. I'm not sure I've seen clear guarantees about when the window gets destroyed with respect to paused/stopped events. I think right now the backend chooses to suspend based on the window being destroyed because that immediately impacts the application which must no longer touch that window - so without more fine-grained lifecycle events, it's probably the most important event to communicate on Android.

  • After the application is Suspended, it won't receive any events until it is Resumed.

Hopefully there won't be significant OS events while suspended but I also wouldn't expect there's much strict guarantee for this. Winit doesn't expose low-memory events on mobile atm but I could imagine some corner cases where they might be delivered out of sync with pausing, or e.g. a race wake up for input events. More significantly though, user events could be delivered or there could be wake ups due to timeouts or redraw requests. The notable things that you can expect won't be delivered while suspended on Android are window Resized and Redraw events. On iOS the backend becomes suspended based applicationWillResignActive which e.g. doesn't event seem to strictly block redrawing at that stage: https://developer.apple.com/documentation/uikit/uiapplicationdelegate/1622950-applicationwillresignactive (unlike applicationDidEnterBackground which seems to clearly stop all UI updates)

  • (Assuming they are indeed exclusive to mobile platforms) Suspended fires when the user switches to another app (or the home screen), but not when it is killed. Resumed fires when it is reopened later.

I think on Android I'd also expect the app to transition to suspended before it got killed (I'm pretty sure the window will be explicitly destroyed and it will go through the paused/ stopped states before either being killed or sometimes getting an explicitly notification about being destroyed)

I don't know about iOS, though i can see they have a will_terminate callback that jumps straight to sending a WindowEvent::Destroyed - I guess it depends on whether the OS always takes the app through applicationWillResignActivie which is currently the only place that would trigger a suspend in the iOS backend.

rib commented 2 years ago

Just as a random example of the kind of broken assumption I was imagining when I was thinking that back-to-back suspends/resumes might happen in corner cases I stumbled across this stackoverflow page where someone was empirically tracing when their Android surface gets destroyed in relation to lifecycle events:

https://stackoverflow.com/questions/69422482/lifecycle-of-surfaceview

What they found was that in some circumstances when their application was being terminated they would actually get an unexpected second surface (maybe that part was a random bug/quirk of their app) and both would only be destroyed after onDestroyed - not in sync with the onPaused lifecycle event.

On the one had it should probably reinforce that idea that the Android backend should probably be reporting apps as suspended based onPaused or onSurfaceDestroyed but it also shows that this state tracking can get ugly in corner cases so if winit wants to try and guarantee there will never be redundant suspended / resumed events it might be best to have some common filter code in winit that doesn't need to rely on every backend getting this 100% right.

MarijnS95 commented 2 years ago

@rib I've added this issue to the 0.27 milestone. Without reading all the details of your messages above, do you think we can piggyback this issue to address the platform divergence specifically, and leave https://github.com/rust-windowing/winit/pull/2307 for an immediate lifetime fix and https://github.com/rust-windowing/winit/issues/2293 for a separate, future discussion of migrating away from ndk_glue (or better: replace revamp ndk_glue ie. by replacing it with your new crate).

rib commented 2 years ago

Hey @MarijnS95 hopefully you'll see PR https://github.com/rust-windowing/winit/pull/2331 which takes a stab at consistently emitting a Resumed event on all platforms and adding some docs about portability / recommending that apps lazily initialize render state on resume.

MarijnS95 commented 2 years ago

@rib Do you think we should go one step further and remove RawWindowHandle from Window, instead only provide its value in Resumed(RawWindowHandle)? Or per other discussions, revise how windows are made available through the loop entirely?

Definitely for a followup-PR though.

rib commented 2 years ago

Something like that could be a strong nudge towards getting apps to be portable by default, but as a breaking change it would probably deserve quite a bit more consideration and trying out how that might work in practice.

Initially I'd be inclined to start with this simpler, backwards compatible change and like you said we could think about a follow up along these lines perhaps.

I'm personally not super satisfied with the opaque RawWindowHandle as a general concept. At the moment it's essentially cooperatively defined as "whatever is the useful handle that egl/glx/wgl/vulkan/wgpu/... etc might want to make some kind of render target". Sometimes I think it could be a little ambiguous what handle you want depending on what you need the native handle for. I think it's good to have a way to punch though and get e.g. the x11 handle for a window, or maybe the wl_surface or the HWND etc but if I'm doing something platform specific I feel like I'd also want to know exactly what I'm getting, instead of an opaque handle. The fact that it's opaque right now means it's tricky to define the use cases for the handle to try and sanity check whether it would be too restrictive to only expose the handle via Resumed events - because all use cases have to be compatible with whatever APIs like wgpu expect the handle to be. Also not sure if the handles are can be freely copied and what the implications of that are.

Another option.. oooh pun incoming.. I had considered at some point was to at least make the API for querying the native handle return an Option<>

MarijnS95 commented 2 years ago

A RawWindowHandle is straightforward to "punch through": they're all structs and you can require e.g. an AndroidNdkHandle specifically when your function only supports Android. Does that fully address your concern or did I miss something (e.g. per-platform cfgs for compiletime safety here)?

However yes, I do agree that RawWindowHandle lacks any context of lifetime nor ownership, but not sure if it should take on that burden somehow.

Another option.. oooh pun incoming.. I had considered at some point was to at least make the API for querying the native handle return an Option<>

Yes, that way the caller has to handle this explicitly. But alas, that'd require a breaking change on the HasRawWindowHandle trait :/