Skip to content
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

Use output_management_v1 protocol for wayland rotation #53

Merged
merged 14 commits into from
Oct 4, 2023

Conversation

Smona
Copy link
Contributor

@Smona Smona commented Sep 30, 2023

Switches to using the wlr_output_management_v1 protocol for any wayland compositors which support it, expanding screen rotation support to them. I've omitted a version bump, but I think this change definitely justifies at least a minor version bump. It seems like the options interface is also pretty stable and this would technically break rot8 for anyone using ancient versions of sway without the output management protocol, so maybe this justifies a 1.0.0 release 🙂

Resolves #5

Design

This uses the safe wayland-rs wrappers to communicate with the wayland socket on startup. If it's accessible and advertises support for output management, the wayland backend will be selected. If sway is running, a specialized sway backend will be used instead. If wayland can't be connected to, it will check if X11 is running and switch to it like the existing impl does. If neither the output management protocol nor an X11 process exist, it will exit with an error.

I chose to drop the sway-specific rotation method in the interest of code sharing & simplicity. I think this is a good tradeoff, since sway has supported the output_management protocol for a very long time, and communicating via the socket seems to be a bit snappier than the subprocess call. This does interact a bit awkwardly with #45 though (see inline comment).

Since working with the wayland protocols requires a lot of event listener code, main.rs would have gotten pretty inscrutable if I had sprinkled the per-backend logic throughout main.rs like the current implementation does. So I decided to move all the platform-specific logic into a DisplayManager trait, with one struct per implementation. The SwayBackend implementation simply wraps the WaylandBackend, to preserve keyboard disabling functionality. The logic for the XorgBackend should be entirely unchanged, besides making the touchscreens argument Vec<String> instead of Vec<&str>. This design makes the main loop easier to understand, and keeps all the platform-specific code nice and centralized.

Testing

Tested thoroughly on Sway and Hyprland where I have been using this branch for a few weeks (minus the merge commit). I also tested the --disable-keyboards option on Sway, and rotation with exwm, which was the only X window manager I have lying around. Since the main event loop works the same way and the wayland socket is not read/written unless the orientation changed, performance is not noticeably different.

Comment on lines +269 to +270
let old_env = transform_to_env(&old_state);
let new_env = transform_to_env(&current_orient.wayland_state);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not really sure what the best way to set these env variables is now. Before #45 was merged, I switched the tracking variable to use the Transform enum from wayland-rs, since the values in new_state were no longer needed anywhere else. and it got rid of some bidirectional mapping code.

For now I've kept the user-facing API identical to #45, but imo it's a bit confusing because swaymsg and wayland transforms use opposite rotation directions for some reason. I think the best way to go would be setting the ORIENTATION and PREV_ORIENTATION env variables using the less-ambiguous x_state values. What do you think?

@efernau efernau added the enhancement New feature or request label Oct 4, 2023
@efernau efernau merged commit dda7e2b into efernau:master Oct 4, 2023
4 checks passed
@Smona Smona deleted the wlroots-support branch October 4, 2023 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider the wlr-output-management protocol over swaymsg
2 participants