Skip to content
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

iOS: Refactor event handling to share code with macOS #3865

Merged
merged 1 commit into from
Aug 15, 2024

Conversation

madsmtm
Copy link
Member

@madsmtm madsmtm commented Aug 14, 2024

Instead of storing the event handler within the AppState, and extracting it our every time we need it, we now use the same event handling implementation as for macOS that ensures we don't re-entrantly call the event handler, and that we un-register the handler again after we're done using it (UIApplicationMain won't return, but may still unwind, so this is very important for panic safety).

This is the first step of me trying to untangle the inscrutable mess that is the current event handling on iOS.

  • Tested on all platforms changed

@madsmtm madsmtm added DS - ios S - maintenance Repaying technical debt labels Aug 14, 2024
@madsmtm madsmtm force-pushed the madsmtm/uikit-cleanup-event-handling branch from 6ae7dc4 to 9d42d00 Compare August 14, 2024 04:46
Instead of storing the event handler within the AppState, and extracting
it our every time we need it, we now use the same event handling
implementation as for macOS that ensures we don't re-entrantly call the
event handler, and that we un-register the handler again after we're
done using it (`UIApplicationMain` won't return, but may still unwind,
so this is very important for panic safety).
@madsmtm madsmtm force-pushed the madsmtm/uikit-cleanup-event-handling branch from 9d42d00 to 6e9d0dc Compare August 14, 2024 04:54
Comment on lines +66 to +67
static GLOBAL: StaticMainThreadBound<OnceCell<EventHandler>> =
StaticMainThreadBound(OnceCell::new());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remembering our talk about not storing anything in static memory.
No clue what the story here is exactly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I would still like to achieve that at some point, but it's going to be quite difficult with the event handler (especially so on AppKit, where we have to submit events from inside -[NSApplication sendEvent:], without the ability to store anything there, since I plan to swizzle (replace) the method to avoid requiring WinitApplication being registered as the main application).

The EventHandler is effectively moved from AppState, which is already stored in a static, so this PR doesn't really change the status quo (we just have two smaller statics now, instead of one big one).

@madsmtm madsmtm merged commit a61e7bb into master Aug 15, 2024
58 checks passed
@madsmtm madsmtm deleted the madsmtm/uikit-cleanup-event-handling branch August 15, 2024 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DS - ios S - maintenance Repaying technical debt
Development

Successfully merging this pull request may close these issues.

2 participants