1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix ChangeHandler notification for eager properties

Browse Source 
ChangeHandler's evaluated the binding to detect if the value actually
changed. This is a valid strategy for lazy bindings, but eager bindings
were already evaluated at that point, and thus the change would not be
detected.
Change the binding loop test, so that there isn't a fixpoint in the
binding loop, and we can still detect it. Changing the binding loop
detection code to deal with this case is left as an exercise for the
future.

Change-Id: Ia5d9ce2cd98a5780e69c993b5824024eb186c154
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
This commit is contained in:
Fabian Kosmale 2020-09-29 11:18:36 +02:00
parent d90647b26a
commit 8572f84467
3 changed files with 19 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
src/corelib/kernel/qproperty.cpp
View File

@ -239,8 +239,11 @@ QUntypedPropertyBinding QPropertyBindingData::setBinding(const QUntypedPropertyB
if (observer)
newBinding->prependObserver(observer);
newBinding->setStaticObserver(staticObserverCallback, guardCallback);
if (newBinding->requiresEagerEvaluation())
newBinding->evaluateIfDirtyAndReturnTrueIfValueChanged(propertyDataPtr);
if (newBinding->requiresEagerEvaluation()) {
auto changed = newBinding->evaluateIfDirtyAndReturnTrueIfValueChanged(propertyDataPtr);
if (changed)
observer.notify(newBinding.data(), propertyDataPtr, /*alreadyKnownToHaveChanged=*/true);
}
} else if (observer) {
d.setObservers(observer.ptr);
} else {
@ -435,9 +438,18 @@ void QPropertyObserverPointer::setBindingToMarkDirty(QPropertyBindingPrivate *bi
ptr->next.setTag(QPropertyObserver::ObserverNotifiesBinding);
}
void QPropertyObserverPointer::notify(QPropertyBindingPrivate *triggeringBinding, QUntypedPropertyData *propertyDataPtr)
/*! \internal
\a propertyDataPtr is a pointer to the observed property's property data
In case that property has a binding, \a triggeringBinding points to the binding's QPropertyBindingPrivate
\a alreadyKnownToHaveChanged is an optional parameter, which is needed in the case
of eager evaluation:
There, we have already evaluated the binding, and thus the change detection for the
ObserverNotifiesChangeHandler case would not work. Thus we instead pass the knowledge of
whether the value has changed we obtained when evaluating the binding eagerly along
*/
void QPropertyObserverPointer::notify(QPropertyBindingPrivate *triggeringBinding, QUntypedPropertyData *propertyDataPtr,bool alreadyKnownToHaveChanged)
{
bool knownIfPropertyChanged = false;
bool knownIfPropertyChanged = alreadyKnownToHaveChanged;
bool propertyChanged = true;
auto observer = const_cast<QPropertyObserver*>(ptr);

7
src/corelib/kernel/qproperty_p.h
View File

@ -106,7 +106,7 @@ struct QPropertyObserverPointer
void setChangeHandler(QPropertyObserver::ChangeHandler changeHandler);
void setAliasedProperty(QUntypedPropertyData *propertyPtr);
void notify(QPropertyBindingPrivate *triggeringBinding, QUntypedPropertyData *propertyDataPtr);
void notify(QPropertyBindingPrivate *triggeringBinding, QUntypedPropertyData *propertyDataPtr, const bool alreadyKnownToHaveChanged = false);
void observeProperty(QPropertyBindingDataPointer property);
explicit operator bool() const { return ptr != nullptr; }
@ -409,10 +409,7 @@ public:
{
QtPrivate::QPropertyBindingData *bd = qGetBindingStorage(owner())->bindingData(this, true);
QUntypedPropertyBinding oldBinding(bd->setBinding(newBinding, this, nullptr, bindingWrapper));
// refetch the binding data, as the eager evaluation in setBinding() above could cause a reallocation
// in the binding storage
bd = qGetBindingStorage(owner())->bindingData(this);
notify(bd);
// notification is already handled in QPropertyBindingData::setBinding
return static_cast<QPropertyBinding<T> &>(oldBinding);
}

3
tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
View File

@ -432,7 +432,7 @@ class BindingLoopTester : public QObject
public:
BindingLoopTester(QProperty<int> *i, QObject *parent = nullptr) : QObject(parent) {
eagerData.setBinding(Qt::makePropertyBinding([&](){ return eagerData2.value() + i->value(); } ) );
eagerData2.setBinding(Qt::makePropertyBinding([&](){ return eagerData.value(); } ) );
eagerData2.setBinding(Qt::makePropertyBinding([&](){ return eagerData.value() + 1; } ) );
i->setValue(42);
}
@ -532,7 +532,6 @@ void tst_QProperty::realloc()
tester.bindableProp3().setBinding([&](){return tester.prop5();});
tester.bindableProp4().setBinding([&](){return tester.prop5();});
tester.bindableProp5().setBinding([&]() -> int{return 42;});
QEXPECT_FAIL("", "ChangeHandler bug with eager properties", Continue);
QCOMPARE(modificationCount, 2);
}
};