Merge pull request #7 from pop-os/dmabuf-subsurfaces_jammy

Screencopy to dmabuf presented using subsurfaces

src/view/mod.rs

@@ -14,6 +14,7 @@ use cosmic::{
         Border,
     },
     iced_core::Shadow,
+    iced_sctk::subsurface_widget::Subsurface,
     widget,
 };
 use cosmic_comp_config::workspace::WorkspaceLayout;
@@ -284,7 +285,7 @@ fn toplevel_previews<'a>(
 
 fn capture_image(image: Option<&CaptureImage>) -> cosmic::Element<'_, Msg> {
     if let Some(image) = image {
-        widget::Image::new(image.img.clone()).into()
+        Subsurface::new(image.width, image.height, &image.wl_buffer).into()
     } else {
         widget::Image::new(widget::image::Handle::from_pixels(1, 1, vec![0, 0, 0, 255])).into()
     }

src/wayland/buffer.rs

@@ -8,11 +8,13 @@ use cctk::{
 };
 use cosmic::cctk;
 use cosmic::iced::widget::image;
+use cosmic::iced_sctk::subsurface_widget::{BufferSource, Dmabuf, Plane, Shmbuf, SubsurfaceBuffer};
 use memmap2::Mmap;
 use rustix::{io::Errno, shm::ShmOFlags};
 use std::{
     os::fd::{AsFd, OwnedFd},
     path::{Path, PathBuf},
+    sync::Arc,
     time::{SystemTime, UNIX_EPOCH},
 };
 use wayland_protocols::wp::linux_dmabuf::zv1::client::zwp_linux_buffer_params_v1;
@@ -70,10 +72,9 @@ enum BufferBacking {
 }
 
 pub struct Buffer {
-    backing: BufferBacking,
+    pub backing: Arc<BufferSource>,
     pub buffer: wl_buffer::WlBuffer,
     pub buffer_info: BufferInfo,
-    mmap: Mmap,
     node: Option<PathBuf>,
 }
 
@@ -89,6 +90,12 @@ impl AppData {
             (),
         );
 
+        pool.destroy();
+
+        // XXX
+        let fd = rustix::fs::memfd_create("shm-buffer", rustix::fs::MemfdFlags::CLOEXEC).unwrap();
+        rustix::fs::ftruncate(&fd, buffer_info.stride as u64 * buffer_info.height as u64).unwrap();
+
         let format = wl_shm::Format::try_from(buffer_info.format).unwrap();
         let buffer = pool.create_buffer(
             0,
@@ -100,11 +107,18 @@ impl AppData {
             (),
         );
 
-        let mmap = unsafe { Mmap::map(&fd).unwrap() };
-
         Buffer {
-            backing: BufferBacking::Shm { fd },
+            backing: Arc::new(
-            mmap,
+                Shmbuf {
+                    fd,
+                    offset: 0,
+                    width: buffer_info.width as i32,
+                    height: buffer_info.height as i32,
+                    stride: buffer_info.stride as i32,
+                    format,
+                }
+                .into(),
+            ),
             buffer,
             buffer_info: buffer_info.clone(),
             node: None,
@@ -158,8 +172,8 @@ impl AppData {
             )?
         };
 
-        let fd = bo.fd()?;
+        let mut planes = Vec::new();
-        let stride = bo.stride()?;
+
         let params = self.dmabuf_state.create_params(&self.qh)?;
         let modifier = bo.modifier()?;
         for i in 0..bo.plane_count()? as i32 {
@@ -173,6 +187,12 @@ impl AppData {
                 plane_stride,
                 modifier.into(),
             );
+            planes.push(Plane {
+                fd: plane_fd,
+                plane_idx: i as u32,
+                offset: plane_offset,
+                stride: plane_stride,
+            });
         }
         let buffer = params
             .create_immed(
@@ -184,12 +204,17 @@ impl AppData {
             )
             .0;
 
-        // Is there any cost to mmapping dma memory if it isn't accessed?
-        let mmap = unsafe { Mmap::map(&fd).unwrap() };
-
         Ok(Some(Buffer {
-            backing: BufferBacking::Dmabuf { fd, stride },
+            backing: Arc::new(
-            mmap,
+                Dmabuf {
+                    width: buffer_info.width as i32,
+                    height: buffer_info.height as i32,
+                    planes,
+                    format: buffer_info.format,
+                    modifier: modifier.into(),
+                }
+                .into(),
+            ),
             buffer,
             buffer_info: buffer_info.clone(),
             node: Some(node.clone()),
@@ -200,12 +225,11 @@ impl AppData {
         // XXX Handle other formats?
         let format = wl_shm::Format::Abgr8888.into();
 
-        /*
         if let Some(buffer_info) = buffer_infos
             .iter()
             .find(|x| x.type_ == WEnum::Value(BufferType::Dmabuf) && x.format == format)
         {
-            match self.create_gbm_buffer(buffer_info, true) {
+            match self.create_gbm_buffer(buffer_info, false) {
                 Ok(Some(buffer)) => {
                     return buffer;
                 }
@@ -213,7 +237,6 @@ impl AppData {
                 Err(err) => eprintln!("Failed to create gbm buffer: {}", err),
             }
         }
-        */
 
         // Fallback to shm buffer
         // Assume format is already known to be valid
@@ -226,33 +249,6 @@ impl AppData {
 }
 
 impl Buffer {
-    // Buffer must be released by server for safety
-    // XXX is this at all a performance issue?
-    #[allow(clippy::wrong_self_convention)]
-    pub unsafe fn to_image(&mut self) -> image::Handle {
-        let pixels = match &self.backing {
-            BufferBacking::Shm { .. } => self.mmap.to_vec(),
-            // NOTE: Only will work with linear modifier
-            BufferBacking::Dmabuf { fd, stride } => {
-                if self.buffer_info.stride == self.buffer_info.width * 4 {
-                    self.mmap.to_vec()
-                } else {
-                    let width = self.buffer_info.width as usize;
-                    let height = self.buffer_info.height as usize;
-                    let stride = *stride as usize;
-                    let output_stride = width * 4;
-                    let mut pixels = vec![0; height * output_stride];
-                    for y in 0..height {
-                        pixels[y * output_stride..y * output_stride + output_stride]
-                            .copy_from_slice(&self.mmap[y * stride..y * stride + output_stride]);
-                    }
-                    pixels
-                }
-            }
-        };
-        image::Handle::from_pixels(self.buffer_info.width, self.buffer_info.height, pixels)
-    }
-
     pub fn node(&self) -> Option<&Path> {
         self.node.as_deref()
     }

src/wayland/mod.rs

@@ -20,7 +20,7 @@ use cctk::{
     toplevel_management::ToplevelManagerState,
     wayland_client::{
         globals::registry_queue_init,
-        protocol::{wl_output, wl_seat},
+        protocol::{wl_buffer, wl_output, wl_seat},
         Connection, QueueHandle,
     },
     workspace::WorkspaceState,
@@ -30,6 +30,7 @@ use cosmic::iced::{
     self,
     futures::{executor::block_on, FutureExt, SinkExt},
 };
+use cosmic::iced_sctk::subsurface_widget::SubsurfaceBuffer;
 use futures_channel::mpsc;
 use std::{
     cell::RefCell,
@@ -83,7 +84,9 @@ pub enum Event {
 
 #[derive(Clone, Debug)]
 pub struct CaptureImage {
-    pub img: iced::widget::image::Handle,
+    pub width: u32,
+    pub height: u32,
+    pub wl_buffer: SubsurfaceBuffer,
 }
 
 pub fn subscription(conn: Connection) -> iced::Subscription<Event> {

src/wayland/screencopy.rs

@@ -9,6 +9,7 @@ use cosmic::cctk::{
     },
     wayland_client::{Connection, QueueHandle, WEnum},
 };
+use cosmic::iced_sctk::subsurface_widget::{SubsurfaceBuffer, SubsurfaceBufferRelease};
 use std::{
     array,
     sync::{Arc, Weak},
@@ -21,6 +22,9 @@ pub struct ScreencopySession {
     buffers: Option<[Buffer; 2]>,
     session: zcosmic_screencopy_session_v1::ZcosmicScreencopySessionV1,
     first_frame: bool,
+    // Future signaled when buffer is signaled.
+    // if triple buffer is used, will need more than one.
+    release: Option<SubsurfaceBufferRelease>,
 }
 
 impl ScreencopySession {
@@ -54,6 +58,7 @@ impl ScreencopySession {
             buffers: None,
             session,
             first_frame: true,
+            release: None,
         }
     }
 
@@ -70,8 +75,9 @@ impl ScreencopySession {
                 .commit(zcosmic_screencopy_session_v1::Options::empty());
             self.first_frame = false;
         } else {
+            // TODO Not updating properly if `Options::OnDamage` is used
             self.session
-                .commit(zcosmic_screencopy_session_v1::Options::OnDamage);
+                .commit(zcosmic_screencopy_session_v1::Options::empty());
         }
         conn.flush().unwrap();
     }
@@ -149,11 +155,34 @@ impl ScreencopyHandler for AppData {
         session.buffers.as_mut().unwrap().rotate_left(1);
 
         // Capture again on damage
-        session.attach_buffer_and_commit(&capture, conn);
+        let capture_clone = capture.clone();
+        let conn = conn.clone();
+        let release = session.release.take();
+        self.scheduler
+            .schedule(async move {
+                if let Some(release) = release {
+                    // Wait for buffer to be released by server
+                    release.await;
+                }
+                let mut session = capture_clone.session.lock().unwrap();
+                let Some(session) = session.as_mut() else {
+                    return;
+                };
+                session.attach_buffer_and_commit(&capture_clone, &conn);
+            })
+            .unwrap();
 
         let front = session.buffers.as_mut().unwrap().first_mut().unwrap();
-        let img = unsafe { front.to_image() };
+        let (buffer, release) = SubsurfaceBuffer::new(front.backing.clone());
-        let image = CaptureImage { img };
+        session.release = Some(release);
+        // let img = unsafe { front.to_image() };
+        // let image = CaptureImage { img };
+        let buffer_info = &front.buffer_info;
+        let image = CaptureImage {
+            wl_buffer: buffer,
+            width: buffer_info.width,
+            height: buffer_info.height,
+        };
         match &capture.source {
             CaptureSource::Toplevel(toplevel) => {
                 self.send_event(Event::ToplevelCapture(toplevel.clone(), image))