diff --git a/src/corelib/animation/qpropertyanimation.cpp b/src/corelib/animation/qpropertyanimation.cpp index 03efb7f6913..fe7ab0c0700 100644 --- a/src/corelib/animation/qpropertyanimation.cpp +++ b/src/corelib/animation/qpropertyanimation.cpp @@ -91,7 +91,7 @@ QT_BEGIN_NAMESPACE void QPropertyAnimationPrivate::updateMetaProperty() { - if (!target || propertyName.value().isEmpty()) { + if (!targetObject || propertyName.value().isEmpty()) { propertyType = QMetaType::UnknownType; propertyIndex = -1; return; @@ -99,19 +99,19 @@ void QPropertyAnimationPrivate::updateMetaProperty() //propertyType will be set to a valid type only if there is a Q_PROPERTY //otherwise it will be set to QVariant::Invalid at the end of this function - propertyType = targetValue->property(propertyName.value()).userType(); - propertyIndex = targetValue->metaObject()->indexOfProperty(propertyName.value()); + propertyType = targetObject->property(propertyName.value()).userType(); + propertyIndex = targetObject->metaObject()->indexOfProperty(propertyName.value()); if (propertyType != QMetaType::UnknownType) convertValues(propertyType); if (propertyIndex == -1) { //there is no Q_PROPERTY on the object propertyType = QMetaType::UnknownType; - if (!targetValue->dynamicPropertyNames().contains(propertyName)) + if (!targetObject->dynamicPropertyNames().contains(propertyName)) qWarning("QPropertyAnimation: you're trying to animate a non-existing property %s of " "your QObject", propertyName.value().constData()); - } else if (!targetValue->metaObject()->property(propertyIndex).isWritable()) { + } else if (!targetObject->metaObject()->property(propertyIndex).isWritable()) { qWarning("QPropertyAnimation: you're trying to animate the non-writable property %s of " "your QObject", propertyName.value().constData()); @@ -123,10 +123,8 @@ void QPropertyAnimationPrivate::updateProperty(const QVariant &newValue) if (state == QAbstractAnimation::Stopped) return; - if (!target) { - q_func()->stop(); //the target was destroyed we need to stop the animation + if (!targetObject) return; - } if (newValue.userType() == propertyType) { //no conversion is needed, we directly call the QMetaObject::metacall @@ -134,9 +132,9 @@ void QPropertyAnimationPrivate::updateProperty(const QVariant &newValue) int status = -1; int flags = 0; void *argv[] = { const_cast(newValue.constData()), const_cast(&newValue), &status, &flags }; - QMetaObject::metacall(targetValue, QMetaObject::WriteProperty, propertyIndex, argv); + QMetaObject::metacall(targetObject, QMetaObject::WriteProperty, propertyIndex, argv); } else { - targetValue->setProperty(propertyName.value().constData(), newValue); + targetObject->setProperty(propertyName.value().constData(), newValue); } } @@ -179,22 +177,36 @@ QPropertyAnimation::~QPropertyAnimation() */ QObject *QPropertyAnimation::targetObject() const { - return d_func()->target.data(); + return d_func()->targetObject; +} + +QBindable QPropertyAnimation::bindableTargetObject() +{ + return &d_func()->targetObject; } void QPropertyAnimation::setTargetObject(QObject *target) { Q_D(QPropertyAnimation); - if (d->target.data() == target) - return; - if (d->state != QAbstractAnimation::Stopped) { qWarning("QPropertyAnimation::setTargetObject: you can't change the target of a running animation"); return; } - d->target = d->targetValue = target; + d->targetObject.removeBindingUnlessInWrapper(); + if (d->targetObject == target) + return; + + if (d->targetObject != nullptr) + QObject::disconnect(d->targetObject, &QObject::destroyed, this, nullptr); + d->targetObject.setValueBypassingBindings(target); + + if (d->targetObject != nullptr) { + QObject::connect(d->targetObject, &QObject::destroyed, this, + [d] { d->targetObjectDestroyed(); }); + } d->updateMetaProperty(); + d->targetObject.notify(); } /*! @@ -265,7 +277,7 @@ void QPropertyAnimation::updateState(QAbstractAnimation::State newState, { Q_D(QPropertyAnimation); - if (!d->target && oldState == Stopped) { + if (!d->targetObject && oldState == Stopped) { qWarning("QPropertyAnimation::updateState (%s): Changing state of an animation without " "target", d->propertyName.value().constData()); @@ -281,9 +293,16 @@ void QPropertyAnimation::updateState(QAbstractAnimation::State newState, typedef QPair QPropertyAnimationPair; typedef QHash QPropertyAnimationHash; static QPropertyAnimationHash hash; - //here we need to use value because we need to know to which pointer - //the animation was referring in case stopped because the target was destroyed - QPropertyAnimationPair key(d->targetValue, d->propertyName); + + // in case the targetObject gets deleted, the following happens: + // 1. targetObject's destroyed signal calls our targetObjectDestroyed. + // 2. targetObjectDestroyed calls stop() + // 3. QAbstractAnimation::stop() calls setState(Stopped) + // 4. setState(Stopped) calls updateState(newState, oldState) + // 5. we arrive here. d->targetObject is not yet set to nullptr, we can safely use it. + Q_ASSERT(d->targetObject); + + QPropertyAnimationPair key(d->targetObject, d->propertyName); if (newState == Running) { d->updateMetaProperty(); animToStop = hash.value(key, nullptr); @@ -292,7 +311,7 @@ void QPropertyAnimation::updateState(QAbstractAnimation::State newState, // update the default start value if (oldState == Stopped) { d->setDefaultStartEndValue( - d->targetValue->property(d->propertyName.value().constData())); + d->targetObject->property(d->propertyName.value().constData())); //let's check if we have a start value and an end value const char *what = nullptr; if (!startValue().isValid() && (d->direction == Backward || !d->defaultStartEndValue.isValid())) { @@ -308,8 +327,8 @@ void QPropertyAnimation::updateState(QAbstractAnimation::State newState, qWarning("QPropertyAnimation::updateState (%s, %s, %ls): starting an animation " "without %s value", d->propertyName.value().constData(), - d->target.data()->metaObject()->className(), - qUtf16Printable(d->target.data()->objectName()), what); + d->targetObject->metaObject()->className(), + qUtf16Printable(d->targetObject->objectName()), what); } } } else if (hash.value(key) == this) { diff --git a/src/corelib/animation/qpropertyanimation.h b/src/corelib/animation/qpropertyanimation.h index 4482e7f7ab6..5768ec6f017 100644 --- a/src/corelib/animation/qpropertyanimation.h +++ b/src/corelib/animation/qpropertyanimation.h @@ -50,9 +50,10 @@ class QPropertyAnimationPrivate; class Q_CORE_EXPORT QPropertyAnimation : public QVariantAnimation { Q_OBJECT - Q_PROPERTY(QByteArray propertyName READ propertyName WRITE setPropertyName BINDABLE - bindablePropertyName) - Q_PROPERTY(QObject* targetObject READ targetObject WRITE setTargetObject) + Q_PROPERTY(QByteArray propertyName READ propertyName WRITE setPropertyName + BINDABLE bindablePropertyName) + Q_PROPERTY(QObject* targetObject READ targetObject WRITE setTargetObject + BINDABLE bindableTargetObject) public: QPropertyAnimation(QObject *parent = nullptr); @@ -61,6 +62,7 @@ public: QObject *targetObject() const; void setTargetObject(QObject *target); + QBindable bindableTargetObject(); QByteArray propertyName() const; void setPropertyName(const QByteArray &propertyName); diff --git a/src/corelib/animation/qpropertyanimation_p.h b/src/corelib/animation/qpropertyanimation_p.h index 0b1bb2bd032..2552fec0888 100644 --- a/src/corelib/animation/qpropertyanimation_p.h +++ b/src/corelib/animation/qpropertyanimation_p.h @@ -64,14 +64,20 @@ class QPropertyAnimationPrivate : public QVariantAnimationPrivate { Q_DECLARE_PUBLIC(QPropertyAnimation) public: - QPropertyAnimationPrivate() - : targetValue(nullptr), propertyType(0), propertyIndex(-1) - { - } + QPropertyAnimationPrivate() : propertyType(0), propertyIndex(-1) { } - QPointer target; - //we use targetValue to be able to unregister the target from the global hash - QObject *targetValue; + void setTargetObjectForwarder(QObject *target) { q_func()->setTargetObject(target); } + Q_OBJECT_COMPAT_PROPERTY_WITH_ARGS(QPropertyAnimationPrivate, QObject *, targetObject, + &QPropertyAnimationPrivate::setTargetObjectForwarder, + nullptr) + void targetObjectDestroyed() + { + // stop() has to be called before targetObject is set to nullptr. + // targetObject must not be nullptr in states unequal to "Stopped". + q_func()->stop(); + targetObject.setValueBypassingBindings(nullptr); + targetObject.notify(); + } //for the QProperty int propertyType; diff --git a/tests/auto/corelib/animation/qpropertyanimation/tst_qpropertyanimation.cpp b/tests/auto/corelib/animation/qpropertyanimation/tst_qpropertyanimation.cpp index 5286daec6ac..1da69f83385 100644 --- a/tests/auto/corelib/animation/qpropertyanimation/tst_qpropertyanimation.cpp +++ b/tests/auto/corelib/animation/qpropertyanimation/tst_qpropertyanimation.cpp @@ -1403,11 +1403,26 @@ void tst_QPropertyAnimation::recursiveAnimations() void tst_QPropertyAnimation::bindings() { - AnimationObject o; - QPropertyAnimation a(&o, "value"); + std::unique_ptr o1(new AnimationObject); + std::unique_ptr o2(new AnimationObject); + QPropertyAnimation a(o1.get(), "value"); - QTestPrivate::testReadWritePropertyBasics(a, QByteArray("value"), QByteArray("realValue"), - "propertyName"); + QTestPrivate::testReadWritePropertyBasics( + a, QByteArray("realValue"), QByteArray("value"), "propertyName"); + if (QTest::currentTestFailed()) { + qDebug() << "Failed property test for QPropertyAnimation::propertyName"; + return; + } + QTestPrivate::testReadWritePropertyBasics(a, o2.get(), o1.get(), + "targetObject"); + if (QTest::currentTestFailed()) { + qDebug() << "Failed property test for QPropertyAnimation::targetObject"; + return; + } + + a.setTargetObject(o1.get()); + o1.reset(); + QCOMPARE(a.targetObject(), nullptr); } QTEST_MAIN(tst_QPropertyAnimation)