Closed brightcloudy closed 3 years ago
Hey thanks for the PR. I didn't have the time to look at all the changes yet, but I have some suggestions.
Maybe a command like the following, would be better to determine if the user is using wayland or X11.
loginctl show-session $(loginctl | grep $(whoami) | awk '{print $1}') -p Type | cut -c 6-
I don't have a wayland compatible wm installed currently, so could you post the output of this command:
$WLRRANDR | egrep '(current|Position|")'
Sure thing! I posted a picture in the PR which shows the output from that command in the terminal on the screen, but here's the relevant section:
[brighty@sunrise ~]$ $(command -v wlr-randr) | egrep '(current|Position|")'
DP-1 "Goldstar Company Ltd LG FULL HD 0x000067DE (DP-1)"
1920x1080 px, 74.973000 Hz (preferred, current)
Position: 0,0
HDMI-A-1 "Dell Inc. DELL U2410 C592M06I5VUL (HDMI-A-1)"
1920x1200 px, 59.950001 Hz (preferred, current)
Position: 1920,0
I picked those lines to pull out from the output of wlr-randr because they give the identifier of the output as well as its current resolution, refresh rate, and position on the screen.
About the other suggestion, I'd prefer to use a tool which already exists and was created for this task. I don't know what advantage using the output from loginctl
would give the script, and it creates more potential for unexpected behavior here. Parsing the session type from what loginctl
prints to the screen can easily break if the systemd developers change how that command displays its output.
I would have never expected this project would support wayland
I don't mind using a tool like xiswayland as long as it falls back to X when its not found
There's also an inverted boolean condition at the bottom which can be changed to = false
instead of ! = true
Other then that, i'll let botiapa merge this when they consider it done
The only problem really, is that xiswayland might not be installed even if a user is using wayland. For example IIRC arch doesn't include it in the package. But I agree that it's better to use a utility which is meant to be used this way.
Oh and one more thing. @brightcloudy you mentioned that you couldn't implement some features, because of wfrecorder. It might be a good idea to create an issue after merging, so when the tool does implement these, we don't forget about it.
And I just remembered that the readme has to be edited as well. Could you include that in your PR?
Thanks again.
Got it, those all are very reasonable changes. I had the thought the other day as well that a possible additional check for Wayland sessions is to test for the presence of the $WAYLAND_DISPLAY
environment variable, and that's simple enough to implement. I plan to finish those commits sometime today.
Thank you very much. Completely off topic: How do you like sway and wayland so far? Currently I'm using i3 and I love it, but I've been thinking about switching. The only problem is that I have an nvidia gpu and the open source driver isn't working very well on it as I observed.
I've heard sway and wayland is good, but yeah if you have a nvidia gpu you're basically forced to use nouveau which doesn't get good performance, sway doesn't support eglstreams, kde and gnome have somewhat working wayland on nvidia prop drivers, but it's not usable yet
I've addressed issues raised earlier in my recent commits and rebased them on top of the most recent commits to ShayBox/master. I will open the issue after merging, and I would like to add a section to the README to better document Wayland support as well which I can create another pull request for.
Thank you very much. Completely off topic: How do you like sway and wayland so far? Currently I'm using i3 and I love it, but I've been thinking about switching. The only problem is that I have an nvidia gpu and the open source driver isn't working very well on it as I observed.
I've found it to be a smooth experience, but I do have an AMD GPU. One of the features that I've really enjoyed is the fact that displays will now automatically be configured and enabled based on the matching sections in my config files, so that if I unplug a monitor it will automatically move my windows to where I can see them. There's also transparency support, since in Wayland the window manager is also the compositor, and tools like waybar are also pretty nice.
Is this ready to merge? I'm going to be implementing this in the deno rewrite, but I still want to merge this to get it in the bash version
Yes, I've been using the version that is my feature branch personally for a while now and haven't run into any issues, and it should be up to date with the most recent changes in your master branch.
Could you test the wayland feature on deno-rewrite branch to make sure it works
Just install deno via your package manager and ./mod.ts
in src
I recently switched from using i3 as my window manager to using sway so a lot of things that depended on various parts of X stopped working. The Xwayland compatibility layer only records a black screen if used to capture the display, but when I saw wf-recorder in the Fedora repositories I realized that it would be possible to add support for Wayland sessions to this script.
Currently, the script checks for the presence of the
xisxwayland
utility and if found, runs it to determine whether or not to use X or Wayland recording. I was uncertain about how common this utility is on systems which do not have Wayland stuff installed; on Fedora it is part of thexorg-x11-server-utils
package which includesxrandr
and other very common X utilities but I am not sure this is true for other distributions.The prompt to select the output uses wlr-randr, just to let the user select the output they want to stream in a similar fashion to how
xrandr
is used. The identifiers are no longer integers but strings like "DP-2" or "HDMI-A-1", so I changed the variable name to reflect this, though nothing changed in how it is used in the existing code.I was not able to add equivalent features for a couple things, like forcing a framerate, border/specific resolution, and vertical/horizontal flipping. This is mostly because currently wf-recorder doesn't allow attempting to capture a set number of frames per second, and it also doesn't allow adding arbitrary options to be passed to ffmpeg, like video filters. There's already at least one issue open requesting one of these features, and it seems to be under active development, so these may be implemented soon and I can then use those features in this script.