From e9b25f176a41108f465b625498ae19ead95f1286 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= <mgraesslin@kde.org>
Date: Fri, 14 Oct 2016 11:34:56 +0200
Subject: [PATCH] [effects] Delay taking screenshot into next rendering cycle

Summary:
For windows the screenshot effect already delayed the rendering into the
next cycle. But for the fullscreen variants which save to a file the
Screenshot effect did try to take the shot directly.

This can be problematic as e.g. the OpenGL context might not be bound at
all. Also this makes it impossible to screenshot multiple screens
together in the Wayland case.

This change delays the DBus replies, schedules the geometry and takes
the screenshot in the next rendering pass. It does not fix the
multiscreen wayland problem yet.

Reviewers: #kwin

Subscribers: kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D3052
---
 effects/screenshot/screenshot.cpp | 59 ++++++++++++++++++++++++++++---
 effects/screenshot/screenshot.h   |  8 ++++-
 2 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/effects/screenshot/screenshot.cpp b/effects/screenshot/screenshot.cpp
index 5a0c52eec3..9de6201fe1 100644
--- a/effects/screenshot/screenshot.cpp
+++ b/effects/screenshot/screenshot.cpp
@@ -41,6 +41,7 @@ bool ScreenShotEffect::supported()
 
 ScreenShotEffect::ScreenShotEffect()
     : m_scheduledScreenshot(0)
+    , m_replyConnection(QDBusConnection::sessionBus())
 {
     connect ( effects, SIGNAL(windowClosed(KWin::EffectWindow*)), SLOT(windowClosed(KWin::EffectWindow*)) );
     QDBusConnection::sessionBus().registerObject(QStringLiteral("/Screenshot"), this, QDBusConnection::ExportScriptableContents);
@@ -177,6 +178,11 @@ void ScreenShotEffect::postPaintScreen()
         }
         m_scheduledScreenshot = NULL;
     }
+
+    if (!m_scheduledGeometry.isNull()) {
+        m_replyConnection.send(m_replyMessage.createReply(blitScreenshot(m_scheduledGeometry)));
+        m_scheduledGeometry = QRect();
+    }
 }
 
 void ScreenShotEffect::screenshotWindowUnderCursor(int mask)
@@ -210,17 +216,62 @@ void ScreenShotEffect::screenshotForWindow(qulonglong winid, int mask)
 
 QString ScreenShotEffect::screenshotFullscreen()
 {
-    return blitScreenshot(effects->virtualScreenGeometry());
+    if (!calledFromDBus()) {
+        return QString();
+    }
+    if (!m_scheduledGeometry.isNull()) {
+        sendErrorReply(QDBusError::Failed, "A screenshot is already been taken");
+        return QString();
+    }
+    m_replyConnection = connection();
+    m_replyMessage = message();
+    setDelayedReply(true);
+    m_scheduledGeometry = effects->virtualScreenGeometry();
+    effects->addRepaintFull();
+    return QString();
 }
 
 QString ScreenShotEffect::screenshotScreen(int screen)
 {
-    return blitScreenshot(effects->clientArea(FullScreenArea, screen, 0));
+    if (!calledFromDBus()) {
+        return QString();
+    }
+    if (!m_scheduledGeometry.isNull()) {
+        sendErrorReply(QDBusError::Failed, "A screenshot is already been taken");
+        return QString();
+    }
+    m_scheduledGeometry = effects->clientArea(FullScreenArea, screen, 0);
+    if (m_scheduledGeometry.isNull()) {
+        sendErrorReply(QDBusError::Failed, "Invalid screen requested");
+        return QString();
+    }
+    m_replyConnection = connection();
+    m_replyMessage = message();
+    setDelayedReply(true);
+    effects->addRepaint(m_scheduledGeometry);
+    return QString();
 }
 
 QString ScreenShotEffect::screenshotArea(int x, int y, int width, int height)
 {
-    return blitScreenshot(QRect(x, y, width, height));
+    if (!calledFromDBus()) {
+        return QString();
+    }
+    if (!m_scheduledGeometry.isNull()) {
+        sendErrorReply(QDBusError::Failed, "A screenshot is already been taken");
+        return QString();
+    }
+    m_scheduledGeometry = QRect(x, y, width, height);
+    if (m_scheduledGeometry.isNull() || m_scheduledGeometry.isEmpty()) {
+        m_scheduledGeometry = QRect();
+        sendErrorReply(QDBusError::Failed, "Invalid area requested");
+        return QString();
+    }
+    m_replyConnection = connection();
+    m_replyMessage = message();
+    setDelayedReply(true);
+    effects->addRepaint(m_scheduledGeometry);
+    return QString();
 }
 
 QString ScreenShotEffect::blitScreenshot(const QRect &geometry)
@@ -321,7 +372,7 @@ void ScreenShotEffect::convertFromGLImage(QImage &img, int w, int h)
 
 bool ScreenShotEffect::isActive() const
 {
-    return m_scheduledScreenshot != NULL && !effects->isScreenLocked();
+    return (m_scheduledScreenshot != NULL || !m_scheduledGeometry.isNull()) && !effects->isScreenLocked();
 }
 
 void ScreenShotEffect::windowClosed( EffectWindow* w )
diff --git a/effects/screenshot/screenshot.h b/effects/screenshot/screenshot.h
index dcb6149e21..8caae54c05 100644
--- a/effects/screenshot/screenshot.h
+++ b/effects/screenshot/screenshot.h
@@ -22,13 +22,16 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #define KWIN_SCREENSHOT_H
 
 #include <kwineffects.h>
+#include <QDBusContext>
+#include <QDBusConnection>
+#include <QDBusMessage>
 #include <QObject>
 #include <QImage>
 
 namespace KWin
 {
 
-class ScreenShotEffect : public Effect
+class ScreenShotEffect : public Effect, protected QDBusContext
 {
     Q_OBJECT
     Q_CLASSINFO("D-Bus Interface", "org.kde.kwin.Screenshot")
@@ -86,6 +89,9 @@ private:
     QString blitScreenshot(const QRect &geometry);
     EffectWindow *m_scheduledScreenshot;
     ScreenShotType m_type;
+    QRect m_scheduledGeometry;
+    QDBusConnection m_replyConnection;
+    QDBusMessage m_replyMessage;
 };
 
 } // namespace