Make default WindowBuilder Send and Sync#3136
Make default WindowBuilder Send and Sync#3136kchibisov merged 1 commit intorust-windowing:masterfrom
WindowBuilder Send and Sync#3136Conversation
madsmtm
left a comment
There was a problem hiding this comment.
I disagree that this is the right way to solve this issue, window handles are thread safe on macOS/iOS, since we guarantee to never use them outside the main thread.
Rather, we should wrap the window handle in something that implements Send + Sync (or maybe we should figure it out in rust-windowing/raw-window-handle#85 instead).
|
@madsmtm the issue is not only a window handle as it was said. You can see the CI logs and discover, that all the Web can't be Send, what should we do about it? As well as monitors on macOS. |
|
Like, surely, we could keep the |
|
I believe the solution is the same on the web: Since the window handle can only be created and accessed from the main thread, then it doesn't matter if the handle was sent between threads (or service workers) in between those accesses. @daxpedda?
I think it's the same thing here, monitors can be |
daxpedda
left a comment
There was a problem hiding this comment.
I'm against introducing this generic, we already have solutions in place for this kind of issue in MacOS and Web, we just need to apply them here as well as @madsmtm already suggested.
For Web, we just need to wrap the innards into MainThreadSafe, that's it. I'm happy to figure it out once NotSendSyncWindowAttributes is removed.
38f7b29 to
fa3aabe
Compare
kchibisov
left a comment
There was a problem hiding this comment.
I'm only not sure what to do with the video mode stuff, Like it's not send/sync in general, but given that macOS will allow creating it only on the main thread, and then winit will use it on the main thread, I think just marking Fullscreen in builder should be fine.
There probably better solutions, but searching for macOS docs on what is MT-safe and what is not is really hard.
src/lib.rs
Outdated
| /// mean thread if the platform demands it. Thus marking such objects as `Send + Sync` is safe. | ||
| #[doc(hidden)] | ||
| #[derive(Clone, Debug)] | ||
| pub struct SendSyncWrapper<T>(pub T); |
There was a problem hiding this comment.
Not sure whether this object should be public at all, the only reason it's public is because we allow access to WindowAttributes and also building them with struct builder.
We could make it crate private and just provide a get/set for parent` on WindowAttributes?
fa3aabe to
3f605e7
Compare
Window builder is always accessed by winit on the thread event loop is on, thus it's safe to mark the data it gets as `Send + Sync`. Each unsafe object is marked individually as `Send + Sync` instead of just implementing `Send` and `Sync` for the whole builder.
3f605e7 to
8e6b336
Compare
|
This is broken on Web and shouldn't be EDIT: False alarm, I forgot |
|
Exactly, we discussed that, it's always used on the thread event loop is on. |
The default window builder is neither
SendnorSyncdue to use ofRawWindowHandleinternally. While we could ensure to make the handleSend + Syncwithin our use, we can't guarantee that the same will hold true in the future.Thus add a generic to split the builder into
MT-safeand notMT-safeversions.This could also help with the lifetimes, since default could be bound by static lifetime.
CHANGELOG.mdif knowledge of this change could be valuable to users