Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow non-static eframe::App lifetime #5060

Merged
merged 4 commits into from
Sep 3, 2024
Merged

Allow non-static eframe::App lifetime #5060

merged 4 commits into from
Sep 3, 2024

Conversation

timstr
Copy link
Contributor

@timstr timstr commented Sep 2, 2024

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 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:

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:

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

@emilk emilk marked this pull request as draft September 2, 2024 16:20
@timstr timstr changed the title Draft: Allow non-static eframe::App's with lifetimes Allow non-static eframe::App's with lifetimes Sep 2, 2024
@timstr timstr marked this pull request as ready for review September 2, 2024 17:23
@timstr
Copy link
Contributor Author

timstr commented Sep 2, 2024

I've added lifetime parameters to the AppCreator as well as the native backends. I'm not familiar with the web backend and whether it makes sense for them to store lifetimes, so I've required AppCreator<'static> for the web backend without loss of existing functionality.

@emilk emilk added the eframe Relates to epi and eframe label Sep 2, 2024
@emilk emilk changed the title Allow non-static eframe::App's with lifetimes Allow non-static eframe::App lifetime Sep 2, 2024
crates/eframe/src/native/glow_integration.rs Outdated Show resolved Hide resolved
Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - this is great to have!

@emilk emilk merged commit b943554 into emilk:master Sep 3, 2024
20 checks passed
hacknus pushed a commit to hacknus/egui that referenced this pull request Oct 30, 2024
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 emilk#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 emilk#2152
* [x] I have followed the instructions in the PR template
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
eframe Relates to epi and eframe
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow the use of non-static App
2 participants