Skip to content

Commit

Permalink
Deal with multi-display scenarios when positioning menu popups
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
ec1oud committed Jun 21, 2019
1 parent 121692e commit 82da830
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 8 deletions.
21 changes: 17 additions & 4 deletions src/widgets/kernel/qwidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions src/widgets/kernel/qwidget_p.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -356,6 +357,7 @@ class Q_WIDGETS_EXPORT QWidgetPrivate : public QObjectPrivate
void createWinId();

bool setScreenForPoint(const QPoint &pos);
bool setScreen(QScreen *screen);

void createTLExtra();
void createExtra();
Expand Down
12 changes: 11 additions & 1 deletion src/widgets/widgets/qmenu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 2 additions & 0 deletions src/widgets/widgets/qmenu_p.h
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,8 @@ class QMenuPrivate : public QWidgetPrivate
bool tearoffHighlighted : 1;
//menu fading/scrolling effects
bool doChildEffects : 1;

int popupScreen = -1;
};

QT_END_NAMESPACE
Expand Down
15 changes: 12 additions & 3 deletions src/widgets/widgets/qmenubar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@

#include "qmenu_p.h"
#include "qmenubar_p.h"
#include <private/qscreen_p.h>
#include "qdebug.h"

QT_BEGIN_NAMESPACE
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down

0 comments on commit 82da830

Please sign in to comment.