Re-use screencopy session instead of creating one each frame

src/main.rs

@@ -322,7 +322,6 @@ impl App {
         if let Some(sender) = self.wayland_cmd_sender.as_ref() {
             let mut capture_filter = wayland::CaptureFilter::default();
             if self.visible {
-                // XXX handle on wrong connection
                 capture_filter.workspaces_on_outputs =
                     self.outputs.iter().map(|x| x.handle.clone()).collect();
                 capture_filter.toplevels_on_workspaces = self

src/wayland/capture.rs

@@ -5,7 +5,7 @@ use cctk::{
         workspace::v1::client::zcosmic_workspace_handle_v1,
     },
     screencopy::{ScreencopySessionData, ScreencopySessionDataExt},
-    wayland_client::{protocol::wl_output, Proxy, QueueHandle},
+    wayland_client::{protocol::wl_output, Connection, Proxy, QueueHandle},
 };
 use cosmic::cctk;
 
@@ -37,75 +37,119 @@ pub struct Capture {
     pub source: CaptureSource,
     first_frame: AtomicBool,
     running: AtomicBool,
+    capturing: AtomicBool,
+    session: zcosmic_screencopy_session_v1::ZcosmicScreencopySessionV1,
 }
 
 impl Capture {
-    pub fn new(source: CaptureSource) -> Capture {
-        Capture {
-            buffer: Mutex::new(None),
-            source,
-            first_frame: AtomicBool::new(true),
-            running: AtomicBool::new(false),
-        }
+    pub fn new(
+        source: CaptureSource,
+        manager: &zcosmic_screencopy_manager_v1::ZcosmicScreencopyManagerV1,
+        qh: &QueueHandle<AppData>,
+    ) -> Arc<Capture> {
+        Arc::new_cyclic(|weak_capture| {
+            let udata = SessionData {
+                session_data: Default::default(),
+                capture: weak_capture.clone(),
+            };
+
+            let session = match &source {
+                CaptureSource::Toplevel(toplevel) => manager.capture_toplevel(
+                    toplevel,
+                    zcosmic_screencopy_manager_v1::CursorMode::Hidden,
+                    qh,
+                    udata,
+                ),
+                CaptureSource::Workspace(workspace, output) => manager.capture_workspace(
+                    workspace,
+                    output,
+                    zcosmic_screencopy_manager_v1::CursorMode::Hidden,
+                    qh,
+                    udata,
+                ),
+            };
+
+            Capture {
+                buffer: Mutex::new(None),
+                source,
+                first_frame: AtomicBool::new(true),
+                running: AtomicBool::new(false),
+                capturing: AtomicBool::new(false),
+                session,
+            }
+        })
     }
 
-    // Returns `None` if capture is no longer active
+    // Returns `None` if capture is destroyed
     // (or if `session` wasn't created with `SessionData`)
     pub fn for_session(
         session: &zcosmic_screencopy_session_v1::ZcosmicScreencopySessionV1,
     ) -> Option<Arc<Self>> {
-        session
-            .data::<SessionData>()?
-            .capture
-            .upgrade()
-            .filter(|c| c.running())
+        session.data::<SessionData>()?.capture.upgrade()
     }
 
     pub fn running(&self) -> bool {
         self.running.load(Ordering::SeqCst)
     }
 
+    // Buffer is currently attached and commited for capture by server
+    pub fn capturing(&self) -> bool {
+        self.capturing.load(Ordering::SeqCst)
+    }
+
+    pub fn set_capturing(&self, value: bool) {
+        if value {
+            self.first_frame.store(false, Ordering::SeqCst);
+        }
+        self.capturing.store(value, Ordering::SeqCst);
+    }
+
     pub fn first_frame(&self) -> bool {
         self.first_frame.load(Ordering::SeqCst)
     }
 
-    pub fn cancel(&self) {
-        self.running.store(false, Ordering::SeqCst);
-        *self.buffer.lock().unwrap() = None;
+    // Start capturing frames
+    pub fn start(&self, conn: &Connection) {
+        let already_running = self.running.swap(true, Ordering::SeqCst);
+        let have_buffer = self.buffer.lock().unwrap().is_some();
+        if have_buffer && !already_running {
+            self.attach_buffer_and_commit(conn);
+        }
     }
 
-    pub fn capture(
-        self: &Arc<Self>,
-        manager: &zcosmic_screencopy_manager_v1::ZcosmicScreencopyManagerV1,
-        qh: &QueueHandle<AppData>,
-    ) {
-        // Mark as running. If already running, this is not the first frame.
-        let already_running = self.running.swap(true, Ordering::SeqCst);
-        self.first_frame.store(!already_running, Ordering::SeqCst);
+    // Stop capturing. Can be started again with `start`
+    pub fn stop(&self) {
+        self.running.store(false, Ordering::SeqCst);
+        self.first_frame.store(true, Ordering::SeqCst);
+        // TODO: Reallocate buffers on re-start
+        // *self.buffer.lock().unwrap() = None;
+    }
 
-        let udata = SessionData {
-            session_data: Default::default(),
-            capture: Arc::downgrade(self),
-        };
-        match &self.source {
-            CaptureSource::Toplevel(toplevel) => {
-                manager.capture_toplevel(
-                    toplevel,
-                    zcosmic_screencopy_manager_v1::CursorMode::Hidden,
-                    qh,
-                    udata,
-                );
-            }
-            CaptureSource::Workspace(workspace, output) => {
-                manager.capture_workspace(
-                    workspace,
-                    output,
-                    zcosmic_screencopy_manager_v1::CursorMode::Hidden,
-                    qh,
-                    udata,
-                );
-            }
+    pub fn attach_buffer_and_commit(&self, conn: &Connection) {
+        let buffer = self.buffer.lock().unwrap();
+        let buffer = buffer.as_ref().unwrap();
+
+        let node = buffer
+            .node()
+            .and_then(|x| x.to_str().map(|x| x.to_string()));
+
+        self.session.attach_buffer(&buffer.buffer, node, 0); // XXX age?
+        if self.first_frame() {
+            self.session
+                .commit(zcosmic_screencopy_session_v1::Options::empty());
+        } else {
+            self.session
+                .commit(zcosmic_screencopy_session_v1::Options::OnDamage);
         }
+        conn.flush().unwrap();
+
+        self.set_capturing(true);
+    }
+}
+
+impl Drop for Capture {
+    fn drop(&mut self) {
+        self.session.destroy();
     }
 }
 

src/wayland/mod.rs

@@ -1,10 +1,6 @@
-// Workspaces Info, Toplevel Info
-// Capture
-// - subscribe to all workspaces, to start with? All that are associated with an output should be
-// shown on one.
-//   * Need output name to compare?
+// A thread handles screencopy, and other wayland protocols, returning information as a
+// subscription.
 
-// TODO: Way to activate workspace, toplevel? Close? Move?
 use cctk::{
     cosmic_protocols::{
         toplevel_info::v1::client::zcosmic_toplevel_handle_v1,
@@ -99,6 +95,7 @@ pub enum Cmd {
 }
 
 pub struct AppData {
+    conn: Connection,
     qh: QueueHandle<Self>,
     dmabuf_state: DmabufState,
     registry_state: RegistryState,
@@ -151,10 +148,10 @@ impl AppData {
         for (source, capture) in self.captures.borrow_mut().iter_mut() {
             let matches = self.matches_capture_filter(source);
             let running = capture.running();
-            if running && !matches {
-                capture.cancel();
-            } else if !running & matches {
-                capture.capture(&self.screencopy_state.screencopy_manager, &self.qh);
+            if matches {
+                capture.start(&self.conn);
+            } else {
+                capture.stop();
             }
         }
     }
@@ -165,9 +162,10 @@ impl AppData {
             .entry(source.clone())
             .or_insert_with(|| {
                 let matches = self.matches_capture_filter(&source);
-                let capture = Arc::new(Capture::new(source));
+                let capture =
+                    Capture::new(source, &self.screencopy_state.screencopy_manager, &self.qh);
                 if matches {
-                    capture.capture(&self.screencopy_state.screencopy_manager, &self.qh);
+                    capture.start(&self.conn);
                 }
                 capture
             });
@@ -175,7 +173,7 @@ impl AppData {
 
     fn remove_capture_source(&self, source: CaptureSource) {
         if let Some(capture) = self.captures.borrow_mut().remove(&source) {
-            capture.cancel();
+            capture.stop();
         }
     }
 }
@@ -240,6 +238,7 @@ fn start(conn: Connection) -> mpsc::Receiver<Event> {
 
     let registry_state = RegistryState::new(&globals);
     let mut app_data = AppData {
+        conn: conn.clone(),
         qh: qh.clone(),
         dmabuf_state,
         workspace_state: WorkspaceState::new(&registry_state, &qh), // Create before toplevel info state

src/wayland/screencopy.rs

@@ -20,7 +20,6 @@ impl ScreencopyHandler for AppData {
         buffer_infos: &[BufferInfo],
     ) {
         let Some(capture) = Capture::for_session(session) else {
-            session.destroy();
             return;
         };
 
@@ -32,30 +31,26 @@ impl ScreencopyHandler for AppData {
         {
             *buffer = Some(self.create_buffer(buffer_infos));
         }
-        let buffer = buffer.as_ref().unwrap();
 
-        let node = buffer
-            .node()
-            .and_then(|x| x.to_str().map(|x| x.to_string()));
-        session.attach_buffer(&buffer.buffer, node, 0); // XXX age?
-        if capture.first_frame() {
-            session.commit(zcosmic_screencopy_session_v1::Options::empty());
-        } else {
-            session.commit(zcosmic_screencopy_session_v1::Options::OnDamage);
+        drop(buffer);
+
+        if !capture.running() {
+            capture.attach_buffer_and_commit(conn);
         }
-        conn.flush().unwrap();
     }
 
     fn ready(
         &mut self,
-        _conn: &Connection,
+        conn: &Connection,
         _qh: &QueueHandle<Self>,
         session: &zcosmic_screencopy_session_v1::ZcosmicScreencopySessionV1,
     ) {
         let Some(capture) = Capture::for_session(session) else {
-            session.destroy();
             return;
         };
+        if !capture.running() {
+            return;
+        }
 
         let mut buffer = capture.buffer.lock().unwrap();
         if buffer.is_none() {
@@ -76,10 +71,13 @@ impl ScreencopyHandler for AppData {
                 ));
             }
         };
-        session.destroy();
+
+        capture.set_capturing(false);
+
+        drop(buffer);
 
         // Capture again on damage
-        capture.capture(&self.screencopy_state.screencopy_manager, &self.qh);
+        capture.attach_buffer_and_commit(conn);
     }
 
     fn failed(
@@ -92,8 +90,8 @@ impl ScreencopyHandler for AppData {
         // TODO
         println!("Failed");
         if let Some(capture) = Capture::for_session(session) {
-            capture.cancel();
+            capture.set_capturing(false);
+            capture.stop();
         }
-        session.destroy();
     }
 }