Clarify logic around how viewports are retained, and add ViewportIdSet

emilk · Nov 7, 2023 · 645521b · 645521b
1 parent 54c6d51
commit 645521b
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 15 deletions.
diff --git a/crates/eframe/src/native/run.rs b/crates/eframe/src/native/run.rs
@@ -459,8 +459,8 @@ fn run_and_exit(event_loop: EventLoop<UserEvent>, mut winit_app: impl WinitApp +
 mod glow_integration {
 
     use egui::{
-        epaint::ahash::HashMap, NumExt as _, ViewportIdMap, ViewportIdPair, ViewportOutput,
-        ViewportUiCallback,
+        epaint::ahash::HashMap, NumExt as _, ViewportIdMap, ViewportIdPair, ViewportIdSet,
+        ViewportOutput, ViewportUiCallback,
     };
     use egui_winit::{create_winit_window_builder, process_viewport_commands, EventResponse};
     use glutin::{
@@ -1256,7 +1256,8 @@ mod glow_integration {
             glutin_ctx: &Rc<RefCell<GlutinWindowContext>>,
             mut viewports: Vec<ViewportOutput>,
         ) {
-            let mut active_viewports_ids = vec![ViewportId::ROOT];
+            let mut active_viewports_ids = ViewportIdSet::default();
+            active_viewports_ids.insert(ViewportId::ROOT);
 
             viewports.retain_mut(
                 |ViewportOutput {
@@ -1280,7 +1281,7 @@ mod glow_integration {
                         } else if let Some(w) = viewport.window.clone() {
                             process_viewport_commands(commands, *id, None, &w.borrow());
                         }
-                        active_viewports_ids.push(*id);
+                        active_viewports_ids.insert(*id);
                         false
                     } else {
                         true
@@ -1316,7 +1317,7 @@ mod glow_integration {
                     );
                     glutin.builders.insert(id_pair.this, builder);
                 }
-                active_viewports_ids.push(id_pair.this);
+                active_viewports_ids.insert(id_pair.this);
             }
 
             let mut glutin = glutin_ctx.borrow_mut();
@@ -1886,10 +1887,11 @@ pub use glow_integration::run_glow;
 
 #[cfg(feature = "wgpu")]
 mod wgpu_integration {
-    use egui::{ViewportIdMap, ViewportIdPair, ViewportOutput, ViewportUiCallback};
-    use egui_winit::{create_winit_window_builder, process_viewport_commands};
     use parking_lot::Mutex;
 
+    use egui::{ViewportIdMap, ViewportIdPair, ViewportIdSet, ViewportOutput, ViewportUiCallback};
+    use egui_winit::{create_winit_window_builder, process_viewport_commands};
+
     use super::*;
 
     #[derive(Clone)]
@@ -2510,7 +2512,8 @@ mod wgpu_integration {
                     integration.post_present(&window.borrow());
                 }
 
-                let mut active_viewports_ids = vec![ViewportId::ROOT];
+                let mut active_viewports_ids = ViewportIdSet::default();
+                active_viewports_ids.insert(ViewportId::ROOT);
 
                 out_viewports.retain_mut(
                     |ViewportOutput {
@@ -2521,7 +2524,7 @@ mod wgpu_integration {
                         if let Some(viewport) = viewports.borrow_mut().get_mut(this) {
                             viewport.viewport_ui_cb = viewport_ui_cb.clone();
                             viewport.parent_id = *parent;
-                            active_viewports_ids.push(*this);
+                            active_viewports_ids.insert(*this);
                             false
                         } else {
                             true
@@ -2577,7 +2580,7 @@ mod wgpu_integration {
                         builders.insert(id_pair.this, new_builder);
                     }
 
-                    active_viewports_ids.push(id_pair.this);
+                    active_viewports_ids.insert(id_pair.this);
                 }
 
                 for (viewport_id, command) in viewport_commands {

diff --git a/crates/egui-wgpu/src/winit.rs b/crates/egui-wgpu/src/winit.rs
@@ -1,6 +1,6 @@
 use std::{num::NonZeroU32, sync::Arc};
 
-use egui::{ViewportId, ViewportIdMap};
+use egui::{ViewportId, ViewportIdMap, ViewportIdSet};
 
 use crate::{renderer, RenderState, SurfaceErrorAction, WgpuConfiguration};
 
@@ -625,7 +625,7 @@ impl Painter {
         screenshot
     }
 
-    pub fn clean_surfaces(&mut self, available_viewports: &[ViewportId]) {
+    pub fn clean_surfaces(&mut self, available_viewports: &ViewportIdSet) {
         self.surfaces
             .retain(|id, _| available_viewports.contains(id));
         self.depth_texture_view

diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs
@@ -1551,7 +1551,7 @@ impl Context {
                 let was_used = viewport.used;
 
                 if viewport_id == viewport.id_pair.parent {
-                    viewport.used = false;
+                    viewport.used = false; // reset so we can check again next frame
                 }
 
                 viewports.push(ViewportOutput {
@@ -1560,8 +1560,19 @@ impl Context {
                     viewport_ui_cb: viewport.viewport_ui_cb.clone(),
                 });
 
-                (was_used || viewport_id != viewport.id_pair.parent)
-                    && all_viewport_ids.contains(&viewport.id_pair.parent)
+                if !all_viewport_ids.contains(&viewport.id_pair.parent) {
+                    // Parent is gone - remove this viewport.
+                    return false;
+                }
+
+                let is_child = viewport_id == viewport.id_pair.parent;
+                if is_child {
+                    // Keep all children that have been updated this frame
+                    was_used
+                } else {
+                    // Somebody elses child - don't touch
+                    true
+                }
             });
         });
 

diff --git a/crates/egui/src/data/output.rs b/crates/egui/src/data/output.rs
@@ -22,8 +22,10 @@ pub struct FullOutput {
     /// You can use [`crate::Context::tessellate`] to turn this into triangles.
     pub shapes: Vec<epaint::ClippedShape>,
 
+    /// All the active viewports, including the root.
     pub viewports: Vec<ViewportOutput>,
 
+    /// Commands sent to different viewports.
     pub viewport_commands: Vec<(ViewportId, ViewportCommand)>,
 }
 

diff --git a/crates/egui/src/viewport.rs b/crates/egui/src/viewport.rs
@@ -641,6 +641,7 @@ pub(crate) struct Viewport {
     /// Id of us and our parent.
     pub(crate) id_pair: ViewportIdPair,
 
+    /// Has this viewport been updated this frame?
     pub(crate) used: bool,
 
     /// The user-code that shows the GUI, used for "async" viewports.