From 2556bfdab42dc0aefb34bb7cf304063c7db0ff00 Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Tue, 7 Feb 2017 13:54:29 +0100 Subject: [PATCH] Positioners: allow distinguishing between implicit/explicit child size In Qt Quick Controls 2, we plan on using positioners to layout an icon next to text in a button, for example. Consider the following example: AbstractButton { id: button text: "Button" contentItem: Row { Text { text: button.text width: parent.width } } background: Rectangle { radius: 5 color: "lightsteelblue" opacity: button.pressed ? 1.0 : 0.8 } } In Qt Quick Controls 2, implicit size propagates "up" from the delegates/building blocks to the control, whereas explicit size propagates "down" from the control to the delegates/building blocks. Providing a reasonable implicit size is important to make controls behave well in layouts, etc., and the internal building blocks must follow the size of the control to retain sensible looks when a control is resized. In the example above, contentItem needs to have a "natural" (implicit) size representing the ideal fit of the content, but it needs to respect the explicitly provided size from the control too. With the current behavior, as the explicit width of the Row is 0, the Text item (via the width binding) sets explicit width to 0, and Row uses that explicit width rather than the implicit width, thus, Row here will have an implicit width of 0, which is not what the control wants. This patch: - Allows subclasses of positioners to set QQuickBasePositionerPrivate::useImplicitSize to true in order to tell positioners to use implicit size rather than explicit size. This is not exposed as public API, as this behavior is typically not something desirable in the positioners themselves. For example, Row { Rectangle { width: 100; height: 100 } } would have an implicit size of 0, as Rectangle has no implicit size. - Adds QQuickImplicitRow and QQuickImplicitGrid, which are private subclasses of their respective positioners that simply set useImplicitSize to true in their constructors. - Exports the wrappers privately so that they can be registered by other modules as QML types. Change-Id: Ie68aabd7fbf6c76375badf6e338f2f238f3fc392 Reviewed-by: J-P Nurmi Reviewed-by: Robin Burchell --- src/quick/items/qquickpositioners.cpp | 100 ++++++++++++------ src/quick/items/qquickpositioners_p.h | 33 +++++- src/quick/items/qquickpositioners_p_p.h | 41 ++++++- .../data/implicitGridOneItem.qml | 12 +++ .../data/implicitRowOneItem.qml | 9 ++ .../tst_qquickpositioners.cpp | 44 ++++++++ 6 files changed, 198 insertions(+), 41 deletions(-) create mode 100644 tests/auto/quick/qquickpositioners/data/implicitGridOneItem.qml create mode 100644 tests/auto/quick/qquickpositioners/data/implicitRowOneItem.qml diff --git a/src/quick/items/qquickpositioners.cpp b/src/quick/items/qquickpositioners.cpp index 51eafd9217..be92bb0954 100644 --- a/src/quick/items/qquickpositioners.cpp +++ b/src/quick/items/qquickpositioners.cpp @@ -48,8 +48,19 @@ QT_BEGIN_NAMESPACE -static const QQuickItemPrivate::ChangeTypes watchedChanges - = QQuickItemPrivate::Geometry +// The default item change types that positioners are interested in. +static const QQuickItemPrivate::ChangeTypes explicitSizeItemChangeTypes = + QQuickItemPrivate::Geometry + | QQuickItemPrivate::SiblingOrder + | QQuickItemPrivate::Visibility + | QQuickItemPrivate::Destroyed; + +// The item change types for positioners that are only interested in the implicit +// size of the items they manage. These are used if useImplicitSize is true. +// useImplicitSize should be set in the constructor, before any items are added. +static const QQuickItemPrivate::ChangeTypes implicitSizeItemChangeTypes = + QQuickItemPrivate::ImplicitWidth + | QQuickItemPrivate::ImplicitHeight | QQuickItemPrivate::SiblingOrder | QQuickItemPrivate::Visibility | QQuickItemPrivate::Destroyed; @@ -57,13 +68,15 @@ static const QQuickItemPrivate::ChangeTypes watchedChanges void QQuickBasePositionerPrivate::watchChanges(QQuickItem *other) { QQuickItemPrivate *otherPrivate = QQuickItemPrivate::get(other); - otherPrivate->addItemChangeListener(this, watchedChanges); + otherPrivate->addItemChangeListener(this, useImplicitSize + ? implicitSizeItemChangeTypes : explicitSizeItemChangeTypes); } void QQuickBasePositionerPrivate::unwatchChanges(QQuickItem* other) { QQuickItemPrivate *otherPrivate = QQuickItemPrivate::get(other); - otherPrivate->removeItemChangeListener(this, watchedChanges); + otherPrivate->removeItemChangeListener(this, useImplicitSize + ? implicitSizeItemChangeTypes : explicitSizeItemChangeTypes); } @@ -323,7 +336,7 @@ void QQuickBasePositioner::prePositioning() if (wIdx < 0) { d->watchChanges(child); posItem.isNew = true; - if (!childPrivate->explicitVisible || !child->width() || !child->height()) { + if (!childPrivate->explicitVisible || !d->itemWidth(child) || !d->itemHeight(child)) { posItem.isVisible = false; posItem.index = -1; unpositionedItems.append(posItem); @@ -345,7 +358,7 @@ void QQuickBasePositioner::prePositioning() PositionedItem *item = &oldItems[wIdx]; // Items are only omitted from positioning if they are explicitly hidden // i.e. their positioning is not affected if an ancestor is hidden. - if (!childPrivate->explicitVisible || !child->width() || !child->height()) { + if (!childPrivate->explicitVisible || !d->itemWidth(child) || !d->itemHeight(child)) { item->isVisible = false; item->index = -1; unpositionedItems.append(*item); @@ -944,6 +957,7 @@ QQuickColumn::QQuickColumn(QQuickItem *parent) void QQuickColumn::doPositioning(QSizeF *contentSize) { //Precondition: All items in the positioned list have a valid item pointer and should be positioned + QQuickBasePositionerPrivate *d = static_cast(QQuickBasePositionerPrivate::get(this)); qreal voffset = topPadding(); const qreal padding = leftPadding() + rightPadding(); contentSize->setWidth(qMax(contentSize->width(), padding)); @@ -952,9 +966,9 @@ void QQuickColumn::doPositioning(QSizeF *contentSize) PositionedItem &child = positionedItems[ii]; positionItem(child.itemX() + leftPadding() - child.leftPadding, voffset, &child); child.updatePadding(leftPadding(), topPadding(), rightPadding(), bottomPadding()); - contentSize->setWidth(qMax(contentSize->width(), child.item->width() + padding)); + contentSize->setWidth(qMax(contentSize->width(), d->itemWidth(child.item) + padding)); - voffset += child.item->height(); + voffset += d->itemHeight(child.item); voffset += spacing(); } @@ -1222,9 +1236,9 @@ void QQuickRow::doPositioning(QSizeF *contentSize) hoffsets << hoffset; } - contentSize->setHeight(qMax(contentSize->height(), child.item->height() + padding)); + contentSize->setHeight(qMax(contentSize->height(), d->itemHeight(child.item) + padding)); - hoffset += child.item->width(); + hoffset += d->itemWidth(child.item); hoffset += spacing(); } @@ -1245,7 +1259,7 @@ void QQuickRow::doPositioning(QSizeF *contentSize) int acc = 0; for (int ii = 0; ii < positionedItems.count(); ++ii) { PositionedItem &child = positionedItems[ii]; - hoffset = end - hoffsets[acc++] - child.item->width(); + hoffset = end - hoffsets[acc++] - d->itemWidth(child.item); positionItem(hoffset, child.itemY() + topPadding() - child.topPadding, &child); child.updatePadding(leftPadding(), topPadding(), rightPadding(), bottomPadding()); } @@ -1746,10 +1760,12 @@ void QQuickGrid::doPositioning(QSizeF *contentSize) break; const PositionedItem &child = positionedItems.at(childIndex++); - if (child.item->width() > maxColWidth[j]) - maxColWidth[j] = child.item->width(); - if (child.item->height() > maxRowHeight[i]) - maxRowHeight[i] = child.item->height(); + const qreal childWidth = d->itemWidth(child.item); + const qreal childHeight = d->itemHeight(child.item); + if (childWidth > maxColWidth[j]) + maxColWidth[j] = childWidth; + if (childHeight > maxRowHeight[i]) + maxRowHeight[i] = childHeight; } } } else { @@ -1764,10 +1780,12 @@ void QQuickGrid::doPositioning(QSizeF *contentSize) break; const PositionedItem &child = positionedItems.at(childIndex++); - if (child.item->width() > maxColWidth[j]) - maxColWidth[j] = child.item->width(); - if (child.item->height() > maxRowHeight[i]) - maxRowHeight[i] = child.item->height(); + const qreal childWidth = d->itemWidth(child.item); + const qreal childHeight = d->itemHeight(child.item); + if (childWidth > maxColWidth[j]) + maxColWidth[j] = childWidth; + if (childHeight > maxRowHeight[i]) + maxRowHeight[i] = childHeight; } } } @@ -1809,20 +1827,22 @@ void QQuickGrid::doPositioning(QSizeF *contentSize) for (int i = 0; i < positionedItems.count(); ++i) { PositionedItem &child = positionedItems[i]; qreal childXOffset = xoffset; + const qreal childWidth = d->itemWidth(child.item); + const qreal childHeight = d->itemHeight(child.item); if (effectiveHAlign() == AlignRight) - childXOffset += maxColWidth[curCol] - child.item->width(); + childXOffset += maxColWidth[curCol] - childWidth; else if (hItemAlign() == AlignHCenter) - childXOffset += (maxColWidth[curCol] - child.item->width())/2.0; + childXOffset += (maxColWidth[curCol] - childWidth)/2.0; if (!d->isLeftToRight()) childXOffset -= maxColWidth[curCol]; qreal alignYOffset = yoffset; if (m_vItemAlign == AlignVCenter) - alignYOffset += (maxRowHeight[curRow] - child.item->height())/2.0; + alignYOffset += (maxRowHeight[curRow] - childHeight)/2.0; else if (m_vItemAlign == AlignBottom) - alignYOffset += maxRowHeight[curRow] - child.item->height(); + alignYOffset += maxRowHeight[curRow] - childHeight; positionItem(childXOffset, alignYOffset, &child); child.updatePadding(leftPadding(), topPadding(), rightPadding(), bottomPadding()); @@ -2140,15 +2160,17 @@ void QQuickFlow::doPositioning(QSizeF *contentSize) for (int i = 0; i < positionedItems.count(); ++i) { PositionedItem &child = positionedItems[i]; + const qreal childWidth = d->itemWidth(child.item); + const qreal childHeight = d->itemHeight(child.item); if (d->flow == LeftToRight) { - if (widthValid() && hoffset != hoffset1 && hoffset + child.item->width() + hoffset2 > width()) { + if (widthValid() && hoffset != hoffset1 && hoffset + childWidth + hoffset2 > width()) { hoffset = hoffset1; voffset += linemax + spacing(); linemax = 0; } } else { - if (heightValid() && voffset != voffset1 && voffset + child.item->height() + bottomPadding() > height()) { + if (heightValid() && voffset != voffset1 && voffset + childHeight + bottomPadding() > height()) { voffset = voffset1; hoffset += linemax + spacing(); linemax = 0; @@ -2165,17 +2187,17 @@ void QQuickFlow::doPositioning(QSizeF *contentSize) child.bottomPadding = bottomPadding(); } - contentSize->setWidth(qMax(contentSize->width(), hoffset + child.item->width() + hoffset2)); - contentSize->setHeight(qMax(contentSize->height(), voffset + child.item->height() + bottomPadding())); + contentSize->setWidth(qMax(contentSize->width(), hoffset + childWidth + hoffset2)); + contentSize->setHeight(qMax(contentSize->height(), voffset + childHeight + bottomPadding())); if (d->flow == LeftToRight) { - hoffset += child.item->width(); + hoffset += childWidth; hoffset += spacing(); - linemax = qMax(linemax, child.item->height()); + linemax = qMax(linemax, childHeight); } else { - voffset += child.item->height(); + voffset += childHeight; voffset += spacing(); - linemax = qMax(linemax, child.item->width()); + linemax = qMax(linemax, childWidth); } } @@ -2190,7 +2212,7 @@ void QQuickFlow::doPositioning(QSizeF *contentSize) int acc = 0; for (int i = 0; i < positionedItems.count(); ++i) { PositionedItem &child = positionedItems[i]; - hoffset = end - hoffsets[acc++] - child.item->width(); + hoffset = end - hoffsets[acc++] - d->itemWidth(child.item); positionItemX(hoffset, &child); child.leftPadding = leftPadding(); child.rightPadding = rightPadding(); @@ -2214,4 +2236,18 @@ void QQuickFlow::reportConflictingAnchors() qmlWarning(this) << "Cannot specify anchors for items inside Flow." << " Flow will not function."; } +QQuickImplicitRow::QQuickImplicitRow(QQuickItem *parent) + : QQuickRow(parent) +{ + QQuickBasePositionerPrivate *d = QQuickBasePositioner::get(this); + d->useImplicitSize = true; +} + +QQuickImplicitGrid::QQuickImplicitGrid(QQuickItem *parent) + : QQuickGrid(parent) +{ + QQuickBasePositionerPrivate *d = QQuickBasePositioner::get(this); + d->useImplicitSize = true; +} + QT_END_NAMESPACE diff --git a/src/quick/items/qquickpositioners_p.h b/src/quick/items/qquickpositioners_p.h index 9ae7029d69..8dc0d90a2f 100644 --- a/src/quick/items/qquickpositioners_p.h +++ b/src/quick/items/qquickpositioners_p.h @@ -132,6 +132,11 @@ public: static QQuickPositionerAttached *qmlAttachedProperties(QObject *obj); + static QQuickBasePositionerPrivate* get(QQuickBasePositioner *positioner) + { + return positioner->d_func(); + } + void updateAttachedProperties(QQuickPositionerAttached *specificProperty = 0, QQuickItem *specificPropertyOwner = 0) const; qreal padding() const; @@ -182,7 +187,7 @@ protected: virtual void doPositioning(QSizeF *contentSize)=0; virtual void reportConflictingAnchors()=0; - class PositionedItem + class Q_QUICK_PRIVATE_EXPORT PositionedItem { public : PositionedItem(QQuickItem *i); @@ -227,7 +232,7 @@ private: Q_DECLARE_PRIVATE(QQuickBasePositioner) }; -class Q_AUTOTEST_EXPORT QQuickColumn : public QQuickBasePositioner +class Q_QUICK_PRIVATE_EXPORT QQuickColumn : public QQuickBasePositioner { Q_OBJECT public: @@ -241,7 +246,7 @@ private: }; class QQuickRowPrivate; -class Q_AUTOTEST_EXPORT QQuickRow: public QQuickBasePositioner +class Q_QUICK_PRIVATE_EXPORT QQuickRow: public QQuickBasePositioner { Q_OBJECT Q_PROPERTY(Qt::LayoutDirection layoutDirection READ layoutDirection WRITE setLayoutDirection NOTIFY layoutDirectionChanged) @@ -266,7 +271,7 @@ private: }; class QQuickGridPrivate; -class Q_AUTOTEST_EXPORT QQuickGrid : public QQuickBasePositioner +class Q_QUICK_PRIVATE_EXPORT QQuickGrid : public QQuickBasePositioner { Q_OBJECT Q_PROPERTY(int rows READ rows WRITE setRows NOTIFY rowsChanged) @@ -353,7 +358,7 @@ private: }; class QQuickFlowPrivate; -class Q_AUTOTEST_EXPORT QQuickFlow: public QQuickBasePositioner +class Q_QUICK_PRIVATE_EXPORT QQuickFlow: public QQuickBasePositioner { Q_OBJECT Q_PROPERTY(Flow flow READ flow WRITE setFlow NOTIFY flowChanged) @@ -386,6 +391,22 @@ private: Q_DECLARE_PRIVATE(QQuickFlow) }; +class Q_QUICK_PRIVATE_EXPORT QQuickImplicitRow : public QQuickRow +{ + Q_OBJECT + +public: + QQuickImplicitRow(QQuickItem *parent = nullptr); +}; + +class Q_QUICK_PRIVATE_EXPORT QQuickImplicitGrid : public QQuickGrid +{ + Q_OBJECT + +public: + QQuickImplicitGrid(QQuickItem *parent = nullptr); +}; + QT_END_NAMESPACE @@ -393,6 +414,8 @@ QML_DECLARE_TYPE(QQuickColumn) QML_DECLARE_TYPE(QQuickRow) QML_DECLARE_TYPE(QQuickGrid) QML_DECLARE_TYPE(QQuickFlow) +QML_DECLARE_TYPE(QQuickImplicitRow) +QML_DECLARE_TYPE(QQuickImplicitGrid) QML_DECLARE_TYPE(QQuickBasePositioner) QML_DECLARE_TYPEINFO(QQuickBasePositioner, QML_HAS_ATTACHED_PROPERTIES) diff --git a/src/quick/items/qquickpositioners_p_p.h b/src/quick/items/qquickpositioners_p_p.h index 0be4c56df6..1a7051615c 100644 --- a/src/quick/items/qquickpositioners_p_p.h +++ b/src/quick/items/qquickpositioners_p_p.h @@ -89,10 +89,14 @@ public: QLazilyAllocated extra; QQuickBasePositionerPrivate() - : spacing(0), type(QQuickBasePositioner::None) - , transitioner(0), positioningDirty(false) - , doingPositioning(false), anchorConflict(false), layoutDirection(Qt::LeftToRight) - + : spacing(0) + , type(QQuickBasePositioner::None) + , transitioner(0) + , positioningDirty(false) + , doingPositioning(false) + , anchorConflict(false) + , useImplicitSize(false) + , layoutDirection(Qt::LeftToRight) { } @@ -119,6 +123,7 @@ public: bool positioningDirty : 1; bool doingPositioning : 1; bool anchorConflict : 1; + bool useImplicitSize : 1; Qt::LayoutDirection layoutDirection; @@ -174,6 +179,34 @@ public: { } + void itemImplicitWidthChanged(QQuickItem *) override + { + Q_ASSERT(useImplicitSize); + setPositioningDirty(); + } + + void itemImplicitHeightChanged(QQuickItem *) override + { + Q_ASSERT(useImplicitSize); + setPositioningDirty(); + } + + qreal itemWidth(QQuickItem *item) const + { + if (Q_LIKELY(!useImplicitSize)) + return item->width(); + + return item->implicitWidth(); + } + + qreal itemHeight(QQuickItem *item) const + { + if (Q_LIKELY(!useImplicitSize)) + return item->height(); + + return item->implicitHeight(); + } + inline qreal padding() const { return extra.isAllocated() ? extra->padding : 0.0; } void setTopPadding(qreal value, bool reset = false); void setLeftPadding(qreal value, bool reset = false); diff --git a/tests/auto/quick/qquickpositioners/data/implicitGridOneItem.qml b/tests/auto/quick/qquickpositioners/data/implicitGridOneItem.qml new file mode 100644 index 0000000000..8da9fc270d --- /dev/null +++ b/tests/auto/quick/qquickpositioners/data/implicitGridOneItem.qml @@ -0,0 +1,12 @@ +import QtQuick 2.9 +import PositionerTest 1.0 + +ImplicitGrid { + columns: 2 + rows: 1 + + Text { + text: "Text" + width: parent.width + } +} diff --git a/tests/auto/quick/qquickpositioners/data/implicitRowOneItem.qml b/tests/auto/quick/qquickpositioners/data/implicitRowOneItem.qml new file mode 100644 index 0000000000..25a287cf31 --- /dev/null +++ b/tests/auto/quick/qquickpositioners/data/implicitRowOneItem.qml @@ -0,0 +1,9 @@ +import QtQuick 2.9 +import PositionerTest 1.0 + +ImplicitRow { + Text { + text: "Text" + width: parent.width + } +} diff --git a/tests/auto/quick/qquickpositioners/tst_qquickpositioners.cpp b/tests/auto/quick/qquickpositioners/tst_qquickpositioners.cpp index 1b3939401a..4c4afb7a7b 100644 --- a/tests/auto/quick/qquickpositioners/tst_qquickpositioners.cpp +++ b/tests/auto/quick/qquickpositioners/tst_qquickpositioners.cpp @@ -98,6 +98,8 @@ private slots: void test_attachedproperties(); void test_attachedproperties_data(); void test_attachedproperties_dynamic(); + void test_useImplicitSize_oneItem_data(); + void test_useImplicitSize_oneItem(); void populateTransitions_row(); void populateTransitions_row_data(); @@ -304,6 +306,8 @@ void tst_qquickpositioners::moveTransitions_flow_data() tst_qquickpositioners::tst_qquickpositioners() { + qmlRegisterType("PositionerTest", 1, 0, "ImplicitRow"); + qmlRegisterType("PositionerTest", 1, 0, "ImplicitGrid"); } void tst_qquickpositioners::test_horizontal() @@ -4004,6 +4008,46 @@ void tst_qquickpositioners::test_attachedproperties_dynamic() } +void tst_qquickpositioners::test_useImplicitSize_oneItem_data() +{ + QTest::addColumn("positionerType"); + + QTest::newRow("Grid") << "Grid"; + QTest::newRow("Row") << "Row"; +} + +void tst_qquickpositioners::test_useImplicitSize_oneItem() +{ + QFETCH(QString, positionerType); + + QQuickView view; + view.setSource(testFileUrl(QString::fromLatin1("implicit%1OneItem.qml").arg(positionerType))); + QCOMPARE(view.status(), QQuickView::Ready); + view.show(); + QVERIFY(QTest::qWaitForWindowExposed(&view)); + + QQuickItem *positioner = view.rootObject(); + QVERIFY(positioner); + const qreal oldPositionerImplicitWidth = positioner->implicitWidth(); + + QQuickText *text = qobject_cast(positioner->childItems().first()); + QVERIFY(text); + const qreal oldTextImplicitWidth = text->implicitWidth(); + QCOMPARE(positioner->implicitWidth(), text->implicitWidth()); + + // Ensure that the implicit size of the positioner changes when the implicit size + // of one of its children changes. + text->setText(QLatin1String("Even More Text")); + const qreal textImplicitWidthIncrease = text->implicitWidth() - oldTextImplicitWidth; + QVERIFY(textImplicitWidthIncrease > 0); + QTRY_COMPARE(positioner->implicitWidth(), oldPositionerImplicitWidth + textImplicitWidthIncrease); + + // Ensure that the implicit size of the positioner does not change when the + // explicit size of one of its children changes. + text->setWidth(10); + QTRY_COMPARE(positioner->implicitWidth(), oldPositionerImplicitWidth + textImplicitWidthIncrease); +} + QQuickView *tst_qquickpositioners::createView(const QString &filename, bool wait) { QQuickView *window = new QQuickView(0);