Fix closing of child viewports

crates/eframe/src/native/epi_integration.rs

@@ -231,7 +231,6 @@ impl EpiIntegration {
 
         match event {
             WindowEvent::CloseRequested => {
-                log::debug!("Received WindowEvent::CloseRequested for viewport {viewport_id:?}");
                 if viewport_id == ViewportId::ROOT {
                     self.close = app.on_close_event();
                     log::debug!("App::on_close_event returned {}", self.close);
@@ -257,7 +256,7 @@ impl EpiIntegration {
             _ => {}
         }
 
-        egui_winit.on_window_event(&self.egui_ctx, event, viewport_id)
+        egui_winit.on_window_event(&self.egui_ctx, event)
     }
 
     pub fn pre_update(&mut self) {
@@ -283,7 +282,7 @@ impl EpiIntegration {
                 viewport_ui_cb(egui_ctx);
             } else {
                 // Root viewport
-                if egui_ctx.input(|i| i.viewport().close_requested) {
+                if egui_ctx.input(|i| i.viewport().close_requested()) {
                     self.close = app.on_close_event();
                     log::debug!("App::on_close_event returned {}", self.close);
                 }

crates/eframe/src/native/glow_integration.rs

@@ -656,12 +656,8 @@ impl GlowWinitRunning {
     ) -> EventResult {
         crate::profile_function!(egui_winit::short_window_event_description(event));
 
-        let viewport_id = self
-            .glutin
-            .borrow()
-            .viewport_from_window
-            .get(&window_id)
-            .copied();
+        let mut glutin = self.glutin.borrow_mut();
+        let viewport_id = glutin.viewport_from_window.get(&window_id).copied();
 
         // On Windows, if a window is resized by the user, it should repaint synchronously, inside the
         // event handler.
@@ -680,8 +676,7 @@ impl GlowWinitRunning {
 
         match event {
             winit::event::WindowEvent::Focused(new_focused) => {
-                self.glutin.borrow_mut().focused_viewport =
-                    new_focused.then(|| viewport_id).flatten();
+                glutin.focused_viewport = new_focused.then(|| viewport_id).flatten();
             }
 
             winit::event::WindowEvent::Resized(physical_size) => {
@@ -691,64 +686,67 @@ impl GlowWinitRunning {
                 if 0 < physical_size.width && 0 < physical_size.height {
                     if let Some(viewport_id) = viewport_id {
                         repaint_asap = true;
-                        self.glutin.borrow_mut().resize(viewport_id, *physical_size);
+                        glutin.resize(viewport_id, *physical_size);
                     }
                 }
             }
 
             winit::event::WindowEvent::ScaleFactorChanged { new_inner_size, .. } => {
                 if let Some(viewport_id) = viewport_id {
                     repaint_asap = true;
-                    self.glutin
-                        .borrow_mut()
-                        .resize(viewport_id, **new_inner_size);
+                    glutin.resize(viewport_id, **new_inner_size);
                 }
             }
 
             winit::event::WindowEvent::CloseRequested => {
-                log::debug!("Received WindowEvent::CloseRequested for viewport {viewport_id:?}");
-
-                if let Some(viewport_id) = viewport_id {
-                    if let Some(viewport) = self.glutin.borrow_mut().viewports.get_mut(&viewport_id)
-                    {
-                        viewport.info.close_requested = true;
-                    }
-                }
-
-                let is_root = viewport_id == Some(ViewportId::ROOT);
-                if is_root && self.integration.should_close() {
+                if viewport_id == Some(ViewportId::ROOT) && self.integration.should_close() {
                     log::debug!(
                         "Received WindowEvent::CloseRequested for main viewport - shutting down."
                     );
                     return EventResult::Exit;
                 }
+
+                log::debug!("Received WindowEvent::CloseRequested for viewport {viewport_id:?}");
+
+                if let Some(viewport_id) = viewport_id {
+                    if let Some(viewport) = glutin.viewports.get_mut(&viewport_id) {
+                        // Tell viewport it should close:
+                        viewport.info.events.push(egui::ViewportEvent::Close);
+
+                        // We may need to repaint both us and our parent to close the window,
+                        // and perhaps twice (once to notice the close-event, once again to enforce it).
+                        // `request_repaint_of` does a double-repaint though:
+                        self.integration.egui_ctx.request_repaint_of(viewport_id);
+                        self.integration
+                            .egui_ctx
+                            .request_repaint_of(viewport.ids.parent);
+                    }
+                }
             }
 
             _ => {}
         }
 
-        let event_response = 'res: {
-            if let Some(viewport_id) = viewport_id {
-                let mut glutin = self.glutin.borrow_mut();
-                if let Some(viewport) = glutin.viewports.get_mut(&viewport_id) {
-                    break 'res self.integration.on_window_event(
-                        self.app.as_mut(),
-                        event,
-                        viewport.egui_winit.as_mut().unwrap(),
-                        viewport.ids.this,
-                    );
-                }
-            }
+        if self.integration.should_close() {
+            return EventResult::Exit;
+        }
 
-            EventResponse {
-                consumed: false,
-                repaint: false,
-            }
+        let mut event_response = EventResponse {
+            consumed: false,
+            repaint: false,
         };
+        if let Some(viewport_id) = viewport_id {
+            if let Some(viewport) = glutin.viewports.get_mut(&viewport_id) {
+                event_response = self.integration.on_window_event(
+                    self.app.as_mut(),
+                    event,
+                    viewport.egui_winit.as_mut().unwrap(),
+                    viewport.ids.this,
+                );
+            }
+        }
 
-        if self.integration.should_close() {
-            EventResult::Exit
-        } else if event_response.repaint {
+        if event_response.repaint {
             if repaint_asap {
                 EventResult::RepaintNow(window_id)
             } else {

crates/eframe/src/native/run.rs

@@ -147,7 +147,6 @@ fn run_and_return(
                 }
             }
             EventResult::RepaintNext(window_id) => {
-                log::trace!("Repaint caused by {}", short_event_description(&event));
                 windows_next_repaint_times.insert(window_id, Instant::now());
             }
             EventResult::RepaintAt(window_id, repaint_time) => {

crates/eframe/src/native/wgpu_integration.rs

@@ -697,53 +697,57 @@ impl WgpuWinitRunning {
                 if let (Some(width), Some(height), Some(viewport_id)) = (
                     NonZeroU32::new(new_inner_size.width),
                     NonZeroU32::new(new_inner_size.height),
-                    shared.viewport_from_window.get(&window_id).copied(),
+                    viewport_id,
                 ) {
                     repaint_asap = true;
                     shared.painter.on_window_resized(viewport_id, width, height);
                 }
             }
 
             winit::event::WindowEvent::CloseRequested => {
+                if viewport_id == Some(ViewportId::ROOT) && integration.should_close() {
+                    log::debug!(
+                        "Received WindowEvent::CloseRequested for main viewport - shutting down."
+                    );
+                    return EventResult::Exit;
+                }
+
                 log::debug!("Received WindowEvent::CloseRequested for viewport {viewport_id:?}");
 
                 if let Some(viewport_id) = viewport_id {
                     if let Some(viewport) = shared.viewports.get_mut(&viewport_id) {
-                        viewport.info.close_requested = true;
+                        // Tell viewport it should close:
+                        viewport.info.events.push(egui::ViewportEvent::Close);
+
+                        // We may need to repaint both us and our parent to close the window,
+                        // and perhaps twice (once to notice the close-event, once again to enforce it).
+                        // `request_repaint_of` does a double-repaint though:
+                        integration.egui_ctx.request_repaint_of(viewport_id);
+                        integration.egui_ctx.request_repaint_of(viewport.ids.parent);
                     }
                 }
-
-                let is_root = viewport_id == Some(ViewportId::ROOT);
-                if is_root && integration.should_close() {
-                    log::debug!(
-                        "Received WindowEvent::CloseRequested for main viewport - shutting down."
-                    );
-                    return EventResult::Exit;
-                }
             }
 
             _ => {}
         };
 
-        let event_response = viewport_id.and_then(|viewport_id| {
-            shared.viewports.get_mut(&viewport_id).and_then(|viewport| {
-                viewport.egui_winit.as_mut().map(|egui_winit| {
-                    integration.on_window_event(app.as_mut(), event, egui_winit, viewport_id)
+        let event_response = viewport_id
+            .and_then(|viewport_id| {
+                shared.viewports.get_mut(&viewport_id).and_then(|viewport| {
+                    viewport.egui_winit.as_mut().map(|egui_winit| {
+                        integration.on_window_event(app.as_mut(), event, egui_winit, viewport_id)
+                    })
                 })
             })
-        });
+            .unwrap_or_default();
 
         if integration.should_close() {
             EventResult::Exit
-        } else if let Some(event_response) = event_response {
-            if event_response.repaint {
-                if repaint_asap {
-                    EventResult::RepaintNow(window_id)
-                } else {
-                    EventResult::RepaintNext(window_id)
-                }
+        } else if event_response.repaint {
+            if repaint_asap {
+                EventResult::RepaintNow(window_id)
             } else {
-                EventResult::Wait
+                EventResult::RepaintNext(window_id)
             }
         } else {
             EventResult::Wait

crates/egui-winit/src/lib.rs

@@ -38,6 +38,7 @@ pub fn screen_size_in_pixels(window: &Window) -> egui::Vec2 {
 // ----------------------------------------------------------------------------
 
 #[must_use]
+#[derive(Clone, Copy, Debug, Default)]
 pub struct EventResponse {
     /// If true, egui consumed this event, i.e. wants exclusive use of this event
     /// (e.g. a mouse click on an egui window, or entering text into a text field).
@@ -237,7 +238,6 @@ impl State {
         &mut self,
         egui_ctx: &egui::Context,
         event: &winit::event::WindowEvent<'_>,
-        viewport_id: ViewportId,
     ) -> EventResponse {
         crate::profile_function!(short_window_event_description(event));
 
@@ -421,15 +421,11 @@ impl State {
             }
 
             // Things that may require repaint:
-            WindowEvent::CloseRequested => {
-                if let Some(viewport_info) = self.egui_input.viewports.get_mut(&viewport_id) {
-                    viewport_info.close_requested = true;
-                }
-                EventResponse {
-                    consumed: true,
-                    repaint: true,
-                }
-            }
+            WindowEvent::CloseRequested => EventResponse {
+                consumed: true,
+                repaint: true,
+            },
+
             WindowEvent::CursorEntered { .. }
             | WindowEvent::Destroyed
             | WindowEvent::Occluded(_)
@@ -1053,7 +1049,7 @@ pub fn process_viewport_commands(
     for command in commands {
         match command {
             ViewportCommand::Close => {
-                info.close_requested = true;
+                info.events.push(egui::ViewportEvent::Close);
             }
             ViewportCommand::StartDrag => {
                 // If `is_viewport_focused` is not checked on x11 the input will be permanently taken until the app is killed!

crates/egui/src/context.rs

@@ -2563,8 +2563,13 @@ impl Context {
     ///
     /// This lets you affect another viewport, e.g. resizing its window.
     pub fn send_viewport_cmd_to(&self, id: ViewportId, command: ViewportCommand) {
-        self.write(|ctx| ctx.viewport_for(id).commands.push(command));
         self.request_repaint_of(id);
+
+        if command.requires_parent_repaint() {
+            self.request_repaint_of(self.parent_viewport_id());
+        }
+
+        self.write(|ctx| ctx.viewport_for(id).commands.push(command));
     }
 
     /// Show a deferred viewport, creating a new native window, if possible.

crates/egui/src/data/input.rs

@@ -163,6 +163,18 @@ impl RawInput {
     }
 }
 
+/// An input event from the backend into egui, about a specific [viewport](crate::viewport).
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
+pub enum ViewportEvent {
+    /// The user clicked the close-button on the window, or similar.
+    ///
+    /// It is up to the user to react to this by _not_ showing the viewport in the next frame in the parent viewport.
+    ///
+    /// This even will wake up both the child and parent viewport.
+    Close,
+}
+
 /// Information about the current viewport,
 /// given as input each frame.
 ///
@@ -176,9 +188,7 @@ pub struct ViewportInfo {
     /// Name of the viewport, if known.
     pub title: Option<String>,
 
-    /// The user requested the viewport should close,
-    /// e.g. by pressing the close button in the window decoration.
-    pub close_requested: bool,
+    pub events: Vec<ViewportEvent>,
 
     /// Number of physical pixels per ui point.
     pub pixels_per_point: f32,
@@ -212,15 +222,17 @@ pub struct ViewportInfo {
 }
 
 impl ViewportInfo {
-    pub fn take(&mut self) -> Self {
-        core::mem::take(self)
+    pub fn close_requested(&self) -> bool {
+        self.events
+            .iter()
+            .any(|&event| event == ViewportEvent::Close)
     }
 
     pub fn ui(&self, ui: &mut crate::Ui) {
         let Self {
             parent,
             title,
-            close_requested,
+            events,
             pixels_per_point,
             monitor_size,
             inner_rect,
@@ -240,8 +252,8 @@ impl ViewportInfo {
             ui.label(opt_as_str(title));
             ui.end_row();
 
-            ui.label("Close requested:");
-            ui.label(close_requested.to_string());
+            ui.label("Events:");
+            ui.label(format!("{events:?}"));
             ui.end_row();
 
             ui.label("Pixels per point:");

crates/egui/src/viewport.rs

@@ -765,7 +765,9 @@ pub enum ResizeDirection {
     SouthWest,
 }
 
-/// You can send a [`ViewportCommand`] to the viewport with [`Context::send_viewport_cmd`].
+/// An output [viewport](crate::viewport)-command from egui to the backend, e.g. to change the window title or size.
+///
+///  You can send a [`ViewportCommand`] to the viewport with [`Context::send_viewport_cmd`].
 ///
 /// See [`crate::viewport`] for how to build new viewports (native windows).
 ///
@@ -903,6 +905,11 @@ impl ViewportCommand {
             }
         })
     }
+
+    /// This command requires the parent viewport to repaint.
+    pub fn requires_parent_repaint(&self) -> bool {
+        self == &Self::Close
+    }
 }
 
 /// Describes a viewport, i.e. a native window.

crates/egui_demo_lib/src/demo/extra_viewport.rs

@@ -66,7 +66,7 @@ fn viewport_content(ui: &mut egui::Ui, ctx: &egui::Context, open: &mut bool) {
         }
     });
 
-    if ui.input(|i| i.viewport().close_requested) {
+    if ui.input(|i| i.viewport().close_requested()) {
         *open = false;
     }
 }