[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

@@ -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)

plugins/qpa/integration.cpp

@@ -28,11 +28,11 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include "window.h"
 #include "../../virtualkeyboard.h"
 #include "../../main.h"
+#include "../../screens.h"
 #include "../../wayland_server.h"
 
 #include <KWayland/Client/compositor.h>
 #include <KWayland/Client/connection_thread.h>
-#include <KWayland/Client/output.h>
 #include <KWayland/Client/registry.h>
 #include <KWayland/Client/shell.h>
 #include <KWayland/Client/surface.h>
@@ -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,
-    connect(kwinApp(), &Application::screensCreated, this, &Integration::initializeWayland, Qt::QueuedConnection);
+        [this] {
+            connect(screens(), &Screens::changed, this, &Integration::initScreens);
+            initScreens();
+        }
+    );
     QPlatformIntegration::initialize();
-    m_dummyScreen = new Screen(nullptr);
+    auto dummyScreen = new Screen(-1);
-    screenAdded(m_dummyScreen);
+    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<Integration*>(this));
 }
 
-void Integration::initializeWayland()
+void Integration::initScreens()
 {
-    if (m_registry) {
+    QVector<Screen*> newScreens;
-        return;
+    for (int i = 0; i < screens()->count(); i++) {
+        auto screen = new Screen(i);
+        screenAdded(screen);
+        newScreens << screen;
     }
-    using namespace KWayland::Client;
+    while (!m_screens.isEmpty()) {
-    auto setupRegistry = [this] {
+        destroyScreen(m_screens.takeLast());
-        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);
     }
-}
+    m_screens = newScreens;
-
-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();
 }
 
 KWayland::Client::Compositor *Integration::compositor() const

plugins/qpa/integration.h

@@ -66,19 +66,18 @@ public:
     EGLDisplay eglDisplay() const;
 
 private:
-    void initializeWayland();
+    void initScreens();
-    void createWaylandOutput(quint32 name, quint32 version);
     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<QPlatformInputContext> m_inputContext;
+    QVector<Screen*> m_screens;
 };
 
 }

plugins/qpa/screen.cpp

@@ -19,21 +19,19 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 *********************************************************************/
 #include "screen.h"
 #include "platformcursor.h"
+#include "screens.h"
 #include "wayland_server.h"
 
-#include <KWayland/Client/output.h>
-
 namespace KWin
 {
 namespace QPA
 {
 
-Screen::Screen(KWayland::Client::Output *o)
+Screen::Screen(int screen)
     : QPlatformScreen()
-    , m_output(QPointer<KWayland::Client::Output>(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;
 }
 
 }

plugins/qpa/screen.h

@@ -21,15 +21,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #define KWIN_QPA_SCREEN_H
 
 #include <qpa/qplatformscreen.h>
-#include <QPointer>
+#include <QScopedPointer>
-
-namespace KWayland
-{
-namespace Client
-{
-class Output;
-}
-}
 
 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<KWayland::Client::Output> m_output;
+    int m_screen;
     QScopedPointer<PlatformCursor> m_cursor;
 };