diff options
-rw-r--r-- | src/qml/jsruntime/qv4referenceobject_p.h | 10 | ||||
-rw-r--r-- | src/qml/qml/qqmlproperty.cpp | 26 | ||||
-rw-r--r-- | src/qml/qml/qqmlpropertydata_p.h | 3 | ||||
-rw-r--r-- | src/qml/qml/qqmlvaluetype.cpp | 5 | ||||
-rw-r--r-- | src/qml/qml/qqmlvaluetype_p.h | 4 | ||||
-rw-r--r-- | src/qml/qml/qqmlvaluetypewrapper.cpp | 11 | ||||
-rw-r--r-- | src/qml/qml/qqmlvaluetypewrapper_p.h | 2 | ||||
-rw-r--r-- | src/qml/qml/qqmlvmemetaobject.cpp | 116 | ||||
-rw-r--r-- | tests/auto/qml/qqmlmetatype/data/animationOnValueType.qml | 26 | ||||
-rw-r--r-- | tests/auto/qml/qqmlmetatype/tst_qqmlmetatype.cpp | 13 |
10 files changed, 139 insertions, 77 deletions
diff --git a/src/qml/jsruntime/qv4referenceobject_p.h b/src/qml/jsruntime/qv4referenceobject_p.h index ad6a7b5817..5378a0c362 100644 --- a/src/qml/jsruntime/qv4referenceobject_p.h +++ b/src/qml/jsruntime/qv4referenceobject_p.h @@ -110,6 +110,8 @@ struct ReferenceObject : public Object V4_NEEDS_DESTROY public: + static constexpr const int AllProperties = -1; + template<typename HeapObject> static bool readReference(HeapObject *ref) { @@ -131,7 +133,7 @@ public: } template<typename HeapObject> - static bool writeBack(HeapObject *ref) + static bool writeBack(HeapObject *ref, int internalIndex = AllProperties) { if (!ref->object() || !ref->canWriteBack()) return false; @@ -139,15 +141,15 @@ public: QV4::Scope scope(ref->internalClass->engine); QV4::ScopedObject object(scope, ref->object()); - int flags = 0; + int flags = QQmlPropertyData::HasInternalIndex; int status = -1; if (ref->isVariant()) { QVariant variant = ref->toVariant(); - void *a[] = { &variant, nullptr, &status, &flags }; + void *a[] = { &variant, nullptr, &status, &flags, &internalIndex }; return object->metacall(QMetaObject::WriteProperty, ref->property(), a); } - void *a[] = { ref->storagePointer(), nullptr, &status, &flags }; + void *a[] = { ref->storagePointer(), nullptr, &status, &flags, &internalIndex }; return object->metacall(QMetaObject::WriteProperty, ref->property(), a); } diff --git a/src/qml/qml/qqmlproperty.cpp b/src/qml/qml/qqmlproperty.cpp index cfdcc9ea60..18e2c417ff 100644 --- a/src/qml/qml/qqmlproperty.cpp +++ b/src/qml/qml/qqmlproperty.cpp @@ -1271,29 +1271,31 @@ static void removeValuePropertyBinding( template<typename Op> bool changePropertyAndWriteBack( QObject *object, int coreIndex, QQmlGadgetPtrWrapper *wrapper, - QQmlPropertyData::WriteFlags flags, Op op) + QQmlPropertyData::WriteFlags flags, int internalIndex, Op op) { wrapper->read(object, coreIndex); const bool rv = op(wrapper); - wrapper->write(object, coreIndex, flags); + wrapper->write(object, coreIndex, flags, internalIndex); return rv; } template<typename Op> bool changeThroughGadgetPtrWrapper( - QObject *object, const QQmlPropertyData &core, - const QQmlRefPointer<QQmlContextData> &context, QQmlPropertyData::WriteFlags flags, - Op op) + QObject *object, const QQmlPropertyData &core, + const QQmlRefPointer<QQmlContextData> &context, QQmlPropertyData::WriteFlags flags, + int internalIndex, Op op) { if (QQmlGadgetPtrWrapper *wrapper = context ? QQmlGadgetPtrWrapper::instance(context->engine(), core.propType()) : nullptr) { - return changePropertyAndWriteBack(object, core.coreIndex(), wrapper, flags, op); + return changePropertyAndWriteBack( + object, core.coreIndex(), wrapper, flags, internalIndex, op); } if (QQmlValueType *valueType = QQmlMetaType::valueType(core.propType())) { QQmlGadgetPtrWrapper wrapper(valueType, nullptr); - return changePropertyAndWriteBack(object, core.coreIndex(), &wrapper, flags, op); + return changePropertyAndWriteBack( + object, core.coreIndex(), &wrapper, flags, internalIndex, op); } return false; @@ -1309,8 +1311,9 @@ bool QQmlPropertyPrivate::writeValueProperty( if (!valueTypeData.isValid()) return write(object, core, value, context, flags); - return changeThroughGadgetPtrWrapper(object, core, context, flags, - [&](QQmlGadgetPtrWrapper *wrapper) { + return changeThroughGadgetPtrWrapper( + object, core, context, flags | QQmlPropertyData::HasInternalIndex, + valueTypeData.coreIndex(), [&](QQmlGadgetPtrWrapper *wrapper) { return write(wrapper, valueTypeData, value, context, flags); }); } @@ -1324,8 +1327,9 @@ bool QQmlPropertyPrivate::resetValueProperty( if (!valueTypeData.isValid()) return reset(object, core, flags); - return changeThroughGadgetPtrWrapper(object, core, context, flags, - [&](QQmlGadgetPtrWrapper *wrapper) { + return changeThroughGadgetPtrWrapper( + object, core, context, flags | QQmlPropertyData::HasInternalIndex, + valueTypeData.coreIndex(), [&](QQmlGadgetPtrWrapper *wrapper) { return reset(wrapper, valueTypeData, flags); }); } diff --git a/src/qml/qml/qqmlpropertydata_p.h b/src/qml/qml/qqmlpropertydata_p.h index b51b0ba619..19a045107d 100644 --- a/src/qml/qml/qqmlpropertydata_p.h +++ b/src/qml/qml/qqmlpropertydata_p.h @@ -28,7 +28,8 @@ public: enum WriteFlag { BypassInterceptor = 0x01, DontRemoveBinding = 0x02, - RemoveBindingOnAliasWrite = 0x04 + RemoveBindingOnAliasWrite = 0x04, + HasInternalIndex = 0x8, }; Q_DECLARE_FLAGS(WriteFlags, WriteFlag) diff --git a/src/qml/qml/qqmlvaluetype.cpp b/src/qml/qml/qqmlvaluetype.cpp index f234cb1312..428ce9b697 100644 --- a/src/qml/qml/qqmlvaluetype.cpp +++ b/src/qml/qml/qqmlvaluetype.cpp @@ -43,11 +43,12 @@ void QQmlGadgetPtrWrapper::read(QObject *obj, int idx) QMetaObject::metacall(obj, QMetaObject::ReadProperty, idx, a); } -void QQmlGadgetPtrWrapper::write(QObject *obj, int idx, QQmlPropertyData::WriteFlags flags) const +void QQmlGadgetPtrWrapper::write( + QObject *obj, int idx, QQmlPropertyData::WriteFlags flags, int internalIndex) const { Q_ASSERT(m_gadgetPtr); int status = -1; - void *a[] = { m_gadgetPtr, nullptr, &status, &flags }; + void *a[] = { m_gadgetPtr, nullptr, &status, &flags, &internalIndex }; QMetaObject::metacall(obj, QMetaObject::WriteProperty, idx, a); } diff --git a/src/qml/qml/qqmlvaluetype_p.h b/src/qml/qml/qqmlvaluetype_p.h index 779ba01f12..bc7a210cdf 100644 --- a/src/qml/qml/qqmlvaluetype_p.h +++ b/src/qml/qml/qqmlvaluetype_p.h @@ -19,6 +19,7 @@ #include <private/qqmlnullablevalue_p.h> #include <private/qmetatype_p.h> +#include <private/qv4referenceobject_p.h> #include <QtCore/qobject.h> #include <QtCore/qrect.h> @@ -69,7 +70,8 @@ public: ~QQmlGadgetPtrWrapper(); void read(QObject *obj, int idx); - void write(QObject *obj, int idx, QQmlPropertyData::WriteFlags flags) const; + void write(QObject *obj, int idx, QQmlPropertyData::WriteFlags flags, + int internalIndex = QV4::ReferenceObject::AllProperties) const; QVariant value() const; void setValue(const QVariant &value); diff --git a/src/qml/qml/qqmlvaluetypewrapper.cpp b/src/qml/qml/qqmlvaluetypewrapper.cpp index 5ad5747e7f..4ef757b8e1 100644 --- a/src/qml/qml/qqmlvaluetypewrapper.cpp +++ b/src/qml/qml/qqmlvaluetypewrapper.cpp @@ -118,9 +118,9 @@ bool Heap::QQmlValueTypeWrapper::readReference() return enforcesLocation() || QV4::ReferenceObject::readReference(this); } -bool Heap::QQmlValueTypeWrapper::writeBack() +bool Heap::QQmlValueTypeWrapper::writeBack(int propertyIndex) { - return isAttachedToProperty() && QV4::ReferenceObject::writeBack(this); + return isAttachedToProperty() && QV4::ReferenceObject::writeBack(this, propertyIndex); } ReturnedValue QQmlValueTypeWrapper::create( @@ -178,9 +178,12 @@ int QQmlValueTypeWrapper::virtualMetacall( switch (call) { case QMetaObject::ReadProperty: break; - case QMetaObject::InvokeMetaMethod: case QMetaObject::WriteProperty: case QMetaObject::ResetProperty: + if (wrapper->d()->object()) + wrapper->d()->writeBack(index); + break; + case QMetaObject::InvokeMetaMethod: case QMetaObject::CustomCall: if (wrapper->d()->object()) wrapper->d()->writeBack(); @@ -840,7 +843,7 @@ bool QQmlValueTypeWrapper::virtualPut(Managed *m, PropertyKey id, const Value &v property.writeOnGadget(gadget, v); if (heapObject) - r->d()->writeBack(); + r->d()->writeBack(pd.coreIndex()); return true; } diff --git a/src/qml/qml/qqmlvaluetypewrapper_p.h b/src/qml/qml/qqmlvaluetypewrapper_p.h index cb8bfdf106..0c4cbb7dd3 100644 --- a/src/qml/qml/qqmlvaluetypewrapper_p.h +++ b/src/qml/qml/qqmlvaluetypewrapper_p.h @@ -73,7 +73,7 @@ DECLARE_HEAP_OBJECT(QQmlValueTypeWrapper, ReferenceObject) { bool setVariant(const QVariant &variant); bool readReference(); - bool writeBack(); + bool writeBack(int propertyIndex = QV4::ReferenceObject::AllProperties); private: void setMetaObject(const QMetaObject *metaObject) { m_metaObject = metaObject; } diff --git a/src/qml/qml/qqmlvmemetaobject.cpp b/src/qml/qml/qqmlvmemetaobject.cpp index 05b9328d99..c863fa6647 100644 --- a/src/qml/qml/qqmlvmemetaobject.cpp +++ b/src/qml/qml/qqmlvmemetaobject.cpp @@ -275,63 +275,72 @@ bool QQmlInterceptorMetaObject::doIntercept(QMetaObject::Call c, int id, void ** if (metaType.isValid()) { if (valueIndex != -1 && c == QMetaObject::WriteProperty) { - // TODO: handle intercepting bindable properties for value types? - QQmlGadgetPtrWrapper *valueType = QQmlGadgetPtrWrapper::instance( - data->context->engine(), metaType); - Q_ASSERT(valueType); - - // - // Consider the following case: - // color c = { 0.1, 0.2, 0.3 } - // interceptor exists on c.r - // write { 0.2, 0.4, 0.6 } - // - // The interceptor may choose not to update the r component at this - // point (for example, a behavior that creates an animation). But we - // need to ensure that the g and b components are updated correctly. - // - // So we need to perform a full write where the value type is: - // r = old value, g = new value, b = new value - // - // And then call the interceptor which may or may not write the - // new value to the r component. - // - // This will ensure that the other components don't contain stale data - // and any relevant signals are emitted. - // - // To achieve this: - // (1) Store the new value type as a whole (needed due to - // aliasing between a[0] and static storage in value type). - // (2) Read the entire existing value type from object -> valueType temp. - // (3) Read the previous value of the component being changed - // from the valueType temp. - // (4) Write the entire new value type into the temp. - // (5) Overwrite the component being changed with the old value. - // (6) Perform a full write to the value type (which may emit signals etc). - // (7) Issue the interceptor call with the new component value. - // - - QMetaProperty valueProp = valueType->property(valueIndex); - QVariant newValue(metaType, a[0]); - - valueType->read(object, id); - QVariant prevComponentValue = valueType->readOnGadget(valueProp); - - valueType->setValue(newValue); - QVariant newComponentValue = valueType->readOnGadget(valueProp); - - // Don't apply the interceptor if the intercepted value has not changed - bool updated = false; - if (newComponentValue != prevComponentValue) { + + // If we didn't intend to change the property this interceptor cares about, + // then don't bother intercepting it. There may be an animation running on + // the property. We shouldn't disturb it. + const int changedProperty + = (*static_cast<int *>(a[3]) & QQmlPropertyData::HasInternalIndex) + ? *static_cast<int *>(a[4]) + : QV4::ReferenceObject::AllProperties; + if (changedProperty == QV4::ReferenceObject::AllProperties + || changedProperty == valueIndex) { + // TODO: handle intercepting bindable properties for value types? + QQmlGadgetPtrWrapper *valueType = QQmlGadgetPtrWrapper::instance( + data->context->engine(), metaType); + Q_ASSERT(valueType); + + // + // Consider the following case: + // color c = { 0.1, 0.2, 0.3 } + // interceptor exists on c.r + // write { 0.2, 0.4, 0.6 } + // + // The interceptor may choose not to update the r component at this + // point (for example, a behavior that creates an animation). But we + // need to ensure that the g and b components are updated correctly. + // + // So we need to perform a full write where the value type is: + // r = old value, g = new value, b = new value + // + // And then call the interceptor which may or may not write the + // new value to the r component. + // + // This will ensure that the other components don't contain stale data + // and any relevant signals are emitted. + // + // To achieve this: + // (1) Store the new value type as a whole (needed due to + // aliasing between a[0] and static storage in value type). + // (2) Read the entire existing value type from object -> valueType temp. + // (3) Read the previous value of the component being changed + // from the valueType temp. + // (4) Write the entire new value type into the temp. + // (5) Overwrite the component being changed with the old value. + // (6) Perform a full write to the value type (which may emit signals etc). + // (7) Issue the interceptor call with the new component value. + // + + QMetaProperty valueProp = valueType->property(valueIndex); + QVariant newValue(metaType, a[0]); + + valueType->read(object, id); + QVariant prevComponentValue = valueType->readOnGadget(valueProp); + + valueType->setValue(newValue); + QVariant newComponentValue = valueType->readOnGadget(valueProp); + + // If the intercepted value seemingly has not changed, we still need to + // invoke the interceptor. There may be a pending animation that will + // change the value soon. Such an animation needs to be canceled if the + // current value is explicitly set. + // So, we cannot return here if prevComponentValue == newComponentValue. valueType->writeOnGadget(valueProp, prevComponentValue); valueType->write(object, id, QQmlPropertyData::DontRemoveBinding | QQmlPropertyData::BypassInterceptor); vi->write(newComponentValue); - updated = true; - } - - if (updated) return true; + } } else if (c == QMetaObject::WriteProperty) { vi->write(QVariant(metaType, a[0])); return true; @@ -993,7 +1002,8 @@ int QQmlVMEMetaObject::metaCall(QObject *o, QMetaObject::Call c, int _id, void * int rv = QMetaObject::metacall(valueType, c, valueTypePropertyIndex, a); if (c == QMetaObject::WriteProperty) - valueType->write(target, coreIndex, {}); + valueType->write(target, coreIndex, QQmlPropertyData::HasInternalIndex, + valueTypePropertyIndex); return rv; } else { diff --git a/tests/auto/qml/qqmlmetatype/data/animationOnValueType.qml b/tests/auto/qml/qqmlmetatype/data/animationOnValueType.qml new file mode 100644 index 0000000000..196715dd74 --- /dev/null +++ b/tests/auto/qml/qqmlmetatype/data/animationOnValueType.qml @@ -0,0 +1,26 @@ +import QtQuick + +Text { + property bool large: false + property bool check: false + font.pointSize: large ? 24 : 12 + font.letterSpacing: check ? 24 : 12 + + Behavior on font.pointSize { + SmoothedAnimation { duration: 100; } + } + + Behavior on font.letterSpacing { + SmoothedAnimation { duration: 100; } + } + + Component.onCompleted: { + large = true; + large = false; + check = true; + } + + property real pointSize: font.pointSize + property real letterSpacing: font.letterSpacing +} + diff --git a/tests/auto/qml/qqmlmetatype/tst_qqmlmetatype.cpp b/tests/auto/qml/qqmlmetatype/tst_qqmlmetatype.cpp index 6f4221539d..c1bac33d87 100644 --- a/tests/auto/qml/qqmlmetatype/tst_qqmlmetatype.cpp +++ b/tests/auto/qml/qqmlmetatype/tst_qqmlmetatype.cpp @@ -49,6 +49,8 @@ private slots: void enumsInRecursiveImport_data(); void enumsInRecursiveImport(); + + void revertValueTypeAnimation(); }; class TestType : public QObject @@ -711,6 +713,17 @@ void tst_qqmlmetatype::enumsInRecursiveImport() QTRY_COMPARE(obj->property("color").toString(), QString("green")); } +void tst_qqmlmetatype::revertValueTypeAnimation() +{ + QQmlEngine engine; + QQmlComponent c(&engine, testFileUrl("animationOnValueType.qml")); + QVERIFY2(c.isReady(), qPrintable(c.errorString())); + + QScopedPointer<QObject> o(c.create()); + QTRY_COMPARE(o->property("letterSpacing").toDouble(), 24.0); + QCOMPARE(o->property("pointSize").toDouble(), 12.0); +} + QTEST_MAIN(tst_qqmlmetatype) #include "tst_qqmlmetatype.moc" |