From 256ad734b4107da0d4156cd8a902e21c922344cb Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Fri, 3 Nov 2023 13:36:12 +0100 Subject: [PATCH] More cleanup --- crates/eframe/src/native/run.rs | 54 +++++++++-------- crates/egui/src/context.rs | 100 +++++++++++++++----------------- crates/egui/src/viewport.rs | 42 ++++++++------ examples/viewports/src/main.rs | 2 +- 4 files changed, 101 insertions(+), 97 deletions(-) diff --git a/crates/eframe/src/native/run.rs b/crates/eframe/src/native/run.rs index ed0961f18..1f0091ce9 100644 --- a/crates/eframe/src/native/run.rs +++ b/crates/eframe/src/native/run.rs @@ -836,9 +836,9 @@ mod glow_integration { let width = std::num::NonZeroU32::new(physical_size.width.at_least(1)).unwrap(); let height = std::num::NonZeroU32::new(physical_size.height.at_least(1)).unwrap(); - if let Some(window) = self.viewports.get(&viewport_id) { - let window = window.borrow(); - if let Some(gl_surface) = &window.gl_surface { + if let Some(viewport) = self.viewports.get(&viewport_id) { + let viewport = viewport.borrow(); + if let Some(gl_surface) = &viewport.gl_surface { self.current_gl_context = Some( self.current_gl_context .take() @@ -1128,7 +1128,7 @@ mod glow_integration { ) { crate::profile_function!(); - let has_window = glutin.borrow().viewports.get(&pair).is_some(); + let has_window = glutin.borrow().viewports.get(&pair.this).is_some(); // This will create a new native window if is needed if !has_window { @@ -1136,7 +1136,8 @@ mod glow_integration { // Inherit parent icon if none { - if viewport_builder.icon.is_none() && glutin.builders.get(&pair).is_none() { + if viewport_builder.icon.is_none() && glutin.builders.get(&pair.this).is_none() + { viewport_builder.icon = glutin .builders .get(&pair.parent) @@ -1158,7 +1159,7 @@ mod glow_integration { glutin.builders.entry(pair.this).or_insert(viewport_builder); } - let win = glutin.viewports[&pair].clone(); + let win = glutin.viewports[&pair.this].clone(); #[allow(unsafe_code)] let event_loop = unsafe { @@ -1173,7 +1174,7 @@ mod glow_integration { // Rendering the sync viewport - let window = glutin.borrow().viewports.get(&pair).cloned(); + let window = glutin.borrow().viewports.get(&pair.this).cloned(); let Some(window) = window else { return }; let window = &mut *window.borrow_mut(); @@ -1214,7 +1215,7 @@ mod glow_integration { .unwrap() .is_current(glutin.current_gl_context.as_ref().unwrap()) { - let builder = &&glutin.builders[&window.pair]; + let builder = &&glutin.builders[&window.pair.this]; log::error!("egui::create_viewport_sync with title: `{}` is not created in main thread, try to use wgpu!", builder.title); } @@ -1260,15 +1261,15 @@ mod glow_integration { let last_builder = glutin.builders.entry(*id).or_insert(builder.clone()); let (commands, recreate) = changes_between_builders(builder, last_builder); drop(glutin); - if let Some(window) = glutin_ctx.borrow().viewports.get(id) { - let mut window = window.borrow_mut(); + if let Some(viewport) = glutin_ctx.borrow().viewports.get(id) { + let mut viewport = viewport.borrow_mut(); if recreate { - window.window = None; - window.gl_surface = None; - window.viewport_ui_cb = viewport_ui_cb.clone(); - window.pair.parent = *id; + viewport.window = None; + viewport.gl_surface = None; + viewport.viewport_ui_cb = viewport_ui_cb.clone(); + viewport.pair.parent = *id; } - if let Some(w) = window.window.clone() { + if let Some(w) = viewport.window.clone() { process_viewport_commands(commands, *id, None, &w.borrow()); } active_viewports_ids.push(*id); @@ -1285,14 +1286,13 @@ mod glow_integration { viewport_ui_cb, } in viewports { - let default_icon = glutin_ctx - .borrow() - .builders - .get(&pair.parent) - .and_then(|b| b.icon.clone()); - if builder.icon.is_none() { - builder.icon = default_icon; + // Inherit from parent as fallback. + builder.icon = glutin_ctx + .borrow() + .builders + .get(&pair.parent) + .and_then(|b| b.icon.clone()); } { let mut glutin = glutin_ctx.borrow_mut(); @@ -1887,8 +1887,12 @@ mod wgpu_integration { #[derive(Clone)] pub struct Viewport { window: Option>>, + state: Rc>>, + + /// `None` for sync viewports. viewport_ui_cb: Option>>, + parent_id: ViewportId, } @@ -2237,11 +2241,11 @@ mod wgpu_integration { crate::profile_function!(); // Creating a new native window if is needed - if viewports.borrow().get(&pair).is_none() { + if viewports.borrow().get(&pair.this).is_none() { let mut builders = builders.borrow_mut(); { - if viewport_builder.icon.is_none() && builders.get(&pair).is_none() { + if viewport_builder.icon.is_none() && builders.get(&pair.this).is_none() { viewport_builder.icon = builders.get(&pair.parent).and_then(|b| b.icon.clone()); } @@ -2279,7 +2283,7 @@ mod wgpu_integration { } // Render sync viewport: - let viewport = viewports.borrow().get(&pair).cloned(); + let viewport = viewports.borrow().get(&pair.this).cloned(); let Some(viewport) = viewport else { return }; let Some(winit_state) = &mut *viewport.state.borrow_mut() else { return; diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index e54e6804d..d7dfdea7c 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -27,7 +27,7 @@ pub struct RequestRepaintInfo { /// triggered the painting of the next frame. pub current_frame_nr: u64, - /// This is used to specify what viewport that should be redraw + /// This is used to specify what viewport that should repaint. pub viewport_id: ViewportId, } @@ -195,21 +195,22 @@ struct ContextImpl { impl ContextImpl { fn begin_frame_mut(&mut self, mut new_raw_input: RawInput, pair: ViewportIdPair) { - // This is used to pause the last frame - if !self.viewport_stack.is_empty() { - let viewport_id = self.viewport_id(); + if let Some(pair) = self.viewport_stack.last().copied() { + let previous_viewport_id = pair.this; - self.memory.pause_frame(viewport_id); + // Pause the active viewport + self.memory.pause_frame(previous_viewport_id); self.layer_rects_this_viewports.insert( - viewport_id, + previous_viewport_id, std::mem::take(&mut self.layer_rects_this_frame), ); self.layer_rects_prev_viewports.insert( - viewport_id, + previous_viewport_id, std::mem::take(&mut self.layer_rects_prev_frame), ); } + let viewport_id = pair.this; self.viewport_stack.push(pair); self.output.entry(self.viewport_id()).or_default(); self.repaint.start_frame(self.viewport_id()); @@ -217,13 +218,13 @@ impl ContextImpl { if let Some(new_pixels_per_point) = self.memory.override_pixels_per_point { if self .input - .get(&pair) + .get(&viewport_id) .map(|input| input.pixels_per_point) .map_or(true, |pixels| pixels != new_pixels_per_point) { new_raw_input.pixels_per_point = Some(new_pixels_per_point); - let input = self.input.entry(pair.this).or_default(); + let input = self.input.entry(viewport_id).or_default(); // This is a bit hacky, but is required to avoid jitter: let ratio = input.pixels_per_point / new_pixels_per_point; let mut rect = input.screen_rect; @@ -235,30 +236,34 @@ impl ContextImpl { self.layer_rects_prev_frame = self .layer_rects_prev_viewports - .remove(&pair) + .remove(&viewport_id) .unwrap_or_default(); self.memory.begin_frame( - self.input.get(&pair).unwrap_or(&Default::default()), + self.input.get(&viewport_id).unwrap_or(&Default::default()), &new_raw_input, - pair.this, + viewport_id, ); - let input = self.input.remove(&pair).unwrap_or_default().begin_frame( - new_raw_input, - self.repaint.requested_repaint_last_frame(&pair), - ); - self.input.insert(pair.this, input); + let input = self + .input + .remove(&viewport_id) + .unwrap_or_default() + .begin_frame( + new_raw_input, + self.repaint.requested_repaint_last_frame(&viewport_id), + ); + self.input.insert(viewport_id, input); self.frame_state - .entry(pair.this) + .entry(viewport_id) .or_default() - .begin_frame(&self.input[&pair]); + .begin_frame(&self.input[&viewport_id]); self.update_fonts_mut(); // Ensure we register the background area so panels and background ui can catch clicks: - let input = &self.input[&pair]; + let input = &self.input[&viewport_id]; let screen_rect = input.screen_rect(); self.memory.areas.set_state( LayerId::background(), @@ -1486,18 +1491,7 @@ impl Context { ctx.viewport_id(), std::mem::take(&mut ctx.layer_rects_this_frame), ); - ctx.viewports - .iter() - .map( - |( - _, - Viewport { - pair: ViewportIdPair { this, .. }, - .. - }, - )| *this, - ) - .collect() + ctx.viewports.values().map(|vp| vp.pair.this).collect() }); viewports.push(ViewportId::MAIN); @@ -1564,12 +1558,8 @@ impl Context { // If there are no viewport that contains the current viewport that viewport needs to be destroyed! let available_viewports = self.read(|ctx| { let mut available_viewports = vec![ViewportId::MAIN]; - for Viewport { - pair: ViewportIdPair { this, .. }, - .. - } in ctx.viewports.values() - { - available_viewports.push(*this); + for vp in ctx.viewports.values() { + available_viewports.push(vp.pair.this); } available_viewports }); @@ -1609,17 +1599,8 @@ impl Context { ctx.viewport_stack.is_empty() }); - if !is_last { - let viewport_id = self.viewport_id(); - self.write(|ctx| { - ctx.layer_rects_prev_frame = - ctx.layer_rects_prev_viewports.remove(&viewport_id).unwrap(); - ctx.layer_rects_this_frame = - ctx.layer_rects_this_viewports.remove(&viewport_id).unwrap(); - ctx.memory.resume_frame(viewport_id); - }); - } else { - // ## Context Cleanup + if is_last { + // Context Cleanup self.write(|ctx| { ctx.input.retain(|id, _| available_viewports.contains(id)); ctx.layer_rects_prev_viewports @@ -1632,6 +1613,15 @@ impl Context { ctx.graphics .retain(|id, _| available_viewports.contains(id)); }); + } else { + let viewport_id = self.viewport_id(); + self.write(|ctx| { + ctx.layer_rects_prev_frame = + ctx.layer_rects_prev_viewports.remove(&viewport_id).unwrap(); + ctx.layer_rects_this_frame = + ctx.layer_rects_this_viewports.remove(&viewport_id).unwrap(); + ctx.memory.resume_frame(viewport_id); + }); } self.write(|ctx| ctx.repaint.end_frame(viewport_id, &available_viewports)); @@ -2574,18 +2564,20 @@ impl Context { self.write(|ctx| ctx.force_embedding = value || !ctx.is_desktop); } - /// This will send the `ViewportCommand` to the current viewport + /// Send a command to the current viewport. pub fn viewport_command(&self, command: ViewportCommand) { self.viewport_command_for(self.viewport_id(), command); } - /// With this you can send a command to a viewport + /// Send a command to a speicfic viewport. pub fn viewport_command_for(&self, id: ViewportId, command: ViewportCommand) { self.write(|ctx| ctx.viewport_commands.push((id, command))); } /// This creates a new native window, if possible. /// + /// You should call this each frame when the viewport should be visible. + /// /// You will need to wrap your viewport state in an `Arc>` or `Arc>`. /// When this is called again with the same id in `ViewportBuilder` the render function for that viewport will be updated. /// * `viewport_ui_cb`: will be called when the viewport receives a event or is requested to be rendered @@ -2594,7 +2586,7 @@ impl Context { /// /// If you use a [`egui::CentralPanel`] you need to check if the viewport is a new window like: /// `ctx.viewport_id() != ctx.parent_viewport_id` if false you should create a [`egui::Window`]. - pub fn create_viewport( + pub fn create_viewport_async( &self, viewport_builder: ViewportBuilder, viewport_ui_cb: impl Fn(&Context) + Send + Sync + 'static, @@ -2610,8 +2602,8 @@ impl Context { window.used = true; window.viewport_ui_cb = Some(Arc::new(Box::new(viewport_ui_cb))); } else { - let id = ViewportId(ctx.viewport_id_generator + 1); ctx.viewport_id_generator += 1; + let id = ViewportId(ctx.viewport_id_generator); ctx.viewports.insert( viewport_builder.id, Viewport { @@ -2665,8 +2657,8 @@ impl Context { window.pair } else { // New - let id = ViewportId(ctx.viewport_id_generator + 1); ctx.viewport_id_generator += 1; + let id = ViewportId(ctx.viewport_id_generator); let id_pair = ViewportIdPair { this: id, parent }; ctx.viewports.insert( viewport_builder.id, diff --git a/crates/egui/src/viewport.rs b/crates/egui/src/viewport.rs index 33b8717d8..7b2a72f12 100644 --- a/crates/egui/src/viewport.rs +++ b/crates/egui/src/viewport.rs @@ -1,10 +1,19 @@ +//! egui supports multiple viewports, corresponding to multiple native windows. +//! +//! Viewports come in two flavors: "sync" and "async". +//! +//! * Sync viewports are executed immediately. +//! * Async viewports are executed later. + use std::{fmt::Display, sync::Arc}; use epaint::{ColorImage, Pos2, Vec2}; use crate::{Context, Id}; -/// This is used to send a command to a specific viewport +/// A unique identifier of a viewport. +/// +/// Generated by [`Context`]. /// /// This is returned by [`Context::viewport_id`] and [`Context::parent_viewport_id`]. #[derive(Default, Debug, Hash, Clone, Copy, PartialEq, Eq)] @@ -36,15 +45,7 @@ impl ViewportIdPair { }; } -impl std::ops::Deref for ViewportIdPair { - type Target = ViewportId; - - fn deref(&self) -> &Self::Target { - &self.this - } -} - -/// The user-code that shows the ui in the viewport. +/// The user-code that shows the ui in the viewport, used for "async" viewports. pub type ViewportUiCallback = dyn Fn(&Context) + Sync + Send; /// Render the given viewport, calling the given ui callback. @@ -97,7 +98,7 @@ impl ViewportBuilder { pub fn new(id: impl Into) -> Self { Self { id: id.into(), - title: "Dummy egui viewport".into(), + title: "egui".into(), name: None, position: None, inner_size: Some(Some(Vec2::new(300.0, 200.0))), @@ -121,13 +122,11 @@ impl ViewportBuilder { hittest: Some(true), } } -} -impl ViewportBuilder { pub fn empty(id: impl Into) -> Self { Self { id: id.into(), - title: "Dummy egui viewport".into(), + title: "egui".into(), name: None, position: None, inner_size: None, @@ -154,7 +153,7 @@ impl ViewportBuilder { /// Sets the initial title of the window in the title bar. /// - /// The default is `"Dummy egui viewport"`. + /// The default is `"egui"`. /// /// Look at winit for more details pub fn with_title(mut self, title: impl Into) -> Self { @@ -424,18 +423,27 @@ pub enum ViewportCommand { #[derive(Clone)] pub(crate) struct Viewport { pub(crate) builder: ViewportBuilder, + + /// Id of us and our parent. pub(crate) pair: ViewportIdPair, + pub(crate) used: bool, - /// The user-code that shows the GUI. + /// The user-code that shows the GUI, used for "async" viewports. + /// + /// `None` for "sync" viewports. pub(crate) viewport_ui_cb: Option>>, } #[derive(Clone)] pub struct ViewportOutput { pub builder: ViewportBuilder, + + /// Id of us and our parent. pub pair: ViewportIdPair, - /// The user-code that shows the GUI. + /// The user-code that shows the GUI, used for "async" viewports. + /// + /// `None` for "sync" viewports. pub viewport_ui_cb: Option>>, } diff --git a/examples/viewports/src/main.rs b/examples/viewports/src/main.rs index efa68b5a2..8650e1609 100644 --- a/examples/viewports/src/main.rs +++ b/examples/viewports/src/main.rs @@ -137,7 +137,7 @@ fn show_async_viewport( ) { let name: String = name.into(); - ctx.create_viewport( + ctx.create_viewport_async( ViewportBuilder::new(name.clone()) .with_title(name.as_str()) .with_inner_size(Some(egui::vec2(450.0, 350.0))),