From 524d78160726b25ed424a2c7a6d5e423b7ea4b93 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Mon, 25 May 2020 12:51:39 +0200 Subject: [PATCH] QProperty: Support multiple observers Previously, only the first observer would get notified. Also, make sure that the notifiers are always retained when switching between bindings and values. Change-Id: I9c25c0f2e288dac3a335b68e618f7ddeb44be25a Reviewed-by: Fabian Kosmale --- src/corelib/kernel/qproperty.cpp | 37 +++++++++++-------- src/corelib/kernel/qproperty.h | 4 +- src/corelib/kernel/qproperty_p.h | 1 + src/corelib/kernel/qpropertybinding_p.h | 7 ++++ .../kernel/qproperty/tst_qproperty.cpp | 36 ++++++++++++++++++ 5 files changed, 68 insertions(+), 17 deletions(-) diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp index 7554e91aba..4ed4af4c8e 100644 --- a/src/corelib/kernel/qproperty.cpp +++ b/src/corelib/kernel/qproperty.cpp @@ -83,8 +83,11 @@ void QPropertyBase::moveAssign(QPropertyBase &&other, void *propertyDataPtr) QPropertyBase::~QPropertyBase() { QPropertyBasePointer d{this}; - if (auto observer = d.firstObserver()) + for (auto observer = d.firstObserver(); observer;) { + auto next = observer.nextObserver(); observer.unlink(); + observer = next; + } if (auto binding = d.bindingPtr()) binding->unlinkAndDeref(); } @@ -95,18 +98,19 @@ QUntypedPropertyBinding QPropertyBase::setBinding(const QUntypedPropertyBinding QPropertyBindingPrivatePtr newBinding = binding.d; QPropertyBasePointer d{this}; - - auto observer = d.firstObserver(); - if (observer) - observer.unlink(); + QPropertyObserverPointer observer; if (auto *existingBinding = d.bindingPtr()) { if (existingBinding == newBinding.data()) return QUntypedPropertyBinding(oldBinding.data()); oldBinding = QPropertyBindingPrivatePtr(existingBinding); + observer = oldBinding->takeObservers(); oldBinding->unlinkAndDeref(); d_ptr &= FlagMask; + } else { + observer = d.firstObserver(); } + if (newBinding) { newBinding.data()->ref.ref(); d_ptr = (d_ptr & FlagMask) | reinterpret_cast(newBinding.data()); @@ -115,8 +119,10 @@ QUntypedPropertyBinding QPropertyBase::setBinding(const QUntypedPropertyBinding newBinding->setProperty(propertyDataPtr); if (observer) newBinding->prependObserver(observer); + } else if (observer) { + d.setObservers(observer.ptr); } else { - d_ptr &= ~BindingBit; + d_ptr &= ~QPropertyBase::BindingBit; } return QUntypedPropertyBinding(oldBinding.data()); @@ -137,6 +143,12 @@ QPropertyBindingPrivate *QPropertyBasePointer::bindingPtr() const return nullptr; } +void QPropertyBasePointer::setObservers(QPropertyObserver *observer) +{ + observer->prev = reinterpret_cast(&(ptr->d_ptr)); + ptr->d_ptr = (reinterpret_cast(observer) & ~QPropertyBase::FlagMask); +} + void QPropertyBasePointer::addObserver(QPropertyObserver *observer) { if (auto *binding = bindingPtr()) { @@ -199,18 +211,13 @@ void QPropertyBase::removeBinding() { QPropertyBasePointer d{this}; - auto observer = d.firstObserver(); - if (observer) - observer.unlink(); - if (auto *existingBinding = d.bindingPtr()) { + auto observer = existingBinding->takeObservers(); existingBinding->unlinkAndDeref(); - d_ptr &= FlagMask; + d_ptr &= ExtraBit; + if (observer) + d.setObservers(observer.ptr); } - d_ptr &= ~BindingBit; - - if (observer) - observer.observeProperty(d); } void QPropertyBase::registerWithCurrentlyEvaluatingBinding() const diff --git a/src/corelib/kernel/qproperty.h b/src/corelib/kernel/qproperty.h index 22313bc92e..938de61fd5 100644 --- a/src/corelib/kernel/qproperty.h +++ b/src/corelib/kernel/qproperty.h @@ -254,16 +254,16 @@ public: void setValue(T &&newValue) { + d.priv.removeBinding(); if (d.setValueAndReturnTrueIfChanged(std::move(newValue))) notify(); - d.priv.removeBinding(); } void setValue(const T &newValue) { + d.priv.removeBinding(); if (d.setValueAndReturnTrueIfChanged(newValue)) notify(); - d.priv.removeBinding(); } QProperty &operator=(T &&newValue) diff --git a/src/corelib/kernel/qproperty_p.h b/src/corelib/kernel/qproperty_p.h index 13cdb311c1..6638785ccf 100644 --- a/src/corelib/kernel/qproperty_p.h +++ b/src/corelib/kernel/qproperty_p.h @@ -65,6 +65,7 @@ struct Q_AUTOTEST_EXPORT QPropertyBasePointer QPropertyBindingPrivate *bindingPtr() const; + void setObservers(QPropertyObserver *observer); void addObserver(QPropertyObserver *observer); void setFirstObserver(QPropertyObserver *observer); QPropertyObserverPointer firstObserver() const; diff --git a/src/corelib/kernel/qpropertybinding_p.h b/src/corelib/kernel/qpropertybinding_p.h index 66e969547c..7c4166592b 100644 --- a/src/corelib/kernel/qpropertybinding_p.h +++ b/src/corelib/kernel/qpropertybinding_p.h @@ -101,6 +101,13 @@ public: firstObserver = observer; } + QPropertyObserverPointer takeObservers() + { + auto observers = firstObserver; + firstObserver.ptr = nullptr; + return observers; + } + void clearDependencyObservers() { for (size_t i = 0; i < inlineDependencyObservers.size(); ++i) { QPropertyObserver empty; diff --git a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp index 2cfcc8e22f..a8846e0987 100644 --- a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp +++ b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp @@ -69,6 +69,7 @@ private slots: void genericPropertyBindingBool(); void staticChangeHandler(); void setBindingFunctor(); + void multipleObservers(); }; void tst_QProperty::functorBinding() @@ -686,6 +687,41 @@ void tst_QProperty::setBindingFunctor() QCOMPARE(property.value(), 200); } +void tst_QProperty::multipleObservers() +{ + QProperty property; + property.setValue(5); + QCOMPARE(property.value(), 5); + + int value1 = 1; + auto changeHandler = property.onValueChanged([&]() { value1 = property.value(); }); + QCOMPARE(value1, 1); + + int value2 = 2; + auto subscribeHandler = property.subscribe([&]() { value2 = property.value(); }); + QCOMPARE(value2, 5); + + property.setValue(6); + QCOMPARE(property.value(), 6); + QCOMPARE(value1, 6); + QCOMPARE(value2, 6); + + property.setBinding([]() { return 12; }); + QCOMPARE(value1, 12); + QCOMPARE(value2, 12); + QCOMPARE(property.value(), 12); + + property.setBinding(QPropertyBinding()); + QCOMPARE(value1, 12); + QCOMPARE(value2, 12); + QCOMPARE(property.value(), 12); + + property.setValue(22); + QCOMPARE(value1, 22); + QCOMPARE(value2, 22); + QCOMPARE(property.value(), 22); +} + QTEST_MAIN(tst_QProperty); #include "tst_qproperty.moc"