QDesktopWidget: fix tracking QScreens.

The old code failed to reliably detect new QScreens and connect their signals to
its own slots.  For example, if the QApplication added a new screen at the
beginning of the screen list (which happens if the new screen is primary), the
signal-connecting loop would actually instead add the now second screen to
QDesktopWidget's list *again*, and connect its signals, but not connect any
signal to the new, first screen.

Furthermore, QDesktopWidget would miss geometry changes because QWidget (and
hence QDesktopScreenWidget) automatically shrinks when the screen it is on gets
smaller.

To fix all of this, QDesktopScreenWidget now keeps its own record of the screen
and its geometry, and it always scans over the entire screen list without
relying on any ordering guarantees on behalf of QApplication.

Change-Id: I2ee8361adf643849f43b7dd9a95966920fd13528
Task-number: QTBUG-52101
Reviewed-by: Błażej Szczygieł <spaz16@wp.pl>
Reviewed-by: Marc Mutz <marc.mutz@kdab.com>

2 files changed, 80 insertions, 35 deletions

diff --git a/src/widgets/kernel/qdesktopwidget.cpp b/src/widgets/kernel/qdesktopwidget.cpp
index d1b37bc03a..e87b13c630 100644
--- a/src/widgets/kernel/qdesktopwidget.cpp
+++ b/src/widgets/kernel/qdesktopwidget.cpp
@@ -40,6 +40,21 @@
 
 QT_BEGIN_NAMESPACE
 
+QDesktopScreenWidget::QDesktopScreenWidget(QScreen *screen, const QRect &geometry)
+    : QWidget(Q_NULLPTR, Qt::Desktop), m_screen(screen)
+{
+    setVisible(false);
+    if (QWindow *winHandle = windowHandle())
+        winHandle->setScreen(screen);
+    setScreenGeometry(geometry);
+}
+
+void QDesktopScreenWidget::setScreenGeometry(const QRect &geometry)
+{
+    m_geometry = geometry;
+    setGeometry(geometry);
+}
+
 int QDesktopScreenWidget::screenNumber() const
 {
     const QDesktopWidgetPrivate *desktopWidgetP
@@ -74,54 +89,76 @@ const QRect QDesktopWidget::availableGeometry(const QWidget *widget) const
         return rect;
 }
 
+QDesktopScreenWidget *QDesktopWidgetPrivate::widgetForScreen(QScreen *qScreen) const
+{
+    foreach (QDesktopScreenWidget *widget, screens) {
+        if (widget->screen() == qScreen)
+            return widget;
+    }
+    return Q_NULLPTR;
+}
+
 void QDesktopWidgetPrivate::_q_updateScreens()
 {
     Q_Q(QDesktopWidget);
     const QList<QScreen *> screenList = QGuiApplication::screens();
     const int targetLength = screenList.length();
-    const int oldLength = screens.length();
-
-    // Add or remove screen widgets as necessary
-    while (screens.size() > targetLength)
-        delete screens.takeLast();
-
-    for (int currentLength = screens.size(); currentLength < targetLength; ++currentLength) {
-        QScreen *qScreen = screenList.at(currentLength);
-        QDesktopScreenWidget *screenWidget = new QDesktopScreenWidget;
-        screenWidget->setGeometry(qScreen->geometry());
-        QObject::connect(qScreen, SIGNAL(geometryChanged(QRect)),
-                         q, SLOT(_q_updateScreens()), Qt::QueuedConnection);
-        QObject::connect(qScreen, SIGNAL(availableGeometryChanged(QRect)),
-                         q, SLOT(_q_availableGeometryChanged()), Qt::QueuedConnection);
-        QObject::connect(qScreen, SIGNAL(destroyed()),
-                         q, SLOT(_q_updateScreens()), Qt::QueuedConnection);
-        screens.append(screenWidget);
-    }
+    bool screenCountChanged = false;
 
+    // Re-build our screens list. This is the easiest way to later compute which signals to emit.
+    // Create new screen widgets as necessary. While iterating, keep the old list in place so
+    // that widgetForScreen works.
+    // Furthermore, we note which screens have changed, and compute the overall virtual geometry.
+    QList<QDesktopScreenWidget *> newScreens;
+    QList<int> changedScreens;
     QRegion virtualGeometry;
 
-    // update the geometry of each screen widget, determine virtual geometry,
-    // set the new screen for window handle and emit change signals afterwards.
-    QList<int> changedScreens;
-    for (int i = 0; i < screens.length(); i++) {
-        QDesktopScreenWidget *screenWidget = screens.at(i);
+    for (int i = 0; i < targetLength; ++i) {
         QScreen *qScreen = screenList.at(i);
-        QWindow *winHandle = screenWidget->windowHandle();
-        if (winHandle && winHandle->screen() != qScreen)
-            winHandle->setScreen(qScreen);
         const QRect screenGeometry = qScreen->geometry();
-        if (screenGeometry != screenWidget->geometry()) {
-            screenWidget->setGeometry(screenGeometry);
-            changedScreens.push_back(i);
+        QDesktopScreenWidget *screenWidget = widgetForScreen(qScreen);
+        if (screenWidget) {
+            // an old screen. update geometry and remember the index in the *new* list
+            if (screenGeometry != screenWidget->screenGeometry()) {
+                screenWidget->setScreenGeometry(screenGeometry);
+                changedScreens.push_back(i);
+            }
+        } else {
+            // a new screen, create a widget and connect the signals.
+            screenWidget = new QDesktopScreenWidget(qScreen, screenGeometry);
+            QObject::connect(qScreen, SIGNAL(geometryChanged(QRect)),
+                             q, SLOT(_q_updateScreens()), Qt::QueuedConnection);
+            QObject::connect(qScreen, SIGNAL(availableGeometryChanged(QRect)),
+                             q, SLOT(_q_availableGeometryChanged()), Qt::QueuedConnection);
+            QObject::connect(qScreen, SIGNAL(destroyed()),
+                             q, SLOT(_q_updateScreens()), Qt::QueuedConnection);
+            screenCountChanged = true;
         }
+        // record all the screens and the overall geometry.
+        newScreens.push_back(screenWidget);
         virtualGeometry += screenGeometry;
     }
 
+    // Now we apply the accumulated updates.
+    screens.swap(newScreens); // now [newScreens] is the old screen list
+    Q_ASSERT(screens.size() == targetLength);
     q->setGeometry(virtualGeometry.boundingRect());
 
-    if (oldLength != targetLength)
-        emit q->screenCountChanged(targetLength);
+    // Delete the QDesktopScreenWidget that are not used any more.
+    foreach (QDesktopScreenWidget *screen, newScreens) {
+        if (!screens.contains(screen)) {
+            delete screen;
+            screenCountChanged = true;
+        }
+    }
 
+    // Finally, emit the signals.
+    if (screenCountChanged) {
+        // Notice that we trigger screenCountChanged even if a screen was removed and another one added,
+        // in which case the total number of screens did not change. This is the only way for applications
+        // to notice that a screen was swapped out against another one.
+        emit q->screenCountChanged(targetLength);
+    }
     foreach (int changedScreen, changedScreens)
         emit q->resized(changedScreen);
 }
diff --git a/src/widgets/kernel/qdesktopwidget_p.h b/src/widgets/kernel/qdesktopwidget_p.h
index 1fcad7fa65..d5ae629c64 100644
--- a/src/widgets/kernel/qdesktopwidget_p.h
+++ b/src/widgets/kernel/qdesktopwidget_p.h
@@ -55,12 +55,19 @@ QT_BEGIN_NAMESPACE
 class QDesktopScreenWidget : public QWidget {
     Q_OBJECT
 public:
-    QDesktopScreenWidget() : QWidget(Q_NULLPTR, Qt::Desktop)
-    {
-        setVisible(false);
-    }
+    explicit QDesktopScreenWidget(QScreen *screen, const QRect &geometry);
 
     int screenNumber() const;
+    void setScreenGeometry(const QRect &geometry);
+
+    QScreen *screen() const { return m_screen.data(); }
+    QRect screenGeometry() const { return m_geometry; }
+
+private:
+    // The widget updates its screen and geometry automatically. We need to save them separately
+    // to detect changes, and trigger the appropriate signals.
+    const QPointer<QScreen> m_screen;
+    QRect m_geometry;
 };
 
 class QDesktopWidgetPrivate : public QWidgetPrivate {
@@ -70,6 +77,7 @@ public:
     ~QDesktopWidgetPrivate() { qDeleteAll(screens); }
     void _q_updateScreens();
     void _q_availableGeometryChanged();
+    QDesktopScreenWidget *widgetForScreen(QScreen *qScreen) const;
 
     QList<QDesktopScreenWidget *> screens;
 };