Skip to content

Conversation

@Liamolucko
Copy link
Contributor

  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior this is purely a signature change, so no extra docs are needed on top.
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

This PR changes the signature of EventLoop::run to make the EventLoopWindowTarget passed to the callback live for 'static, and changes the signature of EventLoop::run_return to make the EventLoopWindowTarget live for as long as the reference to the event loop.

The main motivation for this is writing an async executor on top of winit (ref #1199). Such an executor needs to pass an EventLoopWindowTarget to the future to create windows with, which then needs to live for the entirety of the future (i.e., 'static).

This plus #2294 should make it possible to write an async executor on top of winit.

Copy link
Member

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

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

GitHub gave me a ping on this for some reason?

Anyhow, I'm gonna have to reject this change; we're actively trying to get the codebase in a spot where the EventLoopWindowTarget (now ActiveEventLoop) is specifically not static, but rather an &mut reference to internal data, and which might change upon each iteration.

The main motivation for this is writing an async executor on top of winit

I believe this may already have been done in async-winit?

In any case, the way to solve this would be via. message-passing and by waking the event loop, and letting it process things in there.

@madsmtm madsmtm closed this Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants