From 9d7ef58b2bd6a5e89e10ff93897f4d6c46e5ff3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Mon, 18 Jul 2016 10:27:56 +0200 Subject: [PATCH] Support restarting the OpenGL compositor on Wayland Summary: KWin needs to support restarting the OpenGL compositor in case of a graphics reset event. On Wayland the tricky part is that the applications should not notice this. Most importantly KWin cannot just destroy the EGLDisplay and create a new one. But this is how a restart works: the complete compositor gets torn down and recreated - including the EGLDisplay. This change moves ownership of the EGLDisplay to the Platform. The AbstractEglBackend subclasses query the Platform whether there is already an EGLDisplay. Only if there is no EGLDisplay the EGLDisplay is created and only if no EGLDisplay is registered with Wayland the bind is performed. Another change is regarding the destruction: the AbstractEglDisplay does no longer unbind the Wayland display and does no longer destroy the EGLDisplay. The EGLDisplay is destroyed by the Platform - so very late on application exit. The Wayland display is unbound when the Compositor terminates. Test Plan: Limited testing with the added auto-test. This one needs to be extended to fully verify that OpenGL applications continue to work. But this requires build.kde.org to support OpenGL on Wayland. Reviewers: #kwin, #plasma_on_wayland Subscribers: plasma-devel, kwin Tags: #plasma_on_wayland, #kwin Differential Revision: https://phabricator.kde.org/D2202 --- abstract_egl_backend.cpp | 31 ++++-- abstract_egl_backend.h | 6 +- autotests/integration/CMakeLists.txt | 1 + autotests/integration/scene_opengl_test.cpp | 103 ++++++++++++++++++ composite.cpp | 2 + platform.cpp | 18 +-- platform.h | 4 +- plugins/platforms/drm/egl_gbm_backend.cpp | 26 +++-- .../hwcomposer/egl_hwcomposer_backend.cpp | 6 +- plugins/platforms/virtual/egl_gbm_backend.cpp | 16 +-- .../platforms/wayland/egl_wayland_backend.cpp | 20 ++-- .../platforms/x11/common/eglonxbackend.cpp | 37 ++++--- 12 files changed, 203 insertions(+), 67 deletions(-) create mode 100644 autotests/integration/scene_opengl_test.cpp diff --git a/abstract_egl_backend.cpp b/abstract_egl_backend.cpp index 82bec21a93..7b4f756684 100644 --- a/abstract_egl_backend.cpp +++ b/abstract_egl_backend.cpp @@ -19,6 +19,7 @@ along with this program. If not, see . *********************************************************************/ #include "abstract_egl_backend.h" #include "options.h" +#include "platform.h" #include "wayland_server.h" #include #include @@ -56,16 +57,20 @@ AbstractEglBackend::AbstractEglBackend() AbstractEglBackend::~AbstractEglBackend() = default; +void AbstractEglBackend::unbindWaylandDisplay() +{ + auto display = kwinApp()->platform()->sceneEglDisplay(); + if (eglUnbindWaylandDisplayWL && display != EGL_NO_DISPLAY) { + eglUnbindWaylandDisplayWL(display, *(WaylandServer::self()->display())); + } +} + void AbstractEglBackend::cleanup() { - if (eglUnbindWaylandDisplayWL && eglDisplay() != EGL_NO_DISPLAY) { - eglUnbindWaylandDisplayWL(eglDisplay(), *(WaylandServer::self()->display())); - } cleanupGL(); doneCurrent(); eglDestroyContext(m_display, m_context); cleanupSurfaces(); - eglTerminate(m_display); eglReleaseThread(); } @@ -137,11 +142,14 @@ void AbstractEglBackend::initWayland() eglBindWaylandDisplayWL = (eglBindWaylandDisplayWL_func)eglGetProcAddress("eglBindWaylandDisplayWL"); eglUnbindWaylandDisplayWL = (eglUnbindWaylandDisplayWL_func)eglGetProcAddress("eglUnbindWaylandDisplayWL"); eglQueryWaylandBufferWL = (eglQueryWaylandBufferWL_func)eglGetProcAddress("eglQueryWaylandBufferWL"); - if (!eglBindWaylandDisplayWL(eglDisplay(), *(WaylandServer::self()->display()))) { - eglUnbindWaylandDisplayWL = nullptr; - eglQueryWaylandBufferWL = nullptr; - } else { - waylandServer()->display()->setEglDisplay(eglDisplay()); + // only bind if not already done + if (waylandServer()->display()->eglDisplay() != eglDisplay()) { + if (!eglBindWaylandDisplayWL(eglDisplay(), *(WaylandServer::self()->display()))) { + eglUnbindWaylandDisplayWL = nullptr; + eglQueryWaylandBufferWL = nullptr; + } else { + waylandServer()->display()->setEglDisplay(eglDisplay()); + } } } } @@ -263,6 +271,11 @@ bool AbstractEglBackend::createContext() return true; } +void AbstractEglBackend::setEglDisplay(const EGLDisplay &display) { + m_display = display; + kwinApp()->platform()->setSceneEglDisplay(display); +} + AbstractEglTexture::AbstractEglTexture(SceneOpenGL::Texture *texture, AbstractEglBackend *backend) : SceneOpenGL::TexturePrivate() , q(texture) diff --git a/abstract_egl_backend.h b/abstract_egl_backend.h index ece35d6e32..c3510f63e5 100644 --- a/abstract_egl_backend.h +++ b/abstract_egl_backend.h @@ -40,6 +40,8 @@ public: return m_context; } + static void unbindWaylandDisplay(); + protected: AbstractEglBackend(); EGLSurface surface() const { @@ -48,9 +50,7 @@ protected: EGLConfig config() const { return m_config; } - void setEglDisplay(const EGLDisplay &display) { - m_display = display; - } + void setEglDisplay(const EGLDisplay &display); void setSurface(const EGLSurface &surface) { m_surface = surface; } diff --git a/autotests/integration/CMakeLists.txt b/autotests/integration/CMakeLists.txt index 6d9faa6b72..7d53e05f25 100644 --- a/autotests/integration/CMakeLists.txt +++ b/autotests/integration/CMakeLists.txt @@ -34,6 +34,7 @@ integrationTest(NAME testMaximized SRCS maximize_test.cpp) integrationTest(NAME testShellClient SRCS shell_client_test.cpp) integrationTest(NAME testDontCrashNoBorder SRCS dont_crash_no_border.cpp) integrationTest(NAME testXClipboardSync SRCS xclipboardsync_test.cpp) +integrationTest(NAME testSceneOpenGL SRCS scene_opengl_test.cpp) integrationTest(NAME testSceneQPainter SRCS scene_qpainter_test.cpp) integrationTest(NAME testNoXdgRuntimeDir SRCS no_xdg_runtime_dir_test.cpp) integrationTest(NAME testScreenChanges SRCS screen_changes_test.cpp) diff --git a/autotests/integration/scene_opengl_test.cpp b/autotests/integration/scene_opengl_test.cpp new file mode 100644 index 0000000000..825b99a696 --- /dev/null +++ b/autotests/integration/scene_opengl_test.cpp @@ -0,0 +1,103 @@ +/******************************************************************** +KWin - the KDE window manager +This file is part of the KDE project. + +Copyright (C) 2016 Martin Gräßlin + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program. If not, see . +*********************************************************************/ +#include "kwin_wayland_test.h" +#include "composite.h" +#include "effectloader.h" +#include "cursor.h" +#include "platform.h" +#include "scene_opengl.h" +#include "shell_client.h" +#include "wayland_server.h" +#include "effect_builtins.h" + +#include + +using namespace KWin; +static const QString s_socketName = QStringLiteral("wayland_test_kwin_scene_opengl-0"); + +class SceneOpenGLTest : public QObject +{ +Q_OBJECT +private Q_SLOTS: + void initTestCase(); + void cleanup(); + void testRestart(); +}; + +void SceneOpenGLTest::cleanup() +{ + Test::destroyWaylandConnection(); +} + +void SceneOpenGLTest::initTestCase() +{ + if (!QFile::exists(QStringLiteral("/dev/dri/card0"))) { + QSKIP("Needs a dri device"); + } + qRegisterMetaType(); + qRegisterMetaType(); + QSignalSpy workspaceCreatedSpy(kwinApp(), &Application::workspaceCreated); + QVERIFY(workspaceCreatedSpy.isValid()); + kwinApp()->platform()->setInitialWindowSize(QSize(1280, 1024)); + QVERIFY(waylandServer()->init(s_socketName.toLocal8Bit())); + + // disable all effects - we don't want to have it interact with the rendering + auto config = KSharedConfig::openConfig(QString(), KConfig::SimpleConfig); + KConfigGroup plugins(config, QStringLiteral("Plugins")); + ScriptedEffectLoader loader; + const auto builtinNames = BuiltInEffects::availableEffectNames() << loader.listOfKnownEffects(); + for (QString name : builtinNames) { + plugins.writeEntry(name + QStringLiteral("Enabled"), false); + } + + config->sync(); + kwinApp()->setConfig(config); + + qputenv("XCURSOR_THEME", QByteArrayLiteral("DMZ-White")); + qputenv("XCURSOR_SIZE", QByteArrayLiteral("24")); + qputenv("KWIN_COMPOSE", QByteArrayLiteral("O2")); + + kwinApp()->start(); + QVERIFY(workspaceCreatedSpy.wait()); + QVERIFY(Compositor::self()); +} + +void SceneOpenGLTest::testRestart() +{ + // simple restart of the OpenGL compositor without any windows being shown + QSignalSpy sceneCreatedSpy(KWin::Compositor::self(), &Compositor::sceneCreated); + QVERIFY(sceneCreatedSpy.isValid()); + KWin::Compositor::self()->slotReinitialize(); + if (sceneCreatedSpy.isEmpty()) { + QVERIFY(sceneCreatedSpy.wait()); + } + QCOMPARE(sceneCreatedSpy.count(), 1); + auto scene = qobject_cast(KWin::Compositor::self()->scene()); + QVERIFY(scene); + + // trigger a repaint + KWin::Compositor::self()->addRepaintFull(); + // and wait 100 msec to ensure it's rendered + // TODO: introduce frameRendered signal in SceneOpenGL + QTest::qWait(100); +} + +WAYLANDTEST_MAIN(SceneOpenGLTest) +#include "scene_opengl_test.moc" diff --git a/composite.cpp b/composite.cpp index 44052eeab4..cc43d0bbb4 100644 --- a/composite.cpp +++ b/composite.cpp @@ -19,6 +19,7 @@ along with this program. If not, see . *********************************************************************/ #include "composite.h" +#include "abstract_egl_backend.h" #include "dbusinterface.h" #include "utils.h" #include @@ -145,6 +146,7 @@ Compositor::Compositor(QObject* workspace) Compositor::~Compositor() { emit aboutToDestroy(); + AbstractEglBackend::unbindWaylandDisplay(); finish(); deleteUnusedSupportProperties(); delete cm_selection; diff --git a/platform.cpp b/platform.cpp index 78eb41c38b..56406adecd 100644 --- a/platform.cpp +++ b/platform.cpp @@ -33,11 +33,15 @@ namespace KWin Platform::Platform(QObject *parent) : QObject(parent) + , m_eglDisplay(EGL_NO_DISPLAY) { } Platform::~Platform() { + if (m_eglDisplay != EGL_NO_DISPLAY) { + eglTerminate(m_eglDisplay); + } } QImage Platform::softwareCursor() const @@ -250,14 +254,14 @@ bool Platform::supportsQpaContext() const return hasGLExtension(QByteArrayLiteral("EGL_KHR_surfaceless_context")); } -EGLDisplay Platform::sceneEglDisplay() const +EGLDisplay KWin::Platform::sceneEglDisplay() const { - if (Compositor *c = Compositor::self()) { - if (SceneOpenGL *s = dynamic_cast(c->scene())) { - return static_cast(s->backend())->eglDisplay(); - } - } - return EGL_NO_DISPLAY; + return m_eglDisplay; +} + +void Platform::setSceneEglDisplay(EGLDisplay display) +{ + m_eglDisplay = display; } EGLContext Platform::sceneEglContext() const diff --git a/platform.h b/platform.h index d6ea45a49b..af8317740d 100644 --- a/platform.h +++ b/platform.h @@ -65,7 +65,8 @@ public: /** * The EGLDisplay used by the compositing scene. **/ - virtual EGLDisplay sceneEglDisplay() const; + EGLDisplay sceneEglDisplay() const; + void setSceneEglDisplay(EGLDisplay display); /** * The EGLContext used by the compositing scene. **/ @@ -231,6 +232,7 @@ private: bool m_pointerWarping = false; bool m_outputsEnabled = true; int m_initialOutputCount = 1; + EGLDisplay m_eglDisplay; }; } diff --git a/plugins/platforms/drm/egl_gbm_backend.cpp b/plugins/platforms/drm/egl_gbm_backend.cpp index 6203acb138..15a64d7320 100644 --- a/plugins/platforms/drm/egl_gbm_backend.cpp +++ b/plugins/platforms/drm/egl_gbm_backend.cpp @@ -90,23 +90,25 @@ void EglGbmBackend::cleanupOutput(const Output &o) bool EglGbmBackend::initializeEgl() { initClientExtensions(); - EGLDisplay display = EGL_NO_DISPLAY; + EGLDisplay display = m_backend->sceneEglDisplay(); // Use eglGetPlatformDisplayEXT() to get the display pointer // if the implementation supports it. - if (!hasClientExtension(QByteArrayLiteral("EGL_EXT_platform_base")) || - !hasClientExtension(QByteArrayLiteral("EGL_MESA_platform_gbm"))) { - setFailed("EGL_EXT_platform_base and/or EGL_MESA_platform_gbm missing"); - return false; - } + if (display == EGL_NO_DISPLAY) { + if (!hasClientExtension(QByteArrayLiteral("EGL_EXT_platform_base")) || + !hasClientExtension(QByteArrayLiteral("EGL_MESA_platform_gbm"))) { + setFailed("EGL_EXT_platform_base and/or EGL_MESA_platform_gbm missing"); + return false; + } - m_device = gbm_create_device(m_backend->fd()); - if (!m_device) { - setFailed("Could not create gbm device"); - return false; - } + m_device = gbm_create_device(m_backend->fd()); + if (!m_device) { + setFailed("Could not create gbm device"); + return false; + } - display = eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_MESA, m_device, nullptr); + display = eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_MESA, m_device, nullptr); + } if (display == EGL_NO_DISPLAY) return false; diff --git a/plugins/platforms/hwcomposer/egl_hwcomposer_backend.cpp b/plugins/platforms/hwcomposer/egl_hwcomposer_backend.cpp index 8ce61871b1..7b649bf8ee 100644 --- a/plugins/platforms/hwcomposer/egl_hwcomposer_backend.cpp +++ b/plugins/platforms/hwcomposer/egl_hwcomposer_backend.cpp @@ -43,9 +43,11 @@ bool EglHwcomposerBackend::initializeEgl() { // cannot use initClientExtensions as that crashes in libhybris qputenv("EGL_PLATFORM", QByteArrayLiteral("hwcomposer")); - EGLDisplay display = EGL_NO_DISPLAY; + EGLDisplay display = m_backend->sceneEglDisplay(); - display = eglGetDisplay(nullptr); + if (display == EGL_NO_DISPLAY) { + display = eglGetDisplay(nullptr); + } if (display == EGL_NO_DISPLAY) { return false; } diff --git a/plugins/platforms/virtual/egl_gbm_backend.cpp b/plugins/platforms/virtual/egl_gbm_backend.cpp index 15ad75ed3a..28179f6fa8 100644 --- a/plugins/platforms/virtual/egl_gbm_backend.cpp +++ b/plugins/platforms/virtual/egl_gbm_backend.cpp @@ -53,17 +53,19 @@ EglGbmBackend::~EglGbmBackend() bool EglGbmBackend::initializeEgl() { initClientExtensions(); - EGLDisplay display = EGL_NO_DISPLAY; + EGLDisplay display = m_backend->sceneEglDisplay(); // Use eglGetPlatformDisplayEXT() to get the display pointer // if the implementation supports it. - if (!hasClientExtension(QByteArrayLiteral("EGL_EXT_platform_base")) || - !hasClientExtension(QByteArrayLiteral("EGL_MESA_platform_gbm"))) { - setFailed("EGL_EXT_platform_base and/or EGL_MESA_platform_gbm missing"); - return false; - } + if (display == EGL_NO_DISPLAY) { + if (!hasClientExtension(QByteArrayLiteral("EGL_EXT_platform_base")) || + !hasClientExtension(QByteArrayLiteral("EGL_MESA_platform_gbm"))) { + setFailed("EGL_EXT_platform_base and/or EGL_MESA_platform_gbm missing"); + return false; + } - display = eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_MESA, EGL_DEFAULT_DISPLAY, nullptr); + display = eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_MESA, EGL_DEFAULT_DISPLAY, nullptr); + } if (display == EGL_NO_DISPLAY) return false; diff --git a/plugins/platforms/wayland/egl_wayland_backend.cpp b/plugins/platforms/wayland/egl_wayland_backend.cpp index 62754292e5..1c3910ff8a 100644 --- a/plugins/platforms/wayland/egl_wayland_backend.cpp +++ b/plugins/platforms/wayland/egl_wayland_backend.cpp @@ -70,19 +70,21 @@ EglWaylandBackend::~EglWaylandBackend() bool EglWaylandBackend::initializeEgl() { initClientExtensions(); - EGLDisplay display = EGL_NO_DISPLAY; + EGLDisplay display = m_wayland->sceneEglDisplay(); // Use eglGetPlatformDisplayEXT() to get the display pointer // if the implementation supports it. - m_havePlatformBase = hasClientExtension(QByteArrayLiteral("EGL_EXT_platform_base")); - if (m_havePlatformBase) { - // Make sure that the wayland platform is supported - if (!hasClientExtension(QByteArrayLiteral("EGL_EXT_platform_wayland"))) - return false; + if (display == EGL_NO_DISPLAY) { + m_havePlatformBase = hasClientExtension(QByteArrayLiteral("EGL_EXT_platform_base")); + if (m_havePlatformBase) { + // Make sure that the wayland platform is supported + if (!hasClientExtension(QByteArrayLiteral("EGL_EXT_platform_wayland"))) + return false; - display = eglGetPlatformDisplayEXT(EGL_PLATFORM_WAYLAND_EXT, m_wayland->display(), nullptr); - } else { - display = eglGetDisplay(m_wayland->display()); + display = eglGetPlatformDisplayEXT(EGL_PLATFORM_WAYLAND_EXT, m_wayland->display(), nullptr); + } else { + display = eglGetDisplay(m_wayland->display()); + } } if (display == EGL_NO_DISPLAY) diff --git a/plugins/platforms/x11/common/eglonxbackend.cpp b/plugins/platforms/x11/common/eglonxbackend.cpp index 79b1f87ea4..a55cc9cdf0 100644 --- a/plugins/platforms/x11/common/eglonxbackend.cpp +++ b/plugins/platforms/x11/common/eglonxbackend.cpp @@ -22,6 +22,7 @@ along with this program. If not, see . #include "main.h" #include "options.h" #include "overlaywindow.h" +#include "platform.h" #include "screens.h" #include "xcbutils.h" // kwin libs @@ -171,27 +172,29 @@ void EglOnXBackend::init() bool EglOnXBackend::initRenderingContext() { initClientExtensions(); - EGLDisplay dpy; + EGLDisplay dpy = kwinApp()->platform()->sceneEglDisplay(); // Use eglGetPlatformDisplayEXT() to get the display pointer // if the implementation supports it. - const bool havePlatformBase = hasClientExtension(QByteArrayLiteral("EGL_EXT_platform_base")); - setHavePlatformBase(havePlatformBase); - if (havePlatformBase) { - // Make sure that the X11 platform is supported - if (!hasClientExtension(QByteArrayLiteral("EGL_EXT_platform_x11"))) { - qCWarning(KWIN_CORE) << "EGL_EXT_platform_base is supported, but EGL_EXT_platform_x11 is not. Cannot create EGLDisplay on X11"; - return false; + if (display == EGL_NO_DISPLAY) { + const bool havePlatformBase = hasClientExtension(QByteArrayLiteral("EGL_EXT_platform_base")); + setHavePlatformBase(havePlatformBase); + if (havePlatformBase) { + // Make sure that the X11 platform is supported + if (!hasClientExtension(QByteArrayLiteral("EGL_EXT_platform_x11"))) { + qCWarning(KWIN_CORE) << "EGL_EXT_platform_base is supported, but EGL_EXT_platform_x11 is not. Cannot create EGLDisplay on X11"; + return false; + } + + const int attribs[] = { + EGL_PLATFORM_X11_SCREEN_EXT, m_x11ScreenNumber, + EGL_NONE + }; + + dpy = eglGetPlatformDisplayEXT(EGL_PLATFORM_X11_EXT, m_x11Display, attribs); + } else { + dpy = eglGetDisplay(m_x11Display); } - - const int attribs[] = { - EGL_PLATFORM_X11_SCREEN_EXT, m_x11ScreenNumber, - EGL_NONE - }; - - dpy = eglGetPlatformDisplayEXT(EGL_PLATFORM_X11_EXT, m_x11Display, attribs); - } else { - dpy = eglGetDisplay(m_x11Display); } if (dpy == EGL_NO_DISPLAY) {