Hyprland
Hyprland copied to clipboard
Screencopy / Toplevel Export with shm does not transform image data properly
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.
@WhySoBad can you still repro this? I can't
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)
how? grim works for me
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).
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
patch.txt can you try this
can you give some repro steps?
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:
-
clone https://github.com/WhySoBad/hyprland-preview-share-picker and it's submodules
-
apply the following patch to the
masterbranch 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");
- 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
I think this might be correct, since everything else works?
What do you mean? That the transforms are not applied on the server?
yes, essentially
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.
feel free to gather that list then. With toplevel export I agree.
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.