yukijoou/kwin
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

placementtracker: fix quick tiled windows not being restored correctly

Browse source 
There were two problems:
1. Workspace interacted with the tile mode of windows before inhibiting
the placement tracker, so the wrong window state was stored in the placement
tracker
2. Window::setQuickTileMode is unintuitive and has some undesired side effects,
meant to handle quick tiling with keyboard shortcuts and by dragging the
window with a mouse specifically. This commit just works around that by
un-setting the tile mode first

BUG: 461886
This commit is contained in:
Xaver Hugl 2024-04-25 01:48:59 +02:00
parent df9f8f8346
commit 14c08422ca
3 changed files with 73 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

64
autotests/integration/outputchanges_test.cpp
View file

@ -41,6 +41,7 @@ private Q_SLOTS:
void testWindowRestoredAfterEnablingOutput();
void testMaximizedWindowRestoredAfterEnablingOutput();
void testFullScreenWindowRestoredAfterEnablingOutput();
void testQuickTiledWindowRestoredAfterEnablingOutput();
void testWindowRestoredAfterChangingScale();
void testMaximizeStateRestoredAfterEnablingOutput_data();
void testMaximizeStateRestoredAfterEnablingOutput();
@ -462,6 +463,69 @@ void OutputChangesTest::testFullScreenWindowRestoredAfterEnablingOutput()
QCOMPARE(window->fullscreenGeometryRestore(), QRectF(1280 + 50, 100, 100, 50));
}
void OutputChangesTest::testQuickTiledWindowRestoredAfterEnablingOutput()
{
// This test verifies that a quick tiled window will be moved to
// its original output and tile when the output is re-enabled
const auto outputs = kwinApp()->outputBackend()->outputs();
// Create a window.
std::unique_ptr<KWayland::Client::Surface> surface(Test::createSurface());
std::unique_ptr<Test::XdgToplevel> shellSurface(Test::createXdgToplevelSurface(surface.get()));
auto window = Test::renderAndWaitForShown(surface.get(), QSize(100, 50), Qt::blue);
QVERIFY(window);
// kwin will send a configure event with the actived state.
QSignalSpy toplevelConfigureRequestedSpy(shellSurface.get(), &Test::XdgToplevel::configureRequested);
QSignalSpy surfaceConfigureRequestedSpy(shellSurface->xdgSurface(), &Test::XdgSurface::configureRequested);
QVERIFY(surfaceConfigureRequestedSpy.wait());
// Move the window to the right monitor and tile it to the right.
QSignalSpy frameGeometryChangedSpy(window, &Window::frameGeometryChanged);
window->move(QPointF(1280 + 50, 100));
window->setQuickTileMode(QuickTileFlag::Right, true);
QVERIFY(surfaceConfigureRequestedSpy.wait());
QCOMPARE(toplevelConfigureRequestedSpy.last().at(0).value<QSize>(), QSize(1280 / 2, 1024));
shellSurface->xdgSurface()->ack_configure(surfaceConfigureRequestedSpy.last().at(0).value<quint32>());
Test::render(surface.get(), QSize(1280 / 2, 1024), Qt::blue);
QVERIFY(frameGeometryChangedSpy.wait());
const QRectF rightQuickTileGeom = QRectF(1280 + 1280 / 2, 0, 1280 / 2, 1024);
QCOMPARE(window->frameGeometry(), rightQuickTileGeom);
QCOMPARE(window->moveResizeGeometry(), rightQuickTileGeom);
QCOMPARE(window->output(), outputs[1]);
QCOMPARE(window->quickTileMode(), QuickTileFlag::Right);
QCOMPARE(window->requestedQuickTileMode(), QuickTileFlag::Right);
QCOMPARE(window->geometryRestore(), QRectF(1280 + 50, 100, 100, 50));
// Disable the right output.
OutputConfiguration config1;
{
auto changeSet = config1.changeSet(outputs[1]);
changeSet->enabled = false;
}
workspace()->applyOutputConfiguration(config1);
// The window will be moved to the left monitor
QCOMPARE(window->output(), outputs[0]);
// Enable the right monitor again
OutputConfiguration config2;
{
auto changeSet = config2.changeSet(outputs[1]);
changeSet->enabled = true;
}
workspace()->applyOutputConfiguration(config2);
// The window will be moved back to the right monitor, and put in the correct tile
QCOMPARE(window->frameGeometry(), rightQuickTileGeom);
QCOMPARE(window->moveResizeGeometry(), rightQuickTileGeom);
QCOMPARE(window->output(), outputs[1]);
QCOMPARE(window->quickTileMode(), QuickTileFlag::Right);
QCOMPARE(window->requestedQuickTileMode(), QuickTileFlag::Right);
QCOMPARE(window->geometryRestore(), QRectF(1280 + 50, 100, 100, 50));
}
void OutputChangesTest::testWindowRestoredAfterChangingScale()
{
// This test verifies that a window will be moved to its original position after changing the scale of an output

4
src/placementtracker.cpp
View file

@ -115,6 +115,10 @@ void PlacementTracker::restore(const QString &key)
}
}
if (restore) {
// to work around setQuickTileMode having unexpected side effects, make sure the window isn't tiled before
// setting the desired quick tile mode
// TODO fix this more properly
window->setQuickTileMode(QuickTileFlag::None, true);
window->setQuickTileMode(newData.quickTile, true);
window->setMaximize(newData.maximize & MaximizeMode::MaximizeVertical, newData.maximize & MaximizeMode::MaximizeHorizontal);
window->setFullScreen(newData.fullscreen);

9
src/workspace.cpp
View file

@ -1382,6 +1382,8 @@ void Workspace::updateOutputs(const QList<Output *> &outputOrder)
}
maybeDestroyDpmsFilter();
m_placementTracker->inhibit();
const auto removed = oldOutputsSet - outputsSet;
for (Output *output : removed) {
Q_EMIT outputRemoved(output);
@ -1426,6 +1428,9 @@ void Workspace::updateOutputs(const QList<Output *> &outputOrder)
desktopResized();
m_placementTracker->uninhibit();
m_placementTracker->restore(getPlacementTrackerHash());
for (Output *output : removed) {
output->unref();
}
@ -2200,8 +2205,6 @@ void Workspace::checkTransients(xcb_window_t w)
*/
void Workspace::desktopResized()
{
m_placementTracker->inhibit();
const QRect oldGeometry = m_geometry;
m_geometry = QRect();
for (const Output *output : std::as_const(m_outputs)) {
@ -2245,8 +2248,6 @@ void Workspace::desktopResized()
// TODO: emit a signal instead and remove the deep function calls into edges and effects
m_screenEdges->recreateEdges();
m_placementTracker->uninhibit();
m_placementTracker->restore(getPlacementTrackerHash());
if (m_geometry != oldGeometry) {
Q_EMIT geometryChanged();
}