Improve how we store and compress animation value changes

Instead of using a multi-hash and re-iterating over the same
values multiple times, we now crate a hash from the AnimationValueChange
data and can assume change updates are compressed and are unique for each
target-property pair.

Change-Id: Id0bbbe887b7f7630a426021774d68f6307a5cf65
Reviewed-by: Laszlo Agocs <laszlo.agocs@qt.io>

2 files changed, 40 insertions, 34 deletions

diff --git a/src/runtime/q3dsanimationmanager.cpp b/src/runtime/q3dsanimationmanager.cpp
index 1bab52f..0ab3310 100644
--- a/src/runtime/q3dsanimationmanager.cpp
+++ b/src/runtime/q3dsanimationmanager.cpp
@@ -65,25 +65,31 @@ public:
 private:
     static QVariant stabilizeAnimatedValue(const QVariant &value, QVariant::Type type);
 
+    friend class Q3DSAnimationManager;
+    friend uint qHash(const Q3DSAnimationManager::AnimationValueChange &, uint);
+    friend bool operator==(const Q3DSAnimationManager::AnimationValueChange &, const Q3DSAnimationManager::AnimationValueChange &);
+
     Q3DSGraphObject *m_target;
     Q3DSAnimationManager *m_animationManager;
     QMetaProperty m_property;
     QString m_propertyName;
+    QVariant m_value;
     QVariant::Type m_type;
+    int m_key = -1;
 };
 
 void Q3DSAnimationCallback::valueChanged(const QVariant &value)
 {
+    if (m_key < 0)
+        m_key = int(::qHash(Q3DSAnimationManager::AnimationValueChange{this}, uint(qGlobalQHashSeed())));
+
     // Do not directly change the value and trigger change notifications.
     // Instead, queue up (and compress), and defer to applyChanges() which is
     // invoked once per frame.
 
-    Q3DSAnimationManager::AnimationValueChange change;
     // Don't use the property type/name directly as it might be a dynamic type, i.e., a QVariantMap
-    change.value = stabilizeAnimatedValue(value, m_type);
-    change.property = m_property;
-    change.propertyName = m_propertyName;
-    m_animationManager->queueChange(m_target, change);
+    m_value = stabilizeAnimatedValue(value, m_type);
+    m_animationManager->queueChange({this});
 }
 
 QVariant Q3DSAnimationCallback::stabilizeAnimatedValue(const QVariant &value, QVariant::Type type)
@@ -544,28 +550,16 @@ void Q3DSAnimationManager::applyChanges()
     // initializing animations for an object with previously animated values).
     // One target can have more than one property change. These get compressed
     // so that there is still only one notifyPropertyChanges() call per object.
-
-    static const bool animDebug = qEnvironmentVariableIntValue("Q3DS_DEBUG") >= 3;
-    const QList<Q3DSGraphObject *> keys = m_changes.uniqueKeys();
-    for (Q3DSGraphObject *target : keys) {
-        if (!m_activeTargets.contains(target))
+    for (const auto &change : qAsConst(m_changes)) {
+        if (!m_activeTargets.contains(change.cb->m_target))
             continue;
 
-        auto it = m_changes.find(target);
-        Q3DSPropertyChangeList changeList;
-        while (it != m_changes.cend() && it.key() == target) {
-            if (Q_UNLIKELY(animDebug))
-                qDebug() << "animate:" << target->id() << it->propertyName << it->value;
-            if (it->property.type() == QVariant::Map) {
-                it->property.writeOnGadget(target, QVariantMap{{it->propertyName, it->value}});
-            } else {
-                it->property.writeOnGadget(target, it->value);
-            }
-            changeList.append(Q3DSPropertyChange(it->propertyName));
-            ++it;
+        if (change.cb->m_property.type() == QVariant::Map) {
+            change.cb->m_property.writeOnGadget(change.cb->m_target, QVariantMap{{change.cb->m_propertyName, change.cb->m_value}});
+        } else {
+            change.cb->m_property.writeOnGadget(change.cb->m_target, change.cb->m_value);
         }
-        if (!changeList.isEmpty())
-            target->notifyPropertyChanges(changeList);
+        change.cb->m_target->notifyPropertyChanges({Q3DSPropertyChange(change.cb->m_propertyName)});
     }
     m_changes.clear();
 }
@@ -580,10 +574,20 @@ void Q3DSAnimationManager::objectAboutToBeRemovedFromScene(Q3DSGraphObject *obj)
     m_activeTargets.remove(obj);
 }
 
-void Q3DSAnimationManager::queueChange(Q3DSGraphObject *target, const AnimationValueChange &change)
+void Q3DSAnimationManager::queueChange(const AnimationValueChange &change)
+{
+    m_changes.insert(change);
+}
+
+uint qHash(const Q3DSAnimationManager::AnimationValueChange &key, uint seed)
+{
+    return (key.cb->m_key < 0) ? ::qHash(QStringLiteral("%1%2").arg(key.cb->m_propertyName).arg(quintptr(key.cb->m_target)), seed)
+                               : uint(key.cb->m_key);
+}
+
+bool operator==(const Q3DSAnimationManager::AnimationValueChange &cl, const Q3DSAnimationManager::AnimationValueChange &cr)
 {
-    if (m_activeTargets.contains(target))
-        m_changes.insert(target, change);
+    return cl.cb && (cl.cb->m_target == cr.cb->m_target) && (cl.cb->m_propertyName == cr.cb->m_propertyName);
 }
 
 QT_END_NAMESPACE
diff --git a/src/runtime/q3dsanimationmanager_p.h b/src/runtime/q3dsanimationmanager_p.h
index f86bb9c..db1469b 100644
--- a/src/runtime/q3dsanimationmanager_p.h
+++ b/src/runtime/q3dsanimationmanager_p.h
@@ -48,6 +48,8 @@
 
 QT_BEGIN_NAMESPACE
 
+class Q3DSAnimationCallback;
+
 class Q3DSAnimationManager
 {
 public:
@@ -60,13 +62,9 @@ public:
     void clearPendingChanges();
     void objectAboutToBeRemovedFromScene(Q3DSGraphObject *obj);
 
-    struct AnimationValueChange {
-        QVariant value;
-        QString propertyName;
-        QMetaProperty property;
-    };
+    struct AnimationValueChange { Q3DSAnimationCallback *cb; };
 
-    void queueChange(Q3DSGraphObject *target, const AnimationValueChange &change);
+    void queueChange(const AnimationValueChange &change);
 
 private:
     void updateAnimationHelper(const AnimationTrackListMap &targets,
@@ -75,13 +73,17 @@ private:
 
     void buildClipAnimator(Q3DSSlide *slide);
 
-    QMultiHash<Q3DSGraphObject *, AnimationValueChange> m_changes;
+    QSet<AnimationValueChange> m_changes;
 
     QSet<Q3DSGraphObject *> m_activeTargets;
 
     friend class Q3DSAnimationCallback;
 };
 
+inline uint qHash(const Q3DSAnimationManager::AnimationValueChange &key, uint seed);
+inline bool operator==(const Q3DSAnimationManager::AnimationValueChange &cl,
+                       const Q3DSAnimationManager::AnimationValueChange &cr);
+
 QT_END_NAMESPACE
 
 #endif // Q3DSANIMATIONBUILDER_P_H