macOS: Deliver and handle screen change unconditionally

We can't rely on the previous screen and current screen to accurately
reflect whether or not the window has been moved from one screen to
another, or if the window just stayed on the same screen but the screen
was reconfigured by macOS. The reasons for this are many-fold, but
include factors such as Qt using the screen of the top level window
to resolve the screen of the child windows, and AppKit delivering
screen change events in an order that makes things harder to track.

The result is that we need to always send screen change events, for
all windows, including child windows, and we also need to restart the
display link by re-requesting an update request if needed, so that
child windows that are running animations will continue to animate
on the new screen.

Change-Id: I0b87849c41323e92c08f5115842be067fa8f8490
Reviewed-by: Morten Johan Sørvig <morten.sorvig@qt.io>

1 files changed, 28 insertions, 17 deletions

diff --git a/src/plugins/platforms/cocoa/qcocoawindow.mm b/src/plugins/platforms/cocoa/qcocoawindow.mm
index 50adbad518..f992248275 100644
--- a/src/plugins/platforms/cocoa/qcocoawindow.mm
+++ b/src/plugins/platforms/cocoa/qcocoawindow.mm
@@ -1207,23 +1207,34 @@ void QCocoaWindow::windowDidChangeScreen()
     if (!window())
         return;
 
-    const bool wasRunningDisplayLink = static_cast<QCocoaScreen *>(screen())->isRunningDisplayLink();
-
-    if (QCocoaScreen *newScreen = QCocoaIntegration::instance()->screenForNSScreen(m_view.window.screen)) {
-        if (newScreen == screen()) {
-            // Screen properties have changed. Will be handled by
-            // NSApplicationDidChangeScreenParametersNotification
-            // in QCocoaIntegration::updateScreens().
-            return;
-        }
-
-        qCDebug(lcQpaWindow) << window() << "moved to" << newScreen;
-        QWindowSystemInterface::handleWindowScreenChanged<QWindowSystemInterface::SynchronousDelivery>(window(), newScreen->screen());
-
-        if (hasPendingUpdateRequest() && wasRunningDisplayLink)
-            requestUpdate(); // Restart display-link on new screen
-    } else {
-        qCWarning(lcQpaWindow) << "Failed to get QCocoaScreen for" << m_view.window.screen;
+    // Note: When a window is resized to 0x0 Cocoa will report the window's screen as nil
+    auto *currentScreen = QCocoaIntegration::instance()->screenForNSScreen(m_view.window.screen);
+    auto *previousScreen = static_cast<QCocoaScreen*>(screen());
+
+    Q_ASSERT_X(!m_view.window.screen || currentScreen,
+        "QCocoaWindow", "Failed to get QCocoaScreen for NSScreen");
+
+    // Note: The previous screen may be the same as the current screen, either because
+    // the screen was just reconfigured, which still results in AppKit sending an
+    // NSWindowDidChangeScreenNotification, because the previous screen was removed,
+    // and we ended up calling QWindow::setScreen to move the window, which doesn't
+    // actually move the window to the new screen, or because we've delivered the
+    // screen change to the top level window, which will make all the child windows
+    // of that window report the new screen when requested via QWindow::screen().
+    // We still need to deliver the screen change in all these cases, as the
+    // device-pixel ratio may have changed, and needs to be delivered to all
+    // windows, both top level and child windows.
+
+    qCDebug(lcQpaWindow) << "Screen changed for" << window() << "from" << previousScreen << "to" << currentScreen;
+    QWindowSystemInterface::handleWindowScreenChanged<QWindowSystemInterface::SynchronousDelivery>(
+        window(), currentScreen ? currentScreen->screen() : nullptr);
+
+    if (currentScreen && hasPendingUpdateRequest()) {
+        // Restart display-link on new screen. We need to do this unconditionally,
+        // since we can't rely on the previousScreen reflecting whether or not the
+        // window actually moved from one screen to another, or just stayed on the
+        // same screen.
+        currentScreen->requestUpdate();
     }
 }