From 7cdcc267220db301080e7ade8e2f21e530e77b82 Mon Sep 17 00:00:00 2001 From: Jan-Arve Saether Date: Wed, 23 May 2012 14:58:55 +0200 Subject: [PATCH] Fix Qt 5 todo issues for QSizePolicy. * Merge the two public ctors. * Use bitflags instead of shifting bits (more readable). * Add autotest * Use int datatype for the "stretch setters". (values out of bounds are clamped) Streaming to QDataStream will still use the Qt 4 format. Task-number: QTBUG-25100 Change-Id: Iecb1e78cb12717e4d84448484c3ad8ca469d571a Reviewed-by: Paul Olav Tvete --- src/widgets/kernel/qlayout.cpp | 38 +++- src/widgets/kernel/qsizepolicy.h | 82 ++++---- src/widgets/kernel/qsizepolicy.qdoc | 8 +- tests/auto/widgets/kernel/kernel.pro | 1 + .../widgets/kernel/qsizepolicy/qsizepolicy.pro | 6 + .../widgets/kernel/qsizepolicy/tst_qsizepolicy.cpp | 224 +++++++++++++++++++++ 6 files changed, 299 insertions(+), 60 deletions(-) create mode 100644 tests/auto/widgets/kernel/qsizepolicy/qsizepolicy.pro create mode 100644 tests/auto/widgets/kernel/qsizepolicy/tst_qsizepolicy.cpp diff --git a/src/widgets/kernel/qlayout.cpp b/src/widgets/kernel/qlayout.cpp index 9a638b0..fc49689 100644 --- a/src/widgets/kernel/qlayout.cpp +++ b/src/widgets/kernel/qlayout.cpp @@ -1419,17 +1419,17 @@ void QSizePolicy::setControlType(ControlType type) Example: - 0x00000001 maps to 0x00000000 - 0x00000002 maps to 0x00000200 - 0x00000004 maps to 0x00000400 - 0x00000008 maps to 0x00000600 + 0x00000001 maps to 0 + 0x00000002 maps to 1 + 0x00000004 maps to 2 + 0x00000008 maps to 3 etc. */ int i = 0; while (true) { if (type & (0x1 << i)) { - data = (data & ~CTMask) | (i << CTShift); + bits.ctype = i; return; } ++i; @@ -1438,10 +1438,11 @@ void QSizePolicy::setControlType(ControlType type) QSizePolicy::ControlType QSizePolicy::controlType() const { - return QSizePolicy::ControlType(0x1 << ((data & CTMask) >> CTShift)); + return QSizePolicy::ControlType(1 << bits.ctype); } #ifndef QT_NO_DATASTREAM + /*! \relates QSizePolicy \since 4.2 @@ -1452,9 +1453,20 @@ QSizePolicy::ControlType QSizePolicy::controlType() const */ QDataStream &operator<<(QDataStream &stream, const QSizePolicy &policy) { - return stream << policy.data; + // The order here is for historical reasons. (compatibility with Qt4) + quint32 data = (policy.bits.horPolicy | // [0, 3] + policy.bits.verPolicy << 4 | // [4, 7] + policy.bits.hfw << 8 | // [8] + policy.bits.ctype << 9 | // [9, 13] + policy.bits.wfh << 14 | // [14] + //policy.bits.padding << 15 | // [15] + policy.bits.verStretch << 16 | // [16, 23] + policy.bits.horStretch << 24); // [24, 31] + return stream << data; } +#define VALUE_OF_BITS(data, bitstart, bitcount) ((data >> bitstart) & ((1 << bitcount) -1)) + /*! \relates QSizePolicy \since 4.2 @@ -1465,7 +1477,17 @@ QDataStream &operator<<(QDataStream &stream, const QSizePolicy &policy) */ QDataStream &operator>>(QDataStream &stream, QSizePolicy &policy) { - return stream >> policy.data; + quint32 data; + stream >> data; + policy.bits.horPolicy = VALUE_OF_BITS(data, 0, 4); + policy.bits.verPolicy = VALUE_OF_BITS(data, 4, 4); + policy.bits.hfw = VALUE_OF_BITS(data, 8, 1); + policy.bits.ctype = VALUE_OF_BITS(data, 9, 5); + policy.bits.wfh = VALUE_OF_BITS(data, 14, 1); + policy.bits.padding = 0; + policy.bits.verStretch = VALUE_OF_BITS(data, 16, 8); + policy.bits.horStretch = VALUE_OF_BITS(data, 24, 8); + return stream; } #endif // QT_NO_DATASTREAM diff --git a/src/widgets/kernel/qsizepolicy.h b/src/widgets/kernel/qsizepolicy.h index 5a3ef19..54ee78d 100644 --- a/src/widgets/kernel/qsizepolicy.h +++ b/src/widgets/kernel/qsizepolicy.h @@ -56,19 +56,6 @@ class Q_WIDGETS_EXPORT QSizePolicy Q_GADGET Q_ENUMS(Policy) -private: - enum SizePolicyMasks { - HSize = 4, - HMask = 0x0f, - VMask = HMask << HSize, - CTShift = 9, - CTSize = 5, - CTMask = ((0x1 << CTSize) - 1) << CTShift, - WFHShift = CTShift + CTSize, - UnusedShift = WFHShift + 1, - UnusedSize = 1 - }; - public: enum PolicyFlag { GrowFlag = 1, @@ -109,17 +96,18 @@ public: QSizePolicy() : data(0) { } // ### Qt 5: merge these two constructors (with type == DefaultType) - QSizePolicy(Policy horizontal, Policy vertical) - : data(horizontal | (vertical << HSize)) { } - QSizePolicy(Policy horizontal, Policy vertical, ControlType type) - : data(horizontal | (vertical << HSize)) { setControlType(type); } - - Policy horizontalPolicy() const { return static_cast(data & HMask); } - Policy verticalPolicy() const { return static_cast((data & VMask) >> HSize); } + QSizePolicy(Policy horizontal, Policy vertical, ControlType type = DefaultType) + : data(0) { + bits.horPolicy = horizontal; + bits.verPolicy = vertical; + setControlType(type); + } + Policy horizontalPolicy() const { return static_cast(bits.horPolicy); } + Policy verticalPolicy() const { return static_cast(bits.verPolicy); } ControlType controlType() const; - void setHorizontalPolicy(Policy d) { data = (data & ~HMask) | d; } - void setVerticalPolicy(Policy d) { data = (data & ~(HMask << HSize)) | (d << HSize); } + void setHorizontalPolicy(Policy d) { bits.horPolicy = d; } + void setVerticalPolicy(Policy d) { bits.verPolicy = d; } void setControlType(ControlType type); Qt::Orientations expandingDirections() const { @@ -131,19 +119,19 @@ public: return result; } - void setHeightForWidth(bool b) { data = b ? (data | (1 << 2*HSize)) : (data & ~(1 << 2*HSize)); } - bool hasHeightForWidth() const { return data & (1 << 2*HSize); } - void setWidthForHeight(bool b) { data = b ? (data | (1 << (WFHShift))) : (data & ~(1 << (WFHShift))); } - bool hasWidthForHeight() const { return data & (1 << (WFHShift)); } + void setHeightForWidth(bool b) { bits.hfw = b; } + bool hasHeightForWidth() const { return bits.hfw; } + void setWidthForHeight(bool b) { bits.wfh = b; } + bool hasWidthForHeight() const { return bits.wfh; } bool operator==(const QSizePolicy& s) const { return data == s.data; } bool operator!=(const QSizePolicy& s) const { return data != s.data; } - operator QVariant() const; // implemented in qabstractlayout.cpp + operator QVariant() const; // implemented in qlayoutitem.cpp - int horizontalStretch() const { return data >> 24; } - int verticalStretch() const { return (data >> 16) & 0xff; } - void setHorizontalStretch(uchar stretchFactor) { data = (data&0x00ffffff) | (uint(stretchFactor)<<24); } - void setVerticalStretch(uchar stretchFactor) { data = (data&0xff00ffff) | (uint(stretchFactor)<<16); } + int horizontalStretch() const { return static_cast(bits.horStretch); } + int verticalStretch() const { return static_cast(bits.verStretch); } + void setHorizontalStretch(int stretchFactor) { bits.horStretch = static_cast(qBound(0, stretchFactor, 255)); } + void setVerticalStretch(int stretchFactor) { bits.verStretch = static_cast(qBound(0, stretchFactor, 255)); } void transpose(); @@ -155,21 +143,19 @@ private: #endif QSizePolicy(int i) : data(i) { } - quint32 data; -/* Qt5: Use bit flags instead, keep it here for improved readability for now. - We can maybe change it for Qt4, but we'd have to be careful, since the behaviour - is implementation defined. It usually varies between little- and big-endian compilers, but - it might also not vary. - quint32 horzPolicy : 4; - quint32 vertPolicy : 4; - quint32 hfw : 1; - quint32 ctype : 5; - quint32 wfh : 1; - quint32 padding : 1; // we cannot use the highest bit - quint32 verStretch : 8; - quint32 horStretch : 8; -*/ - + union { + struct { + quint32 horStretch : 8; + quint32 verStretch : 8; + quint32 horPolicy : 4; + quint32 verPolicy : 4; + quint32 ctype : 5; + quint32 hfw : 1; + quint32 wfh : 1; + quint32 padding : 1; // feel free to use + } bits; + quint32 data; + }; }; Q_DECLARE_OPERATORS_FOR_FLAGS(QSizePolicy::ControlTypes) @@ -187,8 +173,8 @@ Q_WIDGETS_EXPORT QDebug operator<<(QDebug dbg, const QSizePolicy &); inline void QSizePolicy::transpose() { Policy hData = horizontalPolicy(); Policy vData = verticalPolicy(); - uchar hStretch = uchar(horizontalStretch()); - uchar vStretch = uchar(verticalStretch()); + int hStretch = horizontalStretch(); + int vStretch = verticalStretch(); setHorizontalPolicy(vData); setVerticalPolicy(hData); setHorizontalStretch(vStretch); diff --git a/src/widgets/kernel/qsizepolicy.qdoc b/src/widgets/kernel/qsizepolicy.qdoc index 670491e..3c7d1e8 100644 --- a/src/widgets/kernel/qsizepolicy.qdoc +++ b/src/widgets/kernel/qsizepolicy.qdoc @@ -319,19 +319,19 @@ */ /*! - \fn void QSizePolicy::setHorizontalStretch(uchar stretchFactor) + \fn void QSizePolicy::setHorizontalStretch(int stretchFactor) Sets the horizontal stretch factor of the size policy to the given \a - stretchFactor. + stretchFactor. \a stretchFactor must be in the range [0,255]. \sa horizontalStretch(), setVerticalStretch(), setHorizontalPolicy() */ /*! - \fn void QSizePolicy::setVerticalStretch(uchar stretchFactor) + \fn void QSizePolicy::setVerticalStretch(int stretchFactor) Sets the vertical stretch factor of the size policy to the given - \a stretchFactor. + \a stretchFactor. \a stretchFactor must be in the range [0,255]. \sa verticalStretch(), setHorizontalStretch(), setVerticalPolicy() */ diff --git a/tests/auto/widgets/kernel/kernel.pro b/tests/auto/widgets/kernel/kernel.pro index c2540ec..14ebda2 100644 --- a/tests/auto/widgets/kernel/kernel.pro +++ b/tests/auto/widgets/kernel/kernel.pro @@ -14,5 +14,6 @@ SUBDIRS=\ qwidget_window \ qwidgetaction \ qshortcut \ + qsizepolicy SUBDIRS -= qsound diff --git a/tests/auto/widgets/kernel/qsizepolicy/qsizepolicy.pro b/tests/auto/widgets/kernel/qsizepolicy/qsizepolicy.pro new file mode 100644 index 0000000..84629c7 --- /dev/null +++ b/tests/auto/widgets/kernel/qsizepolicy/qsizepolicy.pro @@ -0,0 +1,6 @@ +CONFIG += testcase +TARGET = tst_qsizepolicy + +QT += widgets widgets-private testlib + +SOURCES += tst_qsizepolicy.cpp diff --git a/tests/auto/widgets/kernel/qsizepolicy/tst_qsizepolicy.cpp b/tests/auto/widgets/kernel/qsizepolicy/tst_qsizepolicy.cpp new file mode 100644 index 0000000..7108c9d --- /dev/null +++ b/tests/auto/widgets/kernel/qsizepolicy/tst_qsizepolicy.cpp @@ -0,0 +1,224 @@ +/**************************************************************************** +** +** Copyright (C) 2012 Nokia Corporation and/or its subsidiary(-ies). +** Contact: http://www.qt-project.org/ +** +** This file is part of the test suite of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL$ +** GNU Lesser General Public License Usage +** This file may be used under the terms of the GNU Lesser General Public +** License version 2.1 as published by the Free Software Foundation and +** appearing in the file LICENSE.LGPL included in the packaging of this +** file. Please review the following information to ensure the GNU Lesser +** General Public License version 2.1 requirements will be met: +** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. +** +** In addition, as a special exception, Nokia gives you certain additional +** rights. These rights are described in the Nokia Qt LGPL Exception +** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU General +** Public License version 3.0 as published by the Free Software Foundation +** and appearing in the file LICENSE.GPL included in the packaging of this +** file. Please review the following information to ensure the GNU General +** Public License version 3.0 requirements will be met: +** http://www.gnu.org/copyleft/gpl.html. +** +** Other Usage +** Alternatively, this file may be used in accordance with the terms and +** conditions contained in a signed written agreement between you and Nokia. +** +** +** +** +** +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + + +#include +#include + +class tst_QSizePolicy : public QObject +{ +Q_OBJECT + +public: + tst_QSizePolicy(); + virtual ~tst_QSizePolicy(); + +private slots: + void getSetCheck(); + void dataStream(); + void horizontalStretch(); + void verticalStretch(); +}; + +tst_QSizePolicy::tst_QSizePolicy() +{ +} + +tst_QSizePolicy::~tst_QSizePolicy() +{ +} + + +// Testing get/set functions +void tst_QSizePolicy::getSetCheck() +{ + { + // check values of a default constructed QSizePolicy + QSizePolicy sp; + QCOMPARE(sp.horizontalPolicy(), QSizePolicy::Fixed); + QCOMPARE(sp.verticalPolicy(), QSizePolicy::Fixed); + QCOMPARE(sp.horizontalStretch(), 0); + QCOMPARE(sp.verticalStretch(), 0); + QCOMPARE(sp.verticalStretch(), 0); + QCOMPARE(sp.controlType(), QSizePolicy::DefaultType); + QCOMPARE(sp.hasHeightForWidth(), false); + QCOMPARE(sp.hasWidthForHeight(), false); + } + + { + static const QSizePolicy::Policy policies[3] = { + QSizePolicy::Fixed, + QSizePolicy::Minimum, + QSizePolicy::Ignored + }; + static const QSizePolicy::ControlType controlTypes[4] = { + QSizePolicy::DefaultType, + QSizePolicy::ButtonBox, + QSizePolicy::CheckBox, + QSizePolicy::ToolButton + }; + +#define ITEMCOUNT(arr) sizeof(arr)/sizeof(arr[0]) + QSizePolicy sp, oldsp; +#ifdef GENERATE_BASELINE + QFile out(QString::fromAscii("qsizepolicy-Qt%1%2.txt").arg((QT_VERSION >> 16) & 0xff).arg((QT_VERSION) >> 8 & 0xff)); + if (out.open(QIODevice::WriteOnly | QIODevice::Truncate)) { + QDataStream stream(&out); +#endif + /* Loop for permutating over the values most likely to trigger a bug: + - mininumum, maximum values + - Some values with LSB set, others with MSB unset. (check if shifts are ok) + + */ + // Look specifically for + for (int ihp = 0; ihp < ITEMCOUNT(policies); ++ihp) { + QSizePolicy::Policy hp = policies[ihp]; + for (int ivp = 0; ivp < ITEMCOUNT(policies); ++ivp) { + QSizePolicy::Policy vp = policies[ivp]; + for (int ict = 0; ict < ITEMCOUNT(controlTypes); ++ict) { + QSizePolicy::ControlType ct = controlTypes[ict]; + for (int hst= 0; hst <= 255; hst+=85) { //[0,85,170,255] + for (int vst = 0; vst <= 255; vst+=85) { + for (int j = 0; j < 3; ++j) { + bool hfw = j & 1; + bool wfh = j & 2; // cannot set hfw and wfh at the same time + oldsp = sp; + for (int i = 0; i < 5; ++i) { + switch (i) { + case 0: sp.setHorizontalPolicy(hp); break; + case 1: sp.setVerticalPolicy(vp); break; + case 2: sp.setHorizontalStretch(hst); break; + case 3: sp.setVerticalStretch(vst); break; + case 4: sp.setControlType(ct); break; + case 5: sp.setHeightForWidth(hfw); sp.setWidthForHeight(wfh); break; + default: break; + } + QCOMPARE(sp.horizontalPolicy(), (i >= 0 ? hp : oldsp.horizontalPolicy())); + QCOMPARE(sp.verticalPolicy(), (i >= 1 ? vp : oldsp.verticalPolicy())); + QCOMPARE(sp.horizontalStretch(), (i >= 2 ? hst : oldsp.horizontalStretch())); + QCOMPARE(sp.verticalStretch(), (i >= 3 ? vst : oldsp.verticalStretch())); + QCOMPARE(sp.controlType(), (i >= 4 ? ct : oldsp.controlType())); + QCOMPARE(sp.hasHeightForWidth(), (i >= 5 ? hfw : oldsp.hasHeightForWidth())); + QCOMPARE(sp.hasWidthForHeight(), (i >= 5 ? wfh : oldsp.hasWidthForHeight())); + + Qt::Orientations orients; + if (sp.horizontalPolicy() & QSizePolicy::ExpandFlag) + orients |= Qt::Horizontal; + if (sp.verticalPolicy() & QSizePolicy::ExpandFlag) + orients |= Qt::Vertical; + + QCOMPARE(sp.expandingDirections(), orients); +#ifdef GENERATE_BASELINE + stream << sp; +#endif + } + } + } + } + } + } + } +#ifdef GENERATE_BASELINE + out.close(); + } +#endif + } +} + +void tst_QSizePolicy::dataStream() +{ + QByteArray data; + QSizePolicy sp(QSizePolicy::Minimum, QSizePolicy::Expanding); + { + QDataStream stream(&data, QIODevice::ReadWrite); + sp.setHorizontalStretch(42); + sp.setVerticalStretch(10); + sp.setControlType(QSizePolicy::CheckBox); + sp.setHeightForWidth(true); + + stream << sp; // big endian +/* +| BYTE 0 | BYTE 1 | +| 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | ++-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+ +| Horizontal stretch | Vertical stretch | ++-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+ + +| BYTE 2 | BYTE 3 | +| 16 | 17 | 18 | 19 | 20 | 21 | 22 | 23 | 24 | 25 | 26 | 27 | 28 | 29 | 30 | 31 | ++-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+ +| pad | wfh | Control Type | hfw | Vertical policy | Horizontal policy | ++-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+ +*/ + QCOMPARE((char)data[0], char(42)); // h stretch + QCOMPARE((char)data[1], char(10)); // v stretch + QCOMPARE((char)data[2], char(1 | (2 << 1))); // (hfw + CheckBox) + QCOMPARE((char)data[3], char(QSizePolicy::Minimum | (QSizePolicy::Expanding << 4))); + } + + { + QSizePolicy readSP; + QDataStream stream(data); + stream >> readSP; + QCOMPARE(sp, readSP); + } +} + + +void tst_QSizePolicy::horizontalStretch() +{ + QSizePolicy sp; + sp.setHorizontalStretch(257); + QCOMPARE(sp.horizontalStretch(), 255); + sp.setHorizontalStretch(-2); + QCOMPARE(sp.horizontalStretch(), 0); +} + +void tst_QSizePolicy::verticalStretch() +{ + QSizePolicy sp; + sp.setVerticalStretch(-2); + QCOMPARE(sp.verticalStretch(), 0); + sp.setVerticalStretch(257); + QCOMPARE(sp.verticalStretch(), 255); +} +QTEST_MAIN(tst_QSizePolicy) +#include "tst_qsizepolicy.moc" -- 2.7.4