-
Notifications
You must be signed in to change notification settings - Fork 912
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
Split out Surface
from Window
#3942
base: master
Are you sure you want to change the base?
Split out Surface
from Window
#3942
Conversation
Still wonder whether to use Also not sure about the |
I'd also add a |
I'll argue that
The
I already addressed event handling in my proposal. Summary:
|
With the Windows commit, that's every platform I can test on my machine at the moment. I don't have a Mac nor do I have Android development set up at the moment. |
Now that I'm assigned for review: I'd like this PR description and the resulting squashed commit to contain a description and justification of the respective changes, rather than merely mentioning that it addresses some part of a large discussion thread in an issue. That should make it easier to review and backtrack. |
I've updated the PR description. |
5b40621
to
9c7aae3
Compare
9c7aae3
to
1428596
Compare
Note to maintainers: I'm able to test Linux (X11 and Wayland) and Windows personally, but not any of the other platforms. As of now, I'm currently relying on CI to ensure the other platforms work (which given we don't have integration tests, I can't confirm they work). What should I do? |
Given that PR should simply move code around it's unilkely to break things, unless you decide to do something fancy, though, PRs like that require approval from maintainers on the platforms you change stuff, so don't worry about it. |
Surface
from Window
Surface
from Window
The implementation should be complete. Not sure why the formatting CI run failed, since I ran |
it requires nightly. |
This should be ready for review now. |
236e2ba
to
be6b958
Compare
That's not correct on wayland. |
that's true, though not in the context of what winit has. Also, you can safely bubble them to ensure that all the platforms are consistent here. |
changelog
module if knowledge of this change could be valuable to usersThis PR splits some methods of
Window
into a new supertraitSurface
, laying groundwork for the implementation of #3928.As stated there, this split is needed because popups and subsurfaces may not necessarily allow the same operations as windows.