Skip to content

Encapsulate glutin#1797

Merged
shaunsingh merged 3 commits intoneovide:mainfrom
fredizzimo:fsundvik/remove-glutin-refs
Mar 20, 2023
Merged

Encapsulate glutin#1797
shaunsingh merged 3 commits intoneovide:mainfrom
fredizzimo:fsundvik/remove-glutin-refs

Conversation

@fredizzimo
Copy link
Copy Markdown
Contributor

What kind of change does this PR introduce?

  • Refactor

This moves the Glutin context creation to it's own file. It also passes the winit window around rather than the whole context, so that the code mostly becomes dependent on winit rather than glutin.

The only direct reference to glutin is in the new opengl.rs file, but the context is still used outside of that, so some of the code is still dependent on it at an interface level.

This is done mostly to make it easier to review my upcoming render changes. But it should also be quite easy to switch over to not use a forked version of Glutin anymore. The new Glutin versions, no longer depend on winit, so we can use our own version of it. Instead we could directly do something similar as the Glutin winit module https://github.com/rust-windowing/glutin/tree/master/glutin-winit/src

Did this PR introduce a breaking change?

  • No

@shaunsingh
Copy link
Copy Markdown
Contributor

Code LGTM

Regarding switching to upstream glutin, as per #1789 the only change we have is neovide/glutin@cf50967 (apart from one other macOS change that can be upstreamed), our winit fork (https://github.com/neovide/winit/tree/new-keyboard-v3) matches upstream, so we can move everything to upstream if we'd like

(the PR linked above is currently broken on linux, but I believe the changes in this PR would fix it

Copy link
Copy Markdown
Contributor

@last-partizan last-partizan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@shaunsingh shaunsingh merged commit 2766fe7 into neovide:main Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants