WayVR: Process cleanup support, Refactoring

src/backend/wayvr/client.rs

@@ -9,13 +9,13 @@ use smithay::{
 
 use super::{
     comp::{self},
-    display,
+    display, process,
 };
 
 pub struct WayVRClient {
     pub client: wayland_server::Client,
     pub display_handle: display::DisplayHandle,
-    pub pid: i32,
+    pub pid: u32,
 }
 
 pub struct WayVRManager {
@@ -28,6 +28,8 @@ pub struct WayVRManager {
     display: wayland_server::Display<comp::Application>,
     listener: wayland_server::ListeningSocket,
 
+    toplevel_surf_count: u32, // for logging purposes
+
     pub clients: Vec<WayVRClient>,
 }
 
@@ -67,13 +69,15 @@ impl WayVRManager {
             wayland_env,
             serial_counter: SerialCounter::new(),
             clients: Vec::new(),
+            toplevel_surf_count: 0,
         })
     }
 
     fn accept_connection(
         &mut self,
         stream: UnixStream,
         displays: &mut display::DisplayVec,
+        processes: &mut process::ProcessVec,
     ) -> anyhow::Result<()> {
         let client = self
             .display
@@ -84,43 +88,59 @@ impl WayVRManager {
         let creds = client.get_credentials(&self.display.handle())?;
         let auth_key = get_display_auth_from_pid(creds.pid)?;
 
-        for (idx, cell) in displays.vec.iter().enumerate() {
-            if let Some(cell) = &cell {
-                let display = &cell.obj;
-                if display.auth_key_matches(auth_key.as_str()) {
-                    let display_handle = display::DisplayVec::get_handle(cell, idx);
+        // Find suitable auth key from the process list
+        for process in processes.vec.iter().flatten() {
+            let process = &process.obj;
 
+            // Find process with matching auth key
+            if process.auth_key.as_str() == auth_key {
+                // Check if display handle is valid
+                if displays.get(&process.display_handle).is_some() {
+                    // Add client
                     self.clients.push(WayVRClient {
                         client,
-                        display_handle,
-                        pid: creds.pid,
+                        display_handle: process.display_handle,
+                        pid: creds.pid as u32,
                     });
                     return Ok(());
                 }
             }
         }
-
         anyhow::bail!("Process auth key is invalid or selected display is non-existent");
     }
 
-    fn accept_connections(&mut self, displays: &mut display::DisplayVec) -> anyhow::Result<()> {
+    fn accept_connections(
+        &mut self,
+        displays: &mut display::DisplayVec,
+        processes: &mut process::ProcessVec,
+    ) -> anyhow::Result<()> {
         if let Some(stream) = self.listener.accept()? {
-            if let Err(e) = self.accept_connection(stream, displays) {
+            if let Err(e) = self.accept_connection(stream, displays, processes) {
                 log::error!("Failed to accept connection: {}", e);
             }
         }
 
         Ok(())
     }
 
-    pub fn tick_wayland(&mut self, displays: &mut display::DisplayVec) -> anyhow::Result<()> {
-        if let Err(e) = self.accept_connections(displays) {
+    pub fn tick_wayland(
+        &mut self,
+        displays: &mut display::DisplayVec,
+        processes: &mut process::ProcessVec,
+    ) -> anyhow::Result<()> {
+        if let Err(e) = self.accept_connections(displays, processes) {
             log::error!("accept_connections failed: {}", e);
         }
 
         self.display.dispatch_clients(&mut self.state)?;
         self.display.flush_clients()?;
 
+        let surf_count = self.state.xdg_shell.toplevel_surfaces().len() as u32;
+        if surf_count != self.toplevel_surf_count {
+            self.toplevel_surf_count = surf_count;
+            log::info!("Toplevel surface count changed: {}", surf_count);
+        }
+
         Ok(())
     }
 

src/backend/wayvr/comp.rs

@@ -27,6 +27,7 @@ use wayland_server::protocol::wl_surface::WlSurface;
 use wayland_server::Client;
 
 use super::event_queue::SyncEventQueue;
+use super::WayVRTask;
 
 pub struct Application {
     pub compositor: compositor::CompositorState,
@@ -35,7 +36,7 @@ pub struct Application {
     pub shm: ShmState,
     pub data_device: DataDeviceState,
 
-    pub queue_new_toplevel: SyncEventQueue<(ClientId, ToplevelSurface)>,
+    pub wayvr_tasks: SyncEventQueue<WayVRTask>,
 }
 
 impl compositor::CompositorHandler for Application {
@@ -125,7 +126,8 @@ impl XdgShellHandler for Application {
 
     fn new_toplevel(&mut self, surface: ToplevelSurface) {
         if let Some(client) = surface.wl_surface().client() {
-            self.queue_new_toplevel.send((client.id(), surface.clone()));
+            self.wayvr_tasks
+                .send(WayVRTask::NewToplevel(client.id(), surface.clone()));
         }
         surface.with_pending_state(|state| {
             state.states.set(xdg_toplevel::State::Activated);

src/backend/wayvr/display.rs

@@ -18,28 +18,28 @@ use smithay::{
 use crate::{backend::overlay::OverlayID, gen_id};
 
 use super::{
-    client::WayVRManager, comp::send_frames_surface_tree, egl_data, smithay_wrapper, window,
+    client::WayVRManager, comp::send_frames_surface_tree, egl_data, event_queue::SyncEventQueue,
+    process, smithay_wrapper, window,
 };
 
 fn generate_auth_key() -> String {
     let uuid = uuid::Uuid::new_v4();
     uuid.to_string()
 }
 
-struct Process {
-    auth_key: String,
-    child: std::process::Child,
+struct DisplayWindow {
+    window_handle: window::WindowHandle,
+    process_handle: process::ProcessHandle,
+    toplevel: ToplevelSurface,
 }
 
-impl Drop for Process {
-    fn drop(&mut self) {
-        let _dont_care = self.child.kill();
-    }
+pub struct SpawnProcessResult {
+    pub auth_key: String,
+    pub child: std::process::Child,
 }
 
-struct DisplayWindow {
-    handle: window::WindowHandle,
-    toplevel: ToplevelSurface,
+pub enum DisplayTask {
+    ProcessCleanup(process::ProcessHandle),
 }
 
 pub struct Display {
@@ -59,7 +59,7 @@ pub struct Display {
     egl_data: Rc<egl_data::EGLData>,
     pub dmabuf_data: egl_data::DMAbufData,
 
-    processes: Vec<Process>,
+    pub tasks: SyncEventQueue<DisplayTask>,
 }
 
 impl Drop for Display {
@@ -113,25 +113,22 @@ impl Display {
             egl_image,
             gles_texture,
             wayland_env,
-            processes: Vec::new(),
             visible: true,
             overlay_id: None,
+            tasks: SyncEventQueue::new(),
         })
     }
 
-    pub fn auth_key_matches(&self, auth_key: &str) -> bool {
-        for process in &self.processes {
-            if process.auth_key.as_str() == auth_key {
-                return true;
-            }
-        }
-        false
-    }
-
-    pub fn add_window(&mut self, window_handle: window::WindowHandle, toplevel: &ToplevelSurface) {
+    pub fn add_window(
+        &mut self,
+        window_handle: window::WindowHandle,
+        process_handle: process::ProcessHandle,
+        toplevel: &ToplevelSurface,
+    ) {
         log::debug!("Attaching toplevel surface into display");
         self.displayed_windows.push(DisplayWindow {
-            handle: window_handle,
+            window_handle,
+            process_handle,
             toplevel: toplevel.clone(),
         });
         self.reposition_windows();
@@ -141,7 +138,7 @@ impl Display {
         let window_count = self.displayed_windows.len();
 
         for (i, win) in self.displayed_windows.iter_mut().enumerate() {
-            if let Some(window) = self.wm.borrow_mut().windows.get_mut(&win.handle) {
+            if let Some(window) = self.wm.borrow_mut().windows.get_mut(&win.window_handle) {
                 let d_cur = i as f32 / window_count as f32;
                 let d_next = (i + 1) as f32 / window_count as f32;
 
@@ -154,6 +151,24 @@ impl Display {
         }
     }
 
+    pub fn tick(&mut self) {
+        while let Some(task) = self.tasks.read() {
+            match task {
+                DisplayTask::ProcessCleanup(process_handle) => {
+                    self.displayed_windows
+                        .retain(|win| win.process_handle != process_handle);
+                    log::info!(
+                        "Cleanup finished for display \"{}\". Current window count: {}",
+                        self.name,
+                        self.displayed_windows.len()
+                    );
+
+                    self.reposition_windows();
+                }
+            }
+        }
+    }
+
     pub fn tick_render(&self, renderer: &mut GlesRenderer, time_ms: u64) -> anyhow::Result<()> {
         renderer.bind(self.gles_texture.clone())?;
 
@@ -166,7 +181,7 @@ impl Display {
             .iter()
             .flat_map(|display_window| {
                 let wm = self.wm.borrow_mut();
-                if let Some(window) = wm.windows.get(&display_window.handle) {
+                if let Some(window) = wm.windows.get(&display_window.window_handle) {
                     render_elements_from_surface_tree(
                         renderer,
                         display_window.toplevel.wl_surface(),
@@ -207,13 +222,13 @@ impl Display {
         let wm = self.wm.borrow();
 
         for cell in self.displayed_windows.iter() {
-            if let Some(window) = wm.windows.get(&cell.handle) {
+            if let Some(window) = wm.windows.get(&cell.window_handle) {
                 if (cursor_x as i32) >= window.pos_x
                     && (cursor_x as i32) < window.pos_x + window.size_x as i32
                     && (cursor_y as i32) >= window.pos_y
                     && (cursor_y as i32) < window.pos_y + window.size_y as i32
                 {
-                    return Some(cell.handle);
+                    return Some(cell.window_handle);
                 }
             }
         }
@@ -335,7 +350,7 @@ impl Display {
         exec_path: &str,
         args: &[&str],
         env: &[(&str, &str)],
-    ) -> anyhow::Result<()> {
+    ) -> anyhow::Result<SpawnProcessResult> {
         log::info!("Spawning subprocess with exec path \"{}\"", exec_path);
 
         let auth_key = generate_auth_key();
@@ -349,9 +364,7 @@ impl Display {
         }
 
         match cmd.spawn() {
-            Ok(child) => {
-                self.processes.push(Process { child, auth_key });
-            }
+            Ok(child) => Ok(SpawnProcessResult { auth_key, child }),
             Err(e) => {
                 anyhow::bail!(
 					"Failed to launch process with path \"{}\": {}. Make sure your exec path exists.",
@@ -360,8 +373,6 @@ impl Display {
 				);
             }
         }
-
-        Ok(())
     }
 }
 

src/backend/wayvr/egl_ex.rs

@@ -1,32 +1,34 @@
+#![allow(clippy::all)]
+
 //eglExportDMABUFImageMESA
 pub type PFNEGLEXPORTDMABUFIMAGEMESAPROC = Option<
-	unsafe extern "C" fn(
-		dpy: khronos_egl::EGLDisplay,
-		image: khronos_egl::EGLImage,
-		fds: *mut i32,
-		strides: *mut khronos_egl::Int,
-		offsets: *mut khronos_egl::Int,
-	) -> khronos_egl::Boolean,
+    unsafe extern "C" fn(
+        dpy: khronos_egl::EGLDisplay,
+        image: khronos_egl::EGLImage,
+        fds: *mut i32,
+        strides: *mut khronos_egl::Int,
+        offsets: *mut khronos_egl::Int,
+    ) -> khronos_egl::Boolean,
 >;
 
 //eglQueryDmaBufModifiersEXT
 pub type PFNEGLQUERYDMABUFMODIFIERSEXTPROC = Option<
-	unsafe extern "C" fn(
-		dpy: khronos_egl::EGLDisplay,
-		format: khronos_egl::Int,
-		max_modifiers: khronos_egl::Int,
-		modifiers: *mut u64,
-		external_only: *mut khronos_egl::Boolean,
-		num_modifiers: *mut khronos_egl::Int,
-	) -> khronos_egl::Boolean,
+    unsafe extern "C" fn(
+        dpy: khronos_egl::EGLDisplay,
+        format: khronos_egl::Int,
+        max_modifiers: khronos_egl::Int,
+        modifiers: *mut u64,
+        external_only: *mut khronos_egl::Boolean,
+        num_modifiers: *mut khronos_egl::Int,
+    ) -> khronos_egl::Boolean,
 >;
 
 //eglQueryDmaBufFormatsEXT
 pub type PFNEGLQUERYDMABUFFORMATSEXTPROC = Option<
-	unsafe extern "C" fn(
-		dpy: khronos_egl::EGLDisplay,
-		max_formats: khronos_egl::Int,
-		formats: *mut khronos_egl::Int,
-		num_formats: *mut khronos_egl::Int,
-	) -> khronos_egl::Boolean,
+    unsafe extern "C" fn(
+        dpy: khronos_egl::EGLDisplay,
+        max_formats: khronos_egl::Int,
+        formats: *mut khronos_egl::Int,
+        num_formats: *mut khronos_egl::Int,
+    ) -> khronos_egl::Boolean,
 >;