rust-windowing / winit

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

Allow the user to register the application delegate on macOS and iOS #3758

Closed madsmtm closed 1 month ago

madsmtm commented 3 months ago

Register for event-loop lifecycle events using the NSNotification API instead of application delegates. This gives the user full control over the application delegate, which opens several doors for customization that were previously closed.

In particular, this fixes https://github.com/rust-windowing/winit/issues/1751, closes https://github.com/rust-windowing/winit/pull/3713, fixes https://github.com/rust-windowing/winit/issues/2674, fixes https://github.com/rust-windowing/winit/pull/3499, fixes https://github.com/rust-windowing/winit/pull/3650 and fixes https://github.com/rust-windowing/winit/pull/2141, all by allowing the user to override [NS|UI]ApplicationDelegate themselves outside Winit.

Also fixes https://github.com/rust-windowing/winit/issues/1281 by listening on notifications instead of using the application delegate.

Resolves part of the macOS side of https://github.com/rust-windowing/winit/issues/2120.

madsmtm commented 2 months ago

Also does it affect performance/input latency in any aspect

I looked into it:

On AppKit, when calling -[NSApplication setDelegate:], most of the delegate's NSApplicationDelegate methods (at least the ones we use) are internally registered as observer methods for the notifications we use here. So the performance impact of this PR is basically zero.

On UIKit, the delegate method is called directly, while the notification is emitted immediately afterwards. I'd suspect the performance impact of this PR to be very minimal, as UIKit still creates the NSNotification object even when there are no observers for it, but I haven't measured it.

In any case, you don't need to be concerned with buffering of notifications or other such shenanigans, the notifications are emitted at (basically) the same time as the methods are called.

Korne127 commented 2 months ago

@kchibisov Just to make sure you didn't miss it

kchibisov commented 2 months ago

@Korne127 miss what? I approved the PR long time ago...

Korne127 commented 2 months ago

Sorry if I'm wrong, to me it just looked like you were expected to give feedback to the new changes / address Mads' answer to your question.

kchibisov commented 2 months ago

I've asked for feedback just to confirm my assumption, otherwise I'd request changes.

nicoburns commented 2 months ago

This looks absolutely fantastic. This would solve a lot of the problems I've had with Winit on MacOS. Giving the application control of the AppDelegate is excellent on it's own. But I'm particular excited that this approach seems to open up the possibility of modular libraries that independently hook into apple system events!

madsmtm commented 1 month ago

The holdup for this PR has not been Kirill at all, it's because I was on vacation ;)

Korne127 commented 1 month ago

Woohoo! Awesome :D Thanks for all your work on this, and I hope you had a great vacation! :)