Change run_app(app: &mut A) to run_app(app: A)

[madsmtm]

Since #3709, we do a blanket impl of ApplicationHandler for &mut references. This means that we can now change run_app_x methods to consume the application instead of taking a reference to it.

This allows the user more control over how they pass their application state to Winit.

Internally, each backend is free to convert the app to an &mut app (or &mut dyn ApplicationHandler) if that's easier for them to pass around.

• Tested on all platforms changed
• Added an entry to the changelog module if knowledge of this change could be valuable to users

[kchibisov]

The API is intentionally that way because we want to &mut dyn in the future, which I don't think we'll be able to otherwise without some kind of boxing of the entire state.

[madsmtm]

Hmm, I think you can still convert it internally to &mut dyn, if need be?

E.g. (playground):

trait ApplicationHandler {}

fn run_app<A: ApplicationHandler>(mut app: A) {
    run_app_inner(&mut app)
}


fn run_app_inner(app: &mut dyn ApplicationHandler) {
    // ...
}

But perhaps I'm missing something?

[kchibisov]

You can not really have generic on the trait, which you want to make &mut dyn.

What's the end goal with this?

[madsmtm]

You can not really have generic on the trait, which you want to make &mut dyn.

Hmm, from what I understood, it's the ActiveEventLoop that you want to use in dyn, right? I.e. use &mut dyn ActiveEventLoop in the methods on ApplicationHandler?

What's the end goal with this?

It's twofold:

1. Moving Event::NewEvents(StartCause::Init) to an initialization step, something like:

   struct MyApp {
       window: Window, // Doesn't need to be `Option` any more
   }
   impl ApplicationHandler for MyApp { ... }
   
   let attrs = WindowAttributes::new();
   // Pass closure that runs once in `applicationDidFinishLaunching:` and initializes the application
   event_loop.run_app_with_init(|event_loop: &ActiveEventLoop| {
       let window = event_loop.create_window(attrs);
       MyApp { window }
   })?;

2. Removing exiting, and using Drop for this instead. Normal cleanup that users expect to be able to do in Drop (e.g. close open files and flush buffers, ...) currently doesn't happen on iOS, you have to do it manually inside exiting.

(Not directly proposing either of those here, in particular the story around number 1 and Android isn't fleshed out, but that's at least how it ought to work on iOS (i.e. the current "drop window on Suspended" is wrong there)).

[madsmtm]

Resolution from meeting: We want to do this, though the path is a bit more unclear for pump events. I'll try to flesh out the PR some more.

Things noted in our meeting document:

1. Box<dyn A> vs &mut dyn A.
2. run_on_demand and pump_events should take and return the state instead of &mut, so it's consistent internally.

[madsmtm]

Responding to @kchibisov's concerns:

Box<dyn A> vs &mut dyn A.

Whether to use Box or a reference is an implementation detail for each backend. In the AppKit and UIKit implementations, I will probably end up using Box<dyn A> in the future, since it allows me to drop the application state when the event loop is terminating (which it otherwise won't be).

run_on_demand and pump_events should take and return the state instead of &mut, so it's consistent internally.

I changed these to have the same interface as run_app, and from a type-system perspective it seems to work fine?

For run_on_demand I definitely think it makes sense to allow passing in the application state, as the user might e.g. want to run an application twice, without sharing state between these runs.

I agree that usually the user would probably want to pass &mut app to pump_events, but that is not a strict requirement, and I think being generic here is desirable? But it's not a hill I want to die on, and I guess the compiler can guide the user better if we require &mut.

[kchibisov]

The main issue with it is that later on you can not really put it on a trait that way, though, maybe it'll be possible somehow, but for now it's fine.