Skip to content

Fix recording with rotated monitors #313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed

Conversation

dotaxis
Copy link

@dotaxis dotaxis commented Jul 13, 2025

Addresses #173

Not sure this is the best way to go about this, but open to suggestions.

Only tested with 90 and 270 degree rotations because I can't handle mirroring. I guess I could add 180 degree rotation without too much work.

Still requires -F transpose= to be used get the actual output file to be rotated accordingly.

wf-recorder -F transpose=1 --transform 1 -g "107,2000 1247x532" -f ~/Videos/test.mp4 works as expected on a monitor rotated 90 degrees, and coordinates grabbed via slurp.

wf-recorder -F transpose=2 --transform 3 -g "107,2000 1247x532" -f ~/Videos/test.mp4 works the same when using 270 degree rotation.

@apiraino
Copy link

Just a .2 cents worth comment:

I use sway and with swaymsg -t get_outputs I get the current transform of the displays (man page), which tells me if the display is rotated. Example:

$ swaymsg -t get_outputs 
Output eDP-1 'LG Display 0x0521 Unknown'
  Current mode: 1920x1080 @ 60.020 Hz
  Power: on
  Position: 320,1440
  Scale factor: 1.000000
  Scale filter: nearest
  Subpixel hinting: unknown
  Transform: normal
  Workspace: 1
  Max render time: off
  Adaptive sync: disabled
  Allow tearing: no
  Available modes:
    1920x1080 @ 60.020 Hz
    1920x1080 @ 47.999 Hz

These info are exposed by the compositor (in this case wl_root). I'm curious if there could be a generic way to programmatically retrieve this transform value of a display without having the user needing to worry about it.

@soreau
Copy link
Collaborator

soreau commented Jul 14, 2025

I suppose that wf-recorder could be packaged with a python script which connects via pywayfire ipc to get the output information (as a way to side-step having wf-recorder reinvent the ipc wheel in C++, though maybe it could live in ipc.cpp). It could be supplied an output name and and then output the current transform for it. However, this would require ipc + ipc-rules to be enabled and wayfire be patched to yield this information when queried.

@soreau
Copy link
Collaborator

soreau commented Jul 14, 2025

Actually, this patch gets the transform of outputs upfront (and when they change). If you can pipe that into the transform, it should be automatic.

@dotaxis
Copy link
Author

dotaxis commented Jul 14, 2025

I actually realized right after I opened this that grim handles transforms automatically by polling the transform info from the compositor and that it would be much better to do that here as well.

I'll work on this using @soreau's patch above and update this when I make headway.

@soreau
Copy link
Collaborator

soreau commented Jul 14, 2025

This version of the patch removes variable names that are unused to avoid warnings.

@dotaxis
Copy link
Author

dotaxis commented Jul 14, 2025

Alright, I've pushed a couple of commits to handle rotation automatically, but I will have to spin my monitor around in circles and do a bunch of testing to account for all the possible cases.

If anyone wants to build and test with 90 and 270 rotations for now, it would be much appreciated.

@dotaxis dotaxis changed the title Add --transform flag to account for rotated monitors Fix recording with rotated monitors Jul 14, 2025
Copy link
Collaborator

@soreau soreau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall it looks good to me, aside from the two minor things I commented on.

@dotaxis
Copy link
Author

dotaxis commented Jul 14, 2025

I'm going to move the switch statement into contained_in() so we don't have a redundant rotate_coordinates value and if block, and clean some other stuff up while I'm at it. Thanks for going over it! Will ping when ready.

Edit: couldn't move it into contained_in as the x and y values need to be modified outside of its scope. I've cleaned it up a bit and will add some better console output and also account for more transform settings tomorrow afternoon.

Copy link
Collaborator

@soreau soreau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's shaping up, but there a few things that need attention.

@soreau
Copy link
Collaborator

soreau commented Jul 15, 2025

@dotaxis Hi, I sort of got off into the weeds the other day but today I tested a bit and came up with this. It pipes the transform into frame-writer.cpp, where it sets the transpose like you were doing with the -F flag. Glean what you will, hope it helps. Cheers. 👍

@dotaxis
Copy link
Author

dotaxis commented Jul 16, 2025

@dotaxis Hi, I sort of got off into the weeds the other day but today I tested a bit and came up with this. It pipes the transform into frame-writer.cpp, where it sets the transpose like you were doing with the -F flag. Glean what you will, hope it helps. Cheers. 👍

The goal of this PR is not to apply transposition. It is to fix the handling of coordinates which do not work in a logical way when a monitor is rotated.

@soreau
Copy link
Collaborator

soreau commented Jul 16, 2025

@dotaxis Hi, I sort of got off into the weeds the other day but today I tested a bit and came up with this. It pipes the transform into frame-writer.cpp, where it sets the transpose like you were doing with the -F flag. Glean what you will, hope it helps. Cheers. 👍

The goal of this PR is not to apply transposition. It is to fix the handling of coordinates which do not work in a logical way when a monitor is rotated.

This already works. What version of wayfire are you using? If you can use wayfire latest git, this would be optimal.

@dotaxis
Copy link
Author

dotaxis commented Jul 16, 2025

I am not using wayfire, I'm using hyprland. Nothing in this project hints at being exclusive to wayfire users.

@soreau
Copy link
Collaborator

soreau commented Jul 16, 2025

I am not using wayfire, I'm using hyprland. Nothing in this project hints at being exclusive to wayfire users.

Can you or anyone test with wayfire? Maybe this is an issue with some other compositors but not wayfire.

@dotaxis
Copy link
Author

dotaxis commented Jul 16, 2025

Can you test with Aquamarine?

If this project is only meant to support Wayfire, perhaps the readme should be updated to not confuse users of other compositors.

@soreau
Copy link
Collaborator

soreau commented Jul 16, 2025

Can you test with Aquamarine?

I refuse to install any untrusted software on my machine.

If this project is only meant to support Wayfire, perhaps the readme should be updated to not confuse users of other compositors.

It is meant to support any compositor that properly implements the wlr_screencopy protocol.

@dotaxis dotaxis closed this Jul 16, 2025
@apiraino
Copy link

thank you @dotaxis anyway for kicking off the work to fix the issue. That's really appreciated! 🙂

@soreau
Copy link
Collaborator

soreau commented Jul 16, 2025

I wonder if wf-recorder --no-dmabuf makes any difference..

@ammen99
Copy link
Owner

ammen99 commented Jul 17, 2025

@dotaxis I have tested wf-recorder with sway+slurp and coordinates work normally. Maybe Hyprland/Aquamarine do not handle the rotation correctly. In any case, since it works on Sway/Wayfire, if we include patches to make it work on Hyprland, it would break the other compositors. And this project is aimed first and foremost at wlroots-based compositors.

Unless I am mistaken, is there anyone who is having issues (beyond the video being transposed) on Sway / River / Wayfire / etc ?

@soreau
Copy link
Collaborator

soreau commented Jul 17, 2025

I have tested sway and wayfire with 90 rotation - both work with grim or wf-recorder using slurp. If you are having this issue, can you try with wf-recorder --no-dmabuf to see if it makes a difference?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants