Wayland compositor (wlroots)
git clone https://git.lucas.co/cce-compositor.git
fix: screenshots on 24-bit readback, honest replies, unique names
Three defects, all found while verifying a headless shadow session:
Unsupported 24-bit read format. `to_rgba` knew only the four 8888
formats, so on the NVIDIA GPU — whose GLES readback format is BGR888,
3 bytes/px — every capture died at "unsupported read format 0x34324742".
BGR888/RGB888 now widen to RGBA with forced-opaque alpha, and the
readback sizes its buffer and stride from a new `bytes_per_pixel` rather
than assuming 4 (a 24-bit read into a 4-byte stride shifts every row).
Note the fourcc names read backwards from the memory order: BGR888
arrives R,G,B. Assuming otherwise swapped red and blue in every capture,
which is what a known-colour desktop caught. Verified on both GPUs: the
NVIDIA capture is now pixel-identical to the Intel one.
`ccectl screenshot` reported success optimistically. It replied
`ok <path>` as the capture was parked — a frame before anything was read
back — so a failed capture still handed the user a path to a file that
never appeared. The reply channel now rides along on the
`PendingScreenshot` and is answered where the outcome is known, with a
`Drop` impl covering every path that discards a parked capture (failed
commit, WM reset, output teardown) so nobody waits out a timeout. That
also means the IPC client must wait for a frame: `screenshot` gets a 5s
leash, since a cold readback has been measured over a second and timing
it out would report failure for a capture that lands.
Filename collisions silently overwrote. One-second resolution is not
enough — `ccectl screenshot` followed by `ccectl screenshot window`
produced one file. Names now carry milliseconds and are uniquified with
a `-2`, `-3` suffix against both the filesystem and the names already
issued this run (the file only appears once the encode thread gets to
it, so the filesystem alone cannot see a collision).
Two things noticed on the way:
- `notifications_enabled()` returned true when the config is unreadable,
so an isolated session fires toasts onto the real screen. An absent
*key* still means enabled, but an unreadable *config* now stays quiet.
- `Server::default()` must `ptr::write` both `pending_ipc_reply` and
`pending_screenshot`: an mpsc endpoint has no null niche, so zeroed
bytes decode as `Some(<null channel>)` and dropping that segfaults
(it took out `config::tests::test_my_config` first).
Co-Authored-By: Claude <[email protected]>
src/server/ipc_server.rs | 15 ++-
src/server/output.rs | 3 +-
src/server/screenshot.rs | 273 ++++++++++++++++++++++++++++++++++++++-----
src/server/server.rs | 7 ++
src/server/window_manager.rs | 44 ++++++-
5 files changed, 307 insertions(+), 35 deletions(-)
diff --git a/src/server/ipc_server.rs b/src/server/ipc_server.rs
index 3c90805..d8584ea 100644
--- a/src/server/ipc_server.rs
+++ b/src/server/ipc_server.rs
@@ -67,9 +67,22 @@ fn handle_client(mut stream: UnixStream, tx: mpsc::Sender<IpcRequest>) {
let s = String::from_utf8_lossy(&buf[..n]);
let cmd = s.trim().to_string();
if !cmd.is_empty() {
+ // Commands answer from the IPC drain and so are quick; a
+ // second is a generous leash that still surfaces a wedged
+ // compositor. `screenshot` is the exception: its reply now
+ // waits for the capture, which happens on the next composited
+ // frame, and a cold readback (first capture after an idle
+ // spell — NVIDIA recompiles shaders on the way) has been
+ // measured over a second. Timing that out would report
+ // failure for a capture that lands.
+ let timeout = if cmd.starts_with("screenshot") {
+ std::time::Duration::from_secs(5)
+ } else {
+ std::time::Duration::from_millis(1000)
+ };
let (reply_tx, reply_rx) = mpsc::channel();
if tx.send(IpcRequest { command: cmd, reply_tx }).is_ok() {
- if let Ok(reply) = reply_rx.recv_timeout(std::time::Duration::from_millis(1000)) {
+ if let Ok(reply) = reply_rx.recv_timeout(timeout) {
let _ = stream.write_all(reply.as_bytes());
} else {
let _ = stream.write_all(b"error: timeout processing command\n");
diff --git a/src/server/output.rs b/src/server/output.rs
index 98c7f46..e80fdc8 100644
--- a/src/server/output.rs
+++ b/src/server/output.rs
@@ -607,9 +607,10 @@ impl Output {
// Read the just-committed frame back while the state's buffer is
// still alive; encode/notify happen on a worker thread.
- if let Some(shot) = pending_shot {
+ if let Some(mut shot) = pending_shot {
if state.buffer.is_null() {
log::warn!("screenshot: output state has no buffer");
+ shot.reply_err("screenshot: output state has no buffer");
} else {
crate::screenshot::capture_state_buffer(
(*self.server).renderer,
diff --git a/src/server/screenshot.rs b/src/server/screenshot.rs
index 8ab8425..8f9cd12 100644
--- a/src/server/screenshot.rs
+++ b/src/server/screenshot.rs
@@ -1,14 +1,17 @@
//! Native screenshots.
//!
-//! Two capture paths, both replying over IPC with the destination path and
-//! finishing (PNG encode + notification) on a worker thread:
+//! Two capture paths, both finishing (PNG encode + notification) on a worker
+//! thread, and both answering the waiting `ccectl` only once the readback has
+//! actually produced pixels:
//!
//! - Full-output / region: `process_ipc_command` parks a [`PendingScreenshot`]
//! on the window manager and schedules a frame; `Output::render_and_commit`
//! picks it up right after `wlr_scene_output_build_state` renders the frame
//! into the output state's buffer, and reads that buffer back
//! (`wlr_texture_from_buffer` + `wlr_texture_read_pixels`). Regions are
-//! cropped CPU-side in buffer pixels.
+//! cropped CPU-side in buffer pixels. Because that happens a frame later,
+//! the IPC reply travels with the parked capture (see [`PendingScreenshot`])
+//! instead of being answered optimistically at park time.
//! - Window: the window's committed surface textures are read back directly
//! (root surface + subsurfaces composited by their offsets), so it works
//! even when the window is panned outside the visible viewport — the
@@ -17,19 +20,32 @@
//! Completion is announced through the freedesktop notification daemon
//! (`notify-send` with the standard `image-path` hint, which cce-notifier
//! renders as a thumbnail). The `notifications { screenshots }` key in
-//! config.kdl disables the announcement (default enabled); it is re-read per
-//! screenshot on the worker thread, so edits take effect immediately.
+//! config.kdl disables the announcement (default enabled, but silent when
+//! the config itself cannot be read); it is re-read per screenshot on the
+//! worker thread, so edits take effect immediately.
+//!
+//! Destination names carry milliseconds and are uniquified before being
+//! handed out — a second is long enough for two captures, and the loser used
+//! to overwrite the winner.
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
+use std::sync::mpsc::Sender;
use crate::ffi;
// DRM fourcc codes wlr_texture_preferred_read_format may hand us; all are
-// 8-bit-per-channel, little-endian packed (so ARGB8888 is B,G,R,A in memory).
+// 8-bit-per-channel, little-endian packed. The fourcc name lists channels
+// most-significant first, so the memory order is that name *reversed*:
+// ARGB8888 is B,G,R,A in memory, and BGR888 — despite the name — is R,G,B.
const DRM_FORMAT_XRGB8888: u32 = 0x34325258;
const DRM_FORMAT_ARGB8888: u32 = 0x34325241;
const DRM_FORMAT_XBGR8888: u32 = 0x34324258;
const DRM_FORMAT_ABGR8888: u32 = 0x34324241;
+// The 24-bit pair: 3 bytes/px, no alpha channel at all. NVIDIA's GLES
+// renderer hands these back where Intel's offers a 32-bit format, so a
+// compositor that only knows the 8888 formats cannot screenshot on it.
+const DRM_FORMAT_BGR888: u32 = 0x34324742;
+const DRM_FORMAT_RGB888: u32 = 0x34324752;
/// A full-output / region capture waiting for the next composited frame.
pub struct PendingScreenshot {
@@ -38,39 +54,139 @@ pub struct PendingScreenshot {
/// Crop in output-buffer pixels; `None` captures the whole output.
pub region: Option<ffi::wlr_box>,
pub path: PathBuf,
+ /// The `ccectl` connection still waiting to hear how this went. The
+ /// capture only runs on the next composited frame, so replying `ok
+ /// <path>` at park time reported success before anything had been read
+ /// back — an unsupported read format then left the user holding a path
+ /// to a file that never appeared. Answered by [`Self::reply_ok`] /
+ /// [`Self::reply_err`], or from `Drop` for the paths that discard a
+ /// parked capture (failed commit, WM reset, output teardown).
+ reply: Option<Sender<String>>,
+}
+
+impl PendingScreenshot {
+ pub fn new(
+ output: *mut crate::output::Output,
+ region: Option<ffi::wlr_box>,
+ path: PathBuf,
+ reply: Option<Sender<String>>,
+ ) -> Self {
+ Self { output, region, path, reply }
+ }
+
+ /// Report the capture as landed. Sent once the pixels are in hand and the
+ /// path is settled — the PNG write itself still happens on the encode
+ /// thread and only logs, since holding the reply until a large output is
+ /// compressed would push it past the IPC client's timeout.
+ pub fn reply_ok(&mut self) {
+ let msg = format!("ok {}\n", self.path.display());
+ self.answer(msg);
+ }
+
+ pub fn reply_err(&mut self, msg: &str) {
+ self.answer(format!("error: {msg}\n"));
+ }
+
+ fn answer(&mut self, msg: String) {
+ if let Some(tx) = self.reply.take() {
+ let _ = tx.send(msg);
+ }
+ }
}
-/// `~/Pictures/screenshots/screenshot-YYYYMMDD-HHMMSS.png` (the directory is
-/// created by the encode thread).
+impl Drop for PendingScreenshot {
+ fn drop(&mut self) {
+ // Anything that drops a parked capture without capturing is still an
+ // outcome someone is blocked on; answer rather than let ccectl sit
+ // out its timeout.
+ self.answer("error: screenshot: capture dropped before a frame rendered\n".to_string());
+ }
+}
+
+/// `~/Pictures/screenshots/screenshot-YYYYMMDD-HHMMSS-mmm.png` (the directory
+/// is created by the encode thread).
pub fn default_path() -> PathBuf {
let home = std::env::var("HOME").unwrap_or_else(|_| ".".to_string());
+ let dir = PathBuf::from(home).join("Pictures").join("screenshots");
+ unique_path(&dir, ×tamp_stem())
+}
+
+/// Millisecond-resolution stem. Seconds were not enough: `ccectl screenshot`
+/// followed by `ccectl screenshot window` lands inside one second, and the
+/// second capture silently overwrote the first.
+fn timestamp_stem() -> String {
+ let mut ts: libc::timespec = unsafe { std::mem::zeroed() };
+ unsafe { libc::clock_gettime(libc::CLOCK_REALTIME, &mut ts) };
let mut tm: libc::tm = unsafe { std::mem::zeroed() };
- let now = unsafe { libc::time(std::ptr::null_mut()) };
- unsafe { libc::localtime_r(&now, &mut tm) };
- let name = format!(
- "screenshot-{:04}{:02}{:02}-{:02}{:02}{:02}.png",
+ unsafe { libc::localtime_r(&ts.tv_sec, &mut tm) };
+ format!(
+ "screenshot-{:04}{:02}{:02}-{:02}{:02}{:02}-{:03}",
tm.tm_year + 1900,
tm.tm_mon + 1,
tm.tm_mday,
tm.tm_hour,
tm.tm_min,
- tm.tm_sec
- );
- PathBuf::from(home).join("Pictures").join("screenshots").join(name)
+ tm.tm_sec,
+ ts.tv_nsec / 1_000_000,
+ )
}
-/// Read a texture's full contents into a tightly packed `w*h*4` byte buffer.
+/// `<stem>.png`, then `<stem>-2.png`, … until the name is free.
+///
+/// Checking the filesystem alone is not enough: the file is created by the
+/// encode thread well after the path is handed out, so two captures in the
+/// same millisecond would both see an empty directory. Hence the set of names
+/// already issued this run (one `PathBuf` per capture, never reclaimed —
+/// bounded by how many screenshots a session takes).
+fn unique_path(dir: &Path, stem: &str) -> PathBuf {
+ static ISSUED: std::sync::LazyLock<std::sync::Mutex<std::collections::HashSet<PathBuf>>> =
+ std::sync::LazyLock::new(|| std::sync::Mutex::new(std::collections::HashSet::new()));
+ let mut issued = ISSUED.lock().unwrap_or_else(|e| e.into_inner());
+ let mut n = 1u32;
+ loop {
+ let path = dir.join(match n {
+ 1 => format!("{stem}.png"),
+ n => format!("{stem}-{n}.png"),
+ });
+ if !issued.contains(&path) && !path.exists() {
+ issued.insert(path.clone());
+ return path;
+ }
+ n += 1;
+ }
+}
+
+/// Bytes per pixel of the formats [`to_rgba`] can convert; `None` for
+/// anything else, which is the one place that knowledge is written down.
+fn bytes_per_pixel(format: u32) -> Option<usize> {
+ match format {
+ DRM_FORMAT_XRGB8888 | DRM_FORMAT_ARGB8888 | DRM_FORMAT_XBGR8888 | DRM_FORMAT_ABGR8888 => {
+ Some(4)
+ }
+ DRM_FORMAT_BGR888 | DRM_FORMAT_RGB888 => Some(3),
+ _ => None,
+ }
+}
+
+/// Read a texture's full contents into a tightly packed `w*h*bpp` byte buffer.
/// Returns the bytes plus the DRM format they are in.
unsafe fn read_texture(texture: *mut ffi::wlr_texture, w: i32, h: i32) -> Option<(Vec<u8>, u32)> {
if texture.is_null() || w <= 0 || h <= 0 {
return None;
}
let format = ffi::wlr_texture_preferred_read_format(texture);
- let mut data = vec![0u8; (w as usize) * (h as usize) * 4];
+ // Both the buffer and the stride must be sized for the format the
+ // renderer is about to write: a 24-bit format read into a 4-byte-strided
+ // buffer would leave every row short and shifted.
+ let Some(bpp) = bytes_per_pixel(format) else {
+ log::warn!("screenshot: unsupported read format {format:#x}");
+ return None;
+ };
+ let mut data = vec![0u8; (w as usize) * (h as usize) * bpp];
let options = ffi::wlr_texture_read_pixels_options {
data: data.as_mut_ptr() as *mut std::ffi::c_void,
format,
- stride: (w as u32) * 4,
+ stride: (w as u32) * bpp as u32,
dst_x: 0,
dst_y: 0,
src_box: std::mem::zeroed(), // empty = full texture
@@ -81,8 +197,9 @@ unsafe fn read_texture(texture: *mut ffi::wlr_texture, w: i32, h: i32) -> Option
Some((data, format))
}
-/// Convert read-back pixels to RGBA in place. Alpha is forced opaque — the
-/// X-variants carry garbage alpha, and screenshots should not be translucent.
+/// Convert read-back pixels to RGBA. Alpha is forced opaque — the X-variants
+/// carry garbage alpha, the 24-bit formats carry none at all, and screenshots
+/// should not be translucent.
fn to_rgba(mut pixels: Vec<u8>, format: u32) -> Option<Vec<u8>> {
match format {
DRM_FORMAT_XRGB8888 | DRM_FORMAT_ARGB8888 => {
@@ -98,6 +215,9 @@ fn to_rgba(mut pixels: Vec<u8>, format: u32) -> Option<Vec<u8>> {
}
Some(pixels)
}
+ // 24-bit: widen rather than swizzle in place.
+ DRM_FORMAT_BGR888 => Some(widen_24(&pixels, false)),
+ DRM_FORMAT_RGB888 => Some(widen_24(&pixels, true)),
_ => {
log::warn!("screenshot: unsupported read format {format:#x}");
None
@@ -105,6 +225,23 @@ fn to_rgba(mut pixels: Vec<u8>, format: u32) -> Option<Vec<u8>> {
}
}
+/// 3-byte pixels to RGBA with opaque alpha. `swap_rb` covers RGB888, whose
+/// memory order is B,G,R; BGR888 is already R,G,B. (Reversed from how the
+/// names read — verified against a known-colour desktop on NVIDIA, which
+/// hands back BGR888: assuming the intuitive order swapped every capture's
+/// red and blue.)
+fn widen_24(pixels: &[u8], swap_rb: bool) -> Vec<u8> {
+ let mut out = Vec::with_capacity(pixels.len() / 3 * 4);
+ for px in pixels.chunks_exact(3) {
+ if swap_rb {
+ out.extend_from_slice(&[px[2], px[1], px[0], 255]);
+ } else {
+ out.extend_from_slice(&[px[0], px[1], px[2], 255]);
+ }
+ }
+ out
+}
+
fn crop_rgba(pixels: &[u8], w: i32, h: i32, region: ffi::wlr_box) -> Option<(Vec<u8>, i32, i32)> {
let x0 = region.x.clamp(0, w);
let y0 = region.y.clamp(0, h);
@@ -129,31 +266,38 @@ pub unsafe fn capture_state_buffer(
buffer: *mut ffi::wlr_buffer,
buf_w: i32,
buf_h: i32,
- shot: PendingScreenshot,
+ mut shot: PendingScreenshot,
) {
let texture = ffi::wlr_texture_from_buffer(renderer, buffer);
if texture.is_null() {
log::warn!("screenshot: wlr_texture_from_buffer failed");
+ shot.reply_err("screenshot: wlr_texture_from_buffer failed");
return;
}
let read = read_texture(texture, buf_w, buf_h);
ffi::wlr_texture_destroy(texture);
let Some((pixels, format)) = read else {
log::warn!("screenshot: pixel readback failed");
+ shot.reply_err("screenshot: pixel readback failed");
+ return;
+ };
+ let Some(rgba) = to_rgba(pixels, format) else {
+ shot.reply_err(&format!("screenshot: unsupported read format {format:#x}"));
return;
};
- let Some(rgba) = to_rgba(pixels, format) else { return };
let (rgba, out_w, out_h) = match shot.region {
Some(region) => match crop_rgba(&rgba, buf_w, buf_h, region) {
Some(cropped) => cropped,
None => {
log::warn!("screenshot: region outside the output");
+ shot.reply_err("screenshot: region outside the output");
return;
}
},
None => (rgba, buf_w, buf_h),
};
- spawn_encode(rgba, out_w as u32, out_h as u32, shot.path);
+ shot.reply_ok();
+ spawn_encode(rgba, out_w as u32, out_h as u32, shot.path.clone());
}
/// Window capture straight from the committed surface textures: the root
@@ -284,11 +428,16 @@ fn spawn_encode(rgba: Vec<u8>, w: u32, h: u32, path: PathBuf) {
});
}
-/// `notifications { screenshots <bool> }` in the shared config.kdl; absent
-/// means enabled.
+/// `notifications { screenshots <bool> }` in the shared config.kdl. Absent
+/// *key* in a readable config means enabled — that is the documented default.
+/// An unreadable config is a different thing: we know nothing about the
+/// user's wishes, and a session running against a config we cannot read is
+/// typically an isolated one (a headless shadow session, say) whose toasts
+/// would land on someone else's screen. Stay quiet there.
fn notifications_enabled() -> bool {
let Ok(content) = std::fs::read_to_string(cce_ui::config::get_config_path()) else {
- return true;
+ log::debug!("screenshot: config unreadable, staying quiet about the capture");
+ return false;
};
cce_ui::config::parse_kdl_to_json(&content)
.pointer("/notifications/screenshots")
@@ -308,9 +457,79 @@ mod tests {
// RGBA passthrough, alpha forced opaque.
let out = to_rgba(vec![1, 2, 3, 4], DRM_FORMAT_ABGR8888).unwrap();
assert_eq!(out, vec![1, 2, 3, 255]);
+ // 24-bit, two pixels. The names read backwards from the memory
+ // order: BGR888 arrives R,G,B and widens as-is, RGB888 arrives
+ // B,G,R and needs the swap. Both gain an alpha they never carried.
+ let out = to_rgba(vec![1, 2, 3, 4, 5, 6], DRM_FORMAT_BGR888).unwrap();
+ assert_eq!(out, vec![1, 2, 3, 255, 4, 5, 6, 255]);
+ let out = to_rgba(vec![1, 2, 3, 4, 5, 6], DRM_FORMAT_RGB888).unwrap();
+ assert_eq!(out, vec![3, 2, 1, 255, 6, 5, 4, 255]);
assert!(to_rgba(vec![0; 4], 0x1234).is_none());
}
+ #[test]
+ fn bytes_per_pixel_matches_what_to_rgba_accepts() {
+ // The readback allocates and strides by this, so a format to_rgba
+ // handles must have a size here and vice versa.
+ for (format, bpp) in [
+ (DRM_FORMAT_XRGB8888, 4usize),
+ (DRM_FORMAT_ARGB8888, 4),
+ (DRM_FORMAT_XBGR8888, 4),
+ (DRM_FORMAT_ABGR8888, 4),
+ (DRM_FORMAT_BGR888, 3),
+ (DRM_FORMAT_RGB888, 3),
+ ] {
+ assert_eq!(bytes_per_pixel(format), Some(bpp), "{format:#x}");
+ // One pixel's worth of bytes converts to exactly one RGBA pixel.
+ assert_eq!(to_rgba(vec![0; bpp], format).map(|p| p.len()), Some(4));
+ }
+ assert_eq!(bytes_per_pixel(0x1234), None);
+ }
+
+ #[test]
+ fn unique_path_never_reuses_a_name() {
+ let dir = std::env::temp_dir().join(format!("cce-shot-test-{}", std::process::id()));
+ std::fs::create_dir_all(&dir).unwrap();
+ // Same stem twice: the second capture must not be handed the first
+ // one's path, even though the encode thread has created no file yet.
+ let a = unique_path(&dir, "screenshot-20260815-120000-000");
+ let b = unique_path(&dir, "screenshot-20260815-120000-000");
+ assert_ne!(a, b);
+ assert!(a.ends_with("screenshot-20260815-120000-000.png"));
+ assert!(b.ends_with("screenshot-20260815-120000-000-2.png"));
+ // A name already on disk is skipped too (a stem reused across runs).
+ std::fs::write(dir.join("screenshot-20260815-130000-000.png"), b"").unwrap();
+ let c = unique_path(&dir, "screenshot-20260815-130000-000");
+ assert!(c.ends_with("screenshot-20260815-130000-000-2.png"));
+ let _ = std::fs::remove_dir_all(&dir);
+ }
+
+ #[test]
+ fn pending_screenshot_answers_exactly_once() {
+ let (tx, rx) = std::sync::mpsc::channel();
+ let mut shot = PendingScreenshot::new(
+ std::ptr::null_mut(),
+ None,
+ PathBuf::from("/tmp/shot.png"),
+ Some(tx),
+ );
+ shot.reply_ok();
+ assert_eq!(rx.recv().unwrap(), "ok /tmp/shot.png\n");
+ drop(shot); // already answered: Drop must not send a second verdict
+ assert!(rx.recv().is_err());
+
+ // A capture discarded before it ran answers from Drop, so ccectl
+ // hears an error instead of sitting out its timeout.
+ let (tx, rx) = std::sync::mpsc::channel();
+ drop(PendingScreenshot::new(
+ std::ptr::null_mut(),
+ None,
+ PathBuf::from("/tmp/shot.png"),
+ Some(tx),
+ ));
+ assert!(rx.recv().unwrap().starts_with("error: "));
+ }
+
#[test]
fn crop_clamps_to_bounds() {
// 2x2 image, pixels numbered 0..4 in the red channel.
diff --git a/src/server/server.rs b/src/server/server.rs
index 51e5f5e..a798a62 100644
--- a/src/server/server.rs
+++ b/src/server/server.rs
@@ -875,6 +875,13 @@ impl Default for Server {
std::ptr::write(&mut (*server.as_mut_ptr()).wm.pointer_binds, Vec::new());
std::ptr::write(&mut (*server.as_mut_ptr()).wm.gesture_binds, Vec::new());
std::ptr::write(&mut (*server.as_mut_ptr()).wm.ipc_rx, None);
+ // Same reason as ipc_rx above, and not optional: an mpsc endpoint
+ // has no null niche, so `Option` tags it out of band and zeroed
+ // bytes decode as `Some(<null channel>)` — dropping that segfaults.
+ // pending_screenshot holds one too (its deferred IPC reply), which
+ // is what makes zeroed bytes decode as a live `Some` there as well.
+ std::ptr::write(&mut (*server.as_mut_ptr()).wm.pending_ipc_reply, None);
+ std::ptr::write(&mut (*server.as_mut_ptr()).wm.pending_screenshot, None);
std::ptr::write(&mut (*server.as_mut_ptr()).wm.startup, Vec::new());
std::ptr::write(&mut (*server.as_mut_ptr()).wm.startup_pids, Vec::new());
std::ptr::write(&mut (*server.as_mut_ptr()).wm.status_sender, None);
diff --git a/src/server/window_manager.rs b/src/server/window_manager.rs
index 46e09c1..58a2b98 100644
--- a/src/server/window_manager.rs
+++ b/src/server/window_manager.rs
@@ -98,6 +98,12 @@ pub struct WindowManager {
/// A full-output/region screenshot parked for the next composited frame
/// (`ccectl screenshot`); consumed by `Output::render_and_commit`.
pub pending_screenshot: Option<crate::screenshot::PendingScreenshot>,
+ /// The reply channel of the IPC command currently being dispatched, so a
+ /// command that cannot answer yet can carry it away and answer later
+ /// (only `screenshot` does). Set by `handle_ipc_timer` around the
+ /// dispatch; if it is still here afterwards, the command answered
+ /// synchronously and the timer sends its return value.
+ pub pending_ipc_reply: Option<std::sync::mpsc::Sender<String>>,
pub startup: Vec<crate::config::StartupConfig>,
pub startup_pids: Vec<(crate::config::StartupConfig, nix::unistd::Pid)>,
pub status_sender: Option<crate::status_server::StatusSender>,
@@ -264,6 +270,7 @@ impl WindowManager {
self.viewport_settle_timer = std::ptr::null_mut();
self.desk_zoom = 1.0;
self.pending_screenshot = None;
+ self.pending_ipc_reply = None;
self.mode = WindowManagerMode::Normal;
self.restore_queue = Vec::new();
self.last_window_states = Vec::new();
@@ -2620,7 +2627,15 @@ impl WindowManager {
Action::Screenshot => {
// Same capture as `ccectl screenshot`: the enabled output's
// next frame, saved under ~/Pictures/screenshots.
+ //
+ // Hide any borrowed reply channel first: this action can be
+ // reached from an IPC command of its own, and the screenshot
+ // dispatch takes the channel to answer later — which would
+ // hand this capture's verdict to whoever asked for the
+ // *action*, and leave them waiting a frame for it.
+ let borrowed = self.pending_ipc_reply.take();
let _ = self.process_ipc_command("screenshot");
+ self.pending_ipc_reply = borrowed;
}
Action::Reload => {
log::info!("monolithic execute_action: Reload requested");
@@ -3305,13 +3320,22 @@ impl WindowManager {
}
});
- self.pending_screenshot = Some(crate::screenshot::PendingScreenshot {
- output: target_out,
+ // The capture happens a frame from now, in
+ // `Output::render_and_commit`, so the reply channel
+ // rides along with it: answering `ok <path>` here
+ // claimed success for captures that then failed (an
+ // unsupported readback format, a failed commit) and
+ // named a file that never appeared. Taking the
+ // channel is what tells `handle_ipc_timer` not to
+ // answer, so the returned string goes nowhere.
+ self.pending_screenshot = Some(crate::screenshot::PendingScreenshot::new(
+ target_out,
region,
- path: path.clone(),
- });
+ path,
+ self.pending_ipc_reply.take(),
+ ));
ffi::wlr_output_schedule_frame((*target_out).wlr_output);
- format!("ok {}\n", path.display())
+ String::new()
}
Some(other) => format!("error: unknown screenshot target: {}\n", other),
}
@@ -3910,8 +3934,16 @@ unsafe extern "C" fn handle_ipc_timer(data: *mut std::ffi::c_void) -> std::os::r
if let Some(ref rx) = (*wm).ipc_rx {
while let Ok(req) = rx.try_recv() {
+ // Lend the reply channel to the dispatch: a command whose real
+ // outcome is only known later (`screenshot`, which lands a frame
+ // from now) takes it and answers itself. If it is still here, the
+ // command answered synchronously and its return value is the
+ // reply.
+ (*wm).pending_ipc_reply = Some(req.reply_tx);
let reply = (*wm).process_ipc_command(&req.command);
- let _ = req.reply_tx.send(reply);
+ if let Some(tx) = (*wm).pending_ipc_reply.take() {
+ let _ = tx.send(reply);
+ }
}
}