From 04b3921923df340c2ac5ad884c4b9165a78ada58 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Fri, 20 Aug 2021 18:59:32 +0200 Subject: [PATCH] egui_glium: run app code outside event loop to fix file dialogs (#631) Previously app code was run from within the event loop which lead to file dialogs (e.g. using nfd2) to hang (see https://github.com/rust-windowing/winit/issues/1779) Now egui_glium polls for events and then runs the app code. --- eframe/CHANGELOG.md | 1 + eframe/src/lib.rs | 2 +- egui_glium/CHANGELOG.md | 1 + egui_glium/src/backend.rs | 135 +++++++++++++++++++++++--------------- 4 files changed, 84 insertions(+), 55 deletions(-) diff --git a/eframe/CHANGELOG.md b/eframe/CHANGELOG.md index 2596eb3b..7b7071ae 100644 --- a/eframe/CHANGELOG.md +++ b/eframe/CHANGELOG.md @@ -4,6 +4,7 @@ All notable changes to the `eframe` crate. ## Unreleased * Improve http fetch API. +* `run_native` now returns when the app is closed. ## 0.13.1 - 2021-06-24 diff --git a/eframe/src/lib.rs b/eframe/src/lib.rs index ee94db2e..219d63e9 100644 --- a/eframe/src/lib.rs +++ b/eframe/src/lib.rs @@ -68,6 +68,6 @@ pub fn start_web(canvas_id: &str, app: Box) -> Result<(), wasm_bin /// Call from `fn main` like this: `eframe::run_native(Box::new(MyEguiApp::default()))` #[cfg(not(target_arch = "wasm32"))] -pub fn run_native(app: Box, native_options: epi::NativeOptions) -> ! { +pub fn run_native(app: Box, native_options: epi::NativeOptions) { egui_glium::run(app, native_options) } diff --git a/egui_glium/CHANGELOG.md b/egui_glium/CHANGELOG.md index 3949ee06..a5d27ca1 100644 --- a/egui_glium/CHANGELOG.md +++ b/egui_glium/CHANGELOG.md @@ -4,6 +4,7 @@ All notable changes to the `egui_glium` integration will be noted in this file. ## Unreleased +* Fix native file dialogs hanging (eg. when using [`nfd2`](https://github.com/EmbarkStudios/nfd2) * [Fix minimize on Windows](https://github.com/emilk/egui/issues/518) * Change `drag_and_drop_support` to `false` by default (Windows only). See . * Don't restore window position on Windows, because the position would sometimes be invalid. diff --git a/egui_glium/src/backend.rs b/egui_glium/src/backend.rs index 173edfe9..66a545ea 100644 --- a/egui_glium/src/backend.rs +++ b/egui_glium/src/backend.rs @@ -172,7 +172,7 @@ fn load_icon(icon_data: epi::IconData) -> Option { // ---------------------------------------------------------------------------- /// Run an egui app -pub fn run(mut app: Box, native_options: epi::NativeOptions) -> ! { +pub fn run(mut app: Box, native_options: epi::NativeOptions) { #[allow(unused_mut)] let mut storage = create_storage(app.name()); @@ -180,7 +180,7 @@ pub fn run(mut app: Box, native_options: epi::NativeOptions) -> ! let http = std::sync::Arc::new(crate::http::GliumHttp {}); let window_settings = deserialize_window_settings(&storage); - let event_loop = glutin::event_loop::EventLoop::with_user_event(); + let mut event_loop = glutin::event_loop::EventLoop::with_user_event(); let icon = native_options.icon_data.clone().and_then(load_icon); let display = create_display(&*app, &native_options, window_settings, icon, &event_loop); @@ -208,8 +208,6 @@ pub fn run(mut app: Box, native_options: epi::NativeOptions) -> ! let mut previous_frame_time = None; - let mut is_focused = true; - #[cfg(feature = "persistence")] let mut last_auto_save = Instant::now(); @@ -241,8 +239,67 @@ pub fn run(mut app: Box, native_options: epi::NativeOptions) -> ! // eprintln!("Warmed up in {} ms", warm_up_start.elapsed().as_millis()) } - event_loop.run(move |event, _, control_flow| { - let mut redraw = || { + let mut is_focused = true; + let mut running = true; + let mut repaint_asap = true; + + while running { + use glium::glutin::platform::run_return::EventLoopExtRunReturn as _; + event_loop.run_return(|event, _, control_flow| { + use glium::glutin::event_loop::ControlFlow; + + *control_flow = ControlFlow::Wait; + + match event { + // Platform-dependent event handlers to workaround a winit bug + // See: https://github.com/rust-windowing/winit/issues/987 + // See: https://github.com/rust-windowing/winit/issues/1619 + glutin::event::Event::RedrawEventsCleared if cfg!(windows) => { + *control_flow = ControlFlow::Exit; // Time to redraw + } + glutin::event::Event::RedrawRequested(_) if !cfg!(windows) => { + *control_flow = ControlFlow::Exit; // Time to redraw + } + glutin::event::Event::MainEventsCleared => { + if repaint_asap { + *control_flow = ControlFlow::Exit; // Time to redraw + } else { + // Winit uses up all the CPU of one core when returning ControlFlow::Wait. + // Sleeping here helps, but still uses 1-3% of CPU :( + if is_focused { + std::thread::sleep(std::time::Duration::from_millis(10)); + } else { + std::thread::sleep(std::time::Duration::from_millis(100)); + } + } + } + glutin::event::Event::WindowEvent { event, .. } => { + if egui.is_quit_event(&event) { + *control_flow = ControlFlow::Exit; + running = false; + } + + if let glutin::event::WindowEvent::Focused(new_focused) = event { + is_focused = new_focused; + } + + egui.on_event(&event); + + // TODO: ask egui if the events warrants a repaint instead of repainting on each event. + display.gl_window().window().request_redraw(); + } + glutin::event::Event::UserEvent(RequestRepaintEvent) => { + display.gl_window().window().request_redraw(); + *control_flow = ControlFlow::Exit; // Time to redraw + } + + _ => (), + } + }); + + repaint_asap = false; + + if running { if !is_focused { // On Mac, a minimized Window uses up all CPU: https://github.com/emilk/egui/issues/325 // We can't know if we are minimized: https://github.com/rust-windowing/winit/issues/208 @@ -299,13 +356,11 @@ pub fn run(mut app: Box, native_options: epi::NativeOptions) -> ! ); } - *control_flow = if quit { - glutin::event_loop::ControlFlow::Exit + if quit { + running = false; } else if needs_repaint { display.gl_window().window().request_redraw(); - glutin::event_loop::ControlFlow::Poll - } else { - glutin::event_loop::ControlFlow::Wait + repaint_asap = true; }; } @@ -324,48 +379,20 @@ pub fn run(mut app: Box, native_options: epi::NativeOptions) -> ! last_auto_save = now; } } - }; - - match event { - // Platform-dependent event handlers to workaround a winit bug - // See: https://github.com/rust-windowing/winit/issues/987 - // See: https://github.com/rust-windowing/winit/issues/1619 - glutin::event::Event::RedrawEventsCleared if cfg!(windows) => redraw(), - glutin::event::Event::RedrawRequested(_) if !cfg!(windows) => redraw(), - - glutin::event::Event::WindowEvent { event, .. } => { - if egui.is_quit_event(&event) { - *control_flow = glium::glutin::event_loop::ControlFlow::Exit; - } - - if let glutin::event::WindowEvent::Focused(new_focused) = event { - is_focused = new_focused; - } - - egui.on_event(&event); - - display.gl_window().window().request_redraw(); // TODO: ask egui if the events warrants a repaint instead - } - glutin::event::Event::LoopDestroyed => { - app.on_exit(); - #[cfg(feature = "persistence")] - if let Some(storage) = &mut storage { - epi::set_value( - storage.as_mut(), - WINDOW_KEY, - &WindowSettings::from_display(&display), - ); - epi::set_value(storage.as_mut(), EGUI_MEMORY_KEY, &*egui.ctx().memory()); - app.save(storage.as_mut()); - storage.flush(); - } - } - - glutin::event::Event::UserEvent(RequestRepaintEvent) => { - display.gl_window().window().request_redraw(); - } - - _ => (), } - }); + } + + app.on_exit(); + + #[cfg(feature = "persistence")] + if let Some(storage) = &mut storage { + epi::set_value( + storage.as_mut(), + WINDOW_KEY, + &WindowSettings::from_display(&display), + ); + epi::set_value(storage.as_mut(), EGUI_MEMORY_KEY, &*egui.ctx().memory()); + app.save(storage.as_mut()); + storage.flush(); + } }