From ab8f05a57faf081114d56c7c9d67c7f602c987c2 Mon Sep 17 00:00:00 2001 From: Xaver Hugl Date: Tue, 30 Jul 2024 17:50:54 +0200 Subject: [PATCH] activation: fix X11 windows being stuck in should_get_focus more properly amends d01e20b6a9b455c209fea0e1e4eba7976a8a051f, and adds an autotest for the bug --- autotests/integration/CMakeLists.txt | 2 +- autotests/integration/activation_test.cpp | 66 +++++++++++++++++++++++ src/activation.cpp | 21 ++++---- 3 files changed, 79 insertions(+), 10 deletions(-) diff --git a/autotests/integration/CMakeLists.txt b/autotests/integration/CMakeLists.txt index 7233a22d7d..32bf7ba247 100644 --- a/autotests/integration/CMakeLists.txt +++ b/autotests/integration/CMakeLists.txt @@ -140,7 +140,7 @@ integrationTest(NAME testIdleInhibition SRCS idle_inhibition_test.cpp) integrationTest(NAME testDontCrashReinitializeCompositor SRCS dont_crash_reinitialize_compositor.cpp BUILTIN_EFFECTS) integrationTest(NAME testNoGlobalShortcuts SRCS no_global_shortcuts_test.cpp LIBS KF6::GlobalAccel) integrationTest(NAME testPlacement SRCS placement_test.cpp) -integrationTest(NAME testActivation SRCS activation_test.cpp) +integrationTest(NAME testActivation SRCS activation_test.cpp LIBS XCB::ICCCM) integrationTest(NAME testInputMethod SRCS inputmethod_test.cpp LIBS XKB::XKB) integrationTest(NAME testScreens SRCS screens_test.cpp) integrationTest(NAME testScreenEdges SRCS screenedges_test.cpp LIBS XCB::ICCCM) diff --git a/autotests/integration/activation_test.cpp b/autotests/integration/activation_test.cpp index 734d6d0e09..36e6107b7d 100644 --- a/autotests/integration/activation_test.cpp +++ b/autotests/integration/activation_test.cpp @@ -13,8 +13,11 @@ #include "wayland_server.h" #include "window.h" #include "workspace.h" +#include "x11window.h" #include +#include +#include namespace KWin { @@ -36,6 +39,7 @@ private Q_SLOTS: void testSwitchToWindowBelow(); void testSwitchToWindowMaximized(); void testSwitchToWindowFullScreen(); + void testActiveFullscreen(); private: void stackScreensHorizontally(); @@ -530,6 +534,68 @@ void ActivationTest::stackScreensVertically() Test::setOutputConfig(screenGeometries); } +static X11Window *createX11Window(xcb_connection_t *connection, const QRect &geometry, std::function setup = {}) +{ + xcb_window_t windowId = xcb_generate_id(connection); + xcb_create_window(connection, XCB_COPY_FROM_PARENT, windowId, rootWindow(), + geometry.x(), + geometry.y(), + geometry.width(), + geometry.height(), + 0, XCB_WINDOW_CLASS_INPUT_OUTPUT, XCB_COPY_FROM_PARENT, 0, nullptr); + + xcb_size_hints_t hints; + memset(&hints, 0, sizeof(hints)); + xcb_icccm_size_hints_set_position(&hints, 1, geometry.x(), geometry.y()); + xcb_icccm_size_hints_set_size(&hints, 1, geometry.width(), geometry.height()); + xcb_icccm_set_wm_normal_hints(connection, windowId, &hints); + + if (setup) { + setup(windowId); + } + + xcb_map_window(connection, windowId); + xcb_flush(connection); + + QSignalSpy windowCreatedSpy(workspace(), &Workspace::windowAdded); + if (!windowCreatedSpy.wait()) { + return nullptr; + } + return windowCreatedSpy.last().first().value(); +} + +void ActivationTest::testActiveFullscreen() +{ + // Tests that an active X11 fullscreen window gets removed from the active layer + // when activating a Wayland window, even if there's a pending activation request + // for the X11 window + Test::setOutputConfig({QRect(0, 0, 1280, 1024)}); + + Test::XcbConnectionPtr c = Test::createX11Connection(); + QVERIFY(!xcb_connection_has_error(c.get())); + X11Window *x11Window = createX11Window(c.get(), QRect(0, 0, 100, 200)); + + // make it fullscreen + x11Window->setFullScreen(true); + QVERIFY(x11Window->isFullScreen()); + QCOMPARE(x11Window->layer(), Layer::ActiveLayer); + + // now, activate it again + workspace()->activateWindow(x11Window); + + // now, create and activate a Wayland window + std::unique_ptr surface(Test::createSurface()); + std::unique_ptr shellSurface(Test::createXdgToplevelSurface(surface.get())); + auto waylandWindow = Test::renderAndWaitForShown(surface.get(), QSize(500, 300), Qt::blue); + QVERIFY(waylandWindow); + + // the Wayland window should become active + // and the X11 window should not be in the active layer anymore + QSignalSpy stackingOrder(workspace(), &Workspace::stackingOrderChanged); + workspace()->activateWindow(waylandWindow); + QCOMPARE(workspace()->activeWindow(), waylandWindow); + QCOMPARE(x11Window->layer(), Layer::NormalLayer); +} } WAYLANDTEST_MAIN(KWin::ActivationTest) diff --git a/src/activation.cpp b/src/activation.cpp index fdc2d6e933..11143d1655 100644 --- a/src/activation.cpp +++ b/src/activation.cpp @@ -21,7 +21,11 @@ #if KWIN_BUILD_ACTIVITIES #include "activities.h" #endif +#include "rules.h" +#include "useractions.h" #include "virtualdesktops.h" +#include "waylandwindow.h" +#include "window.h" #if KWIN_BUILD_X11 #include "atoms.h" @@ -32,12 +36,8 @@ #endif #include -#include - -#include "rules.h" -#include "useractions.h" -#include "window.h" #include +#include namespace KWin { @@ -232,6 +232,13 @@ void Workspace::setActiveWindow(Window *window) StackingUpdatesBlocker blocker(this); ++m_setActiveWindowRecursion; updateFocusMousePosition(Cursors::self()->mouse()->pos()); + + if (qobject_cast(window)) { + // focusIn events only arrive for X11 windows, Wayland windows don't use such a mechanism + // and so X11 windows could wrongly get stuck in the list + should_get_focus.clear(); + } + if (m_activeWindow != nullptr) { // note that this may call setActiveWindow( NULL ), therefore the recursion counter m_activeWindow->setActive(false); @@ -557,10 +564,6 @@ void Workspace::gotFocusIn(const Window *window) void Workspace::setShouldGetFocus(Window *window) { - if (m_activeWindow == window) { - // the matching focusIn event will never arrive - return; - } should_get_focus.append(window); updateStackingOrder(); // e.g. fullscreens have different layer when active/not-active }