Cleanup QBindingPrivate

Simplify the data structure. We only need one pointer for either
the static callback or a bindingWrapper, so don't share it
with the dependency observer array.

Also ensure we reset the propertyDataPtr and clear the observers
when the binding gets removed from a property.

Change-Id: I4c1e7ec7823c3ef12c63d6f758b757e7bac60cae
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
This commit is contained in:
Lars Knoll 2020-08-21 17:52:22 +02:00
parent 6778b247a8
commit b788c64dc3
3 changed files with 34 additions and 37 deletions

View File

@ -68,8 +68,6 @@ QPropertyBindingPrivate::~QPropertyBindingPrivate()
{
if (firstObserver)
firstObserver.unlink();
if (!hasStaticObserver)
inlineDependencyObservers.~ObserverArray(); // Explicit because of union.
}
void QPropertyBindingPrivate::unlinkAndDeref()
@ -115,8 +113,8 @@ bool QPropertyBindingPrivate::evaluateIfDirtyAndReturnTrueIfValueChanged()
bool changed = false;
if (hasStaticObserver && staticGuardCallback) {
changed = staticGuardCallback(metaType, propertyDataPtr, evaluationFunction);
if (hasBindingWrapper) {
changed = staticBindingWrapper(metaType, propertyDataPtr, evaluationFunction);
} else {
changed = evaluationFunction(metaType, propertyDataPtr);
}
@ -231,7 +229,7 @@ QPropertyBindingData::~QPropertyBindingData()
QUntypedPropertyBinding QPropertyBindingData::setBinding(const QUntypedPropertyBinding &binding,
QUntypedPropertyData *propertyDataPtr,
QPropertyObserverCallback staticObserverCallback,
QtPrivate::QPropertyGuardFunction guardCallback)
QtPrivate::QPropertyBindingWrapper guardCallback)
{
QPropertyBindingPrivatePtr oldBinding;
QPropertyBindingPrivatePtr newBinding = binding.d;
@ -265,6 +263,9 @@ QUntypedPropertyBinding QPropertyBindingData::setBinding(const QUntypedPropertyB
d_ptr &= ~QPropertyBindingData::BindingBit;
}
if (oldBinding)
oldBinding->detachFromProperty();
return QUntypedPropertyBinding(oldBinding.data());
}

View File

@ -140,17 +140,16 @@ private:
bool dirty = false;
bool updating = false;
bool hasStaticObserver = false;
bool hasBindingWrapper = false;
QUntypedPropertyBinding::BindingEvaluationFunction evaluationFunction;
QPropertyObserverPointer firstObserver;
union {
ObserverArray inlineDependencyObservers;
struct {
QtPrivate::QPropertyObserverCallback staticObserverCallback;
QtPrivate::QPropertyGuardFunction staticGuardCallback;
};
QtPrivate::QPropertyObserverCallback staticObserverCallback = nullptr;
QtPrivate::QPropertyBindingWrapper staticBindingWrapper;
};
ObserverArray inlineDependencyObservers;
QScopedPointer<std::vector<QPropertyObserver>> heapObservers;
QUntypedPropertyData *propertyDataPtr = nullptr;
@ -175,29 +174,21 @@ public:
void setDirty(bool d) { dirty = d; }
void setProperty(QUntypedPropertyData *propertyPtr) { propertyDataPtr = propertyPtr; }
void setStaticObserver(QtPrivate::QPropertyObserverCallback callback, QtPrivate::QPropertyGuardFunction guardCallback)
void setStaticObserver(QtPrivate::QPropertyObserverCallback callback, QtPrivate::QPropertyBindingWrapper guardCallback)
{
Q_ASSERT(!(callback && guardCallback));
if (callback) {
if (!hasStaticObserver) {
if (dependencyObserverCount > 0) {
if (!heapObservers)
heapObservers.reset(new std::vector<QPropertyObserver>());
for (size_t i = 0, end = qMin(dependencyObserverCount, inlineDependencyObservers.size()); i < end; ++i)
heapObservers->push_back(std::move(inlineDependencyObservers[i]));
}
inlineDependencyObservers.~ObserverArray();
}
hasStaticObserver = true;
hasBindingWrapper = false;
staticObserverCallback = callback;
staticGuardCallback = guardCallback;
} else if (hasStaticObserver) {
} else if (guardCallback) {
hasStaticObserver = false;
new (&inlineDependencyObservers) ObserverArray();
for (size_t i = 0, end = qMin(dependencyObserverCount, inlineDependencyObservers.size()); i < end; ++i) {
inlineDependencyObservers[i] = std::move(heapObservers->back());
heapObservers->pop_back();
}
hasBindingWrapper = true;
staticBindingWrapper = guardCallback;
} else {
hasStaticObserver = false;
hasBindingWrapper = false;
staticObserverCallback = nullptr;
}
}
void prependObserver(QPropertyObserverPointer observer) {
@ -213,18 +204,16 @@ public:
}
void clearDependencyObservers() {
if (!hasStaticObserver) {
for (size_t i = 0; i < qMin(dependencyObserverCount, inlineDependencyObservers.size()); ++i) {
QPropertyObserverPointer p{&inlineDependencyObservers[i]};
p.unlink();
}
for (size_t i = 0; i < qMin(dependencyObserverCount, inlineDependencyObservers.size()); ++i) {
QPropertyObserverPointer p{&inlineDependencyObservers[i]};
p.unlink();
}
if (heapObservers)
heapObservers->clear();
dependencyObserverCount = 0;
}
QPropertyObserverPointer allocateDependencyObserver() {
if (!hasStaticObserver && dependencyObserverCount < inlineDependencyObservers.size()) {
if (dependencyObserverCount < inlineDependencyObservers.size()) {
++dependencyObserverCount;
return {&inlineDependencyObservers[dependencyObserverCount - 1]};
}
@ -249,6 +238,13 @@ public:
void setError(QPropertyBindingError &&e)
{ error = std::move(e); }
void detachFromProperty() {
hasStaticObserver = false;
hasBindingWrapper = false;
propertyDataPtr = nullptr;
clearDependencyObservers();
}
static QPropertyBindingPrivate *currentlyEvaluatingBinding();
};

View File

@ -77,7 +77,7 @@ namespace QtPrivate {
// writes binding result into dataPtr
using QPropertyBindingFunction = std::function<bool(QMetaType metaType, QUntypedPropertyData *dataPtr)>;
using QPropertyGuardFunction = bool(*)(QMetaType, QUntypedPropertyData *dataPtr,
using QPropertyBindingWrapper = bool(*)(QMetaType, QUntypedPropertyData *dataPtr,
QPropertyBindingFunction);
using QPropertyObserverCallback = void (*)(QUntypedPropertyData *);
@ -102,7 +102,7 @@ public:
QUntypedPropertyBinding setBinding(const QUntypedPropertyBinding &newBinding,
QUntypedPropertyData *propertyDataPtr,
QPropertyObserverCallback staticObserverCallback = nullptr,
QPropertyGuardFunction guardCallback = nullptr);
QPropertyBindingWrapper guardCallback = nullptr);
QPropertyBindingPrivate *binding() const;
void evaluateIfDirty() const;
@ -184,7 +184,7 @@ namespace detail {
template<typename T, typename Class, auto Guard, bool = std::is_same_v<decltype(Guard), std::nullptr_t>>
struct QPropertyGuardFunctionHelper
{
static constexpr QPropertyGuardFunction guard = nullptr;
static constexpr QPropertyBindingWrapper guard = nullptr;
};
template<typename T, typename Class, auto Guard>
struct QPropertyGuardFunctionHelper<T, Class, Guard, false>