Skip to content

Commit

Permalink
Allow non-static eframe::App lifetime (#5060)
Browse files Browse the repository at this point in the history
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<dyn for<'a, 'b> FnOnce(&'a CreationContext<'b>) -> Result<Box<dyn App>, Box<dyn std::error::Error + Send + Sync>>>` actually means `Box<(dyn for<'a, 'b> FnOnce(&'a CreationContext<'b>) -> Result<Box<dyn App>, Box<dyn std::error::Error + Send + Sync>> + '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 #2152

<!--
Please read the "Making a PR" section of
[`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/master/CONTRIBUTING.md)
before opening a Pull Request!

* Keep your PR:s small and focused.
* The PR title is what ends up in the changelog, so make it descriptive!
* If applicable, add a screenshot or gif.
* If it is a non-trivial addition, consider adding a demo for it to
`egui_demo_lib`, or a new example.
* Do NOT open PR:s from your `master` branch, as that makes it hard for
maintainers to test and add commits to your PR.
* Remember to run `cargo fmt` and `cargo clippy`.
* Open the PR as a draft until you have self-reviewed it and run
`./scripts/check.sh`.
* When you have addressed a PR comment, mark it as resolved.

Please be patient! I will review your PR, but my time is limited!
-->

* Closes #2152
* [x] I have followed the instructions in the PR template
  • Loading branch information
timstr authored Sep 3, 2024
1 parent 7bac528 commit b943554
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 29 deletions.
3 changes: 2 additions & 1 deletion crates/eframe/src/epi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ type DynError = Box<dyn std::error::Error + Send + Sync>;
/// 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<dyn FnOnce(&CreationContext<'_>) -> Result<Box<dyn App>, DynError>>;
pub type AppCreator<'app> =
Box<dyn 'app + FnOnce(&CreationContext<'_>) -> Result<Box<dyn 'app + App>, DynError>>;

/// Data that is passed to [`AppCreator`] that can be used to setup and initialize your app.
pub struct CreationContext<'s> {
Expand Down
2 changes: 1 addition & 1 deletion crates/eframe/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
25 changes: 14 additions & 11 deletions crates/eframe/src/native/glow_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,24 +48,24 @@ use super::{
// ----------------------------------------------------------------------------
// Types:

pub struct GlowWinitApp {
pub struct GlowWinitApp<'app> {
repaint_proxy: Arc<egui::mutex::Mutex<EventLoopProxy<UserEvent>>>,
app_name: String,
native_options: NativeOptions,
running: Option<GlowWinitRunning>,
running: Option<GlowWinitRunning<'app>>,

// 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<AppCreator>,
app_creator: Option<AppCreator<'app>>,
}

/// 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<dyn App>,
app: Box<dyn 'app + App>,

// These needs to be shared with the immediate viewport renderer, hence the Rc/Arc/RefCells:
glutin: Rc<RefCell<GlutinWindowContext>>,
Expand Down Expand Up @@ -126,12 +126,12 @@ struct Viewport {

// ----------------------------------------------------------------------------

impl GlowWinitApp {
impl<'app> GlowWinitApp<'app> {
pub fn new(
event_loop: &EventLoop<UserEvent>,
app_name: &str,
native_options: NativeOptions,
app_creator: AppCreator,
app_creator: AppCreator<'app>,
) -> Self {
crate::profile_function!();
Self {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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<dyn 'app + App> = {
// Use latest raw_window_handle for eframe compatibility
use raw_window_handle::{HasDisplayHandle as _, HasWindowHandle as _};

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -483,7 +486,7 @@ impl WinitApp for GlowWinitApp {
}
}

impl GlowWinitRunning {
impl<'app> GlowWinitRunning<'app> {
fn run_ui_and_paint(
&mut self,
event_loop: &ActiveEventLoop,
Expand Down
6 changes: 3 additions & 3 deletions crates/eframe/src/native/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ fn run_and_return(event_loop: &mut EventLoop<UserEvent>, winit_app: impl WinitAp
app.return_result
}

fn run_and_exit(event_loop: EventLoop<UserEvent>, winit_app: impl WinitApp + 'static) -> Result {
fn run_and_exit(event_loop: EventLoop<UserEvent>, winit_app: impl WinitApp) -> Result {
log::trace!("Entering the winit event loop (run_app)…");

// When to repaint what window
Expand All @@ -314,7 +314,7 @@ fn run_and_exit(event_loop: EventLoop<UserEvent>, 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

Expand All @@ -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

Expand Down
22 changes: 11 additions & 11 deletions crates/eframe/src/native/wgpu_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Mutex<EventLoopProxy<UserEvent>>>,
app_name: String,
native_options: NativeOptions,

/// Set at initialization, then taken and set to `None` in `init_run_state`.
app_creator: Option<AppCreator>,
app_creator: Option<AppCreator<'app>>,

/// Set when we are actually up and running.
running: Option<WgpuWinitRunning>,
running: Option<WgpuWinitRunning<'app>>,
}

/// 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<dyn App>,
app: Box<dyn 'app + App>,

/// Wrapped in an `Rc<RefCell<…>>` so it can be re-entrantly shared via a weak-pointer.
shared: Rc<RefCell<SharedState>>,
Expand Down Expand Up @@ -95,12 +95,12 @@ pub struct Viewport {

// ----------------------------------------------------------------------------

impl WgpuWinitApp {
impl<'app> WgpuWinitApp<'app> {
pub fn new(
event_loop: &EventLoop<UserEvent>,
app_name: &str,
native_options: NativeOptions,
app_creator: AppCreator,
app_creator: AppCreator<'app>,
) -> Self {
crate::profile_function!();

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -180,7 +180,7 @@ impl WgpuWinitApp {
storage: Option<Box<dyn Storage>>,
window: Window,
builder: ViewportBuilder,
) -> crate::Result<&mut WgpuWinitRunning> {
) -> crate::Result<&mut WgpuWinitRunning<'app>> {
crate::profile_function!();

#[allow(unsafe_code, unused_mut, unused_unsafe)]
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -489,7 +489,7 @@ impl WinitApp for WgpuWinitApp {
}
}

impl WgpuWinitRunning {
impl<'app> WgpuWinitRunning<'app> {
fn save_and_destroy(&mut self) {
crate::profile_function!();

Expand Down
2 changes: 1 addition & 1 deletion crates/eframe/src/web/app_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Self, String> {
let painter = super::ActiveWebPainter::new(canvas, &web_options).await?;
Expand Down
2 changes: 1 addition & 1 deletion crates/eframe/src/web/web_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down

0 comments on commit b943554

Please sign in to comment.