Allow eframe apps to be detached #3340

rockisch · 2023-09-15T03:19:44Z

Closes [Eframe] Allow eframe to run on an existing event loop #2875.

One choice that I'm not really sure which I prefer is whether we should check for window.id() == window_id on WindowEvent events inside the on_event method or not. Since we're already exposing the window, the user could easily check that outside. However, when writing some example code, it ended up looking really bad and confusing. At the same time, people who will use this are probably people who have a good understanding of winit and GUI development in general, so maybe that's not an issue.

This gist shows how the 2 approaches would look like:
https://gist.github.com/rockisch/c672345661dd5411f5611b7ab14faea0

This initial version of the PR has the check inside, but I could remove it if wanted.

crates/eframe/src/native/run.rs

emilk · 2023-09-18T19:18:36Z

This looks very promising! Can you please add a working example too? 🙏

rockisch · 2023-09-19T11:51:35Z

Added a working example and fixed the CI issues (was on a Windows machine before, so I couldn't run the check.sh script)

rockisch · 2023-09-27T01:08:21Z

Btw, I went and changed one of my projects to this approach, and I noticed that although it does work, communication between the 'outer' code and the 'detached eframe app' is kinda clunky, since there's no way to access or modify the app data from the returned reference.

The way I'm doing this at the moment is by leaking a RefCell, more or less like this:

struct SharedState {
    flag: bool
}

struct MyApp {
    shared: &'static RefCell<SharedState>
}

let shared_state = Box::leak(Box::new(RefCell::new(SharedState { flag: true }))) as &_;
let detached_app = eframe::run_detached_native(
    ...,
    Box::new(|_cc| Box::new(MyApp { shared: shared_state }))
)
// 'shared_state' can be accessed and modified

How would you feel about exposing a get_app(&self) -> MyApp and get_app_mut(&mut self) -> MyApp on Detached? I took a look at the source code and I feel like I could add those with minimal changes.

CoryRobertson · 2023-11-21T00:49:36Z

not sure I follow entirely but would this pr allow you to open a completely detached eframe, meaning you would have two separate event loops?

Aaron1011 · 2024-01-15T03:04:55Z

What's the status of this PR? This would be very useful for Ruffle

samyak-jain · 2024-02-27T18:59:19Z

Btw, I went and changed one of my projects to this approach, and I noticed that although it does work, communication between the 'outer' code and the 'detached eframe app' is kinda clunky, since there's no way to access or modify the app data from the returned reference.

The way I'm doing this at the moment is by leaking a RefCell, more or less like this:
struct SharedState {
    flag: bool
}

struct MyApp {
    shared: &'static RefCell<SharedState>
}

let shared_state = Box::leak(Box::new(RefCell::new(SharedState { flag: true }))) as &_;
let detached_app = eframe::run_detached_native(
    ...,
    Box::new(|_cc| Box::new(MyApp { shared: shared_state }))
)
// 'shared_state' can be accessed and modified
How would you feel about exposing a get_app(&self) -> MyApp and get_app_mut(&mut self) -> MyApp on Detached? I took a look at the source code and I feel like I could add those with minimal changes.

Is the problem that we need to give away ownership of the MyApp instance? Shouldn't the shared state be accessible when you are implementing the update method on MyApp?

Btw, if there's anyway I can contribute, I would be happy to help.
Thanks for taking your time to implement this!

emilk

Needs a merging in of master and testing with multi-viewports, otherwise its looking good

emilk · 2024-03-25T11:08:09Z

examples/detached_app/README.md

@@ -0,0 +1,7 @@
+Example showing some UI controls like `Label`, `TextEdit`, `Slider`, `Button`.


This is not a good description

And the screenshot is not very useful either

emilk · 2024-03-25T11:09:31Z

examples/detached_app/src/main.rs

+    );
+    // Winit window managed by the application
+    let winit_window = winit::window::WindowBuilder::new()
+        .build(&event_loop)


Giving this a title of e.g. "winit window" would help explain to the user what is going on

emilk · 2024-03-25T11:10:03Z

examples/detached_app/src/main.rs

+            ui.heading("My egui Application");
+            ui.horizontal(|ui| {
+                let name_label = ui.label("Your name: ");
+                ui.text_edit_singleline(&mut self.name)
+                    .labelled_by(name_label.id);
+            });
+            ui.add(egui::Slider::new(&mut self.age, 0..=120).text("age"));
+            if ui.button("Click each year").clicked() {
+                self.age += 1;
+            }
+            ui.label(format!("Hello '{}', age {}", self.name, self.age));


Instead of showing the generic egui application, it would be better to show some text about what is going on in this example

emilk · 2024-03-25T11:13:05Z

crates/eframe/src/native/run.rs

+/// An eframe app detached from the event loop.
+///
+/// This is useful to run `eframe` apps while still having control over the event loop.
+/// For example, when you need to have a window managed by `egui` and another managed


Suggested change

/// For example, when you need to have a window managed by `egui` and another managed

/// For example, when you need to have a window managed by `eframe` and another managed

emilk · 2024-03-25T11:13:18Z

crates/eframe/src/lib.rs

+/// Execute an app in a detached state.
+///
+/// This allows you to control the event loop itself, which is necessary in a few
+/// occasions, like when you need a screen not managed by `egui`.


Suggested change

/// occasions, like when you need a screen not managed by `egui`.

/// occasions, like when you need a screen not managed by `eframe`.

emilk · 2024-03-25T11:15:27Z

crates/eframe/src/native/run.rs

+                // Ignore window events for other windows
+                if let Some(window) = self.winit_app.window() {


I think you need to merge in master and handle the case of multi-viewports

Allow eframe apps to be detached

b8a1ac3

rockisch commented Sep 15, 2023

View reviewed changes

crates/eframe/src/native/run.rs Outdated Show resolved Hide resolved

rockisch added 2 commits September 19, 2023 08:49

Add detached app example

acd30d8

Fix detached app CI issues

2385348

rockisch force-pushed the run-detached branch from 31caf84 to 2385348 Compare September 19, 2023 11:50

rockisch requested a review from emilk September 19, 2023 11:51

rockisch marked this pull request as ready for review September 19, 2023 11:51

Fix 'detached_app' example README.md

d92e226

emilk requested changes Mar 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow eframe apps to be detached #3340

Allow eframe apps to be detached #3340

rockisch commented Sep 15, 2023 •

edited

Loading

emilk commented Sep 18, 2023

rockisch commented Sep 19, 2023

rockisch commented Sep 27, 2023

CoryRobertson commented Nov 21, 2023

Aaron1011 commented Jan 15, 2024

samyak-jain commented Feb 27, 2024

emilk left a comment

emilk Mar 25, 2024

emilk Mar 25, 2024

emilk Mar 25, 2024

emilk Mar 25, 2024

emilk Mar 25, 2024

emilk Mar 25, 2024

emilk Mar 25, 2024

		@@ -0,0 +1,7 @@
		Example showing some UI controls like `Label`, `TextEdit`, `Slider`, `Button`.

	/// For example, when you need to have a window managed by `egui` and another managed
	/// For example, when you need to have a window managed by `eframe` and another managed

	/// occasions, like when you need a screen not managed by `egui`.
	/// occasions, like when you need a screen not managed by `eframe`.

		// Ignore window events for other windows
		if let Some(window) = self.winit_app.window() {

Allow eframe apps to be detached #3340

Are you sure you want to change the base?

Allow eframe apps to be detached #3340

Conversation

rockisch commented Sep 15, 2023 • edited Loading

emilk commented Sep 18, 2023

rockisch commented Sep 19, 2023

rockisch commented Sep 27, 2023

CoryRobertson commented Nov 21, 2023

Aaron1011 commented Jan 15, 2024

samyak-jain commented Feb 27, 2024

emilk left a comment

Choose a reason for hiding this comment

emilk Mar 25, 2024

Choose a reason for hiding this comment

emilk Mar 25, 2024

Choose a reason for hiding this comment

emilk Mar 25, 2024

Choose a reason for hiding this comment

emilk Mar 25, 2024

Choose a reason for hiding this comment

emilk Mar 25, 2024

Choose a reason for hiding this comment

emilk Mar 25, 2024

Choose a reason for hiding this comment

emilk Mar 25, 2024

Choose a reason for hiding this comment

rockisch commented Sep 15, 2023 •

edited

Loading