From cef452a2792cc15705f677c9b9c689496eeb500f Mon Sep 17 00:00:00 2001 From: Martin Jones Date: Mon, 17 May 2010 13:14:18 +1000 Subject: Don't crash due to recursive positioning. Also extend positioner anchor check to include fill and centerIn. Task-number: QTBUG-10731 --- .../graphicsitems/qdeclarativepositioners.cpp | 60 +++++++++++++--------- .../graphicsitems/qdeclarativepositioners_p_p.h | 5 +- .../tst_qdeclarativepositioners.cpp | 27 +++++++++- 3 files changed, 65 insertions(+), 27 deletions(-) diff --git a/src/declarative/graphicsitems/qdeclarativepositioners.cpp b/src/declarative/graphicsitems/qdeclarativepositioners.cpp index 93bff3e..8796e63 100644 --- a/src/declarative/graphicsitems/qdeclarativepositioners.cpp +++ b/src/declarative/graphicsitems/qdeclarativepositioners.cpp @@ -204,7 +204,11 @@ void QDeclarativeBasePositioner::prePositioning() if (!isComponentComplete()) return; + if (d->doingPositioning) + return; + d->queuedPositioning = false; + d->doingPositioning = true; //Need to order children by creation order modified by stacking order QList children = d->QGraphicsItemPrivate::children; qSort(children.begin(), children.end(), d->insertionOrder); @@ -242,6 +246,7 @@ void QDeclarativeBasePositioner::prePositioning() doPositioning(&contentSize); if(d->addTransition || d->moveTransition) finishApplyTransitions(); + d->doingPositioning = false; //Set implicit size to the size of its children setImplicitHeight(contentSize.height()); setImplicitWidth(contentSize.width()); @@ -339,7 +344,8 @@ Column { Note that the positioner assumes that the x and y positions of its children will not change. If you manually change the x or y properties in script, bind - the x or y properties, or use anchors on a child of a positioner, then the + the x or y properties, use anchors on a child of a positioner, or have the + height of a child depend on the position of a child, then the positioner may exhibit strange behaviour. */ @@ -437,7 +443,7 @@ void QDeclarativeColumn::doPositioning(QSizeF *contentSize) void QDeclarativeColumn::reportConflictingAnchors() { - bool childsWithConflictingAnchors(false); + QDeclarativeBasePositionerPrivate *d = static_cast(QDeclarativeBasePositionerPrivate::get(this)); for (int ii = 0; ii < positionedItems.count(); ++ii) { const PositionedItem &child = positionedItems.at(ii); if (child.item) { @@ -446,15 +452,16 @@ void QDeclarativeColumn::reportConflictingAnchors() QDeclarativeAnchors::Anchors usedAnchors = anchors->usedAnchors(); if (usedAnchors & QDeclarativeAnchors::TopAnchor || usedAnchors & QDeclarativeAnchors::BottomAnchor || - usedAnchors & QDeclarativeAnchors::VCenterAnchor) { - childsWithConflictingAnchors = true; + usedAnchors & QDeclarativeAnchors::VCenterAnchor || + anchors->fill() || anchors->centerIn()) { + d->anchorConflict = true; break; } } } } - if (childsWithConflictingAnchors) { - qmlInfo(this) << "Cannot specify top, bottom or verticalCenter anchors for items inside Column"; + if (d->anchorConflict) { + qmlInfo(this) << "Cannot specify top, bottom, verticalCenter, fill or centerIn anchors for items inside Column"; } } @@ -486,7 +493,8 @@ Row { Note that the positioner assumes that the x and y positions of its children will not change. If you manually change the x or y properties in script, bind - the x or y properties, or use anchors on a child of a positioner, then the + the x or y properties, use anchors on a child of a positioner, or have the + width of a child depend on the position of a child, then the positioner may exhibit strange behaviour. */ @@ -574,7 +582,7 @@ void QDeclarativeRow::doPositioning(QSizeF *contentSize) void QDeclarativeRow::reportConflictingAnchors() { - bool childsWithConflictingAnchors(false); + QDeclarativeBasePositionerPrivate *d = static_cast(QDeclarativeBasePositionerPrivate::get(this)); for (int ii = 0; ii < positionedItems.count(); ++ii) { const PositionedItem &child = positionedItems.at(ii); if (child.item) { @@ -583,16 +591,16 @@ void QDeclarativeRow::reportConflictingAnchors() QDeclarativeAnchors::Anchors usedAnchors = anchors->usedAnchors(); if (usedAnchors & QDeclarativeAnchors::LeftAnchor || usedAnchors & QDeclarativeAnchors::RightAnchor || - usedAnchors & QDeclarativeAnchors::HCenterAnchor) { - childsWithConflictingAnchors = true; + usedAnchors & QDeclarativeAnchors::HCenterAnchor || + anchors->fill() || anchors->centerIn()) { + d->anchorConflict = true; break; } } } } - if (childsWithConflictingAnchors) { - qmlInfo(this) << "Cannot specify left, right or horizontalCenter anchors for items inside Row"; - } + if (d->anchorConflict) + qmlInfo(this) << "Cannot specify left, right, horizontalCenter, fill or centerIn anchors for items inside Row"; } /*! @@ -638,7 +646,8 @@ Grid { Note that the positioner assumes that the x and y positions of its children will not change. If you manually change the x or y properties in script, bind - the x or y properties, or use anchors on a child of a positioner, then the + the x or y properties, use anchors on a child of a positioner, or have the + width or height of a child depend on the position of a child, then the positioner may exhibit strange behaviour. */ /*! @@ -866,20 +875,19 @@ void QDeclarativeGrid::doPositioning(QSizeF *contentSize) void QDeclarativeGrid::reportConflictingAnchors() { - bool childsWithConflictingAnchors(false); + QDeclarativeBasePositionerPrivate *d = static_cast(QDeclarativeBasePositionerPrivate::get(this)); for (int ii = 0; ii < positionedItems.count(); ++ii) { const PositionedItem &child = positionedItems.at(ii); if (child.item) { QDeclarativeAnchors *anchors = QDeclarativeItemPrivate::get(child.item)->_anchors; - if (anchors && anchors->usedAnchors()) { - childsWithConflictingAnchors = true; + if (anchors && (anchors->usedAnchors() || anchors->fill() || anchors->centerIn())) { + d->anchorConflict = true; break; } } } - if (childsWithConflictingAnchors) { + if (d->anchorConflict) qmlInfo(this) << "Cannot specify anchors for items inside Grid"; - } } /*! @@ -888,6 +896,11 @@ void QDeclarativeGrid::reportConflictingAnchors() \brief The Flow item lines up its children side by side, wrapping as necessary. \inherits Item + Note that the positioner assumes that the x and y positions of its children + will not change. If you manually change the x or y properties in script, bind + the x or y properties, use anchors on a child of a positioner, or have the + width or height of a child depend on the position of a child, then the + positioner may exhibit strange behaviour. */ /*! @@ -1026,20 +1039,19 @@ void QDeclarativeFlow::doPositioning(QSizeF *contentSize) void QDeclarativeFlow::reportConflictingAnchors() { - bool childsWithConflictingAnchors(false); + Q_D(QDeclarativeFlow); for (int ii = 0; ii < positionedItems.count(); ++ii) { const PositionedItem &child = positionedItems.at(ii); if (child.item) { QDeclarativeAnchors *anchors = QDeclarativeItemPrivate::get(child.item)->_anchors; - if (anchors && anchors->usedAnchors()) { - childsWithConflictingAnchors = true; + if (anchors && (anchors->usedAnchors() || anchors->fill() || anchors->centerIn())) { + d->anchorConflict = true; break; } } } - if (childsWithConflictingAnchors) { + if (d->anchorConflict) qmlInfo(this) << "Cannot specify anchors for items inside Flow"; - } } QT_END_NAMESPACE diff --git a/src/declarative/graphicsitems/qdeclarativepositioners_p_p.h b/src/declarative/graphicsitems/qdeclarativepositioners_p_p.h index 576f35b..04f0181 100644 --- a/src/declarative/graphicsitems/qdeclarativepositioners_p_p.h +++ b/src/declarative/graphicsitems/qdeclarativepositioners_p_p.h @@ -75,6 +75,7 @@ public: QDeclarativeBasePositionerPrivate() : spacing(0), type(QDeclarativeBasePositioner::None) , moveTransition(0), addTransition(0), queuedPositioning(false) + , doingPositioning(false), anchorConflict(false) { } @@ -95,7 +96,9 @@ public: void watchChanges(QDeclarativeItem *other); void unwatchChanges(QDeclarativeItem* other); - bool queuedPositioning; + bool queuedPositioning : 1; + bool doingPositioning : 1; + bool anchorConflict : 1; virtual void itemSiblingOrderChanged(QDeclarativeItem* other) { diff --git a/tests/auto/declarative/qdeclarativepositioners/tst_qdeclarativepositioners.cpp b/tests/auto/declarative/qdeclarativepositioners/tst_qdeclarativepositioners.cpp index 7a23773..e639014 100644 --- a/tests/auto/declarative/qdeclarativepositioners/tst_qdeclarativepositioners.cpp +++ b/tests/auto/declarative/qdeclarativepositioners/tst_qdeclarativepositioners.cpp @@ -687,7 +687,13 @@ void tst_QDeclarativePositioners::test_conflictinganchors() component.setData("import Qt 4.7\nColumn { Item { anchors.top: parent.top } }", QUrl::fromLocalFile("")); item = qobject_cast(component.create()); QVERIFY(item); - QCOMPARE(warningMessage, QString("file::2:1: QML Column: Cannot specify top, bottom or verticalCenter anchors for items inside Column")); + QCOMPARE(warningMessage, QString("file::2:1: QML Column: Cannot specify top, bottom, verticalCenter, fill or centerIn anchors for items inside Column")); + warningMessage.clear(); + + component.setData("import Qt 4.7\nColumn { Item { anchors.centerIn: parent } }", QUrl::fromLocalFile("")); + item = qobject_cast(component.create()); + QVERIFY(item); + QCOMPARE(warningMessage, QString("file::2:1: QML Column: Cannot specify top, bottom, verticalCenter, fill or centerIn anchors for items inside Column")); warningMessage.clear(); component.setData("import Qt 4.7\nColumn { Item { anchors.left: parent.left } }", QUrl::fromLocalFile("")); @@ -699,7 +705,13 @@ void tst_QDeclarativePositioners::test_conflictinganchors() component.setData("import Qt 4.7\nRow { Item { anchors.left: parent.left } }", QUrl::fromLocalFile("")); item = qobject_cast(component.create()); QVERIFY(item); - QCOMPARE(warningMessage, QString("file::2:1: QML Row: Cannot specify left, right or horizontalCenter anchors for items inside Row")); + QCOMPARE(warningMessage, QString("file::2:1: QML Row: Cannot specify left, right, horizontalCenter, fill or centerIn anchors for items inside Row")); + warningMessage.clear(); + + component.setData("import Qt 4.7\nRow { Item { anchors.fill: parent } }", QUrl::fromLocalFile("")); + item = qobject_cast(component.create()); + QVERIFY(item); + QCOMPARE(warningMessage, QString("file::2:1: QML Row: Cannot specify left, right, horizontalCenter, fill or centerIn anchors for items inside Row")); warningMessage.clear(); component.setData("import Qt 4.7\nRow { Item { anchors.top: parent.top } }", QUrl::fromLocalFile("")); @@ -714,10 +726,21 @@ void tst_QDeclarativePositioners::test_conflictinganchors() QCOMPARE(warningMessage, QString("file::2:1: QML Grid: Cannot specify anchors for items inside Grid")); warningMessage.clear(); + component.setData("import Qt 4.7\nGrid { Item { anchors.centerIn: parent } }", QUrl::fromLocalFile("")); + item = qobject_cast(component.create()); + QVERIFY(item); + QCOMPARE(warningMessage, QString("file::2:1: QML Grid: Cannot specify anchors for items inside Grid")); + warningMessage.clear(); + component.setData("import Qt 4.7\nFlow { Item { anchors.verticalCenter: parent.verticalCenter } }", QUrl::fromLocalFile("")); item = qobject_cast(component.create()); QVERIFY(item); QCOMPARE(warningMessage, QString("file::2:1: QML Flow: Cannot specify anchors for items inside Flow")); + + component.setData("import Qt 4.7\nFlow { Item { anchors.fill: parent } }", QUrl::fromLocalFile("")); + item = qobject_cast(component.create()); + QVERIFY(item); + QCOMPARE(warningMessage, QString("file::2:1: QML Flow: Cannot specify anchors for items inside Flow")); } QDeclarativeView *tst_QDeclarativePositioners::createView(const QString &filename) -- cgit v0.12