From bb687ff6f18de74f060cd159ec50e4596ab97dc9 Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Sat, 30 Oct 2021 12:48:05 +0300 Subject: [PATCH 1/5] Stop abusing AbstractClient::geometryRestore() Currently, AbstractClient::geometryRestore() is abused to put windows back on their original screen. It makes window placement more complex and it breaks restoring initially maximized windows. --- src/abstract_client.cpp | 7 ++----- src/internal_client.cpp | 1 - src/x11client.cpp | 1 - src/xdgshellclient.cpp | 27 --------------------------- 4 files changed, 2 insertions(+), 34 deletions(-) diff --git a/src/abstract_client.cpp b/src/abstract_client.cpp index 7687b5c75b..cccee90a17 100644 --- a/src/abstract_client.cpp +++ b/src/abstract_client.cpp @@ -260,10 +260,7 @@ void AbstractClient::updateLayer() void AbstractClient::placeIn(const QRect &area) { - // TODO: Get rid of this method eventually. We need to call setGeometryRestore() because - // checkWorkspacePosition() operates on geometryRestore() and because of quick tiling. Placement::self()->place(this, area); - setGeometryRestore(moveResizeGeometry()); } void AbstractClient::invalidateLayer() @@ -3411,12 +3408,12 @@ void AbstractClient::checkWorkspacePosition(QRect oldGeometry, QRect oldClientGe int oldRightMax = oldScreenArea.x() + oldScreenArea.width(); int oldBottomMax = oldScreenArea.y() + oldScreenArea.height(); int oldLeftMax = oldScreenArea.x(); - const QRect screenArea = workspace()->clientArea(ScreenArea, this, geometryRestore().center()); + const QRect screenArea = workspace()->clientArea(ScreenArea, this, frameGeometry().center()); int topMax = screenArea.y(); int rightMax = screenArea.x() + screenArea.width(); int bottomMax = screenArea.y() + screenArea.height(); int leftMax = screenArea.x(); - QRect newGeom = geometryRestore(); // geometry(); + QRect newGeom = frameGeometry(); QRect newClientGeom = newGeom.adjusted(border[Left], border[Top], -border[Right], -border[Bottom]); const QRect newGeomTall = QRect(newGeom.x(), screenArea.y(), newGeom.width(), screenArea.height()); // Full screen height const QRect newGeomWide = QRect(screenArea.x(), newGeom.y(), screenArea.width(), newGeom.height()); // Full screen width diff --git a/src/internal_client.cpp b/src/internal_client.cpp index 948f48c933..43eceef3be 100644 --- a/src/internal_client.cpp +++ b/src/internal_client.cpp @@ -58,7 +58,6 @@ InternalClient::InternalClient(QWindow *window) commitGeometry(m_internalWindow->geometry()); updateDecoration(true); moveResize(clientRectToFrameRect(m_internalWindow->geometry())); - setGeometryRestore(moveResizeGeometry()); blockGeometryUpdates(false); m_internalWindow->installEventFilter(this); diff --git a/src/x11client.cpp b/src/x11client.cpp index 7b57b09637..54d6d6481f 100644 --- a/src/x11client.cpp +++ b/src/x11client.cpp @@ -3877,7 +3877,6 @@ void X11Client::configureRequest(int value_mask, int rx, int ry, int rw, int rh, } } } - setGeometryRestore(frameGeometry()); // No need to send synthetic configure notify event here, either it's sent together // with geometry change, or there's no need to send it. // Handling of the real ConfigureRequest event forces sending it, as there it's necessary. diff --git a/src/xdgshellclient.cpp b/src/xdgshellclient.cpp index 9e8216a3a0..6aab4fc6c6 100644 --- a/src/xdgshellclient.cpp +++ b/src/xdgshellclient.cpp @@ -88,13 +88,6 @@ XdgSurfaceClient::XdgSurfaceClient(XdgSurfaceInterface *shellSurface) m_configureTimer->setSingleShot(true); connect(m_configureTimer, &QTimer::timeout, this, &XdgSurfaceClient::sendConfigure); - - // Unfortunately, AbstractClient::checkWorkspacePosition() operates on the geometry restore - // so we need to initialize it with some reasonable value; otherwise bad things will happen - // when we want to decorate the client or move the client to another screen. This is a hack. - - connect(this, &XdgSurfaceClient::frameGeometryChanged, - this, &XdgSurfaceClient::updateGeometryRestoreHack); } XdgSurfaceClient::~XdgSurfaceClient() @@ -314,23 +307,6 @@ void XdgSurfaceClient::moveResizeInternal(const QRect &rect, MoveResizeMode mode } } -/** - * \internal - * \todo We have to check the current frame geometry in checkWorskpacePosition(). - * - * Sets the geometry restore to the first valid frame geometry. This is a HACK! - * - * Unfortunately, AbstractClient::checkWorkspacePosition() operates on the geometry restore - * rather than the current frame geometry, so we have to ensure that it's initialized with - * some reasonable value even if the client is not maximized or quick tiled. - */ -void XdgSurfaceClient::updateGeometryRestoreHack() -{ - if (geometryRestore().isEmpty() && !frameGeometry().isEmpty()) { - setGeometryRestore(frameGeometry()); - } -} - QRect XdgSurfaceClient::frameRectToBufferRect(const QRect &rect) const { const int left = rect.left() + borderLeft() - m_windowGeometry.left(); @@ -638,10 +614,7 @@ void XdgToplevelClient::updateDecoration(bool check_workspace_pos, bool force) } updateShadow(); if (check_workspace_pos) { - const QRect oldGeometryRestore = geometryRestore(); - setGeometryRestore(frameGeometry()); checkWorkspacePosition(oldFrameGeometry, oldClientGeometry); - setGeometryRestore(oldGeometryRestore); } } -- GitLab From 6faf4ec3a2c519b7d20325ed4eeb9d4993c3cd2f Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Sat, 30 Oct 2021 12:54:19 +0300 Subject: [PATCH 2/5] Make mapping between outputs and cached screen geometries in Workspace robust Integer screen ids are not robust. On the other hand, AbstractOutput does not change if an output has been connected or disconnected. --- src/workspace.cpp | 8 ++++---- src/workspace.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/workspace.cpp b/src/workspace.cpp index e4ae1ae18f..1698ad7fcb 100644 --- a/src/workspace.cpp +++ b/src/workspace.cpp @@ -2091,11 +2091,11 @@ void Workspace::desktopResized() void Workspace::saveOldScreenSizes() { olddisplaysize = m_geometry.size(); - oldscreensizes.clear(); + m_oldScreenGeometries.clear(); const auto outputs = kwinApp()->platform()->enabledOutputs(); for (const AbstractOutput *output : outputs) { - oldscreensizes.append(output->geometry()); + m_oldScreenGeometries.insert(output, output->geometry()); } } @@ -2405,9 +2405,9 @@ QRegion Workspace::previousRestrictedMoveArea(const VirtualDesktop *desktop, Str return strutsToRegion(areas, m_oldRestrictedAreas[desktop]); } -QVector< QRect > Workspace::previousScreenSizes() const +QHash Workspace::previousScreenSizes() const { - return oldscreensizes; + return m_oldScreenGeometries; } int Workspace::oldDisplayWidth() const diff --git a/src/workspace.h b/src/workspace.h index 1157ce6dbb..d3bc39f63e 100644 --- a/src/workspace.h +++ b/src/workspace.h @@ -271,7 +271,7 @@ public: // The calls below are valid only in that case. bool inUpdateClientArea() const; QRegion previousRestrictedMoveArea(const VirtualDesktop *desktop, StrutAreas areas = StrutAreaAll) const; - QVector< QRect > previousScreenSizes() const; + QHash previousScreenSizes() const; int oldDisplayWidth() const; int oldDisplayHeight() const; @@ -670,7 +670,7 @@ private: QHash> m_screenAreas; QRect m_geometry; - QVector< QRect > oldscreensizes; // array of previous sizes of xinerama screens + QHash m_oldScreenGeometries; QSize olddisplaysize; // previous sizes od displayWidth()/displayHeight() QHash m_oldRestrictedAreas; -- GitLab From ee41e9b6e7c2b558c4857edd4fbf2548e792a9be Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Sat, 30 Oct 2021 13:43:08 +0300 Subject: [PATCH 3/5] Fix Workspace::inUpdateClientArea() with auto-hide panels If there's an auto-hide panel, m_oldRestrictedAreas will be empty, which will break AbstractClient::checkWorkspacePosition(). --- src/workspace.cpp | 4 +++- src/workspace.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/workspace.cpp b/src/workspace.cpp index 1698ad7fcb..1bc7964f7a 100644 --- a/src/workspace.cpp +++ b/src/workspace.cpp @@ -2254,6 +2254,7 @@ void Workspace::updateClientArea() m_workAreas = workAreas; m_screenAreas = screenAreas; + m_inUpdateClientArea = true; m_oldRestrictedAreas = m_restrictedAreas; m_restrictedAreas = restrictedAreas; @@ -2276,6 +2277,7 @@ void Workspace::updateClientArea() } m_oldRestrictedAreas.clear(); // reset, no longer valid or needed + m_inUpdateClientArea = false; } } @@ -2397,7 +2399,7 @@ QRegion Workspace::restrictedMoveArea(const VirtualDesktop *desktop, StrutAreas bool Workspace::inUpdateClientArea() const { - return !m_oldRestrictedAreas.isEmpty(); + return m_inUpdateClientArea; } QRegion Workspace::previousRestrictedMoveArea(const VirtualDesktop *desktop, StrutAreas areas) const diff --git a/src/workspace.h b/src/workspace.h index d3bc39f63e..8d61a0921b 100644 --- a/src/workspace.h +++ b/src/workspace.h @@ -673,6 +673,7 @@ private: QHash m_oldScreenGeometries; QSize olddisplaysize; // previous sizes od displayWidth()/displayHeight() QHash m_oldRestrictedAreas; + bool m_inUpdateClientArea = false; int set_active_client_recursion; int block_stacking_updates; // When > 0, stacking updates are temporarily disabled -- GitLab From 4928f4db5b810969c70d549729c23ff95d1a6781 Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Sat, 30 Oct 2021 13:02:40 +0300 Subject: [PATCH 4/5] Try to preserve window position relative to their outputs during hotplug Currently, if an output is hotplugged, all windows will be scrambled, which is highly annoying. With this change, windows will stick to their outputs if an output has been connected or disconnected. BUG: 296673 BUG: 378896 BUG: 412703 BUG: 443698 --- src/abstract_client.cpp | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/abstract_client.cpp b/src/abstract_client.cpp index cccee90a17..5f306970db 100644 --- a/src/abstract_client.cpp +++ b/src/abstract_client.cpp @@ -3346,8 +3346,10 @@ void AbstractClient::checkWorkspacePosition(QRect oldGeometry, QRect oldClientGe } enum { Left = 0, Top, Right, Bottom }; const int border[4] = { borderLeft(), borderTop(), borderRight(), borderBottom() }; + + QRect newGeom = moveResizeGeometry(); if( !oldGeometry.isValid()) - oldGeometry = moveResizeGeometry(); + oldGeometry = newGeom; if (!oldClientGeometry.isValid()) oldClientGeometry = oldGeometry.adjusted(border[Left], border[Top], -border[Right], -border[Bottom]); if (isFullScreen()) { @@ -3387,20 +3389,18 @@ void AbstractClient::checkWorkspacePosition(QRect oldGeometry, QRect oldClientGe // Old and new maximums have different starting values so windows on the screen // edge will move when a new strut is placed on the edge. QRect oldScreenArea; + QRect screenArea; if( workspace()->inUpdateClientArea()) { // we need to find the screen area as it was before the change - oldScreenArea = QRect( 0, 0, workspace()->oldDisplayWidth(), workspace()->oldDisplayHeight()); - int distance = INT_MAX; - const auto previousSizes = workspace()->previousScreenSizes(); - for (const QRect &r : previousSizes) { - int d = r.contains( oldGeometry.center()) ? 0 : ( r.center() - oldGeometry.center()).manhattanLength(); - if( d < distance ) { - distance = d; - oldScreenArea = r; - } + oldScreenArea = workspace()->previousScreenSizes().value(output()); + if (oldScreenArea.isNull()) { + oldScreenArea = output()->geometry(); } + screenArea = output()->geometry(); + newGeom.translate(screenArea.topLeft() - oldScreenArea.topLeft()); } else { oldScreenArea = workspace()->clientArea(ScreenArea, kwinApp()->platform()->outputAt(oldGeometry.center()), oldDesktop); + screenArea = workspace()->clientArea(ScreenArea, this, newGeom.center()); } const QRect oldGeomTall = QRect(oldGeometry.x(), oldScreenArea.y(), oldGeometry.width(), oldScreenArea.height()); // Full screen height const QRect oldGeomWide = QRect(oldScreenArea.x(), oldGeometry.y(), oldScreenArea.width(), oldGeometry.height()); // Full screen width @@ -3408,12 +3408,10 @@ void AbstractClient::checkWorkspacePosition(QRect oldGeometry, QRect oldClientGe int oldRightMax = oldScreenArea.x() + oldScreenArea.width(); int oldBottomMax = oldScreenArea.y() + oldScreenArea.height(); int oldLeftMax = oldScreenArea.x(); - const QRect screenArea = workspace()->clientArea(ScreenArea, this, frameGeometry().center()); int topMax = screenArea.y(); int rightMax = screenArea.x() + screenArea.width(); int bottomMax = screenArea.y() + screenArea.height(); int leftMax = screenArea.x(); - QRect newGeom = frameGeometry(); QRect newClientGeom = newGeom.adjusted(border[Left], border[Top], -border[Right], -border[Bottom]); const QRect newGeomTall = QRect(newGeom.x(), screenArea.y(), newGeom.width(), screenArea.height()); // Full screen height const QRect newGeomWide = QRect(screenArea.x(), newGeom.y(), screenArea.width(), newGeom.height()); // Full screen width -- GitLab From 218db7400006fddacd3dcd3db2e6ded1e54022ea Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Sat, 30 Oct 2021 13:51:57 +0300 Subject: [PATCH 5/5] Remove a GeometryUpdatesBlocker in AbstractClient::checkWorkspacePosition() It was added to work around an async geometry issue on Wayland, which is no longer actual. --- src/abstract_client.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/abstract_client.cpp b/src/abstract_client.cpp index 5f306970db..fda6f94dd9 100644 --- a/src/abstract_client.cpp +++ b/src/abstract_client.cpp @@ -3358,7 +3358,6 @@ void AbstractClient::checkWorkspacePosition(QRect oldGeometry, QRect oldClientGe } if (maximizeMode() != MaximizeRestore) { - GeometryUpdatesBlocker block(this); changeMaximize(false, false, true); // adjust size QRect geom = moveResizeGeometry(); const QRect screenArea = workspace()->clientArea(ScreenArea, this, geom.center()); -- GitLab