From 8f22e6da9005f7495d38d89f2430a2649c4499ef Mon Sep 17 00:00:00 2001 From: Andreas Backx Date: Wed, 21 Feb 2024 01:46:03 +0000 Subject: [PATCH] First version of scaling fix. (#85) --- libwayshot/src/dispatch.rs | 21 ++++++++------- libwayshot/src/image_util.rs | 33 +++++++++++++++-------- libwayshot/src/lib.rs | 51 ++++++++++++++++++++---------------- libwayshot/src/output.rs | 12 ++++++--- libwayshot/src/region.rs | 27 ++++++++++++------- libwayshot/src/screencopy.rs | 3 ++- 6 files changed, 92 insertions(+), 55 deletions(-) diff --git a/libwayshot/src/dispatch.rs b/libwayshot/src/dispatch.rs index 574fc813..c8f21fcf 100644 --- a/libwayshot/src/dispatch.rs +++ b/libwayshot/src/dispatch.rs @@ -27,7 +27,7 @@ use wayland_protocols_wlr::screencopy::v1::client::{ use crate::{ output::OutputInfo, - region::{Position, Region, Size}, + region::{LogicalRegion, Position, Size}, screencopy::FrameFormat, }; @@ -64,8 +64,8 @@ impl Dispatch for OutputCaptureState { name: "".to_string(), description: String::new(), transform: wl_output::Transform::Normal, - scale: 1, - region: Region::default(), + physical_size: Size::default(), + logical_region: LogicalRegion::default(), }); } else { tracing::error!("Ignoring a wl_output with version < 4."); @@ -100,16 +100,19 @@ impl Dispatch for OutputCaptureState { wl_output::Event::Description { description } => { output.description = description; } - wl_output::Event::Mode { .. } => {} + wl_output::Event::Mode { width, height, .. } => { + output.physical_size = Size { + width: width as u32, + height: height as u32, + }; + } wl_output::Event::Geometry { transform: WEnum::Value(transform), .. } => { output.transform = transform; } - wl_output::Event::Scale { factor } => { - output.scale = factor; - } + wl_output::Event::Scale { .. } => {} wl_output::Event::Done => {} _ => {} } @@ -139,10 +142,10 @@ impl Dispatch for OutputCaptureState { match event { zxdg_output_v1::Event::LogicalPosition { x, y } => { - output_info.region.position = Position { x, y }; + output_info.logical_region.inner.position = Position { x, y }; } zxdg_output_v1::Event::LogicalSize { width, height } => { - output_info.region.size = Size { + output_info.logical_region.inner.size = Size { width: width as u32, height: height as u32, }; diff --git a/libwayshot/src/image_util.rs b/libwayshot/src/image_util.rs index 197e8f11..e06af24d 100644 --- a/libwayshot/src/image_util.rs +++ b/libwayshot/src/image_util.rs @@ -1,21 +1,24 @@ use image::{DynamicImage, GenericImageView}; use wayland_client::protocol::wl_output::Transform; +use crate::region::Size; + +#[tracing::instrument(skip(image))] pub(crate) fn rotate_image_buffer( image: DynamicImage, transform: Transform, - width: u32, - height: u32, + logical_size: Size, + max_scale: f64, ) -> DynamicImage { // TODO Better document whether width and height are before or after the transform. // Perhaps this should be part of a cleanup of the FrameCopy struct. - let (width, height) = match transform { + let (logical_width, logical_height) = match transform { Transform::_90 | Transform::_270 | Transform::Flipped90 | Transform::Flipped270 => { - (height, width) + (logical_size.height, logical_size.width) } - _ => (width, height), + _ => (logical_size.width, logical_size.height), }; - let final_image = match transform { + let rotated_image = match transform { Transform::_90 => image::imageops::rotate90(&image).into(), Transform::_180 => image::imageops::rotate180(&image).into(), Transform::_270 => image::imageops::rotate270(&image).into(), @@ -35,14 +38,22 @@ pub(crate) fn rotate_image_buffer( _ => image, }; - if final_image.dimensions() == (width, height) { - return final_image; + let scale = rotated_image.width() as f64 / logical_width as f64; + // The amount of scaling left to perform. + let scaling_left = max_scale / scale; + if scaling_left <= 1.0 { + tracing::debug!("No scaling left to do"); + return rotated_image; } + tracing::debug!("Scaling left to do: {scaling_left}"); + let new_width = (rotated_image.width() as f64 * scaling_left).round() as u32; + let new_height = (rotated_image.height() as f64 * scaling_left).round() as u32; + tracing::debug!("Resizing image to {new_width}x{new_height}"); image::imageops::resize( - &final_image, - width, - height, + &rotated_image, + new_width, + new_height, image::imageops::FilterType::Gaussian, ) .into() diff --git a/libwayshot/src/lib.rs b/libwayshot/src/lib.rs index beda48fe..3938112a 100644 --- a/libwayshot/src/lib.rs +++ b/libwayshot/src/lib.rs @@ -336,7 +336,7 @@ impl WayshotConnection { } /// Get a FrameCopy instance with screenshot pixel data for any wl_output object. - #[tracing::instrument(skip_all, fields(output = format!("{output_info}"), region = capture_region.map(|r| format!("{:}", r))))] + #[tracing::instrument(skip_all, fields(output = format!("{output_info}"), region = capture_region.map(|r| format!("{:}", r)).unwrap_or("fullscreen".to_string())))] fn capture_frame_copy( &self, cursor_overlay: bool, @@ -364,7 +364,7 @@ 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 { + let rotated_physical_size = match output_info.transform { Transform::_90 | Transform::_270 | Transform::Flipped90 | Transform::Flipped270 => { Size { width: frame_format.size.height, @@ -378,16 +378,10 @@ impl WayshotConnection { frame_color_type, frame_mmap, transform: output_info.transform, - 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, - }, - }, + logical_region: capture_region + .map(|capture_region| capture_region.logical()) + .unwrap_or(output_info.logical_region), + physical_size: rotated_physical_size, }; tracing::debug!("Created frame copy: {:#?}", frame_copy); Ok((frame_copy, frame_guard)) @@ -492,7 +486,7 @@ impl WayshotConnection { } surface.set_buffer_transform(output_info.transform); - surface.set_buffer_scale(output_info.scale); + // surface.set_buffer_scale(output_info.scale()); surface.attach(Some(&frame_guard.buffer), 0, 0); debug!("Committing surface with attached buffer."); @@ -507,6 +501,7 @@ impl WayshotConnection { } /// Take a screenshot from the specified region. + #[tracing::instrument(skip_all, fields(max_scale = tracing::field::Empty))] fn screenshot_region_capturer( &self, region_capturer: RegionCapturer, @@ -562,7 +557,17 @@ impl WayshotConnection { } }; + // TODO When freeze was used, we can still further remove the outputs + // that don't intersect with the capture region. + thread::scope(|scope| { + let max_scale = outputs_capture_regions + .iter() + .map(|(output_info, _)| output_info.scale()) + .fold(1.0, f64::max); + + tracing::Span::current().record("max_scale", &max_scale); + let rotate_join_handles = frames .into_iter() .map(|(frame_copy, _, _)| { @@ -572,8 +577,8 @@ impl WayshotConnection { image_util::rotate_image_buffer( image, frame_copy.transform, - frame_copy.frame_format.size.width, - frame_copy.frame_format.size.height, + frame_copy.logical_region.inner.size, + max_scale, ), frame_copy, )) @@ -591,8 +596,8 @@ impl WayshotConnection { // Default to a transparent image. let composite_image = composite_image.unwrap_or_else(|| { Ok(DynamicImage::new_rgba8( - capture_region.inner.size.width, - capture_region.inner.size.height, + (capture_region.inner.size.width as f64 * max_scale) as u32, + (capture_region.inner.size.height as f64 * max_scale) as u32, )) }); @@ -600,15 +605,17 @@ impl WayshotConnection { let mut composite_image = composite_image?; let (image, frame_copy) = image?; let (x, y) = ( - 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, + ((frame_copy.logical_region.inner.position.x as f64 + - capture_region.inner.position.x as f64) + * max_scale) as i64, + ((frame_copy.logical_region.inner.position.y as f64 + - capture_region.inner.position.y as f64) + * max_scale) as i64, ); tracing::span!( tracing::Level::DEBUG, "replace", - frame_copy_region = format!("{}", frame_copy.region), + frame_copy_region = format!("{}", frame_copy.logical_region), capture_region = format!("{}", capture_region), x = x, y = y, diff --git a/libwayshot/src/output.rs b/libwayshot/src/output.rs index 5449f4f7..03d0a725 100644 --- a/libwayshot/src/output.rs +++ b/libwayshot/src/output.rs @@ -2,7 +2,7 @@ use std::fmt::Display; use wayland_client::protocol::{wl_output, wl_output::WlOutput}; -use crate::region::Region; +use crate::region::{LogicalRegion, Size}; /// Represents an accessible wayland output. /// @@ -13,8 +13,8 @@ pub struct OutputInfo { pub name: String, pub description: String, pub transform: wl_output::Transform, - pub scale: i32, - pub region: Region, + pub physical_size: Size, + pub logical_region: LogicalRegion, } impl Display for OutputInfo { @@ -27,3 +27,9 @@ impl Display for OutputInfo { ) } } + +impl OutputInfo { + pub(crate) fn scale(&self) -> f64 { + self.physical_size.height as f64 / self.logical_region.inner.size.height as f64 + } +} diff --git a/libwayshot/src/region.rs b/libwayshot/src/region.rs index bbd81d39..394e1fb3 100644 --- a/libwayshot/src/region.rs +++ b/libwayshot/src/region.rs @@ -17,8 +17,9 @@ pub enum RegionCapturer { /// `Region` where the coordinate system is the logical coordinate system used /// in Wayland to position outputs. Top left is (0, 0) and any transforms and -/// scaling have been applied. -#[derive(Debug, Copy, Clone)] +/// scaling have been applied. A unit is a logical pixel, meaning that this is +/// after scaling has been applied. +#[derive(Debug, Default, Copy, Clone, PartialEq, Eq, Hash)] pub struct LogicalRegion { pub inner: Region, } @@ -27,8 +28,9 @@ pub struct LogicalRegion { /// /// It can only be contained inside of another and cannot exceed its bounds. /// -/// Example of what +/// Example: /// +/// ```` /// ┌─────────────┐ /// │ │ /// │ ┌──────────┼──────┐ @@ -44,6 +46,7 @@ pub struct LogicalRegion { /// │ │ /// │ Screen 1 │ /// └─────────────┘ +/// ```` #[derive(Debug, Copy, Clone)] pub struct EmbeddedRegion { /// The coordinate sysd @@ -171,7 +174,7 @@ impl std::fmt::Display for Region { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( f, - "({position}) ({size})", + "{position} {size}", position = self.position, size = self.size, ) @@ -187,7 +190,7 @@ impl std::fmt::Display for LogicalRegion { impl From<&OutputInfo> for LogicalRegion { fn from(output_info: &OutputInfo) -> Self { LogicalRegion { - inner: output_info.region, + inner: output_info.logical_region.inner, } } } @@ -198,22 +201,28 @@ impl TryFrom<&Vec> for LogicalRegion { fn try_from(output_info: &Vec) -> std::result::Result { let x1 = output_info .iter() - .map(|output| output.region.position.x) + .map(|output| output.logical_region.inner.position.x) .min() .ok_or(Error::NoOutputs)?; let y1 = output_info .iter() - .map(|output| output.region.position.y) + .map(|output| output.logical_region.inner.position.y) .min() .ok_or(Error::NoOutputs)?; let x2 = output_info .iter() - .map(|output| output.region.position.x + output.region.size.width as i32) + .map(|output| { + output.logical_region.inner.position.x + + output.logical_region.inner.size.width as i32 + }) .max() .ok_or(Error::NoOutputs)?; let y2 = output_info .iter() - .map(|output| output.region.position.y + output.region.size.height as i32) + .map(|output| { + output.logical_region.inner.position.y + + output.logical_region.inner.size.height as i32 + }) .max() .ok_or(Error::NoOutputs)?; Ok(LogicalRegion { diff --git a/libwayshot/src/screencopy.rs b/libwayshot/src/screencopy.rs index 734b6bfd..8b01c7c6 100644 --- a/libwayshot/src/screencopy.rs +++ b/libwayshot/src/screencopy.rs @@ -79,7 +79,8 @@ pub struct FrameCopy { pub frame_mmap: MmapMut, pub transform: wl_output::Transform, /// Logical region with the transform already applied. - pub region: LogicalRegion, + pub logical_region: LogicalRegion, + pub physical_size: Size, } impl TryFrom<&FrameCopy> for DynamicImage {