Client: Fix reverse screen order

[ChangeLog][QPA plugin] Fixed a bug where QGuiApplication::screens() and
primaryScreen() would return initial screens in the reverse order they were
added by the compositor. QGuiApplication::primaryScreen() will now return the
first output added by the compositor.

Calling forceRoundTrip in registry_global() meant it would call itself
recursively if there were additional wl_output events in the queue. This in
turn meant the screens got added in the reverse order. Instead we now add the
screen to a list of not yet initialized screens and add it properly when we've
received the required done events (wl_output and possibly zdg_output_v1).

This also has the added benefit of wl_output hot plugging not calling
forceRoundTrip().

Fixes: QTBUG-72828
Change-Id: I35c6959d6c219f65fd19d571a25b5a6cdb3f741b
Reviewed-by: Gatis Paeglis <gatis.paeglis@qt.io>

src/plugins/platforms/wayland/qwaylanddisplay.cpp

@@ -148,6 +148,11 @@ QWaylandDisplay::QWaylandDisplay(QWaylandIntegration *waylandIntegration)
 #endif
 
     forceRoundTrip();
+
+    if (!mWaitingScreens.isEmpty()) {
+        // Give wl_output.done and zxdg_output_v1.done events a chance to arrive
+        forceRoundTrip();
+    }
 }
 
 QWaylandDisplay::~QWaylandDisplay(void)
@@ -162,6 +167,7 @@ QWaylandDisplay::~QWaylandDisplay(void)
         QWindowSystemInterface::handleScreenRemoved(screen);
     }
     mScreens.clear();
+    qDeleteAll(mWaitingScreens);
 
 #if QT_CONFIG(wayland_datadevice)
     delete mDndSelectionHandler.take();
@@ -222,6 +228,14 @@ QWaylandScreen *QWaylandDisplay::screenForOutput(struct wl_output *output) const
     return nullptr;
 }
 
+void QWaylandDisplay::handleScreenInitialized(QWaylandScreen *screen)
+{
+    if (!mWaitingScreens.removeOne(screen))
+        return;
+    mScreens.append(screen);
+    QWindowSystemInterface::handleScreenAdded(screen);
+}
+
 void QWaylandDisplay::waitForScreens()
 {
     flushRequests();
@@ -246,11 +260,7 @@ void QWaylandDisplay::registry_global(uint32_t id, const QString &interface, uin
     struct ::wl_registry *registry = object();
 
     if (interface == QStringLiteral("wl_output")) {
-        QWaylandScreen *screen = new QWaylandScreen(this, version, id);
-        mScreens.append(screen);
-        // We need to get the output events before creating surfaces
-        forceRoundTrip();
-        QWindowSystemInterface::handleScreenAdded(screen);
+        mWaitingScreens << new QWaylandScreen(this, version, id);
     } else if (interface == QStringLiteral("wl_compositor")) {
         mCompositorVersion = qMin((int)version, 3);
         mCompositor.init(registry, id, mCompositorVersion);
@@ -286,7 +296,7 @@ void QWaylandDisplay::registry_global(uint32_t id, const QString &interface, uin
         }
     } else if (interface == QLatin1String("zxdg_output_manager_v1")) {
         mXdgOutputManager.reset(new QtWayland::zxdg_output_manager_v1(registry, id, qMin(2, int(version))));
-        for (auto *screen : qAsConst(mScreens))
+        for (auto *screen : qAsConst(mWaitingScreens))
             screen->initXdgOutput(xdgOutputManager());
         forceRoundTrip();
     }
@@ -303,6 +313,14 @@ void QWaylandDisplay::registry_global_remove(uint32_t id)
         RegistryGlobal &global = mGlobals[i];
         if (global.id == id) {
             if (global.interface == QStringLiteral("wl_output")) {
+                for (auto *screen : mWaitingScreens) {
+                    if (screen->outputId() == id) {
+                        mWaitingScreens.removeOne(screen);
+                        delete screen;
+                        break;
+                    }
+                }
+
                 foreach (QWaylandScreen *screen, mScreens) {
                     if (screen->outputId() == id) {
                         mScreens.removeOne(screen);

src/plugins/platforms/wayland/qwaylanddisplay_p.h

@@ -122,6 +122,7 @@ public:
     QList<QWaylandScreen *> screens() const { return mScreens; }
 
     QWaylandScreen *screenForOutput(struct wl_output *output) const;
+    void handleScreenInitialized(QWaylandScreen *screen);
 
     struct wl_surface *createSurface(void *handle);
     struct ::wl_region *createRegion(const QRegion &qregion);
@@ -216,6 +217,7 @@ private:
     struct wl_display *mDisplay = nullptr;
     QtWayland::wl_compositor mCompositor;
     QScopedPointer<QWaylandShm> mShm;
+    QList<QWaylandScreen *> mWaitingScreens;
     QList<QWaylandScreen *> mScreens;
     QList<QWaylandInputDevice *> mInputDevices;
     QList<Listener> mRegistryListeners;

src/plugins/platforms/wayland/qwaylandscreen.cpp

@@ -40,6 +40,7 @@
 #include "qwaylandscreen_p.h"
 
 #include "qwaylanddisplay_p.h"
+#include "qwaylandintegration_p.h"
 #include "qwaylandcursor_p.h"
 #include "qwaylandwindow_p.h"
 
@@ -60,6 +61,14 @@ QWaylandScreen::QWaylandScreen(QWaylandDisplay *waylandDisplay, int version, uin
 {
     if (auto *xdgOutputManager = waylandDisplay->xdgOutputManager())
         initXdgOutput(xdgOutputManager);
+
+    if (version < WL_OUTPUT_DONE_SINCE_VERSION) {
+        qCWarning(lcQpaWayland) << "wl_output done event not supported by compositor,"
+                                << "QScreen may not work correctly";
+        mWaylandDisplay->forceRoundTrip(); // Give the compositor a chance to send geometry etc.
+        mOutputDone = true; // Fake the done event
+        maybeInitialize();
+    }
 }
 
 QWaylandScreen::~QWaylandScreen()
@@ -68,6 +77,24 @@ QWaylandScreen::~QWaylandScreen()
         zxdg_output_v1::destroy();
 }
 
+void QWaylandScreen::maybeInitialize()
+{
+    Q_ASSERT(!mInitialized);
+
+    if (!mOutputDone)
+        return;
+
+    if (mWaylandDisplay->xdgOutputManager() && !mXdgOutputDone)
+        return;
+
+    mInitialized = true;
+    mWaylandDisplay->handleScreenInitialized(this);
+
+    updateOutputProperties();
+    if (zxdg_output_v1::isInitialized())
+        updateXdgOutputProperties();
+}
+
 void QWaylandScreen::initXdgOutput(QtWayland::zxdg_output_manager_v1 *xdgOutputManager)
 {
     Q_ASSERT(xdgOutputManager);
@@ -232,10 +259,15 @@ void QWaylandScreen::output_scale(int32_t factor)
 
 void QWaylandScreen::output_done()
 {
-    // the done event is sent after all the geometry and the mode events are sent,
-    // and the last mode event to be sent is the active one, so we can trust the
-    // values of mGeometry and mRefreshRate here
+    mOutputDone = true;
+    if (mInitialized)
+        updateOutputProperties();
+    else
+        maybeInitialize();
+}
 
+void QWaylandScreen::updateOutputProperties()
+{
     if (mTransform >= 0) {
         bool isPortrait = mGeometry.height() > mGeometry.width();
         switch (mTransform) {
@@ -262,7 +294,9 @@ void QWaylandScreen::output_done()
         QWindowSystemInterface::handleScreenOrientationChange(screen(), m_orientation);
         mTransform = -1;
     }
+
     QWindowSystemInterface::handleScreenRefreshRateChange(screen(), refreshRate());
+
     if (!zxdg_output_v1::isInitialized())
         QWindowSystemInterface::handleScreenGeometryChange(screen(), geometry(), geometry());
 }
@@ -280,7 +314,11 @@ void QWaylandScreen::zxdg_output_v1_logical_size(int32_t width, int32_t height)
 
 void QWaylandScreen::zxdg_output_v1_done()
 {
-    QWindowSystemInterface::handleScreenGeometryChange(screen(), geometry(), geometry());
+    mXdgOutputDone = true;
+    if (mInitialized)
+        updateXdgOutputProperties();
+    else
+        maybeInitialize();
 }
 
 void QWaylandScreen::zxdg_output_v1_name(const QString &name)
@@ -288,6 +326,12 @@ void QWaylandScreen::zxdg_output_v1_name(const QString &name)
     mOutputName = name;
 }
 
+void QWaylandScreen::updateXdgOutputProperties()
+{
+    Q_ASSERT(zxdg_output_v1::isInitialized());
+    QWindowSystemInterface::handleScreenGeometryChange(screen(), geometry(), geometry());
+}
+
 } // namespace QtWaylandClient
 
 QT_END_NAMESPACE

src/plugins/platforms/wayland/qwaylandscreen_p.h

@@ -71,6 +71,8 @@ public:
     QWaylandScreen(QWaylandDisplay *waylandDisplay, int version, uint32_t id);
     ~QWaylandScreen() override;
 
+    void maybeInitialize();
+
     void initXdgOutput(QtWayland::zxdg_output_manager_v1 *xdgOutputManager);
 
     QWaylandDisplay *display() const;
@@ -116,12 +118,14 @@ private:
                          int32_t transform) override;
     void output_scale(int32_t factor) override;
     void output_done() override;
+    void updateOutputProperties();
 
     // XdgOutput
     void zxdg_output_v1_logical_position(int32_t x, int32_t y) override;
     void zxdg_output_v1_logical_size(int32_t width, int32_t height) override;
     void zxdg_output_v1_done() override;
     void zxdg_output_v1_name(const QString &name) override;
+    void updateXdgOutputProperties();
 
     int m_outputId;
     QWaylandDisplay *mWaylandDisplay = nullptr;
@@ -137,6 +141,9 @@ private:
     QSize mPhysicalSize;
     QString mOutputName;
     Qt::ScreenOrientation m_orientation = Qt::PrimaryOrientation;
+    bool mOutputDone = false;
+    bool mXdgOutputDone = false;
+    bool mInitialized = false;
 
 #if QT_CONFIG(cursor)
     QScopedPointer<QWaylandCursor> mWaylandCursor;

tests/auto/wayland/output/tst_output.cpp

@@ -215,10 +215,7 @@ void tst_output::screenOrder()
     QTRY_COMPARE(QGuiApplication::screens().size(), 3);
     const auto screens = QGuiApplication::screens();
 
-    QEXPECT_FAIL(nullptr, "TODO: fix screen order", Continue);
     QCOMPARE(screens[1]->model(), "Screen 1");
-
-    QEXPECT_FAIL(nullptr, "TODO: fix screen order", Continue);
     QCOMPARE(screens[2]->model(), "Screen 2");
 
     exec([=] {