From eb88762e0793b7f4f3facbaccb6a783370d6ee0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Fri, 28 Oct 2016 14:37:10 +0200 Subject: [PATCH 1/4] [autotests] Add test case for translucency effect of dialog window This test case simulates a condition of the translucency effect modifying windows of certain types (e.g. dialogs). In case the effect got activated for a window it does not end after the window gets closed and creates a non-interactive zombie window. CCBUG: 342716 --- .../integration/effects/translucency_test.cpp | 81 +++++++++++++++++-- 1 file changed, 74 insertions(+), 7 deletions(-) diff --git a/autotests/integration/effects/translucency_test.cpp b/autotests/integration/effects/translucency_test.cpp index 507e873616..92f15efe93 100644 --- a/autotests/integration/effects/translucency_test.cpp +++ b/autotests/integration/effects/translucency_test.cpp @@ -46,6 +46,7 @@ private Q_SLOTS: void cleanup(); void testMoveAfterDesktopChange(); + void testDialogClose(); private: Effect *m_translucencyEffect = nullptr; @@ -73,6 +74,8 @@ void TranslucencyTest::initTestCase() config->sync(); kwinApp()->setConfig(config); + // TODO: make effects use KWin's config directly + KSharedConfig::openConfig(QStringLiteral(KWIN_CONFIG), KConfig::NoGlobals)->group("Effect-kwin4_effect_translucency").writeEntry(QStringLiteral("Dialogs"), 90); qputenv("KWIN_EFFECTS_FORCE_ANIMATIONS", "1"); kwinApp()->start(); @@ -109,6 +112,14 @@ void TranslucencyTest::cleanup() m_translucencyEffect = nullptr; } +struct XcbConnectionDeleter +{ + static inline void cleanup(xcb_connection_t *pointer) + { + xcb_disconnect(pointer); + } +}; + void TranslucencyTest::testMoveAfterDesktopChange() { // test tries to simulate the condition of bug 366081 @@ -118,13 +129,6 @@ void TranslucencyTest::testMoveAfterDesktopChange() QVERIFY(windowAddedSpy.isValid()); // create an xcb window - struct XcbConnectionDeleter - { - static inline void cleanup(xcb_connection_t *pointer) - { - xcb_disconnect(pointer); - } - }; QScopedPointer c(xcb_connect(nullptr, nullptr)); QVERIFY(!xcb_connection_has_error(c.data())); const QRect windowGeometry(0, 0, 100, 200); @@ -182,5 +186,68 @@ void TranslucencyTest::testMoveAfterDesktopChange() c.reset(); } +void TranslucencyTest::testDialogClose() +{ + // this test simulates the condition of BUG 342716 + // with translucency settings for window type dialog the effect never ends when the window gets destroyed + QVERIFY(!m_translucencyEffect->isActive()); + QSignalSpy windowAddedSpy(effects, &EffectsHandler::windowAdded); + QVERIFY(windowAddedSpy.isValid()); + + // create an xcb window + QScopedPointer c(xcb_connect(nullptr, nullptr)); + QVERIFY(!xcb_connection_has_error(c.data())); + const QRect windowGeometry(0, 0, 100, 200); + xcb_window_t w = xcb_generate_id(c.data()); + xcb_create_window(c.data(), XCB_COPY_FROM_PARENT, w, rootWindow(), + windowGeometry.x(), + windowGeometry.y(), + windowGeometry.width(), + windowGeometry.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, windowGeometry.x(), windowGeometry.y()); + xcb_icccm_size_hints_set_size(&hints, 1, windowGeometry.width(), windowGeometry.height()); + xcb_icccm_set_wm_normal_hints(c.data(), w, &hints); + NETWinInfo winInfo(c.data(), w, rootWindow(), NET::Properties(), NET::Properties2()); + winInfo.setWindowType(NET::Dialog); + xcb_map_window(c.data(), w); + xcb_flush(c.data()); + + // we should get a client for it + QSignalSpy windowCreatedSpy(workspace(), &Workspace::clientAdded); + QVERIFY(windowCreatedSpy.isValid()); + QVERIFY(windowCreatedSpy.wait()); + Client *client = windowCreatedSpy.first().first().value(); + QVERIFY(client); + QCOMPARE(client->window(), w); + QVERIFY(client->isDecorated()); + QVERIFY(client->isDialog()); + + QVERIFY(windowAddedSpy.wait()); + QTRY_VERIFY(m_translucencyEffect->isActive()); + // and destroy the window again + xcb_unmap_window(c.data(), w); + xcb_flush(c.data()); + + QSignalSpy windowClosedSpy(client, &Client::windowClosed); + QVERIFY(windowClosedSpy.isValid()); + + QSignalSpy windowDeletedSpy(effects, &EffectsHandler::windowDeleted); + QVERIFY(windowDeletedSpy.isValid()); + QVERIFY(windowClosedSpy.wait()); + if (windowDeletedSpy.isEmpty()) { + QEXPECT_FAIL("", "BUG 342716", Continue); + QVERIFY(windowDeletedSpy.wait()); + } + QEXPECT_FAIL("", "BUG 342716", Continue); + QCOMPARE(windowDeletedSpy.count(), 1); + QEXPECT_FAIL("", "BUG 342716", Continue); + QTRY_VERIFY(!m_translucencyEffect->isActive()); + xcb_destroy_window(c.data(), w); + c.reset(); +} + WAYLANDTEST_MAIN(TranslucencyTest) #include "translucency_test.moc" From 2e3c6c92e94e9902a9c808f6af92aa3afc797fdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Mon, 24 Oct 2016 15:10:19 +0200 Subject: [PATCH 2/4] Trigger resize of input window when deco size changes Summary: Client::updateInputWindow operates with the decoration size. The method gets called from various points when changing the window geometry. If at that moment the decoration has not updated yet, the borders might be at a wrong position. This behavior could be triggered when a window requested to change the state to maximized. During maximization the decoration still had the wrong size when updateInputWindow was called, thus an interactive area inside the window was created. To circumvent this problem updateInputWindow is now also called whenever the window decoration changes. As a note: that a maximized window has resize only borders is wrong. Kwin should be protected against that. BUG: 371284 FIXED-IN: 5.8.3 Test Plan: Checked xwininfo for the deco extends window Reviewers: #kwin, #plasma Subscribers: plasma-devel, kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D3151 --- client.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/client.cpp b/client.cpp index 8542ba6be8..8ae4d9e172 100644 --- a/client.cpp +++ b/client.cpp @@ -410,6 +410,8 @@ void Client::createDecoration(const QRect& oldgeom) emit geometryShapeChanged(this, oldgeom); } ); + connect(decoratedClient()->decoratedClient(), &KDecoration2::DecoratedClient::widthChanged, this, &Client::updateInputWindow); + connect(decoratedClient()->decoratedClient(), &KDecoration2::DecoratedClient::heightChanged, this, &Client::updateInputWindow); } setDecoration(decoration); From 37067f538ec071e8ab9637052410953973b7c864 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Fri, 28 Oct 2016 11:44:57 +0200 Subject: [PATCH 3/4] Ensure the complete decoration texture gets repainted on recreation Summary: When the decoration size changes the textures get recreated and need to be properly filled. So far KWin used the scheduled repaint geometry in this situation. If the decoration didn't schedule the complete geometry for repain there will be an empty area in the decoration texture. This change ensures that the complete texture gets repainted when they are recreated. Thus the decoration rendering is more fault tolerant towards potential bugs in the decoration. With no-compositing and XRender compositing this problem was not reproducable and already fault tolerant, so OpenGL just catches up with the other modes. BUG: 371735 FIXED-IN: 5.8.3 Reviewers: #kwin, #plasma Subscribers: plasma-devel, kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D3186 --- scene_opengl.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scene_opengl.cpp b/scene_opengl.cpp index c8ce5db210..5e133820c5 100644 --- a/scene_opengl.cpp +++ b/scene_opengl.cpp @@ -2524,7 +2524,8 @@ void SceneOpenGLDecorationRenderer::render() if (scheduled.isEmpty()) { return; } - if (areImageSizesDirty()) { + const bool dirty = areImageSizesDirty(); + if (dirty) { resizeTexture(); resetImageSizesDirty(); } @@ -2537,7 +2538,7 @@ void SceneOpenGLDecorationRenderer::render() QRect left, top, right, bottom; client()->client()->layoutDecorationRects(left, top, right, bottom); - const QRect geometry = scheduled.boundingRect(); + const QRect geometry = dirty ? QRect(QPoint(0, 0), client()->client()->geometry().size()) : scheduled.boundingRect(); auto renderPart = [this](const QRect &geo, const QRect &partRect, const QPoint &offset, bool rotated = false) { if (geo.isNull()) { From 94c5704af74db5e0f8e86f0c572e4e11453e7002 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Fri, 28 Oct 2016 14:42:57 +0200 Subject: [PATCH 4/4] [effects/translucency] Cancel existing animations before starting new Summary: It can happen that startAnimation is invoked multiple times for a window. In case it was invoked a second time the previous animation was not cancelled. This resulted in the set-animation to never end. When closing a window, it would stay around as a translucent, non-interactive window zombie. This change ensures that existing animations get cancelled. BUG: 342716 FIXED-IN: 5.8.3 Test Plan: Tested through autotest and manually. Reviewers: #kwin, #plasma Subscribers: plasma-devel, kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D3190 --- autotests/integration/effects/translucency_test.cpp | 3 --- effects/translucency/package/contents/code/main.js | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/autotests/integration/effects/translucency_test.cpp b/autotests/integration/effects/translucency_test.cpp index 92f15efe93..428026d48c 100644 --- a/autotests/integration/effects/translucency_test.cpp +++ b/autotests/integration/effects/translucency_test.cpp @@ -238,12 +238,9 @@ void TranslucencyTest::testDialogClose() QVERIFY(windowDeletedSpy.isValid()); QVERIFY(windowClosedSpy.wait()); if (windowDeletedSpy.isEmpty()) { - QEXPECT_FAIL("", "BUG 342716", Continue); QVERIFY(windowDeletedSpy.wait()); } - QEXPECT_FAIL("", "BUG 342716", Continue); QCOMPARE(windowDeletedSpy.count(), 1); - QEXPECT_FAIL("", "BUG 342716", Continue); QTRY_VERIFY(!m_translucencyEffect->isActive()); xcb_destroy_window(c.data(), w); c.reset(); diff --git a/effects/translucency/package/contents/code/main.js b/effects/translucency/package/contents/code/main.js index f6468a946e..69c051187d 100644 --- a/effects/translucency/package/contents/code/main.js +++ b/effects/translucency/package/contents/code/main.js @@ -78,6 +78,9 @@ var translucencyEffect = { to: value / 100.0 }] }); + if (window.translucencyWindowTypeAnimation !== undefined) { + cancel(window.translucencyWindowTypeAnimation); + } window.translucencyWindowTypeAnimation = ids; } };