From cc54267e93851f067aba51cae015ca8fc3147c11 Mon Sep 17 00:00:00 2001 From: Méven Car Date: Wed, 18 Aug 2021 18:28:20 +0200 Subject: [PATCH] Wayland client: use wl_keyboard to determine active state Commit f497a5bb87270174b8e0106b7eca1992d44ff15d made QWaylandDisplay use the xdgshell's active state for QWindow::isActive(), instead of using wl_keyboard activate/deactivate events. That seems to have been a misunderstanding, since xdgshell activation is only supposed to be used to determine visual appearance, and there is an explicit warning not to assume it means focus. This commit reverts this logic back to listening to wl_keyboard. It adds a fallback when there is no wl_keyboard available to handle activated/deactivated events through xdg-shell, in order to fix QTBUG-53702. windowStates is handled so that we're not using the Xdg hint for anything with QWindowSystemInterface::handleWindowStateChanged or anything where we need to track only having one active. We are still exposing it for decorations, which is the only reason to use the Xdghint over keyboard focus - so you can keep the toplevel active whilst you show a popup. Change-Id: I4343d2ed9fb5b066cde95628ed0b4ccc84a424db --- diff --git a/src/client/qwaylanddisplay.cpp b/src/client/qwaylanddisplay.cpp index e0dfe8b..2730311 100644 --- a/src/client/qwaylanddisplay.cpp +++ b/src/client/qwaylanddisplay.cpp @@ -575,14 +575,10 @@ void QWaylandDisplay::handleKeyboardFocusChanged(QWaylandInputDevice *inputDevic if (mLastKeyboardFocus == keyboardFocus) return; - if (mWaylandIntegration->mShellIntegration) { - mWaylandIntegration->mShellIntegration->handleKeyboardFocusChanged(keyboardFocus, mLastKeyboardFocus); - } else { - if (keyboardFocus) - handleWindowActivated(keyboardFocus); - if (mLastKeyboardFocus) - handleWindowDeactivated(mLastKeyboardFocus); - } + if (keyboardFocus) + handleWindowActivated(keyboardFocus); + if (mLastKeyboardFocus) + handleWindowDeactivated(mLastKeyboardFocus); mLastKeyboardFocus = keyboardFocus; } @@ -627,6 +623,13 @@ QWaylandInputDevice *QWaylandDisplay::defaultInputDevice() const return mInputDevices.isEmpty() ? 0 : mInputDevices.first(); } +bool QWaylandDisplay::isKeyboardAvailable() const +{ + return std::any_of( + mInputDevices.constBegin(), mInputDevices.constEnd(), + [this](const QWaylandInputDevice *device) { return device->keyboard() != nullptr; }); +} + #if QT_CONFIG(cursor) QWaylandCursor *QWaylandDisplay::waylandCursor() diff --git a/src/client/qwaylanddisplay_p.h b/src/client/qwaylanddisplay_p.h index 3b092bc..09a1736 100644 --- a/src/client/qwaylanddisplay_p.h +++ b/src/client/qwaylanddisplay_p.h @@ -215,6 +215,7 @@ public: void destroyFrameQueue(const FrameQueue &q); void dispatchQueueWhile(wl_event_queue *queue, std::function condition, int timeout = -1); + bool isKeyboardAvailable() const; public slots: void blockingReadEvents(); void flushRequests(); diff --git a/src/client/qwaylandwindow.cpp b/src/client/qwaylandwindow.cpp index e0b91dd..8d56be1 100644 --- a/src/client/qwaylandwindow.cpp +++ b/src/client/qwaylandwindow.cpp @@ -96,7 +96,6 @@ QWaylandWindow::QWaylandWindow(QWindow *window, QWaylandDisplay *display) QWaylandWindow::~QWaylandWindow() { mDisplay->destroyFrameQueue(mFrameQueue); - mDisplay->handleWindowDestroyed(this); delete mWindowDecoration; @@ -266,6 +265,8 @@ void QWaylandWindow::reset() mMask = QRegion(); mQueuedBuffer = nullptr; + + mDisplay->handleWindowDestroyed(this); } QWaylandWindow *QWaylandWindow::fromWlSurface(::wl_surface *surface) @@ -1097,7 +1098,10 @@ Qt::WindowStates QWaylandWindow::windowStates() const void QWaylandWindow::handleWindowStatesChanged(Qt::WindowStates states) { createDecoration(); - QWindowSystemInterface::handleWindowStateChanged(window(), states, mLastReportedWindowStates); + Qt::WindowStates statesWithoutActive = states & ~Qt::WindowActive; + Qt::WindowStates lastStatesWithoutActive = mLastReportedWindowStates & ~Qt::WindowActive; + QWindowSystemInterface::handleWindowStateChanged(window(), statesWithoutActive, + lastStatesWithoutActive); mLastReportedWindowStates = states; } diff --git a/src/client/shellintegration/qwaylandshellintegration_p.h b/src/client/shellintegration/qwaylandshellintegration_p.h index ccad004..b308ffe 100644 --- a/src/client/shellintegration/qwaylandshellintegration_p.h +++ b/src/client/shellintegration/qwaylandshellintegration_p.h @@ -73,12 +73,6 @@ public: return true; } virtual QWaylandShellSurface *createShellSurface(QWaylandWindow *window) = 0; - virtual void handleKeyboardFocusChanged(QWaylandWindow *newFocus, QWaylandWindow *oldFocus) { - if (newFocus) - m_display->handleWindowActivated(newFocus); - if (oldFocus) - m_display->handleWindowDeactivated(oldFocus); - } virtual void *nativeResourceForWindow(const QByteArray &resource, QWindow *window) { Q_UNUSED(resource); Q_UNUSED(window); diff --git a/src/plugins/shellintegration/xdg-shell-v5/qwaylandxdgshellv5integration.cpp b/src/plugins/shellintegration/xdg-shell-v5/qwaylandxdgshellv5integration.cpp index 4e25949..cfc6093 100644 --- a/src/plugins/shellintegration/xdg-shell-v5/qwaylandxdgshellv5integration.cpp +++ b/src/plugins/shellintegration/xdg-shell-v5/qwaylandxdgshellv5integration.cpp @@ -85,13 +85,6 @@ QWaylandShellSurface *QWaylandXdgShellV5Integration::createShellSurface(QWayland return m_xdgShell->createXdgSurface(window); } -void QWaylandXdgShellV5Integration::handleKeyboardFocusChanged(QWaylandWindow *newFocus, QWaylandWindow *oldFocus) { - if (newFocus && qobject_cast(newFocus->shellSurface())) - m_display->handleWindowActivated(newFocus); - if (oldFocus && qobject_cast(oldFocus->shellSurface())) - m_display->handleWindowDeactivated(oldFocus); -} - } QT_END_NAMESPACE diff --git a/src/plugins/shellintegration/xdg-shell-v5/qwaylandxdgshellv5integration_p.h b/src/plugins/shellintegration/xdg-shell-v5/qwaylandxdgshellv5integration_p.h index ce6bdb9..aed8867 100644 --- a/src/plugins/shellintegration/xdg-shell-v5/qwaylandxdgshellv5integration_p.h +++ b/src/plugins/shellintegration/xdg-shell-v5/qwaylandxdgshellv5integration_p.h @@ -67,7 +67,6 @@ public: QWaylandXdgShellV5Integration() {} bool initialize(QWaylandDisplay *display) override; QWaylandShellSurface *createShellSurface(QWaylandWindow *window) override; - void handleKeyboardFocusChanged(QWaylandWindow *newFocus, QWaylandWindow *oldFocus) override; private: QScopedPointer m_xdgShell; diff --git a/src/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6integration.cpp b/src/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6integration.cpp index 0316431..e8da8ba 100644 --- a/src/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6integration.cpp +++ b/src/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6integration.cpp @@ -68,20 +68,6 @@ QWaylandShellSurface *QWaylandXdgShellV6Integration::createShellSurface(QWayland return m_xdgShell->getXdgSurface(window); } -void QWaylandXdgShellV6Integration::handleKeyboardFocusChanged(QWaylandWindow *newFocus, QWaylandWindow *oldFocus) -{ - if (newFocus) { - auto *xdgSurface = qobject_cast(newFocus->shellSurface()); - if (xdgSurface && !xdgSurface->handlesActiveState()) - m_display->handleWindowActivated(newFocus); - } - if (oldFocus && qobject_cast(oldFocus->shellSurface())) { - auto *xdgSurface = qobject_cast(oldFocus->shellSurface()); - if (xdgSurface && !xdgSurface->handlesActiveState()) - m_display->handleWindowDeactivated(oldFocus); - } -} - } QT_END_NAMESPACE diff --git a/src/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6integration_p.h b/src/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6integration_p.h index 261f8cb..c1bcd5c 100644 --- a/src/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6integration_p.h +++ b/src/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6integration_p.h @@ -65,7 +65,6 @@ public: QWaylandXdgShellV6Integration() {} bool initialize(QWaylandDisplay *display) override; QWaylandShellSurface *createShellSurface(QWaylandWindow *window) override; - void handleKeyboardFocusChanged(QWaylandWindow *newFocus, QWaylandWindow *oldFocus) override; private: QScopedPointer m_xdgShell; diff --git a/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp b/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp index cf7eb4e..2c6e84b 100644 --- a/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp +++ b/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp @@ -67,11 +67,6 @@ QWaylandXdgSurface::Toplevel::Toplevel(QWaylandXdgSurface *xdgSurface) QWaylandXdgSurface::Toplevel::~Toplevel() { - if (m_applied.states & Qt::WindowActive) { - QWaylandWindow *window = m_xdgSurface->window(); - window->display()->handleWindowDeactivated(window); - } - // The protocol spec requires that the decoration object is deleted before xdg_toplevel. delete m_decoration; m_decoration = nullptr; @@ -85,17 +80,16 @@ void QWaylandXdgSurface::Toplevel::applyConfigure() if (!(m_applied.states & (Qt::WindowMaximized|Qt::WindowFullScreen))) m_normalSize = m_xdgSurface->m_window->windowFrameGeometry().size(); - if ((m_pending.states & Qt::WindowActive) && !(m_applied.states & Qt::WindowActive)) + if ((m_pending.states & Qt::WindowActive) && !(m_applied.states & Qt::WindowActive) + && !m_xdgSurface->m_window->display()->isKeyboardAvailable()) m_xdgSurface->m_window->display()->handleWindowActivated(m_xdgSurface->m_window); - if (!(m_pending.states & Qt::WindowActive) && (m_applied.states & Qt::WindowActive)) + if (!(m_pending.states & Qt::WindowActive) && (m_applied.states & Qt::WindowActive) + && !m_xdgSurface->m_window->display()->isKeyboardAvailable()) m_xdgSurface->m_window->display()->handleWindowDeactivated(m_xdgSurface->m_window); - // TODO: none of the other plugins send WindowActive either, but is it on purpose? - Qt::WindowStates statesWithoutActive = m_pending.states & ~Qt::WindowActive; - m_xdgSurface->m_window->handleToplevelWindowTilingStatesChanged(m_toplevelStates); - m_xdgSurface->m_window->handleWindowStatesChanged(statesWithoutActive); + m_xdgSurface->m_window->handleWindowStatesChanged(m_pending.states); if (m_pending.size.isEmpty()) { // An empty size in the configure means it's up to the client to choose the size diff --git a/src/plugins/shellintegration/xdg-shell/qwaylandxdgshellintegration.cpp b/src/plugins/shellintegration/xdg-shell/qwaylandxdgshellintegration.cpp index 8769d97..da0dd6a 100644 --- a/src/plugins/shellintegration/xdg-shell/qwaylandxdgshellintegration.cpp +++ b/src/plugins/shellintegration/xdg-shell/qwaylandxdgshellintegration.cpp @@ -69,20 +69,6 @@ QWaylandShellSurface *QWaylandXdgShellIntegration::createShellSurface(QWaylandWi return m_xdgShell->getXdgSurface(window); } -void QWaylandXdgShellIntegration::handleKeyboardFocusChanged(QWaylandWindow *newFocus, QWaylandWindow *oldFocus) -{ - if (newFocus) { - auto *xdgSurface = qobject_cast(newFocus->shellSurface()); - if (xdgSurface && !xdgSurface->handlesActiveState()) - m_display->handleWindowActivated(newFocus); - } - if (oldFocus && qobject_cast(oldFocus->shellSurface())) { - auto *xdgSurface = qobject_cast(oldFocus->shellSurface()); - if (xdgSurface && !xdgSurface->handlesActiveState()) - m_display->handleWindowDeactivated(oldFocus); - } -} - } QT_END_NAMESPACE diff --git a/src/plugins/shellintegration/xdg-shell/qwaylandxdgshellintegration_p.h b/src/plugins/shellintegration/xdg-shell/qwaylandxdgshellintegration_p.h index b6caa6c..2f929f9 100644 --- a/src/plugins/shellintegration/xdg-shell/qwaylandxdgshellintegration_p.h +++ b/src/plugins/shellintegration/xdg-shell/qwaylandxdgshellintegration_p.h @@ -65,7 +65,6 @@ public: QWaylandXdgShellIntegration() {} bool initialize(QWaylandDisplay *display) override; QWaylandShellSurface *createShellSurface(QWaylandWindow *window) override; - void handleKeyboardFocusChanged(QWaylandWindow *newFocus, QWaylandWindow *oldFocus) override; private: QScopedPointer m_xdgShell; diff --git a/tests/auto/client/xdgshell/tst_xdgshell.cpp b/tests/auto/client/xdgshell/tst_xdgshell.cpp index 2fdd0a7..5346b6e 100644 --- a/tests/auto/client/xdgshell/tst_xdgshell.cpp +++ b/tests/auto/client/xdgshell/tst_xdgshell.cpp @@ -31,6 +31,7 @@ #include #include #include +#include using namespace MockCompositor; @@ -154,9 +155,12 @@ void tst_xdgshell::configureStates() // Toplevel windows don't know their position on xdg-shell // QCOMPARE(window.frameGeometry().topLeft(), QPoint()); // TODO: this doesn't currently work when window decorations are enabled -// QEXPECT_FAIL("", "configure has already been acked, we shouldn't have to wait for isActive", Continue); -// QVERIFY(window.isActive()); - QTRY_VERIFY(window.isActive()); // Just make sure it eventually get's set correctly + // window.windowstate() is driven by keyboard focus, however for decorations we want to follow + // XDGShell this is internal to QtWayland so it is queried directly + auto waylandWindow = static_cast(window.handle()); + Q_ASSERT(waylandWindow); + QTRY_VERIFY(waylandWindow->windowStates().testFlag( + Qt::WindowActive)); // Just make sure it eventually get's set correctly const QSize screenSize(640, 480); const uint maximizedSerial = exec([=] {