From e1342fb2e36ae115bd61630940c5089a517445d5 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Wed, 17 Sep 2025 11:40:51 -0700 Subject: [PATCH] image-copy: Use "buffer age" of 1 for capture The logic `age_for_buffer` used seems to be a misinterpretation of the protocol. The wording is a little unclear, but it seems tracking buffer age is the responsibility of the client, and the client is required to accumulate damage and pass it in `damage_buffer`. Our clients initially weren't doing that correctly. I updated xdg-desktop-portal-cosmic to use `damage_buffer` after testing on wlroots, and cosmic-workspaces was recently updated as well. --- src/backend/kms/surface/mod.rs | 18 ++--------- .../handlers/image_copy_capture/render.rs | 2 +- .../handlers/image_copy_capture/user_data.rs | 32 ++----------------- 3 files changed, 6 insertions(+), 46 deletions(-) diff --git a/src/backend/kms/surface/mod.rs b/src/backend/kms/surface/mod.rs index 3f8baec1..91715be6 100644 --- a/src/backend/kms/surface/mod.rs +++ b/src/backend/kms/surface/mod.rs @@ -1353,13 +1353,6 @@ impl SurfaceThreadState { (&session, frame, res), now.into(), ) { - session - .user_data() - .get::() - .unwrap() - .lock() - .unwrap() - .reset(); tracing::warn!(?err, "Failed to screencopy"); } } @@ -1388,14 +1381,7 @@ impl SurfaceThreadState { } } Err(err) => { - for (session, frame, _) in frames { - session - .user_data() - .get::() - .unwrap() - .lock() - .unwrap() - .reset(); + for (_session, frame, _) in frames { frame.fail(CaptureFailureReason::Unknown); } return Err(err).with_context(|| "Failed to submit result for display"); @@ -1628,7 +1614,7 @@ fn take_screencopy_frames( // TODO re-use offscreen buffer to damage track screencopy to shm 0 } else { - damage_tracking.age_for_buffer(&buffer) + 1 }; if !additional_damage.is_empty() { diff --git a/src/wayland/handlers/image_copy_capture/render.rs b/src/wayland/handlers/image_copy_capture/render.rs index e7615222..a0e58f5d 100644 --- a/src/wayland/handlers/image_copy_capture/render.rs +++ b/src/wayland/handlers/image_copy_capture/render.rs @@ -225,7 +225,7 @@ where // TODO re-use offscreen buffer to damage track screencopy to shm 0 } else { - session_damage_tracking.age_for_buffer(&buffer) + 1 }; let mut fb = offscreen .as_mut() diff --git a/src/wayland/handlers/image_copy_capture/user_data.rs b/src/wayland/handlers/image_copy_capture/user_data.rs index 3188cb39..083f4220 100644 --- a/src/wayland/handlers/image_copy_capture/user_data.rs +++ b/src/wayland/handlers/image_copy_capture/user_data.rs @@ -1,11 +1,10 @@ // SPDX-License-Identifier: GPL-3.0-only -use std::{cell::RefCell, collections::HashMap, sync::Mutex}; +use std::{cell::RefCell, sync::Mutex}; use smithay::{ - backend::renderer::{damage::OutputDamageTracker, utils::CommitCounter}, + backend::renderer::damage::OutputDamageTracker, output::Output, - reexports::wayland_server::{Resource, Weak, protocol::wl_buffer::WlBuffer}, wayland::image_copy_capture::{ CursorSession, CursorSessionRef, Frame, FrameRef, Session, SessionRef, }, @@ -20,36 +19,11 @@ pub type SessionData = Mutex; pub struct SessionUserData { pub dt: OutputDamageTracker, - commit_counter: CommitCounter, - buffer_age: HashMap, CommitCounter>, } impl SessionUserData { pub fn new(tracker: OutputDamageTracker) -> SessionUserData { - SessionUserData { - dt: tracker, - commit_counter: CommitCounter::default(), - buffer_age: HashMap::new(), - } - } - - pub fn age_for_buffer(&mut self, buffer: &WlBuffer) -> usize { - self.buffer_age.retain(|k, _| k.upgrade().is_ok()); - - let weak = buffer.downgrade(); - let age = self - .commit_counter - .distance(self.buffer_age.get(&weak).copied()) - .unwrap_or(0); - self.buffer_age.insert(weak, self.commit_counter); - - self.commit_counter.increment(); - age - } - - pub fn reset(&mut self) { - self.commit_counter = CommitCounter::default(); - self.buffer_age.clear(); + SessionUserData { dt: tracker } } }