Fix Animators in itemview and positioner transitions

In itemview/positioner transitions, Animator's target-property is never
explicitly set. QQuickItemViewTransition passes the animation target as
a "default target" instead. This change adds missing handling for the
default target.

QQuickItemViewTransition sets up default "x" and "y" state actions.
If Animator drives opacity, scale, or basically anything else than x/y
animations, it failed to extract the from/to values from the state
action list.

This change fixes the issue that if the default state actions do not
match the animator property (x/y vs. scale/opacity), it uses from/to
values specified on the animator itself. Before, it did that only if
the default state action list was empty. This is not the case with
itemview/positioner transitions.

Change-Id: I0f15e20bc860ddec23e59efebbc9cd346317f4de
Task-number: QTBUG-50908
Reviewed-by: Gunnar Sletta <gunnar@sletta.org>
Reviewed-by: Robin Burchell <robin.burchell@viroteck.net>

4 files changed, 102 insertions, 7 deletions

diff --git a/src/quick/util/qquickanimator.cpp b/src/quick/util/qquickanimator.cpp
index 8a5cad4011..ca6dc74519 100644
--- a/src/quick/util/qquickanimator.cpp
+++ b/src/quick/util/qquickanimator.cpp
@@ -211,7 +211,8 @@ qreal QQuickAnimator::from() const
 void QQuickAnimatorPrivate::apply(QQuickAnimatorJob *job,
                                      const QString &propertyName,
                                      QQuickStateActions &actions,
-                                     QQmlProperties &modified)
+                                     QQmlProperties &modified,
+                                     QObject *defaultTarget)
 {
 
     if (actions.size()) {
@@ -243,14 +244,20 @@ void QQuickAnimatorPrivate::apply(QQuickAnimatorJob *job,
             // the item when a transition is cancelled.
             action.fromValue = action.toValue;
        }
-    } else {
+    }
+
+    if (modified.isEmpty()) {
         job->setTarget(target);
         job->setFrom(from);
         job->setTo(to);
     }
 
-    if (!job->target() && defaultProperty.object())
-        job->setTarget(qobject_cast<QQuickItem *>(defaultProperty.object()));
+    if (!job->target()) {
+        if (defaultProperty.object())
+            job->setTarget(qobject_cast<QQuickItem *>(defaultProperty.object()));
+        else
+            job->setTarget(qobject_cast<QQuickItem *>(defaultTarget));
+    }
 
     job->setDuration(duration);
     job->setLoopCount(loopCount);
@@ -260,7 +267,7 @@ void QQuickAnimatorPrivate::apply(QQuickAnimatorJob *job,
 QAbstractAnimationJob *QQuickAnimator::transition(QQuickStateActions &actions,
                                                   QQmlProperties &modified,
                                                   TransitionDirection direction,
-                                                  QObject *)
+                                                  QObject *defaultTarget)
 {
     Q_D(QQuickAnimator);
 
@@ -277,7 +284,7 @@ QAbstractAnimationJob *QQuickAnimator::transition(QQuickStateActions &actions,
     if (!job)
         return 0;
 
-    d->apply(job, propertyName(), actions, modified);
+    d->apply(job, propertyName(), actions, modified, defaultTarget);
 
     if (!job->target()) {
         delete job;
diff --git a/src/quick/util/qquickanimator_p_p.h b/src/quick/util/qquickanimator_p_p.h
index f5f0295f74..11598bf78e 100644
--- a/src/quick/util/qquickanimator_p_p.h
+++ b/src/quick/util/qquickanimator_p_p.h
@@ -76,7 +76,7 @@ public:
     uint isFromDefined : 1;
     uint isToDefined : 1;
 
-    void apply(QQuickAnimatorJob *job, const QString &propertyName, QQuickStateActions &actions, QQmlProperties &modified);
+    void apply(QQuickAnimatorJob *job, const QString &propertyName, QQuickStateActions &actions, QQmlProperties &modified, QObject *defaultTarget);
 };
 
 class QQuickRotationAnimatorPrivate : public QQuickAnimatorPrivate
diff --git a/tests/auto/quick/qquickanimators/data/positionerWithAnimator.qml b/tests/auto/quick/qquickanimators/data/positionerWithAnimator.qml
new file mode 100644
index 0000000000..bfd475266e
--- /dev/null
+++ b/tests/auto/quick/qquickanimators/data/positionerWithAnimator.qml
@@ -0,0 +1,63 @@
+/****************************************************************************
+**
+** Copyright (C) 2015 The Qt Company Ltd.
+** Contact: http://www.qt.io/licensing/
+**
+** This file is part of the test suite of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL21$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see http://www.qt.io/terms-conditions. For further
+** information use the contact form at http://www.qt.io/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 2.1 or version 3 as published by the Free
+** Software Foundation and appearing in the file LICENSE.LGPLv21 and
+** LICENSE.LGPLv3 included in the packaging of this file. Please review the
+** following information to ensure the GNU Lesser General Public License
+** requirements will be met: https://www.gnu.org/licenses/lgpl.html and
+** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** As a special exception, The Qt Company gives you certain additional
+** rights. These rights are described in The Qt Company LGPL Exception
+** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+import QtQuick 2.2
+
+Column {
+    width: 200
+    height: 200
+
+    property alias repeater: repeater
+    property alias transition: transition
+
+    anchors.centerIn: parent
+    populate: Transition {
+        id: transition
+        ScaleAnimator {
+            from: 0
+            to: 1
+        }
+    }
+
+    Repeater {
+        id: repeater
+        model: ["red", "green", "blue"]
+
+        Rectangle {
+            width: 100
+            height: 100
+            color: modelData
+            scale: 0
+        }
+    }
+}
diff --git a/tests/auto/quick/qquickanimators/tst_qquickanimators.cpp b/tests/auto/quick/qquickanimators/tst_qquickanimators.cpp
index 4108c1a124..7cacf3ba90 100644
--- a/tests/auto/quick/qquickanimators/tst_qquickanimators.cpp
+++ b/tests/auto/quick/qquickanimators/tst_qquickanimators.cpp
@@ -35,6 +35,8 @@
 
 #include <QtQuick>
 #include <private/qquickanimator_p.h>
+#include <private/qquickrepeater_p.h>
+#include <private/qquicktransition_p.h>
 
 #include <QtQml>
 
@@ -45,6 +47,7 @@ class tst_Animators: public QObject
 private slots:
     void testMultiWinAnimator_data();
     void testMultiWinAnimator();
+    void testTransitions();
 };
 
 void tst_Animators::testMultiWinAnimator_data()
@@ -99,6 +102,28 @@ void tst_Animators::testMultiWinAnimator()
     QVERIFY(true);
 }
 
+void tst_Animators::testTransitions()
+{
+    QQuickView view(QUrl::fromLocalFile("data/positionerWithAnimator.qml"));
+    view.show();
+    QVERIFY(QTest::qWaitForWindowExposed(&view));
+    QVERIFY(view.rootObject());
+
+    QQuickRepeater *repeater = view.rootObject()->property("repeater").value<QQuickRepeater *>();
+    QVERIFY(repeater);
+
+    QQuickItem *child = repeater->itemAt(0);
+    QVERIFY(child);
+    QCOMPARE(child->scale(), qreal(0.0));
+
+    QQuickTransition *transition = view.rootObject()->property("transition").value<QQuickTransition *>();
+    QVERIFY(transition);
+
+    QTRY_VERIFY(transition->running());
+    QTRY_VERIFY(!transition->running());
+    QCOMPARE(child->scale(), qreal(1.0));
+}
+
 #include "tst_qquickanimators.moc"
 
 QTEST_MAIN(tst_Animators)