shell: Handle unmapped windows correctly

src/shell/element/mod.rs

@@ -22,7 +22,7 @@ use smithay::{
             ImportAll, ImportMem, Renderer,
         },
     },
-    desktop::{space::SpaceElement, PopupManager, WindowSurface, WindowSurfaceType},
+    desktop::{space::SpaceElement, PopupManager, WindowSurfaceType},
     input::{
         keyboard::{KeyboardTarget, KeysymHandle, ModifiersState},
         pointer::{

src/shell/element/stack.rs

@@ -235,18 +235,18 @@ impl CosmicStack {
         self.0.force_redraw()
     }
 
-    pub fn remove_idx(&self, idx: usize) {
+    pub fn remove_idx(&self, idx: usize) -> Option<CosmicSurface> {
-        self.0.with_program(|p| {
+        let window = self.0.with_program(|p| {
             let mut windows = p.windows.lock().unwrap();
             if windows.len() == 1 {
                 p.override_alive.store(false, Ordering::SeqCst);
                 let window = windows.get(0).unwrap();
                 window.try_force_undecorated(false);
                 window.set_tiled(false);
-                return;
+                return Some(window.clone());
             }
             if windows.len() <= idx {
-                return;
+                return None;
             }
             if idx == p.active.load(Ordering::SeqCst) {
                 p.reenter.store(true, Ordering::SeqCst);
@@ -256,8 +256,11 @@ impl CosmicStack {
             window.set_tiled(false);
 
             p.active.fetch_min(windows.len() - 1, Ordering::SeqCst);
+
+            Some(window)
         });
-        self.0.force_redraw()
+        self.0.force_redraw();
+        window
     }
 
     pub fn len(&self) -> usize {

src/shell/grabs/moving.rs

@@ -232,6 +232,10 @@ impl MoveGrabState {
             .collect()
     }
 
+    pub fn element(&self) -> CosmicMapped {
+        self.window.clone()
+    }
+
     pub fn window(&self) -> CosmicSurface {
         self.window.active_window()
     }

src/shell/mod.rs

@@ -29,7 +29,7 @@ use smithay::{
         },
         wayland_server::{protocol::wl_surface::WlSurface, Client, DisplayHandle},
     },
-    utils::{Logical, Point, Rectangle, Serial, Size, SERIAL_COUNTER},
+    utils::{IsAlive, Logical, Point, Rectangle, Serial, Size, SERIAL_COUNTER},
     wayland::{
         compositor::with_states,
         seat::WaylandFocus,
@@ -262,7 +262,7 @@ impl WorkspaceDelta {
 #[derive(Debug)]
 pub struct WorkspaceSet {
     previously_active: Option<(usize, WorkspaceDelta)>,
-    active: usize,
+    pub active: usize,
     pub group: WorkspaceGroupHandle,
     idx: usize,
     tiling_enabled: bool,
@@ -1682,6 +1682,9 @@ impl Shell {
             .iter()
             .for_each(|or| or.refresh());
 
+        self.pending_layers.retain(|(s, _, _)| s.alive());
+        self.pending_windows.retain(|(s, _, _)| s.alive());
+
         self.toplevel_info_state
             .refresh(Some(&self.workspace_state));
     }
@@ -1994,6 +1997,62 @@ impl Shell {
         }
     }
 
+    pub fn unmap_surface<S>(&mut self, surface: &S, seat: &Seat<State>)
+    where
+        CosmicSurface: PartialEq<S>,
+    {
+        for set in self.workspaces.sets.values_mut() {
+            let sticky_res = set.sticky_layer.mapped().find_map(|m| {
+                m.windows()
+                    .position(|(s, _)| &s == surface)
+                    .map(|idx| (idx, m.clone()))
+            });
+            let surface = if let Some((idx, mut mapped)) = sticky_res {
+                if mapped.is_stack() {
+                    mapped.stack_ref_mut().unwrap().remove_idx(idx)
+                } else {
+                    set.sticky_layer.unmap(&mapped);
+                    Some(mapped.active_window())
+                }
+            } else if let Some(idx) = set
+                .minimized_windows
+                .iter()
+                .map(|w| &w.window)
+                .position(|w| w.windows().any(|(s, _)| &s == surface))
+            {
+                if set.minimized_windows.get(idx).unwrap().window.is_stack() {
+                    let window = &mut set.minimized_windows.get_mut(idx).unwrap().window;
+                    let stack = window.stack_ref_mut().unwrap();
+                    let idx = stack.surfaces().position(|s| &s == surface);
+                    idx.and_then(|idx| stack.remove_idx(idx))
+                } else {
+                    Some(set.minimized_windows.remove(idx).window.active_window())
+                }
+            } else if let Some((workspace, mut elem)) = set.workspaces.iter_mut().find_map(|w| {
+                w.element_for_surface(&surface)
+                    .cloned()
+                    .map(|elem| (w, elem))
+            }) {
+                if elem.is_stack() {
+                    let stack = elem.stack_ref_mut().unwrap();
+                    let idx = stack.surfaces().position(|s| &s == surface);
+                    idx.and_then(|idx| stack.remove_idx(idx))
+                } else {
+                    workspace.unmap(&elem);
+                    Some(elem.active_window())
+                }
+            } else {
+                None
+            };
+
+            if let Some(surface) = surface {
+                self.toplevel_info_state.remove_toplevel(&surface);
+                self.pending_windows.push((surface, seat.clone(), None));
+                return;
+            }
+        }
+    }
+
     pub fn element_under(
         &mut self,
         location: Point<f64, Global>,

src/wayland/handlers/compositor.rs

@@ -1,12 +1,16 @@
 // SPDX-License-Identifier: GPL-3.0-only
 
-use crate::{state::ClientState, utils::prelude::*, wayland::protocols::screencopy::SessionType};
+use crate::{
+    shell::grabs::SeatMoveGrabState, state::ClientState, utils::prelude::*,
+    wayland::protocols::screencopy::SessionType,
+};
 use calloop::Interest;
 use smithay::{
     backend::renderer::utils::{on_commit_buffer_handler, with_renderer_surface_state},
     delegate_compositor,
     desktop::{layer_map_for_output, LayerSurface, PopupKind, WindowSurfaceType},
     reexports::wayland_server::{protocol::wl_surface::WlSurface, Client, Resource},
+    utils::SERIAL_COUNTER,
     wayland::{
         compositor::{
             add_blocker, add_pre_commit_hook, with_states, BufferAssignment, CompositorClientState,
@@ -142,10 +146,10 @@ impl CompositorHandler for State {
 
     fn commit(&mut self, surface: &WlSurface) {
         X11Wm::commit_hook::<State>(surface);
-        // first load the buffer for various smithay helper functions
+        // first load the buffer for various smithay helper functions (which also initializes the RendererSurfaceState)
         on_commit_buffer_handler::<Self>(surface);
 
-        // then handle initial configure events and map windows if necessary
+        // handle initial configure events and map windows if necessary
         if let Some((window, _, _)) = self
             .common
             .shell
@@ -182,27 +186,79 @@ impl CompositorHandler for State {
             self.xdg_popup_ensure_initial_configure(&popup);
         }
 
-        // at last handle some special cases, like grabs and changing layer surfaces
+        if with_renderer_surface_state(surface, |state| state.buffer().is_none()).unwrap_or(false) {
+            // handle null-commits causing weird conflicts:
 
-        // If we would re-position the window inside the grab we would get a weird jittery animation.
+            // session-lock disallows null commits
-        // We only want to resize once the client has acknoledged & commited the new size,
+
-        // so we need to carefully track the state through different handlers.
+            // if it was a move-grab?
-        if let Some(element) = self.common.shell.element_for_wl_surface(surface).cloned() {
+            let seat = self.common.last_active_seat().clone();
-            crate::shell::layout::floating::ResizeSurfaceGrab::apply_resize_to_location(
+            let moved_window = seat
-                element.clone(),
+                .user_data()
-                &mut self.common.shell,
+                .get::<SeatMoveGrabState>()
-            );
+                .unwrap()
-            if let Some(workspace) = self.common.shell.space_for_mut(&element) {
+                .borrow()
-                workspace.commit(surface);
+                .as_ref()
+                .and_then(|state| {
+                    state
+                        .element()
+                        .windows()
+                        .any(|(s, _)| {
+                            s.wl_surface()
+                                .as_ref()
+                                .map(|s| s == surface)
+                                .unwrap_or(false)
+                        })
+                        .then(|| state.element())
+                });
+            if let Some(mut window) = moved_window {
+                if window.is_stack() {
+                    let stack = window.stack_ref_mut().unwrap();
+                    if let Some(i) = stack.surfaces().position(|s| {
+                        s.wl_surface()
+                            .as_ref()
+                            .map(|s| s == surface)
+                            .unwrap_or(false)
+                    }) {
+                        stack.remove_idx(i);
+                    }
+                } else {
+                    seat.get_pointer()
+                        .unwrap()
+                        .unset_grab(self, SERIAL_COUNTER.next_serial(), 0);
+                }
             }
+
+            // if it was a layer-shell surface?
+            // ignore, that will affect recompute normally
+
+            // if it was a sticky / floating / tiled window
+            // we could unmap, I guess?
+
+            // if it was an x11 surface => do nothing, we will get a separate UnmapNotify anyway
+        } else {
+            // handle some special cases, like grabs and changing layer surfaces
+
+            // If we would re-position the window inside the grab we would get a weird jittery animation.
+            // We only want to resize once the client has acknoledged & commited the new size,
+            // so we need to carefully track the state through different handlers.
+            if let Some(element) = self.common.shell.element_for_surface(surface).cloned() {
+                crate::shell::layout::floating::ResizeSurfaceGrab::apply_resize_to_location(
+                    element.clone(),
+                    &mut self.common.shell,
+                );
+                if let Some(workspace) = self.common.shell.space_for_mut(&element) {
+                    workspace.commit(surface);
+                }
+            }
+
+            //handle window screencopy sessions
+            self.schedule_window_session(surface);
+
+            // and refresh smithays internal state
+            self.common.shell.popups.commit(surface);
         }
 
-        //handle window screencopy sessions
-        self.schedule_window_session(surface);
-
-        // and refresh smithays internal state
-        self.common.shell.popups.commit(surface);
-
         // re-arrange layer-surfaces (commits may change size and positioning)
         let layer_output = self
             .common

src/wayland/handlers/xdg_shell/mod.rs

@@ -382,6 +382,9 @@ impl XdgShellHandler for State {
     }
 
     fn toplevel_destroyed(&mut self, surface: ToplevelSurface) {
+        let seat = self.common.last_active_seat().clone();
+        self.common.shell.unmap_surface(surface.wl_surface(), &seat);
+
         let output = self
             .common
             .shell

src/wayland/protocols/toplevel_info.rs

@@ -19,7 +19,7 @@ use cosmic_protocols::toplevel_info::v1::server::{
     zcosmic_toplevel_info_v1::{self, ZcosmicToplevelInfoV1},
 };
 
-pub trait Window: IsAlive + Clone + Send {
+pub trait Window: IsAlive + Clone + PartialEq + Send {
     fn title(&self) -> String;
     fn app_id(&self) -> String;
     fn is_activated(&self) -> bool;
@@ -247,6 +247,24 @@ where
         }
     }
 
+    pub fn remove_toplevel(&mut self, toplevel: &W) {
+        if let Some(state) = toplevel.user_data().get::<ToplevelState>() {
+            let mut state_inner = state.lock().unwrap();
+            for handle in &state_inner.instances {
+                // don't send events to stopped instances
+                if self
+                    .instances
+                    .iter()
+                    .any(|i| i.id().same_client_as(&handle.id()))
+                {
+                    handle.closed();
+                }
+            }
+            *state_inner = Default::default();
+        }
+        self.toplevels.retain(|w| w != toplevel);
+    }
+
     pub fn refresh(&mut self, workspace_state: Option<&WorkspaceState<D>>) {
         self.toplevels.retain(|window| {
             let mut state = window

src/xwayland.rs

@@ -335,28 +335,9 @@ impl XwmHandler for State {
                 .shell
                 .override_redirect_windows
                 .retain(|or| or != &window);
-        } else if let Some((element, space)) = self
+        } else {
-            .common
+            let seat = self.common.last_active_seat().clone();
-            .shell
+            self.common.shell.unmap_surface(&window, &seat);
-            .element_for_x11_surface(&window)
-            .cloned()
-            .and_then(|element| {
-                self.common
-                    .shell
-                    .space_for_mut(&element)
-                    .map(|space| (element, space))
-            })
-        {
-            if element.is_stack() && element.stack_ref().unwrap().len() >= 2 {
-                if let Some((surface, _)) = element
-                    .windows()
-                    .find(|(w, _)| w.x11_surface() == Some(&window))
-                {
-                    element.stack_ref().unwrap().remove_window(&surface);
-                }
-            } else {
-                space.unmap(&element);
-            }
         }
 
         let outputs = if let Some(wl_surface) = window.wl_surface() {