From 27d39e7b3a385285431c06dae6eb2f5a0ef3cda8 Mon Sep 17 00:00:00 2001 From: David Edmundson Date: Fri, 9 Nov 2018 15:00:21 +0000 Subject: [PATCH 1/2] Client: Don't require an input device for non-grabbing popups It's valid to create a non-grabbing XDG-popup, such as a tooltip, before an input event. The current guard should only apply for grabbing popups, otherwise tooltips created early are turned into XDG-toplevels resulting in incorrect positioning. Change-Id: I5bb59a68964ef2375c81e8b4e1a73361b9d2cbcc Reviewed-by: Johan Helsing --- .../xdg-shell-v6/qwaylandxdgshellv6.cpp | 30 +++++++++++-------- .../xdg-shell-v6/qwaylandxdgshellv6_p.h | 3 +- .../xdg-shell/qwaylandxdgshell.cpp | 30 +++++++++++-------- .../xdg-shell/qwaylandxdgshell_p.h | 3 +- 4 files changed, 40 insertions(+), 26 deletions(-) diff --git a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp index 3d3af6929dd..ea0315c4e15 100644 --- a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp +++ b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp @@ -199,8 +199,10 @@ QWaylandXdgSurfaceV6::QWaylandXdgSurfaceV6(QWaylandXdgShellV6 *shell, ::zxdg_sur Qt::WindowType type = window->window()->type(); auto *transientParent = window->transientParent(); - if ((type == Qt::Popup || type == Qt::ToolTip) && transientParent && display->lastInputDevice()) { - setPopup(transientParent, display->lastInputDevice(), display->lastInputSerial(), type == Qt::Popup); + if (type == Qt::ToolTip && transientParent) { + setPopup(transientParent); + } else if (type == Qt::Popup && transientParent && display->lastInputDevice()) { + setGrabPopup(transientParent, display->lastInputDevice(), display->lastInputSerial()); } else { setToplevel(); if (transientParent) { @@ -304,19 +306,12 @@ void QWaylandXdgSurfaceV6::setToplevel() m_toplevel = new Toplevel(this); } -void QWaylandXdgSurfaceV6::setPopup(QWaylandWindow *parent, QWaylandInputDevice *device, int serial, bool grab) +void QWaylandXdgSurfaceV6::setPopup(QWaylandWindow *parent) { Q_ASSERT(!m_toplevel && !m_popup); auto parentXdgSurface = static_cast(parent->shellSurface()); - auto *top = m_shell->m_topmostPopup; - if (grab && top && top->m_xdgSurface != parentXdgSurface) { - qCWarning(lcQpaWayland) << "setPopup called for a surface that was not the topmost popup, positions might be off."; - parentXdgSurface = top->m_xdgSurface; - parent = top->m_xdgSurface->m_window; - } - auto positioner = new QtWayland::zxdg_positioner_v6(m_shell->create_positioner()); // set_popup expects a position relative to the parent QPoint transientPos = m_window->geometry().topLeft(); // this is absolute @@ -332,8 +327,19 @@ void QWaylandXdgSurfaceV6::setPopup(QWaylandWindow *parent, QWaylandInputDevice m_popup = new Popup(this, parentXdgSurface, positioner); positioner->destroy(); delete positioner; - if (grab) - m_popup->grab(device, serial); +} + +void QWaylandXdgSurfaceV6::setGrabPopup(QWaylandWindow *parent, QWaylandInputDevice *device, int serial) +{ + auto parentXdgSurface = static_cast(parent->shellSurface()); + auto *top = m_shell->m_topmostPopup; + + if (top && top->m_xdgSurface != parentXdgSurface) { + qCWarning(lcQpaWayland) << "setGrabPopup called for a surface that was not the topmost popup, positions might be off."; + parent = top->m_xdgSurface->m_window; + } + setPopup(parent); + m_popup->grab(device, serial); } void QWaylandXdgSurfaceV6::zxdg_surface_v6_configure(uint32_t serial) diff --git a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6_p.h b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6_p.h index 874dba014a7..659cd75a15c 100644 --- a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6_p.h +++ b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6_p.h @@ -131,7 +131,8 @@ private: }; void setToplevel(); - void setPopup(QWaylandWindow *parent, QWaylandInputDevice *device, int serial, bool grab); + void setPopup(QWaylandWindow *parent); + void setGrabPopup(QWaylandWindow *parent, QWaylandInputDevice *device, int serial); QWaylandXdgShellV6 *m_shell = nullptr; QWaylandWindow *m_window = nullptr; diff --git a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp index c723192c8e0..9787fa5eba4 100644 --- a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp +++ b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp @@ -227,8 +227,10 @@ QWaylandXdgSurface::QWaylandXdgSurface(QWaylandXdgShell *shell, ::xdg_surface *s Qt::WindowType type = window->window()->type(); auto *transientParent = window->transientParent(); - if ((type == Qt::Popup || type == Qt::ToolTip) && transientParent && display->lastInputDevice()) { - setPopup(transientParent, display->lastInputDevice(), display->lastInputSerial(), type == Qt::Popup); + if (type == Qt::ToolTip && transientParent) { + setPopup(transientParent); + } else if (type == Qt::Popup && transientParent && display->lastInputDevice()) { + setGrabPopup(transientParent, display->lastInputDevice(), display->lastInputSerial()); } else { setToplevel(); if (transientParent) { @@ -338,19 +340,12 @@ void QWaylandXdgSurface::setToplevel() m_toplevel = new Toplevel(this); } -void QWaylandXdgSurface::setPopup(QWaylandWindow *parent, QWaylandInputDevice *device, int serial, bool grab) +void QWaylandXdgSurface::setPopup(QWaylandWindow *parent) { Q_ASSERT(!m_toplevel && !m_popup); auto parentXdgSurface = static_cast(parent->shellSurface()); - auto *top = m_shell->m_topmostPopup; - if (grab && top && top->m_xdgSurface != parentXdgSurface) { - qCWarning(lcQpaWayland) << "setPopup called for a surface that was not the topmost popup, positions might be off."; - parentXdgSurface = top->m_xdgSurface; - parent = top->m_xdgSurface->m_window; - } - 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 @@ -366,8 +361,19 @@ void QWaylandXdgSurface::setPopup(QWaylandWindow *parent, QWaylandInputDevice *d m_popup = new Popup(this, parentXdgSurface, positioner); positioner->destroy(); delete positioner; - if (grab) - m_popup->grab(device, serial); +} + +void QWaylandXdgSurface::setGrabPopup(QWaylandWindow *parent, QWaylandInputDevice *device, int serial) +{ + auto parentXdgSurface = static_cast(parent->shellSurface()); + auto *top = m_shell->m_topmostPopup; + + if (top && top->m_xdgSurface != parentXdgSurface) { + qCWarning(lcQpaWayland) << "setGrabPopup called for a surface that was not the topmost popup, positions might be off."; + parent = top->m_xdgSurface->m_window; + } + setPopup(parent); + m_popup->grab(device, serial); } void QWaylandXdgSurface::xdg_surface_configure(uint32_t serial) diff --git a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h index 5e97a34b31c..8804d580739 100644 --- a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h +++ b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h @@ -138,7 +138,8 @@ private: }; void setToplevel(); - void setPopup(QWaylandWindow *parent, QWaylandInputDevice *device, int serial, bool grab); + void setPopup(QWaylandWindow *parent); + void setGrabPopup(QWaylandWindow *parent, QWaylandInputDevice *device, int serial); QWaylandXdgShell *m_shell = nullptr; QWaylandWindow *m_window = nullptr; From 3684b835891746df632bf6aa976074662a7ed290 Mon Sep 17 00:00:00 2001 From: David Edmundson Date: Mon, 12 Nov 2018 10:14:33 +0000 Subject: [PATCH 2/2] Client: Rename m_topmostPopup to clarify its goal This is used when creating new grabbed popups to ensure they are only created on the topmost grabbing popup as per the specification. Non-grabbing popups don't have this restriction. Change-Id: I8e2b8cd0f091688c847f81f2e5d33ce2f0df96a1 Reviewed-by: Johan Helsing --- .../shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp | 8 ++++---- .../shellintegration/xdg-shell-v6/qwaylandxdgshellv6_p.h | 2 +- .../shellintegration/xdg-shell/qwaylandxdgshell.cpp | 8 ++++---- .../shellintegration/xdg-shell/qwaylandxdgshell_p.h | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp index ea0315c4e15..fb33b4ab488 100644 --- a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp +++ b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp @@ -172,14 +172,14 @@ QWaylandXdgSurfaceV6::Popup::~Popup() if (m_grabbing) { auto *shell = m_xdgSurface->m_shell; - Q_ASSERT(shell->m_topmostPopup == this); - shell->m_topmostPopup = m_parent->m_popup; + Q_ASSERT(shell->m_topmostGrabbingPopup == this); + shell->m_topmostGrabbingPopup = m_parent->m_popup; } } void QWaylandXdgSurfaceV6::Popup::grab(QWaylandInputDevice *seat, uint serial) { - m_xdgSurface->m_shell->m_topmostPopup = this; + m_xdgSurface->m_shell->m_topmostGrabbingPopup = this; zxdg_popup_v6::grab(seat->wl_seat(), serial); m_grabbing = true; } @@ -332,7 +332,7 @@ void QWaylandXdgSurfaceV6::setPopup(QWaylandWindow *parent) void QWaylandXdgSurfaceV6::setGrabPopup(QWaylandWindow *parent, QWaylandInputDevice *device, int serial) { auto parentXdgSurface = static_cast(parent->shellSurface()); - auto *top = m_shell->m_topmostPopup; + auto *top = m_shell->m_topmostGrabbingPopup; if (top && top->m_xdgSurface != parentXdgSurface) { qCWarning(lcQpaWayland) << "setGrabPopup called for a surface that was not the topmost popup, positions might be off."; diff --git a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6_p.h b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6_p.h index 659cd75a15c..659aad047b8 100644 --- a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6_p.h +++ b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6_p.h @@ -156,7 +156,7 @@ public: private: void zxdg_shell_v6_ping(uint32_t serial) override; - QWaylandXdgSurfaceV6::Popup *m_topmostPopup = nullptr; + QWaylandXdgSurfaceV6::Popup *m_topmostGrabbingPopup = nullptr; friend class QWaylandXdgSurfaceV6; }; diff --git a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp index 9787fa5eba4..6c98dc6ad1d 100644 --- a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp +++ b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp @@ -200,14 +200,14 @@ QWaylandXdgSurface::Popup::~Popup() if (m_grabbing) { auto *shell = m_xdgSurface->m_shell; - Q_ASSERT(shell->m_topmostPopup == this); - shell->m_topmostPopup = m_parent->m_popup; + Q_ASSERT(shell->m_topmostGrabbingPopup == this); + shell->m_topmostGrabbingPopup = m_parent->m_popup; } } void QWaylandXdgSurface::Popup::grab(QWaylandInputDevice *seat, uint serial) { - m_xdgSurface->m_shell->m_topmostPopup = this; + m_xdgSurface->m_shell->m_topmostGrabbingPopup = this; xdg_popup::grab(seat->wl_seat(), serial); m_grabbing = true; } @@ -366,7 +366,7 @@ void QWaylandXdgSurface::setPopup(QWaylandWindow *parent) void QWaylandXdgSurface::setGrabPopup(QWaylandWindow *parent, QWaylandInputDevice *device, int serial) { auto parentXdgSurface = static_cast(parent->shellSurface()); - auto *top = m_shell->m_topmostPopup; + auto *top = m_shell->m_topmostGrabbingPopup; if (top && top->m_xdgSurface != parentXdgSurface) { qCWarning(lcQpaWayland) << "setGrabPopup called for a surface that was not the topmost popup, positions might be off."; diff --git a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h index 8804d580739..741b83cfd60 100644 --- a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h +++ b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h @@ -170,7 +170,7 @@ private: QWaylandDisplay *m_display = nullptr; QScopedPointer m_xdgDecorationManager; - QWaylandXdgSurface::Popup *m_topmostPopup = nullptr; + QWaylandXdgSurface::Popup *m_topmostGrabbingPopup = nullptr; friend class QWaylandXdgSurface; };