From 1a0dd421b04a114f44a62e6d2c1bc403dc143f1b Mon Sep 17 00:00:00 2001 From: Andreas Backx Date: Thu, 9 Nov 2023 22:00:22 +0000 Subject: [PATCH] Make region, sizing, and positioning data structs reusable We use different structs/fields to store the same three types of data everywhere: - Position (x,y) - Size (width,height) - Region(position, size) This makes it so they all reuse the same information. --- libwayshot/src/dispatch.rs | 22 +++--- libwayshot/src/lib.rs | 92 +++++++++++++---------- libwayshot/src/output.rs | 28 +++---- libwayshot/src/region.rs | 140 ++++++++++++++++++----------------- libwayshot/src/screencopy.rs | 28 +++++-- wayshot/src/utils.rs | 23 +++--- 6 files changed, 183 insertions(+), 150 deletions(-) diff --git a/libwayshot/src/dispatch.rs b/libwayshot/src/dispatch.rs index 5352deb4..574fc813 100644 --- a/libwayshot/src/dispatch.rs +++ b/libwayshot/src/dispatch.rs @@ -26,7 +26,8 @@ use wayland_protocols_wlr::screencopy::v1::client::{ }; use crate::{ - output::{OutputInfo, OutputPositioning, WlOutputMode}, + output::OutputInfo, + region::{Position, Region, Size}, screencopy::FrameFormat, }; @@ -64,8 +65,7 @@ impl Dispatch for OutputCaptureState { description: String::new(), transform: wl_output::Transform::Normal, scale: 1, - dimensions: OutputPositioning::default(), - mode: WlOutputMode::default(), + region: Region::default(), }); } else { tracing::error!("Ignoring a wl_output with version < 4."); @@ -100,9 +100,7 @@ impl Dispatch for OutputCaptureState { wl_output::Event::Description { description } => { output.description = description; } - wl_output::Event::Mode { width, height, .. } => { - output.mode = WlOutputMode { width, height }; - } + wl_output::Event::Mode { .. } => {} wl_output::Event::Geometry { transform: WEnum::Value(transform), .. @@ -141,12 +139,13 @@ impl Dispatch for OutputCaptureState { match event { zxdg_output_v1::Event::LogicalPosition { x, y } => { - output_info.dimensions.x = x; - output_info.dimensions.y = y; + output_info.region.position = Position { x, y }; } zxdg_output_v1::Event::LogicalSize { width, height } => { - output_info.dimensions.width = width; - output_info.dimensions.height = height; + output_info.region.size = Size { + width: width as u32, + height: height as u32, + }; } zxdg_output_v1::Event::Done => {} zxdg_output_v1::Event::Name { .. } => {} @@ -191,8 +190,7 @@ impl Dispatch for CaptureFrameState { if let Value(f) = format { frame.formats.push(FrameFormat { format: f, - width, - height, + size: Size { width, height }, stride, }) } else { diff --git a/libwayshot/src/lib.rs b/libwayshot/src/lib.rs index c14c061c..a4dff0d4 100644 --- a/libwayshot/src/lib.rs +++ b/libwayshot/src/lib.rs @@ -29,7 +29,7 @@ use wayland_client::{ globals::{registry_queue_init, GlobalList}, protocol::{ wl_compositor::WlCompositor, - wl_output::WlOutput, + wl_output::{Transform, WlOutput}, wl_shm::{self, WlShm}, }, Connection, EventQueue, @@ -52,7 +52,7 @@ use crate::{ convert::create_converter, dispatch::{CaptureFrameState, FrameState, OutputCaptureState, WayshotState}, output::OutputInfo, - region::LogicalRegion, + region::{LogicalRegion, Region, Size}, screencopy::{create_shm_fd, FrameCopy, FrameFormat}, }; @@ -214,10 +214,10 @@ impl WayshotConnection { screencopy_manager.capture_output_region( cursor_overlay, output, - embedded_region.inner.x, - embedded_region.inner.y, - embedded_region.inner.width, - embedded_region.inner.height, + embedded_region.inner.position.x, + embedded_region.inner.position.y, + embedded_region.inner.size.width as i32, + embedded_region.inner.size.height as i32, &qh, (), ) @@ -274,16 +274,21 @@ impl WayshotConnection { // Connecting to wayland environment. let qh = event_queue.handle(); - // Bytes of data in the frame = stride * height. - let frame_bytes = frame_format.stride * frame_format.height; - // Instantiate shm global. let shm = self.globals.bind::(&qh, 1..=1, ())?; - let shm_pool = shm.create_pool(fd.as_fd(), frame_bytes as i32, &qh, ()); + let shm_pool = shm.create_pool( + fd.as_fd(), + frame_format + .byte_size() + .try_into() + .map_err(|_| Error::BufferTooSmall)?, + &qh, + (), + ); let buffer = shm_pool.create_buffer( 0, - frame_format.width as i32, - frame_format.height as i32, + frame_format.size.width as i32, + frame_format.size.height as i32, frame_format.stride as i32, frame_format.format, &qh, @@ -321,9 +326,7 @@ impl WayshotConnection { let (state, event_queue, frame, frame_format) = self.capture_output_frame_get_state(cursor_overlay as i32, output, capture_region)?; - // Bytes of data in the frame = stride * height. - let frame_bytes = frame_format.stride * frame_format.height; - file.set_len(frame_bytes as u64)?; + file.set_len(frame_format.byte_size())?; let frame_guard = self.capture_output_frame_inner(state, event_queue, frame, frame_format, file)?; @@ -332,7 +335,7 @@ impl WayshotConnection { } /// Get a FrameCopy instance with screenshot pixel data for any wl_output object. - #[tracing::instrument(skip_all, fields(output = output_info.name, region = capture_region.map(|r| format!("{:}", r))))] + #[tracing::instrument(skip_all, fields(output = format!("{output_info}"), region = capture_region.map(|r| format!("{:}", r))))] fn capture_frame_copy( &self, cursor_overlay: bool, @@ -360,22 +363,30 @@ impl WayshotConnection { tracing::error!("You can send a feature request for the above format to the mailing list for wayshot over at https://sr.ht/~shinyzenith/wayshot."); return Err(Error::NoSupportedBufferFormat); }; + let rotated_size = match output_info.transform { + Transform::_90 | Transform::_270 | Transform::Flipped90 | Transform::Flipped270 => { + Size { + width: frame_format.size.height, + height: frame_format.size.width, + } + } + _ => frame_format.size, + }; let frame_copy = FrameCopy { frame_format, frame_color_type, frame_mmap, transform: output_info.transform, - position: capture_region - .map(|capture_region| { - let logical_region = capture_region.logical(); - (logical_region.inner.x as i64, logical_region.inner.y as i64) - }) - .unwrap_or_else(|| { - ( - output_info.dimensions.x as i64, - output_info.dimensions.y as i64, - ) - }), + region: LogicalRegion { + inner: Region { + position: capture_region + .map(|capture_region: EmbeddedRegion| { + capture_region.logical().inner.position + }) + .unwrap_or_else(|| output_info.region.position), + size: rotated_size, + }, + }, }; tracing::debug!("Created frame copy: {:#?}", frame_copy); Ok((frame_copy, frame_guard)) @@ -450,7 +461,7 @@ impl WayshotConnection { tracing::span!( tracing::Level::DEBUG, "overlay_frames::surface", - output = output_info.name.as_str() + output = format!("{output_info}") ) .in_scope(|| -> Result<()> { let surface = compositor.create_surface(&qh, ()); @@ -467,8 +478,8 @@ impl WayshotConnection { layer_surface.set_exclusive_zone(-1); layer_surface.set_anchor(Anchor::Top | Anchor::Left); layer_surface.set_size( - frame_copy.frame_format.width, - frame_copy.frame_format.height, + frame_copy.frame_format.size.width, + frame_copy.frame_format.size.height, ); debug!("Committing surface creation changes."); @@ -514,8 +525,8 @@ impl WayshotConnection { tracing::Level::DEBUG, "filter_map", output = format!( - "{name} at {region}", - name = output_info.name, + "{output_info} at {region}", + output_info = format!("{output_info}"), region = LogicalRegion::from(output_info), ), capture_region = format!("{}", capture_region), @@ -560,8 +571,8 @@ impl WayshotConnection { image_util::rotate_image_buffer( image, frame_copy.transform, - frame_copy.frame_format.width, - frame_copy.frame_format.height, + frame_copy.frame_format.size.width, + frame_copy.frame_format.size.height, ), frame_copy, )) @@ -579,8 +590,8 @@ impl WayshotConnection { // Default to a transparent image. let composite_image = composite_image.unwrap_or_else(|| { Ok(RgbaImage::from_pixel( - capture_region.inner.width as u32, - capture_region.inner.height as u32, + capture_region.inner.size.width, + capture_region.inner.size.height, Rgba([0 as u8, 0 as u8, 0 as u8, 255 as u8]), )) }); @@ -588,15 +599,16 @@ impl WayshotConnection { Some(|| -> Result<_> { let mut composite_image = composite_image?; let (image, frame_copy) = image?; - let frame_copy_region = LogicalRegion::from(&frame_copy); let (x, y) = ( - frame_copy_region.inner.x as i64 - capture_region.inner.x as i64, - frame_copy_region.inner.y as i64 - capture_region.inner.y as i64, + frame_copy.region.inner.position.x as i64 + - capture_region.inner.position.x as i64, + frame_copy.region.inner.position.y as i64 + - capture_region.inner.position.y as i64, ); tracing::span!( tracing::Level::DEBUG, "replace", - frame_copy_region = format!("{}", frame_copy_region), + frame_copy_region = format!("{}", frame_copy.region), capture_region = format!("{}", capture_region), x = x, y = y, diff --git a/libwayshot/src/output.rs b/libwayshot/src/output.rs index 986f74df..5449f4f7 100644 --- a/libwayshot/src/output.rs +++ b/libwayshot/src/output.rs @@ -1,5 +1,9 @@ +use std::fmt::Display; + use wayland_client::protocol::{wl_output, wl_output::WlOutput}; +use crate::region::Region; + /// Represents an accessible wayland output. /// /// Do not instantiate, instead use [`crate::WayshotConnection::get_all_outputs`]. @@ -10,20 +14,16 @@ pub struct OutputInfo { pub description: String, pub transform: wl_output::Transform, pub scale: i32, - pub dimensions: OutputPositioning, - pub mode: WlOutputMode, -} - -#[derive(Default, Debug, Clone, PartialEq, Eq, Hash)] -pub struct WlOutputMode { - pub width: i32, - pub height: i32, + pub region: Region, } -#[derive(Default, Debug, Clone, PartialEq, Eq, Hash)] -pub struct OutputPositioning { - pub x: i32, - pub y: i32, - pub width: i32, - pub height: i32, +impl Display for OutputInfo { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "{name} ({description})", + name = self.name, + description = self.description + ) + } } diff --git a/libwayshot/src/region.rs b/libwayshot/src/region.rs index 27a35693..bbd81d39 100644 --- a/libwayshot/src/region.rs +++ b/libwayshot/src/region.rs @@ -1,10 +1,7 @@ use std::cmp; -use wayland_client::protocol::wl_output::Transform; - use crate::error::{Error, Result}; use crate::output::OutputInfo; -use crate::screencopy::FrameCopy; /// Ways to say how a region for a screenshot should be captured. pub enum RegionCapturer { @@ -58,16 +55,28 @@ pub struct EmbeddedRegion { /// /// Use `LogicalRegion` or `EmbeddedRegion` instead as they convey the /// coordinate system used. -#[derive(Debug, Copy, Clone)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Default)] pub struct Region { - /// X coordinate of the area to capture. + /// Position of the region. + pub position: Position, + /// Size of the region. + pub size: Size, +} + +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Default)] +pub struct Position { + /// X coordinate. pub x: i32, - /// y coordinate of the area to capture. + /// Y coordinate. pub y: i32, - /// Width of the capture area. - pub width: i32, - /// Height of the capture area. - pub height: i32, +} + +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Default)] +pub struct Size { + /// Width. + pub width: u32, + /// Height. + pub height: u32, } impl EmbeddedRegion { @@ -78,30 +87,36 @@ impl EmbeddedRegion { /// See `EmbeddedRegion` for an example ASCII visualisation. #[tracing::instrument(ret, level = "debug")] pub fn new(viewport: LogicalRegion, relative_to: LogicalRegion) -> Option { - let x_relative: i32 = viewport.inner.x - relative_to.inner.x; - let y_relative = viewport.inner.y - relative_to.inner.y; + let x_relative: i32 = viewport.inner.position.x - relative_to.inner.position.x; + let y_relative = viewport.inner.position.y - relative_to.inner.position.y; let x1 = cmp::max(x_relative, 0); - let x2 = cmp::min(x_relative + viewport.inner.width, relative_to.inner.width); - let width = x2 - x1; - if width <= 0 { + let x2 = cmp::min( + x_relative + viewport.inner.size.width as i32, + relative_to.inner.size.width as i32, + ); + let width = if let Ok(width) = (x2 - x1).try_into() { + width + } else { return None; - } + }; let y1 = cmp::max(y_relative, 0); - let y2 = cmp::min(y_relative + viewport.inner.height, relative_to.inner.height); - let height = y2 - y1; - if height <= 0 { + let y2 = cmp::min( + y_relative + viewport.inner.size.height as i32, + relative_to.inner.size.height as i32, + ); + let height = if let Ok(height) = (y2 - y1).try_into() { + height + } else { return None; - } + }; Some(Self { relative_to: relative_to, inner: Region { - x: x1, - y: y1, - width, - height, + position: Position { x: x1, y: y1 }, + size: Size { width, height }, }, }) } @@ -114,10 +129,11 @@ impl EmbeddedRegion { pub fn logical(&self) -> LogicalRegion { LogicalRegion { inner: Region { - x: self.relative_to.inner.x as i32 + self.inner.x, - y: self.relative_to.inner.y as i32 + self.inner.y, - width: self.inner.width, - height: self.inner.height, + position: Position { + x: self.relative_to.inner.position.x + self.inner.position.x, + y: self.relative_to.inner.position.y + self.inner.position.y, + }, + size: self.inner.size, }, } } @@ -134,19 +150,34 @@ impl std::fmt::Display for EmbeddedRegion { } } -impl std::fmt::Display for Region { +impl std::fmt::Display for Position { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "({x}, {y})", x = self.x, y = self.y,) + } +} + +impl std::fmt::Display for Size { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( f, - "({x}, {y}) ({width}x{height})", - x = self.x, - y = self.y, + "({width}x{height})", width = self.width, height = self.height, ) } } +impl std::fmt::Display for Region { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "({position}) ({size})", + position = self.position, + size = self.size, + ) + } +} + impl std::fmt::Display for LogicalRegion { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{inner}", inner = self.inner) @@ -156,12 +187,7 @@ impl std::fmt::Display for LogicalRegion { impl From<&OutputInfo> for LogicalRegion { fn from(output_info: &OutputInfo) -> Self { LogicalRegion { - inner: Region { - x: output_info.dimensions.x, - y: output_info.dimensions.y, - width: output_info.dimensions.width, - height: output_info.dimensions.height, - }, + inner: output_info.region, } } } @@ -172,52 +198,32 @@ impl TryFrom<&Vec> for LogicalRegion { fn try_from(output_info: &Vec) -> std::result::Result { let x1 = output_info .iter() - .map(|output| output.dimensions.x) + .map(|output| output.region.position.x) .min() .ok_or(Error::NoOutputs)?; let y1 = output_info .iter() - .map(|output| output.dimensions.y) + .map(|output| output.region.position.y) .min() .ok_or(Error::NoOutputs)?; let x2 = output_info .iter() - .map(|output| output.dimensions.x + output.dimensions.width) + .map(|output| output.region.position.x + output.region.size.width as i32) .max() .ok_or(Error::NoOutputs)?; let y2 = output_info .iter() - .map(|output| output.dimensions.y + output.dimensions.height) + .map(|output| output.region.position.y + output.region.size.height as i32) .max() .ok_or(Error::NoOutputs)?; Ok(LogicalRegion { inner: Region { - x: x1, - y: y1, - width: x2 - x1, - height: y2 - y1, + position: Position { x: x1, y: y1 }, + size: Size { + width: (x2 - x1) as u32, + height: (y2 - y1) as u32, + }, }, }) } } - -impl From<&FrameCopy> for LogicalRegion { - fn from(frame_copy: &FrameCopy) -> Self { - let (width, height) = ( - frame_copy.frame_format.width as i32, - frame_copy.frame_format.height as i32, - ); - let is_portait = match frame_copy.transform { - Transform::_90 | Transform::_270 | Transform::Flipped90 | Transform::Flipped270 => true, - _ => false, - }; - LogicalRegion { - inner: Region { - x: frame_copy.position.0 as i32, - y: frame_copy.position.1 as i32, - width: if is_portait { height } else { width }, - height: if is_portait { width } else { height }, - }, - } - } -} diff --git a/libwayshot/src/screencopy.rs b/libwayshot/src/screencopy.rs index 4fcafb5c..0378686b 100644 --- a/libwayshot/src/screencopy.rs +++ b/libwayshot/src/screencopy.rs @@ -15,7 +15,10 @@ use wayland_client::protocol::{ wl_buffer::WlBuffer, wl_output, wl_shm::Format, wl_shm_pool::WlShmPool, }; -use crate::{Error, Result}; +use crate::{ + region::{LogicalRegion, Size}, + Error, Result, +}; pub struct FrameGuard { pub buffer: WlBuffer, @@ -36,12 +39,20 @@ impl Drop for FrameGuard { #[derive(Debug, Copy, Clone, PartialEq)] pub struct FrameFormat { pub format: Format, - pub width: u32, - pub height: u32, + /// Size of the frame in pixels. This will always be in "landscape" so a + /// portrait 1080x1920 frame will be 1920x1080 and will need to be rotated! + pub size: Size, /// Stride is the number of bytes between the start of a row and the start of the next row. pub stride: u32, } +impl FrameFormat { + /// Returns the size of the frame in bytes, which is the stride * height. + pub fn byte_size(&self) -> u64 { + self.stride as u64 * self.size.height as u64 + } +} + #[tracing::instrument(skip(frame_mmap))] fn create_image_buffer

( frame_format: &FrameFormat, @@ -51,8 +62,12 @@ where P: Pixel, { tracing::debug!("Creating image buffer"); - ImageBuffer::from_vec(frame_format.width, frame_format.height, frame_mmap.to_vec()) - .ok_or(Error::BufferTooSmall) + ImageBuffer::from_vec( + frame_format.size.width, + frame_format.size.height, + frame_mmap.to_vec(), + ) + .ok_or(Error::BufferTooSmall) } /// The copied frame comprising of the FrameFormat, ColorType (Rgba8), and a memory backed shm @@ -63,7 +78,8 @@ pub struct FrameCopy { pub frame_color_type: ColorType, pub frame_mmap: MmapMut, pub transform: wl_output::Transform, - pub position: (i64, i64), + /// Logical region with the transform already applied. + pub region: LogicalRegion, } impl TryFrom<&FrameCopy> for RgbaImage { diff --git a/wayshot/src/utils.rs b/wayshot/src/utils.rs index bb9a7a96..117c1f0c 100644 --- a/wayshot/src/utils.rs +++ b/wayshot/src/utils.rs @@ -8,14 +8,14 @@ use std::{ time::{SystemTime, UNIX_EPOCH}, }; -use libwayshot::region::{LogicalRegion, Region}; +use libwayshot::region::{LogicalRegion, Position, Region, Size}; pub fn parse_geometry(g: &str) -> Result { let tail = g.trim(); let x_coordinate: i32; let y_coordinate: i32; - let width: i32; - let height: i32; + let width: u32; + let height: u32; let validation_error = "Invalid geometry provided.\nValid geometries:\n1) %d,%d %dx%d\n2) %d %d %d %d"; @@ -27,8 +27,8 @@ pub fn parse_geometry(g: &str) -> Result { let (head, tail) = tail.split_once(' ').wrap_err(validation_error)?; y_coordinate = head.parse::()?; let (head, tail) = tail.split_once('x').wrap_err(validation_error)?; - width = head.parse::()?; - height = tail.parse::()?; + width = head.parse::()?; + height = tail.parse::()?; } else { // this accepts: "%d %d %d %d" let (head, tail) = tail.split_once(' ').wrap_err(validation_error)?; @@ -36,16 +36,17 @@ pub fn parse_geometry(g: &str) -> Result { let (head, tail) = tail.split_once(' ').wrap_err(validation_error)?; y_coordinate = head.parse::()?; let (head, tail) = tail.split_once(' ').wrap_err(validation_error)?; - width = head.parse::()?; - height = tail.parse::()?; + width = head.parse::()?; + height = tail.parse::()?; } Ok(LogicalRegion { inner: Region { - x: x_coordinate, - y: y_coordinate, - width, - height, + position: Position { + x: x_coordinate, + y: y_coordinate, + }, + size: Size { width, height }, }, }) }