Bug 348527

Summary: kwin_wayland should ignore OpenGLIsUnsafe in kwinrc
Product: [Plasma] kwin Reporter: bluescreenavenger
Component: wayland-genericAssignee: KWin default assignee <kwin-bugs-null>
Status: RESOLVED FIXED    
Severity: normal Flags: mgraesslin: ReviewRequest+
Priority: NOR    
Version First Reported In: git master   
Target Milestone: ---   
Platform: Other   
OS: Linux   
URL: https://git.reviewboard.kde.org/r/123986/
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:

Description bluescreenavenger 2015-05-31 23:35:51 UTC
It seems that when Kwin sets this variable after a crash, it prevents kwin_wayland from working, and causes it to hang... ...even with KWIN_COMPOSE=Q

Reproducible: Always
Comment 1 Thomas Lübking 2015-05-31 23:54:02 UTC
I assume we should
a) have GlxIsUnsafe, GlesOnXIsUnsafe & GlesIsUnsafe
b) do not "break" if kwinApp()->requiresCompositing but resort to (in this case) QPainter
c) have different backend keys for X11 and wayland (so we won't try wayland on xrender)

but I don't really see how OpenGLIsUnsafe could have an impact with KWIN_COMPOSE=Q since OpenGLIsUnsafe isn't (supposed to be) touched in that case anyway?
Comment 2 Martin Flöser 2015-06-01 00:26:59 UTC
> I assume we should
> a) have GlxIsUnsafe, GlesOnXIsUnsafe & GlesIsUnsafe
> b) do not "break" if kwinApp()->requiresCompositing but resort to (in
> this
> case) QPainter
> c) have different backend keys for X11 and wayland (so we won't try
> wayland on
> xrender)

what I wanted to add is allowing the backends to better announce which
compositors are supported. E.g. on fbdev backend it's pointless to try 
to use
the OpenGL compositor and the default currently falls back to XRender 
which is
also not supported. So overall it's still rather meh.
Comment 3 bluescreenavenger 2015-06-01 01:42:06 UTC
(In reply to Thomas Lübking from comment #1)
> I assume we should
> a) have GlxIsUnsafe, GlesOnXIsUnsafe & GlesIsUnsafe
> b) do not "break" if kwinApp()->requiresCompositing but resort to (in this
> case) QPainter
> c) have different backend keys for X11 and wayland (so we won't try wayland
> on xrender)
> 
> but I don't really see how OpenGLIsUnsafe could have an impact with
> KWIN_COMPOSE=Q since OpenGLIsUnsafe isn't (supposed to be) touched in that
> case anyway?

I'm not sure... It seems that when OpenGLIsUnsafe is set to true, that even the QPainter backend stops working correctly...
Comment 4 Martin Flöser 2015-06-02 18:18:31 UTC
diff --git a/compositingprefs.cpp b/compositingprefs.cpp
index 103123f..e3a0121 100644
--- a/compositingprefs.cpp
+++ b/compositingprefs.cpp
@@ -56,6 +56,10 @@ bool CompositingPrefs::openGlIsBroken()
 
 bool CompositingPrefs::compositingPossible()
 {
+    if (kwinApp()->shouldUseWaylandForCompositing()) {
+        // don't do X specific checks if we are not going to use X for compositing
+        return true;
+    }
     // first off, check whether we figured that we'll crash on detection because of a buggy driver
     KConfigGroup gl_workaround_group(KSharedConfig::openConfig(), "Compositing");
     const QString unsafeKey(QStringLiteral("OpenGLIsUnsafe") + (is_multihead ? QString::number(screen_number) : QString()));
@@ -63,11 +67,6 @@ bool CompositingPrefs::compositingPossible()
         gl_workaround_group.readEntry(unsafeKey, false))
         return false;
 
-    if (kwinApp()->shouldUseWaylandForCompositing()) {
-        // don't do X specific checks if we are not going to use X for compositing
-        return true;
-    }
-
     if (!Xcb::Extensions::self()->isCompositeAvailable()) {
         qCDebug(KWIN_CORE) << "No composite extension available";
         return false;
Comment 5 Martin Flöser 2016-08-29 11:18:20 UTC
This is now properly implemented:

commit 7c822fadeecf4875f6b06593bf2c053f4df312c8
Author: Martin Gräßlin <mgraesslin@kde.org>
Date:   Tue May 10 10:34:09 2016 +0200

    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