Windows: Make EventLoopWindowTarget independent of UserEvent type#3061
Merged
madsmtm merged 4 commits intorust-windowing:masterfrom Jan 4, 2024
nerditation:non-generic-ELWT-windows
Merged
Windows: Make EventLoopWindowTarget independent of UserEvent type#3061madsmtm merged 4 commits intorust-windowing:masterfrom nerditation:non-generic-ELWT-windows
EventLoopWindowTarget independent of UserEvent type#3061madsmtm merged 4 commits intorust-windowing:masterfrom
nerditation:non-generic-ELWT-windows
Conversation
9 tasks
EventLoopWindowTarget independent of UserEvent typeEventLoopWindowTarget independent of UserEvent type
madsmtm
reviewed
Aug 29, 2023
Member
madsmtm
left a comment
There was a problem hiding this comment.
I'm not sure about the rest, indeed there are room for future improvement, but the idea looks fine to my eyes for now. Would like a Windows expert to have a look first though.
Member
|
@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.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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