From b1bffa445823a956cd4dce072c92787ef83502de Mon Sep 17 00:00:00 2001 From: Fabian Vogt Date: Sun, 4 Mar 2018 18:43:42 +0100 Subject: [PATCH 1/2] Fix typo in config group name Summary: Quickly fix it before anyone relies on this typo. Test Plan: None. Yes, really untested. Reviewers: #plasma, graesslin, davidedmundson Reviewed By: #plasma, davidedmundson Subscribers: kwin, plasma-devel, #kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D11047 --- plugins/platforms/drm/drm_output.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/platforms/drm/drm_output.cpp b/plugins/platforms/drm/drm_output.cpp index 8a21bf56df..bd3dac1a8c 100644 --- a/plugins/platforms/drm/drm_output.cpp +++ b/plugins/platforms/drm/drm_output.cpp @@ -303,7 +303,7 @@ bool DrmOutput::init(drmModeConnector *connector) QSize physicalSize = !m_edid.physicalSize.isEmpty() ? m_edid.physicalSize : QSize(connector->mmWidth, connector->mmHeight); // the size might be completely borked. E.g. Samsung SyncMaster 2494HS reports 160x90 while in truth it's 520x292 // as this information is used to calculate DPI info, it's going to result in everything being huge - const QByteArray unknown = QByteArrayLiteral("unkown"); + const QByteArray unknown = QByteArrayLiteral("unknown"); KConfigGroup group = kwinApp()->config()->group("EdidOverwrite").group(m_edid.eisaId.isEmpty() ? unknown : m_edid.eisaId) .group(m_edid.monitorName.isEmpty() ? unknown : m_edid.monitorName) .group(m_edid.serialNumber.isEmpty() ? unknown : m_edid.serialNumber); From e1afef3d4563803249dc25487bc086f753c4e0ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Fl=C3=B6ser?= Date: Sun, 4 Mar 2018 11:23:28 +0100 Subject: [PATCH 2/2] Sanity check WindowQuad before trying to create a grid out of it Summary: When one uses: * breeze as of 5.12 * wobbly windows * shaded window * a distribution building with assert enabled and starts to move a shaded window, KWin asserts. The root cause for this is that WindowQuad::makeSubQuad has an assert for y1 being smaller than y2. With the combination listed above this is not guaranteed. For the left shadow quad the y1 and y2 are identical and thus trying to split it, results in the assert condition. The problem of the shadow quad having an invalid size might be addressed as well with D10811. Due to that the generation of the quads is not touched. Instead a sanity check is introduced to not try to split already invalid sized quads. BUG: 390953 FIXED-IN: 5.12.3 Test Plan: Added unit test hit the assert, now doesn't hit it any more Reviewers: #kwin, #plasma Subscribers: kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D11015 --- autotests/integration/effects/CMakeLists.txt | 1 + .../integration/effects/wobbly_shade_test.cpp | 197 ++++++++++++++++++ libkwineffects/kwineffects.cpp | 12 ++ 3 files changed, 210 insertions(+) create mode 100644 autotests/integration/effects/wobbly_shade_test.cpp diff --git a/autotests/integration/effects/CMakeLists.txt b/autotests/integration/effects/CMakeLists.txt index 294acb01f5..10446bba1c 100644 --- a/autotests/integration/effects/CMakeLists.txt +++ b/autotests/integration/effects/CMakeLists.txt @@ -1,6 +1,7 @@ if (XCB_ICCCM_FOUND) integrationTest(NAME testTranslucency SRCS translucency_test.cpp LIBS XCB::ICCCM) integrationTest(NAME testSlidingPopups SRCS slidingpopups_test.cpp LIBS XCB::ICCCM) + integrationTest(NAME testShadeWobblyWindows SRCS wobbly_shade_test.cpp LIBS XCB::ICCCM) endif() integrationTest(NAME testFade SRCS fade_test.cpp) integrationTest(WAYLAND_ONLY NAME testEffectWindowGeometry SRCS windowgeometry_test.cpp) diff --git a/autotests/integration/effects/wobbly_shade_test.cpp b/autotests/integration/effects/wobbly_shade_test.cpp new file mode 100644 index 0000000000..f251b9623e --- /dev/null +++ b/autotests/integration/effects/wobbly_shade_test.cpp @@ -0,0 +1,197 @@ +/******************************************************************** +KWin - the KDE window manager +This file is part of the KDE project. + +Copyright (C) 2018 Martin Flöser + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program. If not, see . +*********************************************************************/ +#include "kwin_wayland_test.h" +#include "composite.h" +#include "cursor.h" +#include "effects.h" +#include "effectloader.h" +#include "cursor.h" +#include "platform.h" +#include "shell_client.h" +#include "wayland_server.h" +#include "workspace.h" +#include "effect_builtins.h" + +#include + +#include +#include +#include +#include +#include + +#include +#include + +using namespace KWin; +static const QString s_socketName = QStringLiteral("wayland_test_effects_wobbly_shade-0"); + +class WobblyWindowsShadeTest : public QObject +{ +Q_OBJECT +private Q_SLOTS: + void initTestCase(); + void init(); + void cleanup(); + + void testShadeMove(); +}; + +void WobblyWindowsShadeTest::initTestCase() +{ + qRegisterMetaType(); + qRegisterMetaType(); + qRegisterMetaType(); + QSignalSpy workspaceCreatedSpy(kwinApp(), &Application::workspaceCreated); + QVERIFY(workspaceCreatedSpy.isValid()); + kwinApp()->platform()->setInitialWindowSize(QSize(1280, 1024)); + QVERIFY(waylandServer()->init(s_socketName.toLocal8Bit())); + + // disable all effects - we don't want to have it interact with the rendering + auto config = KSharedConfig::openConfig(QString(), KConfig::SimpleConfig); + KConfigGroup plugins(config, QStringLiteral("Plugins")); + ScriptedEffectLoader loader; + const auto builtinNames = BuiltInEffects::availableEffectNames() << loader.listOfKnownEffects(); + for (QString name : builtinNames) { + plugins.writeEntry(name + QStringLiteral("Enabled"), false); + } + + config->sync(); + kwinApp()->setConfig(config); + + qputenv("KWIN_COMPOSE", QByteArrayLiteral("O2")); + qputenv("KWIN_EFFECTS_FORCE_ANIMATIONS", "1"); + kwinApp()->start(); + QVERIFY(workspaceCreatedSpy.wait()); + QVERIFY(Compositor::self()); +} + +void WobblyWindowsShadeTest::init() +{ + QVERIFY(Test::setupWaylandConnection(Test::AdditionalWaylandInterface::Decoration)); +} + +void WobblyWindowsShadeTest::cleanup() +{ + Test::destroyWaylandConnection(); + EffectsHandlerImpl *e = static_cast(effects); + while (!e->loadedEffects().isEmpty()) { + const QString effect = e->loadedEffects().first(); + e->unloadEffect(effect); + QVERIFY(!e->isEffectLoaded(effect)); + } +} + +struct XcbConnectionDeleter +{ + static inline void cleanup(xcb_connection_t *pointer) + { + xcb_disconnect(pointer); + } +}; + +void WobblyWindowsShadeTest::testShadeMove() +{ + // this test simulates the condition from BUG 390953 + EffectsHandlerImpl *e = static_cast(effects); + QVERIFY(e->loadEffect(BuiltInEffects::nameForEffect(BuiltInEffect::WobblyWindows))); + QVERIFY(e->isEffectLoaded(BuiltInEffects::nameForEffect(BuiltInEffect::WobblyWindows))); + + + 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); + 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->isShadeable()); + QVERIFY(!client->isShade()); + QVERIFY(client->isActive()); + + QSignalSpy windowShownSpy(client, &AbstractClient::windowShown); + QVERIFY(windowShownSpy.isValid()); + QVERIFY(windowShownSpy.wait()); + + // now shade the window + workspace()->slotWindowShade(); + QVERIFY(client->isShade()); + + QSignalSpy windowStartUserMovedResizedSpy(e, &EffectsHandler::windowStartUserMovedResized); + QVERIFY(windowStartUserMovedResizedSpy.isValid()); + + // begin move + QVERIFY(workspace()->getMovingClient() == nullptr); + QCOMPARE(client->isMove(), false); + workspace()->slotWindowMove(); + QCOMPARE(workspace()->getMovingClient(), client); + QCOMPARE(client->isMove(), true); + QCOMPARE(windowStartUserMovedResizedSpy.count(), 1); + + // wait for frame rendered + QTest::qWait(100); + + // send some key events, not going through input redirection + client->keyPressEvent(Qt::Key_Right); + client->updateMoveResize(KWin::Cursor::pos()); + + // wait for frame rendered + QTest::qWait(100); + + client->keyPressEvent(Qt::Key_Right); + client->updateMoveResize(KWin::Cursor::pos()); + + // wait for frame rendered + QTest::qWait(100); + + client->keyPressEvent(Qt::Key_Down | Qt::ALT); + client->updateMoveResize(KWin::Cursor::pos()); + + // wait for frame rendered + QTest::qWait(100); + + // let's end + client->keyPressEvent(Qt::Key_Enter); + + // wait for frame rendered + QTest::qWait(100); +} + +WAYLANDTEST_MAIN(WobblyWindowsShadeTest) +#include "wobbly_shade_test.moc" diff --git a/libkwineffects/kwineffects.cpp b/libkwineffects/kwineffects.cpp index 47aeb24379..d60fdf3523 100644 --- a/libkwineffects/kwineffects.cpp +++ b/libkwineffects/kwineffects.cpp @@ -1155,6 +1155,12 @@ WindowQuadList WindowQuadList::makeGrid(int maxQuadSize) const const double quadTop = quad.top(); const double quadBottom = quad.bottom(); + // sanity check, see BUG 390953 + if (quadLeft == quadRight || quadTop == quadBottom) { + ret.append(quad); + continue; + } + // Compute the top-left corner of the first intersecting grid cell const double xBegin = left + qFloor((quadLeft - left) / maxQuadSize) * maxQuadSize; const double yBegin = top + qFloor((quadTop - top) / maxQuadSize) * maxQuadSize; @@ -1209,6 +1215,12 @@ WindowQuadList WindowQuadList::makeRegularGrid(int xSubdivisions, int ySubdivisi const double quadTop = quad.top(); const double quadBottom = quad.bottom(); + // sanity check, see BUG 390953 + if (quadLeft == quadRight || quadTop == quadBottom) { + ret.append(quad); + continue; + } + // Compute the top-left corner of the first intersecting grid cell const double xBegin = left + qFloor((quadLeft - left) / xIncrement) * xIncrement; const double yBegin = top + qFloor((quadTop - top) / yIncrement) * yIncrement;