From 7c822fadeecf4875f6b06593bf2c053f4df312c8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= <mgraesslin@kde.org>
Date: Tue, 10 May 2016 10:34:09 +0200
Subject: [PATCH] Move the OpenGL unsafe check into the Platform

Summary:
A new virtual method createOpenGLSafePoint is added to Platform.
This is invoked through the Compositor with a PreInit and a PostInit
argument pre and post creating the SceneOpenGL.

The Platform plugin can implement this and use it for detecting whether
creating the OpenGL compositor on this platform crashed in the past.
Thus it's the base for the openGLIsBroken platform check.

The x11 standalone plugin is the first to implement this functionality
using the previous code which was designed for X11.

This also means that a crash of the OpenGL compositor during init on
Wayland won't result in OpenGL being disabled.

Reviewers: #plasma

Subscribers: plasma-devel

Projects: #plasma

Differential Revision: https://phabricator.kde.org/D1582
---
 composite.cpp                                     | 11 +++--------
 platform.cpp                                      |  5 +++++
 platform.h                                        | 14 ++++++++++++++
 plugins/platforms/x11/standalone/x11_platform.cpp | 15 +++++++++++++++
 plugins/platforms/x11/standalone/x11_platform.h   |  1 +
 5 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/composite.cpp b/composite.cpp
index a5b036bb66..b867be5d79 100644
--- a/composite.cpp
+++ b/composite.cpp
@@ -202,20 +202,15 @@ void Compositor::slotCompositingOptionsInitialized()
         qCDebug(KWIN_CORE) << "Initializing OpenGL compositing";
 
         // Some broken drivers crash on glXQuery() so to prevent constant KWin crashes:
-        KSharedConfigPtr unsafeConfigPtr = kwinApp()->config();
-        KConfigGroup unsafeConfig(unsafeConfigPtr, "Compositing");
-        const QString openGLIsUnsafe = QLatin1String("OpenGLIsUnsafe") + (is_multihead ? QString::number(screen_number) : QString());
-        if (unsafeConfig.readEntry(openGLIsUnsafe, false))
+        if (kwinApp()->platform()->openGLCompositingIsBroken())
             qCWarning(KWIN_CORE) << "KWin has detected that your OpenGL library is unsafe to use";
         else {
-            unsafeConfig.writeEntry(openGLIsUnsafe, true);
-            unsafeConfig.sync();
+            kwinApp()->platform()->createOpenGLSafePoint(Platform::OpenGLSafePoint::PreInit);
 
             m_scene = SceneOpenGL::createScene(this);
 
             // TODO: Add 30 second delay to protect against screen freezes as well
-            unsafeConfig.writeEntry(openGLIsUnsafe, false);
-            unsafeConfig.sync();
+            kwinApp()->platform()->createOpenGLSafePoint(Platform::OpenGLSafePoint::PostInit);
 
             if (m_scene && !m_scene->initFailed()) {
                 connect(static_cast<SceneOpenGL*>(m_scene), &SceneOpenGL::resetCompositing, this, &Compositor::restart);
diff --git a/platform.cpp b/platform.cpp
index 736e38446d..83d444f80c 100644
--- a/platform.cpp
+++ b/platform.cpp
@@ -301,4 +301,9 @@ bool Platform::openGLCompositingIsBroken() const
     return false;
 }
 
+void Platform::createOpenGLSafePoint(OpenGLSafePoint safePoint)
+{
+    Q_UNUSED(safePoint)
+}
+
 }
diff --git a/platform.h b/platform.h
index b865afc866..9254d38932 100644
--- a/platform.h
+++ b/platform.h
@@ -122,8 +122,22 @@ public:
      * broke (e.g. triggered a crash in a previous run).
      *
      * Default implementation returns @c false.
+     * @see createOpenGLSafePoint
      **/
     virtual bool openGLCompositingIsBroken() const;
+    enum class OpenGLSafePoint {
+        PreInit,
+        PostInit
+    };
+    /**
+     * This method is invoked before and after creating the OpenGL rendering Scene.
+     * An implementing Platform can use it to detect crashes triggered by the OpenGL implementation.
+     * This can be used for @link{openGLCompositingIsBroken}.
+     *
+     * The default implementation does nothing.
+     * @see openGLCompositingIsBroken.
+     **/
+    virtual void createOpenGLSafePoint(OpenGLSafePoint safePoint);
 
     bool usesSoftwareCursor() const {
         return m_softWareCursor;
diff --git a/plugins/platforms/x11/standalone/x11_platform.cpp b/plugins/platforms/x11/standalone/x11_platform.cpp
index 2e0195fc24..d0cf7ddac2 100644
--- a/plugins/platforms/x11/standalone/x11_platform.cpp
+++ b/plugins/platforms/x11/standalone/x11_platform.cpp
@@ -161,4 +161,19 @@ bool X11StandalonePlatform::hasGlx()
     return Xcb::Extensions::self()->hasGlx();
 }
 
+void X11StandalonePlatform::createOpenGLSafePoint(OpenGLSafePoint safePoint)
+{
+    const QString unsafeKey(QLatin1String("OpenGLIsUnsafe") + (kwinApp()->isX11MultiHead() ? QString::number(kwinApp()->x11ScreenNumber()) : QString()));
+    auto group = KConfigGroup(kwinApp()->config(), "Compositing");
+    switch (safePoint) {
+    case OpenGLSafePoint::PreInit:
+        group.writeEntry(unsafeKey, true);
+        break;
+    case OpenGLSafePoint::PostInit:
+        group.writeEntry(unsafeKey, false);
+        break;
+    }
+    group.sync();
+}
+
 }
diff --git a/plugins/platforms/x11/standalone/x11_platform.h b/plugins/platforms/x11/standalone/x11_platform.h
index dd6c71b6ba..c0caa7b99f 100644
--- a/plugins/platforms/x11/standalone/x11_platform.h
+++ b/plugins/platforms/x11/standalone/x11_platform.h
@@ -45,6 +45,7 @@ public:
     bool compositingPossible() const override;
     QString compositingNotPossibleReason() const override;
     bool openGLCompositingIsBroken() const override;
+    void createOpenGLSafePoint(OpenGLSafePoint safePoint) override;
 
 private:
     /**