image-copy: Hold references to Buffers to block release

The `wl_buffer` now won't be released until sync fence polls ready.

src/backend/kms/surface/mod.rs

@@ -1674,7 +1674,7 @@ fn send_screencopy_result<'a>(
     pre_postprocess_data: &mut PrePostprocessData,
     tx: &std::sync::mpsc::Sender<PendingImageCopyData>,
     frame_result: &RenderFrameResult<GbmBuffer, GbmFramebuffer, CosmicElement<GlMultiRenderer<'a>>>,
-    elements: &[CosmicElement<GlMultiRenderer>],
+    elements: &[CosmicElement<GlMultiRenderer<'a>>],
     (session, frame, res): (
         &ScreencopySessionRef,
         ScreencopyFrame,
@@ -1855,6 +1855,8 @@ fn send_screencopy_result<'a>(
         transform,
         damage.as_deref(),
         sync,
+        // Don't reference `Buffer`s since we blit from framebuffer/postprocess buffer
+        vec![],
     )? {
         if frame_result.is_empty {
             data.frame

src/backend/render/mod.rs

@@ -41,7 +41,9 @@ use crate::{
             compositor::FRAME_TIME_FILTER,
             corner_radius::{pad_rect, surface_corners, surface_padding},
             data_device::get_dnd_icon,
-            image_copy_capture::{FrameHolder, SessionData, render_session},
+            image_copy_capture::{
+                FrameHolder, SessionData, render_element_buffers, render_session,
+            },
         },
         protocols::workspace::WorkspaceHandle,
     },
@@ -1505,11 +1507,16 @@ where
                             }
                         }
 
-                        Ok(RenderOutputResult {
+                        let buffers = render_element_buffers(renderer, &elements);
-                            damage: res.0,
+
-                            sync,
+                        Ok((
-                            states: res.1,
+                            RenderOutputResult {
-                        })
+                                damage: res.0,
+                                sync,
+                                states: res.1,
+                            },
+                            buffers,
+                        ))
                     },
                 )? {
                     pending_image_copy_data.send_success_when_ready(

src/wayland/handlers/image_copy_capture/render.rs

@@ -9,7 +9,7 @@ use smithay::{
             buffer_dimensions, buffer_type,
             damage::{Error as DTError, OutputDamageTracker, RenderOutputResult},
             element::{
-                RenderElement,
+                RenderElement, UnderlyingStorage,
                 utils::{Relocate, RelocateRenderElement},
             },
             gles::{GlesError, GlesRenderbuffer},
@@ -58,10 +58,30 @@ use crate::{
 
 use super::{super::data_device::get_dnd_icon, user_data::SessionHolder};
 
+pub fn render_element_buffers<R, E>(
+    renderer: &mut R,
+    elements: &[E],
+) -> Vec<smithay::backend::renderer::utils::Buffer>
+where
+    R: AsGlowRenderer,
+    E: RenderElement<R>,
+{
+    elements
+        .iter()
+        .filter_map(|elem| match elem.underlying_storage(renderer) {
+            Some(UnderlyingStorage::Wayland(buffer)) => Some(buffer.clone()),
+            Some(UnderlyingStorage::Memory(_)) | None => None,
+        })
+        .collect()
+}
+
 pub struct PendingImageCopyData {
     pub frame: Frame,
     pub damage: Vec<smithay::utils::Rectangle<i32, BufferCoords>>,
     pub sync: SyncPoint,
+    // Hold reference so `wl_buffer` isn't released and sync point isn't signaled
+    // until image copy completes.
+    _buffers: Vec<smithay::backend::renderer::utils::Buffer>,
 }
 
 impl PendingImageCopyData {
@@ -104,6 +124,7 @@ pub fn submit_buffer<R>(
     transform: Transform,
     damage: Option<&[Rectangle<i32, Physical>]>,
     mut sync: SyncPoint,
+    buffers: Vec<smithay::backend::renderer::utils::Buffer>,
 ) -> Result<Option<PendingImageCopyData>, R::Error>
 where
     R: ExportMem + AsGlowRenderer,
@@ -182,6 +203,7 @@ where
             })
             .collect(),
         sync,
+        _buffers: buffers,
     }))
 }
 
@@ -201,7 +223,13 @@ where
         &'d mut OutputDamageTracker,
         usize,
         Vec<Rectangle<i32, BufferCoords>>,
-    ) -> Result<RenderOutputResult<'d>, DTError<R::Error>>,
+    ) -> Result<
+        (
+            RenderOutputResult<'d>,
+            Vec<smithay::backend::renderer::utils::Buffer>,
+        ),
+        DTError<R::Error>,
+    >,
 {
     let mut session_user_data = session.lock().unwrap();
 
@@ -245,30 +273,25 @@ where
         .as_mut()
         .map(|(_, tex)| renderer.bind(tex).map_err(DTError::Rendering))
         .transpose()?;
-    let res = render_fn(
+    let (result, buffers) = render_fn(
         &frame.buffer(),
         renderer,
         fb.as_mut(),
         dt,
         age,
         frame.damage(),
-    );
+    )?;
 
-    match res {
+    submit_buffer(
-        Ok(result) => submit_buffer(
+        frame,
-            frame,
+        renderer,
-            renderer,
+        fb.as_mut(),
-            fb.as_mut(),
+        transform,
-            transform,
+        result.damage.map(|x| x.as_slice()),
-            result.damage.map(|x| x.as_slice()),
+        result.sync,
-            result.sync,
+        buffers,
-        )
+    )
-        .map_err(DTError::Rendering),
+    .map_err(DTError::Rendering)
-        Err(err) => {
-            frame.fail(CaptureFailureReason::Unknown);
-            Err(err)
-        }
-    }
 }
 
 pub fn render_workspace_to_buffer(
@@ -316,7 +339,13 @@ pub fn render_workspace_to_buffer(
         common: &mut Common,
         output: &Output,
         handle: (WorkspaceHandle, usize),
-    ) -> Result<RenderOutputResult<'d>, DTError<R::Error>>
+    ) -> Result<
+        (
+            RenderOutputResult<'d>,
+            Vec<smithay::backend::renderer::utils::Buffer>,
+        ),
+        DTError<R::Error>,
+    >
     where
         R: AsGlowRenderer,
         R::TextureId: Send + Clone + 'static,
@@ -356,7 +385,7 @@ pub fn render_workspace_to_buffer(
                 .collect()
         });
 
-        if let Ok(dmabuf) = get_dmabuf(buffer) {
+        let (res, elements) = if let Ok(dmabuf) = get_dmabuf(buffer) {
             let mut dmabuf = dmabuf.clone();
             let mut fb = renderer.bind(&mut dmabuf).map_err(DTError::Rendering)?;
             render_workspace(
@@ -374,8 +403,7 @@ pub fn render_workspace_to_buffer(
                 handle,
                 cursor_mode,
                 ElementFilter::ExcludeWorkspaceOverview,
-            )
+            )?
-            .map(|res| res.0)
         } else {
             let target = offscreen.expect("shm buffers should have an offscreen target");
             render_workspace(
@@ -393,9 +421,12 @@ pub fn render_workspace_to_buffer(
                 handle,
                 cursor_mode,
                 ElementFilter::ExcludeWorkspaceOverview,
-            )
+            )?
-            .map(|res| res.0)
+        };
-        }
+
+        let buffers = render_element_buffers(renderer, &elements);
+
+        Ok((res, buffers))
     }
 
     let draw_cursor = session.draw_cursor();
@@ -549,7 +580,13 @@ pub fn render_window_to_buffer(
         common: &mut Common,
         toplevel: &CosmicSurface,
         geometry: Rectangle<i32, Logical>,
-    ) -> Result<RenderOutputResult<'d>, DTError<R::Error>>
+    ) -> Result<
+        (
+            RenderOutputResult<'d>,
+            Vec<smithay::backend::renderer::utils::Buffer>,
+        ),
+        DTError<R::Error>,
+    >
     where
         R: AsGlowRenderer,
         R::TextureId: Send + Clone + 'static,
@@ -650,16 +687,20 @@ pub fn render_window_to_buffer(
             None,
         );
 
-        if let Ok(dmabuf) = get_dmabuf(buffer) {
+        let res = if let Ok(dmabuf) = get_dmabuf(buffer) {
             let mut dmabuf_clone = dmabuf.clone();
             let mut fb = renderer
                 .bind(&mut dmabuf_clone)
                 .map_err(DTError::Rendering)?;
-            dt.render_output(renderer, &mut fb, age, &elements, Color32F::TRANSPARENT)
+            dt.render_output(renderer, &mut fb, age, &elements, Color32F::TRANSPARENT)?
         } else {
             let fb = offscreen.expect("shm buffer should have an offscreen target");
-            dt.render_output(renderer, fb, age, &elements, Color32F::TRANSPARENT)
+            dt.render_output(renderer, fb, age, &elements, Color32F::TRANSPARENT)?
-        }
+        };
+
+        let buffers = render_element_buffers(renderer, &elements);
+
+        Ok((res, buffers))
     }
 
     let common = &mut state.common;
@@ -797,7 +838,13 @@ pub fn render_cursor_to_buffer(
         additional_damage: Vec<Rectangle<i32, BufferCoords>>,
         common: &mut Common,
         seat: &Seat<State>,
-    ) -> Result<RenderOutputResult<'d>, DTError<R::Error>>
+    ) -> Result<
+        (
+            RenderOutputResult<'d>,
+            Vec<smithay::backend::renderer::utils::Buffer>,
+        ),
+        DTError<R::Error>,
+    >
     where
         R: AsGlowRenderer,
         R::TextureId: Send + Clone + 'static,
@@ -830,16 +877,20 @@ pub fn render_cursor_to_buffer(
             },
         );
 
-        if let Ok(dmabuf) = get_dmabuf(buffer) {
+        let res = if let Ok(dmabuf) = get_dmabuf(buffer) {
             let mut dmabuf_clone = dmabuf.clone();
             let mut fb = renderer
                 .bind(&mut dmabuf_clone)
                 .map_err(DTError::Rendering)?;
-            dt.render_output(renderer, &mut fb, age, &elements, [0.0, 0.0, 0.0, 0.0])
+            dt.render_output(renderer, &mut fb, age, &elements, [0.0, 0.0, 0.0, 0.0])?
         } else {
             let fb = offscreen.expect("shm buffers should have offscreen target");
-            dt.render_output(renderer, fb, age, &elements, [0.0, 0.0, 0.0, 0.0])
+            dt.render_output(renderer, fb, age, &elements, [0.0, 0.0, 0.0, 0.0])?
-        }
+        };
+
+        let buffers = render_element_buffers(renderer, &elements);
+
+        Ok((res, buffers))
     }
 
     let common = &mut state.common;