Added Wayland support via interactive or non-interactive call - #818
Added Wayland support via interactive or non-interactive call#818Photon89 wants to merge 15 commits into
Conversation
|
Hi Michael, I'm Maurizio. I wrote the gnome-screenshot hack for Ubuntu 24.04. I was looking for a solution to extend Shutter's life on Ubuntu 26.04, where gnome-screenshot no longer works. I tested your PR just now, and it seems to be a temporary but nevertheless pretty usable solution. Thanks, Maurizio |
|
Hi Maurizio, great, thanks for testing! As Gnome hasn't implemented non-interactive targets so far, Shutter has to fall back to the interactive mode chooser. That's actually worse than what you have implemented in your PR. And the sad news is that the non-interactive calls will be supposedly forwarded to gnome-screenshot once they have been implemented, so yeah... At least that's what happens in Cinnamon so far. Let's hope that this will be good enough for potential users. I guess, we've lost many of them due to lacking Wayland support... |
| $supported_targets{'1'} = 1 if ($targets_raw & 1); | ||
| $supported_targets{'2'} = 1 if ($targets_raw & 2); | ||
| $supported_targets{'4'} = 1 if ($targets_raw & 4); | ||
| $supported_targets{'8'} = 1 if ($targets_raw & 8); |
There was a problem hiding this comment.
in the non-raw version use names instead of digits? Or reverse, even keep it as raw number and check & directly?
There was a problem hiding this comment.
Good idea, I'll rework it to use names!
There was a problem hiding this comment.
I noticed that the names cannot be directly passed to xdg-portal, so we need to convert them back to numbers in Wayland.pm before they are passed to xdg-portal. Maybe it's actually better to keep the numbers?
There was a problem hiding this comment.
I'm fine with numbers, but then you don't need distinguishing between "raw" numbers and "non-raw" numbers
There was a problem hiding this comment.
Please check the latest commit, is this better now?
There was a problem hiding this comment.
Yeah, that looks simpler. Added a comment about a comment
| sub xdg_portal { | ||
| my $screenshooter = shift; | ||
| my $interactive = shift; | ||
| my $target = shift; |
There was a problem hiding this comment.
what does target mean? some comments would help
| # For some period of time desktops won't support non-interactive targets, we need to deal with it. | ||
| my $target; | ||
| my $interactive; | ||
| # For fullscreen capture just use plain xdg_portal |
There was a problem hiding this comment.
wouldn't that show the interactive prompt even on supporting compositors?
There was a problem hiding this comment.
Do you mean the full screen case? I'm not sure, that's the very early Wayland.pm which you wrote, I haven't looked inside tbh. For the moment, I didn't want to change anything with full screen capture because I assumed that it works well already. Do you know what the status actually is with full screen capture?
There was a problem hiding this comment.
what I did is a horrible hack :)
the problem with "my" method is that depending on compositor it will either show the prompt, or it won't, and shutter cannot detect that. This spawned the discussion in the first place.
I randomly put it to "fullscreen" button of shutter just because I needed to put it somewhere. It could as well be a new button which invokes the compositor-specific prompt (or even does screenshot without prompt, we never know); but depending on what users selects in that prompt in may not in fact be fullscreen.
If the specific implementation of the portal reports that it supports promptless fullscreen target, we should use that
There was a problem hiding this comment.
If the specific implementation of the portal reports that it supports promptless fullscreen target, we should use that
I see, will do! :)
|
I cannot find where you commented about #828 but yes, these PRs do conflict. But adding non-interactive fullscreen screenshot should help with per-monitor screenshot of that PR too. And non-interactive screenshot should be repeatable too, not just ones which happen to be interactive (or not) depending on DE |
Yeah, my comment was wrong so I deleted it, but the conflict is nevertheless there. I will try to figure out how to solve it. |
|
Is there anything to improve here besides of compatibility with #828? |
|
I can't easily test it, as I don't use wayland, but if you tested it, fine. Yeah, #828 is probably the only thing |
|
I did some testing in VMs before the latest changes. Will now also test full screen and report back. |
|
btw, we need to get some way to test all the wayland stuff in CI no clue if somethig like Xvfb exists |
|
Absolutely, could you look into it when time permits? I also noticed that the file name in Wayland is still just unknown_%nn.png, we can at least make it depend on the chosen mode if Wayland refuses to pass the window's name. |
|
xvfb does work in CI, I use it. Just using xvfb-run in a script works. |
|
@vadi2 That's what unit tests are doing here now. But it's X, not wayland. Or can wayland work on top of xvfb? |
|
Ahh, my bad. I haven't tried xvfb, but Claude was able to spin up a new gnome shell in Wayland for testing for me previously. |
|
So I implemented generic file names for Wayland capture. Since they cannot be acquired from xdg-portal, they are taken directly from the capture mode and can be "Workspaces", "Window" or "Selection w x h". Of course, specific window names won't work, and also no workspace numbers. I'm not sure why it is called "Workspaces" instead of just "Workspace", but it was this way in X11 already (what happens when you just choose the "full" capture mode. On my machine using Compiz, you just get the currently active workspace when using "full", so it is weird to be called "Workspaces". But this is the string which gettext knows, so I kept it this way. I now tested in Gnome and KDE. In both cases there is currently only the interactive API available. In Gnome (Fedora latest, should be Gnome 50) you have once more choose the mode that you actually intend to use, but at least all three modes are supported (full screen, window, selection) and work well. In KDE (tested in 6.2.5 and 6.6.5) you also have to once more choose the mode but, making things worse, there is no selection mode, not even interactively. I think, currently it is a better UX to fall back to the "hackish" full screen capture, because most DEs don't support it via non-interactive capture so far. What do you think? |
as fallback, sure. |
|
Okay, done, so we at least don't get a downgrade by this PR. Ready to merge once the merge conflicts are sorted out. Question is, which PR we merge first. I looked into the conflicts and they are actually not so bad. One conflict in Wayland.pm and three one line conflicts in the main shutter file. |
This PR is based on #817. However, it now tries to use an non-interactive target as defined in flatpak/xdg-desktop-portal#1981. If a target is not yet implemented by a DE, it falls back to an interactive call which shows the DE's interactive mode chooser. As soon as a DE implements the non-interactive targets, Shutter should pick them up and not call the interactive mode chooser any more. Also, the full screen capture is unaffected.
Tested for various DEs:
Tested non-interactive targets: