Allow the user to rearm OpenGLIsUnsafe KWIn autocrash protection

Also add a usable "doesn't work why" info and WARN! the user about clicking the rearm button.
Merge "OpenGLIsUnsafe" and "CheckIsSafe" config keys
Move the entire checking into CompositingPrefs

BUG:250865
FIXED-IN:4.7

composite.cpp

@@ -120,8 +120,7 @@ void Workspace::setupCompositing()
         KSharedConfigPtr unsafeConfigPtr(KSharedConfig::openConfig("kwinrc"));
         KConfigGroup unsafeConfig(unsafeConfigPtr, "Compositing");
         if (unsafeConfig.readEntry("OpenGLIsUnsafe", false))
-            kWarning(1212) << "KWin has detected that your OpenGL library is unsafe to use, "
+            kWarning(1212) << "KWin has detected that your OpenGL library is unsafe to use";
-                           "falling back to XRender.";
         else {
             unsafeConfig.writeEntry("OpenGLIsUnsafe", true);
             unsafeConfig.sync();

compositingprefs.cpp

@@ -23,10 +23,12 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include "kwinglobals.h"
 #include "kwinglplatform.h"
 
+#include <kconfiggroup.h>
 #include <kdebug.h>
 #include <kxerrorhandler.h>
 #include <klocale.h>
 #include <kdeversion.h>
+#include <ksharedconfig.h>
 #include <kstandarddirs.h>
 
 #include <qprocess.h>
@@ -52,8 +54,21 @@ bool CompositingPrefs::recommendCompositing() const
     return mRecommendCompositing;
 }
 
+bool CompositingPrefs::openGlIsBroken()
+{
+    KSharedConfigPtr config = KSharedConfig::openConfig("kwinrc");
+    return KConfigGroup(config, "Compositing").readEntry("OpenGLIsUnsafe", false);
+}
+
 bool CompositingPrefs::compositingPossible()
 {
+    // first off, check whether we figured that we'll crash on detection because of a buggy driver
+    KSharedConfigPtr config = KSharedConfig::openConfig("kwinrc");
+    KConfigGroup gl_workaround_group(config, "Compositing");
+    if (gl_workaround_group.readEntry("Backend", "OpenGL") == "OpenGL" &&
+        gl_workaround_group.readEntry("OpenGLIsUnsafe", false))
+        return false;
+
 #ifdef KWIN_HAVE_COMPOSITING
     Extensions::init();
     if (!Extensions::compositeAvailable()) {
@@ -85,6 +100,17 @@ bool CompositingPrefs::compositingPossible()
 QString CompositingPrefs::compositingNotPossibleReason()
 {
 #ifdef KWIN_HAVE_COMPOSITING
+    // first off, check whether we figured that we'll crash on detection because of a buggy driver
+    KSharedConfigPtr config = KSharedConfig::openConfig("kwinrc");
+    KConfigGroup gl_workaround_group(config, "Compositing");
+    if (gl_workaround_group.readEntry("Backend", "OpenGL") == "OpenGL" &&
+        gl_workaround_group.readEntry("OpenGLIsUnsafe", false))
+        return i18n("<b>OpenGL compositing (the default) has crashed KWin in the past.</b><br>"
+                    "This was most likely due to a driver bug."
+                    "<p>If you think that you have meanwhile upgraded to a stable driver,<br>"
+                    "you can reset this protection but <b>be aware that this might result in an immediate crash!</b></p>"
+                    "<p>Alternatively, you might want to use the XRender backend instead.</p>");
+
     Extensions::init();
     if (!Extensions::compositeAvailable() || !Extensions::damageAvailable()) {
         return i18n("Required X extensions (XComposite and XDamage) are not available.");
@@ -111,10 +137,19 @@ QString CompositingPrefs::compositingNotPossibleReason()
 
 void CompositingPrefs::detect()
 {
-    if (!compositingPossible()) {
+    if (!compositingPossible() || openGlIsBroken()) {
         return;
     }
 
+    // NOTICE: this is intended to workaround broken GL implementations that successfully segfault
+    // on glXQuery :-(
+    // we tag GL as unsafe. It *must* be reset before every return, and in case we "unexpectedly"
+    // end (aka "segfaulted") we know that we shall not try again
+    KSharedConfigPtr config = KSharedConfig::openConfig("kwinrc");
+    KConfigGroup gl_workaround_config = KConfigGroup(config, "Compositing");
+    gl_workaround_config.writeEntry("OpenGLIsUnsafe", true);
+    gl_workaround_config.sync();
+
 #ifdef KWIN_HAVE_OPENGL_COMPOSITING
 #ifdef KWIN_HAVE_OPENGLES
     bool haveContext = false;
@@ -153,6 +188,8 @@ void CompositingPrefs::detect()
     }
     if (!Extensions::glxAvailable()) {
         kDebug(1212) << "No GLX available";
+        gl_workaround_config.writeEntry("OpenGLIsUnsafe", false);
+        gl_workaround_config.sync();
         return;
     }
     int glxmajor, glxminor;
@@ -177,6 +214,8 @@ void CompositingPrefs::detect()
         glXMakeCurrent(display(), olddrawable, oldcontext);
     deleteGLXContext();
 #endif
+    gl_workaround_config.writeEntry("OpenGLIsUnsafe", false);
+    gl_workaround_config.sync();
 #endif
 }
 

compositingprefs.h

@@ -39,6 +39,7 @@ public:
 
     static bool compositingPossible();
     static QString compositingNotPossibleReason();
+    static bool openGlIsBroken();
     bool recommendCompositing() const;
     bool enableVSync() const  {
         return mEnableVSync;

kcmkwin/kwincompositing/main.cpp

@@ -82,6 +82,7 @@ KWinCompositingConfig::KWinCompositingConfig(QWidget *parent, const QVariantList
     layout()->setMargin(0);
     ui.tabWidget->setCurrentIndex(0);
     ui.statusTitleWidget->hide();
+    ui.rearmGlSupport->hide();
 
     // For future use
     (void) I18N_NOOP("Use GLSL shaders");
@@ -102,6 +103,7 @@ KWinCompositingConfig::KWinCompositingConfig(QWidget *parent, const QVariantList
 
     connect(ui.tabWidget, SIGNAL(currentChanged(int)), this, SLOT(currentTabChanged(int)));
 
+    connect(ui.rearmGlSupportButton, SIGNAL(clicked()), this, SLOT(rearmGlSupport()));
     connect(ui.useCompositing, SIGNAL(toggled(bool)), this, SLOT(changed()));
     connect(ui.effectWinManagement, SIGNAL(toggled(bool)), this, SLOT(changed()));
     connect(ui.effectAnimations, SIGNAL(toggled(bool)), this, SLOT(changed()));
@@ -143,34 +145,8 @@ KWinCompositingConfig::KWinCompositingConfig(QWidget *parent, const QVariantList
     a->setGlobalShortcut( KShortcut( Qt::ALT + Qt::SHIFT + Qt::Key_F12 ));
     connect(ui.toggleEffectsShortcut, SIGNAL(keySequenceChanged(const QKeySequence&)), this, SLOT(toggleEffectShortcutChanged(const QKeySequence&)));
 
-    // NOTICE: this is intended to workaround broken GL implementations that successfully segfault on glXQuery :-(
+    // Initialize the user interface with the config loaded from kwinrc.
-    KConfigGroup unsafeConfig(mKWinConfig, "Compositing");
+    load();
-    const bool glUnsafe = unsafeConfig.readEntry("OpenGLIsUnsafe", false);
-    if (!glUnsafe && CompositingPrefs::compositingPossible()) {
-        unsafeConfig.writeEntry("OpenGLIsUnsafe", true);
-        unsafeConfig.sync();
-
-        // Driver-specific config detection
-        mDefaultPrefs.detect();
-        initEffectSelector();
-        // Initialize the user interface with the config loaded from kwinrc.
-        load();
-
-        unsafeConfig.writeEntry("OpenGLIsUnsafe", false);
-        unsafeConfig.sync();
-    } else {
-        // TODO: Add a "force recheck" button that removes the "OpenGLInUnsafe" flag
-
-        ui.useCompositing->setEnabled(false);
-        ui.useCompositing->setChecked(false);
-
-        QString text = i18n("Desktop effects are not available on this system due to the following technical issues:");
-        text += "<br>";
-        text += CompositingPrefs::compositingNotPossibleReason();
-        ui.statusTitleWidget->setText(text);
-        ui.statusTitleWidget->setPixmap(KTitleWidget::InfoMessage, KTitleWidget::ImageLeft);
-        ui.statusTitleWidget->show();
-    }
 
     KAboutData *about = new KAboutData(I18N_NOOP("kcmkwincompositing"), 0,
                                        ki18n("KWin Desktop Effects Configuration Module"),
@@ -357,12 +333,27 @@ void KWinCompositingConfig::loadGeneralTab()
         ui.desktopSwitchingCombo->setCurrentIndex(3);
 }
 
+void KWinCompositingConfig::rearmGlSupport()
+{
+    // rearm config
+    KConfigGroup gl_workaround_config = KConfigGroup(mKWinConfig, "Compositing");
+    gl_workaround_config.writeEntry("OpenGLIsUnsafe", false);
+    gl_workaround_config.sync();
+
+    // save last changes
+    save();
+
+    // Initialize the user interface with the config loaded from kwinrc.
+    load();
+}
+
 
 void KWinCompositingConfig::toogleSmoothScaleUi(int compositingType)
 {
     ui.glScaleFilter->setVisible(compositingType == OPENGL_INDEX);
     ui.xrScaleFilter->setVisible(compositingType == XRENDER_INDEX);
     ui.scaleMethodLabel->setBuddy(compositingType == XRENDER_INDEX ? ui.xrScaleFilter : ui.glScaleFilter);
+    ui.glGroup->setEnabled(compositingType == OPENGL_INDEX);
 }
 
 void KWinCompositingConfig::toggleEffectShortcutChanged(const QKeySequence &seq)
@@ -413,9 +404,33 @@ void KWinCompositingConfig::loadAdvancedTab()
     toogleSmoothScaleUi(ui.compositingType->currentIndex());
 }
 
+void KWinCompositingConfig::updateStatusUI(bool compositingIsPossible)
+{
+    if (compositingIsPossible) {
+        ui.compositingOptionsContainer->show();
+        ui.statusTitleWidget->hide();
+        ui.rearmGlSupport->hide();
+
+        // Driver-specific config detection
+        mDefaultPrefs.detect();
+    }
+    else {
+        ui.compositingOptionsContainer->hide();
+        QString text = i18n("Desktop effects are not available on this system due to the following technical issues:");
+        text += "<hr>";
+        text += CompositingPrefs::compositingNotPossibleReason();
+        ui.statusTitleWidget->setText(text);
+        ui.statusTitleWidget->setPixmap(KTitleWidget::InfoMessage, KTitleWidget::ImageLeft);
+        ui.statusTitleWidget->show();
+        ui.rearmGlSupport->setVisible(CompositingPrefs::openGlIsBroken());
+    }
+}
+
 void KWinCompositingConfig::load()
 {
+    initEffectSelector();
     mKWinConfig->reparseConfiguration();
+    updateStatusUI(CompositingPrefs::compositingPossible());
 
     // Copy Plugins group to temp config file
     QMap<QString, QString> entries = mKWinConfig->entryMap("Plugins");
@@ -537,7 +552,7 @@ bool KWinCompositingConfig::saveAdvancedTab()
     KConfigGroup config(mKWinConfig, "Compositing");
 
     if (config.readEntry("Backend", "OpenGL")
-            != ((ui.compositingType->currentIndex() == 0) ? "OpenGL" : "XRender")
+            != ((ui.compositingType->currentIndex() == OPENGL_INDEX) ? "OpenGL" : "XRender")
             || config.readEntry("GLDirect", mDefaultPrefs.enableDirectRendering())
             != ui.glDirect->isChecked()
             || config.readEntry("GLVSync", mDefaultPrefs.enableVSync()) != ui.glVSync->isChecked()
@@ -568,6 +583,20 @@ bool KWinCompositingConfig::saveAdvancedTab()
 
 void KWinCompositingConfig::save()
 {
+    if (ui.compositingType->currentIndex() == OPENGL_INDEX &&
+        CompositingPrefs::openGlIsBroken() && !ui.rearmGlSupport->isVisible())
+    {
+        KConfigGroup config(mKWinConfig, "Compositing");
+        QString oldBackend = config.readEntry("Backend", "OpenGL");
+        config.writeEntry("Backend", "OpenGL");
+        config.sync();
+        updateStatusUI(false);
+        config.writeEntry("Backend", oldBackend);
+        config.sync();
+        ui.tabWidget->setCurrentIndex(0);
+        return;
+    }
+
     // Save current config. We'll use this for restoring in case something goes wrong.
     KConfigGroup config(mKWinConfig, "Compositing");
     mPreviousConfig = config.entryMap();
@@ -659,11 +688,16 @@ void KWinCompositingConfig::configChanged(bool reinitCompositing)
 {
     // Send signal to kwin
     mKWinConfig->sync();
+
     // Send signal to all kwin instances
     QDBusMessage message = QDBusMessage::createSignal("/KWin", "org.kde.KWin",
                            reinitCompositing ? "reinitCompositing" : "reloadConfig");
     QDBusConnection::sessionBus().send(message);
 
+    // maybe it's ok now?
+    if (reinitCompositing && !ui.compositingOptionsContainer->isVisible())
+        load();
+
     // HACK: We can't just do this here, due to the asynchronous nature of signals.
     // We also can't change reinitCompositing into a message (which would allow
     // callWithCallbac() to do this neater) due to multiple kwin instances.

kcmkwin/kwincompositing/main.h

@@ -76,11 +76,13 @@ public slots:
     void initEffectSelector();
 
 private slots:
+    void rearmGlSupport();
     void toogleSmoothScaleUi(int compositingType);
     void toggleEffectShortcutChanged(const QKeySequence &seq);
 
 private:
     bool effectEnabled(const QString& effect, const KConfigGroup& cfg) const;
+    void updateStatusUI(bool compositingIsPossible);
 
     KSharedConfigPtr mKWinConfig;
     Ui::KWinCompositingConfig ui;

kcmkwin/kwincompositing/main.ui

@@ -6,8 +6,8 @@
    <rect>
     <x>0</x>
     <y>0</y>
-    <width>560</width>
+    <width>544</width>
-    <height>472</height>
+    <height>523</height>
    </rect>
   </property>
   <layout class="QVBoxLayout">
@@ -20,9 +20,117 @@
       <attribute name="title">
        <string>General</string>
       </attribute>
-      <layout class="QVBoxLayout" name="verticalLayout_4">
+      <layout class="QVBoxLayout" name="verticalLayout_2">
        <item>
-        <widget class="KTitleWidget" name="statusTitleWidget"/>
+        <layout class="QHBoxLayout" name="horizontalLayout_4">
+         <item>
+          <spacer name="horizontalSpacer_9">
+           <property name="orientation">
+            <enum>Qt::Horizontal</enum>
+           </property>
+           <property name="sizeHint" stdset="0">
+            <size>
+             <width>40</width>
+             <height>20</height>
+            </size>
+           </property>
+          </spacer>
+         </item>
+         <item>
+          <widget class="KTitleWidget" name="statusTitleWidget">
+           <property name="sizePolicy">
+            <sizepolicy hsizetype="Preferred" vsizetype="Preferred">
+             <horstretch>0</horstretch>
+             <verstretch>0</verstretch>
+            </sizepolicy>
+           </property>
+           <property name="minimumSize">
+            <size>
+             <width>0</width>
+             <height>0</height>
+            </size>
+           </property>
+          </widget>
+         </item>
+         <item>
+          <spacer name="horizontalSpacer_8">
+           <property name="orientation">
+            <enum>Qt::Horizontal</enum>
+           </property>
+           <property name="sizeHint" stdset="0">
+            <size>
+             <width>40</width>
+             <height>20</height>
+            </size>
+           </property>
+          </spacer>
+         </item>
+        </layout>
+       </item>
+       <item>
+        <widget class="QWidget" name="rearmGlSupport" native="true">
+         <layout class="QGridLayout" name="gridLayout_3">
+          <item row="0" column="1" colspan="2">
+           <widget class="QLabel" name="label_9">
+            <property name="font">
+             <font>
+              <weight>75</weight>
+              <bold>true</bold>
+             </font>
+            </property>
+            <property name="text">
+             <string>Pressing this button can crash the desktop.</string>
+            </property>
+            <property name="alignment">
+             <set>Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter</set>
+            </property>
+           </widget>
+          </item>
+          <item row="0" column="0" rowspan="3">
+           <spacer name="horizontalSpacer_14">
+            <property name="orientation">
+             <enum>Qt::Horizontal</enum>
+            </property>
+            <property name="sizeHint" stdset="0">
+             <size>
+              <width>125</width>
+              <height>71</height>
+             </size>
+            </property>
+           </spacer>
+          </item>
+          <item row="1" column="1" colspan="2">
+           <widget class="QCheckBox" name="rearmSafetyCheck">
+            <property name="text">
+             <string>I have saved my data.</string>
+            </property>
+           </widget>
+          </item>
+          <item row="0" column="3" rowspan="3">
+           <spacer name="horizontalSpacer_15">
+            <property name="orientation">
+             <enum>Qt::Horizontal</enum>
+            </property>
+            <property name="sizeHint" stdset="0">
+             <size>
+              <width>125</width>
+              <height>71</height>
+             </size>
+            </property>
+           </spacer>
+          </item>
+          <item row="2" column="1" colspan="2">
+           <widget class="QPushButton" name="rearmGlSupportButton">
+            <property name="enabled">
+             <bool>false</bool>
+            </property>
+            <property name="text">
+             <string>Re-enable OpenGL detection</string>
+            </property>
+           </widget>
+          </item>
+         </layout>
+        </widget>
        </item>
        <item>
         <layout class="QHBoxLayout" name="horizontalLayout_2">
@@ -376,6 +484,9 @@
            <verstretch>0</verstretch>
           </sizepolicy>
          </property>
+         <property name="focusPolicy">
+          <enum>Qt::WheelFocus</enum>
+         </property>
         </widget>
        </item>
       </layout>
@@ -384,8 +495,24 @@
       <attribute name="title">
        <string>Advanced</string>
       </attribute>
-      <layout class="QVBoxLayout" name="verticalLayout_2">
+      <layout class="QGridLayout" name="gridLayout_6" rowstretch="1,0,2,0,0,0,10">
-       <item>
+       <item row="0" column="2">
+        <spacer name="verticalSpacer_2">
+         <property name="orientation">
+          <enum>Qt::Vertical</enum>
+         </property>
+         <property name="sizeType">
+          <enum>QSizePolicy::Fixed</enum>
+         </property>
+         <property name="sizeHint" stdset="0">
+          <size>
+           <width>20</width>
+           <height>8</height>
+          </size>
+         </property>
+        </spacer>
+       </item>
+       <item row="1" column="2">
         <layout class="QGridLayout" name="gridLayout_5">
          <item row="0" column="0">
           <widget class="QLabel" name="label_7">
@@ -435,8 +562,27 @@
          </item>
         </layout>
        </item>
-       <item>
+       <item row="1" column="3">
+        <spacer name="horizontalSpacer_13">
+         <property name="orientation">
+          <enum>Qt::Horizontal</enum>
+         </property>
+         <property name="sizeHint" stdset="0">
+          <size>
+           <width>40</width>
+           <height>20</height>
+          </size>
+         </property>
+        </spacer>
+       </item>
+       <item row="4" column="1" colspan="3">
         <widget class="QGroupBox" name="groupBox_3">
+         <property name="sizePolicy">
+          <sizepolicy hsizetype="MinimumExpanding" vsizetype="Preferred">
+           <horstretch>0</horstretch>
+           <verstretch>0</verstretch>
+          </sizepolicy>
+         </property>
          <property name="title">
           <string>General Options</string>
          </property>
@@ -591,8 +737,40 @@ p, li { white-space: pre-wrap; }
          </layout>
         </widget>
        </item>
-       <item>
+       <item row="0" column="4" rowspan="7">
+        <spacer name="horizontalSpacer_11">
+         <property name="orientation">
+          <enum>Qt::Horizontal</enum>
+         </property>
+         <property name="sizeHint" stdset="0">
+          <size>
+           <width>40</width>
+           <height>20</height>
+          </size>
+         </property>
+        </spacer>
+       </item>
+       <item row="0" column="0" rowspan="7">
+        <spacer name="horizontalSpacer_10">
+         <property name="orientation">
+          <enum>Qt::Horizontal</enum>
+         </property>
+         <property name="sizeHint" stdset="0">
+          <size>
+           <width>40</width>
+           <height>20</height>
+          </size>
+         </property>
+        </spacer>
+       </item>
+       <item row="5" column="1" colspan="3">
         <widget class="QGroupBox" name="glGroup">
+         <property name="sizePolicy">
+          <sizepolicy hsizetype="MinimumExpanding" vsizetype="Preferred">
+           <horstretch>0</horstretch>
+           <verstretch>0</verstretch>
+          </sizepolicy>
+         </property>
          <property name="title">
           <string>OpenGL Options</string>
          </property>
@@ -634,7 +812,7 @@ On legacy hardware disabling Shaders can improve the performance.</string>
          </layout>
         </widget>
        </item>
-       <item>
+       <item row="6" column="2">
         <spacer name="verticalSpacer_3">
          <property name="orientation">
           <enum>Qt::Vertical</enum>
@@ -647,6 +825,32 @@ On legacy hardware disabling Shaders can improve the performance.</string>
          </property>
         </spacer>
        </item>
+       <item row="2" column="2">
+        <spacer name="verticalSpacer">
+         <property name="orientation">
+          <enum>Qt::Vertical</enum>
+         </property>
+         <property name="sizeHint" stdset="0">
+          <size>
+           <width>20</width>
+           <height>40</height>
+          </size>
+         </property>
+        </spacer>
+       </item>
+       <item row="1" column="1">
+        <spacer name="horizontalSpacer_12">
+         <property name="orientation">
+          <enum>Qt::Horizontal</enum>
+         </property>
+         <property name="sizeHint" stdset="0">
+          <size>
+           <width>40</width>
+           <height>20</height>
+          </size>
+         </property>
+        </spacer>
+       </item>
       </layout>
      </widget>
     </widget>
@@ -686,5 +890,22 @@ On legacy hardware disabling Shaders can improve the performance.</string>
   <tabstop>animationSpeedCombo</tabstop>
  </tabstops>
  <resources/>
- <connections/>
+ <connections>
+  <connection>
+   <sender>rearmSafetyCheck</sender>
+   <signal>toggled(bool)</signal>
+   <receiver>rearmGlSupportButton</receiver>
+   <slot>setEnabled(bool)</slot>
+   <hints>
+    <hint type="sourcelabel">
+     <x>161</x>
+     <y>64</y>
+    </hint>
+    <hint type="destinationlabel">
+     <x>194</x>
+     <y>119</y>
+    </hint>
+   </hints>
+  </connection>
+ </connections>
 </ui>

kcmkwin/kwinscreenedges/main.cpp

@@ -64,16 +64,7 @@ KWinScreenEdgesConfig::KWinScreenEdgesConfig(QWidget* parent, const QVariantList
     connect(m_ui->quickMaximizeBox, SIGNAL(stateChanged(int)), this, SLOT(groupChanged()));
     connect(m_ui->quickTileBox, SIGNAL(stateChanged(int)), this, SLOT(groupChanged()));
 
-    // NOTICE: this is intended to workaround broken GL implementations that successfully segfault on glXQuery :-(
+    m_defaultPrefs.detect(); // Driver-specific config detection
-    KConfigGroup gl_workaround_config(m_config, "Compositing");
-    const bool checkIsSafe = gl_workaround_config.readEntry("CheckIsSafe", true);
-    if (checkIsSafe && CompositingPrefs::compositingPossible()) {
-        gl_workaround_config.writeEntry("CheckIsSafe", false);
-        gl_workaround_config.sync();
-        m_defaultPrefs.detect(); // Driver-specific config detection
-        gl_workaround_config.writeEntry("CheckIsSafe", true);
-        gl_workaround_config.sync();
-    }
     load();
 }