From 71203489320d331fa908f388427d4b8cbba4ab49 Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Fri, 14 Jan 2022 18:07:44 +0200 Subject: [PATCH] Make checkWorkspacePosition() work without client geometry A good portion of geometry handling code was written during the X11 times. The main difference between X11 and Wayland is that kwin doesn't know where a window will exactly be after resize() or moveResize(). In order to handle Wayland specifics, every window has a bounding geometry that is being manipulated by move(), resize(), and moveResize(). The frameGeometry(), the clientGeometry(), and the bufferGeometry() are not manipulated by move(), resize(), and moveResize() directly. Almost everything that manipulates geometry should use moveResizeGeometry(). This creates a problem though, since the clientGeometry() will be updated only after the client provides a new buffer, kwin has absolutely no idea what the client geometry for a given move resize geometry will be. Another side of the coin is that decoration updates are performed asynchronously on wayland, meaning that you cannot use border properties for anything related to geometry handling and you should avoid using borderLeft(), borderTop(), borderRight(), and borderBottom() in general. clientGeometry(), bufferGeometry(), and border*() are good only if you want to forward an event or render something. They can't be used for manipulating the geometry. Unfortunately, AbstractClient::checkWorkspacePosition() needs both, which is a bit of a problem. To add more oil to the fire, contents of a decorated window can be snapped to a screen edge. This goes against the nature of geometry updates on wayland, where we try to indicate the bounds of the frame geometry and avoid using client and buffer geometries. In order to make geometry handling more correct on wayland, this change removes the ability to snap the contents of a decorated window to a screen edge. This allows to avoid using the client geometry in checkWorkspacePosition(), which is a very important function that ensures the window is inside the workspace. There is nothing wrong with snapping the frame rather than its contents and that's what kwin used to do. It was changed with the removal of "Display borders on maximized windows" option, the relevant commit didn't provide any reasoning behind the change. --- src/abstract_client.cpp | 63 +++++++++-------------------------------- src/abstract_client.h | 2 +- src/internal_client.cpp | 8 ++---- src/placement.cpp | 59 -------------------------------------- src/workspace.cpp | 30 ++++---------------- src/x11client.cpp | 3 +- 6 files changed, 24 insertions(+), 141 deletions(-) diff --git a/src/abstract_client.cpp b/src/abstract_client.cpp index 12c3db830d..ff61825915 100644 --- a/src/abstract_client.cpp +++ b/src/abstract_client.cpp @@ -3302,19 +3302,16 @@ void AbstractClient::updateGeometryRestoresForFullscreen(AbstractOutput *output) setGeometryRestore(newGeometryRestore); } -void AbstractClient::checkWorkspacePosition(QRect oldGeometry, QRect oldClientGeometry, const VirtualDesktop *oldDesktop) +void AbstractClient::checkWorkspacePosition(QRect oldGeometry, const VirtualDesktop *oldDesktop) { if (isDock() || isDesktop() || !isPlaceable()) { return; } - enum { Left = 0, Top, Right, Bottom }; - const int border[4] = { borderLeft(), borderTop(), borderRight(), borderBottom() }; QRect newGeom = moveResizeGeometry(); - if( !oldGeometry.isValid()) + if (!oldGeometry.isValid()) { oldGeometry = newGeom; - if (!oldClientGeometry.isValid()) - oldClientGeometry = oldGeometry.adjusted(border[Left], border[Top], -border[Right], -border[Bottom]); + } if (isFullScreen()) { moveResize(workspace()->clientArea(FullScreenArea, this, newGeom.center())); updateGeometryRestoresForFullscreen(kwinApp()->platform()->outputAt(newGeom.center())); @@ -3380,7 +3377,6 @@ void AbstractClient::checkWorkspacePosition(QRect oldGeometry, QRect oldClientGe int rightMax = screenArea.x() + screenArea.width(); int bottomMax = screenArea.y() + screenArea.height(); int leftMax = screenArea.x(); - 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 // Get the max strut point for each side where the window is (E.g. Highest point for @@ -3436,82 +3432,51 @@ void AbstractClient::checkWorkspacePosition(QRect oldGeometry, QRect oldClientGe // Check if the sides were inside or touching but are no longer + enum { Left = 0, Top, Right, Bottom }; bool keep[4] = {false, false, false, false}; bool save[4] = {false, false, false, false}; - int padding[4] = {0, 0, 0, 0}; if (oldGeometry.x() >= oldLeftMax) save[Left] = newGeom.x() < leftMax; if (oldGeometry.x() == oldLeftMax) keep[Left] = newGeom.x() != leftMax; - else if (oldClientGeometry.x() == oldLeftMax && newClientGeom.x() != leftMax) { - padding[0] = border[Left]; - keep[Left] = true; - } + if (oldGeometry.y() >= oldTopMax) save[Top] = newGeom.y() < topMax; if (oldGeometry.y() == oldTopMax) keep[Top] = newGeom.y() != topMax; - else if (oldClientGeometry.y() == oldTopMax && newClientGeom.y() != topMax) { - padding[1] = border[Left]; - keep[Top] = true; - } + if (oldGeometry.right() <= oldRightMax - 1) save[Right] = newGeom.right() > rightMax - 1; if (oldGeometry.right() == oldRightMax - 1) keep[Right] = newGeom.right() != rightMax - 1; - else if (oldClientGeometry.right() == oldRightMax - 1 && newClientGeom.right() != rightMax - 1) { - padding[2] = border[Right]; - keep[Right] = true; - } + if (oldGeometry.bottom() <= oldBottomMax - 1) save[Bottom] = newGeom.bottom() > bottomMax - 1; if (oldGeometry.bottom() == oldBottomMax - 1) keep[Bottom] = newGeom.bottom() != bottomMax - 1; - else if (oldClientGeometry.bottom() == oldBottomMax - 1 && newClientGeom.bottom() != bottomMax - 1) { - padding[3] = border[Bottom]; - keep[Bottom] = true; - } + // if randomly touches opposing edges, do not favor either if (keep[Left] && keep[Right]) { keep[Left] = keep[Right] = false; - padding[0] = padding[2] = 0; } if (keep[Top] && keep[Bottom]) { keep[Top] = keep[Bottom] = false; - padding[1] = padding[3] = 0; } if (save[Left] || keep[Left]) - newGeom.moveLeft(qMax(leftMax, screenArea.x()) - padding[0]); - if (padding[0] && screens()->intersecting(newGeom) > 1) - newGeom.moveLeft(newGeom.left() + padding[0]); + newGeom.moveLeft(qMax(leftMax, screenArea.x())); if (save[Top] || keep[Top]) - newGeom.moveTop(qMax(topMax, screenArea.y()) - padding[1]); - if (padding[1] && screens()->intersecting(newGeom) > 1) - newGeom.moveTop(newGeom.top() + padding[1]); + newGeom.moveTop(qMax(topMax, screenArea.y())); if (save[Right] || keep[Right]) - newGeom.moveRight(qMin(rightMax - 1, screenArea.right()) + padding[2]); - if (padding[2] && screens()->intersecting(newGeom) > 1) - newGeom.moveRight(newGeom.right() - padding[2]); + newGeom.moveRight(qMin(rightMax - 1, screenArea.right())); + if (save[Bottom] || keep[Bottom]) + newGeom.moveBottom(qMin(bottomMax - 1, screenArea.bottom())); + if (oldGeometry.x() >= oldLeftMax && newGeom.x() < leftMax) newGeom.setLeft(qMax(leftMax, screenArea.x())); - else if (oldClientGeometry.x() >= oldLeftMax && newGeom.x() + border[Left] < leftMax) { - newGeom.setLeft(qMax(leftMax, screenArea.x()) - border[Left]); - if (screens()->intersecting(newGeom) > 1) - newGeom.setLeft(newGeom.left() + border[Left]); - } - if (save[Bottom] || keep[Bottom]) - newGeom.moveBottom(qMin(bottomMax - 1, screenArea.bottom()) + padding[3]); - if (padding[3] && screens()->intersecting(newGeom) > 1) - newGeom.moveBottom(newGeom.bottom() - padding[3]); if (oldGeometry.y() >= oldTopMax && newGeom.y() < topMax) newGeom.setTop(qMax(topMax, screenArea.y())); - else if (oldClientGeometry.y() >= oldTopMax && newGeom.y() + border[Top] < topMax) { - newGeom.setTop(qMax(topMax, screenArea.y()) - border[Top]); - if (screens()->intersecting(newGeom) > 1) - newGeom.setTop(newGeom.top() + border[Top]); - } checkOffscreenPosition(&newGeom, screenArea); // Obey size hints. TODO: We really should make sure it stays in the right place diff --git a/src/abstract_client.h b/src/abstract_client.h index 66f8df3343..1dfd64b1d8 100644 --- a/src/abstract_client.h +++ b/src/abstract_client.h @@ -583,7 +583,7 @@ public: * The default implementation returns @c false. */ virtual bool dockWantsInput() const; - void checkWorkspacePosition(QRect oldGeometry = QRect(), QRect oldClientGeometry = QRect(), const VirtualDesktop *oldDesktop = nullptr); + void checkWorkspacePosition(QRect oldGeometry = QRect(), const VirtualDesktop *oldDesktop = nullptr); virtual xcb_timestamp_t userTime() const; virtual void updateWindowRules(Rules::Types selection); diff --git a/src/internal_client.cpp b/src/internal_client.cpp index 3be51ff27b..f5159ae64c 100644 --- a/src/internal_client.cpp +++ b/src/internal_client.cpp @@ -329,17 +329,15 @@ void InternalClient::updateDecoration(bool check_workspace_pos, bool force) return; } - const QRect oldFrameGeometry = frameGeometry(); - const QRect oldClientGeometry = oldFrameGeometry - frameMargins(); - GeometryUpdatesBlocker blocker(this); + const QRect oldFrameGeometry = frameGeometry(); if (force) { destroyDecoration(); } if (!noBorder()) { - createDecoration(oldClientGeometry); + createDecoration(oldFrameGeometry); } else { destroyDecoration(); } @@ -347,7 +345,7 @@ void InternalClient::updateDecoration(bool check_workspace_pos, bool force) updateShadow(); if (check_workspace_pos) { - checkWorkspacePosition(oldFrameGeometry, oldClientGeometry); + checkWorkspacePosition(oldFrameGeometry); } } diff --git a/src/placement.cpp b/src/placement.cpp index 3a7782938e..5b0fd493c2 100644 --- a/src/placement.cpp +++ b/src/placement.cpp @@ -100,33 +100,6 @@ void Placement::place(AbstractClient *c, const QRect &area, Policy policy, Polic default: placeSmart(c, area, nextPlacement); } - - if (options->borderSnapZone()) { - // snap to titlebar / snap to window borders on inner screen edges - const QRect geo(c->moveResizeGeometry()); - QPoint corner = geo.topLeft(); - const QMargins frameMargins = c->frameMargins(); - Qt::Edge titlePos = c->titlebarPosition(); - - const QRect fullRect = workspace()->clientArea(FullArea, c); - if (!(c->maximizeMode() & MaximizeHorizontal)) { - if (titlePos != Qt::RightEdge && geo.right() == fullRect.right()) { - corner.rx() += frameMargins.right(); - } - if (titlePos != Qt::LeftEdge && geo.left() == fullRect.left()) { - corner.rx() -= frameMargins.left(); - } - } - if (!(c->maximizeMode() & MaximizeVertical)) { - if (titlePos != Qt::BottomEdge && geo.bottom() == fullRect.bottom()) { - corner.ry() += frameMargins.bottom(); - } - if (titlePos != Qt::TopEdge && geo.top() == fullRect.top()) { - corner.ry() -= frameMargins.top(); - } - } - c->move(corner); - } } /** @@ -863,14 +836,6 @@ int Workspace::packPositionLeft(const AbstractClient *client, int oldX, bool lef client, QPoint(client->frameGeometry().left() - 1, client->frameGeometry().center().y())).left(); } - if (client->titlebarPosition() != Qt::LeftEdge) { - const int right = newX - client->frameMargins().left(); - QRect frameGeometry = client->frameGeometry(); - frameGeometry.moveRight(right); - if (screens()->intersecting(frameGeometry) < 2) { - newX = right; - } - } if (oldX <= newX) { return oldX; } @@ -897,14 +862,6 @@ int Workspace::packPositionRight(const AbstractClient *client, int oldX, bool ri client, QPoint(client->frameGeometry().right() + 1, client->frameGeometry().center().y())).right(); } - if (client->titlebarPosition() != Qt::RightEdge) { - const int right = newX + client->frameMargins().right(); - QRect frameGeometry = client->frameGeometry(); - frameGeometry.moveRight(right); - if (screens()->intersecting(frameGeometry) < 2) { - newX = right; - } - } if (oldX >= newX) { return oldX; } @@ -931,14 +888,6 @@ int Workspace::packPositionUp(const AbstractClient *client, int oldY, bool topEd client, QPoint(client->frameGeometry().center().x(), client->frameGeometry().top() - 1)).top(); } - if (client->titlebarPosition() != Qt::TopEdge) { - const int top = newY - client->frameMargins().top(); - QRect frameGeometry = client->frameGeometry(); - frameGeometry.moveTop(top); - if (screens()->intersecting(frameGeometry) < 2) { - newY = top; - } - } if (oldY <= newY) { return oldY; } @@ -965,14 +914,6 @@ int Workspace::packPositionDown(const AbstractClient *client, int oldY, bool bot client, QPoint(client->frameGeometry().center().x(), client->frameGeometry().bottom() + 1)).bottom(); } - if (client->titlebarPosition() != Qt::BottomEdge) { - const int bottom = newY + client->frameMargins().bottom(); - QRect frameGeometry = client->frameGeometry(); - frameGeometry.moveBottom(bottom); - if (screens()->intersecting(frameGeometry) < 2) { - newY = bottom; - } - } if (oldY >= newY) { return oldY; } diff --git a/src/workspace.cpp b/src/workspace.cpp index e4e3e19728..718cb75d2f 100644 --- a/src/workspace.cpp +++ b/src/workspace.cpp @@ -1284,7 +1284,7 @@ void Workspace::sendClientToDesktop(AbstractClient* c, int desk, bool dont_activ } else raiseClient(c); - c->checkWorkspacePosition( QRect(), QRect(), VirtualDesktopManager::self()->desktopForX11Id(old_desktop) ); + c->checkWorkspacePosition( QRect(), VirtualDesktopManager::self()->desktopForX11Id(old_desktop) ); auto transients_stacking_order = ensureStackingOrder(c->transients()); for (auto it = transients_stacking_order.constBegin(); @@ -2491,42 +2491,22 @@ QPoint Workspace::adjustClientPosition(AbstractClient* c, QPoint pos, bool unres const int snapY = borderSnapZone.height() * snapAdjust; if (snapX || snapY) { QRect geo = c->frameGeometry(); - QMargins frameMargins = c->frameMargins(); - - // snap to titlebar / snap to window borders on inner screen edges - Qt::Edge titlePos = c->titlebarPosition(); - if (frameMargins.left() && (titlePos == Qt::LeftEdge || (c->maximizeMode() & MaximizeHorizontal) || - screens()->intersecting(geo.translated(maxRect.x() - (frameMargins.left() + geo.x()), 0)) > 1)) { - frameMargins.setLeft(0); - } - if (frameMargins.right() && (titlePos == Qt::RightEdge || (c->maximizeMode() & MaximizeHorizontal) || - screens()->intersecting(geo.translated(maxRect.right() + frameMargins.right() - geo.right(), 0)) > 1)) { - frameMargins.setRight(0); - } - if (frameMargins.top() && (titlePos == Qt::TopEdge || (c->maximizeMode() & MaximizeVertical) || - screens()->intersecting(geo.translated(0, maxRect.y() - (frameMargins.top() + geo.y()))) > 1)) { - frameMargins.setTop(0); - } - if (frameMargins.bottom() && (titlePos == Qt::BottomEdge || (c->maximizeMode() & MaximizeVertical) || - screens()->intersecting(geo.translated(0, maxRect.bottom() + frameMargins.bottom() - geo.bottom())) > 1)) { - frameMargins.setBottom(0); - } if ((sOWO ? (cx < xmin) : true) && (qAbs(xmin - cx) < snapX)) { deltaX = xmin - cx; - nx = xmin - frameMargins.left(); + nx = xmin; } if ((sOWO ? (rx > xmax) : true) && (qAbs(rx - xmax) < snapX) && (qAbs(xmax - rx) < deltaX)) { deltaX = rx - xmax; - nx = xmax - cw + frameMargins.right(); + nx = xmax - cw; } if ((sOWO ? (cy < ymin) : true) && (qAbs(ymin - cy) < snapY)) { deltaY = ymin - cy; - ny = ymin - frameMargins.top(); + ny = ymin; } if ((sOWO ? (ry > ymax) : true) && (qAbs(ry - ymax) < snapY) && (qAbs(ymax - ry) < deltaY)) { deltaY = ry - ymax; - ny = ymax - ch + frameMargins.bottom(); + ny = ymax - ch; } } diff --git a/src/x11client.cpp b/src/x11client.cpp index 46bf910285..15c2e967e7 100644 --- a/src/x11client.cpp +++ b/src/x11client.cpp @@ -1105,7 +1105,6 @@ void X11Client::updateDecoration(bool check_workspace_pos, bool force) ((!isDecorated() && noBorder()) || (isDecorated() && !noBorder()))) return; QRect oldgeom = frameGeometry(); - QRect oldClientGeom = oldgeom.adjusted(borderLeft(), borderTop(), -borderRight(), -borderBottom()); blockGeometryUpdates(true); if (force) destroyDecoration(); @@ -1115,7 +1114,7 @@ void X11Client::updateDecoration(bool check_workspace_pos, bool force) destroyDecoration(); updateShadow(); if (check_workspace_pos) - checkWorkspacePosition(oldgeom, oldClientGeom); + checkWorkspacePosition(oldgeom); updateInputWindow(); blockGeometryUpdates(false); updateFrameExtents();