From fbdc8da28f93055c3f0f03a0861457f07ae92b0f Mon Sep 17 00:00:00 2001 From: elwrcl Date: Tue, 29 Sep 2026 21:32:09 +0300 Subject: [PATCH] docs: keep frames in BGRX order for a fast overlay; plan regenerated --- .../plans/2026-09-29-valw-phase-0-1.md | 316 +++++++++++------- .../specs/2026-09-29-valw-phase-0-1-design.md | 27 +- 2 files changed, 210 insertions(+), 133 deletions(-) diff --git a/docs/superpowers/plans/2026-09-29-valw-phase-0-1.md b/docs/superpowers/plans/2026-09-29-valw-phase-0-1.md index c5f2b2a..169dfb2 100644 --- a/docs/superpowers/plans/2026-09-29-valw-phase-0-1.md +++ b/docs/superpowers/plans/2026-09-29-valw-phase-0-1.md @@ -4,7 +4,7 @@ **Goal:** Ship `valw screen`, `valw region` and `valw doctor`: pixel-exact PNG screenshots on niri that are saved, copied to the clipboard, logged, and explained when they fail. Everything is packaged as a dendritic Nix flake. -**Architecture:** One short-lived Rust process per command. The process captures every needed output up front through wlr-screencopy. For `region` it shows the frozen frames in layer-shell overlays and crops in memory. It then writes the PNG and forks a small child that serves the clipboard. Pure logic (pixel formats, selection math, overlay pixels, file names, config, errors) is kept apart from the SCTK/Wayland glue so it can be unit tested without a compositor. +**Architecture:** One short-lived Rust process per command. The process captures every needed output up front through wlr-screencopy and keeps the pixels in the compositor's own byte order (B, G, R, X), so the overlay appears fast even on a slow machine. For `region` it shows the frozen frames in layer-shell overlays and crops in memory; conversion to RGBA happens only for what gets saved. It then writes the PNG and forks a small child that serves the clipboard. Pure logic (pixel formats, selection math, overlay pixels, file names, config, errors) is kept apart from the SCTK/Wayland glue so it can be unit tested without a compositor. **Tech Stack:** Rust 2024 (stable, from nixpkgs), smithay-client-toolkit 0.21, wayland-protocols(-wlr), niri-ipc 26.4.0, wl-clipboard-rs, image (png), clap, serde/toml, tracing, rustix/libc, anyhow; Nix flake-parts with `*.ulu.nix` auto-discovery; jj. @@ -53,9 +53,9 @@ These are the input classes most likely to bite a real user that no unit test fu | `src/log.rs` | per-run log file, pruning, stderr warnings, panic hook | | `src/lock.rs` | single-instance `flock` | | `src/detach.rs` | fork a child that outlives valw with no lock and no stdio | -| `src/frame.rs` | `OutputGeom`, `Frame`, shm format → RGBA, output transforms | +| `src/frame.rs` | `OutputGeom`, `PixelRect`, `Frame` (B,G,R,X pixels), shm format → BGRX, RGBA for saving, output transforms | | `src/selection.rs` | drag state machine; logical → physical pixel rectangles | -| `src/render.rs` | overlay pixels: dim, paste bright selection, outline | +| `src/render.rs` | overlay pixels: dim, paste undimmed selection, outline | | `src/output.rs` | file names, unique paths, PNG encode, atomic write, stdout, clipboard | | `src/niri.rs` | niri IPC: focused output, version, version parsing | | `src/wayland.rs` | SCTK connection, state, handlers, dispatch helpers | @@ -1136,7 +1136,7 @@ jj commit -m "feat: single-instance lock and detached background child" **Interfaces:** - Consumes: nothing. -- Produces: `pub struct OutputGeom { pub name: String, pub x: i32, pub y: i32, pub width: i32, pub height: i32 }` (logical, global coordinates); `pub struct Frame { pub output: OutputGeom, pub image: RgbaImage }`; `pub const SUPPORTED: [Format; 4]` (preference order); `pub fn to_rgba(format: Format, width: u32, height: u32, stride: u32, data: &[u8], y_invert: bool) -> Result`; `pub fn apply_transform(img: RgbaImage, transform: Transform) -> RgbaImage`. `Format` is `wayland_client::protocol::wl_shm::Format`, `Transform` is `wayland_client::protocol::wl_output::Transform`. +- Produces: `pub struct OutputGeom { pub name: String, pub x: i32, pub y: i32, pub width: i32, pub height: i32 }` (logical, global coordinates); `pub struct PixelRect { pub x: u32, pub y: u32, pub width: u32, pub height: u32 }` (a frame's physical pixels); `pub type Bgrx = RgbaImage` (container whose bytes are B, G, R, X, i.e. XRGB8888 in memory); `pub struct Frame { pub output: OutputGeom, pub pixels: Arc }` with `full(&self) -> PixelRect` and `to_rgba(&self, rect: PixelRect) -> RgbaImage` (opaque); `pub const SUPPORTED: [Format; 4]` (preference order); `pub fn to_bgrx(format: Format, width: u32, height: u32, stride: u32, data: &[u8], y_invert: bool) -> Result`; `pub fn apply_transform(img: Bgrx, transform: Transform) -> Bgrx`. `Format` is `wayland_client::protocol::wl_shm::Format`, `Transform` is `wayland_client::protocol::wl_output::Transform`. - [ ] **Step 1: Write the failing tests for `frame`** @@ -1155,12 +1155,17 @@ mod tests { } } + fn bgr(img: &Bgrx, x: u32, y: u32) -> [u8; 3] { + let p = img.get_pixel(x, y).0; + [p[0], p[1], p[2]] + } + #[test] - fn converts_all_supported_formats_to_opaque_rgba() { + fn reads_all_supported_formats_as_bgrx() { for format in SUPPORTED { - let img = to_rgba(format, 2, 1, 8, &two_pixels(format), false).unwrap(); - assert_eq!(img.get_pixel(0, 0).0, [255, 0, 0, 255], "{format:?}"); - assert_eq!(img.get_pixel(1, 0).0, [0, 0, 255, 255], "{format:?}"); + let img = to_bgrx(format, 2, 1, 8, &two_pixels(format), false).unwrap(); + assert_eq!(bgr(&img, 0, 0), [0, 0, 255], "red, {format:?}"); + assert_eq!(bgr(&img, 1, 0), [255, 0, 0], "blue, {format:?}"); } } @@ -1168,38 +1173,76 @@ mod tests { fn honours_stride_padding() { // One pixel per row, 8-byte stride. let data = [0, 0, 255, 0, 9, 9, 9, 9, 255, 0, 0, 0, 9, 9, 9, 9]; - let img = to_rgba(Format::Xrgb8888, 1, 2, 8, &data, false).unwrap(); - assert_eq!(img.get_pixel(0, 0).0, [255, 0, 0, 255]); - assert_eq!(img.get_pixel(0, 1).0, [0, 0, 255, 255]); + let img = to_bgrx(Format::Xrgb8888, 1, 2, 8, &data, false).unwrap(); + assert_eq!(img.as_raw().len(), 8); + assert_eq!(bgr(&img, 0, 0), [0, 0, 255]); + assert_eq!(bgr(&img, 0, 1), [255, 0, 0]); } #[test] fn y_invert_flips_rows() { let data = [0, 0, 255, 0, 255, 0, 0, 0]; - let img = to_rgba(Format::Xrgb8888, 1, 2, 4, &data, true).unwrap(); - assert_eq!(img.get_pixel(0, 0).0, [0, 0, 255, 255]); - assert_eq!(img.get_pixel(0, 1).0, [255, 0, 0, 255]); + let img = to_bgrx(Format::Xrgb8888, 1, 2, 4, &data, true).unwrap(); + assert_eq!(bgr(&img, 0, 0), [255, 0, 0]); + assert_eq!(bgr(&img, 0, 1), [0, 0, 255]); } #[test] fn rejects_unknown_format_by_name() { - let err = to_rgba(Format::Xbgr2101010, 1, 1, 4, &[0; 4], false).unwrap_err(); + let err = to_bgrx(Format::Xbgr2101010, 1, 1, 4, &[0; 4], false).unwrap_err(); assert_eq!(err.to_string(), "unsupported buffer format Xbgr2101010"); } #[test] fn rejects_short_buffer() { - assert!(to_rgba(Format::Xrgb8888, 2, 2, 8, &[0; 8], false).is_err()); + assert!(to_bgrx(Format::Xrgb8888, 2, 2, 8, &[0; 8], false).is_err()); } - fn marked() -> RgbaImage { + fn frame(width: u32, height: u32, data: Vec) -> Frame { + Frame { + output: OutputGeom { + name: "A".into(), + x: 0, + y: 0, + width: width as i32, + height: height as i32, + }, + pixels: Arc::new(Bgrx::from_raw(width, height, data).unwrap()), + } + } + + #[test] + fn to_rgba_swaps_channels_and_makes_opaque() { + // Red with alpha 0, as an ARGB8888 buffer may deliver it. + let f = frame(1, 1, vec![0, 0, 255, 0]); + assert_eq!(f.to_rgba(f.full()).get_pixel(0, 0).0, [255, 0, 0, 255]); + } + + #[test] + fn to_rgba_crops() { + // 3x2: pixel value = its index, in the blue byte. + let data = (0u8..6).flat_map(|i| [i, 0, 0, 0]).collect(); + let f = frame(3, 2, data); + let rect = PixelRect { + x: 1, + y: 1, + width: 2, + height: 1, + }; + let out = f.to_rgba(rect); + assert_eq!(out.dimensions(), (2, 1)); + assert_eq!(out.get_pixel(0, 0).0, [0, 0, 4, 255]); + assert_eq!(out.get_pixel(1, 0).0, [0, 0, 5, 255]); + } + + fn marked() -> Bgrx { // 3x2, top-left pixel marked. - let mut img = RgbaImage::new(3, 2); + let mut img = Bgrx::new(3, 2); img.put_pixel(0, 0, image::Rgba([255, 255, 255, 255])); img } - fn marked_at(img: &RgbaImage) -> (u32, u32, u32, u32) { + fn marked_at(img: &Bgrx) -> (u32, u32, u32, u32) { let (x, y, _) = img .enumerate_pixels() .find(|(_, _, p)| p.0[0] == 255) @@ -1232,13 +1275,15 @@ Add `mod frame;` to the `mod` list at the top of `src/main.rs` (keep it sorted). - [ ] **Step 2: Run the tests and watch them fail** Run: `cargo nextest run frame::` -Expected: compilation fails, e.g. `` cannot find function `to_rgba` in this scope ``. +Expected: compilation fails, e.g. `` cannot find function `to_bgrx` in this scope ``. - [ ] **Step 3: Implement `frame`** Put this above the test module in `src/frame.rs`: ```rust +use std::sync::Arc; + use anyhow::{Result, bail, ensure}; use image::{RgbaImage, imageops}; use wayland_client::protocol::{wl_output::Transform, wl_shm::Format}; @@ -1253,11 +1298,57 @@ pub struct OutputGeom { pub height: i32, } -/// One captured output: upright RGBA pixels at physical resolution. +/// A rectangle in a frame's physical pixels. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct PixelRect { + pub x: u32, + pub y: u32, + pub width: u32, + pub height: u32, +} + +/// Pixels in wl_shm XRGB8888 memory order: B, G, R, X. This is what niri +/// hands out and what the overlay shows, so nothing on the way to the first +/// overlay frame has to reorder bytes. `RgbaImage` is only the container; +/// its channels do not mean R, G, B, A here. +pub type Bgrx = RgbaImage; + +/// One captured output: upright pixels at physical resolution. #[derive(Debug, Clone)] pub struct Frame { pub output: OutputGeom, - pub image: RgbaImage, + /// Shared with the overlay, which shows it as the undimmed selection. + pub pixels: Arc, +} + +impl Frame { + /// The whole frame as a rectangle. + pub fn full(&self) -> PixelRect { + PixelRect { + x: 0, + y: 0, + width: self.pixels.width(), + height: self.pixels.height(), + } + } + + /// The `rect` part as opaque RGBA, for saving. + /// + /// Alpha is always 255: a screenshot of what's on screen is opaque, even + /// where the compositor's buffer has alpha 0. + pub fn to_rgba(&self, rect: PixelRect) -> RgbaImage { + let stride = self.pixels.width() as usize * 4; + let raw = self.pixels.as_raw(); + let mut out = RgbaImage::new(rect.width, rect.height); + for (y, row) in out.rows_mut().enumerate() { + let start = (rect.y as usize + y) * stride + rect.x as usize * 4; + let (src, _) = raw[start..start + rect.width as usize * 4].as_chunks::<4>(); + for (px, s) in row.zip(src) { + px.0 = [s[2], s[1], s[0], 255]; + } + } + out + } } /// Shm formats valw can read, in order of preference. @@ -1268,22 +1359,18 @@ pub const SUPPORTED: [Format; 4] = [ Format::Abgr8888, ]; -/// Converts a screencopy buffer to opaque RGBA. -/// -/// Alpha is always set to 255: a screenshot of what's on screen is opaque, -/// even where the compositor's buffer has alpha 0. -pub fn to_rgba( +/// Copies a screencopy buffer into B, G, R, X order, dropping stride padding. +pub fn to_bgrx( format: Format, width: u32, height: u32, stride: u32, data: &[u8], y_invert: bool, -) -> Result { - // Little-endian byte order of each 32-bit pixel. - let (r, g, b) = match format { - Format::Xrgb8888 | Format::Argb8888 => (2, 1, 0), - Format::Xbgr8888 | Format::Abgr8888 => (0, 1, 2), +) -> Result { + let swap = match format { + Format::Xrgb8888 | Format::Argb8888 => false, + Format::Xbgr8888 | Format::Abgr8888 => true, other => bail!("unsupported buffer format {other:?}"), }; ensure!( @@ -1297,12 +1384,16 @@ pub fn to_rgba( stride * height ); - let mut out = RgbaImage::new(width, height); - for (y, row) in out.rows_mut().enumerate() { + let row_len = width as usize * 4; + let mut out = Bgrx::new(width, height); + for (y, dst) in out.chunks_exact_mut(row_len).enumerate() { let src_y = if y_invert { height as usize - 1 - y } else { y }; - let (src, _) = data[src_y * stride as usize..][..width as usize * 4].as_chunks::<4>(); - for (px, s) in row.zip(src) { - px.0 = [s[r], s[g], s[b], 255]; + let src = &data[src_y * stride as usize..][..row_len]; + dst.copy_from_slice(src); + if swap { + for px in dst.as_chunks_mut::<4>().0 { + px.swap(0, 2); + } } } Ok(out) @@ -1310,7 +1401,7 @@ pub fn to_rgba( /// Turns a buffer in the output's native orientation upright, the same way /// grim does: rotate clockwise by the transform's angle, then mirror. -pub fn apply_transform(img: RgbaImage, transform: Transform) -> RgbaImage { +pub fn apply_transform(img: Bgrx, transform: Transform) -> Bgrx { let rotated = match transform { Transform::_90 | Transform::Flipped90 => imageops::rotate90(&img), Transform::_180 | Transform::Flipped180 => imageops::rotate180(&img), @@ -1330,7 +1421,7 @@ pub fn apply_transform(img: RgbaImage, transform: Transform) -> RgbaImage { - [ ] **Step 4: Run the tests and watch them pass** Run: `cargo nextest run frame::` -Expected: all 6 tests in `frame::tests` pass. +Expected: all 8 tests in `frame::tests` pass. - [ ] **Step 5: Lint** @@ -1340,7 +1431,7 @@ Expected: no warnings; every test passes. - [ ] **Step 6: Commit** ```sh -jj commit -m "feat: screencopy pixel conversion and output transforms" +jj commit -m "feat: screencopy frames, pixel formats and output transforms" ``` --- @@ -1354,8 +1445,8 @@ jj commit -m "feat: screencopy pixel conversion and output transforms" - Modify: `src/main.rs` (add `selection`, `render` to the `mod` list) **Interfaces:** -- Consumes: `frame::OutputGeom` (Task 6). -- Produces: `selection::{MIN_PIXELS, Point { x: f64, y: f64 }, PixelRect { x, y, width, height: u32 }, Selection { Idle, Dragging { start, end } }}` with `press`, `motion`, `release(p) -> Option<(Point, Point)>`; `selection::clip(a, b, out: &OutputGeom, frame_w: u32, frame_h: u32) -> Option`; `selection::resolve(...)` (same, but `None` under `MIN_PIXELS`); `render::{DIM, to_argb8888(img: &RgbaImage, brightness: u8) -> Vec, compose(dst: &mut [u8], dark: &[u8], bright: &[u8], width: u32, sel: Option)}`. +- Consumes: `frame::{OutputGeom, PixelRect}` (Task 6). +- Produces: `selection::{MIN_PIXELS, Point { x: f64, y: f64 }, Selection { Idle, Dragging { start, end } }}` with `press`, `motion`, `release(p) -> Option<(Point, Point)>`; `selection::clip(a, b, out: &OutputGeom, frame_w: u32, frame_h: u32) -> Option`; `selection::resolve(...)` (same, but `None` under `MIN_PIXELS`); `render::{DIM, dim(pixels: &[u8], brightness: u8) -> Vec, compose(dst: &mut [u8], dark: &[u8], bright: &[u8], width: u32, sel: Option)}` (all buffers are B, G, R, X). - [ ] **Step 1: Write the failing tests for `selection`** @@ -1513,7 +1604,7 @@ Expected: compilation fails, e.g. `` cannot find struct, variant or union type ` Put this above the test module in `src/selection.rs`: ```rust -use crate::frame::OutputGeom; +use crate::frame::{OutputGeom, PixelRect}; /// Selections smaller than this (in physical pixels) count as a click. pub const MIN_PIXELS: u32 = 2; @@ -1525,15 +1616,6 @@ pub struct Point { pub y: f64, } -/// A rectangle in a frame's physical pixels. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct PixelRect { - pub x: u32, - pub y: u32, - pub width: u32, - pub height: u32, -} - #[derive(Debug, Clone, Copy, Default, PartialEq)] pub enum Selection { #[default] @@ -1613,10 +1695,10 @@ mod tests { use super::*; #[test] - fn converts_and_dims() { - let img = RgbaImage::from_raw(1, 1, vec![200, 100, 50, 255]).unwrap(); - assert_eq!(to_argb8888(&img, 255), vec![50, 100, 200, 255]); - assert_eq!(to_argb8888(&img, DIM), vec![30, 60, 120, 255]); + fn dims_every_byte() { + assert_eq!(dim(&[200, 100, 50, 0], 255), vec![200, 100, 50, 0]); + assert_eq!(dim(&[200, 100, 50, 255], DIM), vec![120, 60, 30, 153]); + assert_eq!(dim(&[255, 0, 1, 2], 0), vec![0, 0, 0, 0]); } fn px(buf: &[u8], width: u32, x: u32, y: u32) -> [u8; 4] { @@ -1676,26 +1758,28 @@ Add `mod render;` to the `mod` list at the top of `src/main.rs` (keep it sorted) - [ ] **Step 6: Run the tests and watch them fail** Run: `cargo nextest run render::` -Expected: compilation fails, e.g. `` cannot find function `to_argb8888` in this scope ``. +Expected: compilation fails, e.g. `` cannot find function `dim` in this scope ``. - [ ] **Step 7: Implement `render`** Put this above the test module in `src/render.rs`: ```rust -use image::RgbaImage; - -use crate::selection::PixelRect; +use crate::frame::PixelRect; /// Overlay brightness outside the selection (60%, i.e. darkened by 40%). pub const DIM: u8 = 153; -/// `img` as wl_shm ARGB8888 bytes (B, G, R, A in memory), with RGB scaled -/// by `brightness / 255`. -pub fn to_argb8888(img: &RgbaImage, brightness: u8) -> Vec { - let scale = |c: u8| (c as u16 * brightness as u16 / 255) as u8; - img.pixels() - .flat_map(|p| [scale(p.0[2]), scale(p.0[1]), scale(p.0[0]), 255]) +/// B, G, R, X pixels with every byte scaled by about `brightness / 255`. +/// +/// This runs on every output before the overlay can appear, so it is one +/// branch-free pass the compiler can vectorise. Scaling X too is harmless: +/// the overlay buffer is XRGB8888, which ignores it. +pub fn dim(pixels: &[u8], brightness: u8) -> Vec { + let factor = brightness as u16 + 1; + pixels + .iter() + .map(|&b| ((b as u16 * factor) >> 8) as u8) .collect() } @@ -2272,9 +2356,12 @@ impl Wayland { } fn log_environment(&self) { - for (interface, version) in self.global_list() { - tracing::debug!("global {interface} v{version}"); - } + let globals: Vec = self + .global_list() + .into_iter() + .map(|(interface, version)| format!("{interface} v{version}")) + .collect(); + tracing::info!("globals: {}", globals.join(", ")); for o in self.outputs() { let g = &o.geom; tracing::info!( @@ -2961,7 +3048,7 @@ jj commit -m "feat: wayland connection, niri IPC and valw doctor" - Replace: `src/main.rs` **Interfaces:** -- Consumes: `wayland::{Wayland, State, Output}` and `Wayland::dispatch_until` (Task 9); `frame::{Frame, SUPPORTED, to_rgba, apply_transform}` (Task 6); `output::*` (Task 8); `lock::Lock` (Task 5); `niri::{focused_output, version}` (Task 9); `config` (Task 3). +- Consumes: `wayland::{Wayland, State, Output}` and `Wayland::dispatch_until` (Task 9); `frame::{Frame, SUPPORTED, to_bgrx, apply_transform, Frame::to_rgba, Frame::full}` (Task 6); `output::*` (Task 8); `lock::Lock` (Task 5); `niri::{focused_output, version}` (Task 9); `config` (Task 3). - Produces: `pub trait Capturer { fn capture(&mut self, outputs: &[Output], cursor: bool) -> Result>; }` implemented for `Wayland`; `pub struct Pending` (one in-flight screencopy, stored in `State::captures`); CLI `valw screen [--all] [--clipboard-only] [-o PATH|-] [--delay SECS] [--cursor]`. Two facts the code depends on, both verified on niri 26.04: @@ -2973,6 +3060,7 @@ Two facts the code depends on, both verified on niri 26.04: Add `mod capture;` to the `mod` list in `src/main.rs`. The pixel logic here was unit-tested in Task 6; this file is protocol plumbing. ```rust +use std::sync::Arc; use std::time::{Duration, Instant}; use anyhow::{Context, Result, anyhow}; @@ -3050,11 +3138,12 @@ impl Capturer for Wayland { self.dispatch_until(started + TIMEOUT, |s| { s.captures.iter().all(|c| c.result.is_some()) }) - .context("screen capture timed out")?; + .context("screen capture failed")?; tracing::info!( - "captured {} output(s) in {:?}", + "captured {} output(s) in {:?} ({:?} after start)", outputs.len(), - started.elapsed() + started.elapsed(), + crate::log::since_start() ); std::mem::take(&mut self.state.captures) @@ -3183,9 +3272,9 @@ fn finish(c: &mut Pending) -> Result { let l = c.layout.context("ready without a buffer")?; let pool = c.pool.as_mut().context("ready without a buffer")?; let data = &pool.mmap()[..]; - let image = frame::to_rgba(l.format, l.width, l.height, l.stride, data, c.y_invert)?; + let pixels = frame::to_bgrx(l.format, l.width, l.height, l.stride, data, c.y_invert)?; Ok(Frame { - image: frame::apply_transform(image, c.output.transform), + pixels: Arc::new(frame::apply_transform(pixels, c.output.transform)), output: c.output.geom.clone(), }) } @@ -3232,7 +3321,7 @@ Apply this change to `src/wayland.rs`: }; // Two round trips: one for wl_output, one for the xdg-output details. queue -@@ -422,3 +426,4 @@ +@@ -425,3 +429,4 @@ wayland_client::delegate_noop!(State: ZwlrScreencopyManagerV1); wayland_client::delegate_noop!(State: WpViewporter); wayland_client::delegate_noop!(State: WpViewport); @@ -3377,12 +3466,12 @@ fn capture(mode: Mode, common: Common) -> Result<()> { let frames = wl.capture(&outputs, cursor)?; let shots = frames .into_iter() - .map(|f| (f.image, Some(f.output.name))) + .map(|f| (f.to_rgba(f.full()), Some(f.output.name))) .collect(); (shots, focused) } else { let frame = wl.capture(&outputs[focused..=focused], cursor)?.remove(0); - (vec![(frame.image, None)], 0) + (vec![(frame.to_rgba(frame.full()), None)], 0) } } }; @@ -3549,16 +3638,18 @@ jj commit -m "feat: wlr-screencopy capture and valw screen" - Modify: `src/wayland.rs` (route overlay events), `src/main.rs` (add `region`, drop `#![allow(dead_code)]`) **Interfaces:** -- Consumes: `selection::{Selection, Point, PixelRect, clip, resolve}` (Task 7); `render::{to_argb8888, compose, DIM}` (Task 7); `frame::{Frame, OutputGeom}` (Task 6); `wayland::{Wayland, State, Output}`, `Wayland::dispatch_blocking` (Task 9); `log::since_start` (Task 4); `error::{Cancelled, HintExt}` (Task 2). +- Consumes: `selection::{Selection, Point, clip, resolve}` (Task 7); `render::{dim, compose, DIM}` (Task 7); `frame::{Bgrx, Frame, OutputGeom, PixelRect}` (Task 6); `wayland::{Wayland, State, Output}`, `Wayland::dispatch_blocking` (Task 9); `log::since_start` (Task 4); `error::{Cancelled, HintExt}` (Task 2). - Produces: `pub const NAMESPACE: &str = "valw-overlay"`; `pub type Picked = (usize, PixelRect)`; `pub fn select(wl: &mut Wayland, outputs: &[Output], frames: &[Frame]) -> Result` (returns `Cancelled` on Esc or right click); `pub struct Overlay` with `cancel`, `configure`, `frame_done`, `pointer`, called from `State`'s handlers. CLI: `valw region [COMMON]`. -How it works: one fullscreen layer surface per output shows that output's frozen frame, darkened. The buffer has the frame's physical size, and `wp_viewporter` maps it onto the surface's logical size, so it is shown 1:1 at any scale. Pointer positions are converted to global logical coordinates. The drag is drawn (and later cropped) only on the output where it started. Redraws are paced by frame callbacks. +How it works: one fullscreen layer surface per output shows that output's frozen frame, darkened. The buffer is XRGB8888 (the frame's own byte order, so no conversion) at the frame's physical size, and `wp_viewporter` maps it onto the surface's logical size, so it is shown 1:1 at any scale. The undimmed selection is pasted straight from the shared frame (`Arc`, no copy). Pointer positions are converted to global logical coordinates. The drag is drawn (and later cropped) only on the output where it started. Redraws are paced by frame callbacks. - [ ] **Step 1: Write `src/region.rs`** Add `mod region;` to the `mod` list in `src/main.rs`. ```rust +use std::sync::Arc; + use anyhow::{Context, Result}; use smithay_client_toolkit::{ compositor::FrameCallbackData, @@ -3582,9 +3673,9 @@ use wayland_protocols::wp::cursor_shape::v1::client::wp_cursor_shape_device_v1:: use wayland_protocols::wp::viewporter::client::wp_viewport::WpViewport; use crate::error::{Cancelled, HintExt}; -use crate::frame::{Frame, OutputGeom}; +use crate::frame::{Bgrx, Frame, OutputGeom, PixelRect}; use crate::render; -use crate::selection::{self, PixelRect, Point, Selection}; +use crate::selection::{self, Point, Selection}; use crate::wayland::{Output, State, Wayland}; pub const NAMESPACE: &str = "valw-overlay"; @@ -3608,7 +3699,7 @@ struct Surface { width: u32, height: u32, dark: Vec, - bright: Vec, + bright: Arc, pool: SlotPool, buffer: Option, configured: bool, @@ -3650,6 +3741,10 @@ pub fn select(wl: &mut Wayland, outputs: &[Output], frames: &[Frame]) -> Result< surfaces.push(Surface::new(layer, viewport, frame, &s.shm)?); } + tracing::debug!( + "overlay buffers ready {:?} after start", + crate::log::since_start() + ); wl.state.overlay = Some(Overlay { surfaces, selection: Selection::default(), @@ -3666,7 +3761,7 @@ pub fn select(wl: &mut Wayland, outputs: &[Output], frames: &[Frame]) -> Result< impl Surface { fn new(layer: LayerSurface, viewport: WpViewport, frame: &Frame, shm: &Shm) -> Result { - let (width, height) = frame.image.dimensions(); + let (width, height) = frame.pixels.dimensions(); let len = (width * height * 4) as usize; Ok(Surface { layer, @@ -3674,8 +3769,8 @@ impl Surface { geom: frame.output.clone(), width, height, - dark: render::to_argb8888(&frame.image, render::DIM), - bright: render::to_argb8888(&frame.image, 255), + dark: render::dim(frame.pixels.as_raw(), render::DIM), + bright: Arc::clone(&frame.pixels), // Room for two buffers so one can be on screen while we draw the next. pool: SlotPool::new(len * 2, shm).context("could not create shm pool")?, buffer: None, @@ -3691,12 +3786,12 @@ impl Surface { self.width as i32, self.height as i32, stride, - wl_shm::Format::Argb8888, + wl_shm::Format::Xrgb8888, ) else { tracing::warn!("could not allocate an overlay buffer"); return; }; - render::compose(canvas, &self.dark, &self.bright, self.width, sel); + render::compose(canvas, &self.dark, self.bright.as_raw(), self.width, sel); let surface = self.layer.wl_surface(); surface.damage_buffer(0, 0, self.width as i32, self.height as i32); @@ -3866,23 +3961,7 @@ Apply this change to `src/wayland.rs`: }; // Two round trips: one for wl_output, one for the xdg-output details. queue -@@ -148,9 +152,12 @@ - } - - fn log_environment(&self) { -- for (interface, version) in self.global_list() { -- tracing::debug!("global {interface} v{version}"); -- } -+ let globals: Vec = self -+ .global_list() -+ .into_iter() -+ .map(|(interface, version)| format!("{interface} v{version}")) -+ .collect(); -+ tracing::info!("globals: {}", globals.join(", ")); - for o in self.outputs() { - let g = &o.geom; - tracing::info!( -@@ -230,7 +237,17 @@ +@@ -233,7 +237,17 @@ ) { } @@ -3901,7 +3980,7 @@ Apply this change to `src/wayland.rs`: fn surface_enter( &mut self, -@@ -264,16 +281,23 @@ +@@ -267,16 +281,23 @@ } impl LayerShellHandler for State { @@ -3931,7 +4010,7 @@ Apply this change to `src/wayland.rs`: } } -@@ -360,8 +384,13 @@ +@@ -363,8 +384,13 @@ _: &QueueHandle, _: &wl_keyboard::WlKeyboard, _: u32, @@ -3947,7 +4026,7 @@ Apply this change to `src/wayland.rs`: } fn repeat_key( -@@ -401,10 +430,13 @@ +@@ -404,10 +430,13 @@ fn pointer_frame( &mut self, _: &Connection, @@ -3988,15 +4067,6 @@ Apply this change to `src/main.rs`: mod render; mod selection; mod wayland; -@@ -22,7 +20,7 @@ - use anyhow::{Context, Result}; - use chrono::Local; - use clap::{Args, Parser, Subcommand}; --use image::RgbaImage; -+use image::{RgbaImage, imageops}; - - use crate::capture::Capturer; - use crate::config::Config; @@ -45,6 +43,11 @@ /// Capture every output, one file each. #[arg(long, conflicts_with_all = ["output", "clipboard_only"])] @@ -4025,21 +4095,19 @@ Apply this change to `src/main.rs`: } } -@@ -139,6 +144,13 @@ - (vec![(frame.image, None)], 0) +@@ -139,6 +144,11 @@ + (vec![(frame.to_rgba(frame.full()), None)], 0) } } + Mode::Region => { + let frames = wl.capture(&outputs, cursor)?; + let (i, r) = region::select(&mut wl, &outputs, &frames)?; -+ let image = -+ imageops::crop_imm(&frames[i].image, r.x, r.y, r.width, r.height).to_image(); -+ (vec![(image, None)], 0) ++ (vec![(frames[i].to_rgba(r), None)], 0) + } }; drop(wl); -@@ -224,10 +236,10 @@ +@@ -224,10 +234,10 @@ fn flag_conflicts() { assert!(parses(&["screen", "--all"])); assert!(parses(&["screen", "-o", "-"])); @@ -4240,7 +4308,7 @@ Expected: `0` every time. At 1.25 the overlay must also look sharp, with text in - [ ] **Step 3: Latency** Run `$V region -o /dev/null` and press Esc; repeat three times. -Expected: every `overlay on … committed …ms after start` log line is under 150 ms. +Expected: every `overlay on … committed …ms after start` log line is under 150 ms. (Prototype measurements: 38–42 ms warm and 90 ms cold on a headless 1080p output; 270 ms before frames were kept in B, G, R, X order.) If it's over, the `captured … (… after start)` and `overlay buffers ready …` lines (`VALW_LOG=debug`) show which phase is slow. - [ ] **Step 4: Rotated output (Review Focus 1)** diff --git a/docs/superpowers/specs/2026-09-29-valw-phase-0-1-design.md b/docs/superpowers/specs/2026-09-29-valw-phase-0-1-design.md index 5f3263a..40233d5 100644 --- a/docs/superpowers/specs/2026-09-29-valw-phase-0-1-design.md +++ b/docs/superpowers/specs/2026-09-29-valw-phase-0-1-design.md @@ -218,13 +218,20 @@ Every capture command runs these steps in order. names the protocol. 5. **Capture.** `Capturer::capture(&[Output], cursor: bool) -> Vec` issues one screencopy request per output in parallel and waits up to 5 s - for every `ready`. A `Frame` holds upright, opaque RGBA8 pixels at physical - resolution plus its output name and logical geometry; the scale is - `frame width / logical width`. `XRGB8888`, `ARGB8888`, `XBGR8888` and - `ABGR8888` are converted; `y_invert` is honoured; any other format is an - error that names the format. Alpha is always 255, even where the - compositor's buffer says 0. `cursor` comes from `--cursor` or + for every `ready`. A `Frame` holds upright pixels at physical resolution + in **B, G, R, X byte order** (XRGB8888 in memory, which is what niri + delivers), plus its output name and logical geometry; the scale is + `frame width / logical width`. `XRGB8888` and `ARGB8888` are copied + as-is, `XBGR8888` and `ABGR8888` get R and B swapped, `y_invert` is + honoured, and any other format is an error that names the format. + Conversion to RGBA happens only for what is saved (`Frame::to_rgba`, + after the overlay has closed), and alpha is then always 255, even where + the compositor's buffer says 0. `cursor` comes from `--cursor` or `capture.show_cursor`. + - **Why BGRX:** on the user's machine a full 1080p pass over the pixels + costs 5–10 ms. Converting to RGBA up front and back to ARGB for the + overlay put the first overlay frame at 270 ms. Keeping the native + order brought it to about 40 ms (headless 1080p, warm). - **niri quirk (verified in its source):** niri rejects a shm buffer with `invalid buffer` unless the whole `wl_shm_pool` is exactly the buffer's size. Every capture therefore gets its own pool. @@ -251,9 +258,11 @@ output: - namespace `valw-overlay`, layer `Overlay`, anchored to all four edges, `exclusive_zone = -1`, `keyboard_interactivity = Exclusive` -- background: that output's frozen frame, darkened by 40% -- the selection area is copied from the undarkened frame and outlined with a - 1 physical pixel white border +- buffer format XRGB8888, the frame's own byte order, so nothing is + converted and alpha is ignored +- background: that output's frozen frame, darkened by 40% (computed once) +- the selection area is copied from the undarkened frame (shared, not + copied) and outlined with a 1 physical pixel white border - cursor shape: crosshair (`wp_cursor_shape`) Surfaces are owned by the process, so the compositor removes them if valw