From dd6ebd03c933926dc6bdcecbaf3a7cf0a1ba23c1 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Tue, 7 Jul 2026 11:46:42 -0700 Subject: [PATCH] image-copy: Hold references to `Buffer`s to block release The `wl_buffer` now won't be released until sync fence polls ready. --- src/backend/kms/surface/mod.rs | 4 +- src/backend/render/mod.rs | 19 ++- .../handlers/image_copy_capture/render.rs | 123 +++++++++++++----- 3 files changed, 103 insertions(+), 43 deletions(-) diff --git a/src/backend/kms/surface/mod.rs b/src/backend/kms/surface/mod.rs index a277af2e..bb28bc3c 100644 --- a/src/backend/kms/surface/mod.rs +++ b/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, frame_result: &RenderFrameResult>>, - elements: &[CosmicElement], + elements: &[CosmicElement>], (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 diff --git a/src/backend/render/mod.rs b/src/backend/render/mod.rs index 472abb8e..154ad986 100644 --- a/src/backend/render/mod.rs +++ b/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 { - damage: res.0, - sync, - states: res.1, - }) + let buffers = render_element_buffers(renderer, &elements); + + Ok(( + RenderOutputResult { + damage: res.0, + sync, + states: res.1, + }, + buffers, + )) }, )? { pending_image_copy_data.send_success_when_ready( diff --git a/src/wayland/handlers/image_copy_capture/render.rs b/src/wayland/handlers/image_copy_capture/render.rs index ae429b4d..0cb62529 100644 --- a/src/wayland/handlers/image_copy_capture/render.rs +++ b/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( + renderer: &mut R, + elements: &[E], +) -> Vec +where + R: AsGlowRenderer, + E: RenderElement, +{ + 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>, pub sync: SyncPoint, + // Hold reference so `wl_buffer` isn't released and sync point isn't signaled + // until image copy completes. + _buffers: Vec, } impl PendingImageCopyData { @@ -104,6 +124,7 @@ pub fn submit_buffer( transform: Transform, damage: Option<&[Rectangle]>, mut sync: SyncPoint, + buffers: Vec, ) -> Result, 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>, - ) -> Result, DTError>, + ) -> Result< + ( + RenderOutputResult<'d>, + Vec, + ), + DTError, + >, { 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 { - Ok(result) => submit_buffer( - frame, - renderer, - fb.as_mut(), - transform, - result.damage.map(|x| x.as_slice()), - result.sync, - ) - .map_err(DTError::Rendering), - Err(err) => { - frame.fail(CaptureFailureReason::Unknown); - Err(err) - } - } + submit_buffer( + frame, + renderer, + fb.as_mut(), + transform, + result.damage.map(|x| x.as_slice()), + result.sync, + buffers, + ) + .map_err(DTError::Rendering) } 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, DTError> + ) -> Result< + ( + RenderOutputResult<'d>, + Vec, + ), + DTError, + > 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, - ) -> Result, DTError> + ) -> Result< + ( + RenderOutputResult<'d>, + Vec, + ), + DTError, + > 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>, common: &mut Common, seat: &Seat, - ) -> Result, DTError> + ) -> Result< + ( + RenderOutputResult<'d>, + Vec, + ), + DTError, + > 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;