Bug 433776 - Taking screenshots on Wayland with HiDPI takes too long
Summary: Taking screenshots on Wayland with HiDPI takes too long
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: effects-various (show other bugs)
Version: git master
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: KWin default assignee
URL:
Keywords: usability
Depends on:
Blocks:
 
Reported: 2021-03-01 07:38 UTC by Vlad Zahorodnii
Modified: 2021-03-24 17:34 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: Spectacle 21.08 or newer with Plasma 5.22 or newer


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Vlad Zahorodnii 2021-03-01 07:38:26 UTC
SUMMARY
The Screenshot effect encodes the captured screenshots as PNG images. This incurs a lot of overhead.

Instead, the screenshot plugin should send screenshots as raw images. Unfortunately, the current dbus api doesn't allow doing that. It can be changed, but it's probably better to introduce a new interface (org.kde.kwin.Screenshot2) and deprecate the old one.
Comment 1 Vlad Zahorodnii 2021-03-09 09:02:07 UTC
Git commit 1fb44b5bd500a32cb47062b41453848df1799f4f by Vlad Zahorodnii.
Committed on 09/03/2021 at 08:58.
Pushed by vladz into branch 'master'.

effects/screenshot: Prepare for versioned dbus interface

On Wayland, when the compositor sends a screenshot to the requesting
app, it encodes the screenshot as a PNG image and sends the encoded data
over the pipe. The requesting app (Spectacle) then needs to decode the
data.

The issue is that encoding PNG images is not cheap. This is the main
reason why Spectacle is shown with a huge delay after you press the
PrtScr key.

In order to fix the latency issue, we need to transfer raw image data.
Unfortunately, the current dbus api of the screenshot is too cluttered
and the best option at the moment is to start with a clean slate.

This change prepares the screenshot effect for versioned dbus interface.
Most of existing dbus logic was moved out in a separate class. In order
to schedule screen shots, the screenshot effect got some new API.

    QFuture<QImage> scheduleScreenShot(window, flags)
    QFuture<QImage> scheduleScreenShot(area, flags)
    QFuture<QImage> scheduleScreenShot(screen, flags)

If a dbus interface needs to take a screenshot, it needs to call one of
the overloaded scheduleScreenShot() functions. Every overload returns a
QFuture object that can be used for querying the result.

This change also introduces "sink" and "source" objects in the dbus api
implementation to simplify handling of QFuture objects.

Note that the QFutureInterface is undocumented, so if you use it, you do
it on your own risk. However, since Qt 5.15 is frozen for non-commercial
use and some other Plasma projects already use QFutureInterface, this
is not a big concern. For what it's worth, in Qt 6, there's the QPromise
class, which is equivalent to the QFutureInterface class.
Related: bug 430869

M  +1    -0    src/effects/screenshot/CMakeLists.txt
M  +307  -699  src/effects/screenshot/screenshot.cpp
M  +60   -139  src/effects/screenshot/screenshot.h
A  +860  -0    src/effects/screenshot/screenshotdbusinterface1.cpp     [License: GPL(v2.0+)]
C  +61   -74   src/effects/screenshot/screenshotdbusinterface1.h [from: src/effects/screenshot/screenshot.h - 066% similarity]

https://invent.kde.org/plasma/kwin/commit/1fb44b5bd500a32cb47062b41453848df1799f4f
Comment 2 Bug Janitor Service 2021-03-09 09:06:15 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/kwin/-/merge_requests/759
Comment 3 Bug Janitor Service 2021-03-09 09:11:12 UTC
A possibly relevant merge request was started @ https://invent.kde.org/graphics/spectacle/-/merge_requests/54
Comment 4 Méven Car 2021-03-24 11:37:43 UTC
Git commit 0683597099cddc1f54966f1546ae8fada3aa7dd6 by Méven Car, on behalf of Vlad Zahorodnii.
Committed on 24/03/2021 at 10:52.
Pushed by vladz into branch 'master'.

effects/screenshot: Introduce dbus interface version 2

With the new interface, the compositor sends raw image data instead of
encoding it as a png image, which causes a lot of overhead on Wayland.

In addition to that, the new interface is more extensible, for example
we can add new options or change the written image data format, however
the latter is less likely to happen.

M  +3    -0    src/effects/screenshot/CMakeLists.txt
A  +184  -0    src/effects/screenshot/org.kde.KWin.ScreenShot2.xml
M  +2    -0    src/effects/screenshot/screenshot.cpp
M  +2    -0    src/effects/screenshot/screenshot.h
M  +2    -0    src/effects/screenshot/screenshotdbusinterface1.h
A  +477  -0    src/effects/screenshot/screenshotdbusinterface2.cpp     [License: GPL(v2.0+)]
A  +61   -0    src/effects/screenshot/screenshotdbusinterface2.h     [License: GPL(v2.0+)]

https://invent.kde.org/plasma/kwin/commit/0683597099cddc1f54966f1546ae8fada3aa7dd6
Comment 5 Méven Car 2021-03-24 11:38:02 UTC
Git commit be8ee0f7efd17f5f29cbf70ac1374ad05f0481ff by Méven Car, on behalf of Vlad Zahorodnii.
Committed on 24/03/2021 at 10:52.
Pushed by vladz into branch 'master'.

Platforms: Introduce PlatformKWinWayland2

The new interface is more extensible and it fixes the latency issue on
wayland. The old org.kde.kwin.Screenshot interface is deprecated.

M  +1    -1    desktop/org.kde.spectacle.desktop.cmake
M  +1    -0    src/CMakeLists.txt
A  +356  -0    src/Platforms/PlatformKWinWayland2.cpp     [License: LGPL(v2.0+)]
A  +149  -0    src/Platforms/PlatformKWinWayland2.h     [License: LGPL(v2.0+)]
M  +5    -0    src/Platforms/PlatformLoader.cpp

https://invent.kde.org/graphics/spectacle/commit/be8ee0f7efd17f5f29cbf70ac1374ad05f0481ff