From 973cfed87b37728402af4b0b4135cc612ae7936c Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Thu, 21 Mar 2024 12:53:52 +0100 Subject: [PATCH] shell: Handle unmapped windows correctly --- src/shell/element/mod.rs | 2 +- src/shell/element/stack.rs | 13 ++-- src/shell/grabs/moving.rs | 4 ++ src/shell/mod.rs | 63 ++++++++++++++++- src/wayland/handlers/compositor.rs | 96 ++++++++++++++++++++------ src/wayland/handlers/xdg_shell/mod.rs | 3 + src/wayland/protocols/toplevel_info.rs | 20 +++++- src/xwayland.rs | 25 +------ 8 files changed, 175 insertions(+), 51 deletions(-) diff --git a/src/shell/element/mod.rs b/src/shell/element/mod.rs index 37abe49f..b78f83c4 100644 --- a/src/shell/element/mod.rs +++ b/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::{ diff --git a/src/shell/element/stack.rs b/src/shell/element/stack.rs index c15d35c0..6e1c3ee9 100644 --- a/src/shell/element/stack.rs +++ b/src/shell/element/stack.rs @@ -235,18 +235,18 @@ impl CosmicStack { self.0.force_redraw() } - pub fn remove_idx(&self, idx: usize) { - self.0.with_program(|p| { + pub fn remove_idx(&self, idx: usize) -> Option { + 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 { diff --git a/src/shell/grabs/moving.rs b/src/shell/grabs/moving.rs index 67a1abf0..345842e9 100644 --- a/src/shell/grabs/moving.rs +++ b/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() } diff --git a/src/shell/mod.rs b/src/shell/mod.rs index 3e909be6..0f610ed7 100644 --- a/src/shell/mod.rs +++ b/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(&mut self, surface: &S, seat: &Seat) + where + CosmicSurface: PartialEq, + { + 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, diff --git a/src/wayland/handlers/compositor.rs b/src/wayland/handlers/compositor.rs index 68669a08..863d5fdd 100644 --- a/src/wayland/handlers/compositor.rs +++ b/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::(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::(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. - // 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_wl_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); + // session-lock disallows null commits + + // if it was a move-grab? + let seat = self.common.last_active_seat().clone(); + let moved_window = seat + .user_data() + .get::() + .unwrap() + .borrow() + .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 diff --git a/src/wayland/handlers/xdg_shell/mod.rs b/src/wayland/handlers/xdg_shell/mod.rs index 0bdb20ca..2c910161 100644 --- a/src/wayland/handlers/xdg_shell/mod.rs +++ b/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 diff --git a/src/wayland/protocols/toplevel_info.rs b/src/wayland/protocols/toplevel_info.rs index 30386cfb..216d363d 100644 --- a/src/wayland/protocols/toplevel_info.rs +++ b/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::() { + 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>) { self.toplevels.retain(|window| { let mut state = window diff --git a/src/xwayland.rs b/src/xwayland.rs index 77309129..4f05f4a1 100644 --- a/src/xwayland.rs +++ b/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 - .common - .shell - .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); - } + } else { + let seat = self.common.last_active_seat().clone(); + self.common.shell.unmap_surface(&window, &seat); } let outputs = if let Some(wl_surface) = window.wl_surface() {