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

Remove child thread from win32 event loop #445

Closed
wants to merge 4 commits into from

Conversation

Osspial
Copy link
Contributor

@Osspial Osspial commented Apr 2, 2018

This PR removes child thread creation from the win32 event loop, which has the side effect of getting rid of all the hacks present beforehand to get inter-thread communication working properly. It also updates the window callback to use win32's subclassing API to pass winit's data to windows, rather than going through thread-local storage.

At first glance, this closes #415, #391, and #94, though further testing is required and it may close other issues I'm not seeing.

OUTSTANDING ISSUES:

  • How do we handle Windows' modal loops (e.g. on resizing, moving)? Currently, the users' event handling code is paused, which is bad (not to mention inconsistent with other platforms).
  • EventsLoop handles events from ALL windows on a thread, not just windows created within the loop.

@Osspial
Copy link
Contributor Author

Osspial commented Apr 3, 2018

There are two main ways I can see to handle the modal loops:

  • Re-implement the modal loop code, in a non-blocking way, within winit.
  • Adjust the event loop API to give winit total control over the application's main loop, even while polling events, so that the user's provided callback can be called within the window procedure.

I'm in favor of the second option, as the first one would involve a lot of work on our end both to create the initial implementation, with proper window snapping support and such, as well as support additional features Microsoft may add in the future.

@Osspial Osspial force-pushed the single_thread_win32 branch from 7a71b84 to f3103ea Compare April 13, 2018 23:40
@francesca64 francesca64 added this to the EventsLoop 2.0 milestone May 6, 2018
@fschutt
Copy link

fschutt commented May 23, 2018

Any update on this? I'm currently waiting on this because of #506.

If winit ever gets menus / integration with the OS I need to be on the same thread as the HWND, otherwise, Windows just locks up the application.

I'm happy with any solution, but please just make sure that the .get_hwnd() call (which I need to implement Win32-specific functions that act on / modify the HWND) runs on the same thread as the user code.

@francesca64
Copy link
Member

@fschutt this is blocked on #459, where the current status is "waiting for people to respond to me" #459 (comment) (It's also ostensibly blocked by HiDPI support, which is awaiting responses too #105 (comment). The rationale for getting this in first is because #459 is very much a breaking change and IMO #105 is too important to gate behind that API migration.)

Most people would assume that the best way to help winit grow is by working on the code, but I'd say the best thing someone can do is push API discussions forward. I encourage you to participate in both of those discussions, since doing so will likely lead to a faster resolution.

@Osspial
Copy link
Contributor Author

Osspial commented Aug 24, 2018

Closing; obsoleted by #638.

@Osspial Osspial closed this Aug 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Race condition: run_forever can dispatch events after the window is destroyed
3 participants