fix(wayland): popups don't crash anymore

Cargo.lock

@@ -1339,7 +1339,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "e310b3a6b5907f99202fcdb4960ff45b93735d7c7d96b760fcff8db2dc0e103d"
 dependencies = [
  "cfg-if",
- "windows-targets 0.48.5",
+ "windows-targets 0.52.6",
 ]
 
 [[package]]
@@ -2285,7 +2285,7 @@ checksum = "3c5e1a9a646d36c3599cd173a41282daf47c44583ad367b8e6837255952e5c67"
 [[package]]
 name = "smithay"
 version = "0.3.0"
-source = "git+https://github.com/smithay/smithay.git#f208cd758416e4495e9eb8b27a96c523e92817a6"
+source = "git+https://github.com/smithay/smithay.git#c6aab182a3c9f106d9c7a0ea34187f90403e59e7"
 dependencies = [
  "appendlist",
  "bitflags 2.6.0",

src/nodes/items/panel.rs

@@ -110,12 +110,6 @@ impl<B: Backend> PanelItem<B> {
 
 		(node, panel_item)
 	}
-	pub fn drop_toplevel(&self) {
-		let Some(node) = self.node.upgrade() else {
-			return;
-		};
-		node.destroy();
-	}
 }
 
 // Remote signals

src/wayland/compositor.rs

@@ -33,9 +33,6 @@ impl CompositorHandler for WaylandState {
 
 			data.data_map.get::<Arc<CoreSurface>>().cloned()
 		});
-		if let Some(core_surface) = CoreSurface::from_wl_surface(surface) {
-			core_surface.commit(count);
-		}
 	}
 
 	fn client_compositor_state<'a>(&self, client: &'a Client) -> &'a CompositorClientState {

src/wayland/seat.rs

@@ -39,7 +39,7 @@ impl SeatHandler for WaylandState {
 		self.seat.cursor_info_tx.send_modify(|c| match image {
 			CursorImageStatus::Hidden => c.surface = None,
 			CursorImageStatus::Surface(surface) => {
-				CoreSurface::add_to(&surface, || (), |_| ());
+				CoreSurface::add_to(&surface);
 				compositor::with_states(&surface, |data| {
 					if let Some(core_surface) = data.data_map.get::<Arc<CoreSurface>>() {
 						core_surface.set_material_offset(1);

src/wayland/surface.rs

@@ -1,4 +1,4 @@
-use super::utils;
+use super::utils::WlSurfaceExt;
 use crate::{
 	core::{delta::Delta, destroy_queue, registry::Registry},
 	nodes::{
@@ -50,41 +50,24 @@ pub struct CoreSurface {
 	sk_tex: OnceCell<Mutex<TexWrapper>>,
 	sk_mat: OnceCell<Mutex<MaterialWrapper>>,
 	material_offset: Mutex<Delta<u32>>,
-	on_mapped: Mutex<Box<dyn Fn() + Send + Sync>>,
-	on_commit: Mutex<Box<dyn Fn(u32) + Send + Sync>>,
 	pub pending_material_applications: Registry<ModelPart>,
 }
 
 impl CoreSurface {
-	pub fn add_to(
+	pub fn add_to(surface: &WlSurface) {
-		surface: &WlSurface,
-		on_mapped: impl Fn() + Send + Sync + 'static,
-		on_commit: impl Fn(u32) + Send + Sync + 'static,
-	) {
 		let core_surface = CORE_SURFACES.add(CoreSurface {
 			weak_surface: surface.downgrade(),
 			mapped_data: Mutex::new(None),
 			sk_tex: OnceCell::new(),
 			sk_mat: OnceCell::new(),
 			material_offset: Mutex::new(Delta::new(0)),
-			on_mapped: Mutex::new(Box::new(on_mapped) as Box<dyn Fn() + Send + Sync>),
-			on_commit: Mutex::new(Box::new(on_commit) as Box<dyn Fn(u32) + Send + Sync>),
 			pending_material_applications: Registry::new(),
 		});
-		utils::insert_data_raw(surface, core_surface);
+		surface.insert_data(core_surface);
-	}
-
-	pub fn commit(&self, count: u32) {
-		(*self.on_commit.lock())(count);
 	}
 
 	pub fn from_wl_surface(surf: &WlSurface) -> Option<Arc<CoreSurface>> {
-		utils::get_data(surf)
+		surf.get_data()
-	}
-
-	pub fn decycle(&self) {
-		*self.on_mapped.lock() = Box::new(|| {});
-		*self.on_commit.lock() = Box::new(|_| {});
 	}
 
 	pub fn process(&self, renderer: &mut GlesRenderer) {
@@ -128,14 +111,12 @@ impl CoreSurface {
 		}
 
 		let mut mapped_data = self.mapped_data.lock();
-		let just_mapped = mapped_data.is_none();
 		self.with_states(|data| {
 			let Some(renderer_surface_state) = data
 				.data_map
 				.get::<RendererSurfaceStateUserData>()
 				.map(std::sync::Mutex::lock)
-				.map(Result::ok)
+				.and_then(Result::ok)
-				.flatten()
 			else {
 				return;
 			};
@@ -181,9 +162,6 @@ impl CoreSurface {
 			*mapped_data = Some(new_mapped_data);
 		});
 		drop(mapped_data);
-		if just_mapped {
-			(*self.on_mapped.lock())();
-		}
 		self.apply_surface_materials();
 	}
 

src/wayland/utils.rs

@@ -1,14 +1,20 @@
 use smithay::{reexports::wayland_server::protocol::wl_surface::WlSurface, wayland::compositor};
-use std::sync::Arc;
 
-pub fn insert_data<T: Send + Sync + 'static>(wl_surface: &WlSurface, data: T) {
+pub trait WlSurfaceExt {
-	insert_data_raw(wl_surface, Arc::new(data))
+	fn insert_data<T: Send + Sync + 'static>(&self, data: T) -> bool;
+	fn get_data<T: Send + Sync + Clone + 'static>(&self) -> Option<T>;
+	fn get_data_raw<T: Send + Sync + 'static, O, F: FnOnce(&T) -> O>(&self, f: F) -> Option<O>;
 }
-pub fn insert_data_raw<T: Send + Sync + 'static>(wl_surface: &WlSurface, data: Arc<T>) {
+impl WlSurfaceExt for WlSurface {
-	compositor::with_states(wl_surface, |d| {
+	fn insert_data<T: Send + Sync + 'static>(&self, data: T) -> bool {
-		d.data_map.insert_if_missing_threadsafe(move || data)
+		compositor::with_states(self, |d| {
-	});
+			d.data_map.insert_if_missing_threadsafe(move || data)
-}
+		})
-pub fn get_data<T: Send + Sync + 'static>(wl_surface: &WlSurface) -> Option<Arc<T>> {
+	}
-	compositor::with_states(wl_surface, |d| d.data_map.get::<Arc<T>>().cloned())
+	fn get_data<T: Send + Sync + Clone + 'static>(&self) -> Option<T> {
+		compositor::with_states(self, |d| d.data_map.get::<T>().cloned())
+	}
+	fn get_data_raw<T: Send + Sync + 'static, O, F: FnOnce(&T) -> O>(&self, f: F) -> Option<O> {
+		compositor::with_states(self, |d| Some((f)(d.data_map.get::<T>()?)))
+	}
 }

src/wayland/xdg_shell.rs

@@ -2,7 +2,7 @@ use super::{
 	seat::{handle_cursor, SeatWrapper},
 	state::{ClientState, WaylandState},
 	surface::CoreSurface,
-	utils,
+	utils::WlSurfaceExt,
 };
 use crate::nodes::{
 	drawable::model::ModelPart,
@@ -29,7 +29,7 @@ use smithay::{
 	},
 	utils::{Logical, Rectangle, Serial},
 	wayland::{
-		compositor,
+		compositor::{self, add_post_commit_hook},
 		shell::xdg::{
 			PopupSurface, PositionerState, ShellClient, SurfaceCachedState, ToplevelSurface,
 			XdgShellHandler, XdgShellState, XdgToplevelSurfaceData,
@@ -49,20 +49,12 @@ impl From<Rectangle<i32, Logical>> for Geometry {
 }
 
 fn surface_panel_item(wl_surface: &WlSurface) -> Option<Arc<PanelItem<XdgBackend>>> {
-	let panel_item = utils::get_data::<Weak<PanelItem<XdgBackend>>>(wl_surface)
+	let panel_item = wl_surface
-		.as_deref()
+		.get_data::<Weak<PanelItem<XdgBackend>>>()
+		.as_ref()
 		.and_then(Weak::upgrade);
 	if panel_item.is_none() {
 		warn!("Couldn't get panel item");
-		// println!("panel item not found at \n{}\n\n", {
-		// 	let backtrace = Backtrace::force_capture().to_string();
-		// 	let mut split_backtrace = backtrace
-		// 		.split('\n')
-		// 		.map(|s| s.to_string())
-		// 		.collect::<Vec<_>>();
-		// 	split_backtrace.resize(4, "".to_string());
-		// 	split_backtrace.join("\n")
-		// });
 	}
 	panel_item
 }
@@ -76,6 +68,7 @@ impl XdgShellHandler for WaylandState {
 	fn client_destroyed(&mut self, _client: ShellClient) {}
 
 	fn new_toplevel(&mut self, toplevel: ToplevelSurface) {
+		toplevel.wl_surface().insert_data(SurfaceId::Toplevel(()));
 		toplevel.with_pending_state(|s| {
 			s.decoration_mode = Some(Mode::ServerSide);
 			s.states.set(State::TiledTop);
@@ -86,44 +79,50 @@ impl XdgShellHandler for WaylandState {
 			s.states.unset(State::Fullscreen);
 		});
 		toplevel.send_configure();
-		utils::insert_data(toplevel.wl_surface(), SurfaceId::Toplevel);
+		toplevel
-		utils::insert_data(toplevel.wl_surface(), Mutex::new(Vector2::from([0_u32; 2])));
+			.wl_surface()
-		CoreSurface::add_to(
+			.insert_data(Mutex::new(Vector2::from([0_u32; 2])));
+
+		CoreSurface::add_to(toplevel.wl_surface());
+		add_post_commit_hook(
 			toplevel.wl_surface(),
-			{
+			|state: &mut WaylandState, _dh, surf| {
-				let toplevel = toplevel.clone();
+				if surface_panel_item(surf).is_some() {
-				move || {
+					return;
-					let wl_surface = toplevel.wl_surface().client().unwrap();
-					let client_state = wl_surface.get_data::<ClientState>().unwrap();
-					let (node, panel_item) = PanelItem::create(
-						Box::new(XdgBackend::create(
-							toplevel.clone(),
-							client_state.seat.clone(),
-						)),
-						client_state.pid,
-					);
-					handle_cursor(&panel_item, panel_item.backend.seat.cursor_info_rx.clone());
-					utils::insert_data(toplevel.wl_surface(), Arc::downgrade(&panel_item));
-					utils::insert_data_raw(toplevel.wl_surface(), node);
 				}
+				let client = surf.client().unwrap();
+				let client_state = client.get_data::<ClientState>().unwrap();
+				let Some(toplevel) = state
+					.xdg_shell
+					.toplevel_surfaces()
+					.iter()
+					.find(|s| s.wl_surface() == surf)
+				else {
+					return;
+				};
+				let (node, panel_item) = PanelItem::create(
+					Box::new(XdgBackend::create(
+						toplevel.clone(),
+						client_state.seat.clone(),
+					)),
+					client_state.pid,
+				);
+				handle_cursor(&panel_item, panel_item.backend.seat.cursor_info_rx.clone());
+				surf.insert_data(Arc::downgrade(&panel_item));
+				surf.insert_data(node);
 			},
-			{
+		);
-				let toplevel = toplevel.clone();
+
-				move |_c| {
+		add_post_commit_hook(
-					let Some(panel_item) = surface_panel_item(toplevel.wl_surface()) else {
+			toplevel.wl_surface(),
-						// if the wayland toplevel isn't mapped, hammer it again with a configure until it cooperates
+			|_state: &mut WaylandState, _dh, surf| {
-						toplevel.send_configure();
+				let Some(panel_item) = surface_panel_item(surf) else {
-						return;
+					return;
-					};
+				};
-					let Some(core_surface) = CoreSurface::from_wl_surface(toplevel.wl_surface())
+				let Some(core_surface) = CoreSurface::from_wl_surface(surf) else {
-					else {
+					return;
-						return;
+				};
-					};
+				surf.get_data_raw::<Mutex<Vector2<u32>>, _, _>(|old_size| {
-					let Some(old_size) =
-						utils::get_data::<Mutex<Vector2<u32>>>(toplevel.wl_surface())
-					else {
-						return;
-					};
 					let mut old_size = old_size.lock();
 					let Some(size) = core_surface.size() else {
 						return;
@@ -132,94 +131,79 @@ impl XdgShellHandler for WaylandState {
 						panel_item.toplevel_size_changed(size);
 						*old_size = size;
 					}
-				}
+				});
 			},
 		);
 	}
 	fn toplevel_destroyed(&mut self, toplevel: ToplevelSurface) {
-		if let Some(core_surface) = CoreSurface::from_wl_surface(toplevel.wl_surface()) {
-			core_surface.decycle();
-		}
 		let Some(panel_item) = surface_panel_item(toplevel.wl_surface()) else {
 			return;
 		};
 		panel_item.backend.seat.unfocus(toplevel.wl_surface(), self);
 		panel_item.backend.toplevel.lock().take();
 		panel_item.backend.popups.lock().clear();
-		panel_item.drop_toplevel();
-		// println!(
-		// 	"Dropping toplevel resulted in {} references",
-		// 	Arc::strong_count(&panel_item)
-		// );
 	}
 	fn app_id_changed(&mut self, toplevel: ToplevelSurface) {
 		let Some(panel_item) = surface_panel_item(toplevel.wl_surface()) else {
 			return;
 		};
 
-		panel_item.toplevel_app_id_changed(&compositor::with_states(
+		panel_item.toplevel_app_id_changed(
-			toplevel.wl_surface(),
+			&toplevel
-			|states| {
+				.wl_surface()
-				states
+				.get_data_raw::<XdgToplevelSurfaceData, _, _>(|d| {
-					.data_map
+					d.lock().unwrap().app_id.clone().unwrap()
-					.get::<XdgToplevelSurfaceData>()
+				})
-					.unwrap()
+				.unwrap_or_default(),
-					.lock()
+		)
-					.unwrap()
-					.app_id
-					.clone()
-					.unwrap()
-			},
-		))
 	}
 	fn title_changed(&mut self, toplevel: ToplevelSurface) {
 		let Some(panel_item) = surface_panel_item(toplevel.wl_surface()) else {
 			return;
 		};
 
-		panel_item.toplevel_title_changed(&compositor::with_states(
+		panel_item.toplevel_title_changed(
-			toplevel.wl_surface(),
+			&toplevel
-			|states| {
+				.wl_surface()
-				states
+				.get_data_raw::<XdgToplevelSurfaceData, _, _>(|d| {
-					.data_map
+					d.lock().unwrap().title.clone().unwrap()
-					.get::<XdgToplevelSurfaceData>()
+				})
-					.unwrap()
+				.unwrap_or_default(),
-					.lock()
+		)
-					.unwrap()
-					.title
-					.clone()
-					.unwrap()
-			},
-		))
 	}
 
 	fn new_popup(&mut self, popup: PopupSurface, positioner: PositionerState) {
 		let uid = rand::thread_rng().gen_range(0..u64::MAX);
+		popup.wl_surface().insert_data(SurfaceId::Child(uid));
 		let Some(parent) = popup.get_parent_surface() else {
 			return;
 		};
+		let _ = popup.send_configure();
+		CoreSurface::add_to(popup.wl_surface());
+
 		let Some(panel_item) = surface_panel_item(&parent) else {
 			return;
 		};
-		let _ = popup.send_configure();
+		let panel_item_weak = Arc::downgrade(&panel_item);
-		utils::insert_data(popup.wl_surface(), SurfaceId::Child(uid));
+		add_post_commit_hook(
-		utils::insert_data(popup.wl_surface(), Arc::downgrade(&panel_item));
-		CoreSurface::add_to(
 			popup.wl_surface(),
-			{
+			move |state: &mut WaylandState, _dh, surf| {
-				let popup = popup.clone();
+				if surface_panel_item(surf).is_some() {
-				move || {
+					return;
-					panel_item.backend.new_popup(uid, popup.clone(), positioner);
-				}
-			},
-			{
-				let popup = popup.clone();
-				move |_c| {
-					if surface_panel_item(popup.wl_surface()).is_none() {
-						// if the popup toplevel isn't mapped, hammer it again with a configure until it cooperates
-						let _ = popup.send_configure();
-					};
 				}
+				surf.insert_data(panel_item_weak.clone());
+				let Some(panel) = surface_panel_item(surf) else {
+					return;
+				};
+				let Some(popup) = state
+					.xdg_shell
+					.popup_surfaces()
+					.iter()
+					.find(|p| p.wl_surface() == surf)
+				else {
+					return;
+				};
+				panel.backend.new_popup(uid, popup.clone(), positioner);
 			},
 		);
 	}
@@ -232,26 +216,17 @@ impl XdgShellHandler for WaylandState {
 		let Some(panel_item) = surface_panel_item(popup.wl_surface()) else {
 			return;
 		};
-		let Some(SurfaceId::Child(uid)) = utils::get_data::<SurfaceId>(popup.wl_surface())
+		let Some(SurfaceId::Child(uid)) = popup.wl_surface().get_data::<SurfaceId>() else {
-			.as_deref()
-			.cloned()
-		else {
 			return;
 		};
 
 		panel_item.backend.reposition_popup(uid, popup, positioner)
 	}
 	fn popup_destroyed(&mut self, popup: PopupSurface) {
-		if let Some(core_surface) = CoreSurface::from_wl_surface(popup.wl_surface()) {
-			core_surface.decycle();
-		}
 		let Some(panel_item) = surface_panel_item(popup.wl_surface()) else {
 			return;
 		};
-		let Some(SurfaceId::Child(uid)) = utils::get_data::<SurfaceId>(popup.wl_surface())
+		let Some(SurfaceId::Child(uid)) = popup.wl_surface().get_data::<SurfaceId>() else {
-			.as_deref()
-			.cloned()
-		else {
 			return;
 		};
 		panel_item.backend.seat.unfocus(popup.wl_surface(), self);
@@ -375,11 +350,9 @@ impl XdgBackend {
 	}
 
 	fn child_data(&self, id: u64) -> Option<ChildInfo> {
-		let (popup, positioner) = self.popups.lock().get(&id)?.clone();
+		let (popup, positioner) = self.popups.lock().get(&id).unwrap().clone();
-		let parent_surface = popup.get_parent_surface()?;
+		let parent = popup.get_parent_surface().unwrap();
-		let parent = utils::get_data::<SurfaceId>(&parent_surface)?
+		let parent = parent.get_data::<SurfaceId>().unwrap();
-			.as_ref()
-			.clone();
 		Some(ChildInfo {
 			id,
 			parent,
@@ -426,11 +399,7 @@ impl Backend for XdgBackend {
 				.clone()
 		});
 		let toplevel_cached_state = compositor::with_states(toplevel.wl_surface(), |states| {
-			states
+			*states.cached_state.get::<SurfaceCachedState>().current()
-				.cached_state
-				.get::<SurfaceCachedState>()
-				.current()
-				.clone()
 		});
 		let toplevel_core_surface = CoreSurface::from_wl_surface(toplevel.wl_surface()).unwrap();