From b9435541df664f36c0956007afe4155514eebc5c Mon Sep 17 00:00:00 2001 From: Tim Straubinger Date: Tue, 3 Sep 2024 00:29:19 -0700 Subject: [PATCH] Allow non-`static` `eframe::App` lifetime (#5060) I love egui! Thank you Emil <3 This request specifically enables an `eframe::App` which stores a lifetime. In general, I believe this is necessary because `eframe::App` currently does not seem to provide a good place to allocate and then borrow from long-lived data between `update()` calls. To attempt to borrow such long-lived data from a field of the `App` itself would be to create a self-referential struct. A hacky alternative is to allocate long-lived data with `Box::leak`, but that's a code smell and would cause problems if a program ever creates multiple Apps. As a more specific motivating example, I am developing with the [inkwell](https://github.com/TheDan64/inkwell/) crate which requires creating a `inkwell::context::Context` instance which is then borrowed from by a bazillion things with a dedicated `'ctx` lifetime. I need such a `inkwell::context::Context` for the duration of my `eframe::App` but I can't store it as a field of the app. The most natural solution to me is to simply to lift the inkwell context outside of the App and borrow from it, but that currently fails because the AppCreator implicitly has a `'static` lifetime requirement due to the use of `dyn` trait objects. Here is a simpler, self-contained motivating example adapted from the current [hello world example](https://docs.rs/eframe/latest/eframe/): ```rust use eframe::egui; struct LongLivedThing { message: String, } fn main() { let long_lived_thing = LongLivedThing { message: "Hello World!".to_string(), }; let native_options = eframe::NativeOptions::default(); eframe::run_native( "My egui App", native_options, Box::new(|cc| Ok(Box::new(MyEguiApp::new(cc, &long_lived_thing)))), // ^^^^^^^^^^^^^^^^^ // BORROWING from long_lived_thing in App ); } struct MyEguiApp<'a> { long_lived_thing: &'a LongLivedThing, } impl<'a> MyEguiApp<'a> { fn new(cc: &eframe::CreationContext<'_>, long_lived_thing: &'a LongLivedThing) -> Self { // Customize egui here with cc.egui_ctx.set_fonts and cc.egui_ctx.set_visuals. // Restore app state using cc.storage (requires the "persistence" feature). // Use the cc.gl (a glow::Context) to create graphics shaders and buffers that you can use // for e.g. egui::PaintCallback. MyEguiApp { long_lived_thing } } } impl<'a> eframe::App for MyEguiApp<'a> { fn update(&mut self, ctx: &egui::Context, frame: &mut eframe::Frame) { egui::CentralPanel::default().show(ctx, |ui| { ui.heading(&self.long_lived_thing.message); }); } } ``` This currently fails to compile with: ```plaintext error[E0597]: `long_lived_thing` does not live long enough --> src/main.rs:16:55 | 16 | Box::new(|cc| Ok(Box::new(MyEguiApp::new(cc, &long_lived_thing)))), | ----------------------------------------------^^^^^^^^^^^^^^^^---- | | | | | | | borrowed value does not live long enough | | value captured here | cast requires that `long_lived_thing` is borrowed for `'static` 17 | ); 18 | } | - `long_lived_thing` dropped here while still borrowed | = note: due to object lifetime defaults, `Box FnOnce(&'a CreationContext<'b>) -> Result, Box>>` actually means `Box<(dyn for<'a, 'b> FnOnce(&'a CreationContext<'b>) -> Result, Box> + 'static)>` ``` With the added lifetimes in this request, I'm able to compile and run this as expected on Ubuntu + Wayland. I see the CI has been emailing me about some build failures and I'll do what I can to address those. Currently running the check.sh script as well. This is intended to resolve https://github.com/emilk/egui/issues/2152 * Closes https://github.com/emilk/egui/issues/2152 * [x] I have followed the instructions in the PR template --- crates/eframe/src/epi.rs | 3 ++- crates/eframe/src/lib.rs | 2 +- crates/eframe/src/native/glow_integration.rs | 25 +++++++++++--------- crates/eframe/src/native/run.rs | 6 ++--- crates/eframe/src/native/wgpu_integration.rs | 22 ++++++++--------- crates/eframe/src/web/app_runner.rs | 2 +- crates/eframe/src/web/web_runner.rs | 2 +- 7 files changed, 33 insertions(+), 29 deletions(-) diff --git a/crates/eframe/src/epi.rs b/crates/eframe/src/epi.rs index 4f8f07d037f..d0ce7074395 100644 --- a/crates/eframe/src/epi.rs +++ b/crates/eframe/src/epi.rs @@ -46,7 +46,8 @@ type DynError = Box; /// This is how your app is created. /// /// You can use the [`CreationContext`] to setup egui, restore state, setup OpenGL things, etc. -pub type AppCreator = Box) -> Result, DynError>>; +pub type AppCreator<'app> = + Box) -> Result, DynError>>; /// Data that is passed to [`AppCreator`] that can be used to setup and initialize your app. pub struct CreationContext<'s> { diff --git a/crates/eframe/src/lib.rs b/crates/eframe/src/lib.rs index d3849efa5d7..80da6235792 100644 --- a/crates/eframe/src/lib.rs +++ b/crates/eframe/src/lib.rs @@ -229,7 +229,7 @@ pub mod icon_data; pub fn run_native( app_name: &str, mut native_options: NativeOptions, - app_creator: AppCreator, + app_creator: AppCreator<'_>, ) -> Result { #[cfg(not(feature = "__screenshot"))] assert!( diff --git a/crates/eframe/src/native/glow_integration.rs b/crates/eframe/src/native/glow_integration.rs index 599420e597d..c1a3f5eaf89 100644 --- a/crates/eframe/src/native/glow_integration.rs +++ b/crates/eframe/src/native/glow_integration.rs @@ -48,24 +48,24 @@ use super::{ // ---------------------------------------------------------------------------- // Types: -pub struct GlowWinitApp { +pub struct GlowWinitApp<'app> { repaint_proxy: Arc>>, app_name: String, native_options: NativeOptions, - running: Option, + running: Option>, // Note that since this `AppCreator` is FnOnce we are currently unable to support // re-initializing the `GlowWinitRunning` state on Android if the application // suspends and resumes. - app_creator: Option, + app_creator: Option>, } /// State that is initialized when the application is first starts running via /// a Resumed event. On Android this ensures that any graphics state is only /// initialized once the application has an associated `SurfaceView`. -struct GlowWinitRunning { +struct GlowWinitRunning<'app> { integration: EpiIntegration, - app: Box, + app: Box, // These needs to be shared with the immediate viewport renderer, hence the Rc/Arc/RefCells: glutin: Rc>, @@ -126,12 +126,12 @@ struct Viewport { // ---------------------------------------------------------------------------- -impl GlowWinitApp { +impl<'app> GlowWinitApp<'app> { pub fn new( event_loop: &EventLoop, app_name: &str, native_options: NativeOptions, - app_creator: AppCreator, + app_creator: AppCreator<'app>, ) -> Self { crate::profile_function!(); Self { @@ -195,7 +195,10 @@ impl GlowWinitApp { Ok((glutin_window_context, painter)) } - fn init_run_state(&mut self, event_loop: &ActiveEventLoop) -> Result<&mut GlowWinitRunning> { + fn init_run_state( + &mut self, + event_loop: &ActiveEventLoop, + ) -> Result<&mut GlowWinitRunning<'app>> { crate::profile_function!(); let storage = if let Some(file) = &self.native_options.persistence_path { @@ -292,7 +295,7 @@ impl GlowWinitApp { let app_creator = std::mem::take(&mut self.app_creator) .expect("Single-use AppCreator has unexpectedly already been taken"); - let app = { + let app: Box = { // Use latest raw_window_handle for eframe compatibility use raw_window_handle::{HasDisplayHandle as _, HasWindowHandle as _}; @@ -346,7 +349,7 @@ impl GlowWinitApp { } } -impl WinitApp for GlowWinitApp { +impl<'app> WinitApp for GlowWinitApp<'app> { fn frame_nr(&self, viewport_id: ViewportId) -> u64 { self.running .as_ref() @@ -483,7 +486,7 @@ impl WinitApp for GlowWinitApp { } } -impl GlowWinitRunning { +impl<'app> GlowWinitRunning<'app> { fn run_ui_and_paint( &mut self, event_loop: &ActiveEventLoop, diff --git a/crates/eframe/src/native/run.rs b/crates/eframe/src/native/run.rs index 84b2de60937..f38c1b8f27f 100644 --- a/crates/eframe/src/native/run.rs +++ b/crates/eframe/src/native/run.rs @@ -297,7 +297,7 @@ fn run_and_return(event_loop: &mut EventLoop, winit_app: impl WinitAp app.return_result } -fn run_and_exit(event_loop: EventLoop, winit_app: impl WinitApp + 'static) -> Result { +fn run_and_exit(event_loop: EventLoop, winit_app: impl WinitApp) -> Result { log::trace!("Entering the winit event loop (run_app)…"); // When to repaint what window @@ -314,7 +314,7 @@ fn run_and_exit(event_loop: EventLoop, winit_app: impl WinitApp + 'st pub fn run_glow( app_name: &str, mut native_options: epi::NativeOptions, - app_creator: epi::AppCreator, + app_creator: epi::AppCreator<'_>, ) -> Result { #![allow(clippy::needless_return_with_question_mark)] // False positive @@ -339,7 +339,7 @@ pub fn run_glow( pub fn run_wgpu( app_name: &str, mut native_options: epi::NativeOptions, - app_creator: epi::AppCreator, + app_creator: epi::AppCreator<'_>, ) -> Result { #![allow(clippy::needless_return_with_question_mark)] // False positive diff --git a/crates/eframe/src/native/wgpu_integration.rs b/crates/eframe/src/native/wgpu_integration.rs index 9cbd9675e9c..b38b78c9b4e 100644 --- a/crates/eframe/src/native/wgpu_integration.rs +++ b/crates/eframe/src/native/wgpu_integration.rs @@ -34,26 +34,26 @@ use super::{epi_integration, event_loop_context, winit_integration, winit_integr // ---------------------------------------------------------------------------- // Types: -pub struct WgpuWinitApp { +pub struct WgpuWinitApp<'app> { repaint_proxy: Arc>>, app_name: String, native_options: NativeOptions, /// Set at initialization, then taken and set to `None` in `init_run_state`. - app_creator: Option, + app_creator: Option>, /// Set when we are actually up and running. - running: Option, + running: Option>, } /// State that is initialized when the application is first starts running via /// a Resumed event. On Android this ensures that any graphics state is only /// initialized once the application has an associated `SurfaceView`. -struct WgpuWinitRunning { +struct WgpuWinitRunning<'app> { integration: EpiIntegration, /// The users application. - app: Box, + app: Box, /// Wrapped in an `Rc>` so it can be re-entrantly shared via a weak-pointer. shared: Rc>, @@ -95,12 +95,12 @@ pub struct Viewport { // ---------------------------------------------------------------------------- -impl WgpuWinitApp { +impl<'app> WgpuWinitApp<'app> { pub fn new( event_loop: &EventLoop, app_name: &str, native_options: NativeOptions, - app_creator: AppCreator, + app_creator: AppCreator<'app>, ) -> Self { crate::profile_function!(); @@ -143,7 +143,7 @@ impl WgpuWinitApp { } #[cfg(target_os = "android")] - fn recreate_window(&self, event_loop: &ActiveEventLoop, running: &WgpuWinitRunning) { + fn recreate_window(&self, event_loop: &ActiveEventLoop, running: &WgpuWinitRunning<'app>) { let SharedState { egui_ctx, viewports, @@ -180,7 +180,7 @@ impl WgpuWinitApp { storage: Option>, window: Window, builder: ViewportBuilder, - ) -> crate::Result<&mut WgpuWinitRunning> { + ) -> crate::Result<&mut WgpuWinitRunning<'app>> { crate::profile_function!(); #[allow(unsafe_code, unused_mut, unused_unsafe)] @@ -323,7 +323,7 @@ impl WgpuWinitApp { } } -impl WinitApp for WgpuWinitApp { +impl<'app> WinitApp for WgpuWinitApp<'app> { fn frame_nr(&self, viewport_id: ViewportId) -> u64 { self.running .as_ref() @@ -489,7 +489,7 @@ impl WinitApp for WgpuWinitApp { } } -impl WgpuWinitRunning { +impl<'app> WgpuWinitRunning<'app> { fn save_and_destroy(&mut self) { crate::profile_function!(); diff --git a/crates/eframe/src/web/app_runner.rs b/crates/eframe/src/web/app_runner.rs index 6ef30004482..b636f341989 100644 --- a/crates/eframe/src/web/app_runner.rs +++ b/crates/eframe/src/web/app_runner.rs @@ -33,7 +33,7 @@ impl AppRunner { pub async fn new( canvas: web_sys::HtmlCanvasElement, web_options: crate::WebOptions, - app_creator: epi::AppCreator, + app_creator: epi::AppCreator<'static>, text_agent: TextAgent, ) -> Result { let painter = super::ActiveWebPainter::new(canvas, &web_options).await?; diff --git a/crates/eframe/src/web/web_runner.rs b/crates/eframe/src/web/web_runner.rs index 86b61b2a9c4..a2793a13aeb 100644 --- a/crates/eframe/src/web/web_runner.rs +++ b/crates/eframe/src/web/web_runner.rs @@ -54,7 +54,7 @@ impl WebRunner { &self, canvas: web_sys::HtmlCanvasElement, web_options: crate::WebOptions, - app_creator: epi::AppCreator, + app_creator: epi::AppCreator<'static>, ) -> Result<(), JsValue> { self.destroy();