Swizzle sendEvent: instead of subclassing NSApplication
#4036
+162
−47
Conversation
This file contains 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
madsmtm
force-pushed
the
madsmtm/swizzle-app
branch
from
7cad081 to
1c80f39
Compare
This is done to avoid order-dependent behavior that you'd otherwise encounter where `EventLoop::new` had to be called at the beginning of `fn main` to ensure that Winit's application was the one being registered as the main application by calling `sharedApplication`. Fixes #3772. This should also make it (more) possible to use multiple versions of Winit in the same application (though that's still untested). Finally, it should allow the user to override `NSApplication` themselves if they need to do that for some reason.
madsmtm
force-pushed
the
madsmtm/swizzle-app
branch
from
1c80f39 to
62cb23f
Compare
kchibisov
requested changes
Dec 25, 2024
| @@ -76,6 +76,7 @@ changelog entry. | |||
| - Added `Window::surface_position`, which is the position of the surface inside the window. | |||
| - Added `Window::safe_area`, which describes the area of the surface that is unobstructed. | |||
| - On X11, Wayland, Windows and macOS, improved scancode conversions for more obscure key codes. | |||
| - On macOS, no longer need control of the main `NSApplication` class (which means you can now override it yourself). | |||
There was a problem hiding this comment.
I think it should be in the changed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is done to avoid order-dependent behavior that you'd otherwise encounter where
EventLoop::newhad to be called at the beginning offn mainto ensure that Winit's application was the one being registered as the main application by callingsharedApplication.Fixes #3772.
This should also make it (more) possible to use multiple versions of Winit in the same application (though that's still untested and officially unsupported).
Finally, it should allow the user to override
NSApplicationthemselves if they need to do that for some reason.The solution chosen here, method swizzling, is a bit magical (though fully supported by the runtime), alternatives include:
NSApplication, and exposing a method for users to call themselves if they overrideNSApplication. This would be quite confusing, since suddenly overriding the application class would disable parts of Winit's behaviour (unless you knew to call this exposed method), and it still wouldn't fix the ordering issue.NSApplication, and class-swizzling that in. This is still swizzling, and would just be more work to do (note also that we wouldn't be able to add instance variables to such a class, since we don't allocate it ourselves).Just to re-confirm that overriding
sendEvent:is a good idea by itself, I'll note that this is also what's done in other UI frameworks, including the SDL and FLTK.changelogmodule if knowledge of this change could be valuable to usersNSApplicationthat I doubt it makes sense to provide an example. We already have an example of overriding theNSApplicationDelegate, that should be enough.