refactor(wayland): remove surface specialization from roles

src/wayland/core/surface.rs

@@ -4,14 +4,14 @@ use crate::{
 	core::registry::Registry,
 	nodes::{
 		drawable::model::ModelPart,
-		items::panel::{Geometry, SurfaceId},
+		items::panel::{Geometry, PanelItem, SurfaceId},
 	},
 	wayland::{
 		Message, MessageSink,
 		core::buffer::BufferUsage,
 		presentation::{MonotonicTimestamp, PresentationFeedback},
 		util::{ClientExt, DoubleBuffer},
-		xdg::wm_base::XdgSurfaceRole,
+		xdg::backend::XdgBackend,
 	},
 };
 use bevy::{
@@ -22,7 +22,7 @@ use bevy::{
 use bevy_dmabuf::import::ImportedDmatexs;
 use mint::Vector2;
 use parking_lot::Mutex;
-use std::sync::{Arc, OnceLock};
+use std::sync::{Arc, OnceLock, Weak};
 use waynest::{
 	server::{
 		Client, Dispatcher, Result,
@@ -40,7 +40,8 @@ pub static WL_SURFACE_REGISTRY: Registry<Surface> = Registry::new();
 pub enum SurfaceRole {
 	Cursor,
 	Subsurface,
-	Xdg(OnceLock<XdgSurfaceRole>),
+	XdgToplevel,
+	XdgPopup,
 }
 
 #[derive(Debug, Clone)]
@@ -80,6 +81,7 @@ pub struct Surface {
 	state: Mutex<DoubleBuffer<SurfaceState>>,
 	pub message_sink: MessageSink,
 	pub role: OnceLock<SurfaceRole>,
+	pub panel_item: Mutex<Weak<PanelItem<XdgBackend>>>,
 	on_commit_handlers: Mutex<Vec<OnCommitCallback>>,
 	material: OnceLock<Handle<BevyMaterial>>,
 	pending_material_applications: Registry<ModelPart>,
@@ -111,6 +113,7 @@ impl Surface {
 			state: Default::default(),
 			message_sink: client.message_sink(),
 			role: OnceLock::new(),
+			panel_item: Mutex::new(Weak::default()),
 			on_commit_handlers: Mutex::new(Vec::new()),
 			material: OnceLock::new(),
 			pending_material_applications: Registry::new(),

src/wayland/xdg/surface.rs

@@ -1,11 +1,8 @@
 use super::{popup::Popup, positioner::Positioner, toplevel::MappedInner};
 use crate::wayland::util::ClientExt;
-use crate::wayland::{
+use crate::wayland::{core::surface::SurfaceRole, display::Display, xdg::toplevel::Toplevel};
-	core::surface::SurfaceRole,
+use std::sync::Arc;
-	display::Display,
+use waynest::server;
-	xdg::{toplevel::Toplevel, wm_base::XdgSurfaceRole},
-};
-use std::sync::{Arc, OnceLock};
 pub use waynest::server::protocol::stable::xdg_shell::xdg_surface::*;
 use waynest::{
 	server::{Client, Dispatcher, Result},
@@ -62,62 +59,30 @@ impl XdgSurface for Surface {
 		);
 
 		// Check if the surface already has an XDG role
-		let surface_role = self.wl_surface.role.get().unwrap();
+		match self.wl_surface.role.get() {
-
+			Some(SurfaceRole::XdgToplevel) => (),
-		// Now check if this is an XDG surface and set the sub-role
+			None => {
-		if let SurfaceRole::Xdg(role) = surface_role {
+				let _ = self.wl_surface.role.set(SurfaceRole::XdgToplevel);
-			if role.get().is_some() {
+			}
+			_ => {
 				return client
 					.protocol_error(
 						sender_id,
 						toplevel_id,
 						1, // XDG_WM_BASE_ERROR_ROLE
-						"XDG surface already has a sub-role".to_string(),
+						"Surface has a non-XDG role".to_string(),
 					)
 					.await;
 			}
-
-			if role
-				.set(XdgSurfaceRole::Toplevel(Arc::downgrade(&toplevel)))
-				.is_err()
-			{
-				return client
-					.protocol_error(
-						sender_id,
-						toplevel_id,
-						1, // XDG_WM_BASE_ERROR_ROLE
-						"Failed to set XDG sub-role (race condition)".to_string(),
-					)
-					.await;
-			}
-		} else {
-			return client
-				.protocol_error(
-					sender_id,
-					toplevel_id,
-					1, // XDG_WM_BASE_ERROR_ROLE
-					"Surface has a non-XDG role".to_string(),
-				)
-				.await;
 		}
 
 		toplevel.reconfigure(client).await?;
 
+		let toplevel_weak = Arc::downgrade(&toplevel);
 		let pid = client.get::<Display>(ObjectId::DISPLAY).unwrap().pid;
 		let configured = self.configured.clone();
-		self.wl_surface.add_commit_handler(move |surface, state| {
+		self.wl_surface.add_commit_handler(move |_surface, state| {
-			let Some(role_ref) = surface.role.get() else {
+			let Some(toplevel) = toplevel_weak.upgrade() else {
-				return true;
-			};
-
-			let SurfaceRole::Xdg(role) = role_ref else {
-				return true;
-			};
-
-			let Some(XdgSurfaceRole::Toplevel(toplevel)) = role.get() else {
-				return true;
-			};
-			let Some(toplevel) = toplevel.upgrade() else {
 				return true;
 			};
 
@@ -150,6 +115,23 @@ impl XdgSurface for Surface {
 		parent: Option<ObjectId>,
 		positioner: ObjectId,
 	) -> Result<()> {
+		match self.wl_surface.role.get() {
+			Some(SurfaceRole::XdgPopup) => (),
+			None => {
+				let _ = self.wl_surface.role.set(SurfaceRole::XdgPopup);
+			}
+			_ => {
+				return client
+					.protocol_error(
+						sender_id,
+						popup_id,
+						1, // XDG_WM_BASE_ERROR_ROLE
+						"Surface has an incomparible role".to_string(),
+					)
+					.await;
+			}
+		}
+
 		let Some(parent) = parent else {
 			return client
 				.protocol_error(
@@ -160,49 +142,20 @@ impl XdgSurface for Surface {
 				)
 				.await;
 		};
-		let parent = client.get::<Surface>(parent).unwrap();
+		let Some(parent) = client.get::<Surface>(parent) else {
-		let panel_item = match parent.wl_surface.role.get().unwrap() {
+			return client
-			SurfaceRole::Xdg(role) => match role.get().unwrap() {
+				.protocol_error(
-				XdgSurfaceRole::Toplevel(toplevel) => {
+					sender_id,
-					if let Some(toplevel) = toplevel.upgrade() {
+					popup_id,
-						let toplevel_lock = toplevel.mapped.lock();
+					3, // INVALID_POPUP_PARENT
-						toplevel_lock.as_ref().unwrap()._panel_item.clone()
+					"Parent surface does not exist".to_string(),
-					} else {
+				)
-						return client
+				.await;
-							.protocol_error(
+		};
-								sender_id,
+		let Some(panel_item) = parent.wl_surface.panel_item.lock().upgrade() else {
-								popup_id,
+			return Err(server::Error::Custom(
-								3, // INVALID_POPUP_PARENT
+				"Parent surface does not have a panel item".to_string(),
-								"Parent surface does not have an XDG role".to_string(),
+			));
-							)
-							.await;
-					}
-				}
-				XdgSurfaceRole::Popup(popup) => {
-					if let Some(popup) = popup.upgrade() {
-						popup.panel_item.upgrade().unwrap()
-					} else {
-						return client
-							.protocol_error(
-								sender_id,
-								popup_id,
-								3, // INVALID_POPUP_PARENT
-								"Parent surface does not have an XDG role".to_string(),
-							)
-							.await;
-					}
-				}
-			},
-			_ => {
-				return client
-					.protocol_error(
-						sender_id,
-						popup_id,
-						1, // XDG_WM_BASE_ERROR_ROLE
-						"Parent surface does not have an XDG role".to_string(),
-					)
-					.await;
-			}
 		};
 		let positioner = client.get::<Positioner>(positioner).unwrap();
 
@@ -220,62 +173,6 @@ impl XdgSurface for Surface {
 			),
 		);
 
-		// Set up the XDG role if not already done
-		if self.wl_surface.role.get().is_none() {
-			let xdg_role = SurfaceRole::Xdg(OnceLock::new());
-
-			if self.wl_surface.role.set(xdg_role).is_err() {
-				return client
-					.protocol_error(
-						sender_id,
-						popup_id,
-						1, // XDG_WM_BASE_ERROR_ROLE
-						"Failed to set surface role (race condition)".to_string(),
-					)
-					.await;
-			}
-		}
-
-		// Now check if this is an XDG surface and set the sub-role
-		match self.wl_surface.role.get().unwrap() {
-			SurfaceRole::Xdg(role) => {
-				if role.get().is_some() {
-					return client
-						.protocol_error(
-							sender_id,
-							popup_id,
-							1, // XDG_WM_BASE_ERROR_ROLE
-							"XDG surface already has a sub-role".to_string(),
-						)
-						.await;
-				}
-
-				if role
-					.set(XdgSurfaceRole::Popup(Arc::downgrade(&popup)))
-					.is_err()
-				{
-					return client
-						.protocol_error(
-							sender_id,
-							popup_id,
-							1, // XDG_WM_BASE_ERROR_ROLE
-							"Failed to set XDG sub-role (race condition)".to_string(),
-						)
-						.await;
-				}
-			}
-			_ => {
-				return client
-					.protocol_error(
-						sender_id,
-						popup_id,
-						1, // XDG_WM_BASE_ERROR_ROLE
-						"Surface has a non-XDG role".to_string(),
-					)
-					.await;
-			}
-		}
-
 		let serial = client.next_event_serial();
 		self.configure(client, sender_id, serial).await?;
 

src/wayland/xdg/wm_base.rs

@@ -1,20 +1,11 @@
-use super::popup::Popup;
 use super::positioner::Positioner;
-use super::toplevel::Toplevel;
+use crate::wayland::{core::surface::SurfaceRole, util::ClientExt, xdg::surface::Surface};
-use crate::wayland::{core::surface::SurfaceRole, xdg::surface::Surface};
-use std::sync::{OnceLock, Weak};
 pub use waynest::server::protocol::stable::xdg_shell::xdg_wm_base::*;
 use waynest::{
 	server::{Client, Dispatcher, Result},
 	wire::ObjectId,
 };
 
-#[derive(Debug, Clone)]
-pub enum XdgSurfaceRole {
-	Toplevel(Weak<Toplevel>),
-	Popup(Weak<Popup>),
-}
-
 #[derive(Debug, Dispatcher, Default)]
 pub struct WmBase {
 	pub version: u32,
@@ -37,7 +28,7 @@ impl XdgWmBase for WmBase {
 	async fn get_xdg_surface(
 		&self,
 		client: &mut Client,
-		_sender_id: ObjectId,
+		sender_id: ObjectId,
 		xdg_surface_id: ObjectId,
 		wl_surface_id: ObjectId,
 	) -> Result<()> {
@@ -46,7 +37,20 @@ impl XdgWmBase for WmBase {
 			.ok_or(waynest::server::Error::Custom(
 				"can't get wayland surface id".to_string(),
 			))?;
-		let _ = wl_surface.role.set(SurfaceRole::Xdg(OnceLock::new()));
+		match wl_surface.role.get() {
+			Some(SurfaceRole::XdgToplevel | SurfaceRole::XdgPopup) => (),
+			None => (),
+			_ => {
+				client
+					.protocol_error(
+						sender_id,
+						xdg_surface_id,
+						0,
+						"invalid surface role".to_string(),
+					)
+					.await?
+			}
+		};
 		let xdg_surface = Surface::new(xdg_surface_id, self.version, wl_surface);
 		client.insert(xdg_surface_id, xdg_surface);