rust-windowing / glutin

A low-level library for OpenGL context creation
Apache License 2.0
1.98k stars 476 forks source link

feat: Winit 0.30.0 #1675

Closed marc2332 closed 3 months ago

marc2332 commented 4 months ago

Closes https://github.com/rust-windowing/glutin/issues/1674

image

marc2332 commented 4 months ago

I bumped the MSRV to 1.70, updated to winit 0.30 and migrate both glutin-winit and the winit-dependant examples to the new trait-based EventLoop API

I would appreciate it if you could test it works in other platforms, I have tried on Windows 11 and it works great!

justincredible commented 4 months ago

It's likely undesirable, but the deprecated API can still be supported temporarily by implementing a sealed trait for EventLoop<T> and ActiveEventLoop, then using &impl MySealedTrait instead of &ActiveEventLoop as the parameter to DisplayBuilder::build, create_display, and finalize_window. An enum or Fn + closures also work but are even more awkward.

kchibisov commented 4 months ago

It's likely undesirable, but the deprecated API can still be supported temporarily by implementing a sealed trait for EventLoop and ActiveEventLoop, then using &impl MySealedTrait instead of &ActiveEventLoop as the parameter to DisplayBuilder::build, create_display, and finalize_window. An enum or Fn + closures also work but are even more awkward.

Yeah, we should do that as well.

marc2332 commented 4 months ago

Hey @kchibisov , just updated with the latest changes from master as I just noticed you bumped the MSRV some days ago, do you mind reviewing again? Thanks 😄

kchibisov commented 3 months ago

Could apply something like that

diff --git a/glutin-winit/CHANGELOG.md b/glutin-winit/CHANGELOG.md
index a5c458f..9935361 100644
--- a/glutin-winit/CHANGELOG.md
+++ b/glutin-winit/CHANGELOG.md
@@ -1,8 +1,7 @@
 # Unreleased

-# Version 0.5.0
-
 - **Breaking:** Update _winit_ to `0.30`. See [winit's CHANGELOG](https://github.com/rust-windowing/winit/releases/tag/v0.30.0) for more info.
+- Add trait `GlutinEventLoop` to handle `{Active,}EventLoop` simultaneously.

 # Version 0.4.2

diff --git a/glutin_examples/Cargo.toml b/glutin_examples/Cargo.toml
index 981760a..281e1bd 100644
--- a/glutin_examples/Cargo.toml
+++ b/glutin_examples/Cargo.toml
@@ -23,7 +23,7 @@ glutin = { path = "../glutin", default-features = false }
 glutin-winit = { path = "../glutin-winit", default-features = false }
 png = { version = "0.17.6", optional = true }
 raw-window-handle = "0.6"
-winit = { version = "0.30.0", default-features = false, features = ["rwh_05"] }
+winit = { version = "0.30.0", default-features = false, features = ["rwh_06"] }

 [target.'cfg(target_os = "android")'.dependencies]
 winit = { version = "0.30.0", default-features = false, features = ["android-native-activity", "rwh_06"] }
diff --git a/glutin_examples/examples/switch_render_thread.rs b/glutin_examples/examples/switch_render_thread.rs
index fea5352..b751c87 100644
--- a/glutin_examples/examples/switch_render_thread.rs
+++ b/glutin_examples/examples/switch_render_thread.rs
@@ -81,19 +81,21 @@ impl Application {

 impl ApplicationHandler<PlatformThreadEvent> for Application {
     fn resumed(&mut self, active_event_loop: &ActiveEventLoop) {
-        if self.render_thread_senders.is_none() {
-            let (window, render_context) = create_window_with_render_context(active_event_loop)
-                .expect("Failed to create window.");
-            let render_context = Arc::new(Mutex::new(render_context));
+        if self.render_thread_senders.is_some() {
+            return;
+        }

-            let (_render_threads, render_thread_senders) =
-                spawn_render_threads(render_context, &self.event_loop_proxy);
+        let (window, render_context) =
+            create_window_with_render_context(active_event_loop).expect("Failed to create window.");
+        let render_context = Arc::new(Mutex::new(render_context));

-            self.window = Some(window);
-            self.render_thread_senders = Some(render_thread_senders);
+        let (_render_threads, render_thread_senders) =
+            spawn_render_threads(render_context, &self.event_loop_proxy);

-            self.send_event_to_current_render_thread(RenderThreadEvent::MakeCurrent);
-        }
+        self.window = Some(window);
+        self.render_thread_senders = Some(render_thread_senders);
+
+        self.send_event_to_current_render_thread(RenderThreadEvent::MakeCurrent);
     }

     fn user_event(&mut self, _event_loop: &ActiveEventLoop, event: PlatformThreadEvent) {
diff --git a/glutin_examples/src/lib.rs b/glutin_examples/src/lib.rs
index 496efdb..b3e498c 100644
--- a/glutin_examples/src/lib.rs
+++ b/glutin_examples/src/lib.rs
@@ -49,14 +49,9 @@ impl Application {
     fn create_window(&mut self, active_event_loop: &ActiveEventLoop) {
         // Only Windows requires the window to be present before creating the display.
         // Other platforms don't really need one.
-        //
-        // XXX if you don't care about running on Android or so you can safely remove
-        // this condition and always pass the window builder.
-        let window_attributes = cfg!(wgl_backend).then(|| {
-            Window::default_attributes()
-                .with_transparent(true)
-                .with_title("Glutin triangle gradient example (press Escape to exit)")
-        });
+        let window_attributes = Window::default_attributes()
+            .with_transparent(true)
+            .with_title("Glutin triangle gradient example (press Escape to exit)");

         // The template will match only the configurations supporting rendering
         // to windows.
@@ -69,7 +64,7 @@ impl Application {
         let template =
             ConfigTemplateBuilder::new().with_alpha_size(8).with_transparency(cfg!(cgl_backend));

-        let display_builder = DisplayBuilder::new().with_window_attributes(window_attributes);
+        let display_builder = DisplayBuilder::new().with_window_attributes(Some(window_attributes));

         let (window, gl_config) = display_builder
             .build(active_event_loop, template, gl_config_picker)

Since at the current state of things it doesn't work on anything other than windows, I guess.

marc2332 commented 3 months ago

Just pushed @ErichDonGubler changes in this PR so we get the best of both 😄

marc2332 commented 3 months ago

Closing as for some reason https://github.com/rust-windowing/glutin/pull/1678 has been merged instead of this. Both have the same changes anyway, so if anybody was using this branch from git you shall not have any issues when changing the branch to master or when using the official release when it comes out.

ErichDonGubler commented 3 months ago

I think @kchibisov missed what was happening here. Perhaps I should have closed that PR, to make it clear that this was the one we had united behind?

kchibisov commented 3 months ago

I looked into both PRs, and I was only able to push into @ErichDonGubler one, so I just used the later, because I needed to push some minor fixes myself.

Both PRs were the same, since I checked the diffs and just used my own patch on top of whatever worked for me to push.