From 4050ee6ac7da0e5e7414c699c3cd4e26193c653d Mon Sep 17 00:00:00 2001 From: Gatis Paeglis Date: Mon, 25 Jun 2018 10:32:07 +0200 Subject: xcb: rely on WM_SIZE_HINTS gravity to handle x,y positioning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original code was added by b316c3ac5e4acac75505bfd77677cecc181599a (in 2012). This patch changes two things: 1) We now rely on WM to position a window based on the set gravity. It should not be necessary to calculate coordinates manually as was done in windowToWmGeometry(). We don't even know the decoration size before the window is mapped. 2) We now update gravity whenever needed instead of hardcoding based on what Qt APIs (setGeometry vs setFramePosition) where used to set the initial window position. The patch from b316c3a says: "Determine gravity from initial position. Do not change later as it will cause the window to move uncontrollably" Since it did not elaborate on the situation, we can only assume that it was caused by another bug in Qt at the time or perhaps a broken WM. From [1]: "Applications are free to change their win_gravity setting at any time. If an Application changes its win_gravity then the Window Manager should adjust the reference point, so that the client window will not move as the result." Tested on Ubuntu/Unity, KDE/KWin, Gnome-shell/Mutter, Lubuntu/OpenBox. Works as expected everywhere expect Unity. Unity seems to ignore XCB_GRAVITY_STATIC and treats it as XCB_GRAVITY_NORTH_WEST, which means that setGeometry/setFramePosition produce the same placement on this WM (the behavior was the same also before this patch). P.S. Also renamed xRect -> rect, which was a leftover from ae5f2a66720a4bb22c120bd7d1564652cac00367 With this change we can un-blacklist QWidget save/restore geometry auto tests. [1] https://specifications.freedesktop.org/wm-spec/latest/ar01s09.html Task-number: QTBUG-66708 Change-Id: I381eef5d34dddb04de16a897ce5540b9c430b216 Reviewed-by: Tor Arne Vestbø Reviewed-by: Gatis Paeglis --- src/plugins/platforms/xcb/qxcbwindow.cpp | 63 ++++++++++---------------------- src/plugins/platforms/xcb/qxcbwindow.h | 2 - 2 files changed, 20 insertions(+), 45 deletions(-) (limited to 'src/plugins/platforms/xcb') diff --git a/src/plugins/platforms/xcb/qxcbwindow.cpp b/src/plugins/platforms/xcb/qxcbwindow.cpp index d201dc59d6..7297887559 100644 --- a/src/plugins/platforms/xcb/qxcbwindow.cpp +++ b/src/plugins/platforms/xcb/qxcbwindow.cpp @@ -196,11 +196,6 @@ void QXcbWindow::setImageFormatForVisual(const xcb_visualtype_t *visual) } } -static inline bool positionIncludesFrame(QWindow *w) -{ - return qt_window_private(w)->positionPolicy == QWindowPrivate::WindowFrameInclusive; -} - #if QT_CONFIG(xcb_xlib) static inline XTextProperty* qstringToXTP(Display *dpy, const QString& s) { @@ -330,9 +325,6 @@ void QXcbWindow::create() return; } - // Parameters to XCreateWindow() are frame corner + inner size. - // This fits in case position policy is frame inclusive. There is - // currently no way to implement it for frame-exclusive geometries. QPlatformWindow::setGeometry(rect); if (platformScreen != currentScreen) @@ -619,25 +611,23 @@ void QXcbWindow::setGeometry(const QRect &rect) if (!newScreen) newScreen = xcbScreen(); - const QRect wmGeometry = windowToWmGeometry(rect); - if (newScreen != currentScreen) QWindowSystemInterface::handleWindowScreenChanged(window(), newScreen->QPlatformScreen::screen()); if (qt_window_private(window())->positionAutomatic) { const quint32 mask = XCB_CONFIG_WINDOW_WIDTH | XCB_CONFIG_WINDOW_HEIGHT; const qint32 values[] = { - qBound(1, wmGeometry.width(), XCOORD_MAX), - qBound(1, wmGeometry.height(), XCOORD_MAX), + qBound(1, rect.width(), XCOORD_MAX), + qBound(1, rect.height(), XCOORD_MAX), }; xcb_configure_window(xcb_connection(), m_window, mask, reinterpret_cast(values)); } else { const quint32 mask = XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y | XCB_CONFIG_WINDOW_WIDTH | XCB_CONFIG_WINDOW_HEIGHT; const qint32 values[] = { - qBound(-XCOORD_MAX, wmGeometry.x(), XCOORD_MAX), - qBound(-XCOORD_MAX, wmGeometry.y(), XCOORD_MAX), - qBound(1, wmGeometry.width(), XCOORD_MAX), - qBound(1, wmGeometry.height(), XCOORD_MAX), + qBound(-XCOORD_MAX, rect.x(), XCOORD_MAX), + qBound(-XCOORD_MAX, rect.y(), XCOORD_MAX), + qBound(1, rect.width(), XCOORD_MAX), + qBound(1, rect.height(), XCOORD_MAX), }; xcb_configure_window(xcb_connection(), m_window, mask, reinterpret_cast(values)); if (window()->parent() && !window()->transientParent()) { @@ -760,9 +750,6 @@ void QXcbWindow::show() xcb_set_wm_hints(xcb_connection(), m_window, &hints); - m_gravity = positionIncludesFrame(window()) ? - XCB_GRAVITY_NORTH_WEST : XCB_GRAVITY_STATIC; - // update WM_NORMAL_HINTS propagateSizeHints(); @@ -1513,38 +1500,28 @@ void QXcbWindow::lower() xcb_configure_window(xcb_connection(), m_window, mask, values); } -// Adapt the geometry to match the WM expection with regards -// to gravity. -QRect QXcbWindow::windowToWmGeometry(QRect r) const -{ - if (m_dirtyFrameMargins || m_frameMargins.isNull()) - return r; - const bool frameInclusive = positionIncludesFrame(window()); - // XCB_GRAVITY_STATIC requires the inner geometry, whereas - // XCB_GRAVITY_NORTH_WEST requires the frame geometry - if (frameInclusive && m_gravity == XCB_GRAVITY_STATIC) { - r.translate(m_frameMargins.left(), m_frameMargins.top()); - } else if (!frameInclusive && m_gravity == XCB_GRAVITY_NORTH_WEST) { - r.translate(-m_frameMargins.left(), -m_frameMargins.top()); - } - return r; -} - void QXcbWindow::propagateSizeHints() { // update WM_NORMAL_HINTS xcb_size_hints_t hints; memset(&hints, 0, sizeof(hints)); - const QRect xRect = windowToWmGeometry(geometry()); + const QRect rect = geometry(); + QWindowPrivate *win = qt_window_private(window()); + + if (!win->positionAutomatic) + xcb_size_hints_set_position(&hints, true, rect.x(), rect.y()); + if (rect.width() < QWINDOWSIZE_MAX || rect.height() < QWINDOWSIZE_MAX) + xcb_size_hints_set_size(&hints, true, rect.width(), rect.height()); - QWindow *win = window(); + /* Gravity describes how to interpret x and y values the next time + window needs to be positioned on a screen. + XCB_GRAVITY_STATIC : the left top corner of the client window + XCB_GRAVITY_NORTH_WEST : the left top corner of the frame window */ + auto gravity = win->positionPolicy == QWindowPrivate::WindowFrameInclusive + ? XCB_GRAVITY_NORTH_WEST : XCB_GRAVITY_STATIC; - if (!qt_window_private(win)->positionAutomatic) - xcb_size_hints_set_position(&hints, true, xRect.x(), xRect.y()); - if (xRect.width() < QWINDOWSIZE_MAX || xRect.height() < QWINDOWSIZE_MAX) - xcb_size_hints_set_size(&hints, true, xRect.width(), xRect.height()); - xcb_size_hints_set_win_gravity(&hints, m_gravity); + xcb_size_hints_set_win_gravity(&hints, gravity); QSize minimumSize = windowMinimumSize(); QSize maximumSize = windowMaximumSize(); diff --git a/src/plugins/platforms/xcb/qxcbwindow.h b/src/plugins/platforms/xcb/qxcbwindow.h index b44ba77a6b..d75f22bb51 100644 --- a/src/plugins/platforms/xcb/qxcbwindow.h +++ b/src/plugins/platforms/xcb/qxcbwindow.h @@ -253,8 +253,6 @@ protected: Qt::WindowStates m_windowState = Qt::WindowNoState; - xcb_gravity_t m_gravity = XCB_GRAVITY_STATIC; - bool m_mapped = false; bool m_transparent = false; bool m_usingSyncProtocol = false; -- cgit v1.2.3