From 6a24ac7c4e0a7737b0cd0738c4e6fe8b9ac589ca Mon Sep 17 00:00:00 2001 From: Fabian Kosmale Date: Thu, 18 Jun 2020 16:55:48 +0200 Subject: [PATCH] QNotifiedProperty: pass old value to callback if requested Check at compile time whether the static callback takes an argument (which has to be of the same time as the type of the property). If so, retrieve the old value and pass it to the callback. Change-Id: Ib1c4c9e05b826b6be492b03f66fa72ad015963ee Reviewed-by: Ulf Hermann --- src/corelib/kernel/qproperty.cpp | 2 +- src/corelib/kernel/qproperty.h | 99 ++++++++++++++----- src/corelib/kernel/qpropertybinding.cpp | 11 ++- src/corelib/kernel/qpropertybinding_p.h | 4 +- src/corelib/kernel/qpropertyprivate.h | 2 +- .../kernel/qproperty/tst_qproperty.cpp | 22 +++++ 6 files changed, 110 insertions(+), 30 deletions(-) diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp index a1b1d7b127..7ee035030f 100644 --- a/src/corelib/kernel/qproperty.cpp +++ b/src/corelib/kernel/qproperty.cpp @@ -95,7 +95,7 @@ QPropertyBase::~QPropertyBase() QUntypedPropertyBinding QPropertyBase::setBinding(const QUntypedPropertyBinding &binding, void *propertyDataPtr, void *staticObserver, - void (*staticObserverCallback)(void*)) + void (*staticObserverCallback)(void*, void*)) { QPropertyBindingPrivatePtr oldBinding; QPropertyBindingPrivatePtr newBinding = binding.d; diff --git a/src/corelib/kernel/qproperty.h b/src/corelib/kernel/qproperty.h index f0156fac02..b1b4dbeae1 100644 --- a/src/corelib/kernel/qproperty.h +++ b/src/corelib/kernel/qproperty.h @@ -374,11 +374,21 @@ namespace Qt { } } -template -class QNotifiedProperty +namespace detail { + template + struct ExtractClassFromFunctionPointer; + + template + struct ExtractClassFromFunctionPointer { using Class = C; }; +} + +template +class QNotifiedProperty { public: using value_type = T; + using Class = typename detail::ExtractClassFromFunctionPointer::Class; + static_assert(std::is_invocable_v || std::is_invocable_v); QNotifiedProperty() = default; @@ -420,45 +430,83 @@ public: void setValue(Class *owner, T &&newValue) { - if (d.setValueAndReturnTrueIfChanged(std::move(newValue))) - notify(owner); + if constexpr(std::is_invocable_v) { + if (d.setValueAndReturnTrueIfChanged(std::move(newValue))) + notify(owner); + } else { + T oldValue = value(); // TODO: kind of pointless if there was no change + if (d.setValueAndReturnTrueIfChanged(std::move(newValue))) + notify(owner, &oldValue); + } d.priv.removeBinding(); } void setValue(Class *owner, const T &newValue) { - if (d.setValueAndReturnTrueIfChanged(newValue)) - notify(owner); + if constexpr(std::is_invocable_v) { + if (d.setValueAndReturnTrueIfChanged(newValue)) + notify(owner); + } else { + T oldValue = value(); + if (d.setValueAndReturnTrueIfChanged(newValue)) + notify(owner, &oldValue); + } d.priv.removeBinding(); } QPropertyBinding setBinding(Class *owner, const QPropertyBinding &newBinding) { - QPropertyBinding oldBinding(d.priv.setBinding(newBinding, &d, owner, [](void *o) { - (reinterpret_cast(o)->*Callback)(); - })); - notify(owner); - return oldBinding; + if constexpr(std::is_invocable_v) { + QPropertyBinding oldBinding(d.priv.setBinding(newBinding, &d, owner, [](void *o) { + (reinterpret_cast(o)->*Callback)(); + })); + notify(owner); + return oldBinding; + } else { + T oldValue = value(); + QPropertyBinding oldBinding(d.priv.setBinding(newBinding, &d, owner, [](void *o, void *oldValue) { + (reinterpret_cast(o)->*Callback)(*reinterpret_cast(oldValue)); + })); + notify(owner, &oldValue); + return oldBinding; + } } QPropertyBinding setBinding(Class *owner, QPropertyBinding &&newBinding) { QPropertyBinding b(std::move(newBinding)); - QPropertyBinding oldBinding(d.priv.setBinding(b, &d, owner, [](void *o) { - (reinterpret_cast(o)->*Callback)(); - })); - notify(owner); - return oldBinding; + if constexpr(std::is_invocable_v) { + QPropertyBinding oldBinding(d.priv.setBinding(b, &d, owner, [](void *o, void *) { + (reinterpret_cast(o)->*Callback)(); + })); + notify(owner); + return oldBinding; + } else { + T oldValue = value(); + QPropertyBinding oldBinding(d.priv.setBinding(b, &d, owner, [](void *o, void *oldValue) { + (reinterpret_cast(o)->*Callback)(*reinterpret_cast(oldValue)); + })); + notify(owner, &oldValue); + return oldBinding; + } } bool setBinding(Class *owner, const QUntypedPropertyBinding &newBinding) { if (newBinding.valueMetaType().id() != qMetaTypeId()) return false; - d.priv.setBinding(newBinding, &d, owner, [](void *o) { - (reinterpret_cast(o)->*Callback)(); - }); - notify(owner); + if constexpr(std::is_invocable_v) { + d.priv.setBinding(newBinding, &d, owner, [](void *o, void *) { + (reinterpret_cast(o)->*Callback)(); + }); + notify(owner); + } else { + T oldValue = value(); + d.priv.setBinding(newBinding, &d, owner, [](void *o, void *oldValue) { + (reinterpret_cast(o)->*Callback)(*reinterpret_cast(oldValue)); + }); + notify(owner, &oldValue); + } return true; } @@ -493,10 +541,13 @@ public: QPropertyChangeHandler subscribe(Functor f); private: - void notify(Class *owner) + void notify(Class *owner, T *oldValue=nullptr) { d.priv.notifyObservers(&d); - (owner->*Callback)(); + if constexpr(std::is_invocable_v) + (owner->*Callback)(); + else + (owner->*Callback)(*oldValue); } Q_DISABLE_COPY_MOVE(QNotifiedProperty) @@ -624,7 +675,7 @@ QPropertyChangeHandler QProperty::subscribe(Functor f) return onValueChanged(f); } -template +template template QPropertyChangeHandler QNotifiedProperty::onValueChanged(Functor f) { @@ -634,7 +685,7 @@ QPropertyChangeHandler QNotifiedProperty::onValueChanged(F return QPropertyChangeHandler(*this, f); } -template +template template QPropertyChangeHandler QNotifiedProperty::subscribe(Functor f) { diff --git a/src/corelib/kernel/qpropertybinding.cpp b/src/corelib/kernel/qpropertybinding.cpp index cbb9752e2e..3a0f54b44a 100644 --- a/src/corelib/kernel/qpropertybinding.cpp +++ b/src/corelib/kernel/qpropertybinding.cpp @@ -69,8 +69,15 @@ void QPropertyBindingPrivate::markDirtyAndNotifyObservers() dirty = true; if (firstObserver) firstObserver.notify(this, propertyDataPtr); - if (hasStaticObserver) - staticObserverCallback(staticObserver); + if (hasStaticObserver) { + if (metaType == QMetaType::fromType()) { + auto propertyPtr = reinterpret_cast(propertyDataPtr); + bool oldValue = propertyPtr->extraBit(); + staticObserverCallback(staticObserver, &oldValue); + } else { + staticObserverCallback(staticObserver, propertyDataPtr); + } + } } bool QPropertyBindingPrivate::evaluateIfDirtyAndReturnTrueIfValueChanged() diff --git a/src/corelib/kernel/qpropertybinding_p.h b/src/corelib/kernel/qpropertybinding_p.h index e03a23109a..99a6b22b4b 100644 --- a/src/corelib/kernel/qpropertybinding_p.h +++ b/src/corelib/kernel/qpropertybinding_p.h @@ -80,7 +80,7 @@ private: ObserverArray inlineDependencyObservers; struct { void *staticObserver; - void (*staticObserverCallback)(void*); + void (*staticObserverCallback)(void*, void*); }; }; QScopedPointer> heapObservers; @@ -107,7 +107,7 @@ public: void setDirty(bool d) { dirty = d; } void setProperty(void *propertyPtr) { propertyDataPtr = propertyPtr; } - void setStaticObserver(void *observer, void (*callback)(void*)) + void setStaticObserver(void *observer, void (*callback)(void*, void*)) { if (observer) { if (!hasStaticObserver) { diff --git a/src/corelib/kernel/qpropertyprivate.h b/src/corelib/kernel/qpropertyprivate.h index 7a2c6a5b77..43318f25c0 100644 --- a/src/corelib/kernel/qpropertyprivate.h +++ b/src/corelib/kernel/qpropertyprivate.h @@ -84,7 +84,7 @@ public: QUntypedPropertyBinding setBinding(const QUntypedPropertyBinding &newBinding, void *propertyDataPtr, void *staticObserver = nullptr, - void (*staticObserverCallback)(void*) = nullptr); + void (*staticObserverCallback)(void *, void *) = nullptr); QPropertyBindingPrivate *binding(); void evaluateIfDirty(); diff --git a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp index e606d121ec..7200bf176e 100644 --- a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp +++ b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp @@ -72,6 +72,7 @@ private slots: void multipleObservers(); void propertyAlias(); void notifiedProperty(); + void notifiedPropertyWithOldValueCallback(); }; void tst_QProperty::functorBinding() @@ -779,6 +780,15 @@ struct ClassWithNotifiedProperty QNotifiedProperty property; }; +struct ClassWithNotifiedProperty2 +{ + QVector recordedValues; + + void callback(int oldValue) { recordedValues << oldValue; } + + QNotifiedProperty property; +}; + void tst_QProperty::notifiedProperty() { ClassWithNotifiedProperty instance; @@ -854,6 +864,18 @@ void tst_QProperty::notifiedProperty() QCOMPARE(subscribedCount, 10); } +void tst_QProperty::notifiedPropertyWithOldValueCallback() +{ + ClassWithNotifiedProperty2 instance; + instance.property.setValue(&instance, 1); + instance.property.setBinding(&instance, [](){return 2;}); + instance.property.setBinding(&instance, [](){return 3;}); + instance.property.setValue(&instance, 4); + QVector expected {0, 1, 2, 3}; + QCOMPARE(instance.recordedValues, expected); + QCOMPARE(instance.property.value(), 4); +} + QTEST_MAIN(tst_QProperty); #include "tst_qproperty.moc"