Hyprland icon indicating copy to clipboard operation
Hyprland copied to clipboard

Screencopy / Toplevel Export with shm does not transform image data properly

Open vaxerski opened this issue 7 months ago • 15 comments

Discussion link

https://github.com/hyprwm/Hyprland/discussions/9910

Description

When copying via shm, the window / screen's contents are not transformed properly to be upright.

vaxerski avatar Apr 07 '25 10:04 vaxerski

@WhySoBad can you still repro this? I can't

vaxerski avatar Apr 29 '25 17:04 vaxerski

Yes, I can still reproduce it on hyprland-git v0.48.0-110-gf5c5cfa96 (I was unable to start v0.48.0-118 as it just hung up permanently)

WhySoBad avatar Apr 29 '25 20:04 WhySoBad

how? grim works for me

vaxerski avatar Apr 29 '25 20:04 vaxerski

I use a modified version of my hyprland-preview-share-picker to test this. The released version uses IPC to get the monitor transform for the monitor the windows are on (or for the monitor directly) and applies this. I've just disabled that the transform is applied.

I've had a look at grim's codebase and saw that they use the transform property of ext-image-copy-capture-v1 protocol (https://wayland.app/protocols/ext-image-copy-capture-v1#ext_image_copy_capture_frame_v1:event:transform) to transform the captured frames accordingly (https://gitlab.freedesktop.org/emersion/grim/-/blob/master/output-layout.c).

WhySoBad avatar Apr 30 '25 06:04 WhySoBad

Ah nevermind, this protocol is not even implemented by hyprland. They use the transform supplied by the wl_output geometry here https://gitlab.freedesktop.org/emersion/grim/-/blob/master/main.c#L237

WhySoBad avatar Apr 30 '25 07:04 WhySoBad

patch.txt can you try this

vaxerski avatar May 01 '25 23:05 vaxerski

patch.txt can you try this

With this patch it's exactly the same as without it.

Image

WhySoBad avatar May 02 '25 17:05 WhySoBad

can you give some repro steps?

vaxerski avatar May 04 '25 21:05 vaxerski

As I've mentioned earlier, I always test it using a modified version of hyprland-preview-share-picker. With the following steps you can test it yourself:

  1. clone https://github.com/WhySoBad/hyprland-preview-share-picker and it's submodules

  2. apply the following patch to the master branch which prevents transforming the images on the client

diff --git a/src/views/outputs.rs b/src/views/outputs.rs
index 614cf58..0aa0e1c 100644
--- a/src/views/outputs.rs
+++ b/src/views/outputs.rs
@@ -344,7 +344,7 @@ impl<'a> OutputCard<'a> {
                 };
 
                 img.resize_to_fit(resize_size);
-                img = img.transform(transform.into());
+                // img = img.transform(transform.into());
 
                 if tx.send(img).is_err() {
                     log::error!("unable to transmit image for name {name}: channel is closed");
diff --git a/src/views/windows.rs b/src/views/windows.rs
index 53668fe..250dc96 100644
--- a/src/views/windows.rs
+++ b/src/views/windows.rs
@@ -206,7 +206,7 @@ impl<'a> WindowCard<'a> {
                 };
 
                 img.resize_to_fit(resize_size);
-                img = img.transform(transform.into());
+                // img = img.transform(transform.into());
 
                 if tx.send(img).is_err() {
                     log::error!("unable to transmit image for toplevel {id}: channel is closed");

  1. run cargo run (image loading is a bit slower since it's a debug build but compilation is much faster)

The Windows tab won't show anything as the XDPH_WINDOW_SHARING_LIST env is not set but on the Outputs tab you see the sceencopies of the monitors as they are returned from the server.

I've just realized a bit of a inconsistency. Assume I have a monitor which is rotated 270deg (or 90deg), then the height/width of the rotated image are flipped. However, the IPC will still return the original resolution (without flipped axes).

I think this is intended but it's also a bit weird since

  • when we rotate the image on the server we do apply the transformation
  • when we return the monitor on IPC we don't apply the transformation

Anyways, I don't think it's a problem but I just wanted to throw it in as it may be a bit confusing.

Also, on my previous response I accidentally removed the code which flips the height/width on the client if needed. The actual image should be

Image

WhySoBad avatar May 05 '25 08:05 WhySoBad

I think this might be correct, since everything else works?

vaxerski avatar May 07 '25 17:05 vaxerski

What do you mean? That the transforms are not applied on the server?

WhySoBad avatar May 08 '25 06:05 WhySoBad

yes, essentially

vaxerski avatar May 08 '25 16:05 vaxerski

Well I think we need to consider a few things before closing this issue.

When we say non-rotated buffers are the standard (since grim which uses shm relies on them being not rotated), then the dmabuf implementation is should be changed to also not rotate. However, in OBS which uses the pipewire stream from the desktop portal (which uses dmabufs behind the scenes) the screen capture of an output has the correct rotation.

When capturing a toplevel using OBS which again uses the desktop portal (with dmabuf?) the toplevel is not rotated. For shm I know for sure that the toplevels aren't rotated as well.

In my opinion we should keep shm and dmabuf implementations for the same protocol the same (meaning, when it's rotated using shm it should also be rotated using dmabuf).

Additionally, I think for toplevel exports we should apply the transformation directly on the server. The reason for this is that for a non-hyprland specific application the relation between a toplevel and a wloutput containing the transformation is not very clear.

I think for output exports it doesn't matter whether we rotate or not (since it's always possible to get the transformation on the client from the wloutput object which one needs anyways to request the export) as long as it's the same for shm and dmabuf.

With grim and OBS we know two applications which use the output export. Grim expects the output export to not contain the transformation whilst OBS expects it to have it applied. I think we should collect a list of applications using those protocols with either shm or dmabuf and then implement the version which breaks less apps.

WhySoBad avatar May 15 '25 07:05 WhySoBad

feel free to gather that list then. With toplevel export I agree.

vaxerski avatar May 15 '25 09:05 vaxerski

After a bit more thinking about it, I think we can save collecting the list.

The output export also needs to be transformed since (sandboxed) applications which use the pipewire interface for screen capture, only have limited access to the wloutput object and can therefore not know about the transformation of the output.

WhySoBad avatar May 15 '25 09:05 WhySoBad