From 608579a2853ecfa4ef892aa202767788519254f5 Mon Sep 17 00:00:00 2001 From: Weng Xuetian Date: Wed, 20 Jul 2022 15:57:40 -0700 Subject: Only close popup in the the hierchary Imagine following event sequences: 1. a tooltip is shown. activePopups = {tooltip} 2. user click menu bar to show the menu, QMenu::setVisible is called. now activePopups(tooltip, menu} 3. tooltip visibility changed to false. 4. closePopups() close both tooltip and menu. This is a common pattern under wayland that menu is shown as a invisible state. This patch tries to memorize the surface hierchary used to create the popup role. And only close those popups whose ancesotor is hidden. Change-Id: I78aa0b4e32a5812603e003e756d8bcd202e94af4 Reviewed-by: David Edmundson (cherry picked from commit f8e3257e9b1e22d52e9c221c62b8d9b6dd1151a3) Reviewed-by: Qt Cherry-pick Bot --- src/client/qwaylandwindow.cpp | 33 ++++---- src/client/qwaylandwindow_p.h | 6 ++ .../xdg-shell/qwaylandxdgshell.cpp | 22 ++++-- .../xdg-shell/qwaylandxdgshell_p.h | 5 +- tests/auto/client/xdgshell/tst_xdgshell.cpp | 87 ++++++++++++++++++++++ 5 files changed, 127 insertions(+), 26 deletions(-) diff --git a/src/client/qwaylandwindow.cpp b/src/client/qwaylandwindow.cpp index d669e839..27572185 100644 --- a/src/client/qwaylandwindow.cpp +++ b/src/client/qwaylandwindow.cpp @@ -215,6 +215,7 @@ void QWaylandWindow::endFrame() void QWaylandWindow::reset() { + closeChildPopups(); delete mShellSurface; mShellSurface = nullptr; delete mSubSurfaceWindow; @@ -429,20 +430,6 @@ void QWaylandWindow::sendExposeEvent(const QRect &rect) mLastExposeGeometry = rect; } -static QList> activePopups; - -void QWaylandWindow::closePopups(QWaylandWindow *parent) -{ - while (!activePopups.isEmpty()) { - auto popup = activePopups.takeLast(); - if (popup.isNull()) - continue; - if (popup.data() == parent) - return; - popup->reset(); - } -} - QPlatformScreen *QWaylandWindow::calculateScreenFromSurfaceEvents() const { QReadLocker lock(&mSurfaceLock); @@ -462,8 +449,6 @@ void QWaylandWindow::setVisible(bool visible) lastVisible = visible; if (visible) { - if (window()->type() == Qt::Popup || window()->type() == Qt::ToolTip) - activePopups << this; initWindow(); setGeometry(windowGeometry()); @@ -472,7 +457,6 @@ void QWaylandWindow::setVisible(bool visible) // QWaylandShmBackingStore::beginPaint(). } else { sendExposeEvent(QRect()); - closePopups(this); reset(); } } @@ -1520,6 +1504,21 @@ void QWaylandWindow::setXdgActivationToken(const QString &token) { mShellSurface->setXdgActivationToken(token); } + +void QWaylandWindow::addChildPopup(QWaylandWindow *surface) { + mChildPopups.append(surface); +} + +void QWaylandWindow::removeChildPopup(QWaylandWindow *surface) { + mChildPopups.removeAll(surface); +} + +void QWaylandWindow::closeChildPopups() { + while (!mChildPopups.isEmpty()) { + auto popup = mChildPopups.takeLast(); + popup->reset(); + } +} } QT_END_NAMESPACE diff --git a/src/client/qwaylandwindow_p.h b/src/client/qwaylandwindow_p.h index d2d4d659..ea30d9b8 100644 --- a/src/client/qwaylandwindow_p.h +++ b/src/client/qwaylandwindow_p.h @@ -206,6 +206,10 @@ public: void beginFrame(); void endFrame(); + void addChildPopup(QWaylandWindow* child); + void removeChildPopup(QWaylandWindow* child); + void closeChildPopups(); + public slots: void applyConfigure(); @@ -292,6 +296,8 @@ protected: QMargins mCustomMargins; + QList> mChildPopups; + private: void setGeometry_helper(const QRect &rect); void initWindow(); diff --git a/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp b/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp index e227971b..e3dee445 100644 --- a/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp +++ b/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp @@ -174,12 +174,17 @@ QtWayland::xdg_toplevel::resize_edge QWaylandXdgSurface::Toplevel::convertToResi | ((edges & Qt::RightEdge) ? resize_edge_right : 0)); } -QWaylandXdgSurface::Popup::Popup(QWaylandXdgSurface *xdgSurface, QWaylandXdgSurface *parent, +QWaylandXdgSurface::Popup::Popup(QWaylandXdgSurface *xdgSurface, QWaylandWindow *parent, QtWayland::xdg_positioner *positioner) - : xdg_popup(xdgSurface->get_popup(parent ? parent->object() : nullptr, positioner->object())) - , m_xdgSurface(xdgSurface) + : m_xdgSurface(xdgSurface) + , m_parentXdgSurface(qobject_cast(parent->shellSurface())) , m_parent(parent) { + + init(xdgSurface->get_popup(m_parentXdgSurface ? m_parentXdgSurface->object() : nullptr, positioner->object())); + if (m_parent) { + m_parent->addChildPopup(m_xdgSurface->window()); + } } QWaylandXdgSurface::Popup::~Popup() @@ -187,10 +192,14 @@ QWaylandXdgSurface::Popup::~Popup() if (isInitialized()) destroy(); + if (m_parent) { + m_parent->removeChildPopup(m_xdgSurface->window()); + } + if (m_grabbing) { auto *shell = m_xdgSurface->m_shell; Q_ASSERT(shell->m_topmostGrabbingPopup == this); - shell->m_topmostGrabbingPopup = m_parent ? m_parent->m_popup : nullptr; + shell->m_topmostGrabbingPopup = m_parentXdgSurface ? m_parentXdgSurface->m_popup : nullptr; m_grabbing = false; // Synthesize Qt enter/leave events for popup @@ -396,8 +405,6 @@ void QWaylandXdgSurface::setPopup(QWaylandWindow *parent) { Q_ASSERT(!m_toplevel && !m_popup); - auto parentXdgSurface = qobject_cast(parent->shellSurface()); - auto positioner = new QtWayland::xdg_positioner(m_shell->create_positioner()); // set_popup expects a position relative to the parent QPoint transientPos = m_window->geometry().topLeft(); // this is absolute @@ -414,8 +421,9 @@ void QWaylandXdgSurface::setPopup(QWaylandWindow *parent) | QtWayland::xdg_positioner::constraint_adjustment_slide_y | QtWayland::xdg_positioner::constraint_adjustment_flip_x | QtWayland::xdg_positioner::constraint_adjustment_flip_y); - m_popup = new Popup(this, parentXdgSurface, positioner); + m_popup = new Popup(this, parent, positioner); positioner->destroy(); + delete positioner; } diff --git a/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h b/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h index 30366168..8410253c 100644 --- a/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h +++ b/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h @@ -102,14 +102,15 @@ private: class Popup : public QtWayland::xdg_popup { public: - Popup(QWaylandXdgSurface *xdgSurface, QWaylandXdgSurface *parent, QtWayland::xdg_positioner *positioner); + Popup(QWaylandXdgSurface *xdgSurface, QWaylandWindow *parent, QtWayland::xdg_positioner *positioner); ~Popup() override; void grab(QWaylandInputDevice *seat, uint serial); void xdg_popup_popup_done() override; QWaylandXdgSurface *m_xdgSurface = nullptr; - QWaylandXdgSurface *m_parent = nullptr; + QWaylandXdgSurface *m_parentXdgSurface = nullptr; + QWaylandWindow *m_parent = nullptr; bool m_grabbing = false; }; diff --git a/tests/auto/client/xdgshell/tst_xdgshell.cpp b/tests/auto/client/xdgshell/tst_xdgshell.cpp index 80ac3a23..8d275e1e 100644 --- a/tests/auto/client/xdgshell/tst_xdgshell.cpp +++ b/tests/auto/client/xdgshell/tst_xdgshell.cpp @@ -21,6 +21,7 @@ private slots: void configureStates(); void popup(); void tooltipOnPopup(); + void tooltipAndSiblingPopup(); void switchPopups(); void hidePopupParent(); void pongs(); @@ -321,6 +322,92 @@ void tst_xdgshell::tooltipOnPopup() QCOMPOSITOR_TRY_COMPARE(xdgPopup(0), nullptr); } +void tst_xdgshell::tooltipAndSiblingPopup() +{ + class ToolTip : public QRasterWindow { + public: + explicit ToolTip(QWindow *parent) { + setTransientParent(parent); + setFlags(Qt::ToolTip); + resize(100, 100); + show(); + } + void mousePressEvent(QMouseEvent *event) override { + QRasterWindow::mousePressEvent(event); + m_popup = new QRasterWindow; + m_popup->setTransientParent(transientParent()); + m_popup->setFlags(Qt::Popup); + m_popup->resize(100, 100); + m_popup->show(); + } + + QRasterWindow *m_popup = nullptr; + }; + + class Window : public QRasterWindow { + public: + void mousePressEvent(QMouseEvent *event) override { + QRasterWindow::mousePressEvent(event); + m_tooltip = new ToolTip(this); + } + ToolTip *m_tooltip = nullptr; + }; + + Window window; + window.resize(200, 200); + window.show(); + + QCOMPOSITOR_TRY_VERIFY(xdgToplevel()); + exec([=] { xdgToplevel()->sendCompleteConfigure(); }); + QCOMPOSITOR_TRY_VERIFY(xdgToplevel()->m_xdgSurface->m_committedConfigureSerial); + + exec([=] { + auto *surface = xdgToplevel()->surface(); + auto *p = pointer(); + auto *c = client(); + p->sendEnter(surface, {100, 100}); + p->sendFrame(c); + p->sendButton(client(), BTN_LEFT, Pointer::button_state_pressed); + p->sendButton(client(), BTN_LEFT, Pointer::button_state_released); + p->sendFrame(c); + p->sendLeave(surface); + p->sendFrame(c); + }); + + QCOMPOSITOR_TRY_VERIFY(xdgPopup()); + exec([=] { xdgPopup()->sendCompleteConfigure(QRect(100, 100, 100, 100)); }); + QCOMPOSITOR_TRY_VERIFY(xdgPopup()->m_xdgSurface->m_committedConfigureSerial); + QCOMPOSITOR_TRY_VERIFY(!xdgPopup()->m_grabbed); + + exec([=] { + auto *surface = xdgPopup()->surface(); + auto *p = pointer(); + auto *c = client(); + p->sendEnter(surface, {100, 100}); + p->sendFrame(c); + p->sendButton(client(), BTN_LEFT, Pointer::button_state_pressed); + p->sendButton(client(), BTN_LEFT, Pointer::button_state_released); + p->sendFrame(c); + }); + + QCOMPOSITOR_TRY_VERIFY(xdgPopup(1)); + exec([=] { xdgPopup(1)->sendCompleteConfigure(QRect(100, 100, 100, 100)); }); + QCOMPOSITOR_TRY_VERIFY(xdgPopup(1)->m_xdgSurface->m_committedConfigureSerial); + QCOMPOSITOR_TRY_VERIFY(xdgPopup(1)->m_grabbed); + + // Close the middle tooltip (it should not close the sibling popup) + window.m_tooltip->close(); + + QCOMPOSITOR_TRY_COMPARE(xdgPopup(1), nullptr); + // Verify the remaining xdg surface is a grab popup.. + QCOMPOSITOR_TRY_VERIFY(xdgPopup(0)); + QCOMPOSITOR_TRY_VERIFY(xdgPopup(0)->m_grabbed); + + window.m_tooltip->m_popup->close(); + QCOMPOSITOR_TRY_COMPARE(xdgPopup(1), nullptr); + QCOMPOSITOR_TRY_COMPARE(xdgPopup(0), nullptr); +} + // QTBUG-65680 void tst_xdgshell::switchPopups() { -- cgit v1.2.1