Set positioner size to final size, rather than some random size mid animation

Task-number: QTBUG-9559

14 files changed, 100 insertions, 28 deletions

diff --git a/src/declarative/graphicsitems/qdeclarativepositioners.cpp b/src/declarative/graphicsitems/qdeclarativepositioners.cpp
index b23b8c9..781e584 100644
--- a/src/declarative/graphicsitems/qdeclarativepositioners.cpp
+++ b/src/declarative/graphicsitems/qdeclarativepositioners.cpp
@@ -237,21 +237,13 @@ void QDeclarativeBasePositioner::prePositioning()
             positionedItems.append(*item);
         }
     }
-    doPositioning();
+    QSizeF contentSize;
+    doPositioning(&contentSize);
     if(d->addTransition || d->moveTransition)
         finishApplyTransitions();
     //Set implicit size to the size of its children
-    qreal h = 0.0f;
-    qreal w = 0.0f;
-    for (int i = 0; i < positionedItems.count(); ++i) {
-        const PositionedItem &posItem = positionedItems.at(i);
-        if (posItem.isVisible) {
-            h = qMax(h, posItem.item->y() + posItem.item->height());
-            w = qMax(w, posItem.item->x() + posItem.item->width());
-        }
-    }
-    setImplicitHeight(h);
-    setImplicitWidth(w);
+    setImplicitHeight(contentSize.height());
+    setImplicitWidth(contentSize.width());
 }
 
 void QDeclarativeBasePositioner::positionX(int x, const PositionedItem &target)
@@ -423,7 +415,7 @@ static inline bool isInvisible(QDeclarativeItem *child)
     return child->opacity() == 0.0 || !child->isVisible() || !child->width() || !child->height();
 }
 
-void QDeclarativeColumn::doPositioning()
+void QDeclarativeColumn::doPositioning(QSizeF *contentSize)
 {
     int voffset = 0;
 
@@ -435,9 +427,13 @@ void QDeclarativeColumn::doPositioning()
         if(child.item->y() != voffset)
             positionY(voffset, child);
 
+        contentSize->setWidth(qMax(contentSize->width(), child.item->width()));
+
         voffset += child.item->height();
         voffset += spacing();
     }
+
+    contentSize->setHeight(voffset - spacing());
 }
 
 /*!
@@ -534,7 +530,7 @@ QDeclarativeRow::QDeclarativeRow(QDeclarativeItem *parent)
 {
 }
 
-void QDeclarativeRow::doPositioning()
+void QDeclarativeRow::doPositioning(QSizeF *contentSize)
 {
     int hoffset = 0;
 
@@ -546,9 +542,13 @@ void QDeclarativeRow::doPositioning()
         if(child.item->x() != hoffset)
             positionX(hoffset, child);
 
+        contentSize->setHeight(qMax(contentSize->height(), child.item->height()));
+
         hoffset += child.item->width();
         hoffset += spacing();
     }
+
+    contentSize->setWidth(hoffset - spacing());
 }
 
 
@@ -698,7 +698,7 @@ void QDeclarativeGrid::setRows(const int rows)
     emit rowsChanged();
 }
 
-void QDeclarativeGrid::doPositioning()
+void QDeclarativeGrid::doPositioning(QSizeF *contentSize)
 {
     int c=_columns,r=_rows;//Actual number of rows/columns
     int numVisible = positionedItems.count();
@@ -745,6 +745,10 @@ void QDeclarativeGrid::doPositioning()
             positionX(xoffset, child);
             positionY(yoffset, child);
         }
+
+        contentSize->setWidth(qMax(contentSize->width(), xoffset + child.item->width()));
+        contentSize->setHeight(yoffset + maxRowHeight[curRow]);
+
         xoffset+=maxColWidth[curCol]+spacing();
         curCol++;
         curCol%=c;
@@ -854,7 +858,7 @@ void QDeclarativeFlow::setFlow(Flow flow)
     }
 }
 
-void QDeclarativeFlow::doPositioning()
+void QDeclarativeFlow::doPositioning(QSizeF *contentSize)
 {
     Q_D(QDeclarativeFlow);
 
@@ -886,6 +890,9 @@ void QDeclarativeFlow::doPositioning()
             positionY(voffset, child);
         }
 
+        contentSize->setWidth(qMax(contentSize->width(), hoffset + child.item->width()));
+        contentSize->setHeight(qMax(contentSize->height(), voffset + child.item->height()));
+
         if (d->flow == LeftToRight)  {
             hoffset += child.item->width();
             hoffset += spacing();
diff --git a/src/declarative/graphicsitems/qdeclarativepositioners_p.h b/src/declarative/graphicsitems/qdeclarativepositioners_p.h
index f38847c..c4414d1 100644
--- a/src/declarative/graphicsitems/qdeclarativepositioners_p.h
+++ b/src/declarative/graphicsitems/qdeclarativepositioners_p.h
@@ -90,10 +90,10 @@ Q_SIGNALS:
     void addChanged();
 
 protected Q_SLOTS:
-    virtual void doPositioning()=0;
     void prePositioning();
 
 protected:
+    virtual void doPositioning(QSizeF *contentSize)=0;
     struct PositionedItem {
         PositionedItem(QDeclarativeItem *i) : item(i), isNew(false), isVisible(true) {}
         bool operator==(const PositionedItem &other) const { return other.item == item; }
@@ -116,8 +116,8 @@ class Q_DECLARATIVE_EXPORT QDeclarativeColumn : public QDeclarativeBasePositione
     Q_OBJECT
 public:
     QDeclarativeColumn(QDeclarativeItem *parent=0);
-protected Q_SLOTS:
-    virtual void doPositioning();
+protected:
+    virtual void doPositioning(QSizeF *contentSize);
 private:
     Q_DISABLE_COPY(QDeclarativeColumn)
 };
@@ -127,8 +127,8 @@ class Q_DECLARATIVE_EXPORT QDeclarativeRow: public QDeclarativeBasePositioner
     Q_OBJECT
 public:
     QDeclarativeRow(QDeclarativeItem *parent=0);
-protected Q_SLOTS:
-    virtual void doPositioning();
+protected:
+    virtual void doPositioning(QSizeF *contentSize);
 private:
     Q_DISABLE_COPY(QDeclarativeRow)
 };
@@ -151,8 +151,8 @@ Q_SIGNALS:
     void rowsChanged();
     void columnsChanged();
 
-protected Q_SLOTS:
-    virtual void doPositioning();
+protected:
+    virtual void doPositioning(QSizeF *contentSize);
 
 private:
     int _rows;
@@ -176,8 +176,8 @@ public:
 Q_SIGNALS:
     void flowChanged();
 
-protected Q_SLOTS:
-    virtual void doPositioning();
+protected:
+    virtual void doPositioning(QSizeF *contentSize);
 
 protected:
     QDeclarativeFlow(QDeclarativeFlowPrivate &dd, QDeclarativeItem *parent);
diff --git a/src/declarative/graphicsitems/qdeclarativepositioners_p_p.h b/src/declarative/graphicsitems/qdeclarativepositioners_p_p.h
index 3a1edee..7880e3e 100644
--- a/src/declarative/graphicsitems/qdeclarativepositioners_p_p.h
+++ b/src/declarative/graphicsitems/qdeclarativepositioners_p_p.h
@@ -73,8 +73,8 @@ class QDeclarativeBasePositionerPrivate : public QDeclarativeItemPrivate, public
 
 public:
     QDeclarativeBasePositionerPrivate()
-        : spacing(0), type(QDeclarativeBasePositioner::None), moveTransition(0), addTransition(0),
-          queuedPositioning(false)
+        : spacing(0), type(QDeclarativeBasePositioner::None)
+        , moveTransition(0), addTransition(0), queuedPositioning(false)
     {
     }
 
@@ -84,6 +84,7 @@ public:
     }
 
     int spacing;
+
     QDeclarativeBasePositioner::PositionerType type;
     QDeclarativeTransition *moveTransition;
     QDeclarativeTransition *addTransition;
diff --git a/tests/auto/declarative/qdeclarativepositioners/data/flowtest.qml b/tests/auto/declarative/qdeclarativepositioners/data/flowtest.qml
index bd13bac..6c1c823 100644
--- a/tests/auto/declarative/qdeclarativepositioners/data/flowtest.qml
+++ b/tests/auto/declarative/qdeclarativepositioners/data/flowtest.qml
@@ -4,7 +4,8 @@ Item {
     width: 90
     height: 480
     Flow {
-        anchors.fill: parent
+        objectName: "flow"
+        width: parent.width
         Rectangle {
             objectName: "one"
             color: "red"
diff --git a/tests/auto/declarative/qdeclarativepositioners/data/grid-animated.qml b/tests/auto/declarative/qdeclarativepositioners/data/grid-animated.qml
index f6376a1..9741ba9 100644
--- a/tests/auto/declarative/qdeclarativepositioners/data/grid-animated.qml
+++ b/tests/auto/declarative/qdeclarativepositioners/data/grid-animated.qml
@@ -4,6 +4,7 @@ Item {
     width: 640
     height: 480
     Grid {
+        objectName: "grid"
         columns: 3
         add: Transition {
             NumberAnimation {
diff --git a/tests/auto/declarative/qdeclarativepositioners/data/grid-spacing.qml b/tests/auto/declarative/qdeclarativepositioners/data/grid-spacing.qml
index 5b4a30d..e335932 100644
--- a/tests/auto/declarative/qdeclarativepositioners/data/grid-spacing.qml
+++ b/tests/auto/declarative/qdeclarativepositioners/data/grid-spacing.qml
@@ -4,6 +4,7 @@ Item {
     width: 640
     height: 480
     Grid {
+        objectName: "grid"
         columns: 3
         spacing: 4
         Rectangle {
diff --git a/tests/auto/declarative/qdeclarativepositioners/data/gridtest.qml b/tests/auto/declarative/qdeclarativepositioners/data/gridtest.qml
index 830df6a..1d6f44e 100644
--- a/tests/auto/declarative/qdeclarativepositioners/data/gridtest.qml
+++ b/tests/auto/declarative/qdeclarativepositioners/data/gridtest.qml
@@ -4,6 +4,7 @@ Item {
     width: 640
     height: 480
     Grid {
+        objectName: "grid"
         columns: 3
         Rectangle {
             objectName: "one"
diff --git a/tests/auto/declarative/qdeclarativepositioners/data/horizontal-animated.qml b/tests/auto/declarative/qdeclarativepositioners/data/horizontal-animated.qml
index c113a36..a1c05a8 100644
--- a/tests/auto/declarative/qdeclarativepositioners/data/horizontal-animated.qml
+++ b/tests/auto/declarative/qdeclarativepositioners/data/horizontal-animated.qml
@@ -4,6 +4,7 @@ Item {
     width: 640
     height: 480
     Row {
+        objectName: "row"
         add: Transition {
             NumberAnimation {
                 properties: "x";
diff --git a/tests/auto/declarative/qdeclarativepositioners/data/horizontal-spacing.qml b/tests/auto/declarative/qdeclarativepositioners/data/horizontal-spacing.qml
index 32bf775..fb9fdd1 100644
--- a/tests/auto/declarative/qdeclarativepositioners/data/horizontal-spacing.qml
+++ b/tests/auto/declarative/qdeclarativepositioners/data/horizontal-spacing.qml
@@ -4,6 +4,7 @@ Item {
     width: 640
     height: 480
     Row {
+        objectName: "row"
         spacing: 10
         Rectangle {
             objectName: "one"
diff --git a/tests/auto/declarative/qdeclarativepositioners/data/horizontal.qml b/tests/auto/declarative/qdeclarativepositioners/data/horizontal.qml
index 06ae151..3a7a3b1 100644
--- a/tests/auto/declarative/qdeclarativepositioners/data/horizontal.qml
+++ b/tests/auto/declarative/qdeclarativepositioners/data/horizontal.qml
@@ -4,6 +4,7 @@ Item {
     width: 640
     height: 480
     Row {
+        objectName: "row"
         Rectangle {
             objectName: "one"
             color: "red"
diff --git a/tests/auto/declarative/qdeclarativepositioners/data/vertical-animated.qml b/tests/auto/declarative/qdeclarativepositioners/data/vertical-animated.qml
index 10f6cbb..31faa54 100644
--- a/tests/auto/declarative/qdeclarativepositioners/data/vertical-animated.qml
+++ b/tests/auto/declarative/qdeclarativepositioners/data/vertical-animated.qml
@@ -4,6 +4,7 @@ Item {
     width: 640
     height: 480
     Column {
+        objectName: "column"
         add: Transition {
             NumberAnimation {
                 properties: "y";
diff --git a/tests/auto/declarative/qdeclarativepositioners/data/vertical-spacing.qml b/tests/auto/declarative/qdeclarativepositioners/data/vertical-spacing.qml
index 69a8256..1c5696b 100644
--- a/tests/auto/declarative/qdeclarativepositioners/data/vertical-spacing.qml
+++ b/tests/auto/declarative/qdeclarativepositioners/data/vertical-spacing.qml
@@ -4,6 +4,7 @@ Item {
     width: 640
     height: 480
     Column {
+        objectName: "column"
         spacing: 10
         Rectangle {
             objectName: "one"
diff --git a/tests/auto/declarative/qdeclarativepositioners/data/vertical.qml b/tests/auto/declarative/qdeclarativepositioners/data/vertical.qml
index 856c180..cd777e2 100644
--- a/tests/auto/declarative/qdeclarativepositioners/data/vertical.qml
+++ b/tests/auto/declarative/qdeclarativepositioners/data/vertical.qml
@@ -4,6 +4,7 @@ Item {
     width: 640
     height: 480
     Column {
+        objectName: "column"
         Rectangle {
             objectName: "one"
             color: "red"
diff --git a/tests/auto/declarative/qdeclarativepositioners/tst_qdeclarativepositioners.cpp b/tests/auto/declarative/qdeclarativepositioners/tst_qdeclarativepositioners.cpp
index 9026566..08eac0a 100644
--- a/tests/auto/declarative/qdeclarativepositioners/tst_qdeclarativepositioners.cpp
+++ b/tests/auto/declarative/qdeclarativepositioners/tst_qdeclarativepositioners.cpp
@@ -94,6 +94,10 @@ void tst_QDeclarativePositioners::test_horizontal()
     QCOMPARE(two->y(), 0.0);
     QCOMPARE(three->x(), 70.0);
     QCOMPARE(three->y(), 0.0);
+
+    QDeclarativeItem *row = canvas->rootObject()->findChild<QDeclarativeItem*>("row");
+    QCOMPARE(row->width(), 110.0);
+    QCOMPARE(row->height(), 50.0);
 }
 
 void tst_QDeclarativePositioners::test_horizontal_spacing()
@@ -115,6 +119,10 @@ void tst_QDeclarativePositioners::test_horizontal_spacing()
     QCOMPARE(two->y(), 0.0);
     QCOMPARE(three->x(), 90.0);
     QCOMPARE(three->y(), 0.0);
+
+    QDeclarativeItem *row = canvas->rootObject()->findChild<QDeclarativeItem*>("row");
+    QCOMPARE(row->width(), 130.0);
+    QCOMPARE(row->height(), 50.0);
 }
 
 void tst_QDeclarativePositioners::test_horizontal_animated()
@@ -135,6 +143,11 @@ void tst_QDeclarativePositioners::test_horizontal_animated()
     QCOMPARE(two->x(), -100.0);
     QCOMPARE(three->x(), -100.0);
 
+    QDeclarativeItem *row = canvas->rootObject()->findChild<QDeclarativeItem*>("row");
+    QVERIFY(row);
+    QCOMPARE(row->width(), 100.0);
+    QCOMPARE(row->height(), 50.0);
+
     //QTRY_COMPARE used instead of waiting for the expected time of animation completion
     //Note that this means the duration of the animation is NOT tested
 
@@ -149,6 +162,11 @@ void tst_QDeclarativePositioners::test_horizontal_animated()
     //Add 'two'
     two->setOpacity(1.0);
     QCOMPARE(two->opacity(), 1.0);
+
+    // New size should be immediate
+    QCOMPARE(row->width(), 150.0);
+    QCOMPARE(row->height(), 50.0);
+
     QTest::qWait(0);//Let the animation start
     QCOMPARE(two->x(), -100.0);
     QCOMPARE(three->x(), 50.0);
@@ -176,6 +194,11 @@ void tst_QDeclarativePositioners::test_vertical()
     QCOMPARE(two->y(), 50.0);
     QCOMPARE(three->x(), 0.0);
     QCOMPARE(three->y(), 60.0);
+
+    QDeclarativeItem *column = canvas->rootObject()->findChild<QDeclarativeItem*>("column");
+    QVERIFY(column);
+    QCOMPARE(column->height(), 80.0);
+    QCOMPARE(column->width(), 50.0);
 }
 
 void tst_QDeclarativePositioners::test_vertical_spacing()
@@ -197,6 +220,10 @@ void tst_QDeclarativePositioners::test_vertical_spacing()
     QCOMPARE(two->y(), 60.0);
     QCOMPARE(three->x(), 0.0);
     QCOMPARE(three->y(), 80.0);
+
+    QDeclarativeItem *column = canvas->rootObject()->findChild<QDeclarativeItem*>("column");
+    QCOMPARE(column->height(), 100.0);
+    QCOMPARE(column->width(), 50.0);
 }
 
 void tst_QDeclarativePositioners::test_vertical_animated()
@@ -216,6 +243,11 @@ void tst_QDeclarativePositioners::test_vertical_animated()
     QVERIFY(three != 0);
     QCOMPARE(three->y(), -100.0);
 
+    QDeclarativeItem *column = canvas->rootObject()->findChild<QDeclarativeItem*>("column");
+    QVERIFY(column);
+    QCOMPARE(column->height(), 100.0);
+    QCOMPARE(column->width(), 50.0);
+
     //QTRY_COMPARE used instead of waiting for the expected time of animation completion
     //Note that this means the duration of the animation is NOT tested
 
@@ -230,6 +262,8 @@ void tst_QDeclarativePositioners::test_vertical_animated()
     //Add 'two'
     two->setOpacity(1.0);
     QTRY_COMPARE(two->opacity(), 1.0);
+    QCOMPARE(column->height(), 150.0);
+    QCOMPARE(column->width(), 50.0);
     QTest::qWait(0);//Let the animation start
     QCOMPARE(two->y(), -100.0);
     QCOMPARE(three->y(), 50.0);
@@ -264,6 +298,10 @@ void tst_QDeclarativePositioners::test_grid()
     QCOMPARE(four->y(), 50.0);
     QCOMPARE(five->x(), 50.0);
     QCOMPARE(five->y(), 50.0);
+
+    QDeclarativeItem *grid = canvas->rootObject()->findChild<QDeclarativeItem*>("grid");
+    QCOMPARE(grid->width(), 120.0);
+    QCOMPARE(grid->height(), 100.0);
 }
 
 void tst_QDeclarativePositioners::test_grid_spacing()
@@ -291,6 +329,10 @@ void tst_QDeclarativePositioners::test_grid_spacing()
     QCOMPARE(four->y(), 54.0);
     QCOMPARE(five->x(), 54.0);
     QCOMPARE(five->y(), 54.0);
+
+    QDeclarativeItem *grid = canvas->rootObject()->findChild<QDeclarativeItem*>("grid");
+    QCOMPARE(grid->width(), 128.0);
+    QCOMPARE(grid->height(), 104.0);
 }
 
 void tst_QDeclarativePositioners::test_grid_animated()
@@ -323,6 +365,11 @@ void tst_QDeclarativePositioners::test_grid_animated()
     QCOMPARE(five->x(), -100.0);
     QCOMPARE(five->y(), -100.0);
 
+    QDeclarativeItem *grid = canvas->rootObject()->findChild<QDeclarativeItem*>("grid");
+    QVERIFY(grid);
+    QCOMPARE(grid->width(), 150.0);
+    QCOMPARE(grid->height(), 100.0);
+
     //QTRY_COMPARE used instead of waiting for the expected time of animation completion
     //Note that this means the duration of the animation is NOT tested
 
@@ -341,6 +388,8 @@ void tst_QDeclarativePositioners::test_grid_animated()
     //Add 'two'
     two->setOpacity(1.0);
     QCOMPARE(two->opacity(), 1.0);
+    QCOMPARE(grid->width(), 150.0);
+    QCOMPARE(grid->height(), 100.0);
     QTest::qWait(0);//Let the animation start
     QCOMPARE(two->x(), -100.0);
     QCOMPARE(two->y(), -100.0);
@@ -468,6 +517,11 @@ void tst_QDeclarativePositioners::test_flow()
     QCOMPARE(four->y(), 70.0);
     QCOMPARE(five->x(), 50.0);
     QCOMPARE(five->y(), 70.0);
+
+    QDeclarativeItem *flow = canvas->rootObject()->findChild<QDeclarativeItem*>("flow");
+    QVERIFY(flow);
+    QCOMPARE(flow->width(), 90.0);
+    QCOMPARE(flow->height(), 120.0);
 }
 
 void tst_QDeclarativePositioners::test_flow_resize()