Conversation
|
I bumped the MSRV to I would appreciate it if you could test it works in other platforms, I have tried on Windows 11 and it works great! |
|
It's likely undesirable, but the deprecated API can still be supported temporarily by implementing a sealed trait for |
glutin-winit/Cargo.toml
Outdated
| [package] | ||
| name = "glutin-winit" | ||
| version = "0.4.2" | ||
| version = "0.5.0" |
| fail-fast: false | ||
| matrix: | ||
| rust_version: [1.65.0, stable, nightly] | ||
| rust_version: [1.70.0, stable, nightly] |
There was a problem hiding this comment.
Bump rust-version in a separate commit or in a separate PR.
There was a problem hiding this comment.
Separate commit? How does that matter? Anyway, I downgraded it again to 1.65 in this PR, but it now fails https://github.com/rust-windowing/glutin/actions/runs/8950601264/job/24586112813?pr=1675#step:8:30, I can open another PR if you want
There was a problem hiding this comment.
I'll do that myself, in general, you should just do a separate commit before your work because you require 1.70.
I'll just send a PR myself bumping MSRV.
| match event { | ||
| WindowEvent::CloseRequested => event_loop.exit(), | ||
| WindowEvent::Resized(size) => { | ||
| if size.width != 0 && size.height != 0 { |
There was a problem hiding this comment.
Can move to guard while at it.
| ) { | ||
| match event { | ||
| WindowEvent::Resized(size) => { | ||
| if size.width != 0 && size.height != 0 { |
There was a problem hiding this comment.
What guard? There is no multithreading on the standard example
There was a problem hiding this comment.
WinodwEvent::Resized(size) if size.width
There was a problem hiding this comment.
Ah! Sure, I misunderstood
glutin_examples/src/lib.rs
Outdated
| } else { | ||
| // Only Windows requires the window to be present before creating the display. |
There was a problem hiding this comment.
Could you extract all of that into a separate method on Application?
glutin_examples/src/lib.rs
Outdated
| not_current_gl_context: Option<NotCurrentContext>, | ||
| gl_context: Option<PossiblyCurrentContext>, | ||
| gl_surface: Option<Surface<WindowSurface>>, | ||
| window: Option<Window>, | ||
| gl_config: Option<Config>, | ||
| renderer: Option<Renderer>, |
There was a problem hiding this comment.
For the context you should use the same field (treat_as_possibly_current). All gl resources, must be dropped before the window, also add a note that Window is dropped at the end.
Render should be dropped first, because it uses the global GL state. Should document that as well.
All of that applies to multithreaded example as well.
There was a problem hiding this comment.
For the context you should use the same field (treat_as_possibly_current).
Can you clarify this?
treat_as_possibly_current consumes the NotCurrentContext so I need to store PossiblyCurrentContext in order to use it 🤔
There was a problem hiding this comment.
yes, just always store PossiblyCurrentContext
Yeah, we should do that as well. |
|
Hey @kchibisov , just updated with the latest changes from |
glutin-winit/src/event_loop.rs
Outdated
| /// Even though [ActiveEventLoop] is the recommended way for using the event | ||
| /// loop we still want to have support for [EventLoop], for now. |
There was a problem hiding this comment.
| /// Even though [ActiveEventLoop] is the recommended way for using the event | |
| /// loop we still want to have support for [EventLoop], for now. | |
| /// Even though [`ActiveEventLoop`] is the recommended way for using the event | |
| /// loop we still want to have support for [`EventLoop`], for now. |
There was a problem hiding this comment.
Also: "we still want" is not a valid/useful documentation commit, if you don't first outline why this trait exists and what it does.
glutin-winit/src/lib.rs
Outdated
| /// with it when the [`WindowBuilder`] was passed with | ||
| /// [`Self::with_window_builder`]. It's optional, since on some | ||
| /// with it when the [`WindowAttributes`] was passed with | ||
| /// [`Self::with_window_attributes`]. It's optional, since on some |
There was a problem hiding this comment.
| /// [`Self::with_window_attributes`]. It's optional, since on some | |
| /// [`Self::with_window_attributes()`]. It's optional, since on some |
glutin-winit/src/lib.rs
Outdated
| /// **WGL:** - [`WindowBuilder`] **must** be passed in | ||
| /// [`Self::with_window_builder`] if modern OpenGL(ES) is desired, | ||
| /// **WGL:** - [`WindowAttributes`] **must** be passed in | ||
| /// [`Self::with_window_attributes`] if modern OpenGL(ES) is desired, |
There was a problem hiding this comment.
| /// [`Self::with_window_attributes`] if modern OpenGL(ES) is desired, | |
| /// [`Self::with_window_attributes()`] if modern OpenGL(ES) is desired, |
| #[cfg(android_platform)] | ||
| println!("Android window available"); |
There was a problem hiding this comment.
Comparing to #1678, you lost this but still kept Android window removed.
|
|
||
| # 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. |
There was a problem hiding this comment.
Should mention changes to glutin-winit itself.
| fn create_window(&self, window_attributes: WindowAttributes) -> Result<Window, OsError>; | ||
|
|
||
| fn glutin_display_handle(&self) -> Result<DisplayHandle<'_>, HandleError>; |
There was a problem hiding this comment.
Should document what both are for.
|
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. |
Co-Authored-By: marc2332 <mespinsanz@gmail.com> Co-Authored-By: Marijn Suijten <marijns95@gmail.com>
…ct AppState { … }`
…ext` construction
…g to new `App::handle_event`
…dle_event` branches into new `{window,user}_event` handlers
…tLoop::run` to `EventLoop::run_app`
…ventLoop::run_app`
…ventLoop` in `winit` 0.30.0 Co-Authored-By: Erich Gubler <erichdongubler@gmail.com>
658acb2 to
bd053c1
Compare
|
Just pushed @ErichDonGubler changes in this PR so we get the best of both 😄 |
|
Closing as for some reason #1678 has been merged instead of this. |
|
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? |
|
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. |
Closes #1674
CHANGELOG.mdif knowledge of this change could be valuable to users