Tumbler: reduce amount of signal-slot connections and change listeners

Instead of adding change listeners and signal-slot connections for each
attached object, add them once in QQuickTumbler and then notify each
attached object all in one go.

Change-Id: I6c5e76182c026b24922dcddaeb08ae4b1d4c4c38
Reviewed-by: J-P Nurmi <jpnurmi@qt.io>

3 files changed, 28 insertions, 85 deletions

diff --git a/src/quicktemplates2/qquicktumbler.cpp b/src/quicktemplates2/qquicktumbler.cpp
index 241a5fc3..c965095d 100644
--- a/src/quicktemplates2/qquicktumbler.cpp
+++ b/src/quicktemplates2/qquicktumbler.cpp
@@ -119,6 +119,7 @@ namespace {
 */
 QQuickItem *QQuickTumblerPrivate::determineViewType(QQuickItem *contentItem)
 {
+    Q_Q(QQuickTumbler);
     if (!contentItem) {
         resetViewData();
         return nullptr;
@@ -128,11 +129,17 @@ QQuickItem *QQuickTumblerPrivate::determineViewType(QQuickItem *contentItem)
         view = contentItem;
         viewContentItem = contentItem;
         viewContentItemType = PathViewContentItem;
+
+        QObject::connect(view, SIGNAL(offsetChanged()), q, SLOT(_q_calculateAttachedDisplacements()));
+
         return contentItem;
     } else if (contentItem->inherits("QQuickListView")) {
         view = contentItem;
         viewContentItem = qobject_cast<QQuickFlickable*>(contentItem)->contentItem();
         viewContentItemType = ListViewContentItem;
+
+        QObject::connect(view, SIGNAL(contentYChanged()), q, SLOT(_q_calculateAttachedDisplacements()));
+
         return contentItem;
     } else {
         const auto childItems = contentItem->childItems();
@@ -234,6 +241,19 @@ void QQuickTumblerPrivate::_q_onViewCountChanged()
     }
 }
 
+void QQuickTumblerPrivate::_q_calculateAttachedDisplacements()
+{
+    const auto items = viewContentItemChildItems();
+    for (QQuickItem *childItem : items) {
+        QQuickTumblerAttached *attached = qobject_cast<QQuickTumblerAttached *>(
+            qmlAttachedPropertiesObject<QQuickTumbler>(childItem, false));
+        if (attached) {
+            QQuickTumblerAttachedPrivate *attachedPrivate = QQuickTumblerAttachedPrivate::get(attached);
+            attachedPrivate->_q_calculateDisplacement();
+        }
+    }
+}
+
 void QQuickTumblerPrivate::itemChildAdded(QQuickItem *, QQuickItem *)
 {
     _q_updateItemWidths();
@@ -550,7 +570,7 @@ void QQuickTumblerPrivate::disconnectFromView()
     QObject::disconnect(view, SIGNAL(movingChanged()), q, SIGNAL(movingChanged()));
 
     QQuickItemPrivate *oldViewContentItemPrivate = QQuickItemPrivate::get(viewContentItem);
-    oldViewContentItemPrivate->removeItemChangeListener(this, QQuickItemPrivate::Children);
+    oldViewContentItemPrivate->removeItemChangeListener(this, QQuickItemPrivate::Geometry | QQuickItemPrivate::Children);
 
     resetViewData();
 }
@@ -578,20 +598,12 @@ void QQuickTumblerPrivate::setupViewData(QQuickItem *newControlContentItem)
     QObject::connect(view, SIGNAL(movingChanged()), q, SIGNAL(movingChanged()));
 
     QQuickItemPrivate *viewContentItemPrivate = QQuickItemPrivate::get(viewContentItem);
-    viewContentItemPrivate->addItemChangeListener(this, QQuickItemPrivate::Children);
+    viewContentItemPrivate->addItemChangeListener(this, QQuickItemPrivate::Geometry | QQuickItemPrivate::Children);
 
     // Sync the view's currentIndex with ours.
     syncCurrentIndex();
 
-    const auto items = viewContentItemChildItems();
-    for (QQuickItem *childItem : items) {
-        QQuickTumblerAttached *attached = qobject_cast<QQuickTumblerAttached *>(
-            qmlAttachedPropertiesObject<QQuickTumbler>(childItem, false));
-        if (attached) {
-            QQuickTumblerAttachedPrivate *attachedPrivate = QQuickTumblerAttachedPrivate::get(attached);
-            attachedPrivate->viewDataSetUp();
-        }
-    }
+    _q_calculateAttachedDisplacements();
 }
 
 void QQuickTumblerPrivate::syncCurrentIndex()
@@ -769,32 +781,6 @@ void QQuickTumblerAttachedPrivate::init(QQuickItem *delegateItem)
     }
 }
 
-void QQuickTumblerAttachedPrivate::itemGeometryChanged(QQuickItem *, QQuickGeometryChange, const QRectF &)
-{
-    _q_calculateDisplacement();
-}
-
-void QQuickTumblerAttachedPrivate::itemChildAdded(QQuickItem *, QQuickItem *)
-{
-    _q_calculateDisplacement();
-}
-
-void QQuickTumblerAttachedPrivate::itemChildRemoved(QQuickItem *item, QQuickItem *child)
-{
-    _q_calculateDisplacement();
-
-    if (parent == child) {
-        // The child that was removed from the contentItem was the delegate
-        // that our properties are attached to. If we don't remove the change
-        // listener, the contentItem will attempt to notify a destroyed
-        // listener, causing a crash.
-
-        // item is the "actual content item" of Tumbler's contentItem, i.e. a PathView or ListView.contentItem
-        QQuickItemPrivate *p = QQuickItemPrivate::get(item);
-        p->removeItemChangeListener(this, QQuickItemPrivate::Geometry | QQuickItemPrivate::Children);
-    }
-}
-
 void QQuickTumblerAttachedPrivate::_q_calculateDisplacement()
 {
     const int previousDisplacement = displacement;
@@ -852,23 +838,6 @@ void QQuickTumblerAttachedPrivate::emitIfDisplacementChanged(qreal oldDisplaceme
         emit q->displacementChanged();
 }
 
-void QQuickTumblerAttachedPrivate::viewDataSetUp()
-{
-    Q_Q(QQuickTumblerAttached);
-    QQuickTumblerPrivate *tumblerPrivate = QQuickTumblerPrivate::get(tumbler);
-    if (!tumblerPrivate->viewContentItem)
-        return;
-
-    QQuickItemPrivate *viewContentItemPrivate = QQuickItemPrivate::get(tumblerPrivate->viewContentItem);
-    viewContentItemPrivate->addItemChangeListener(this, QQuickItemPrivate::Geometry | QQuickItemPrivate::Children);
-
-    const char *contentItemSignal = tumblerPrivate->viewContentItemType == QQuickTumblerPrivate::PathViewContentItem
-        ? SIGNAL(offsetChanged()) : SIGNAL(contentYChanged());
-    QObject::connect(tumblerPrivate->view, contentItemSignal, q, SLOT(_q_calculateDisplacement()));
-
-    _q_calculateDisplacement();
-}
-
 QQuickTumblerAttachedPrivate *QQuickTumblerAttachedPrivate::get(QQuickTumblerAttached *attached)
 {
     return attached->d_func();
@@ -893,39 +862,17 @@ QQuickTumblerAttached::QQuickTumblerAttached(QObject *parent)
         QQuickTumblerPrivate *tumblerPrivate = QQuickTumblerPrivate::get(d->tumbler);
         tumblerPrivate->setupViewData(tumblerPrivate->contentItem);
 
-        // When creating a custom contentItem with e.g. Component's createObject() function,
-        // the item is created and *then* assigned to Tumbler's contentItem property,
-        // meaning that, at this point, the tumbler's contentItem is still its old one,
-        // as the delegates and their attached objects are created before the contentItem is assigned.
-        // That's why setupViewData() calls viewDataSetUp() on each attached object:
-        // we have to be notified whenever the contentItem changes, as otherwise
-        // we may never end up using the correct view data, and end up calculating incorrect displacements.
-
         if (delegateItem->parentItem() == tumblerPrivate->viewContentItem) {
             // This item belongs to the "new" view, meaning that the tumbler's contentItem
-            // was probably assigned declaratively. If the contentItem was instead created
-            // dynamically and then assigned (as mentioned above), these two won't
-            // be equal and we'll have to wait until viewDataSetUp() is called before
-            // connecting signals to our slots. Since viewDataSetUp() is only called once
-            // for each contentItem, and every attached object after the first one that is
-            // created would miss the chance for it to be called, we call the slot manually here.
-            d->viewDataSetUp();
+            // was probably assigned declaratively. If they're not equal, calling
+            // _q_calculateDisplacement() would use the old contentItem data, which is bad.
+            d->_q_calculateDisplacement();
         }
     }
 }
 
 QQuickTumblerAttached::~QQuickTumblerAttached()
 {
-    Q_D(QQuickTumblerAttached);
-    if (!d->tumbler)
-        return;
-
-    QQuickTumblerPrivate *tumblerPrivate = QQuickTumblerPrivate::get(d->tumbler);
-    if (!tumblerPrivate->viewContentItem)
-        return;
-
-    QQuickItemPrivate *viewContentItemPrivate = QQuickItemPrivate::get(tumblerPrivate->viewContentItem);
-    viewContentItemPrivate->removeItemChangeListener(d, QQuickItemPrivate::Geometry | QQuickItemPrivate::Children);
 }
 
 /*!
diff --git a/src/quicktemplates2/qquicktumbler_p.h b/src/quicktemplates2/qquicktumbler_p.h
index de7de595..50069cd7 100644
--- a/src/quicktemplates2/qquicktumbler_p.h
+++ b/src/quicktemplates2/qquicktumbler_p.h
@@ -127,6 +127,7 @@ private:
     Q_PRIVATE_SLOT(d_func(), void _q_updateItemHeights())
     Q_PRIVATE_SLOT(d_func(), void _q_onViewCurrentIndexChanged())
     Q_PRIVATE_SLOT(d_func(), void _q_onViewCountChanged())
+    Q_PRIVATE_SLOT(d_func(), void _q_calculateAttachedDisplacements())
 };
 
 class QQuickTumblerAttachedPrivate;
diff --git a/src/quicktemplates2/qquicktumbler_p_p.h b/src/quicktemplates2/qquicktumbler_p_p.h
index 38b0f60d..fc38dfcc 100644
--- a/src/quicktemplates2/qquicktumbler_p_p.h
+++ b/src/quicktemplates2/qquicktumbler_p_p.h
@@ -95,6 +95,7 @@ public:
     void _q_updateItemWidths();
     void _q_onViewCurrentIndexChanged();
     void _q_onViewCountChanged();
+    void _q_calculateAttachedDisplacements();
 
     void disconnectFromView();
     void setupViewData(QQuickItem *newControlContentItem);
@@ -119,15 +120,9 @@ public:
 
     void init(QQuickItem *delegateItem);
 
-    void itemGeometryChanged(QQuickItem *item, QQuickGeometryChange change, const QRectF &diff) override;
-    void itemChildAdded(QQuickItem *, QQuickItem *) override;
-    void itemChildRemoved(QQuickItem *, QQuickItem *) override;
-
     void _q_calculateDisplacement();
     void emitIfDisplacementChanged(qreal oldDisplacement, qreal newDisplacement);
 
-    void viewDataSetUp();
-
     static QQuickTumblerAttachedPrivate *get(QQuickTumblerAttached *attached);
 
     // The Tumbler that contains the delegate. Required to calculated the displacement.