From abedb464d501d2dab080fd87fb45edb142b8e327 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Fl=C3=B6ser?= Date: Tue, 17 Oct 2017 19:13:50 +0200 Subject: [PATCH] [QPA] Implement Screen on top of internal Screens API Summary: The test DontCrashUseractionsMenu (Waylandonly) found an issue in our screen handling implementation in the QPA. The code exposed a short time frame between the dummy screen getting destroyed and the first screen being added. This could result in a crash of KWin. There is actually no need to implement Screen on top of Wayland screen. KWin has all the knowledge, so we can also base this on top of the Screens API. Advantages: * no delays due to Wayland roundtrips * handle screen getting removed (was a TODO) * handle resolution changes (was a TODO) The new implementation has a disadvantage that it destroys and readds all screens whenever something around the screen changes. This shouldn't be an issue in practice as it's only for the internal QPA and thus only affects KWin internal windows which is placed in global coordinates anyway. If it turns out to be a problem we need to track better the screen changes - so far those were not tracked at all. Test Plan: Run a few unit tests which change screens Reviewers: #kwin, #plasma Subscribers: plasma-devel, kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D8345 --- autotests/integration/CMakeLists.txt | 2 +- plugins/qpa/integration.cpp | 58 +++++++++------------------- plugins/qpa/integration.h | 5 +-- plugins/qpa/screen.cpp | 14 +++---- plugins/qpa/screen.h | 14 ++----- 5 files changed, 31 insertions(+), 62 deletions(-) diff --git a/autotests/integration/CMakeLists.txt b/autotests/integration/CMakeLists.txt index 7b12216850..3d36b06c70 100644 --- a/autotests/integration/CMakeLists.txt +++ b/autotests/integration/CMakeLists.txt @@ -51,7 +51,7 @@ integrationTest(WAYLAND_ONLY NAME testPointerConstraints SRCS pointer_constraint integrationTest(WAYLAND_ONLY NAME testKeyboardLayout SRCS keyboard_layout_test.cpp) integrationTest(WAYLAND_ONLY NAME testKeymapCreationFailure SRCS keymap_creation_failure_test.cpp) integrationTest(WAYLAND_ONLY NAME testShowingDesktop SRCS showing_desktop_test.cpp) -integrationTest(NAME testDontCrashUseractionsMenu SRCS dont_crash_useractions_menu.cpp) +integrationTest(WAYLAND_ONLY NAME testDontCrashUseractionsMenu SRCS dont_crash_useractions_menu.cpp) integrationTest(WAYLAND_ONLY NAME testKWinBindings SRCS kwinbindings_test.cpp) integrationTest(WAYLAND_ONLY NAME testVirtualDesktop SRCS virtual_desktop_test.cpp) integrationTest(WAYLAND_ONLY NAME testShellClientRules SRCS shell_client_rules_test.cpp) diff --git a/plugins/qpa/integration.cpp b/plugins/qpa/integration.cpp index 6f7d790060..dfde2d1dda 100644 --- a/plugins/qpa/integration.cpp +++ b/plugins/qpa/integration.cpp @@ -28,11 +28,11 @@ along with this program. If not, see . #include "window.h" #include "../../virtualkeyboard.h" #include "../../main.h" +#include "../../screens.h" #include "../../wayland_server.h" #include #include -#include #include #include #include @@ -97,11 +97,16 @@ bool Integration::hasCapability(Capability cap) const void Integration::initialize() { - // TODO: start initialize Wayland once the internal Wayland connection is created - connect(kwinApp(), &Application::screensCreated, this, &Integration::initializeWayland, Qt::QueuedConnection); + connect(kwinApp(), &Application::screensCreated, this, + [this] { + connect(screens(), &Screens::changed, this, &Integration::initScreens); + initScreens(); + } + ); QPlatformIntegration::initialize(); - m_dummyScreen = new Screen(nullptr); - screenAdded(m_dummyScreen); + auto dummyScreen = new Screen(-1); + screenAdded(dummyScreen); + m_screens << dummyScreen; m_inputContext.reset(QPlatformInputContextFactory::create(QStringLiteral("qtvirtualkeyboard"))); qunsetenv("QT_IM_MODULE"); if (!m_inputContext.isNull()) { @@ -203,43 +208,18 @@ QPlatformOpenGLContext *Integration::createPlatformOpenGLContext(QOpenGLContext return new PlatformContextWayland(context, const_cast(this)); } -void Integration::initializeWayland() +void Integration::initScreens() { - if (m_registry) { - return; + QVector newScreens; + for (int i = 0; i < screens()->count(); i++) { + auto screen = new Screen(i); + screenAdded(screen); + newScreens << screen; } - using namespace KWayland::Client; - auto setupRegistry = [this] { - m_registry = waylandServer()->internalClientRegistry(); - connect(m_registry, &Registry::outputAnnounced, this, &Integration::createWaylandOutput); - const auto outputs = m_registry->interfaces(Registry::Interface::Output); - for (const auto &o : outputs) { - createWaylandOutput(o.name, o.version); - } - }; - if (waylandServer()->internalClientRegistry()) { - setupRegistry(); - } else { - connect(waylandServer()->internalClientConection(), &ConnectionThread::connected, this, setupRegistry, Qt::QueuedConnection); + while (!m_screens.isEmpty()) { + destroyScreen(m_screens.takeLast()); } -} - -void Integration::createWaylandOutput(quint32 name, quint32 version) -{ - if (m_dummyScreen) { - destroyScreen(m_dummyScreen); - m_dummyScreen = nullptr; - } - using namespace KWayland::Client; - auto o = m_registry->createOutput(name, version, m_registry); - connect(o, &Output::changed, this, - [this, o] { - disconnect(o, &Output::changed, nullptr, nullptr); - // TODO: handle screen removal - screenAdded(new Screen(o)); - } - ); - waylandServer()->internalClientConection()->flush(); + m_screens = newScreens; } KWayland::Client::Compositor *Integration::compositor() const diff --git a/plugins/qpa/integration.h b/plugins/qpa/integration.h index 3364bbdafa..613e5aff3c 100644 --- a/plugins/qpa/integration.h +++ b/plugins/qpa/integration.h @@ -66,19 +66,18 @@ public: EGLDisplay eglDisplay() const; private: - void initializeWayland(); - void createWaylandOutput(quint32 name, quint32 version); + void initScreens(); void initEgl(); KWayland::Client::Shell *shell() const; QPlatformFontDatabase *m_fontDb; QPlatformNativeInterface *m_nativeInterface; - KWayland::Client::Registry *m_registry = nullptr; KWayland::Client::Compositor *m_compositor = nullptr; KWayland::Client::Shell *m_shell = nullptr; EGLDisplay m_eglDisplay = EGL_NO_DISPLAY; Screen *m_dummyScreen = nullptr; QScopedPointer m_inputContext; + QVector m_screens; }; } diff --git a/plugins/qpa/screen.cpp b/plugins/qpa/screen.cpp index e8cd85e4f5..5eae7d958a 100644 --- a/plugins/qpa/screen.cpp +++ b/plugins/qpa/screen.cpp @@ -19,21 +19,19 @@ along with this program. If not, see . *********************************************************************/ #include "screen.h" #include "platformcursor.h" +#include "screens.h" #include "wayland_server.h" -#include - namespace KWin { namespace QPA { -Screen::Screen(KWayland::Client::Output *o) +Screen::Screen(int screen) : QPlatformScreen() - , m_output(QPointer(o)) + , m_screen(screen) , m_cursor(new PlatformCursor) { - // TODO: connect to resolution changes } Screen::~Screen() = default; @@ -50,12 +48,12 @@ QImage::Format Screen::format() const QRect Screen::geometry() const { - return m_output ? QRect(m_output->globalPosition(), m_output->pixelSize() / m_output->scale()) : QRect(0, 0, 1, 1); + return m_screen != -1 ? screens()->geometry(m_screen) : QRect(0, 0, 1, 1); } QSizeF Screen::physicalSize() const { - return m_output ? m_output->physicalSize() : QPlatformScreen::physicalSize(); + return m_screen != -1 ? screens()->physicalSize(m_screen) : QPlatformScreen::physicalSize(); } QPlatformCursor *Screen::cursor() const @@ -75,7 +73,7 @@ QDpi Screen::logicalDpi() const qreal Screen::devicePixelRatio() const { - return m_output ? (qreal)m_output->scale() : 1.0; + return m_screen != -1 ? screens()->scale(m_screen) : 1.0; } } diff --git a/plugins/qpa/screen.h b/plugins/qpa/screen.h index 7690e1f6bc..2753a4e898 100644 --- a/plugins/qpa/screen.h +++ b/plugins/qpa/screen.h @@ -21,15 +21,7 @@ along with this program. If not, see . #define KWIN_QPA_SCREEN_H #include -#include - -namespace KWayland -{ -namespace Client -{ -class Output; -} -} +#include namespace KWin { @@ -40,7 +32,7 @@ class PlatformCursor; class Screen : public QPlatformScreen { public: - explicit Screen(KWayland::Client::Output *o); + explicit Screen(int screen); virtual ~Screen(); QRect geometry() const override; @@ -52,7 +44,7 @@ public: qreal devicePixelRatio() const override; private: - QPointer m_output; + int m_screen; QScopedPointer m_cursor; };