ksnip / ksnip

ksnip the cross-platform screenshot and annotation tool
GNU General Public License v3.0
2.43k stars 183 forks source link

Add support for cross-platform wayland screenshots using xdg-desktop-portal #243

Closed maximbaz closed 4 years ago

maximbaz commented 4 years ago

I noticed you currently use vendor-specific code to take screenshots on Gnome and KDE and do not support any other DE.

I currently use sway and it's a shame ksnip is not able to take screenshots there.

In fact there is a cross-platform way of taking screenshots, which I discovered here: https://github.com/lupoDharkael/flameshot/issues/446

To quote that ticket:

The cross-platform way to take screenshots on Wayland is via xdg-desktop-portal (which also works outside of Flatpak). See https://github.com/flatpak/xdg-desktop-portal/blob/master/data/org.freedesktop.portal.Screenshot.xml

DamirPorobic commented 4 years ago

Thanks for bringing this to my attention. When I implemented Wayland support at first there was only the Vendor Specific solution. Due to private reasons I don't have much time available so can't say when this will be implemented, but it seems to be one of the more important features. Though we still need to evaluate the xdg-desktop-portal solution.

DamirPorobic commented 4 years ago

Looks like this is coming with the next release. Snaps and Flatpaks relay on this method too so I will implement it and it will be the default for wayland.

DamirPorobic commented 4 years ago

@maximbaz Could you test by any chance the continuous build on sway wayland? I have added a generic portal wayland handler and seems to be working on Plasma and GNOME. The Generic Wayland handler is also used by default for all snaps.

maximbaz commented 4 years ago

It works 👍

Things that I noticed, that may or may not be issues:

  1. The button "New" says that it opens a dialog for selecting type of screenshot, but when I press the button, it just takes screenshot of the entire desktop, there is no dialog.
  2. I use 2.5 scale with my HiDPI screen, and when the screenshot is taken, it is rendered inside twice as large comparing to reality. Here's the demo, on the very top you can see my bar in the size that it really is, but when I press "New" button, the screenshot appears inside ksnip and you can see that the bar is a lot larger:

image

DamirPorobic commented 4 years ago

Thanks for the quick test. It's still under development, issues are expected.

  1. Strange, based on the definition of the portal I was expecting a dialog where you can choose what type of screenshot you want and if you want t share it (GNOME) or save it (Plasma), so both show a dialog, here is now dialog as it looks like. Maybe I just change the name of the option to Screenshot or some other generic name.

  2. The scaling is interesting, I'll have a look into that.

  3. Can you see where the image was save? Plasma saves it under ~ and GNOME under ~/Pictures. The screenshot name seems to be out, what name do you get if you take another one?

maximbaz commented 4 years ago

Once a click on "New" button, it saves the screenshot in /tmp/out.png. A second press on "New" will use the same file name and override the previous image:

image

DamirPorobic commented 4 years ago

That's the world of Wayland, you have a generic portal and everyone does it's own stuff again. Plasma and GNOME permanently store the image in theusers home directory with a timestamp. Your screenshot will be deleted by the OS.

We could check if the path contains /tmp and in case it does, we don't show it as a saved image (like with Plasma and GNOME) but just like a new created screenshot that is not yet saved.

DamirPorobic commented 4 years ago

I'll look into it after work and push a new version hopefully tonight.

DamirPorobic commented 4 years ago

@maximbaz Can you check if the screenshot you've captured and that was saved und /tmp/out.png is scaled already or is it a ksnip issue? I have also screen scaling (2.0x) and for me the screenshot is shown correctly. I'll check on GNOME next what there the behavior is.

maximbaz commented 4 years ago

The screenshot itself has correct dimensions, so it's purely rendering issue inside ksnip

$ file /tmp/out.png
/tmp/out.png: PNG image data, 3840 x 2400, 8-bit/color RGB, non-interlaced
DamirPorobic commented 4 years ago

Yeah but the same code renders the scaling correct on Plasma and GNOME, so there must a difference. What Qt Version do you have and how did you install ksnip?

maximbaz commented 4 years ago

5.15.1, via AUR/ksnip-git 🙂

DamirPorobic commented 4 years ago

Qt has different behaviors with scaling across different versions and it's a major PITA. Any chance you could try out the AppImage? The AppImage should have it's own Qt version with is lower then the one you have there.

maximbaz commented 4 years ago

Same result.

Not sure if this is important, but will mention for reference:

I use this environment variable: QT_QPA_PLATFORM=wayland-egl (no other Qt env vars).

When I launch AppImage, it fails with

$ ~/Downloads/ksnip-1.8.0-continuous-x86_64.AppImage
This application failed to start because it could not find or load the Qt platform plugin "wayland-egl"
in "".

Available platform plugins are: xcb.

And when I launch it like this, it works, but still produces enlarged image inside ksnip:

$ QT_QPA_PLATFORM=xcb ~/Downloads/ksnip-1.8.0-continuous-x86_64.AppImage
DamirPorobic commented 4 years ago

Thanks for checking, I'll keep troubleshooting.

DamirPorobic commented 4 years ago

@maximbaz whats the simplest way to get a working sway environment?

maximbaz commented 4 years ago

Hmm, I'd say install sway package, copy /etc/sway/config to ~/.config/sway/config, on line 17 replace alacritty with terminal that you have, also maybe change $mod to be Mod1 (I cant remember what default button is, Mod1 is Alt), then finally open another tty with Ctrl+Shift+F3 (or other), and in there type the below:

export LIBVA_DRIVER_NAME=iHD
export QT_QPA_PLATFORM=wayland-egl
export XDG_CURRENT_DESKTOP=sway
export XDG_SESSION_TYPE=wayland

sway

(probably not all environment variables are needed, these are just all relevant that I personally have).

When sway is launched, you can open terminal with Alt+Enter, and from there launch browser, or launch AppImage file, etc.

To set custom scale, use $ swaymsg -t get_outputs to see the output name (in my case eDP-1) and then $ swaymsg output eDP-1 scale 2.5 (or other value).

I guess that's pretty much it, give it a go and ping me if you get stuck!

DamirPorobic commented 4 years ago

Awesome, thanks for the detail instruction! I'm going to give it a try.

DamirPorobic commented 4 years ago

I haven't tested sway yet, need to setup the VM but I did a quick test on KDE Neon which also uses Qt 5.15.x and there it seems to be working too.

What I have noticed is that the taken screenshot is always the size of the scaled desktop. For example, my native resolution on this ancient test notebook is 1440 x 900. When I take a screenshot with ksnip with scaling set to default 100% and check the saved image that was generated by the OS and saved in my home directory I get: /home/dporobic/Pictures/Screenshot_20201003_093935.png: PNG image data, 1440 x 900, 8-bit/color RGBA, non-interlaced

When I change my scaling now to 125% I get: /home/dporobic/Pictures/Screenshot_20201003_094046.png: PNG image data, 1152 x 720, 8-bit/color RGBA, non-interlaced

But according to your output, you must have a native resolution of 7680 x 4800 (which seem kind of big) or your OS doesn't behave like Plasma. I'll check this behavior on GNOME to but I assume it behaves the same.

maximbaz commented 4 years ago

My screen resolution is 3840x2400, and I just double-checked, regardless of what scale I set, the /tmp/out.png always has resolution of 3840x2400px.

If I set the scale to 1, then the image inside ksnip has exactly the same size as reality, so when I use scale, I imagine it's something like sway scales ksnip and ksnip internally scales the rendered image by the same factor...

It's silly how much difference there is between all wayland implementations..........

DamirPorobic commented 4 years ago

Next fun fact, GNOME also saves the image with the same resolution no mater what scale you have (just like sway) but there the image is shown in ksnip correct, just like with Plasma that uses different sizes per scaling. What a mess...

I'm thinking about having a config option that is only available for Wayland that allows the users to define if the image captured should be scaled by display scaling factor. By default it would be false and you could set it to true for example.

DamirPorobic commented 4 years ago

It's getting stranger and stranger. My Wayland implementation is not setting any scaling factor. I'm sure that this works under X11 but under wayland, it's just 1 event though I have scaling of 200% enabled. I have pushed a new version where you can enable scaling in settings under ImageGrabber, it should write out the scale factor and set the devicePixelRatio. Can you text and let me know what scale factor was reported on console?

maximbaz commented 4 years ago

Scale factor is 1 - I'm using AppImage now, to make sure all the dependency versions are predictable for you.

DamirPorobic commented 4 years ago

Thanks. I have pushed a new commit, now querying QWindow, QScreen and QWidget for device pixel ratio information. If they don't provide anything useful, the only option that we have is probably setting the required scale factor by hand in setting.

Can you post again here the console output?

maximbaz commented 4 years ago

Wow, wait, I have some good news for once!

I think Qt 5.14 doesn't run properly on Wayland, for one or another reason. If I compile ksnip-git myself, and run it on Qt 5.15, with QT_QPA_PLATFORM=wayland-egl, then with Scale generic Wayland screenshots the image is almost correct!

To remind, my sway output scale is 2.5.

The latest commit prints this:

DesktopWidget devicePixelRatio: 3
DesktopWidget devicePixelRatioF: 3
DesktopWidget logicalDpiX: 96
DesktopWidget logicalDpiY: 96
DesktopWidget physicalDpiX: 135
DesktopWidget physicalDpiY: 135
QScreen devicePixelRatio: 3
QScreen logicalDotsPerInch: 96
QScreen logicalDotsPerInchX: 96
QScreen logicalDotsPerInchY: 96
QScreen physicalDotsPerInch: 135
QScreen physicalDotsPerInchX: 134.532
QScreen physicalDotsPerInchY: 135.467
QWindow devicePixelRatio: 3
Scale factor is 3   <------------------------

And indeed the image in ksnip looks a little smaller than reality. Could it be that you just need to switch from integer to floating number containing the output scale, so that 2.5 wouldn't get rounded to 3, and declare a great success?

And trying the same last commit via AppImage, with QT_QPA_PLATFORM=xcb, the output is:

DesktopWidget devicePixelRatio: 1
DesktopWidget devicePixelRatioF: 1
DesktopWidget logicalDpiX: 96
DesktopWidget logicalDpiY: 96
DesktopWidget physicalDpiX: 135
DesktopWidget physicalDpiY: 135
QScreen devicePixelRatio: 1
QScreen logicalDotsPerInch: 96.0473
QScreen logicalDotsPerInchX: 96.0946
QScreen logicalDotsPerInchY: 96
QScreen physicalDotsPerInch: 135
QScreen physicalDotsPerInchX: 134.532
QScreen physicalDotsPerInchY: 135.467
QWindow devicePixelRatio: 1
Scale factor is 1

I think this is happening because AppImage / Qt 5.14 runs ksnip in XWayland simulation mode, the reason I think so is because AppImage ksnip has slightly blurry menu, while ksnip compiled by me myself has as if sharper text. Let's see if screenshots help seeing the difference:

AppImage my build
image image
maximbaz commented 4 years ago

And by the way, now when I take a screenshot, they are named Capture 1, Capture 2, etc, and when I save them, they get saved as $HOME/ksnip_20201003-164758.png

DamirPorobic commented 4 years ago

Unfortunately, QWindow::devicePixelRatio() return only integers and the method that provides the number for the "Scale factor is" output is already a qreal, which is a floating point number. Good point with the simulated XWayland (I was wondering about the blurry icons already), after switching to Full Wayland Plasma I get there larger numbers then 1 too, but same like in your case, for Scaling 125% I get a Scale factor of 2, which is wrong. Under X11 those methods, especially the last one, returns floating point numbers. What do you get when you disable now the scaling in config, is it again to large?

Should we try with manually setting the scale factor?

Regarding the naming, yes, I did that. We are checking if the path contains /tmp/ in that case we treat the image as unsaved because with the next reboot it's going to be delete. You get there the default screenshot behavior, unsaved image.

maximbaz commented 4 years ago

What do you get when you disable now the scaling in config, is it again to large?

Yes

Should we try with manually setting the scale factor?

Sure, you could maybe pre-fill it with the QWindow::devicePixelRatio() value, and let the user override it to a floating value?

Regarding the naming, yes, I did that.

Nice, I like it 👍

DamirPorobic commented 4 years ago

So,give it another try. You can set the value now in settings.

maximbaz commented 4 years ago

Once I set factor to 2.5, the size of everything inside ksnip now matches the reality perfectly.

image

But the image is ever so slightly out of sharpness, comparing to reality, it's hard to spot on the screenshot perhaps, but it lead me to have a look at the image, and look what I found, the saved image has wrong dimensions!

$ file ksnip_20201003-182038.png
ksnip_20201003-182038.png: PNG image data, 4608 x 2880, 8-bit/color RGBA, non-interlaced

If I set the factor to 3, then the image size is correct:

$ file ksnip_20201003-182413.png
ksnip_20201003-182413.png: PNG image data, 3840 x 2400, 8-bit/color RGBA, non-interlaced

and if I disable the "Scale generic wayland screenshots" altogether, the saved image is ridiculously large:

$ file ksnip_20201003-182447.png
ksnip_20201003-182447.png: PNG image data, 11520 x 7200, 8-bit/color RGBA, non-interlaced
DamirPorobic commented 4 years ago

With the Scale Factor set to 2.5 the screenshot in ksnip is perfect but the exported image is blurry?

With the Scale Factor set to 3 the screenshot in ksnip is slightly too large but the exported image is perfect?

I believe the issue here is that kImageAnnotator, which is responsible for the tabs and everything in them, is also relaying on scaling, which it gets from Qt as being 3, and on the captured image in ksnip we set it to 2.5 so we get this mismatch. Bringing this in sync is difficult. I'm thinking now about reverting the change back with manually setting the scale factor and only leave the checkbox (Scale Generic Wayland Screenshots). For the Qt part I think I should log an issue, returning 3 for Screen Scaling of 250% seems wrong. Could that work for you for now having a slightly larger screenshot in ksnip?

DamirPorobic commented 4 years ago

Also, something that comes to my mind, might or might not help in your case but it's worth trying out. Another user has some issues with KDE Neon and HiDPI and he found a workaround by setting some Qt Environment variables. If you want to try out, here at the end of the ticket https://github.com/ksnip/ksnip/issues/276

maximbaz commented 4 years ago

I think this is the best course of action, use the API that is supposed to return 2.5 and report an upstream bug that it returns 3 instead, and while it returns 3 we say it's expected behavior. Then at least the saved image will have correct dimensions, and inside ksnip it looks although not perfectly scaled, but reasonably good.

I just compiled ksnip from master, yes let's leave it at that.

I discovered another issue, which I didn't think to check before.

Right now, when I take screenshot (with checkbox ticked), I see the image of full screen in ksnip. But when I save it, the image is clipped, not in size (its dimensions are 3840x2400px), but in the contents.

Here's how ksnip looks like after taking screenshot:

image

And here is the saved image:

ksnip_20201003-232942

$ file ksnip_20201003-232942.png
ksnip_20201003-232942.png: PNG image data, 3840 x 2400, 8-bit/color RGBA, non-interlaced

By the way, I tried running with environment variables from #276 but there is no difference.

DamirPorobic commented 4 years ago

Oh boy, that's not good. I think we need to touch the kImageAnnotator.

I have pushed a small change, moving setting the devicePixelRatio on the final image after the image was painted. Let's see if this helps. If you're bulding from source, you will need to pull latest from kImageAnnotator, build & install it again then build ksnip.

maximbaz commented 4 years ago

I think it's working perfectly now, the image is looking good, both in ksnip and the saved one, thank you! 👍

DamirPorobic commented 4 years ago

Honestly, I thought that was too simple to work :smile:

Glad it's working now. What about the 2.5 vs 3 issue? Is the image in ksnip still slightly larger then in real?

maximbaz commented 4 years ago

its slightly smaller, but I think it's fine 👍 shall we close this issue?

DamirPorobic commented 4 years ago

Yes, but I think I'll log a bug with Qt, just for reference before closing this one.

DamirPorobic commented 4 years ago

Bug logged with Qt https://bugreports.qt.io/browse/QTBUG-87321