From 4bee4f8cf60fe37d4389a83e897fdb5e2f1c100b Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 13 Nov 2023 15:06:13 +0100 Subject: [PATCH] Refactor how `EpiIntegration::update` is called --- crates/eframe/src/epi/mod.rs | 3 + crates/eframe/src/native/epi_integration.rs | 50 ++++--- crates/eframe/src/native/run.rs | 150 +++++++++++--------- crates/egui_glow/src/painter.rs | 4 + 4 files changed, 113 insertions(+), 94 deletions(-) diff --git a/crates/eframe/src/epi/mod.rs b/crates/eframe/src/epi/mod.rs index 1309d78ec..634bd411b 100644 --- a/crates/eframe/src/epi/mod.rs +++ b/crates/eframe/src/epi/mod.rs @@ -120,6 +120,9 @@ pub trait App { /// The [`egui::Context`] can be cloned and saved if you like. /// /// To force a repaint, call [`egui::Context::request_repaint`] at any time (e.g. from another thread). + /// + /// This is called for the root viewport ([`egui::ViewportId::ROOT`]). + /// Use [`Context::show_viewport`] to spawn additional viewports (windows). fn update(&mut self, ctx: &egui::Context, frame: &mut Frame); /// Get a handle to the app. diff --git a/crates/eframe/src/native/epi_integration.rs b/crates/eframe/src/native/epi_integration.rs index b08052c5b..8d66a500c 100644 --- a/crates/eframe/src/native/epi_integration.rs +++ b/crates/eframe/src/native/epi_integration.rs @@ -5,11 +5,9 @@ use winit::event_loop::EventLoopWindowTarget; use raw_window_handle::{HasRawDisplayHandle as _, HasRawWindowHandle as _}; use egui::{NumExt as _, ViewportBuilder, ViewportId, ViewportIdPair, ViewportUiCallback}; -#[cfg(feature = "accesskit")] -use egui_winit::accesskit_winit; use egui_winit::{native_pixels_per_point, EventResponse, WindowSettings}; -use crate::{backend::AppOutput, epi, Theme, WindowInfo}; +use crate::{epi, Theme, WindowInfo}; #[derive(Default)] pub struct WindowState { @@ -317,8 +315,9 @@ pub fn create_storage(_app_name: &str) -> Option> { /// Everything needed to make a winit-based integration for [`epi`]. pub struct EpiIntegration { pub frame: epi::Frame, - last_auto_save: std::time::Instant, + last_auto_save: Instant, pub beginning: Instant, + pub frame_start: Instant, pub egui_ctx: egui::Context, pending_full_output: egui::FullOutput, @@ -386,7 +385,7 @@ impl EpiIntegration { Self { frame, - last_auto_save: std::time::Instant::now(), + last_auto_save: Instant::now(), egui_ctx, pending_full_output: Default::default(), close: false, @@ -397,11 +396,12 @@ impl EpiIntegration { persist_window: native_options.persist_window, app_icon_setter, beginning: Instant::now(), + frame_start: Instant::now(), } } #[cfg(feature = "accesskit")] - pub fn init_accesskit + Send>( + pub fn init_accesskit + Send>( &mut self, egui_winit: &mut egui_winit::State, window: &winit::window::Window, @@ -432,8 +432,8 @@ impl EpiIntegration { let raw_input = egui_winit.take_egui_input(window, ViewportIdPair::ROOT); self.pre_update(window); - let (full_output, app_output) = self.update(app, None, raw_input); - self.handle_app_output(window, app_output); + let full_output = self.update(app, None, raw_input); + self.post_update(app, window); self.pending_full_output.append(full_output); // Handle it next frame self.egui_ctx.memory_mut(|mem| *mem = saved_memory); // We don't want to remember that windows were huge. self.egui_ctx.clear_animations(); @@ -485,29 +485,29 @@ impl EpiIntegration { } pub fn pre_update(&mut self, window: &winit::window::Window) { + self.frame_start = Instant::now(); + + self.app_icon_setter.update(); + self.frame.info.window_info = read_window_info(window, self.egui_ctx.pixels_per_point(), &self.window_state); } - /// If `viewport_ui_cb` is None, we are in the root viewport - /// and will cal [`App::update`]. + /// Run user code - this can create immediate viewports, so hold no locks over this! + /// + /// If `viewport_ui_cb` is None, we are in the root viewport and will call [`App::update`]. pub fn update( &mut self, app: &mut dyn epi::App, viewport_ui_cb: Option<&ViewportUiCallback>, mut raw_input: egui::RawInput, - ) -> (egui::FullOutput, AppOutput) { - let frame_start = std::time::Instant::now(); - - self.app_icon_setter.update(); - + ) -> egui::FullOutput { raw_input.time = Some(self.beginning.elapsed().as_secs_f64()); - // Run user code - this can create immediate viewports, so hold no locks over this! let full_output = self.egui_ctx.run(raw_input, |egui_ctx| { if let Some(viewport_ui_cb) = viewport_ui_cb { // Child viewport - crate::profile_scope!("callback"); + crate::profile_scope!("viewport_callback"); viewport_ui_cb(egui_ctx); } else { // Root viewport @@ -517,8 +517,10 @@ impl EpiIntegration { }); self.pending_full_output.append(full_output); - let full_output = std::mem::take(&mut self.pending_full_output); + std::mem::take(&mut self.pending_full_output) + } + pub fn post_update(&mut self, app: &mut dyn epi::App, window: &winit::window::Window) { let app_output = { let mut app_output = self.frame.take_app_output(); app_output.drag_window &= self.can_drag_window; // Necessary on Windows; see https://github.com/emilk/egui/pull/1108 @@ -535,19 +537,15 @@ impl EpiIntegration { app_output }; - let frame_time = frame_start.elapsed().as_secs_f64() as f32; - self.frame.info.cpu_usage = Some(frame_time); - - (full_output, app_output) - } - - pub fn handle_app_output(&mut self, window: &winit::window::Window, app_output: AppOutput) { handle_app_output( window, self.egui_ctx.pixels_per_point(), app_output, &mut self.window_state, ); + + let frame_time = self.frame_start.elapsed().as_secs_f64() as f32; + self.frame.info.cpu_usage = Some(frame_time); } pub fn post_rendering(&mut self, app: &mut dyn epi::App, window: &winit::window::Window) { @@ -581,7 +579,7 @@ impl EpiIntegration { app: &mut dyn epi::App, window: Option<&winit::window::Window>, ) { - let now = std::time::Instant::now(); + let now = Instant::now(); if now - self.last_auto_save > app.auto_save_interval() { self.save(app, window); self.last_auto_save = now; diff --git a/crates/eframe/src/native/run.rs b/crates/eframe/src/native/run.rs index 8aed48142..289c0a895 100644 --- a/crates/eframe/src/native/run.rs +++ b/crates/eframe/src/native/run.rs @@ -1481,7 +1481,7 @@ mod glow_integration { let screen_size_in_pixels: [u32; 2] = window.inner_size().into(); - { + let (raw_input, viewport_ui_cb) = { let viewport = &mut *viewport.borrow_mut(); let egui_winit = viewport.egui_winit.as_mut().unwrap(); @@ -1489,27 +1489,30 @@ mod glow_integration { integration.pre_update(&window); - // ------------------------------------------------------------ - // The update function, which could call immediate viewports, - // so make sure we don't hold any locks here required by the immediate viewports rendeer. + (raw_input, viewport.viewport_ui_cb.clone()) + }; - let (full_output, app_output) = integration.update( - app.as_mut(), - viewport.viewport_ui_cb.as_deref(), - raw_input, - ); - integration.handle_app_output(&window, app_output); + // ------------------------------------------------------------ + // The update function, which could call immediate viewports, + // so make sure we don't hold any locks here required by the immediate viewports rendeer. - egui::FullOutput { - platform_output, - textures_delta, - shapes, - viewports, - viewport_commands, - } = full_output; + let full_output = + integration.update(app.as_mut(), viewport_ui_cb.as_deref(), raw_input); - // ------------------------------------------------------------ + egui::FullOutput { + platform_output, + textures_delta, + shapes, + viewports, + viewport_commands, + } = full_output; + // ------------------------------------------------------------ + + { + let viewport = &mut *viewport.borrow_mut(); + let egui_winit = viewport.egui_winit.as_mut().unwrap(); + integration.post_update(app.as_mut(), &window); integration.handle_platform_output( &window, viewport_id, @@ -1575,8 +1578,8 @@ mod glow_integration { integration.post_present(viewport.borrow().window.as_ref().unwrap()); - #[cfg(feature = "__screenshot")] // give it time to settle: + #[cfg(feature = "__screenshot")] if integration.egui_ctx.frame_nr() == 2 { if let Ok(path) = std::env::var("EFRAME_SCREENSHOT_TO") { assert!( @@ -2381,15 +2384,7 @@ mod wgpu_integration { shared, } = running; - let egui::FullOutput { - platform_output, - textures_delta, - shapes, - viewports: mut out_viewports, - viewport_commands, - }; - - { + let (viewport_ui_cb, raw_input) = { let mut shared_lock = shared.borrow_mut(); let Some(viewport_id) = shared_lock.viewport_maps.get(&window_id).copied() else { @@ -2436,62 +2431,81 @@ mod wgpu_integration { integration.pre_update(&window); - // ------------------------------------------------------------ + (viewport_ui_cb.clone(), raw_input) + }; - // Runs the update, which could call immediate viewports, - // so make sure we hold no locks here! - let (full_output, app_output) = - integration.update(app.as_mut(), viewport_ui_cb.as_deref(), raw_input); + // ------------------------------------------------------------ - // ------------------------------------------------------------ + // Runs the update, which could call immediate viewports, + // so make sure we hold no locks here! + let full_output = + integration.update(app.as_mut(), viewport_ui_cb.as_deref(), raw_input); - integration.handle_app_output(&window, app_output); + // ------------------------------------------------------------ - FullOutput { - platform_output, - textures_delta, - shapes, - viewports: out_viewports, - viewport_commands, - } = full_output; + let mut shared_lock = shared.borrow_mut(); - integration.handle_platform_output( - &window, - viewport_id, - platform_output, - egui_winit.borrow_mut().as_mut().unwrap(), - ); + let Some(viewport_id) = shared_lock.viewport_maps.get(&window_id).copied() else { + return EventResult::Wait; + }; - let clipped_primitives = integration.egui_ctx.tessellate(shapes); + let Some(viewport) = shared_lock.viewports.get(&viewport_id).cloned() else { + return EventResult::Wait; + }; - let screenshot_requested = &mut integration.frame.output.screenshot_requested; + let Viewport { + window, egui_winit, .. + } = viewport; - let pixels_per_point = integration - .egui_ctx - .input_for(viewport_id, |i| i.pixels_per_point()); + let Some(window) = window else { + return EventResult::Wait; + }; - let screenshot = shared.borrow_mut().painter.paint_and_update_textures( - viewport_id, - pixels_per_point, - app.clear_color(&integration.egui_ctx.style().visuals), - &clipped_primitives, - &textures_delta, - *screenshot_requested, - ); - *screenshot_requested = false; - integration.frame.screenshot.set(screenshot); + integration.post_update(app.as_mut(), &window); - integration.post_rendering(app.as_mut(), &window); - integration.post_present(&window); - } + let FullOutput { + platform_output, + textures_delta, + shapes, + viewports: mut out_viewports, + viewport_commands, + } = full_output; + + integration.handle_platform_output( + &window, + viewport_id, + platform_output, + egui_winit.borrow_mut().as_mut().unwrap(), + ); + + let clipped_primitives = integration.egui_ctx.tessellate(shapes); + + let screenshot_requested = &mut integration.frame.output.screenshot_requested; + + let pixels_per_point = integration + .egui_ctx + .input_for(viewport_id, |i| i.pixels_per_point()); + + let screenshot = shared.borrow_mut().painter.paint_and_update_textures( + viewport_id, + pixels_per_point, + app.clear_color(&integration.egui_ctx.style().visuals), + &clipped_primitives, + &textures_delta, + *screenshot_requested, + ); + *screenshot_requested = false; + integration.frame.screenshot.set(screenshot); + + integration.post_rendering(app.as_mut(), &window); + integration.post_present(&window); - let mut shared = shared.borrow_mut(); let SharedState { viewports, builders, painter, viewport_maps, - } = &mut *shared; + } = &mut *shared_lock; let mut active_viewports_ids = ViewportIdSet::default(); active_viewports_ids.insert(ViewportId::ROOT); diff --git a/crates/egui_glow/src/painter.rs b/crates/egui_glow/src/painter.rs index ef0a3d631..1845b5550 100644 --- a/crates/egui_glow/src/painter.rs +++ b/crates/egui_glow/src/painter.rs @@ -622,6 +622,8 @@ impl Painter { } pub fn read_screen_rgba(&self, [w, h]: [u32; 2]) -> egui::ColorImage { + crate::profile_function!(); + let mut pixels = vec![0_u8; (w * h * 4) as usize]; unsafe { self.gl.read_pixels( @@ -645,6 +647,8 @@ impl Painter { } pub fn read_screen_rgb(&self, [w, h]: [u32; 2]) -> Vec { + crate::profile_function!(); + let mut pixels = vec![0_u8; (w * h * 3) as usize]; unsafe { self.gl.read_pixels(