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>

1 files changed, 68 insertions, 31 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);
 }