Fix: support correct saving for scaled displays

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<WlRegistry, ()> 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<WlOutput, ()> 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<ZxdgOutputV1, usize> 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,
                 };

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()

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,24 +596,26 @@ 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,
                             ))
                         });
 
                         Some(|| -> Result<_> {
                             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,

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
+    }
+}

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<OutputInfo>> for LogicalRegion {
     fn try_from(output_info: &Vec<OutputInfo>) -> std::result::Result<Self, Self::Error> {
         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 {

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 {