AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

widgets: fix QWidget::save/restoreGeometry()

Browse Source 
QWidget::restoreGeometry() is calling QWidget::move() with restoredFrameGeometry,
which internally calls setGeometry() and sets positionPolicy = QWindowPrivate::WindowFrameExclusive,
which is invalid: restoredFrameGeometry is WindowFrameInclusive geometry.
QPA plugins rely on correctly set policies when interpreting x,y. QWidget::move()
was not designed for this AFAICT, so making it to accept frame geometry is
no-op. It is widely used legacy code, changing it could cause regressions.

Save/restore API was introduced in Qt 4.2, at that time we did not have APIs
like QWindow::setFramePosition(), so its unclear why geometry() was not stored
instead. The documentation also is somewhat unclear:

"[..] save the geometry when the window closes [..]"

Frame or client geometry? It does not specify. And from the code we see
that frame geometry was passed as client geometry, not making the original
intention clearer. Besides that, restoreGeometry() is full of other undocumented
assumptions where to place windows and when to fail (fortunately its easy
to write your own save/restore logic). Added a Qt 6 note in the source code.

What this patch changes:

Now we store geometry() in saveGeometry() and use that value in restoreGeometry()
by setGeometry(). This does not cause any behavior difference in window
positioning (tst_QWidget::saveRestoreGeometry still works). Geometry restored
from data saved with earlier versions of saveGeometry() might be positioned at:
x + leftMargin, y + topMargin.

This patch makes tst_QWidget::saveRestoreGeometry to always fail instead
of being flaky. Blacklisting for XCB instead of selected distros.

Also enabled excluded code paths for XCB on tst_QDockWidget::restoreDockWidget().
It does not seem to be flaky, maybe it was in 2015, but lot of things have changed
since then.

Task-number: QTBUG-66708
Change-Id: Ic86a6fd091e2c71b7550b2f476386da704253cd4
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
This commit is contained in:
Gatis Paeglis 2018-06-25 10:50:12 +02:00
parent 0877ced980
commit 2f2bfc4e59
4 changed files with 24 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
src/widgets/kernel/qwidget.cpp
View File

@ -7356,7 +7356,8 @@ QByteArray QWidget::saveGeometry() const
// Version history:
// - Qt 4.2 - 4.8.6, 5.0 - 5.3 : Version 1.0
// - Qt 4.8.6 - today, 5.4 - today: Version 2.0, save screen width in addition to check for high DPI scaling.
quint16 majorVersion = 2;
// - Qt 5.12 - today : Version 3.0, save QWidget::geometry()
quint16 majorVersion = 3;
quint16 minorVersion = 0;
const int screenNumber = QDesktopWidgetPrivate::screenNumber(this);
stream << magicNumber
@ -7372,7 +7373,8 @@ QByteArray QWidget::saveGeometry() const
<< qint32(screenNumber)
<< quint8(windowState() & Qt::WindowMaximized)
<< quint8(windowState() & Qt::WindowFullScreen)
<< qint32(QDesktopWidgetPrivate::screenGeometry(screenNumber).width()); // 1.1 onwards
<< qint32(QDesktopWidgetPrivate::screenGeometry(screenNumber).width()) // added in 2.0
<< geometry(); // added in 3.0
return array;
}
@ -7412,7 +7414,7 @@ bool QWidget::restoreGeometry(const QByteArray &geometry)
if (storedMagicNumber != magicNumber)
return false;
const quint16 currentMajorVersion = 2;
const quint16 currentMajorVersion = 3;
quint16 majorVersion = 0;
quint16 minorVersion = 0;
@ -7423,7 +7425,8 @@ bool QWidget::restoreGeometry(const QByteArray &geometry)
// (Allow all minor versions.)
QRect restoredFrameGeometry;
QRect restoredNormalGeometry;
QRect restoredGeometry;
QRect restoredNormalGeometry;
qint32 restoredScreenNumber;
quint8 maximized;
quint8 fullScreen;
@ -7437,6 +7440,10 @@ bool QWidget::restoreGeometry(const QByteArray &geometry)
if (majorVersion > 1)
stream >> restoredScreenWidth;
if (majorVersion > 2)
stream >> restoredGeometry;
// ### Qt 6 - Perhaps it makes sense to dumb down the restoreGeometry() logic, see QTBUG-69104
if (restoredScreenNumber >= QDesktopWidgetPrivate::numScreens())
restoredScreenNumber = QDesktopWidgetPrivate::primaryScreen();
@ -7523,14 +7530,11 @@ bool QWidget::restoreGeometry(const QByteArray &geometry)
setWindowState(ws);
d_func()->topData()->normalGeometry = restoredNormalGeometry;
} else {
QPoint offset;
#if 0 // Used to be included in Qt4 for Q_WS_X11
if (isFullScreen())
offset = d_func()->topData()->fullScreenOffset;
#endif
setWindowState(windowState() & ~(Qt::WindowMaximized | Qt::WindowFullScreen));
move(restoredFrameGeometry.topLeft() + offset);
resize(restoredNormalGeometry.size());
if (majorVersion > 2)
setGeometry(restoredGeometry);
else
setGeometry(restoredNormalGeometry);
}
return true;
}

3
tests/auto/widgets/kernel/qwidget/BLACKLIST
View File

@ -2,10 +2,9 @@
[normalGeometry]
ubuntu-16.04
[saveRestoreGeometry]
ubuntu-16.04
b2qt
# QTBUG-66708
opensuse
xcb
[restoreVersion1Geometry]
xcb
osx

16
tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp
View File

@ -3356,6 +3356,7 @@ void tst_QWidget::restoreVersion1Geometry()
QFETCH(QString, fileName);
QFETCH(uint, expectedWindowState);
QFETCH(QPoint, expectedPosition);
Q_UNUSED(expectedPosition);
QFETCH(QSize, expectedSize);
QFETCH(QRect, expectedNormalGeometry);
@ -3375,9 +3376,10 @@ void tst_QWidget::restoreVersion1Geometry()
QCOMPARE(uint(widget.windowState() & WindowStateMask), expectedWindowState);
if (expectedWindowState == Qt::WindowNoState) {
QCOMPARE(widget.pos(), expectedPosition);
QTRY_COMPARE(widget.geometry(), expectedNormalGeometry);
QCOMPARE(widget.size(), expectedSize);
}
widget.showNormal();
QVERIFY(QTest::qWaitForWindowExposed(&widget));
QTest::qWait(100);
@ -3386,20 +3388,16 @@ void tst_QWidget::restoreVersion1Geometry()
QEXPECT_FAIL("", "WinRT does not support restoreGeometry", Abort);
if (expectedWindowState == Qt::WindowNoState) {
QTRY_COMPARE(widget.pos(), expectedPosition);
QTRY_COMPARE(widget.size(), expectedSize);
QCOMPARE(widget.geometry(), expectedNormalGeometry);
}
widget.showNormal();
QTest::qWait(10);
if (expectedWindowState != Qt::WindowNoState) {
// restoring from maximized or fullscreen, we can only restore to the normal geometry
QTRY_COMPARE(widget.geometry(), expectedNormalGeometry);
} else {
QTRY_COMPARE(widget.pos(), expectedPosition);
QTRY_COMPARE(widget.size(), expectedSize);
}
QTRY_COMPARE(widget.geometry(), expectedNormalGeometry);
if (expectedWindowState == Qt::WindowNoState)
QCOMPARE(widget.size(), expectedSize);
#if 0
// Code for saving a new geometry*.dat files

7
tests/auto/widgets/widgets/qdockwidget/tst_qdockwidget.cpp
View File

@ -781,8 +781,6 @@ void tst_QDockWidget::restoreDockWidget()
QByteArray geometry;
QByteArray state;
const bool isXcb = !QGuiApplication::platformName().compare("xcb", Qt::CaseInsensitive);
const QString name = QStringLiteral("main");
const QRect availableGeometry = QGuiApplication::primaryScreen()->availableGeometry();
const QSize size = availableGeometry.size() / 5;
@ -815,8 +813,7 @@ void tst_QDockWidget::restoreDockWidget()
dock->show();
QVERIFY(QTest::qWaitForWindowExposed(dock));
QTRY_VERIFY(dock->isFloating());
if (!isXcb) // Avoid Window manager positioning issues
QTRY_COMPARE(dock->pos(), dockPos);
QTRY_COMPARE(dock->pos(), dockPos);
}
QVERIFY(!geometry.isEmpty());
@ -837,8 +834,6 @@ void tst_QDockWidget::restoreDockWidget()
restoreWindow.show();
QVERIFY(QTest::qWaitForWindowExposed(&restoreWindow));
QTRY_VERIFY(dock->isFloating());
if (isXcb)
QSKIP("Skip due to Window manager positioning issues", Abort);
QTRY_COMPARE(dock->pos(), dockPos);
}
}