From ac0b9c952f4ac6b61b45b793df3496e48a588ddf Mon Sep 17 00:00:00 2001 From: Mick van Gelderen Date: Wed, 2 Oct 2024 23:54:14 +0200 Subject: [PATCH 1/4] WIP --- glutin_examples/src/lib.rs | 351 +++++++++++++++++++++++-------------- 1 file changed, 217 insertions(+), 134 deletions(-) diff --git a/glutin_examples/src/lib.rs b/glutin_examples/src/lib.rs index e5b75e17bf..699268f987 100644 --- a/glutin_examples/src/lib.rs +++ b/glutin_examples/src/lib.rs @@ -1,6 +1,6 @@ use std::error::Error; use std::ffi::{CStr, CString}; -use std::num::NonZeroU32; +use std::num::{NonZero, NonZeroU32}; use std::ops::Deref; use gl::types::GLfloat; @@ -15,7 +15,7 @@ use glutin::config::{Config, ConfigTemplateBuilder, GetGlConfig}; use glutin::context::{ ContextApi, ContextAttributesBuilder, NotCurrentContext, PossiblyCurrentContext, Version, }; -use glutin::display::GetGlDisplay; +use glutin::display::{Display, GetGlDisplay}; use glutin::prelude::*; use glutin::surface::{Surface, SwapInterval, WindowSurface}; @@ -45,92 +45,24 @@ pub fn main(event_loop: winit::event_loop::EventLoop<()>) -> Result<(), Box { - let (window, gl_config) = match display_builder.clone().build( - event_loop, - self.template.clone(), - gl_config_picker, - ) { - Ok((window, gl_config)) => (window.unwrap(), gl_config), - Err(err) => { - self.exit_state = Err(err); - event_loop.exit(); - return; - }, - }; - - println!("Picked a config with {} samples", gl_config.num_samples()); - - // Mark the display as initialized to not recreate it on resume, since the - // display is valid until we explicitly destroy it. - self.gl_display = GlDisplayCreationState::Init; - - // Create gl context. - self.gl_context = - Some(create_gl_context(&window, &gl_config).treat_as_possibly_current()); - - (window, gl_config) - }, - GlDisplayCreationState::Init => { - println!("Recreating window in `resumed`"); - // Pick the config which we already use for the context. - let gl_config = self.gl_context.as_ref().unwrap().config(); - match glutin_winit::finalize_window(event_loop, window_attributes(), &gl_config) { - Ok(window) => (window, gl_config), - Err(err) => { - self.exit_state = Err(err.into()); - event_loop.exit(); - return; - }, - } - }, - }; - - let attrs = window - .build_surface_attributes(Default::default()) - .expect("Failed to build surface attributes"); - let gl_surface = - unsafe { gl_config.display().create_window_surface(&gl_config, &attrs).unwrap() }; - - // The context needs to be current for the Renderer to set up shaders and - // buffers. It also performs function loading, which needs a current context on - // WGL. - let gl_context = self.gl_context.as_ref().unwrap(); - gl_context.make_current(&gl_surface).unwrap(); - - self.renderer.get_or_insert_with(|| Renderer::new(&gl_config.display())); - - // Try setting vsync. - if let Err(res) = gl_surface - .set_swap_interval(gl_context, SwapInterval::Wait(NonZeroU32::new(1).unwrap())) - { - eprintln!("Error setting vsync: {res:?}"); - } - - assert!(self.state.replace(AppState { gl_surface, window }).is_none()); + self.transition(event_loop, |state| match state { + AppState::Uninitialized(state) => state.initialize(event_loop).map(AppState::Resumed), + AppState::Resumed(state) => Ok(AppState::Resumed(state)), + AppState::Suspended(state) => state.resume(event_loop).map(AppState::Resumed), + }); } - fn suspended(&mut self, _event_loop: &ActiveEventLoop) { - // This event is only raised on Android, where the backing NativeWindow for a GL - // Surface can appear and disappear at any moment. - println!("Android window removed"); - - // Destroy the GL Surface and un-current the GL Context before ndk-glue releases - // the window back to the system. - self.state = None; - - // Make context not current. - self.gl_context = Some( - self.gl_context.take().unwrap().make_not_current().unwrap().treat_as_possibly_current(), - ); + fn suspended(&mut self, event_loop: &ActiveEventLoop) { + self.transition(event_loop, |state| match state { + AppState::Uninitialized { .. } => Err("invalid transition".into()), + AppState::Resumed(state) => state.suspend().map(AppState::Suspended), + AppState::Suspended(state) => Ok(AppState::Suspended(state)), + }); } fn window_event( @@ -140,23 +72,40 @@ impl ApplicationHandler for App { event: WindowEvent, ) { match event { - WindowEvent::Resized(size) if size.width != 0 && size.height != 0 => { - // Some platforms like EGL require resizing GL surface to update the size - // Notable platforms here are Wayland and macOS, other don't require it - // and the function is no-op, but it's wise to resize it for portability - // reasons. - if let Some(AppState { gl_surface, window: _ }) = self.state.as_ref() { - let gl_context = self.gl_context.as_ref().unwrap(); - gl_surface.resize( - gl_context, - NonZeroU32::new(size.width).unwrap(), - NonZeroU32::new(size.height).unwrap(), - ); - - let renderer = self.renderer.as_ref().unwrap(); - renderer.resize(size.width as i32, size.height as i32); + WindowEvent::Resized(size) => { + if let (Some(width), Some(height)) = + (NonZeroU32::new(size.width), NonZero::new(size.height)) + { + self.transition(event_loop, |state| { + match &state { + AppState::Uninitialized { .. } => { + return Err("invalid transition".into()) + }, + AppState::Resumed(state) => { + state.resize(width, height); + }, + AppState::Suspended(AppStateSuspended { renderer, .. }) => { + // TODO: Should we call resize while suspended or should we not do + // anything here? + renderer.resize(size.width as i32, size.height as i32); + }, + }; + Ok(state) + }); } }, + WindowEvent::RedrawRequested => { + self.transition(event_loop, |state| { + match &state { + AppState::Uninitialized { .. } => return Err("invalid transition".into()), + AppState::Resumed(state) => { + state.redraw()?; + }, + AppState::Suspended { .. } => {}, + } + Ok(state) + }); + }, WindowEvent::CloseRequested | WindowEvent::KeyboardInput { event: KeyEvent { logical_key: Key::Named(NamedKey::Escape), .. }, @@ -167,32 +116,65 @@ impl ApplicationHandler for App { } fn exiting(&mut self, _event_loop: &ActiveEventLoop) { - // NOTE: The handling below is only needed due to nvidia on Wayland to not crash - // on exit due to nvidia driver touching the Wayland display from on - // `exit` hook. - let _gl_display = self.gl_context.take().unwrap().display(); + self.final_transition(|state| { + // NOTE: The handling below is only needed due to nvidia on Wayland to not crash + // on exit due to nvidia driver touching the Wayland display from on + // `exit` hook. + + let _gl_display = match state { + AppState::Uninitialized { .. } => return Err("invalid transition".into()), + AppState::Resumed(AppStateResumed { gl_context, renderer, gl_surface, window }) => { + // Clear the window. + drop(gl_surface); + drop(window); + drop(renderer); + gl_context.display() + }, + AppState::Suspended(AppStateSuspended { gl_context, renderer }) => { + drop(renderer); + gl_context.display() + }, + }; + + Ok(()) + }); + } +} + +struct TerminateDisplayOnDrop>(T); + +// impl TerminateDisplayOnDrop { +// fn make_current(self, surface: ()) -> TerminateDisplayOnDrop { +// unsafe { +// let old = std::mem::ManuallyDrop::new(self); +// let context = std::ptr::read(std::ptr::from_ref(&old.0)); +// match +// TerminateDisplayOnDrop(context.make_current(surface)) +// } +// } +// } - // Clear the window. - self.state = None; +impl> Drop for TerminateDisplayOnDrop { + fn drop(&mut self) { #[cfg(egl_backend)] #[allow(irrefutable_let_patterns)] - if let glutin::display::Display::Egl(display) = _gl_display { + if let glutin::display::Display::Egl(display) = self.0.display() { unsafe { display.terminate(); } } } +} - fn about_to_wait(&mut self, _event_loop: &ActiveEventLoop) { - if let Some(AppState { gl_surface, window }) = self.state.as_ref() { - let gl_context = self.gl_context.as_ref().unwrap(); - let renderer = self.renderer.as_ref().unwrap(); - renderer.draw(); - window.request_redraw(); - - gl_surface.swap_buffers(gl_context).unwrap(); - } - } +fn create_gl_surface( + window: &Window, + gl_config: &Config, +) -> Result, Box> { + let attrs = window + .build_surface_attributes(Default::default()) + .expect("Failed to build surface attributes"); + let gl_surface = unsafe { gl_config.display().create_window_surface(gl_config, &attrs)? }; + Ok(gl_surface) } fn create_gl_context(window: &Window, gl_config: &Config) -> NotCurrentContext { @@ -231,49 +213,150 @@ fn create_gl_context(window: &Window, gl_config: &Config) -> NotCurrentContext { } } +fn enable_vsync(gl_surface: &Surface, gl_context: &PossiblyCurrentContext) { + // Try setting vsync. + if let Err(res) = + gl_surface.set_swap_interval(gl_context, SwapInterval::Wait(NonZeroU32::new(1).unwrap())) + { + eprintln!("Error setting vsync: {res:?}"); + } +} + fn window_attributes() -> WindowAttributes { Window::default_attributes() .with_transparent(true) .with_title("Glutin triangle gradient example (press Escape to exit)") } -enum GlDisplayCreationState { - /// The display was not build yet. - Builder(DisplayBuilder), - /// The display was already created for the application. - Init, -} - struct App { - template: ConfigTemplateBuilder, - renderer: Option, - // NOTE: `AppState` carries the `Window`, thus it should be dropped after everything else. state: Option, - gl_context: Option, - gl_display: GlDisplayCreationState, exit_state: Result<(), Box>, } +const INCONSISTENT: &str = "application was left in an inconsistent state"; + impl App { - fn new(template: ConfigTemplateBuilder, display_builder: DisplayBuilder) -> Self { + fn new(template_builder: ConfigTemplateBuilder, display_builder: DisplayBuilder) -> Self { Self { - template, - gl_display: GlDisplayCreationState::Builder(display_builder), + state: Some(AppState::Uninitialized(AppStateUninitialized { + template_builder, + display_builder, + })), exit_state: Ok(()), - gl_context: None, - state: None, - renderer: None, } } + + fn exit_state(self) -> Result<(), Box> { + debug_assert!(self.state.is_none()); + self.exit_state + } + + fn transition Result>>( + &mut self, + event_loop: &ActiveEventLoop, + f: F, + ) { + match f(self.state.take().expect(INCONSISTENT)) { + Ok(state) => self.state = Some(state), + Err(error) => { + event_loop.exit(); + self.exit_state = Err(error); + }, + } + } + + fn final_transition Result<(), Box>>(&mut self, f: F) { + self.exit_state = f(self.state.take().expect(INCONSISTENT)); + } } -struct AppState { - gl_surface: Surface, +enum AppState { + Uninitialized(AppStateUninitialized), + Resumed(AppStateResumed), + Suspended(AppStateSuspended), +} + +struct AppStateUninitialized { + template_builder: ConfigTemplateBuilder, + display_builder: DisplayBuilder, +} + +impl AppStateUninitialized { + fn initialize(self, event_loop: &ActiveEventLoop) -> Result> { + let Self { template_builder, display_builder } = self; + let (window, gl_config) = + display_builder.build(event_loop, template_builder, gl_config_picker)?; + let window = window.ok_or("failed to create window")?; + println!("Picked a config with {} samples", gl_config.num_samples()); + let gl_context = create_gl_context(&window, &gl_config); + let gl_surface = create_gl_surface(&window, &gl_config)?; + let gl_context = gl_context.make_current(&gl_surface)?; + enable_vsync(&gl_surface, &gl_context); + let renderer = Renderer::new(&gl_config.display()); + Ok(AppStateResumed { window, gl_surface, gl_context, renderer }) + } +} + +struct AppStateResumed { + gl_context: PossiblyCurrentContext, + renderer: Renderer, // NOTE: Window should be dropped after all resources created using its // raw-window-handle. + gl_surface: Surface, window: Window, } +impl AppStateResumed { + fn suspend(self: AppStateResumed) -> Result> { + let AppStateResumed { gl_context, renderer, gl_surface, window } = self; + println!("Android window removed"); + drop(gl_surface); + drop(window); + let gl_context = gl_context.make_not_current()?; + Ok(AppStateSuspended { gl_context, renderer }) + } + + fn resize(&self, width: NonZero, height: NonZero) { + // Some platforms like EGL require resizing GL surface to update the + // size Notable platforms here are + // Wayland and macOS, other don't require it + // and the function is no-op, but it's wise to resize it for + // portability reasons. + self.gl_surface.resize(&self.gl_context, width, height); + + self.renderer.resize(width.get() as i32, height.get() as i32); + } + + fn redraw(&self) -> Result<(), Box> { + self.window.pre_present_notify(); + self.renderer.draw(); + self.window.request_redraw(); // TODO: Document why we need to request a redraw. + self.gl_surface.swap_buffers(&self.gl_context)?; + Ok(()) + } +} + +struct AppStateSuspended { + gl_context: NotCurrentContext, + renderer: Renderer, +} + +impl AppStateSuspended { + fn resume(self, event_loop: &ActiveEventLoop) -> Result> { + let AppStateSuspended { gl_context, renderer } = self; + println!("Recreating window in `resumed`"); + // Pick the config which we already use for the context. + + let gl_config = gl_context.config(); + let window = glutin_winit::finalize_window(event_loop, window_attributes(), &gl_config)?; + let gl_surface = create_gl_surface(&window, &gl_config)?; + let gl_context = gl_context.make_current(&gl_surface)?; + enable_vsync(&gl_surface, &gl_context); + + Ok(AppStateResumed { gl_context, renderer, gl_surface, window }) + } +} + // Find the config with the maximum number of samples, so our triangle will be // smooth. pub fn gl_config_picker(configs: Box + '_>) -> Config { From 79c433c7facc7723acd8ccfed41ff736c22af72e Mon Sep 17 00:00:00 2001 From: Mick van Gelderen Date: Wed, 2 Oct 2024 23:59:53 +0200 Subject: [PATCH 2/4] WIP --- glutin_examples/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/glutin_examples/src/lib.rs b/glutin_examples/src/lib.rs index 699268f987..9fad4727f7 100644 --- a/glutin_examples/src/lib.rs +++ b/glutin_examples/src/lib.rs @@ -1,6 +1,6 @@ use std::error::Error; use std::ffi::{CStr, CString}; -use std::num::{NonZero, NonZeroU32}; +use std::num::NonZeroU32; use std::ops::Deref; use gl::types::GLfloat; @@ -74,7 +74,7 @@ impl ApplicationHandler for App { match event { WindowEvent::Resized(size) => { if let (Some(width), Some(height)) = - (NonZeroU32::new(size.width), NonZero::new(size.height)) + (NonZeroU32::new(size.width), NonZeroU32::new(size.height)) { self.transition(event_loop, |state| { match &state { @@ -144,8 +144,8 @@ impl ApplicationHandler for App { struct TerminateDisplayOnDrop>(T); // impl TerminateDisplayOnDrop { -// fn make_current(self, surface: ()) -> TerminateDisplayOnDrop { -// unsafe { +// fn make_current(self, surface: ()) -> +// TerminateDisplayOnDrop { unsafe { // let old = std::mem::ManuallyDrop::new(self); // let context = std::ptr::read(std::ptr::from_ref(&old.0)); // match @@ -316,7 +316,7 @@ impl AppStateResumed { Ok(AppStateSuspended { gl_context, renderer }) } - fn resize(&self, width: NonZero, height: NonZero) { + fn resize(&self, width: NonZeroU32, height: NonZeroU32) { // Some platforms like EGL require resizing GL surface to update the // size Notable platforms here are // Wayland and macOS, other don't require it From 5ec0c00bb8761f446f83446128ca53d14befef01 Mon Sep 17 00:00:00 2001 From: Mick van Gelderen Date: Thu, 3 Oct 2024 09:25:22 +0200 Subject: [PATCH 3/4] Finish context terminate display on drop --- glutin_examples/src/lib.rs | 137 ++++++++++++++++++++----------------- 1 file changed, 73 insertions(+), 64 deletions(-) diff --git a/glutin_examples/src/lib.rs b/glutin_examples/src/lib.rs index 9fad4727f7..1848bcaa0d 100644 --- a/glutin_examples/src/lib.rs +++ b/glutin_examples/src/lib.rs @@ -116,46 +116,50 @@ impl ApplicationHandler for App { } fn exiting(&mut self, _event_loop: &ActiveEventLoop) { - self.final_transition(|state| { - // NOTE: The handling below is only needed due to nvidia on Wayland to not crash - // on exit due to nvidia driver touching the Wayland display from on - // `exit` hook. - - let _gl_display = match state { - AppState::Uninitialized { .. } => return Err("invalid transition".into()), - AppState::Resumed(AppStateResumed { gl_context, renderer, gl_surface, window }) => { - // Clear the window. - drop(gl_surface); - drop(window); - drop(renderer); - gl_context.display() - }, - AppState::Suspended(AppStateSuspended { gl_context, renderer }) => { - drop(renderer); - gl_context.display() - }, - }; - - Ok(()) - }); + self.final_transition(|_| Ok(())) + } +} + +// TODO: Should this be done by glutin by default? +struct ContextTerminateDisplay>(T); + +impl> Deref for ContextTerminateDisplay { + type Target = T; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl> ContextTerminateDisplay { + unsafe fn into_inner_without_drop(self) -> T { + std::ptr::read(&std::mem::ManuallyDrop::new(self).0) } } -struct TerminateDisplayOnDrop>(T); +impl ContextTerminateDisplay { + fn make_not_current( + self, + ) -> Result, glutin::error::Error> { + unsafe { self.into_inner_without_drop().make_not_current().map(ContextTerminateDisplay) } + } +} -// impl TerminateDisplayOnDrop { -// fn make_current(self, surface: ()) -> -// TerminateDisplayOnDrop { unsafe { -// let old = std::mem::ManuallyDrop::new(self); -// let context = std::ptr::read(std::ptr::from_ref(&old.0)); -// match -// TerminateDisplayOnDrop(context.make_current(surface)) -// } -// } -// } +impl ContextTerminateDisplay { + fn make_current( + self, + surface: &glutin::surface::Surface, + ) -> Result, glutin::error::Error> { + unsafe { self.into_inner_without_drop().make_current(surface).map(ContextTerminateDisplay) } + } +} -impl> Drop for TerminateDisplayOnDrop { +impl> Drop for ContextTerminateDisplay { fn drop(&mut self) { + // NOTE: The handling below is only needed due to nvidia on Wayland to not crash + // on exit due to nvidia driver touching the Wayland display from on + // `exit` hook. + #[cfg(egl_backend)] #[allow(irrefutable_let_patterns)] if let glutin::display::Display::Egl(display) = self.0.display() { @@ -177,7 +181,10 @@ fn create_gl_surface( Ok(gl_surface) } -fn create_gl_context(window: &Window, gl_config: &Config) -> NotCurrentContext { +fn create_gl_context( + window: &Window, + gl_config: &Config, +) -> ContextTerminateDisplay { let raw_window_handle = window.window_handle().ok().map(|wh| wh.as_raw()); // The context creation part. @@ -201,15 +208,17 @@ fn create_gl_context(window: &Window, gl_config: &Config) -> NotCurrentContext { let gl_display = gl_config.display(); unsafe { - gl_display.create_context(gl_config, &context_attributes).unwrap_or_else(|_| { - gl_display.create_context(gl_config, &fallback_context_attributes).unwrap_or_else( - |_| { - gl_display - .create_context(gl_config, &legacy_context_attributes) - .expect("failed to create context") - }, - ) - }) + ContextTerminateDisplay( + gl_display.create_context(gl_config, &context_attributes).unwrap_or_else(|_| { + gl_display.create_context(gl_config, &fallback_context_attributes).unwrap_or_else( + |_| { + gl_display + .create_context(gl_config, &legacy_context_attributes) + .expect("failed to create context") + }, + ) + }), + ) } } @@ -281,6 +290,21 @@ struct AppStateUninitialized { display_builder: DisplayBuilder, } +struct AppStateResumed { + // NOTE: Window should be dropped after all resources created using its raw-window-handle. + gl_surface: Surface, + window: Window, + // TODO: Does the context need to be dropped after the window? Specifically, the egl display + // terminate code, when should it run? + gl_context: ContextTerminateDisplay, + renderer: Renderer, +} + +struct AppStateSuspended { + gl_context: ContextTerminateDisplay, + renderer: Renderer, +} + impl AppStateUninitialized { fn initialize(self, event_loop: &ActiveEventLoop) -> Result> { let Self { template_builder, display_builder } = self; @@ -288,6 +312,7 @@ impl AppStateUninitialized { display_builder.build(event_loop, template_builder, gl_config_picker)?; let window = window.ok_or("failed to create window")?; println!("Picked a config with {} samples", gl_config.num_samples()); + // TODO: Can we create the context and the surface in any order? let gl_context = create_gl_context(&window, &gl_config); let gl_surface = create_gl_surface(&window, &gl_config)?; let gl_context = gl_context.make_current(&gl_surface)?; @@ -297,15 +322,6 @@ impl AppStateUninitialized { } } -struct AppStateResumed { - gl_context: PossiblyCurrentContext, - renderer: Renderer, - // NOTE: Window should be dropped after all resources created using its - // raw-window-handle. - gl_surface: Surface, - window: Window, -} - impl AppStateResumed { fn suspend(self: AppStateResumed) -> Result> { let AppStateResumed { gl_context, renderer, gl_surface, window } = self; @@ -317,13 +333,11 @@ impl AppStateResumed { } fn resize(&self, width: NonZeroU32, height: NonZeroU32) { - // Some platforms like EGL require resizing GL surface to update the - // size Notable platforms here are - // Wayland and macOS, other don't require it - // and the function is no-op, but it's wise to resize it for - // portability reasons. + // Some platforms like EGL require resizing GL surface to update the size + // Notable platforms here are Wayland and macOS, other don't require it + // and the function is no-op, but it's wise to resize it for portability + // reasons. self.gl_surface.resize(&self.gl_context, width, height); - self.renderer.resize(width.get() as i32, height.get() as i32); } @@ -336,11 +350,6 @@ impl AppStateResumed { } } -struct AppStateSuspended { - gl_context: NotCurrentContext, - renderer: Renderer, -} - impl AppStateSuspended { fn resume(self, event_loop: &ActiveEventLoop) -> Result> { let AppStateSuspended { gl_context, renderer } = self; From 3a8c47fa230dd954795e50173c00ac1ccda7dd94 Mon Sep 17 00:00:00 2001 From: Mick van Gelderen Date: Thu, 3 Oct 2024 09:43:58 +0200 Subject: [PATCH 4/4] Document safety of unsafe block --- glutin_examples/src/lib.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/glutin_examples/src/lib.rs b/glutin_examples/src/lib.rs index 1848bcaa0d..42116f9470 100644 --- a/glutin_examples/src/lib.rs +++ b/glutin_examples/src/lib.rs @@ -132,8 +132,10 @@ impl> Deref for ContextTerminateDisplay { } impl> ContextTerminateDisplay { - unsafe fn into_inner_without_drop(self) -> T { - std::ptr::read(&std::mem::ManuallyDrop::new(self).0) + fn into_inner_without_drop(self) -> T { + // SAFETY: We ensure that the contents of self are not aliased. Leaking a value + // is safe. + unsafe { std::ptr::read(&std::mem::ManuallyDrop::new(self).0) } } } @@ -141,7 +143,7 @@ impl ContextTerminateDisplay { fn make_not_current( self, ) -> Result, glutin::error::Error> { - unsafe { self.into_inner_without_drop().make_not_current().map(ContextTerminateDisplay) } + self.into_inner_without_drop().make_not_current().map(ContextTerminateDisplay) } } @@ -150,7 +152,7 @@ impl ContextTerminateDisplay { self, surface: &glutin::surface::Surface, ) -> Result, glutin::error::Error> { - unsafe { self.into_inner_without_drop().make_current(surface).map(ContextTerminateDisplay) } + self.into_inner_without_drop().make_current(surface).map(ContextTerminateDisplay) } }