Created attachment 93578 [details]
Patch I tried... Please ignore the if (m_scene->initFailed()) { part. that was another test that didn't work

Comment on attachment 93578 [details]
Patch I tried... Please ignore the if (m_scene->initFailed()) { part. that was another test that didn't work

>diff --git a/composite.cpp b/composite.cpp
>index 8371390..7a84ec6 100644
>--- a/composite.cpp
>+++ b/composite.cpp
>@@ -238,6 +238,10 @@ void Compositor::slotCompositingOptionsInitialized()
>             m_scene = NULL;
>         }
> 
>+        if (kwinApp()->shouldUseWaylandForCompositing()) {
>+            if (m_scene->initFailed()) {
>+            m_scene = SceneQPainter::createScene(this);
>+        }
>         // Do not Fall back to XRender - it causes problems when selfcheck fails during startup, but works later on
>         break;
>     }
>@@ -469,6 +473,17 @@ void Compositor::fallbackToXRenderCompositing()
>     setup();
> }
> 
>+// OpenGL self-check failed, fallback to QPainter
>+void Compositor::fallbackToQPainterCompositing()
>+{
>+    finish();
>+    KConfigGroup config(KSharedConfig::openConfig(), "Compositing");
>+    config.writeEntry("Backend", "QPainter");
>+    config.sync();
>+    options->setCompositingMode(QPainterCompositing);
>+    setup();
>+}
>+
> void Compositor::slotConfigChanged()
> {
>     if (!m_suspended) {
>diff --git a/composite.h b/composite.h
>index fc0ddd5..a961dc0 100644
>--- a/composite.h
>+++ b/composite.h
>@@ -203,6 +203,7 @@ private Q_SLOTS:
>      **/
>     void restart();
>     void fallbackToXRenderCompositing();
>+    void fallbackToQPainterCompositing();
>     void performCompositing();
>     void delayedCheckUnredirect();
>     void slotConfigChanged();
>diff --git a/libkwineffects/kwinglplatform.cpp b/libkwineffects/kwinglplatform.cpp
>index 6dee53b..01131aa 100644
>--- a/libkwineffects/kwinglplatform.cpp
>+++ b/libkwineffects/kwinglplatform.cpp
>@@ -37,7 +37,15 @@ namespace KWin
> {
> 
> GLPlatform *GLPlatform::s_platform = 0;
>-
>+static CompositingType getFallbackCompositor()
>+{
>+    if (QX11Info::isPlatformX11())
>+    {
>+        return XRenderCompositing;
>+    } else {
>+        return QPainterCompositing;
>+    }
>+}
> static qint64 parseVersionString(const QByteArray &version)
> {
>     // Skip any leading non digit
>@@ -502,7 +510,7 @@ QString GLPlatform::chipClassToString(ChipClass chipClass)
> GLPlatform::GLPlatform()
>     : m_driver(Driver_Unknown),
>       m_chipClass(UnknownChipClass),
>-      m_recommendedCompositor(XRenderCompositing),
>+      m_recommendedCompositor(getFallbackCompositor()),
>       m_mesaVersion(0),
>       m_galliumVersion(0),
>       m_looseBinding(false),
>@@ -765,11 +773,11 @@ void GLPlatform::detect(OpenGLPlatformInterface platformInterface)
>         }
> 
>         if (m_chipClass < R300) {
>-            // fallback to XRender for R100 and R200
>-            m_recommendedCompositor = XRenderCompositing;
>+            // fallback for R100 and R200
>+            m_recommendedCompositor = getFallbackCompositor();
>         } else if (m_chipClass < R600) {
>-            // XRender due to NPOT limitations not supported by KWin's shaders
>-            m_recommendedCompositor = XRenderCompositing;
>+            // fallback due to NPOT limitations not supported by KWin's shaders
>+            m_recommendedCompositor = getFallbackCompositor();
>         } else {
>             m_recommendedCompositor = OpenGL2Compositing;
>         }
>@@ -790,7 +798,7 @@ void GLPlatform::detect(OpenGLPlatformInterface platformInterface)
>         }
> 
>         if (m_chipClass < NV40) {
>-            m_recommendedCompositor = XRenderCompositing;
>+            m_recommendedCompositor = getFallbackCompositor();
>         } else {
>             m_recommendedCompositor = OpenGL2Compositing;
>         }
>@@ -808,7 +816,7 @@ void GLPlatform::detect(OpenGLPlatformInterface platformInterface)
>         m_looseBinding = false;
> 
>         if (m_chipClass < I915) {
>-            m_recommendedCompositor = XRenderCompositing;
>+            m_recommendedCompositor = getFallbackCompositor();
>         } else {
>             m_recommendedCompositor = OpenGL2Compositing;
>         }
>@@ -822,8 +830,8 @@ void GLPlatform::detect(OpenGLPlatformInterface platformInterface)
>     }
> 
>     if (isSoftwareEmulation()) {
>-        // we recommend XRender
>-        m_recommendedCompositor = XRenderCompositing;
>+        // we recommend to fallback
>+        m_recommendedCompositor = getFallbackCompositor();
>         if (m_driver < Driver_Llvmpipe) {
>             // Software emulation does not provide GLSL
>             m_limitedGLSL = m_supportsGLSL = false;
>diff --git a/scene_opengl.cpp b/scene_opengl.cpp
>index a35dc59..f801ab3 100644
>--- a/scene_opengl.cpp
>+++ b/scene_opengl.cpp
>@@ -577,11 +577,15 @@ SceneOpenGL *SceneOpenGL::createScene(QObject *parent)
>         }
>     }
>     if (!scene) {
>-        if (GLPlatform::instance()->recommendedCompositor() == XRenderCompositing) {
>-            qCCritical(KWIN_CORE) << "OpenGL driver recommends XRender based compositing. Falling back to XRender.";
>+        if (GLPlatform::instance()->recommendedCompositor() == XRenderCompositing || GLPlatform::instance()->recommendedCompositor() == QPainterCompositing) {
>+            qCCritical(KWIN_CORE) << "OpenGL driver recommends compositing fallback. Falling back";
>             qCCritical(KWIN_CORE) << "To overwrite the detection use the environment variable KWIN_COMPOSE";
>             qCCritical(KWIN_CORE) << "For more information see http://community.kde.org/KWin/Environment_Variables#KWIN_COMPOSE";
>-            QTimer::singleShot(0, Compositor::self(), SLOT(fallbackToXRenderCompositing()));
>+	    if (QX11Info::isPlatformX11()) {
>+                QTimer::singleShot(0, Compositor::self(), SLOT(fallbackToXRenderCompositing()));
>+	    } else {
>+                QTimer::singleShot(0, Compositor::self(), SLOT(fallbackToQPainterCompositing()));
>+	    }
>         }
>         delete backend;
>     }

please clean up the patch and upload it at git.reviewboard.kde.org

2 things:
1. "getFooBar" is javastyle - just name it "failsafeCompositor()" or whatever.
2.  if (GLPlatform::instance()->recommendedCompositor() == XRenderCompositing || GLPlatform::instance()->recommendedCompositor() == QPainterCompositing) {
should probably be if (!(GLPlatform::instance()->recommendedCompositor() & OpenGLCompositing)) or similar.

The problem with the fallback approach is that it moves the re-try with different compositor to the next event cycle. I assume that this is the main problem on why the patch doesn't work.

I had already looked into that problem in the past and decided to delay it for the time being. It needs a restructuring/rethinking of the whole situation and also on how KWin should behave in that case. Especially I'm not sure yet what is the correct strategy for e.g. drm/gl requested, but fails. Should we fallback to drm/qpainter or would it be better to fallback to fbdev/qpainter?

Also I'm not sure whether we want to support things like Qemu anyway or whether we should consider VMs unsupported till there are proper KMS interfaces exposed on them.

This is when I try to call kwin_wayland with --windowed on a fullscreen-shell Weston session.

I tried to comment out the qApp->quit(); as an experiment, and that allowed the fallback to actually work. It's probably not upstreamable, which is why I didn't submit a ReviewBoard...

(In reply to bluescreen_avenger from comment #5)
> I tried to comment out the qApp->quit(); as an experiment, and that allowed
> the fallback to actually work.

That verifies my thought that it's related to the moving to next event cycle.

Could it be upstreamable possibly if I add a check that it only calls qApp->quit(); if the options->compositingMode() is NOT xrender or qpainter?

hmm that also doesn't sound correct. What if a backend only supports OpenGL and it's the only available backend? Then we need to quit. I think it needs to be better combined with the outstanding patch which adds the supportedCompositors to the backend. It then needs to check whether there is another compositor it could try.

It appears that on QXL, this issue is still not resolved, unless I manually specify it to use qpainter with an environment variable...

Automatic compositing type selection in: https://phabricator.kde.org/D8363

Hi

That patch works!

Thanks

Git commit 729bfd04a0c61f306e7e2fbc99d277397049b0d3 by Martin Flöser.
Committed on 19/10/2017 at 16:26.
Pushed by graesslin into branch 'master'.

Try all supported Compositor Types of the current Platform

Summary:
The Compositor now tries to create a Scene not just once but every
type supported by the Platform till it finds one which works. The user's
configuration is only used as a preferred hint and tried first if the
platform supports it.

This brings as an advantage that on platforms such as framebuffer the
user does not need to specify which compositor to use: KWin uses QPainter
automatically.

Also we don't need to do the "translation" from XRender to QPainter any
more. XRender is not supported by any platform using QPainter, so the
user configuration is ignored anyway.
FIXED-IN: 5.12.0

Test Plan: Run on framebuffer, verified debug output.

Reviewers: #kwin, #plasma

Subscribers: plasma-devel, kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D8363

M  +34   -17   composite.cpp
M  +3    -6    options.cpp

https://commits.kde.org/kwin/729bfd04a0c61f306e7e2fbc99d277397049b0d3