Change Window from a borrowed handle to a weakly owned handle
#174
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
|
Win32 backend update done, tested and functional! 🙂 |
prokopyl
force-pushed
the
remove-window-lifetime
branch
3 times, most recently
from
bb1108a
to
2ef375f
Compare
This was referenced Mar 26, 2024
prokopyl
force-pushed
the
remove-window-lifetime
branch
4 times, most recently
from
4a82bfc
to
999f97d
Compare
prokopyl
force-pushed
the
remove-window-lifetime
branch
from
e10382d
to
231bbc6
Compare
glowcoil
pushed a commit
that referenced
this pull request
Apr 5, 2024
This PR splits off the X11 event loop logic into a separate module. It also changes the X11 implementation of the `Window` type to take only a shared reference to the inner type (`&WindowInner` instead of `&mut WindowInner`), bringing it in line with the other backends. This does not change any of the logic however, it only separates some of the window state from the event loop state, to make sure they don't step on each other's toes in the future (particularly around the WindowHandler). This is part of the effort to split up #174 into smaller pieces.
More post-rebase fixes, add missing flush call Post-rebase fixes Fix windows OpenGL context creation Minor fixes Squashed: Port X11 backend to x11rb Fix macOS build Win32 refactor Make the added with_scale_factor fn pub(crate) for now Remove unneeded internal HasRawWindowHandle implementation Simplify everything by making the public Window/GlContext types hold the Weak themselves Remove lifetime from Window type, refactor and split X11 backend Added functional open_parented example
prokopyl
force-pushed
the
remove-window-lifetime
branch
from
231bbc6
to
0b58445
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.
This PR implements the idea discussed with @glowcoil on Discord a few days ago, which is to make the
Windowhandle'staticand clone-able, so that it can be easily stored in theWindowHandler.Note this PR is based on #172 for now, which is why the new example shows up in the diff. This can be changed depending on #172's merge status.This has been properly rebased now.This makes the following breaking API changes, for all platforms and backends:
Windowtype.Windowtype is now an owned, weak handle to the actual underlying window.Windowtype clone-able.Windowargument passed to theWindowHandlerbuilder is now a fully owned value instead of a temporary borrow, which enables the handler to store it for its whole lifetime.Windowargument onWindowHandler::on_frameandWindowHandler::on_eventmethods.Windowmethods now only take&selfinstead of&mut self.Windowmethods are now internally fallible: they will panic if the actual window the handle points to has been destroyed. This can only happen if theWindowHandlermisbehaves and clones theWindowoutside of its own lifetime.GlContexthas received the same treatment, for the same reasons: it's now a weak, owned clone-able handle to the GL context of a window, and its methods will also panic if the underlying context is destroyed.As of now, this PR is still a WIP:
The X11 backend is complete and functional, but it will probably need to be rebased on top of Switch from xcb crate to x11rb #173 before this PR can be merged.It's done!I started working on the Win32 backend, but it's not done yet.Done!I haven't touched the Cocoa backend, and it will not build at all at the moment.Done, but still needs testing.As of now, this PR will not build on macOS or Windows until I've completed the work mentioned above.