From 56a681bfecc5831f41f8eb0ec8c7e96c6b277153 Mon Sep 17 00:00:00 2001 From: Shawn Wallace Date: Sun, 27 Apr 2025 01:08:16 -0400 Subject: [PATCH] Mark windows with _NET_WM_STATE_SKIP_TASKBAR as popups Also includes some light refactoring of the popup flow in general and trimming down some unused code. I suspect this may cause some windows to unexpectedly become popups when they otherwise shouldn't, but that's a bridge we'll cross when we get there. Fixes #110 and #112. --- src/server/mod.rs | 170 ++++++++++++++++++++++--------------------- src/server/tests.rs | 24 ++---- src/xstate/mod.rs | 121 ++++++++++++++---------------- tests/integration.rs | 21 ++++++ 4 files changed, 173 insertions(+), 163 deletions(-) diff --git a/src/server/mod.rs b/src/server/mod.rs index a5cd2f1..3c81cf5 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -85,15 +85,14 @@ where } #[derive(Default, Debug)] -pub struct WindowAttributes { - pub override_redirect: bool, - pub popup_for: Option, - pub dims: WindowDims, - pub size_hints: Option, - pub title: Option, - pub class: Option, - pub group: Option, - pub decorations: Option, +struct WindowAttributes { + is_popup: bool, + dims: WindowDims, + size_hints: Option, + title: Option, + class: Option, + group: Option, + decorations: Option, } #[derive(Debug, Default, PartialEq, Eq, Copy, Clone)] @@ -119,7 +118,6 @@ impl WindowData { window: x::Window, override_redirect: bool, dims: WindowDims, - parent: Option, activation_token: Option, ) -> Self { Self { @@ -128,9 +126,8 @@ impl WindowData { surface_serial: None, mapped: false, attrs: WindowAttributes { - override_redirect, + is_popup: override_redirect, dims, - popup_for: parent, ..Default::default() }, output_offset: WindowOutputOffset::default(), @@ -643,7 +640,6 @@ impl ServerState { window: x::Window, override_redirect: bool, dims: WindowDims, - parent: Option, pid: Option, ) { let activation_token = pid @@ -657,10 +653,19 @@ impl ServerState { }); self.windows.insert( window, - WindowData::new(window, override_redirect, dims, parent, activation_token), + WindowData::new(window, override_redirect, dims, activation_token), ); } + pub fn set_popup(&mut self, window: x::Window) { + let Some(win) = self.windows.get_mut(&window) else { + debug!("not setting popup for unknown window {window:?}"); + return; + }; + + win.attrs.is_popup = true; + } + pub fn set_win_title(&mut self, window: x::Window, name: WmName) { let Some(win) = self.windows.get_mut(&window) else { debug!("not setting title for unknown window {window:?}"); @@ -798,7 +803,7 @@ impl ServerState { return true; }; - !win.mapped || win.attrs.override_redirect + !win.mapped || win.attrs.is_popup } pub fn reconfigure_window(&mut self, event: x::ConfigureNotifyEvent) { @@ -1149,13 +1154,7 @@ impl ServerState { } fn create_role_window(&mut self, window: x::Window, surface_key: ObjectKey) { - // Temporarily remove surface to placate borrow checker - let mut surface: SurfaceData = self.objects[surface_key] - .0 - .take() - .unwrap() - .try_into() - .unwrap(); + let surface: &mut SurfaceData = self.objects.get_mut(surface_key).unwrap().as_mut(); surface.window = Some(window); surface.client.attach(None, 0, 0); surface.client.commit(); @@ -1164,77 +1163,28 @@ impl ServerState { .xdg_wm_base .get_xdg_surface(&surface.client, &self.qh, surface_key); - let window_data = self.windows.get_mut(&window).unwrap(); - if window_data.attrs.override_redirect { - // Override redirect is hard to convert to Wayland! - if let Some(win) = self.last_hovered { - window_data.attrs.popup_for = Some(win); - } else if let Some(win) = self.last_focused_toplevel { - window_data.attrs.popup_for = Some(win); - } + let window = self.windows.get(&window).unwrap(); + let mut popup_for = None; + if window.attrs.is_popup { + popup_for = self.last_hovered.or(self.last_focused_toplevel); } let mut fullscreen = false; - let (width, height) = (window_data.attrs.dims.width, window_data.attrs.dims.height); + let (width, height) = (window.attrs.dims.width, window.attrs.dims.height); for (key, _) in &self.output_keys { let output: &Output = self.objects[key].as_ref(); if output.dimensions.width == width as i32 && output.dimensions.height == height as i32 { fullscreen = true; - window_data.attrs.popup_for = None; + popup_for = None; } } - let surface_id = surface.client.id(); - // Reinsert surface - self.objects[surface_key].0 = Some(surface.into()); - let window = self.windows.get(&window).unwrap(); - let initial_scale; - let role = if let Some(parent) = window.attrs.popup_for { - let parent_window = self.windows.get(&parent).unwrap(); - let parent_surface: &SurfaceData = - self.objects[parent_window.surface_key.unwrap()].as_ref(); - let parent_dims = parent_window.attrs.dims; - - initial_scale = parent_surface.scale_factor; - debug!( - "creating popup ({:?}) {:?} {:?} {:?} {:?} {surface_key:?} (scale: {initial_scale})", - window.window, parent, window.attrs.dims, surface_id, xdg_surface.id() - ); - - let positioner = self.xdg_wm_base.create_positioner(&self.qh, ()); - positioner.set_size( - 1.max((window.attrs.dims.width as f64 / initial_scale) as i32), - 1.max((window.attrs.dims.height as f64 / initial_scale) as i32), - ); - let x = ((window.attrs.dims.x - parent_dims.x) as f64 / initial_scale) as i32; - let y = ((window.attrs.dims.y - parent_dims.y) as f64 / initial_scale) as i32; - positioner.set_offset(x, y); - positioner.set_anchor(Anchor::TopLeft); - positioner.set_gravity(Gravity::BottomRight); - positioner.set_anchor_rect( - 0, - 0, - (parent_window.attrs.dims.width as f64 / initial_scale) as i32, - (parent_window.attrs.dims.height as f64 / initial_scale) as i32, - ); - let popup = xdg_surface.get_popup( - Some(&parent_surface.xdg().unwrap().surface), - &positioner, - &self.qh, - surface_key, - ); - let popup = PopupData { - popup, - positioner, - xdg: XdgSurfaceData { - surface: xdg_surface, - configured: false, - pending: None, - }, - }; - SurfaceRole::Popup(Some(popup)) + let role = if let Some(parent) = popup_for { + let data; + (initial_scale, data) = self.create_popup(window, surface_key, xdg_surface, parent); + SurfaceRole::Popup(Some(data)) } else { initial_scale = 1.0; let data = self.create_toplevel(window, surface_key, xdg_surface, fullscreen); @@ -1330,6 +1280,64 @@ impl ServerState { } } + fn create_popup( + &self, + window: &WindowData, + surface_key: ObjectKey, + xdg: XdgSurface, + parent: x::Window, + ) -> (f64, PopupData) { + let parent_window = self.windows.get(&parent).unwrap(); + let parent_surface: &SurfaceData = + self.objects[parent_window.surface_key.unwrap()].as_ref(); + let parent_dims = parent_window.attrs.dims; + let initial_scale = parent_surface.scale_factor; + + debug!( + "creating popup ({:?}) {:?} {:?} {:?} {surface_key:?} (scale: {initial_scale})", + window.window, + parent, + window.attrs.dims, + xdg.id() + ); + + let positioner = self.xdg_wm_base.create_positioner(&self.qh, ()); + positioner.set_size( + 1.max((window.attrs.dims.width as f64 / initial_scale) as i32), + 1.max((window.attrs.dims.height as f64 / initial_scale) as i32), + ); + let x = ((window.attrs.dims.x - parent_dims.x) as f64 / initial_scale) as i32; + let y = ((window.attrs.dims.y - parent_dims.y) as f64 / initial_scale) as i32; + positioner.set_offset(x, y); + positioner.set_anchor(Anchor::TopLeft); + positioner.set_gravity(Gravity::BottomRight); + positioner.set_anchor_rect( + 0, + 0, + (parent_window.attrs.dims.width as f64 / initial_scale) as i32, + (parent_window.attrs.dims.height as f64 / initial_scale) as i32, + ); + let popup = xdg.get_popup( + Some(&parent_surface.xdg().unwrap().surface), + &positioner, + &self.qh, + surface_key, + ); + + ( + initial_scale, + PopupData { + popup, + positioner, + xdg: XdgSurfaceData { + surface: xdg, + configured: false, + pending: None, + }, + }, + ) + } + fn get_server_surface_from_client( &self, surface: client::wl_surface::WlSurface, diff --git a/src/server/tests.rs b/src/server/tests.rs index baa68f6..98c19ce 100644 --- a/src/server/tests.rs +++ b/src/server/tests.rs @@ -572,17 +572,11 @@ impl TestFixture { .insert(window, data); } - fn new_window( - &mut self, - window: Window, - override_redirect: bool, - data: WindowData, - parent: Option, - ) { + fn new_window(&mut self, window: Window, override_redirect: bool, data: WindowData) { let dims = data.dims; self.register_window(window, data); self.satellite - .new_window(window, override_redirect, dims, parent, None); + .new_window(window, override_redirect, dims, None); } fn map_window( @@ -654,7 +648,7 @@ impl TestFixture { fullscreen: false, }; - self.new_window(window, false, data, None); + self.new_window(window, false, data); self.map_window(comp, window, &surface.obj, &buffer); self.run(); let id = self.check_new_surface(); @@ -724,7 +718,7 @@ impl TestFixture { dims, fullscreen: false, }; - self.new_window(window, true, data, None); + self.new_window(window, true, data); self.map_window(comp, window, &surface.obj, &buffer); self.run(); @@ -1093,7 +1087,7 @@ fn override_redirect_window_after_toplevel_close() { let win2 = unsafe { Window::new(2) }; let (buffer, surface) = comp.create_surface(); - f.new_window(win2, true, WindowData::default(), None); + f.new_window(win2, true, WindowData::default()); f.map_window(&comp, win2, &surface.obj, &buffer); let second = f.check_new_surface(); let data = f.testwl.get_surface_data(second).unwrap(); @@ -1194,7 +1188,6 @@ fn window_group_properties() { ..Default::default() }, None, - None, ); f.satellite .set_win_title(prop_win, WmName::WmName("window".into())); @@ -1214,7 +1207,7 @@ fn window_group_properties() { let (_, surface) = comp.create_surface(); let dims = data.dims; f.register_window(win, data); - f.satellite.new_window(win, false, dims, None, None); + f.satellite.new_window(win, false, dims, None); f.satellite.set_win_hints( win, super::WmHints { @@ -1415,7 +1408,6 @@ fn override_redirect_choose_hover_window() { }, ..Default::default() }, - None, ); f.map_window(&comp, win3, &surface.obj, &buffer); f.run(); @@ -1765,7 +1757,7 @@ fn fullscreen_heuristic() { }, fullscreen: false, }; - f.new_window(window, override_redirect, data, None); + f.new_window(window, override_redirect, data); f.map_window(&comp, window, &surface.obj, &buffer); f.run(); let id = f.check_new_surface(); @@ -2097,7 +2089,7 @@ fn toplevel_size_limits_scaled() { }, fullscreen: false, }; - f.new_window(window, false, data, None); + f.new_window(window, false, data); f.satellite.set_size_hints( window, super::WmNormalHints { diff --git a/src/xstate/mod.rs b/src/xstate/mod.rs index 5c59216..6d61351 100644 --- a/src/xstate/mod.rs +++ b/src/xstate/mod.rs @@ -2,7 +2,7 @@ mod selection; use selection::{Selection, SelectionData}; use wayland_protocols::xdg::decoration::zv1::client::zxdg_toplevel_decoration_v1; -use crate::{server::WindowAttributes, XConnection}; +use crate::XConnection; use bitflags::bitflags; use log::{debug, trace, warn}; use std::collections::HashMap; @@ -301,33 +301,39 @@ impl XState { match event { xcb::Event::X(x::Event::CreateNotify(e)) => { debug!("new window: {:?}", e); - let parent = e.parent(); - let parent = if parent.is_none() || parent == self.root { - None - } else { - Some(parent) - }; server_state.new_window( e.window(), e.override_redirect(), (&e).into(), - parent, self.get_pid(e.window()), ); } xcb::Event::X(x::Event::ReparentNotify(e)) => { debug!("reparent event: {e:?}"); if e.parent() == self.root { + let geometry = self.connection.send_request(&x::GetGeometry { + drawable: x::Drawable::Window(e.window()), + }); + let attrs = self + .connection + .send_request(&x::GetWindowAttributes { window: e.window() }); + let geometry = unwrap_or_skip_bad_window_cont!(self + .connection + .wait_for_reply(geometry)); let attrs = - unwrap_or_skip_bad_window_cont!(self.get_window_attributes(e.window())); + unwrap_or_skip_bad_window_cont!(self.connection.wait_for_reply(attrs)); + server_state.new_window( e.window(), - attrs.override_redirect, - attrs.dims, - None, + attrs.override_redirect(), + WindowDims { + x: geometry.x(), + y: geometry.y(), + width: geometry.width(), + height: geometry.height(), + }, self.get_pid(e.window()), ); - self.handle_window_attributes(server_state, e.window(), attrs); } else { debug!("destroying window since its parent is no longer root!"); server_state.destroy_window(e.window()); @@ -347,9 +353,9 @@ impl XState { value_list: &[x::Cw::EventMask(x::EventMask::PROPERTY_CHANGE)], } )); - let attrs = - unwrap_or_skip_bad_window_cont!(self.get_window_attributes(e.window())); - self.handle_window_attributes(server_state, e.window(), attrs); + unwrap_or_skip_bad_window_cont!( + self.handle_window_properties(server_state, e.window()) + ); server_state.map_window(e.window()); } xcb::Event::X(x::Event::ConfigureNotify(e)) => { @@ -484,68 +490,49 @@ impl XState { } } - fn get_window_attributes(&self, window: x::Window) -> XResult { - let geometry = self.connection.send_request(&x::GetGeometry { - drawable: x::Drawable::Window(window), - }); - let attrs = self - .connection - .send_request(&x::GetWindowAttributes { window }); - + fn handle_window_properties( + &self, + server_state: &mut super::RealServerState, + window: x::Window, + ) -> XResult<()> { let name = self.get_net_wm_name(window); let class = self.get_wm_class(window); - let wm_hints = self.get_wm_hints(window); let size_hints = self.get_wm_size_hints(window); let motif_wm_hints = self.get_motif_wm_hints(window); + let window_state = PropertyCookieWrapper { + connection: &self.connection, + cookie: self.get_property_cookie(window, self.atoms.net_wm_state, x::ATOM_ATOM, 10), + resolver: |reply: x::GetPropertyReply| reply.value::().to_vec(), + }; - let geometry = self.connection.wait_for_reply(geometry)?; - debug!("{window:?} geometry: {geometry:?}"); - let attrs = self.connection.wait_for_reply(attrs)?; let mut title = name.resolve()?; if title.is_none() { title = self.get_wm_name(window).resolve()?; } - let class = class.resolve()?; - let wm_hints = wm_hints.resolve()?; - let size_hints = size_hints.resolve()?; - let motif_wm_hints = motif_wm_hints.resolve()?; - - Ok(WindowAttributes { - override_redirect: attrs.override_redirect(), - popup_for: None, - dims: WindowDims { - x: geometry.x(), - y: geometry.y(), - width: geometry.width(), - height: geometry.height(), - }, - title, - class, - group: wm_hints.and_then(|h| h.window_group), - size_hints, - decorations: motif_wm_hints.and_then(|h| h.decorations), - }) - } - - fn handle_window_attributes( - &self, - server_state: &mut super::RealServerState, - window: x::Window, - attrs: WindowAttributes, - ) { - if let Some(name) = attrs.title { + if let Some(name) = title { server_state.set_win_title(window, name); } - if let Some(class) = attrs.class { + if let Some(class) = class.resolve()? { server_state.set_win_class(window, class); } - if let Some(hints) = attrs.size_hints { + if let Some(hints) = size_hints.resolve()? { server_state.set_size_hints(window, hints); } - if let Some(decorations) = attrs.decorations { + if let Some(decorations) = motif_wm_hints.resolve()?.and_then(|m| m.decorations) { server_state.set_win_decorations(window, decorations); } + + let mut is_popup = false; + if let Some(states) = window_state.resolve()? { + is_popup = states.contains(&self.atoms.skip_taskbar); + } + + if is_popup { + server_state.set_popup(window); + } + + Ok(()) } fn get_property_cookie( @@ -787,6 +774,7 @@ xcb::atoms_struct! { wm_pid => b"_NET_WM_PID" only_if_exists = false, net_wm_state => b"_NET_WM_STATE" only_if_exists = false, wm_fullscreen => b"_NET_WM_STATE_FULLSCREEN" only_if_exists = false, + skip_taskbar => b"_NET_WM_STATE_SKIP_TASKBAR" only_if_exists = false, active_win => b"_NET_ACTIVE_WINDOW" only_if_exists = false, client_list => b"_NET_CLIENT_LIST" only_if_exists = false, supported => b"_NET_SUPPORTED" only_if_exists = false, @@ -803,12 +791,13 @@ xcb::atoms_struct! { } xcb::atoms_struct! { - pub struct WindowTypes { - pub normal => b"_NET_WM_WINDOW_TYPE_NORMAL" only_if_exists = false, - pub dialog => b"_NET_WM_WINDOW_TYPE_DIALOG" only_if_exists = false, - pub splash => b"_NET_WM_WINDOW_TYPE_SPLASH" only_if_exists = false, - pub menu => b"_NET_WM_WINDOW_TYPE_MENU" only_if_exists = false, - pub utility => b"_NET_WM_WINDOW_TYPE_UTILITY" only_if_exists = false, + struct WindowTypes { + ty => b"_NET_WM_WINDOW_TYPE" only_if_exists = false, + normal => b"_NET_WM_WINDOW_TYPE_NORMAL" only_if_exists = false, + dialog => b"_NET_WM_WINDOW_TYPE_DIALOG" only_if_exists = false, + splash => b"_NET_WM_WINDOW_TYPE_SPLASH" only_if_exists = false, + menu => b"_NET_WM_WINDOW_TYPE_MENU" only_if_exists = false, + utility => b"_NET_WM_WINDOW_TYPE_UTILITY" only_if_exists = false, } } diff --git a/tests/integration.rs b/tests/integration.rs index c9fd3ec..449bfd2 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -301,6 +301,9 @@ xcb::atoms_struct! { wm_protocols => b"WM_PROTOCOLS", net_active_window => b"_NET_ACTIVE_WINDOW", wm_delete_window => b"WM_DELETE_WINDOW", + net_wm_state => b"_NET_WM_STATE", + skip_taskbar => b"_NET_WM_STATE_SKIP_TASKBAR", + transient_for => b"WM_TRANSIENT_FOR", clipboard => b"CLIPBOARD", targets => b"TARGETS", multiple => b"MULTIPLE", @@ -1642,3 +1645,21 @@ fn forced_1x_scale_consistent_x11_size() { assert_eq!(geo.width(), 30); assert_eq!(geo.height(), 30); } + +#[test] +fn popup_properties() { + let mut f = Fixture::new(); + let mut connection = Connection::new(&f.display); + + let win_toplevel = connection.new_window(connection.root, 0, 0, 20, 20, false); + f.map_as_toplevel(&mut connection, win_toplevel); + + let win_popup_dialog = connection.new_window(connection.root, 10, 10, 50, 50, false); + connection.set_property( + win_popup_dialog, + x::ATOM_ATOM, + connection.atoms.net_wm_state, + &[connection.atoms.skip_taskbar], + ); + f.map_as_popup(&mut connection, win_popup_dialog); +}