Windows: Make EventLoopWindowTarget independent of UserEvent type
#3061
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
9 tasks
madsmtm
changed the title
EventLoopWindowTarget independent of UserEvent typeEventLoopWindowTarget independent of UserEvent type
madsmtm
reviewed
Aug 29, 2023
madsmtm
force-pushed
the
non-generic-ELWT-windows
branch
from
3925723 to
dac99dd
Compare
|
@msiglreith @maroider last call for getting a review here, otherwise I'll merge this soon, I'd like to progress on #3053. |
5 tasks
the `EventLoopWindowTarget` is needed for window creation. conceptually, only `EventLoop` and `EventLoopProxy` need to be parameterized, and all other parts of the backend should be agnostic about the user event type, parallel to how `Event<T>` is parameterized, but `WindowEvent` is not. this change removes the dependency on the type of user events from the `EventLoopWindowTarget` for the Windows backend, but keep a phantom data to keep the API intact. to achieve this, I moved the `Receiver` end of the mpsc channel from `ThreadMsgTargetData` into `EventLoop` itself, so the `UserEvent` is only passed between `EventLoop` and `EventLoopProxy`, all other part of the backend just use unit type as a placeholder for user events. it's similar to the macos backend where an erased `EventHandler` trait object is used so all component except `EventLoop` and `EventLoopProxy` need to be parameterized. however `EventLoop` of the Windows backend already use an `Box<dyn FnMut>` to wrap the user provided event handler callback, so no need for an dedicated trait object, I just modified the wrapper to replace the placeholder user event with real value pulled from the channel. I find this is the approach which need minimum change to be made to existing code. but it does the job and could serve as a starting point to future Windows backend re-works.
this field is transitional and her to keep API compatibility only. the correct variance and such is already ensured by the top-level `EventLoopWindowTarget`, just use `PhantomData<T>` here.
madsmtm
force-pushed
the
non-generic-ELWT-windows
branch
from
dac99dd to
e0dab55
Compare
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.
the
EventLoopWindowTargetis needed for window creation. conceptually, onlyEventLoopandEventLoopProxyneed to be parameterized, and all other parts of the backend should be agnostic about the user event type, parallel to howEvent<T>is parameterized, butWindowEventis not.this change removes the dependency on the type of user events from the
EventLoopWindowTargetfor the Windows backend, but keep a phantom data to keep the API intact. to achieve this, I moved theReceiverend of the mpsc channel fromThreadMsgTargetDataintoEventLoopitself, so theUserEventis only passed betweenEventLoopandEventLoopProxy, all other part of the backend just use unit type as a placeholder for user events.it's similar to the macos backend where an erased
EventHandlertrait object is used so all component exceptEventLoopandEventLoopProxyneed to be parameterized. howeverEventLoopof the Windows backend already use anBox<dyn FnMut>to wrap the user provided event handler callback, so no need for an dedicated trait object, I just modified the wrapper to replace the placeholder user event with real value pulled from the channel. I find this is the approach which need minimum change to be made to existing code. but it does the job and could serve as a starting point to future Windows backend re-works.CHANGELOG.mdif knowledge of this change could be valuable to users