Skip to content
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

[wgpu-hal] Upgrade glutin to 0.31 #6150

Merged
merged 2 commits into from
Aug 27, 2024
Merged

Conversation

MarijnS95
Copy link
Contributor

Connections
Closes #5649 and should help towards #5709 where raw-window-handle can be used.

Description
glutin 0.30 onwards completely refactored its internals to no longer be reliant on winit, as they (by default) have no direct relation except needing to perform some operations (platform-specific) at strategic times in window creation and event loop handling. Most of that is handled by a new glutin-winit introp crate, while the core glutin crate now exclusively focuses on wrapping the various OpenGL context APIs (CGL, EGL, WGL, ...).

This does result in a little more verbose handling to get the right GLDisplay, GLConfig, GLContext and GLSurface, but gives much more control and makes all intricacies more explicit. Most of the code was copied from glutin 0.31's example crate, with the code for transparency support removed.

Note that the example doesn't at all handle event loop events properly: resizes and redraws are not listened to, and mobile-specific surface events (Resumed and Suspended) are equally ignored.

Testing
cargo r --example raw-gles on wayland only with an AMD GPU and Mesa drivers (for now).

Checklist

  • Run cargo fmt.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
    • --target wasm32-unknown-emscripten
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.
    • Probably irrelevant for this kind of example-only change?

@MarijnS95 MarijnS95 requested a review from a team as a code owner August 23, 2024 21:46
@MarijnS95
Copy link
Contributor Author

MarijnS95 commented Aug 23, 2024

Note that the example doesn't at all handle event loop events properly: resizes and redraws are not listened to, and mobile-specific surface events (Resumed and Suspended) are equally ignored.

This is probably something we should get over with now as it made/makes it easier to copy-paste the boiler-plate over from the glutin example, rather than trying to reconstruct/rethink it after the fact. EDIT: Done.

@MarijnS95
Copy link
Contributor Author

Regarding the build-failure, glutin is not supporting CGL on iOS, only MacOS: https://docs.rs/crate/glutin/0.31.3/source/build.rs

@MarijnS95 MarijnS95 force-pushed the glutin-0.31 branch 2 times, most recently from a22cada to 0181b88 Compare August 24, 2024 09:04
@MarijnS95
Copy link
Contributor Author

The duplicate miniz_oxide was introduced in #6135 but I guess the crate wasn't used until my PR here, which is why cargo-deny didn't stumble over it before?

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Looking good in general, some small questions

wgpu-hal/examples/raw-gles.rs Outdated Show resolved Hide resolved
wgpu-hal/examples/raw-gles.rs Show resolved Hide resolved
`glutin 0.30` onwards completely refactored its internals to no longer
be reliant on `winit`, as they (by default) have no direct relation
except needing to perform _some_ operations (platform-specific) at
strategic times in window creation and event loop handling.  Most of
that is handled by a new `glutin-winit` introp crate, while the core
`glutin` crate now exclusively focuses on wrapping the various OpenGL
context APIs (CGL, EGL, WGL, ...).

This does result in a little more verbose handling to get the right
`GLDisplay`, `GLConfig`, `GLContext` and `GLSurface`, but gives much
more control and makes all intricacies more explicit.  Most of the
code was copied from `glutin 0.31`'s example crate, with the code for
transparency support removed.

Note that the example doesn't at all handle event loop events properly:
resizes and redraws are not listened to, and mobile-specific surface
events (`Resumed` and `Suspended`) are equally ignored.
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Thanks!

@cwfitzgerald cwfitzgerald merged commit 690a3fb into gfx-rs:trunk Aug 27, 2024
25 checks passed
@MarijnS95 MarijnS95 deleted the glutin-0.31 branch August 27, 2024 19:35
MarijnS95 added a commit to MarijnS95/wgpu that referenced this pull request Aug 28, 2024
PR gfx-rs#6150 suffered a much larger rebase "hell" than I anticipated.  On
my Linux box I made this change, but lost it while force-pushing from
Windows (and created some other compiler errors while at it...).

By disabling all features on `glutin`/`glutin-winit` (the latter only
uses `x11`, and only forwards `wayland` to `glutin`) we may have dropped
a lot of "unused" dependencies for other GL backends, but also made the
crate unable to import X11 (Xlib/Xcb) and Wayland handles into EGL.

Also import the missing `glutin::context::Version` struct again which
was added last-minute to gfx-rs#6150 (to make sure my Intel card on Windows
creates a GLES 3.0+ instead of GLES 2.0 context) while the import was
accidentally squashed into gfx-rs#6152 (not merged yet).
teoxoy pushed a commit that referenced this pull request Aug 28, 2024
PR #6150 suffered a much larger rebase "hell" than I anticipated.  On
my Linux box I made this change, but lost it while force-pushing from
Windows (and created some other compiler errors while at it...).

By disabling all features on `glutin`/`glutin-winit` (the latter only
uses `x11`, and only forwards `wayland` to `glutin`) we may have dropped
a lot of "unused" dependencies for other GL backends, but also made the
crate unable to import X11 (Xlib/Xcb) and Wayland handles into EGL.

Also import the missing `glutin::context::Version` struct again which
was added last-minute to #6150 (to make sure my Intel card on Windows
creates a GLES 3.0+ instead of GLES 2.0 context) while the import was
accidentally squashed into #6152 (not merged yet).
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.

2 participants