Capture screenshots in the compositor instead of grim

compositor/src/handlers/mod.rs

@@ -3,6 +3,8 @@ mod compositor;
 mod input_method;
 mod layer_shell;
 pub(crate) mod screencopy;
+pub(crate) mod screenshot;
+pub(crate) mod screenshot_worker;
 mod xdg_shell;
 
 use smithay::{

compositor/src/handlers/screencopy.rs

@@ -1,7 +1,12 @@
+use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::{Arc, Mutex};
 
+use super::screenshot::{CaptureBuffer, PartOutcome, PartSender};
 use smithay::{
-    backend::{allocator::Fourcc, renderer::ExportMem},
+    backend::{
+        allocator::Fourcc,
+        renderer::{ExportMem, TextureMapping},
+    },
     output::Output,
     reexports::{
         wayland_protocols_wlr::screencopy::v1::server::{
@@ -31,18 +36,120 @@ pub(crate) struct FrameData {
 
 #[derive(Debug)]
 pub(crate) struct PendingScreencopy {
-    pub(crate) frame: ZwlrScreencopyFrameV1,
+    sink: CaptureSink,
-    buffer: WlBuffer,
     pub(crate) output: Output,
     region: Rectangle<i32, Buffer>,
     overlay_cursor: bool,
     with_damage: bool,
 }
 
-pub(crate) fn init_global(display: &DisplayHandle) {
+#[derive(Debug)]
-    let enabled = std::env::var_os("FERESE_ENABLE_SCREENCOPY").is_some_and(|value| value == "1");
+pub(crate) enum CaptureSink {
+    Shm {
+        frame: ZwlrScreencopyFrameV1,
+        buffer: WlBuffer,
+    },
+    Owned {
+        request: u64,
+        part: usize,
+        complete: PartSender,
+        published: AtomicBool,
+    },
+}
 
-    if enabled {
+impl CaptureSink {
+    pub(crate) fn owned(request: u64, part: usize, complete: PartSender) -> Self {
+        Self::Owned {
+            request,
+            part,
+            complete,
+            published: AtomicBool::new(false),
+        }
+    }
+
+    // Every part reaches exactly one terminal state: pixels or an error. A
+    // cancelled part is still answered, otherwise its request would wait
+    // forever for a part that can no longer arrive.
+    pub(crate) fn fail(&self) {
+        match self {
+            CaptureSink::Shm { frame, .. } => frame.failed(),
+            CaptureSink::Owned {
+                request,
+                part,
+                complete,
+                published,
+            } => {
+                if !published.swap(true, Ordering::AcqRel) {
+                    let _ = complete.send(PartOutcome {
+                        request: *request,
+                        part: *part,
+                        result: Err("Screenshot part failed".to_string()),
+                    });
+                }
+            }
+        }
+    }
+
+    // One-shot: a part publishes at most once, so a second completion can
+    // never overwrite pixels that were already handed off.
+    pub(crate) fn publish(&self, buffer: CaptureBuffer) -> bool {
+        let CaptureSink::Owned {
+            request,
+            part,
+            complete,
+            published,
+        } = self
+        else {
+            return false;
+        };
+        if published.swap(true, Ordering::AcqRel) {
+            return false;
+        }
+        complete
+            .send(PartOutcome {
+                request: *request,
+                part: *part,
+                result: Ok(buffer),
+            })
+            .is_ok()
+    }
+}
+
+impl PendingScreencopy {
+    pub(crate) fn owned(
+        request: u64,
+        part: usize,
+        complete: PartSender,
+        output: Output,
+        region: Rectangle<i32, Buffer>,
+    ) -> Self {
+        Self {
+            sink: CaptureSink::owned(request, part, complete),
+            output,
+            region,
+            // The compositor-owned path deliberately omits the cursor.
+            overlay_cursor: false,
+            with_damage: false,
+        }
+    }
+
+    pub(crate) fn fail(&self) {
+        self.sink.fail();
+    }
+}
+
+pub(crate) fn capture_allowed() -> bool {
+    static ALLOWED: std::sync::OnceLock<bool> = std::sync::OnceLock::new();
+    *ALLOWED
+        .get_or_init(|| capture_opted_in(std::env::var_os("FERESE_ENABLE_SCREENCOPY").as_deref()))
+}
+
+fn capture_opted_in(value: Option<&std::ffi::OsStr>) -> bool {
+    value.is_some_and(|value| value == "1")
+}
+
+pub(crate) fn init_global(display: &DisplayHandle) {
+    if capture_allowed() {
         display.create_global::<Ferese, ZwlrScreencopyManagerV1, ()>(3, ());
         tracing::info!("authorized screencopy is enabled for this session");
     }
@@ -161,8 +268,10 @@ impl Ferese {
         };
 
         self.pending_screencopies.push(PendingScreencopy {
+            sink: CaptureSink::Shm {
                 frame: frame.clone(),
                 buffer,
+            },
             output,
             region: data.region,
             overlay_cursor: data.overlay_cursor,
@@ -192,14 +301,16 @@ impl Ferese {
                 remaining.push(capture);
                 continue;
             }
-            if !capture.frame.is_alive() || !capture.buffer.is_alive() {
+            if let CaptureSink::Shm { frame, buffer } = &capture.sink
+                && (!frame.is_alive() || !buffer.is_alive())
+            {
                 continue;
             }
 
             framebuffer_binding_changed = true;
             if complete_capture(renderer, framebuffer, &capture, self.start_time.elapsed()).is_err()
             {
-                capture.frame.failed();
+                capture.fail();
             }
         }
 
@@ -283,17 +394,25 @@ fn capture_region(
     capture_region_for_geometry(mode.size, scale, transform, requested)
 }
 
-fn capture_region_for_geometry(
+pub(crate) fn output_logical_size(
+    mode_size: Size<i32, smithay::utils::Physical>,
+    scale: f64,
+    transform: smithay::utils::Transform,
+) -> Size<i32, Logical> {
+    transform
+        .transform_size(mode_size)
+        .to_f64()
+        .to_logical(scale)
+        .to_i32_round()
+}
+
+pub(crate) fn capture_region_for_geometry(
     mode_size: Size<i32, smithay::utils::Physical>,
     scale: f64,
     transform: smithay::utils::Transform,
     requested: Option<Rectangle<i32, Logical>>,
 ) -> Option<Rectangle<i32, Buffer>> {
-    let logical_size = transform
+    let logical_size = output_logical_size(mode_size, scale, transform);
-        .transform_size(mode_size)
-        .to_f64()
-        .to_logical(scale)
-        .to_i32_round();
     let output_region = Rectangle::from_size(logical_size);
     let logical_region = requested
         .unwrap_or(output_region)
@@ -342,50 +461,79 @@ where
         .map_err(|error| {
             tracing::warn!(?error, "failed to copy output framebuffer");
         })?;
+    if mapping.format() != Fourcc::Argb8888 {
+        return Err(());
+    }
     let source = renderer.map_texture(&mapping).map_err(|error| {
         tracing::warn!(?error, "failed to map output framebuffer copy");
     })?;
     let expected = capture.region.size;
-    let copied = with_buffer_contents_mut(&capture.buffer, |destination, length, data| {
+    let stride = (expected.w as usize)
-        let bytes = expected.w as usize * expected.h as usize * BYTES_PER_PIXEL;
+        .checked_mul(BYTES_PER_PIXEL)
-        if source.len() < bytes || data.offset < 0 || data.offset as usize + bytes > length {
+        .ok_or(())?;
+    let bytes = stride.checked_mul(expected.h as usize).ok_or(())?;
+    if source.len() < bytes {
+        return Err(());
+    }
+    // Smithay's output projection already accounts for OpenGL's Y axis, so these
+    // rows must not be flipped vertically again. They are in output-buffer order
+    // including the output transform, which the conversion stage normalizes.
+    let copied = match &capture.sink {
+        CaptureSink::Shm { buffer, .. } => {
+            with_buffer_contents_mut(buffer, |destination, length, data| {
+                let Ok(offset) = usize::try_from(data.offset) else {
+                    return false;
+                };
+                if offset > length || bytes > length - offset {
                     return false;
                 }
 
-        // Smithay's output projection already accounts for OpenGL's Y axis.
+                // SAFETY: the mapped slice is at least `bytes` long, and the
-        // These rows are in output-buffer order, including its output transform.
+                // checks above establish `bytes` writable bytes at `offset`
+                // within the shm buffer. The regions cannot overlap: one is
+                // renderer-owned and the other client-owned.
                 unsafe {
-            std::ptr::copy_nonoverlapping(
+                    std::ptr::copy_nonoverlapping(source.as_ptr(), destination.add(offset), bytes);
-                source.as_ptr(),
-                destination.add(data.offset as usize),
-                bytes,
-            );
                 }
                 true
             })
-    .map_err(|_| ())?;
+            .unwrap_or(false)
+        }
+        CaptureSink::Owned { .. } => {
+            let mut pixels = Vec::new();
+            if pixels.try_reserve_exact(bytes).is_err() {
+                false
+            } else {
+                pixels.extend_from_slice(&source[..bytes]);
+                capture.sink.publish(CaptureBuffer {
+                    width: expected.w,
+                    height: expected.h,
+                    stride,
+                    pixels,
+                })
+            }
+        }
+    };
 
     if !copied {
         return Err(());
     }
 
+    if let CaptureSink::Shm { frame, .. } = &capture.sink {
         if capture.with_damage {
-        capture
+            frame.damage(0, 0, expected.w as u32, expected.h as u32);
-            .frame
-            .damage(0, 0, expected.w as u32, expected.h as u32);
         }
         // TextureMapping::flipped describes importing the mapping as a texture;
         // using it here would make clients flip the already-correct output rows.
-    capture
+        frame.flags(zwlr_screencopy_frame_v1::Flags::empty());
-        .frame
-        .flags(zwlr_screencopy_frame_v1::Flags::empty());
 
         let seconds = timestamp.as_secs();
-    capture.frame.ready(
+        frame.ready(
             (seconds >> 32) as u32,
             seconds as u32,
             timestamp.subsec_nanos(),
         );
+    }
     Ok(())
 }
 
@@ -394,6 +542,19 @@ mod tests {
     use super::*;
     use smithay::utils::{Physical, Transform};
 
+    #[test]
+    fn capture_is_off_unless_the_opt_in_is_exactly_one() {
+        use std::ffi::OsStr;
+
+        assert!(!capture_opted_in(None));
+        assert!(!capture_opted_in(Some(OsStr::new("0"))));
+        assert!(!capture_opted_in(Some(OsStr::new(""))));
+        assert!(!capture_opted_in(Some(OsStr::new("true"))));
+        assert!(!capture_opted_in(Some(OsStr::new("01"))));
+        assert!(!capture_opted_in(Some(OsStr::new(" 1"))));
+        assert!(capture_opted_in(Some(OsStr::new("1"))));
+    }
+
     #[test]
     fn capture_region_clips_then_scales_to_buffer_coordinates() {
         let mode = Size::<i32, Physical>::from((1_920, 1_080));

compositor/src/handlers/screenshot_worker.rs

@@ -0,0 +1,292 @@
+use std::fs;
+use std::io;
+use std::os::unix::fs::{OpenOptionsExt, PermissionsExt};
+use std::path::PathBuf;
+use std::sync::mpsc::{Receiver, SyncSender, TrySendError, sync_channel};
+use std::thread;
+use std::time::{Duration, SystemTime};
+
+use smithay::reexports::calloop::channel;
+
+use super::screenshot::{Canvas, OutputFrame, compose};
+
+const QUEUE_CAPACITY: usize = 4;
+/// A client normally opens and unlinks its screenshot immediately. If it dies
+/// first, the file is left behind, so staged files are swept well after any
+/// realistic request could still be reading one.
+const STAGE_TTL: Duration = Duration::from_secs(60 * 60);
+pub(crate) const SWEEP_INTERVAL: Duration = Duration::from_secs(5 * 60);
+const DIR_MODE: u32 = 0o700;
+const FILE_MODE: u32 = 0o600;
+
+pub(crate) struct Job {
+    pub(crate) request: u64,
+    pub(crate) frames: Vec<OutputFrame>,
+}
+
+pub(crate) struct Encoded {
+    pub(crate) request: u64,
+    pub(crate) result: Result<PathBuf, String>,
+}
+
+pub(crate) struct Worker {
+    queue: SyncSender<Job>,
+}
+
+impl Worker {
+    pub(crate) fn spawn(results: channel::SyncSender<Encoded>) -> Option<Self> {
+        let (queue, receiver) = sync_channel::<Job>(QUEUE_CAPACITY);
+        thread::Builder::new()
+            .name("ferese-screenshot".to_owned())
+            .spawn(move || run(receiver, results))
+            .ok()
+            .map(|_| Self { queue })
+    }
+
+    // The compositor thread must never block, so a full queue is reported back
+    // to the caller instead of waiting.
+    pub(crate) fn submit(&self, job: Job) -> Result<(), Job> {
+        match self.queue.try_send(job) {
+            Ok(()) => Ok(()),
+            Err(TrySendError::Full(job)) | Err(TrySendError::Disconnected(job)) => Err(job),
+        }
+    }
+}
+
+fn run(receiver: Receiver<Job>, results: channel::SyncSender<Encoded>) {
+    while let Ok(job) = receiver.recv() {
+        let result = encode(&job);
+        // The loop drains this channel, so a full queue only delays the worker.
+        if results
+            .send(Encoded {
+                request: job.request,
+                result,
+            })
+            .is_err()
+        {
+            return;
+        }
+    }
+}
+
+// Removes staged screenshots whose client never collected them. Only files
+// older than the TTL are touched, so an in-flight request is never disturbed.
+pub(crate) fn sweep_stale_files() {
+    let Ok(directory) = private_directory() else {
+        return;
+    };
+    let Ok(entries) = fs::read_dir(&directory) else {
+        return;
+    };
+    for entry in entries.flatten() {
+        let path = entry.path();
+        if path.extension().is_none_or(|extension| extension != "png") {
+            continue;
+        }
+        let Ok(metadata) = entry.metadata() else {
+            continue;
+        };
+        if !metadata.is_file() {
+            continue;
+        }
+        // A file dated in the future is treated as fresh rather than swept.
+        let stale = metadata
+            .modified()
+            .ok()
+            .and_then(|modified| SystemTime::now().duration_since(modified).ok())
+            .is_some_and(|age| age > STAGE_TTL);
+        if stale && fs::remove_file(&path).is_ok() {
+            tracing::debug!(path = %path.display(), "removed an uncollected screenshot");
+        }
+    }
+}
+
+fn encode(job: &Job) -> Result<PathBuf, String> {
+    let canvas = compose(&job.frames)?;
+    let png = encode_png(&canvas)?;
+    write_private_file(&png)
+}
+
+fn encode_png(canvas: &Canvas) -> Result<Vec<u8>, String> {
+    let (width, height) = (canvas.width, canvas.height);
+    // A final sanity check on the composed size before handing it to the
+    // encoder, which would otherwise panic on a zero dimension.
+    if width == 0 || height == 0 {
+        return Err("Screenshot is empty".into());
+    }
+    let image = image::RgbaImage::from_raw(width, height, canvas.pixels.clone())
+        .ok_or("Screenshot buffer does not match its dimensions")?;
+    let mut out = Vec::new();
+    let encoder = image::codecs::png::PngEncoder::new_with_quality(
+        std::io::Cursor::new(&mut out),
+        image::codecs::png::CompressionType::Fast,
+        image::codecs::png::FilterType::Adaptive,
+    );
+    image::ImageEncoder::write_image(
+        encoder,
+        image.as_raw(),
+        width,
+        height,
+        image::ExtendedColorType::Rgba8,
+    )
+    .map_err(|error| format!("Could not encode screenshot: {error}"))?;
+    tracing::debug!(encoded = out.len(), "encoded screenshot");
+    Ok(out)
+}
+
+// The client is told to open this path and unlink it immediately, so the
+// directory is ours, private, and files are created with the final mode.
+fn write_private_file(png: &[u8]) -> Result<PathBuf, String> {
+    let directory = private_directory()?;
+    for attempt in 0..64u32 {
+        let path = directory.join(format!("shot-{}-{attempt}.png", std::process::id()));
+        let mut options = fs::OpenOptions::new();
+        options.write(true).create_new(true).mode(FILE_MODE);
+        match options.open(&path) {
+            Ok(mut file) => {
+                use io::Write;
+                file.write_all(png)
+                    .and_then(|()| file.sync_all())
+                    .map_err(|error| format!("Could not write screenshot: {error}"))?;
+                return Ok(path);
+            }
+            Err(error) if error.kind() == io::ErrorKind::AlreadyExists => continue,
+            Err(error) => return Err(format!("Could not write screenshot: {error}")),
+        }
+    }
+    Err("Could not find an unused screenshot filename".into())
+}
+
+fn private_directory() -> Result<PathBuf, String> {
+    let runtime = std::env::var_os("XDG_RUNTIME_DIR")
+        .ok_or("XDG_RUNTIME_DIR is not set, so screenshots cannot be staged")?;
+    let root = PathBuf::from(runtime);
+    let directory = root.join("ferese-screenshots");
+    match fs::symlink_metadata(&directory) {
+        Ok(metadata) if metadata.file_type().is_symlink() => {
+            return Err("Refusing to stage screenshots in a symlinked directory".into());
+        }
+        Ok(metadata) => {
+            use std::os::unix::fs::MetadataExt;
+            if !metadata.is_dir() || metadata.uid() != unsafe { libc::geteuid() } {
+                return Err("Refusing to stage screenshots in an untrusted directory".into());
+            }
+        }
+        Err(error) if error.kind() == io::ErrorKind::NotFound => {
+            fs::create_dir(&directory)
+                .and_then(|()| {
+                    fs::set_permissions(&directory, fs::Permissions::from_mode(DIR_MODE))
+                })
+                .map_err(|error| format!("Could not create screenshot directory: {error}"))?;
+        }
+        Err(error) => return Err(format!("Could not inspect screenshot directory: {error}")),
+    }
+    Ok(directory)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use smithay::utils::Transform;
+
+    fn frame(width: i32, height: i32) -> OutputFrame {
+        let stride = width as usize * 4;
+        let mut pixels = vec![0u8; stride * height as usize];
+        let (quads, _) = pixels.as_chunks_mut::<4>();
+        for pixel in quads {
+            pixel[3] = 255;
+        }
+        OutputFrame {
+            width,
+            height,
+            stride,
+            pixels,
+            transform: Transform::Normal,
+            location: (0, 0),
+            scale: 1.0,
+            logical_width: width,
+            logical_height: height,
+        }
+    }
+
+    #[test]
+    fn encode_produces_a_decodable_png() {
+        let job = Job {
+            request: 1,
+            frames: vec![frame(4, 3)],
+        };
+        let canvas = compose(&job.frames).unwrap();
+        let png = encode_png(&canvas).expect("png");
+        assert_eq!(&png[..8], b"\x89PNG\r\n\x1a\n", "png signature");
+
+        let decoded = image::load_from_memory_with_format(&png, image::ImageFormat::Png)
+            .expect("decodes back")
+            .to_rgba8();
+        assert_eq!(decoded.dimensions(), (4, 3));
+        // The opaque alpha we force is preserved through the round trip.
+        assert_eq!(decoded.get_pixel(0, 0)[3], 255);
+    }
+
+    #[test]
+    fn staged_files_are_owner_only() {
+        let png = b"\x89PNG\r\n\x1a\nnot really a png".to_vec();
+        let path = write_private_file(&png).expect("staged");
+        use std::os::unix::fs::PermissionsExt;
+        let mode = fs::metadata(&path).unwrap().permissions().mode() & 0o777;
+        assert_eq!(
+            mode, FILE_MODE,
+            "staged screenshots are not group/world readable"
+        );
+        fs::remove_file(&path).unwrap();
+    }
+
+    #[test]
+    fn the_sweep_keeps_fresh_files_and_ignores_other_types() {
+        let png = b"\x89PNG\r\n\x1a\n".to_vec();
+        let fresh = write_private_file(&png).expect("staged");
+
+        let directory = private_directory().unwrap();
+        let other = directory.join("keep-me.txt");
+        fs::write(&other, b"not a screenshot").unwrap();
+
+        sweep_stale_files();
+        assert!(
+            fresh.exists(),
+            "a freshly staged screenshot is still in use"
+        );
+        assert!(other.exists(), "unrelated files are left alone");
+
+        fs::remove_file(&fresh).unwrap();
+        fs::remove_file(&other).unwrap();
+    }
+
+    #[test]
+    fn the_sweep_removes_files_past_the_ttl() {
+        let png = b"\x89PNG\r\n\x1a\n".to_vec();
+        let path = write_private_file(&png).expect("staged");
+        // Backdate it past the TTL without waiting an hour.
+        let aged = SystemTime::now() - STAGE_TTL - Duration::from_secs(60);
+        fs::File::options()
+            .write(true)
+            .open(&path)
+            .unwrap()
+            .set_modified(aged)
+            .expect("backdate");
+
+        sweep_stale_files();
+        assert!(!path.exists(), "an uncollected screenshot is reclaimed");
+    }
+
+    #[test]
+    fn a_missing_runtime_dir_is_reported_rather_than_ignored() {
+        // private_directory must never fall back to /tmp or the cwd.
+        let result = private_directory();
+        match result {
+            Ok(path) => assert!(
+                path.ends_with("ferese-screenshots"),
+                "screenshots are staged in the runtime dir, not a fallback location"
+            ),
+            Err(error) => assert!(error.contains("XDG_RUNTIME_DIR") || error.contains("untrusted")),
+        }
+    }
+}

compositor/src/ipc.rs

@@ -11,18 +11,27 @@ use std::sync::{
     mpsc::{SyncSender, sync_channel},
 };
 use std::thread;
+use std::time::Duration;
 
 use ferese_core::LayoutMode;
 use ferese_ipc::{Request, Response, VERSION, read_frame, write_frame};
 use ferese_layout::Direction;
 use serde_json::{Value, json};
-use smithay::reexports::calloop::{EventLoop, LoopSignal, channel};
+use smithay::reexports::calloop::{EventLoop, LoopSignal, channel, timer};
 use smithay::utils::Transform;
 
+use crate::handlers::screencopy;
+use crate::handlers::screenshot::{
+    Action, Geometry, OutputLayout, PartOutcome, PartSender, parse_geometry, plan,
+};
+use crate::handlers::screenshot_worker::{self, Encoded, Job, Worker};
 use crate::{Ferese, config::OutputTransform};
+use smithay::output::Output;
 
 const REQUEST_QUEUE_CAPACITY: usize = 128;
 const MAX_CONNECTIONS: usize = 64;
+const RESULT_QUEUE_CAPACITY: usize = 8;
+const SWEEP_INTERVAL: Duration = crate::handlers::screenshot_worker::SWEEP_INTERVAL;
 
 #[derive(Debug)]
 struct IpcCall {
@@ -61,7 +70,7 @@ impl Drop for IpcSocketGuard {
 
 pub(crate) fn init(
     event_loop: &mut EventLoop<'static, Ferese>,
-) -> Result<IpcSocketGuard, Box<dyn std::error::Error>> {
+) -> Result<ScreenshotInit, Box<dyn std::error::Error>> {
     let path = socket_path()?;
     prepare_parent(&path)?;
     let listener = bind_listener(&path)?;
@@ -79,18 +88,70 @@ pub(crate) fn init(
         .handle()
         .insert_source(receiver, |event, _, state| {
             if let channel::Event::Msg(call) = event {
+                if call.request.command == "screenshot" {
+                    // Deferred: this path answers the caller itself, exactly
+                    // once, whenever the request finishes or is terminated.
+                    state.start_screenshot(call.request, call.response);
+                } else {
                     let response = state.handle_ipc_request(call.request);
                     let _ = call.response.send(response);
                 }
+            }
         })?;
 
+    // Screenshot readbacks publish parts here, so the loop wakes as soon as a
+    // readback lands, whichever backend produced it.
+    let (parts, part_events) = channel::channel::<PartOutcome>();
+    event_loop
+        .handle()
+        .insert_source(part_events, |event, _, state| {
+            if let channel::Event::Msg(outcome) = event {
+                state.on_screenshot_part(outcome);
+            }
+        })?;
+
+    let (encoded, encoded_events) = channel::sync_channel::<Encoded>(RESULT_QUEUE_CAPACITY);
+    event_loop
+        .handle()
+        .insert_source(encoded_events, |event, _, state| {
+            if let channel::Event::Msg(result) = event {
+                state.on_screenshot_encoded(result);
+            }
+        })?;
+    let worker = Worker::spawn(encoded);
+
     let signal = event_loop.get_signal();
     thread::Builder::new()
         .name("ferese-ipc-listener".to_owned())
         .spawn(move || accept_connections(listener, sender, signal))?;
 
+    // A client that dies between receiving a screenshot path and unlinking it
+    // leaves the file behind, so staged files are reclaimed at startup and then
+    // periodically. The TTL keeps this from disturbing a live request.
+    screenshot_worker::sweep_stale_files();
+    event_loop
+        .handle()
+        .insert_source(timer::Timer::from_duration(SWEEP_INTERVAL), |_, _, _| {
+            screenshot_worker::sweep_stale_files();
+            timer::TimeoutAction::ToDuration(SWEEP_INTERVAL)
+        })?;
+
     tracing::info!(path = %path.display(), "Ferese IPC is accepting connections");
-    Ok(guard)
+
+    let Some(worker) = worker else {
+        return Err("Could not start the screenshot worker".into());
+    };
+    Ok(ScreenshotInit {
+        parts,
+        worker,
+        _guard: guard,
+    })
+}
+
+pub(crate) struct ScreenshotInit {
+    pub(crate) parts: PartSender,
+    pub(crate) worker: Worker,
+    pub(crate) _guard: IpcSocketGuard,
 }
 
 fn accept_connections(
@@ -261,6 +322,152 @@ impl Ferese {
         Ok(json!({}))
     }
 
+    // Screenshot replies are deferred: this either answers the caller now, or
+    // hands the reply to the coordinator, which answers exactly once.
+    pub(crate) fn start_screenshot(&mut self, request: Request, response: SyncSender<Response>) {
+        // A macro rather than a closure: the message may be a borrowed str or an
+        // owned String, and Response::error already accepts either.
+        macro_rules! reject {
+            ($code:expr, $message:expr) => {
+                let _ = response.try_send(Response::error(request.id, $code, $message));
+            };
+        }
+        if self.session_lock.active {
+            reject!("session_locked", "IPC unavailable while session is locked");
+            return;
+        }
+        if !screencopy::capture_allowed() {
+            reject!(
+                "capture_disabled",
+                "Screen capture is disabled for this session; start it with \
+                 FERESE_ENABLE_SCREENCOPY=1 to enable screenshots"
+            );
+            return;
+        }
+        if let Err(error) = validate_request(&request) {
+            reject!(error.code, error.message);
+            return;
+        }
+        let geometry = match geometry_arg(&request.args) {
+            Ok(geometry) => geometry,
+            Err(message) => {
+                reject!("invalid_argument", &message);
+                return;
+            }
+        };
+        if self.screenshot_parts.is_none() || self.screenshot_worker.is_none() {
+            reject!("unavailable", "Screenshot capture is not available");
+            return;
+        }
+        let Some(parts) = self.screenshot_parts.clone() else {
+            reject!("unavailable", "Screenshot capture is not available");
+            return;
+        };
+
+        // Snapshot the layout, so a later move, rescale, or transform cannot
+        // change what this request captures.
+        let targets: Vec<(Output, OutputLayout)> = self
+            .space
+            .outputs()
+            .filter_map(|output| {
+                let mode = output.current_mode()?;
+                let geometry = self.space.output_geometry(output)?;
+                Some((
+                    output.clone(),
+                    OutputLayout {
+                        mode_size: mode.size,
+                        scale: output.current_scale().fractional_scale(),
+                        transform: output.current_transform(),
+                        location: geometry.loc,
+                    },
+                ))
+            })
+            .collect();
+        let layouts: Vec<OutputLayout> = targets.iter().map(|(_, layout)| layout.clone()).collect();
+        let planned = match plan(&geometry, &layouts) {
+            Ok(planned) => planned,
+            Err(message) => {
+                reject!("invalid_request", &message);
+                return;
+            }
+        };
+        let specs = planned
+            .iter()
+            .map(|part| part.spec(&layouts[part.index]))
+            .collect();
+
+        // admit stores the reply only on success, so a clone survives the
+        // rejection path and every caller is answered exactly once.
+        let fallback = response.clone();
+        let id = match self.screenshot.admit(request.id, response, specs) {
+            Ok(id) => id,
+            Err(message) => {
+                let _ =
+                    fallback.try_send(Response::error(request.id, "screenshot_rejected", message));
+                return;
+            }
+        };
+
+        for (position, part) in planned.iter().enumerate() {
+            let (output, _) = &targets[part.index];
+            self.pending_screencopies
+                .push(crate::handlers::screencopy::PendingScreencopy::owned(
+                    id,
+                    position,
+                    parts.clone(),
+                    output.clone(),
+                    part.buffer,
+                ));
+        }
+
+        // The nested backend redraws on its refresh timer; this drives the
+        // direct backend immediately and is a no-op otherwise.
+        crate::backends::direct::render_all(self);
+    }
+
+    pub(crate) fn on_screenshot_part(&mut self, outcome: PartOutcome) {
+        let action = self
+            .screenshot
+            .on_part(outcome.request, outcome.part, outcome.result);
+        self.apply_screenshot_action(action);
+    }
+
+    pub(crate) fn on_screenshot_encoded(&mut self, result: Encoded) {
+        let action = self.screenshot.on_encoded(result.request, result.result);
+        self.apply_screenshot_action(action);
+    }
+
+    fn apply_screenshot_action(&mut self, action: Action) {
+        match action {
+            Action::None => {}
+            Action::Encode { request, frames } => {
+                let submitted = self
+                    .screenshot_worker
+                    .as_ref()
+                    .is_some_and(|worker| worker.submit(Job { request, frames }).is_ok());
+                if !submitted {
+                    self.screenshot
+                        .reject(request, "Screenshot encoding queue is full");
+                }
+            }
+            Action::Delivered(path) => {
+                // The caller opens and unlinks it; the path was already sent.
+                tracing::debug!(path = %path.display(), "delivered a screenshot");
+            }
+            Action::DiscardFile(path) => {
+                if let Err(error) = fs::remove_file(&path)
+                    && error.kind() != io::ErrorKind::NotFound
+                {
+                    tracing::debug!(
+                        %error,
+                        path = %path.display(),
+                        "failed to remove a staged screenshot"
+                    );
+                }
+            }
+        }
+    }
+
     fn focused_window_json(&self) -> Value {
         self.focused_window
             .map(|window| json!({ "id": window.0 }))
@@ -413,6 +620,18 @@ impl Ferese {
     }
 }
 
+fn geometry_arg(args: &Value) -> Result<Geometry, String> {
+    match args.get("geometry") {
+        None | Some(Value::Null) => Ok(Geometry::All),
+        Some(value) => {
+            let text = value
+                .as_str()
+                .ok_or("geometry must be a string like \"x,y WxH\"")?;
+            parse_geometry(text)
+        }
+    }
+}
+
 fn validate_request(request: &Request) -> Result<(), CommandError> {
     if request.version != VERSION {
         return Err(CommandError::new(

compositor/src/session_lock.rs

@@ -125,7 +125,16 @@ impl SessionLockHandler for Ferese {
         self.session_lock.owner = Some(confirmation.ext_session_lock().clone());
         self.session_lock.confirmation = Some(confirmation);
         for capture in self.pending_screencopies.drain(..) {
-            capture.frame.failed();
+            capture.fail();
+        }
+        // Draining only covers requests that had not read back yet. One that is
+        // already encoding has no pending capture left, so it is terminated
+        // here and its staged file is discarded when the worker reports back.
+        for request in self.screenshot.terminate_all() {
+            tracing::debug!(
+                request,
+                "cancelled an in-flight screenshot for the session lock"
+            );
         }
         self.set_overview_active(false);
         let serial = SERIAL_COUNTER.next_serial();

compositor/src/state.rs

@@ -174,6 +174,12 @@ pub struct Ferese {
     _ipc_socket: Option<crate::ipc::IpcSocketGuard>,
     pending_dmabuf_imports: Vec<(Dmabuf, ImportNotifier)>,
     pub(crate) pending_screencopies: Vec<crate::handlers::screencopy::PendingScreencopy>,
+    // Screenshot requests outlive the readback that filled them: the
+    // coordinator owns them until encoding finishes and the caller is
+    // answered, or the request is terminated.
+    pub(crate) screenshot: crate::handlers::screenshot::Coordinator,
+    pub(crate) screenshot_parts: Option<crate::handlers::screenshot::PartSender>,
+    pub(crate) screenshot_worker: Option<crate::handlers::screenshot_worker::Worker>,
     pub(crate) shell_resources: Vec<
         smithay::reexports::wayland_server::Weak<
             ferese_protocols::shell::v1::server::ferese_shell_v1::FereseShellV1,
@@ -385,6 +391,9 @@ impl Ferese {
             _ipc_socket: None,
             pending_dmabuf_imports: Vec::new(),
             pending_screencopies: Vec::new(),
+            screenshot: crate::handlers::screenshot::Coordinator::new(),
+            screenshot_parts: None,
+            screenshot_worker: None,
             shell_resources: Vec::new(),
             shell_snapshot_serial: 0,
             last_shell_snapshot: None,
@@ -422,7 +431,10 @@ impl Ferese {
             xdg_shell_state,
             xdg_toplevel_icon_manager,
         };
-        state._ipc_socket = Some(crate::ipc::init(event_loop)?);
+        let screenshot = crate::ipc::init(event_loop)?;
+        state.screenshot_parts = Some(screenshot.parts);
+        state.screenshot_worker = Some(screenshot.worker);
+        state._ipc_socket = Some(screenshot._guard);
         Ok(state)
     }
 
@@ -572,7 +584,7 @@ impl Ferese {
         self.overview_scrims.remove(&output_id);
         self.pending_screencopies.retain(|capture| {
             if capture.output == *output {
-                capture.frame.failed();
+                capture.fail();
                 false
             } else {
                 true

compositor/src/window_rules.rs

@@ -73,7 +73,7 @@ pub fn resolve(
     let mut result = WindowRuleResult::default();
     if matches!(
         app_id.as_deref(),
-        Some("dev.ferese.authentication" | "dev.ferese.screenshare")
+        Some("dev.ferese.authentication" | "dev.ferese.screenshare" | "dev.ferese.screenshot")
     ) {
         result.floating = Some(true);
     }
@@ -220,6 +220,7 @@ mod tests {
             "dev.ferese.ScreenShare",
             "dev.ferese.ScreenShare.desktop",
             "dev.ferese.Authentication",
+            "dev.ferese.Screenshot",
         ] {
             assert_eq!(resolve(&[], Some(app_id), None, false).floating, Some(true));
             let rules = validate(&[WindowRuleConfig {

docs/configuration.md

@@ -294,16 +294,26 @@ Super+M layout; Super+Shift+Space floating; Super+Tab overview;
 Super+Shift+S area screenshot; Print Screen whole-screen screenshot;
 Super+Shift+E immediate logout.
 
+Hold Super and drag a floating window with the left mouse button to move it.
+Hold Super and drag with the right mouse button to resize it from the nearest
+corner. These work even when an app has no title bar or resize border.
+
 The screenshot shortcut runs `ferese-screenshot`: drag to select an area, or
 press Escape to cancel. Captures open in Satty for annotation. Press Enter to
 save the edited PNG under your Pictures directory in `Screenshots` and copy it
-to the clipboard; Escape discards the capture. It requires `grim`, `slurp`,
+to the clipboard; Escape discards the capture. It requires `slurp`, `satty`, and
-`satty`, and `wl-copy` (from `wl-clipboard`). Override the `screenshot` command
+`wl-copy` (from `wl-clipboard`); the capture itself is done by the compositor
-to use another screenshot tool.
+through `feresectl screenshot`. Override the `screenshot` command to use another
-Print Screen runs `ferese-screenshot --full` and captures all enabled outputs
+screenshot tool.
-without a selector. Fn+PrtSc works when the keyboard emits the Print Screen key;
+Print Screen runs `ferese-screenshot --full` and captures the active monitor
-Fn is handled by the keyboard firmware. Override `screenshot-full` to customize
+without a selector, falling back to the sole enabled output when focus is
-this command. Both modes open Satty with the same save and copy workflow.
+unavailable. It never silently captures every output. Fn+PrtSc works when the
+keyboard emits the Print Screen key; Fn is handled by the keyboard firmware.
+Override `screenshot-full` to customize this command. Both modes open Satty with
+the same save and copy workflow.
+
+`ferese-screenshot --all` captures every enabled output as one image.
+
 
 ## Window rules
 
@@ -406,7 +416,11 @@ These are launch-time environment switches, not KDL keys. Enable only for
 trusted clients: `FERESE_ENABLE_INPUT_METHOD=1`,
 `FERESE_ENABLE_SHORTCUT_INHIBIT=1`, `FERESE_ENABLE_SCREENCOPY=1`.
 The installed session launcher enables screencopy for screenshots unless
-`FERESE_ENABLE_SCREENCOPY=0` is explicitly set in its environment.
+`FERESE_ENABLE_SCREENCOPY=0` is explicitly set in its environment. Setting it
+to `0` disables every path that can read the screen, including the built-in
+screenshot command, not just the Wayland protocol. A session launched by hand
+rather than through `ferese-session` needs `FERESE_ENABLE_SCREENCOPY=1` in its
+environment for screenshots to work.
 Text input is available by default; Ctrl+Alt+Escape releases an active shortcut
 inhibitor. Use `feresectl --help` for runtime control commands.
 

docs/installation.md

@@ -203,7 +203,7 @@ Install these through your distribution when you want the corresponding features
 | Feature | Tools or services |
 | --- | --- |
 | Wallpaper file picker | `zenity` |
-| Screenshots and editing | `grim`, `slurp`, `satty`, `wl-clipboard`, Python 3 |
+| Screenshots and editing | `slurp`, `satty`, `wl-clipboard`, Python 3 |
 | Idle locking | `swayidle` with the included `ferese-lock` |
 | Network controls | NetworkManager and its running service |
 | Audio controls | PipeWire, WirePlumber, and `wpctl` |
@@ -244,9 +244,16 @@ A directly launched source build writes to its terminal. Keep another desktop
 available while testing scaling, monitor hotplug, suspend, and locking.
 
 The launcher enables the screencopy protocol for screenshot tools. Set
-`FERESE_ENABLE_SCREENCOPY=0` in the session environment to disable it. When
+`FERESE_ENABLE_SCREENCOPY=0` in the session environment to disable it. This
-enabled, Wayland clients can capture the unlocked desktop. Capture remains
+turns off every path that can read the screen, including the built-in
-blocked while the session is locked. Launcher changes take effect at next login.
+screenshot command, not only the Wayland protocol. When enabled, Wayland clients
+can capture the unlocked desktop, and `feresectl screenshot` is available.
+Capture remains blocked while the session is locked. Launcher changes take
+effect at next login.
+
+Starting the compositor directly instead of through `ferese-session` leaves
+`FERESE_ENABLE_SCREENCOPY` unset, which disables screenshots. Export
+`FERESE_ENABLE_SCREENCOPY=1` to use them.
 
 ## Updating
 

feresectl/src/main.rs

@@ -1,6 +1,7 @@
 use std::env;
 use std::error::Error;
-use std::io;
+use std::fs;
+use std::io::{self, Write};
 use std::os::unix::net::UnixStream;
 use std::path::PathBuf;
 
@@ -13,7 +14,7 @@ fn main() -> Result<(), Box<dyn Error>> {
         version: VERSION,
         id: 1,
         kind: "command".to_owned(),
-        command,
+        command: command.clone(),
         args,
     };
     let mut stream = UnixStream::connect(socket_path()?)?;
@@ -24,11 +25,37 @@ fn main() -> Result<(), Box<dyn Error>> {
         return Err(format!("{}: {}", error.code, error.message).into());
     }
 
+    if command == "screenshot" {
+        write_png(&response)
+    } else {
         println!(
             "{}",
             serde_json::to_string_pretty(&response.result.unwrap_or(Value::Null))?
         );
         Ok(())
+    }
+}
+
+// The compositor stages the PNG in a private directory and replies with its
+// path. Open it before unlinking so a failure never destroys the only copy, and
+// stream the bytes with explicit writes: println! would corrupt binary output.
+fn write_png(response: &Response) -> Result<(), Box<dyn Error>> {
+    let path = response
+        .result
+        .as_ref()
+        .and_then(|result| result.get("path"))
+        .and_then(Value::as_str)
+        .ok_or("the compositor did not return a screenshot path")?;
+    let mut file = fs::File::open(path)?;
+    if let Err(error) = fs::remove_file(path) {
+        // The compositor sweeps anything left behind, so a failure here is not
+        // worth losing the capture over.
+        eprintln!("feresectl: could not remove {path}: {error}");
+    }
+    let mut stdout = io::stdout().lock();
+    io::copy(&mut file, &mut stdout)?;
+    stdout.flush()?;
+    Ok(())
 }
 
 fn parse_args(args: impl IntoIterator<Item = String>) -> Result<(String, Value), String> {
@@ -47,6 +74,18 @@ fn parse_args(args: impl IntoIterator<Item = String>) -> Result<(String, Value),
                 .map_err(|_| format!("{} requires a positive workspace index", command))?;
             json!({ "index": index })
         }
+        "screenshot" => match positional.as_slice() {
+            [] => json!({}),
+            [flag, geometry] if flag == "--geometry" || flag == "-g" => {
+                json!({ "geometry": geometry })
+            }
+            _ => {
+                return Err(format!(
+                    "{command} accepts an optional --geometry \"x,y WxH\" ({})",
+                    usage()
+                ));
+            }
+        },
         "toggle-floating" | "toggle-fullscreen" | "toggle-maximized" | "toggle-layout"
         | "toggle-overview" | "cycle-column-width" | "center-column" | "consume" | "expel"
         | "close" | "get-focused-window" | "get-workspaces" | "get-outputs" | "reload-config"
@@ -79,7 +118,7 @@ fn socket_path() -> Result<PathBuf, io::Error> {
 }
 
 fn usage() -> String {
-    "usage: feresectl <focus|move|resize> <direction>\n       feresectl <workspace|move-to-workspace> <index>\n       feresectl <toggle-floating|toggle-maximized|toggle-fullscreen|toggle-layout|toggle-overview>\n       feresectl <cycle-column-width|center-column|consume|expel|close|exit>\n       feresectl <get-focused-window|get-workspaces|get-outputs|reload-config>".to_owned()
+    "usage: feresectl screenshot [--geometry \"x,y WxH\"]\n       feresectl <focus|move|resize> <direction>\n       feresectl <workspace|move-to-workspace> <index>\n       feresectl <toggle-floating|toggle-maximized|toggle-fullscreen|toggle-layout|toggle-overview>\n       feresectl <cycle-column-width|center-column|consume|expel|close|exit>\n       feresectl <get-focused-window|get-workspaces|get-outputs|reload-config>".to_owned()
 }
 
 #[cfg(test)]
@@ -115,6 +154,38 @@ mod tests {
         );
     }
 
+    #[test]
+    fn screenshot_takes_an_optional_geometry() {
+        assert_eq!(
+            parse_args(["screenshot".into()]).unwrap(),
+            ("screenshot".into(), json!({})),
+            "no geometry captures every enabled output"
+        );
+        assert_eq!(
+            parse_args(["screenshot".into(), "-g".into(), "10,-20 300x200".into()])
+                .unwrap()
+                .1,
+            json!({ "geometry": "10,-20 300x200" }),
+            "negative origins survive argument parsing"
+        );
+        assert_eq!(
+            parse_args(["screenshot".into(), "--geometry".into(), "0,0 8x8".into()])
+                .unwrap()
+                .1,
+            json!({ "geometry": "0,0 8x8" })
+        );
+    }
+
+    #[test]
+    fn screenshot_rejects_stray_arguments() {
+        assert!(parse_args(["screenshot".into(), "extra".into()]).is_err());
+        assert!(parse_args(["screenshot".into(), "-g".into()]).is_err());
+        assert!(
+            parse_args(["screenshot".into(), "0,0 8x8".into()]).is_err(),
+            "a bare geometry is not accepted without the flag"
+        );
+    }
+
     #[test]
     fn rejects_unknown_or_missing_arguments() {
         assert_eq!(

packaging/ferese-screenshot

@@ -25,7 +25,7 @@ case ${1:-} in
 esac
 (($# == 0)) || fail 'Usage: ferese-screenshot [--full | --all]'
 
-for tool in grim satty wl-copy flock; do
+for tool in feresectl satty wl-copy flock python3; do
     command -v "$tool" >/dev/null || fail "Required command not found: $tool"
 done
 : "${XDG_RUNTIME_DIR:?A Wayland login session is required}"
@@ -40,21 +40,18 @@ if [[ $mode == area ]]; then
     [[ -n $geometry ]] || exit 0
     capture_args=(-g "$geometry")
 elif [[ $mode == full ]]; then
-    for tool in feresectl python3; do
-        command -v "$tool" >/dev/null || fail "Required command not found: $tool"
-    done
     # Resolve the active monitor before Satty can change keyboard focus.
-    # Never fall back to grim's all-monitor capture when IPC is unavailable.
+    # Never fall back to an all-monitor capture when IPC is unavailable.
-    output=$(feresectl get-outputs | python3 -c '
+    geometry=$(feresectl get-outputs | python3 -c '
 import json, sys
 outputs = [output for output in json.load(sys.stdin) if output.get("enabled")]
 focused = [output for output in outputs if output.get("focused")]
 selected = focused if len(focused) == 1 else outputs if len(outputs) == 1 else []
-if len(selected) != 1 or not selected[0].get("name"):
+if len(selected) != 1 or any(selected[0].get(k) is None for k in ("x", "y", "width", "height")):
     sys.exit(1)
-print(selected[0]["name"])
+print("%d,%d %dx%d" % (selected[0]["x"], selected[0]["y"], selected[0]["width"], selected[0]["height"]))
 ') || fail 'Could not determine the active monitor. Use area selection or --all.'
-    capture_args=(-o "$output")
+    capture_args=(-g "$geometry")
 fi
 pictures=$(xdg-user-dir PICTURES 2>/dev/null) || pictures="$HOME/Pictures"
 directory=${pictures:-$HOME/Pictures}/Screenshots
@@ -63,12 +60,27 @@ file=$(mktemp "$directory/Screenshot-$(date +%Y-%m-%d_%H-%M-%S)-XXXXXX.png") ||
 input=''
 trap '[[ -z $input ]] || rm -f -- "$input"; [[ -s $file ]] || rm -f -- "$file"' EXIT
 input=$(mktemp "$XDG_RUNTIME_DIR/ferese-screenshot-XXXXXX.png") || fail 'Could not create the temporary capture.'
-grim "${capture_args[@]}" "$input" || fail 'Could not capture the screen. Log back into Ferese after enabling screen capture.'
+feresectl screenshot "${capture_args[@]}" >"$input" || fail 'Could not capture the screen. Ferese must be running, unlocked, and reachable over its socket.'
+editor_size=$(feresectl get-outputs | python3 -c '
+import json, sys
+
+outputs = [output for output in json.load(sys.stdin) if output.get("enabled")]
+focused = [output for output in outputs if output.get("focused")]
+selected = focused if len(focused) == 1 else outputs if len(outputs) == 1 else []
+if len(selected) != 1 or any(selected[0].get(key) is None for key in ("width", "height")):
+    sys.exit(1)
+
+width = max(1, min(1500, round(selected[0]["width"] * 0.8)))
+height = max(1, min(920, round(selected[0]["height"] * 0.8), round(width * 0.68)))
+print(f"{width}x{height}")
+') || editor_size=1000x700
 # Enter saves the annotated image and copies it; Escape discards the capture.
 # Keep the parent lock through editing. Close it only in Satty so clipboard
 # helpers cannot inherit the lock and keep later screenshots blocked.
 satty --filename "$input" --output-filename "$file" \
-    --fullscreen current-screen \
+    --app-id dev.ferese.Screenshot \
+    --resize "$editor_size" \
+    --no-window-decoration \
     --copy-command 'wl-copy --type image/png' \
     --save-after-copy \
     --actions-on-enter save-to-file,save-to-clipboard,exit \

scripts/tests/test_floating_isolated.py

@@ -1,7 +1,10 @@
 """Opt-in rendered regression test; opens only a temporary nested compositor.
 FERESE_TEST_FLOATING=1 FERESE_TEST_BINARY=target/debug/ferese python3 scripts/tests/test_floating_isolated.py
-Requires cc, pkg-config, wayland-scanner, wayland-protocols, grim, and Pillow.
+Requires cc, pkg-config, wayland-scanner, wayland-protocols, and Pillow.
+The compositor and feresectl must be built; set FERESE_TEST_BINARY to point at
+the compositor and FERESE_TEST_CTL at feresectl when they are not siblings.
 """
+import json
 import os
 from pathlib import Path
 import subprocess
@@ -42,6 +45,9 @@ class FloatingSizeTest(unittest.TestCase):
             env = dict(os.environ, XDG_RUNTIME_DIR=str(runtime), XDG_CONFIG_HOME=str(root / "config"),
                        WAYLAND_DISPLAY=display, FERESE_ENABLE_SCREENCOPY="1")
             binary = repo / os.environ.get("FERESE_TEST_BINARY", "target/debug/ferese")
+            ctl = repo / os.environ.get("FERESE_TEST_CTL", str(binary.parent / "feresectl"))
+            if not ctl.is_file():
+                self.fail(f"missing {ctl}; build it with cargo build -p feresectl")
             with (root / "compositor.log").open("w") as log:
                 process = subprocess.Popen([str(binary), "--backend", "nested", "--", str(root / "client")],
                                            env=env, stdout=log, stderr=subprocess.STDOUT)
@@ -56,8 +62,23 @@ class FloatingSizeTest(unittest.TestCase):
                     self.assertTrue(sockets, "nested Wayland socket did not appear")
                     childenv = dict(env, WAYLAND_DISPLAY=str(sockets[0]))
                     time.sleep(2)
+                    listed = subprocess.run([str(ctl), "get-outputs"], env=childenv,
+                                            check=True, capture_output=True, timeout=10, text=True)
+                    enabled = [o for o in json.loads(listed.stdout) if o.get("enabled")]
+                    if not enabled:
+                        self.fail("compositor reported no enabled output to capture")
+                    first = enabled[0]
+                    if any(first.get(k) is None for k in ("x", "y", "width", "height")):
+                        self.fail(f"enabled output {first.get('name')!r} has no logical geometry")
+                    geometry = "%d,%d %dx%d" % (first["x"], first["y"], first["width"], first["height"])
                     screenshot = root / "frame.png"
-                    subprocess.run(["grim", "-s", "1", str(screenshot)], env=childenv, check=True, timeout=15)
+                    with screenshot.open("wb") as png:
+                        captured = subprocess.run([str(ctl), "screenshot", "-g", geometry],
+                                                  env=childenv, stdout=png, stderr=subprocess.PIPE, timeout=15)
+                    if captured.returncode != 0:
+                        self.fail(f"capture of {geometry} failed: {captured.stderr.decode()}")
+                    if screenshot.stat().st_size == 0:
+                        self.fail(f"capture of {geometry} produced an empty file")
                     with Image.open(screenshot) as frame:
                         rgb = frame.convert("RGB")
                         red, green, blue = rgb.split()

scripts/tests/test_lock_isolated.py

@@ -1,6 +1,6 @@
 """Opt-in lock protocol smoke test. Opens a temporary nested compositor.
 Run: FERESE_TEST_LOCK=1 python3 scripts/tests/test_lock_isolated.py
-Requires release binaries, bwrap and grim. Host PAM is never modified.
+Requires release binaries and bwrap. Host PAM is never modified.
 """
 import os
 import unittest
@@ -23,6 +23,8 @@ class NativeLockTest(unittest.TestCase):
         env=dict(os.environ,XDG_RUNTIME_DIR=str(runtime),XDG_CONFIG_HOME=str(root/'config'),WAYLAND_DISPLAY=host_display,FERESE_ENABLE_SCREENCOPY='1')
         log=(root/'compositor.log').open('w')
         compositor=subprocess.Popen(['target/release/ferese','--backend','nested'],env=env,stdout=log,stderr=subprocess.STDOUT)
+        ctl=pathlib.Path('target/release/feresectl')
+        assert ctl.is_file(), f'missing {ctl}; build it with cargo build --release -p feresectl'
         locker=None
         locker_log=(root/'locker.log').open('w')
         try:
@@ -32,19 +34,25 @@ class NativeLockTest(unittest.TestCase):
                 if sockets:break
                 time.sleep(.1)
             childenv=dict(env,WAYLAND_DISPLAY=str(sockets[0]),FERESE_LOCK_READY='1')
+            def capture(destination, timeout=10):
+                with open(destination,'wb') as png:
+                    return subprocess.run([str(ctl),'screenshot'],env=childenv,stdout=png,stderr=subprocess.PIPE,timeout=timeout)
             time.sleep(.6)
-            subprocess.run(['grim',str(root/'unlocked.png')],env=childenv,check=True,timeout=10,capture_output=True)
+            result=capture(root/'unlocked.png')
+            assert result.returncode == 0, f'Capture failed while unlocked: {result.stderr.decode()}'
+            assert (root/'unlocked.png').stat().st_size > 0, 'Capture produced an empty file'
+            print('Native capture succeeded while unlocked',flush=True)
             locker=subprocess.Popen(['bwrap','--bind','/','/','--dev-bind','/dev','/dev','--ro-bind',str(pam),'/etc/pam.d','--unshare-user','--','target/release/ferese-lock'],env=childenv,stdout=subprocess.DEVNULL,stderr=locker_log,start_new_session=True)
             assert locker.wait(timeout=20) == 0, (root/'locker.log').read_text()
             print('Native locker received compositor confirmation',flush=True)
-            result=subprocess.run(['grim',str(root/'locked.png')],env=childenv,timeout=10,capture_output=True)
+            result=capture(root/'locked.png')
             assert result.returncode != 0, 'Capture unexpectedly succeeded during lock'
-            print('Screencopy denied while native locker is running',flush=True)
+            print('Capture denied while native locker is running',flush=True)
             os.killpg(locker.pid, signal.SIGKILL)
             time.sleep(.3)
-            result=subprocess.run(['grim',str(root/'after-crash.png')],env=childenv,timeout=10,capture_output=True)
+            result=capture(root/'after-crash.png')
             assert result.returncode != 0, 'Capture unexpectedly succeeded after locker crash'
-            print('Screencopy remains denied after locker crash',flush=True)
+            print('Capture remains denied after locker crash',flush=True)
         finally:
             if locker is not None:
                 try: os.killpg(locker.pid, signal.SIGKILL)

scripts/tests/test_screenshot.py

@@ -10,88 +10,157 @@ import unittest
 
 SCRIPT = Path(__file__).resolve().parents[2] / 'packaging/ferese-screenshot'
 
+OUTPUTS = [
+    {'name': 'eDP-1', 'enabled': True, 'focused': False,
+     'x': 0, 'y': 0, 'width': 1920, 'height': 1080},
+    {'name': 'HDMI-A-1', 'enabled': True, 'focused': True,
+     'x': 1920, 'y': 0, 'width': 2560, 'height': 1440},
+]
+FOCUSED_GEOMETRY = '1920,0 2560x1440'
 
-class ScreenshotTest(unittest.TestCase):
+FARESECTL = '''#!/bin/bash
-    def test_active_output_all_outputs_and_editor_lock(self):
+case "$1" in
-        with tempfile.TemporaryDirectory() as directory:
+    get-outputs) cat "$TEST_ROOT/outputs.json" ;;
-            root = Path(directory)
+    screenshot)
-            bins = root / 'bin'
+        printf '%s\\n' "$@" >> "$TEST_ROOT/captures"
-            bins.mkdir()
+        printf image
-            env = dict(os.environ, XDG_RUNTIME_DIR=directory, TEST_ROOT=directory,
+        ;;
-                       PATH=f'{bins}:{os.environ["PATH"]}')
+    *) printf 'unexpected feresectl command: %s\\n' "$1" >&2; exit 1 ;;
-            (root / 'outputs.json').write_text(json.dumps([
+esac
-                {'name': 'eDP-1', 'enabled': True, 'focused': False},
+'''
-                {'name': 'HDMI-A-1', 'enabled': True, 'focused': True},
+
-            ]))
+SATTY = '''#!/bin/bash
-            scripts = {
-                'grim': '''#!/bin/bash
-printf '%s\\n' "$@" >> "$TEST_ROOT/captures"
-printf image > "${@: -1}"
-''',
-                'feresectl': '#!/bin/bash\ncat "$TEST_ROOT/outputs.json"\n',
-                'notify-send': '#!/bin/bash\nexit 0\n',
-                'wl-copy': '#!/bin/bash\nexit 0\n',
-                'xdg-user-dir': '#!/bin/bash\nprintf "%s" "$TEST_ROOT/Pictures"\n',
-                'satty': '''#!/bin/bash
 if [[ -e /proc/$$/fd/9 ]]; then touch "$TEST_ROOT/inherited-lock"; fi
+printf '%s\\n' "$@" > "$TEST_ROOT/satty-args"
 printf 'editor\\n' >> "$TEST_ROOT/editors"
 touch "$TEST_ROOT/ready"
 while [[ ! -e "$TEST_ROOT/release" ]]; do sleep 0.02; done
-''',
+'''
+
+
+def fakes():
+    return {
+        'feresectl': FARESECTL,
+        'notify-send': '#!/bin/bash\nexit 0\n',
+        'wl-copy': '#!/bin/bash\nexit 0\n',
+        'xdg-user-dir': '#!/bin/bash\nprintf "%s" "$TEST_ROOT/Pictures"\n',
+        'satty': SATTY,
     }
-            for name, contents in scripts.items():
+
-                path = bins / name
+
+class ScreenshotTest(unittest.TestCase):
+    def setUp(self):
+        self.directory = tempfile.TemporaryDirectory()
+        self.addCleanup(self.directory.cleanup)
+        self.root = Path(self.directory.name)
+        self.bins = self.root / 'bin'
+        self.bins.mkdir()
+        self.env = dict(os.environ, XDG_RUNTIME_DIR=self.directory.name,
+                        TEST_ROOT=str(self.root),
+                        PATH=f'{self.bins}:{os.environ["PATH"]}')
+        self.write_outputs(OUTPUTS)
+        for name, contents in fakes().items():
+            path = self.bins / name
             path.write_text(contents)
             path.chmod(0o755)
-            def run(mode):
+
-                return subprocess.run(['bash', str(SCRIPT), mode], env=env,
+    def write_outputs(self, outputs):
-                                      capture_output=True, timeout=3)
+        (self.root / 'outputs.json').write_text(json.dumps(outputs))
-            first = subprocess.Popen(['bash', str(SCRIPT), '--full'], env=env,
+
+    def captures(self):
+        log = self.root / 'captures'
+        if not log.exists():
+            return []
+        return log.read_text().splitlines()
+
+    def run_script(self, *args, timeout=3):
+        return subprocess.run(['bash', str(SCRIPT), *args], env=self.env,
+                              capture_output=True, timeout=timeout)
+
+    def test_active_output_all_outputs_and_editor_lock(self):
+        first = subprocess.Popen(['bash', str(SCRIPT), '--full'], env=self.env,
                                  stdout=subprocess.PIPE, stderr=subprocess.PIPE)
         try:
             deadline = time.monotonic() + 5
-                while not (root / 'ready').exists():
+            while not (self.root / 'ready').exists():
                 if time.monotonic() > deadline or first.poll() is not None:
                     self.fail('First screenshot did not open its editor')
                 time.sleep(0.02)
-                second = run('--full')
+            second = self.run_script('--full')
             self.assertEqual(second.returncode, 0, second.stderr)
-                self.assertEqual((root / 'editors').read_text().splitlines(), ['editor'])
+            self.assertEqual((self.root / 'editors').read_text().splitlines(), ['editor'])
-                self.assertFalse((root / 'inherited-lock').exists())
+            editor_args = (self.root / 'satty-args').read_text().splitlines()
-                (root / 'release').touch()
+            self.assertEqual(editor_args[editor_args.index('--app-id') + 1],
+                             'dev.ferese.Screenshot')
+            self.assertEqual(editor_args[editor_args.index('--resize') + 1],
+                             '1500x920')
+            self.assertIn('--no-window-decoration', editor_args)
+            self.assertNotIn('--fullscreen', editor_args)
+            self.assertFalse((self.root / 'inherited-lock').exists())
+            (self.root / 'release').touch()
             _, stderr = first.communicate(timeout=3)
             self.assertEqual(first.returncode, 0, stderr)
-                third = run('--full')
+            third = self.run_script('--full')
             self.assertEqual(third.returncode, 0, third.stderr)
-                self.assertEqual((root / 'editors').read_text().splitlines(), ['editor', 'editor'])
+            self.assertEqual((self.root / 'editors').read_text().splitlines(), ['editor', 'editor'])
-                self.assertFalse(list((root / 'Pictures/Screenshots').glob('*.png')))
+            self.assertFalse(list((self.root / 'Pictures/Screenshots').glob('*.png')))
-                self.assertFalse(list(root.glob('ferese-screenshot-*.png')))
+            self.assertFalse(list(self.root.glob('ferese-screenshot-*.png')))
-                captures = (root / 'captures').read_text().splitlines()
+            # Each --full capture asked for the focused output by geometry, not
-                self.assertEqual(captures[0:2], ['-o', 'HDMI-A-1'])
+            # by name, because native capture has no output-name selection.
-                self.assertEqual(captures[3:5], ['-o', 'HDMI-A-1'])
+            self.assertEqual(self.captures()[0:3], ['screenshot', '-g', FOCUSED_GEOMETRY])
-                all_monitors = run('--all')
+            self.assertEqual(self.captures()[3:6], ['screenshot', '-g', FOCUSED_GEOMETRY])
+            all_monitors = self.run_script('--all')
             self.assertEqual(all_monitors.returncode, 0, all_monitors.stderr)
-                captures = (root / 'captures').read_text().splitlines()
+            self.assertEqual(len(self.captures()), 7)  # --all passes only "screenshot"
-                self.assertEqual(len(captures), 7)  # --all passes only the filename
+            self.assertEqual(self.captures()[6], 'screenshot')
-                for outputs in [[], [{'enabled': True, 'name': 'A'}, {'enabled': True, 'name': 'B'}]]:
+            captured = self.captures()
-                    (root / 'outputs.json').write_text(json.dumps(outputs))
+            for outputs in [[], [dict(OUTPUTS[0], focused=False), dict(OUTPUTS[1], focused=False)]]:
-                    unavailable = run('--full')
+                self.write_outputs(outputs)
+                unavailable = self.run_script('--full')
                 self.assertNotEqual(unavailable.returncode, 0)
                 self.assertIn(b'Could not determine the active monitor', unavailable.stderr)
-                    self.assertEqual((root / 'captures').read_text().splitlines(), captures)
+                self.assertEqual(self.captures(), captured)
-                (root / 'outputs.json').write_text(json.dumps([
+            # An enabled output with no geometry is ambiguous, not a reason to
-                    {'name': 'eDP-1', 'enabled': False, 'focused': True},
+            # capture everything.
-                    {'name': 'HDMI-A-1', 'enabled': True, 'focused': False},
+            self.write_outputs([dict(OUTPUTS[1], focused=True, x=None)])
-                ]))
+            unknown = self.run_script('--full')
-                single = run('--full')
+            self.assertNotEqual(unknown.returncode, 0)
+            self.assertIn(b'Could not determine the active monitor', unknown.stderr)
+            self.assertEqual(self.captures(), captured)
+            # Disabled outputs are ignored, so the sole remaining one is used.
+            self.write_outputs([dict(OUTPUTS[0], enabled=False, focused=True),
+                                dict(OUTPUTS[1], focused=False)])
+            single = self.run_script('--full')
             self.assertEqual(single.returncode, 0, single.stderr)
-                self.assertEqual((root / 'captures').read_text().splitlines()[-3:-1], ['-o', 'HDMI-A-1'])
+            self.assertEqual(self.captures()[-2:], ['-g', FOCUSED_GEOMETRY])
         finally:
-                (root / 'release').touch()
+            (self.root / 'release').touch()
             if first.poll() is None:
                 first.terminate()
             first.communicate(timeout=3)
 
+    def test_area_selection_is_passed_through(self):
+        self.bins.joinpath('slurp').write_text(
+            '#!/bin/bash\nprintf "10,-20 300x200"\n')
+        self.bins.joinpath('slurp').chmod(0o755)
+        (self.root / 'release').touch()
+        area = self.run_script()
+        self.assertEqual(area.returncode, 0, area.stderr)
+        self.assertEqual(self.captures(), ['screenshot', '-g', '10,-20 300x200'])
+
+    def test_every_mode_captures_through_the_native_command(self):
+        (self.root / 'release').touch()
+        cases = [(['--all'], ['screenshot']),
+                 (['--full'], ['screenshot', '-g', FOCUSED_GEOMETRY])]
+        for args, expected in cases:
+            with self.subTest(args=args):
+                before = len(self.captures())
+                result = self.run_script(*args)
+                self.assertEqual(result.returncode, 0, result.stderr)
+                # The fake only records these when the compositor command runs,
+                # so a capture taken any other way leaves the log short.
+                self.assertEqual(self.captures()[before:], expected)
+
 
 if __name__ == '__main__':
     unittest.main()