Add support for grouped property changes

Add Qt::begin/endPropertyUpdateGroup() methods.
These methods will group a set of property updates together and delay
bindings evaluations or change notifications until the end of the update
group.

In cases where many properties get updated, this can avoid duplicated
recalculations and change notifications.

Change-Id: Ia78ae1d46abc6b7e5da5023442e081cb5c5ae67b
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Andrei Golubev <andrei.golubev@qt.io>
Reviewed-by: Andreas Buhr <andreas.buhr@qt.io>
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

5 files changed, 372 insertions, 48 deletions

diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp
index 60f21d5b57..a5354532d2 100644
--- a/src/corelib/kernel/qproperty.cpp
+++ b/src/corelib/kernel/qproperty.cpp
@@ -66,16 +66,17 @@ void QPropertyBindingPrivatePtr::reset(QtPrivate::RefCounted *ptr) noexcept
 
 void QPropertyBindingDataPointer::addObserver(QPropertyObserver *observer)
 {
-    if (auto *binding = bindingPtr()) {
-        observer->prev = &binding->firstObserver.ptr;
-        observer->next = binding->firstObserver.ptr;
+    if (auto *b = binding()) {
+        observer->prev = &b->firstObserver.ptr;
+        observer->next = b->firstObserver.ptr;
         if (observer->next)
             observer->next->prev = &observer->next;
-        binding->firstObserver.ptr = observer;
+        b->firstObserver.ptr = observer;
     } else {
-        Q_ASSERT(!(ptr->d_ptr & QPropertyBindingData::BindingBit));
-        auto firstObserver = reinterpret_cast<QPropertyObserver*>(ptr->d_ptr);
-        observer->prev = reinterpret_cast<QPropertyObserver**>(&ptr->d_ptr);
+        auto &d = ptr->d_ref();
+        Q_ASSERT(!(d & QPropertyBindingData::BindingBit));
+        auto firstObserver = reinterpret_cast<QPropertyObserver*>(d);
+        observer->prev = reinterpret_cast<QPropertyObserver**>(&d);
         observer->next = firstObserver;
         if (observer->next)
             observer->next->prev = &observer->next;
@@ -83,6 +84,182 @@ void QPropertyBindingDataPointer::addObserver(QPropertyObserver *observer)
     setFirstObserver(observer);
 }
 
+/*!
+    \internal
+
+    QPropertyDelayedNotifications is used to manage delayed notifications in grouped property updates.
+    It acts as a pool allocator for QPropertyProxyBindingData, and has methods to manage delayed
+    notifications.
+
+    \sa beginPropertyUpdateGroup, endPropertyUpdateGroup
+ */
+struct QPropertyDelayedNotifications
+{
+    // we can't access the dynamic page size as we need a constant value
+    // use 4096 as a sensible default
+    static constexpr inline auto PageSize = 4096;
+    int ref = 0;
+    QPropertyDelayedNotifications *next = nullptr; // in case we have more than size dirty properties...
+    qsizetype used = 0;
+    // Size chosen to avoid allocating more than one page of memory, while still ensuring
+    // that we can store many delayed properties without doing further allocations
+    static constexpr qsizetype size = (PageSize - 3*sizeof(void *))/sizeof(QPropertyProxyBindingData);
+    QPropertyProxyBindingData delayedProperties[size];
+
+    /*!
+        \internal
+        This method is called when a property attempts to notify its observers while inside of a
+        property update group. Instead of actually notifying, it replaces \a bindingData's d_ptr
+        with a QPropertyProxyBindingData.
+        \a bindingData and \a propertyData are the binding data and property data of the property
+        whose notify call gets delayed.
+        \sa QPropertyBindingData::notifyObservers
+     */
+    void addProperty(const QPropertyBindingData *bindingData, QUntypedPropertyData *propertyData) {
+        if (bindingData->isNotificationDelayed())
+            return;
+        auto *data = this;
+        while (data->used == size) {
+            if (!data->next)
+                // add a new page
+                data->next = new QPropertyDelayedNotifications;
+            data = data->next;
+        }
+        auto *delayed = data->delayedProperties + data->used;
+        *delayed = QPropertyProxyBindingData { bindingData->d_ptr, bindingData, propertyData };
+        ++data->used;
+        // preserve the binding bit for faster access
+        quintptr bindingBit = bindingData->d_ptr & QPropertyBindingData::BindingBit;
+        bindingData->d_ptr = reinterpret_cast<quintptr>(delayed) | QPropertyBindingData::DelayedNotificationBit | bindingBit;
+        Q_ASSERT(bindingData->d_ptr > 3);
+        if (!bindingBit) {
+            if (auto observer = reinterpret_cast<QPropertyObserver *>(delayed->d_ptr))
+                observer->prev = reinterpret_cast<QPropertyObserver **>(&delayed->d_ptr);
+        }
+    }
+
+    /*!
+        \internal
+        Called in Qt::endPropertyUpdateGroup. For the QPropertyProxyBindingData at position
+        \a index, it
+        \list
+            \li restores the original binding data that was modified in addProperty and
+            \li evaluates any bindings which depend on properties that were changed inside
+                the group.
+        \endlist
+        Change notifications are sent later with notify (following the logic of separating
+        binding updates and notifications used in non-deferred updates).
+     */
+    void evaluateBindings(int index) {
+        auto *delayed = delayedProperties + index;
+        auto *bindingData = delayed->originalBindingData;
+        if (!bindingData)
+            return;
+
+        bindingData->d_ptr = delayed->d_ptr;
+        Q_ASSERT(!(bindingData->d_ptr & QPropertyBindingData::DelayedNotificationBit));
+        if (!bindingData->hasBinding()) {
+            if (auto observer = reinterpret_cast<QPropertyObserver *>(bindingData->d_ptr))
+                observer->prev = reinterpret_cast<QPropertyObserver **>(&bindingData->d_ptr);
+        }
+
+        QPropertyBindingDataPointer bindingDataPointer{bindingData};
+        QPropertyObserverPointer observer = bindingDataPointer.firstObserver();
+        if (observer)
+            observer.evaluateBindings();
+    }
+
+    /*!
+        \internal
+        Called in Qt::endPropertyUpdateGroup. For the QPropertyProxyBindingData at position
+        \a i, it
+        \list
+            \li resets the proxy binding data and
+            \li sends any pending notifications.
+        \endlist
+     */
+    void notify(int index) {
+        auto *delayed = delayedProperties + index;
+        auto *bindingData = delayed->originalBindingData;
+        if (!bindingData)
+            return;
+
+        delayed->originalBindingData = nullptr;
+        delayed->d_ptr = 0;
+
+        QPropertyBindingDataPointer bindingDataPointer{bindingData};
+        QPropertyObserverPointer observer = bindingDataPointer.firstObserver();
+        if (observer)
+            observer.notify(delayed->propertyData);
+    }
+};
+
+static thread_local QPropertyDelayedNotifications *groupUpdateData = nullptr;
+
+/*!
+    \since 6.2
+
+    \relates template<typename T> QProperty<T>
+
+    Marks the beginning of a property update group. Inside this group,
+    changing a property does neither immediately update any dependent properties
+    nor does it trigger change notifications.
+    Those are instead deferred until the group is ended by a call to endPropertyUpdateGroup.
+
+    Groups can be nested. In that case, the deferral ends only after the outermost group has been
+    ended.
+
+    \note Change notifications are only send after all property values affected by the group have
+    been updated to their new values. This allows re-establishing a  class invariant if multiple
+    properties need to be updated, preventing any external observer from noticing an inconsistent
+    state.
+
+    \sa Qt::endPropertyUpdateGroup
+ */
+void Qt::beginPropertyUpdateGroup()
+{
+    if (!groupUpdateData)
+        groupUpdateData = new QPropertyDelayedNotifications;
+    ++groupUpdateData->ref;
+}
+
+/*!
+    \since 6.2
+    \relates template<typename T> QProperty<T>
+
+    Ends a property update group. If the outermost group has been ended, and deferred
+    binding evaluations and notifications happen now.
+
+    \warning Calling endPropertyUpdateGroup without a preceding call to beginPropertyUpdateGroup
+    results in undefined behavior.
+
+    \sa Qt::beginPropertyUpdateGroup
+ */
+void Qt::endPropertyUpdateGroup()
+{
+    auto *data = groupUpdateData;
+    Q_ASSERT(data->ref);
+    if (--data->ref)
+        return;
+    groupUpdateData = nullptr;
+    // update all delayed properties
+    auto start = data;
+    while (data) {
+        for (int i = 0; i < data->used; ++i)
+            data->evaluateBindings(i);
+        data = data->next;
+    }
+    // notify all delayed properties
+    data = start;
+    while (data) {
+        for (int i = 0; i < data->used; ++i)
+            data->notify(i);
+        auto *next = data->next;
+        delete data;
+        data = next;
+    }
+}
+
 QPropertyBindingPrivate::~QPropertyBindingPrivate()
 {
     if (firstObserver)
@@ -123,13 +300,17 @@ void QPropertyBindingPrivate::evaluateRecursive()
 
     auto bindingFunctor =  reinterpret_cast<std::byte *>(this) +
         QPropertyBindingPrivate::getSizeEnsuringAlignment();
+    bool changed = false;
     if (hasBindingWrapper) {
-        pendingNotify = staticBindingWrapper(metaType, propertyDataPtr,
+        changed = staticBindingWrapper(metaType, propertyDataPtr,
                 {vtable, bindingFunctor});
     } else {
-        pendingNotify = vtable->call(metaType, propertyDataPtr, bindingFunctor);
+        changed = vtable->call(metaType, propertyDataPtr, bindingFunctor);
     }
-    if (!pendingNotify || !firstObserver)
+    // If there was a change, we must set pendingNotify.
+    // If there was not, we must not clear it, as that only should happen in notifyRecursive
+    pendingNotify = pendingNotify || changed;
+    if (!changed || !firstObserver)
         return;
 
     firstObserver.noSelfDependencies(this);
@@ -220,7 +401,7 @@ QPropertyBindingData::~QPropertyBindingData()
         observer.unlink();
         observer = next;
     }
-    if (auto binding = d.bindingPtr())
+    if (auto binding = d.binding())
         binding->unlinkAndDeref();
 }
 
@@ -235,7 +416,8 @@ QUntypedPropertyBinding QPropertyBindingData::setBinding(const QUntypedPropertyB
     QPropertyBindingDataPointer d{this};
     QPropertyObserverPointer observer;
 
-    if (auto *existingBinding = d.bindingPtr()) {
+    auto &data = d_ref();
+    if (auto *existingBinding = d.binding()) {
         if (existingBinding == newBinding.data())
             return QUntypedPropertyBinding(static_cast<QPropertyBindingPrivate *>(oldBinding.data()));
         if (existingBinding->isUpdating()) {
@@ -245,15 +427,15 @@ QUntypedPropertyBinding QPropertyBindingData::setBinding(const QUntypedPropertyB
         oldBinding = QPropertyBindingPrivatePtr(existingBinding);
         observer = static_cast<QPropertyBindingPrivate *>(oldBinding.data())->takeObservers();
         static_cast<QPropertyBindingPrivate *>(oldBinding.data())->unlinkAndDeref();
-        d_ptr = 0;
+        data = 0;
     } else {
         observer = d.firstObserver();
     }
 
     if (newBinding) {
         newBinding.data()->addRef();
-        d_ptr = reinterpret_cast<quintptr>(newBinding.data());
-        d_ptr |= BindingBit;
+        data = reinterpret_cast<quintptr>(newBinding.data());
+        data |= BindingBit;
         auto newBindingRaw = static_cast<QPropertyBindingPrivate *>(newBinding.data());
         newBindingRaw->setProperty(propertyDataPtr);
         if (observer)
@@ -265,7 +447,7 @@ QUntypedPropertyBinding QPropertyBindingData::setBinding(const QUntypedPropertyB
     } else if (observer) {
         d.setObservers(observer.ptr);
     } else {
-        d_ptr &= ~QPropertyBindingData::BindingBit;
+        data = 0;
     }
 
     if (oldBinding)
@@ -276,8 +458,7 @@ QUntypedPropertyBinding QPropertyBindingData::setBinding(const QUntypedPropertyB
 
 QPropertyBindingData::QPropertyBindingData(QPropertyBindingData &&other) : d_ptr(std::exchange(other.d_ptr, 0))
 {
-    QPropertyBindingDataPointer d{this};
-    d.fixupFirstObserverAfterMove();
+    QPropertyBindingDataPointer::fixupAfterMove(this);
 }
 
 static thread_local QBindingStatus bindingStatus;
@@ -325,11 +506,11 @@ void QPropertyBindingData::removeBinding_helper()
 {
     QPropertyBindingDataPointer d{this};
 
-    auto *existingBinding = d.bindingPtr();
+    auto *existingBinding = d.binding();
     Q_ASSERT(existingBinding);
 
     auto observer = existingBinding->takeObservers();
-    d_ptr = 0;
+    d_ref() = 0;
     if (observer)
         d.setObservers(observer.ptr);
     existingBinding->unlinkAndDeref();
@@ -355,11 +536,19 @@ void QPropertyBindingData::registerWithCurrentlyEvaluatingBinding_helper(Binding
 
 void QPropertyBindingData::notifyObservers(QUntypedPropertyData *propertyDataPtr) const
 {
+    if (isNotificationDelayed())
+        return;
     QPropertyBindingDataPointer d{this};
-    if (QPropertyObserverPointer observer = d.firstObserver()) {
-        observer.evaluateBindings();
-        observer.notify(propertyDataPtr);
+    QPropertyObserverPointer observer = d.firstObserver();
+    if (!observer)
+        return;
+    auto *delay = groupUpdateData;
+    if (delay) {
+        delay->addProperty(this, propertyDataPtr);
+        return;
     }
+    observer.evaluateBindings();
+    observer.notify(propertyDataPtr);
 }
 
 int QPropertyBindingDataPointer::observerCount() const
diff --git a/src/corelib/kernel/qproperty.h b/src/corelib/kernel/qproperty.h
index 1c557fbace..a6cb7ada48 100644
--- a/src/corelib/kernel/qproperty.h
+++ b/src/corelib/kernel/qproperty.h
@@ -63,6 +63,11 @@
 
 QT_BEGIN_NAMESPACE
 
+namespace Qt {
+Q_CORE_EXPORT void beginPropertyUpdateGroup();
+Q_CORE_EXPORT void endPropertyUpdateGroup();
+}
+
 template <typename T>
 class QPropertyData : public QUntypedPropertyData
 {
@@ -217,6 +222,7 @@ protected:
     using ChangeHandler = void (*)(QPropertyObserver*, QUntypedPropertyData *);
 
 private:
+    friend struct QPropertyDelayedNotifications;
     friend struct QPropertyObserverNodeProtector;
     friend class QPropertyObserver;
     friend struct QPropertyObserverPointer;
diff --git a/src/corelib/kernel/qproperty_p.h b/src/corelib/kernel/qproperty_p.h
index 3535d7e843..58a000e9bf 100644
--- a/src/corelib/kernel/qproperty_p.h
+++ b/src/corelib/kernel/qproperty_p.h
@@ -71,19 +71,18 @@ struct Q_AUTOTEST_EXPORT QPropertyBindingDataPointer
 {
     const QtPrivate::QPropertyBindingData *ptr = nullptr;
 
-    QPropertyBindingPrivate *bindingPtr() const
+    QPropertyBindingPrivate *binding() const
     {
-        if (ptr->d_ptr & QtPrivate::QPropertyBindingData::BindingBit)
-            return reinterpret_cast<QPropertyBindingPrivate*>(ptr->d_ptr - QtPrivate::QPropertyBindingData::BindingBit);
-        return nullptr;
+        return ptr->binding();
     }
 
     void setObservers(QPropertyObserver *observer)
     {
-        observer->prev = reinterpret_cast<QPropertyObserver**>(&(ptr->d_ptr));
-        ptr->d_ptr = reinterpret_cast<quintptr>(observer);
+        auto &d = ptr->d_ref();
+        observer->prev = reinterpret_cast<QPropertyObserver**>(&d);
+        d = reinterpret_cast<quintptr>(observer);
     }
-    void fixupFirstObserverAfterMove() const;
+    static void fixupAfterMove(QtPrivate::QPropertyBindingData *ptr);
     void addObserver(QPropertyObserver *observer);
     void setFirstObserver(QPropertyObserver *observer);
     QPropertyObserverPointer firstObserver() const;
@@ -351,29 +350,36 @@ public:
 
 inline void QPropertyBindingDataPointer::setFirstObserver(QPropertyObserver *observer)
 {
-    if (auto *binding = bindingPtr()) {
-        binding->firstObserver.ptr = observer;
+    if (auto *b = binding()) {
+        b->firstObserver.ptr = observer;
         return;
     }
-    ptr->d_ptr = reinterpret_cast<quintptr>(observer);
+    auto &d = ptr->d_ref();
+    d = reinterpret_cast<quintptr>(observer);
 }
 
-inline void QPropertyBindingDataPointer::fixupFirstObserverAfterMove() const
+inline void QPropertyBindingDataPointer::fixupAfterMove(QtPrivate::QPropertyBindingData *ptr)
 {
+    auto &d = ptr->d_ref();
+    if (ptr->isNotificationDelayed()) {
+        QPropertyProxyBindingData *proxyData
+                = reinterpret_cast<QPropertyProxyBindingData*>(d & ~QtPrivate::QPropertyBindingData::BindingBit);
+        proxyData->originalBindingData = ptr;
+    }
     // If QPropertyBindingData has been moved, and it has an observer
-    // we have to adjust the firstObesrver's prev pointer to point to
+    // we have to adjust the firstObserver's prev pointer to point to
     // the moved to QPropertyBindingData's d_ptr
-    if (ptr->d_ptr & QtPrivate::QPropertyBindingData::BindingBit)
+    if (d & QtPrivate::QPropertyBindingData::BindingBit)
         return; // nothing to do if the observer is stored in the binding
-    if (auto observer = firstObserver())
-        observer.ptr->prev = reinterpret_cast<QPropertyObserver **>(&(ptr->d_ptr));
+    if (auto observer = reinterpret_cast<QPropertyObserver *>(d))
+        observer->prev = reinterpret_cast<QPropertyObserver **>(&d);
 }
 
 inline QPropertyObserverPointer QPropertyBindingDataPointer::firstObserver() const
 {
-    if (auto *binding = bindingPtr())
-        return binding->firstObserver;
-    return { reinterpret_cast<QPropertyObserver *>(ptr->d_ptr) };
+    if (auto *b = binding())
+        return b->firstObserver;
+    return { reinterpret_cast<QPropertyObserver *>(ptr->d()) };
 }
 
 namespace QtPrivate {
diff --git a/src/corelib/kernel/qpropertyprivate.h b/src/corelib/kernel/qpropertyprivate.h
index b6d93529d1..f59221c4df 100644
--- a/src/corelib/kernel/qpropertyprivate.h
+++ b/src/corelib/kernel/qpropertyprivate.h
@@ -143,7 +143,6 @@ private:
     QtPrivate::RefCounted *d;
 };
 
-
 class QUntypedPropertyBinding;
 class QPropertyBindingPrivate;
 struct QPropertyBindingDataPointer;
@@ -158,6 +157,24 @@ public:
 template <typename T>
 class QPropertyData;
 
+// Used for grouped property evaluations
+namespace QtPrivate {
+class QPropertyBindingData;
+}
+struct QPropertyDelayedNotifications;
+struct QPropertyProxyBindingData
+{
+    // acts as QPropertyBindingData::d_ptr
+    quintptr d_ptr;
+    /*
+        The two members below store the original binding data and property
+        data pointer of the property which gets proxied.
+        They are set in QPropertyDelayedNotifications::addProperty
+    */
+    const QtPrivate::QPropertyBindingData *originalBindingData;
+    QUntypedPropertyData *propertyData;
+};
+
 namespace QtPrivate {
 struct BindingEvaluationState;
 
@@ -218,6 +235,16 @@ struct QPropertyBindingFunction {
 using QPropertyObserverCallback = void (*)(QUntypedPropertyData *);
 using QPropertyBindingWrapper = bool(*)(QMetaType, QUntypedPropertyData *dataPtr, QPropertyBindingFunction);
 
+/*!
+    \internal
+    A property normally consists of the actual property value and metadata for the binding system.
+    QPropertyBindingData is the latter part. It stores a pointer to either
+    - a (potentially empty) linked list of notifiers, in case there is no binding set,
+    - an actual QUntypedPropertyBinding when the property has a binding,
+    - or a pointer to QPropertyProxyBindingData when notifications occur inside a grouped update.
+
+    \sa QPropertyDelayedNotifications, beginPropertyUpdateGroup
+ */
 class Q_CORE_EXPORT QPropertyBindingData
 {
     // Mutable because the address of the observer of the currently evaluating binding is stored here, for
@@ -225,6 +252,7 @@ class Q_CORE_EXPORT QPropertyBindingData
     mutable quintptr d_ptr = 0;
     friend struct QT_PREPEND_NAMESPACE(QPropertyBindingDataPointer);
     friend class QT_PREPEND_NAMESPACE(QQmlPropertyBinding);
+    friend struct QT_PREPEND_NAMESPACE(QPropertyDelayedNotifications);
     Q_DISABLE_COPY(QPropertyBindingData)
 public:
     QPropertyBindingData() = default;
@@ -232,9 +260,13 @@ public:
     QPropertyBindingData &operator=(QPropertyBindingData &&other) = delete;
     ~QPropertyBindingData();
 
-    static inline constexpr quintptr BindingBit = 0x1; // Is d_ptr pointing to a binding (1) or list of notifiers (0)?
+    // Is d_ptr pointing to a binding (1) or list of notifiers (0)?
+    static inline constexpr quintptr BindingBit = 0x1;
+    // Is d_ptr pointing to QPropertyProxyBindingData (1) or to an actual binding/list of notifiers?
+    static inline constexpr quintptr DelayedNotificationBit = 0x2;
 
     bool hasBinding() const { return d_ptr & BindingBit; }
+    bool isNotificationDelayed() const { return d_ptr & DelayedNotificationBit; }
 
     QUntypedPropertyBinding setBinding(const QUntypedPropertyBinding &newBinding,
                                        QUntypedPropertyData *propertyDataPtr,
@@ -243,8 +275,9 @@ public:
 
     QPropertyBindingPrivate *binding() const
     {
-        if (d_ptr & BindingBit)
-            return reinterpret_cast<QPropertyBindingPrivate*>(d_ptr - BindingBit);
+        quintptr dd = d();
+        if (dd & BindingBit)
+            return reinterpret_cast<QPropertyBindingPrivate*>(dd - BindingBit);
         return nullptr;
 
     }
@@ -266,6 +299,25 @@ public:
     void registerWithCurrentlyEvaluatingBinding() const;
     void notifyObservers(QUntypedPropertyData *propertyDataPtr) const;
 private:
+    /*!
+        \internal
+        Returns a reference to d_ptr, except when d_ptr points to a proxy.
+        In that case, a reference to proxy->d_ptr is returned instead.
+
+        To properly support proxying, direct access to d_ptr only occcurs when
+        - a function actually deals with proxying (e.g.
+          QPropertyDelayedNotifications::addProperty),
+        - only the tag value is accessed (e.g. hasBinding) or
+        - inside a constructor.
+     */
+    quintptr &d_ref() const
+    {
+        quintptr &d = d_ptr;
+        if (isNotificationDelayed())
+            return reinterpret_cast<QPropertyProxyBindingData *>(d_ptr & ~(BindingBit|DelayedNotificationBit))->d_ptr;
+        return d;
+    }
+    quintptr d() const { return d_ref(); }
     void registerWithCurrentlyEvaluatingBinding_helper(BindingEvaluationState *currentBinding) const;
     void removeBinding_helper();
 };
diff --git a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
index 8cc4fe486a..0b05353bf0 100644
--- a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
+++ b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
@@ -96,6 +96,8 @@ private slots:
 
     void bindablePropertyWithInitialization();
     void noDoubleNotification();
+    void groupedNotifications();
+    void groupedNotificationConsistency();
 };
 
 void tst_QProperty::functorBinding()
@@ -260,12 +262,12 @@ void tst_QProperty::avoidDependencyAllocationAfterFirstEval()
 
     QCOMPARE(propWithBinding.value(), int(11));
 
-    QVERIFY(QPropertyBindingDataPointer::get(propWithBinding).bindingPtr());
-    QCOMPARE(QPropertyBindingDataPointer::get(propWithBinding).bindingPtr()->dependencyObserverCount, 2u);
+    QVERIFY(QPropertyBindingDataPointer::get(propWithBinding).binding());
+    QCOMPARE(QPropertyBindingDataPointer::get(propWithBinding).binding()->dependencyObserverCount, 2u);
 
     firstDependency = 100;
     QCOMPARE(propWithBinding.value(), int(110));
-    QCOMPARE(QPropertyBindingDataPointer::get(propWithBinding).bindingPtr()->dependencyObserverCount, 2u);
+    QCOMPARE(QPropertyBindingDataPointer::get(propWithBinding).binding()->dependencyObserverCount, 2u);
 }
 
 void tst_QProperty::boolProperty()
@@ -1637,6 +1639,75 @@ void tst_QProperty::noDoubleNotification()
     QCOMPARE(nNotifications, 3);
 }
 
+void tst_QProperty::groupedNotifications()
+{
+    QProperty<int> a(0);
+    QProperty<int> b;
+    b.setBinding([&](){ return a.value(); });
+    QProperty<int> c;
+    c.setBinding([&](){ return a.value(); });
+    QProperty<int> d;
+    QProperty<int> e;
+    e.setBinding([&](){ return b.value() + c.value() + d.value(); });
+    int nNotifications = 0;
+    int expected = 0;
+    auto connection = e.subscribe([&](){
+        ++nNotifications;
+        QCOMPARE(e.value(), expected);
+    });
+    QCOMPARE(nNotifications, 1);
+
+    expected = 2;
+    Qt::beginPropertyUpdateGroup();
+    a = 1;
+    QCOMPARE(b.value(), 0);
+    QCOMPARE(c.value(), 0);
+    QCOMPARE(d.value(), 0);
+    QCOMPARE(nNotifications, 1);
+    Qt::endPropertyUpdateGroup();
+    QCOMPARE(b.value(), 1);
+    QCOMPARE(c.value(), 1);
+    QCOMPARE(e.value(), 2);
+    QCOMPARE(nNotifications, 2);
+
+    expected = 7;
+    Qt::beginPropertyUpdateGroup();
+    a = 2;
+    d = 3;
+    QCOMPARE(b.value(), 1);
+    QCOMPARE(c.value(), 1);
+    QCOMPARE(d.value(), 3);
+    QCOMPARE(nNotifications, 2);
+    Qt::endPropertyUpdateGroup();
+    QCOMPARE(b.value(), 2);
+    QCOMPARE(c.value(), 2);
+    QCOMPARE(e.value(), 7);
+    QCOMPARE(nNotifications, 3);
+
+
+}
+
+void tst_QProperty::groupedNotificationConsistency()
+{
+    QProperty<int> i(0);
+    QProperty<int> j(0);
+    bool areEqual = true;
+
+    auto observer = i.onValueChanged([&](){
+        areEqual = i == j;
+    });
+
+    i = 1;
+    j = 1;
+    QVERIFY(!areEqual); // value changed runs before j = 1
+
+    Qt::beginPropertyUpdateGroup();
+    i = 2;
+    j = 2;
+    Qt::endPropertyUpdateGroup();
+    QVERIFY(areEqual); // value changed runs after everything has been evaluated
+}
+
 QTEST_MAIN(tst_QProperty);
 
 #include "tst_qproperty.moc"