Deal with multi-display scenarios when positioning menu popups

This is to support rare setups involving an X11 server with multiple
independent displays ("zaphod heads" on which the DISPLAY env var
is different), possibly with multiple outputs forming a virtual desktop
on each display.

QMenu::popup() has been assuming that it should show on the screen where
the mouse cursor is.  That's good most of the time; but with multiple
independent screens, QGuiApplication::screenAt(pos) cannot tell us which
screen to use (it's ambiguous), but rather will choose the first screen
that _could_ contain that position (as documented).  In the example in
QTBUG-76162, the QMenu has been constructed with a QDesktopScreenWidget
as its parent specifically for the purpose of telling it which screen to
pop up on; so we need to respect that.  But QWidgetPrivate::init() sets
the QObject::parent() to null (because the widget isn't actually shown
as a child of the QDesktopScreenWidget), and QWidgetPrivate::create_sys()
sets initialScreenIndex back to -1 to provide freedom to change the
screen later; so QMenu has to remember the screen index for itself.

QMenuBarPrivate::popupAction() searches the siblings of the screen on
which the menubar is claiming to be (rather than all screens on all
displays), to find the screen containing the point at the middle of the
bottom edge of the clicked menubar item.  It then sets initialScreenIndex
so that QMenu::popup() will respect it rather than trying to decide for
itself.

Fixes: QTBUG-76162
Change-Id: I7a8f8e7aa2e9cf5340d446dc12726369ebe2589a
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>
 
 
 

5 files changed, 44 insertions, 8 deletions

diff --git a/src/widgets/kernel/qwidget.cpp b/src/widgets/kernel/qwidget.cpp
index 59053f3539..8b225df8be 100644
--- a/src/widgets/kernel/qwidget.cpp
+++ b/src/widgets/kernel/qwidget.cpp
@@ -2589,14 +2589,27 @@ bool QWidgetPrivate::setScreenForPoint(const QPoint &pos)
     Q_Q(QWidget);
     if (!q->isWindow())
         return false;
-    // Find the screen for pos and make the widget undertand it is on that screen.
+    // Find the screen for pos and make the widget understand it is on that screen.
+    return setScreen(QGuiApplication::screenAt(pos));
+}
+
+/*!
+\internal
+Ensures that the widget's QWindow is set to be on the given \a screen.
+Returns true if the screen was changed.
+*/
+
+bool QWidgetPrivate::setScreen(QScreen *screen)
+{
+    Q_Q(QWidget);
+    if (!screen || !q->isWindow())
+        return false;
     const QScreen *currentScreen = windowHandle() ? windowHandle()->screen() : nullptr;
-    QScreen *actualScreen = QGuiApplication::screenAt(pos);
-    if (actualScreen && currentScreen != actualScreen) {
+    if (currentScreen != screen) {
         if (!windowHandle()) // Try to create a window handle if not created.
             createWinId();
         if (windowHandle())
-            windowHandle()->setScreen(actualScreen);
+            windowHandle()->setScreen(screen);
         return true;
     }
     return false;
diff --git a/src/widgets/kernel/qwidget_p.h b/src/widgets/kernel/qwidget_p.h
index ae50624c04..64bc463ae2 100644
--- a/src/widgets/kernel/qwidget_p.h
+++ b/src/widgets/kernel/qwidget_p.h
@@ -181,6 +181,7 @@ struct QTLWExtra {
     QRect frameStrut;
     QRect normalGeometry; // used by showMin/maximized/FullScreen
     Qt::WindowFlags savedFlags; // Save widget flags while showing fullscreen
+    // ### TODO replace initialScreenIndex with QScreen *, in case the screens change at runtime
     int initialScreenIndex; // Screen number when passing a QDesktop[Screen]Widget as parent.
 
     QVector<QPlatformTextureList *> widgetTextures;
@@ -356,6 +357,7 @@ public:
     void createWinId();
 
     bool setScreenForPoint(const QPoint &pos);
+    bool setScreen(QScreen *screen);
 
     void createTLExtra();
     void createExtra();
diff --git a/src/widgets/widgets/qmenu.cpp b/src/widgets/widgets/qmenu.cpp
index 5b1f609b7e..2d107de268 100644
--- a/src/widgets/widgets/qmenu.cpp
+++ b/src/widgets/widgets/qmenu.cpp
@@ -2359,8 +2359,18 @@ void QMenu::popup(const QPoint &p, QAction *atAction)
     d->updateLayoutDirection();
 
     // Ensure that we get correct sizeHints by placing this window on the correct screen.
-    if (d->setScreenForPoint(p))
+    // However if the QMenu was constructed with a QDesktopScreenWidget as its parent,
+    // then initialScreenIndex was set, so we should respect that for the lifetime of this menu.
+    // Use d->popupScreen to remember, because initialScreenIndex will be reset after the first showing.
+    const int screenIndex = d->topData()->initialScreenIndex;
+    if (screenIndex >= 0)
+        d->popupScreen = screenIndex;
+    if (auto s = QGuiApplication::screens().value(d->popupScreen)) {
+        if (d->setScreen(s))
+            d->itemsDirty = true;
+    } else if (d->setScreenForPoint(p)) {
         d->itemsDirty = true;
+    }
 
     const bool contextMenu = d->isContextMenu();
     if (d->lastContextMenu != contextMenu) {
diff --git a/src/widgets/widgets/qmenu_p.h b/src/widgets/widgets/qmenu_p.h
index d0fa8ff113..b8976385f4 100644
--- a/src/widgets/widgets/qmenu_p.h
+++ b/src/widgets/widgets/qmenu_p.h
@@ -509,6 +509,8 @@ public:
     bool tearoffHighlighted : 1;
     //menu fading/scrolling effects
     bool doChildEffects : 1;
+
+    int popupScreen = -1;
 };
 
 QT_END_NAMESPACE
diff --git a/src/widgets/widgets/qmenubar.cpp b/src/widgets/widgets/qmenubar.cpp
index e7984078de..33c10f3a9a 100644
--- a/src/widgets/widgets/qmenubar.cpp
+++ b/src/widgets/widgets/qmenubar.cpp
@@ -68,6 +68,7 @@
 
 #include "qmenu_p.h"
 #include "qmenubar_p.h"
+#include <private/qscreen_p.h>
 #include "qdebug.h"
 
 QT_BEGIN_NAMESPACE
@@ -322,11 +323,18 @@ void QMenuBarPrivate::popupAction(QAction *action, bool activateFirst)
         QRect adjustedActionRect = actionRect(action);
         QPoint pos(q->mapToGlobal(QPoint(adjustedActionRect.left(), adjustedActionRect.bottom() + 1)));
         QSize popup_size = activeMenu->sizeHint();
-
         //we put the popup menu on the screen containing the bottom-center of the action rect
-        QRect screenRect = QDesktopWidgetPrivate::screenGeometry(pos + QPoint(adjustedActionRect.width() / 2, 0));
+        QScreen *popupScreen = q->window()->windowHandle()->screen();
+        QPoint bottomMiddlePos = pos + QPoint(adjustedActionRect.width() / 2, 0);
+        const auto &siblings = popupScreen->virtualSiblings();
+        for (QScreen *sibling : siblings) {
+            if (sibling->geometry().contains(bottomMiddlePos)) {
+                popupScreen = sibling;
+                break;
+            }
+        }
+        QRect screenRect = popupScreen->geometry();
         pos = QPoint(qMax(pos.x(), screenRect.x()), qMax(pos.y(), screenRect.y()));
-
         const bool fitUp = (pos.y() - popup_size.height() >= screenRect.top());
         const bool fitDown = (pos.y() + popup_size.height() <= screenRect.bottom());
         const bool rtl = q->isRightToLeft();
@@ -352,6 +360,7 @@ void QMenuBarPrivate::popupAction(QAction *action, bool activateFirst)
 
         if(!defaultPopDown || (fitUp && !fitDown))
             pos.setY(qMax(screenRect.y(), q->mapToGlobal(QPoint(0, adjustedActionRect.top()-popup_size.height())).y()));
+        QMenuPrivate::get(activeMenu)->topData()->initialScreenIndex = QGuiApplication::screens().indexOf(popupScreen);
         activeMenu->popup(pos);
         if(activateFirst)
             activeMenu->d_func()->setFirstActionActive();