Add a property map job in animation aspect

ClipAnimator is always dirty because at each frame the normalizedTime is
changed. This has a big performance penalisation as we are launching
 job which updates the property map at each frame.

This patch adds a new dirty flag which is use to decide if the animator
need to update the mapping of the fcurve to actual node properties.

Using that flag, a new UpdatePropertyMapJob is launched for dirty
clipAnimators.

Some scenes boost from 30fps to 60fps (vsync) during animation.

Change-Id: Ic8a8db01535c55995bc569ea5a69660a40014401
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>

10 files changed, 253 insertions, 37 deletions

diff --git a/src/animation/backend/backend.pri b/src/animation/backend/backend.pri
index cc1104102..54f0961fb 100644
--- a/src/animation/backend/backend.pri
+++ b/src/animation/backend/backend.pri
@@ -29,7 +29,8 @@ HEADERS += \
     $$PWD/animationclip_p.h \
     $$PWD/clock_p.h \
     $$PWD/skeleton_p.h \
-    $$PWD/gltfimporter_p.h
+    $$PWD/gltfimporter_p.h \
+    $$PWD/updatepropertymapjob_p.h
 
 SOURCES += \
     $$PWD/handler.cpp \
@@ -56,4 +57,5 @@ SOURCES += \
     $$PWD/animationclip.cpp \
     $$PWD/clock.cpp \
     $$PWD/skeleton.cpp \
-    $$PWD/gltfimporter.cpp
+    $$PWD/gltfimporter.cpp \
+    $$PWD/updatepropertymapjob.cpp
diff --git a/src/animation/backend/clipanimator.cpp b/src/animation/backend/clipanimator.cpp
index acb3c8170..1c65946a9 100644
--- a/src/animation/backend/clipanimator.cpp
+++ b/src/animation/backend/clipanimator.cpp
@@ -76,12 +76,14 @@ void ClipAnimator::initializeFromPeer(const Qt3DCore::QNodeCreatedChangeBasePtr
     m_loops = data.loops;
     m_normalizedLocalTime = data.normalizedTime;
     setDirty(Handler::ClipAnimatorDirty);
+    setDirty(Handler::ClipAnimatorMapDirty);
 }
 
 void ClipAnimator::setClipId(Qt3DCore::QNodeId clipId)
 {
     m_clipId = clipId;
     setDirty(Handler::ClipAnimatorDirty);
+    setDirty(Handler::ClipAnimatorMapDirty);
 
     // register at the clip to make sure we are marked dirty when the clip finished loading
     AnimationClip *clip = m_handler->animationClipLoaderManager()->lookupResource(clipId);
@@ -92,7 +94,7 @@ void ClipAnimator::setClipId(Qt3DCore::QNodeId clipId)
 void ClipAnimator::setMapperId(Qt3DCore::QNodeId mapperId)
 {
     m_mapperId = mapperId;
-    setDirty(Handler::ClipAnimatorDirty);
+    setDirty(Handler::ClipAnimatorMapDirty);
 }
 
 void ClipAnimator::setClockId(Qt3DCore::QNodeId clockId)
@@ -107,6 +109,13 @@ void ClipAnimator::setRunning(bool running)
     if (!running)
         m_currentLoop = 0;
     setDirty(Handler::ClipAnimatorDirty);
+    setDirty(Handler::ClipAnimatorMapDirty);
+}
+
+void ClipAnimator::setLoops(int loops)
+{
+    m_loops = loops;
+    setDirty(Handler::ClipAnimatorDirty);
 }
 
 void ClipAnimator::setNormalizedLocalTime(float normalizedTime)
@@ -143,7 +152,7 @@ void ClipAnimator::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e)
         else if (change->propertyName() == QByteArrayLiteral("running"))
             setRunning(change->value().toBool());
         else if (change->propertyName() == QByteArrayLiteral("loops"))
-            m_loops = change->value().toInt();
+            setLoops(change->value().toInt());
         else if (change->propertyName() == QByteArrayLiteral("normalizedTime"))
             setNormalizedLocalTime(change->value().toFloat());
         break;
diff --git a/src/animation/backend/clipanimator_p.h b/src/animation/backend/clipanimator_p.h
index f4c04a4bf..9def57774 100644
--- a/src/animation/backend/clipanimator_p.h
+++ b/src/animation/backend/clipanimator_p.h
@@ -75,7 +75,7 @@ public:
 
     void setRunning(bool running);
     bool isRunning() const { return m_running; }
-    void setLoops(int loops) { m_loops = loops; }
+    void setLoops(int loops);
     int loops() const { return m_loops; }
     void setNormalizedLocalTime(float normalizedLocalTime);
     float normalizedLocalTime() const { return m_normalizedLocalTime; }
diff --git a/src/animation/backend/findrunningclipanimatorsjob.cpp b/src/animation/backend/findrunningclipanimatorsjob.cpp
index fde3f7db8..83b2e840b 100644
--- a/src/animation/backend/findrunningclipanimatorsjob.cpp
+++ b/src/animation/backend/findrunningclipanimatorsjob.cpp
@@ -72,37 +72,6 @@ void FindRunningClipAnimatorsJob::run()
         const bool running = clipAnimator->isRunning();
         const bool seeking = clipAnimator->isSeeking();
         m_handler->setClipAnimatorRunning(clipAnimatorHandle, canRun && (seeking || running));
-
-        // TODO: Actually check if this is needed first, currently we re-build this every time
-        // canRun (or the normalized time) is true.
-        if (!canRun || !(seeking || running))
-            continue;
-
-        // The clip animator needs to know how to map fcurve values through to properties on QNodes.
-        // Now we know this animator can run, build the mapping table. Even though this could be
-        // done a little simpler in the non-blended case, we follow the same code path as the
-        // blended clip animator for consistency and ease of maintenance.
-        const ChannelMapper *mapper = m_handler->channelMapperManager()->lookupResource(clipAnimator->mapperId());
-        Q_ASSERT(mapper);
-        const QVector<ChannelMapping *> channelMappings = mapper->mappings();
-
-        const QVector<ChannelNameAndType> channelNamesAndTypes
-                = buildRequiredChannelsAndTypes(m_handler, mapper);
-        const QVector<ComponentIndices> channelComponentIndices
-                = assignChannelComponentIndices(channelNamesAndTypes);
-
-        const AnimationClip *clip = m_handler->animationClipLoaderManager()->lookupResource(clipAnimator->clipId());
-        Q_ASSERT(clip);
-        const ClipFormat format = generateClipFormatIndices(channelNamesAndTypes,
-                                                            channelComponentIndices,
-                                                            clip);
-        clipAnimator->setClipFormat(format);
-
-        const QVector<MappingData> mappingData = buildPropertyMappings(channelMappings,
-                                                                       channelNamesAndTypes,
-                                                                       format.formattedComponentIndices,
-                                                                       format.sourceClipMask);
-        clipAnimator->setMappingData(mappingData);
     }
 
     qCDebug(Jobs) << "Running clip animators =" << m_handler->runningClipAnimators();
diff --git a/src/animation/backend/findrunningclipanimatorsjob_p.h b/src/animation/backend/findrunningclipanimatorsjob_p.h
index f0e30e80c..5abefde97 100644
--- a/src/animation/backend/findrunningclipanimatorsjob_p.h
+++ b/src/animation/backend/findrunningclipanimatorsjob_p.h
@@ -54,6 +54,7 @@
 
 #if defined(QT_BUILD_INTERNAL)
 class tst_FindRunningClipAnimatorsJob;
+class tst_UpdatePropertyMapJob;
 #endif
 
 QT_BEGIN_NAMESPACE
@@ -82,6 +83,7 @@ private:
 
 #if defined(QT_BUILD_INTERNAL)
     friend class ::tst_FindRunningClipAnimatorsJob;
+    friend class ::tst_UpdatePropertyMapJob;
 #endif
 };
 
diff --git a/src/animation/backend/handler.cpp b/src/animation/backend/handler.cpp
index 112e2742b..da2adf494 100644
--- a/src/animation/backend/handler.cpp
+++ b/src/animation/backend/handler.cpp
@@ -38,6 +38,7 @@
 #include <Qt3DAnimation/private/managers_p.h>
 #include <Qt3DAnimation/private/loadanimationclipjob_p.h>
 #include <Qt3DAnimation/private/findrunningclipanimatorsjob_p.h>
+#include <Qt3DAnimation/private/updatepropertymapjob_p.h>
 #include <Qt3DAnimation/private/evaluateclipanimatorjob_p.h>
 #include <Qt3DAnimation/private/buildblendtreesjob_p.h>
 #include <Qt3DAnimation/private/evaluateblendclipanimatorjob_p.h>
@@ -61,12 +62,14 @@ Handler::Handler()
     , m_skeletonManager(new SkeletonManager)
     , m_loadAnimationClipJob(new LoadAnimationClipJob)
     , m_findRunningClipAnimatorsJob(new FindRunningClipAnimatorsJob)
+    , m_updatePropertyMapJob(new UpdatePropertyMapJob)
     , m_buildBlendTreesJob(new BuildBlendTreesJob)
     , m_simulationTime(0)
 {
     m_loadAnimationClipJob->setHandler(this);
     m_findRunningClipAnimatorsJob->setHandler(this);
     m_buildBlendTreesJob->setHandler(this);
+    m_updatePropertyMapJob->setHandler(this);
 }
 
 Handler::~Handler()
@@ -97,6 +100,13 @@ void Handler::setDirty(DirtyFlag flag, Qt3DCore::QNodeId nodeId)
         break;
     }
 
+    case ClipAnimatorMapDirty: {
+        QMutexLocker lock(&m_mutex);
+        const auto handle = m_clipAnimatorManager->lookupHandle(nodeId);
+        m_dirtyClipAnimatorMaps.push_back(handle);
+        break;
+    }
+
     case BlendedClipAnimatorDirty: {
         QMutexLocker lock(&m_mutex);
         const HBlendedClipAnimator handle = m_blendedClipAnimatorManager->lookupHandle(nodeId);
@@ -217,6 +227,18 @@ QVector<Qt3DCore::QAspectJobPtr> Handler::jobsToExecute(qint64 time)
         m_dirtyClipAnimators.clear();
     }
 
+    const bool hasUpdatePropertyMapJob = !m_dirtyClipAnimatorMaps.isEmpty();
+    if (hasUpdatePropertyMapJob) {
+        qCDebug(HandlerLogic) << "Added UpdatePropertyMapJob";
+        cleanupHandleList(&m_dirtyClipAnimatorMaps);
+        m_updatePropertyMapJob->setDirtyClipAnimators(m_dirtyClipAnimatorMaps);
+        m_updatePropertyMapJob->removeDependency(QWeakPointer<Qt3DCore::QAspectJob>());
+        jobs.push_back(m_updatePropertyMapJob);
+        if (hasFindRunningClipAnimatorsJob)
+            m_updatePropertyMapJob->addDependency(m_findRunningClipAnimatorsJob);
+        m_dirtyClipAnimatorMaps.clear();
+    }
+
     // Rebuild blending trees if a blend tree is dirty
     const bool hasBuildBlendTreesJob = !m_dirtyBlendedAnimators.isEmpty();
     if (hasBuildBlendTreesJob) {
@@ -255,6 +277,10 @@ QVector<Qt3DCore::QAspectJobPtr> Handler::jobsToExecute(qint64 time)
             if (hasFindRunningClipAnimatorsJob &&
                     !m_evaluateClipAnimatorJobs[i]->dependencies().contains(m_findRunningClipAnimatorsJob))
                 m_evaluateClipAnimatorJobs[i]->addDependency(m_findRunningClipAnimatorsJob);
+
+            if (hasUpdatePropertyMapJob &&
+                    !m_evaluateClipAnimatorJobs[i]->dependencies().contains(m_updatePropertyMapJob))
+                m_evaluateClipAnimatorJobs[i]->addDependency(m_updatePropertyMapJob);
             jobs.push_back(m_evaluateClipAnimatorJobs[i]);
         }
     }
diff --git a/src/animation/backend/handler_p.h b/src/animation/backend/handler_p.h
index 99e2ae1f6..449173f6c 100644
--- a/src/animation/backend/handler_p.h
+++ b/src/animation/backend/handler_p.h
@@ -55,6 +55,10 @@
 #include <QtCore/qscopedpointer.h>
 #include <QtCore/qmutex.h>
 
+#if defined(QT_BUILD_INTERNAL)
+class tst_ClipAnimator;
+#endif
+
 QT_BEGIN_NAMESPACE
 
 #if defined(QT_BUILD_INTERNAL)
@@ -79,6 +83,7 @@ class ClipBlendNodeManager;
 class SkeletonManager;
 
 class FindRunningClipAnimatorsJob;
+class UpdatePropertyMapJob;
 class LoadAnimationClipJob;
 class EvaluateClipAnimatorJob;
 class BuildBlendTreesJob;
@@ -97,6 +102,7 @@ public:
         AnimationClipDirty,
         ChannelMappingsDirty,
         ClipAnimatorDirty,
+        ClipAnimatorMapDirty,
         BlendedClipAnimatorDirty
     };
 
@@ -138,6 +144,7 @@ private:
 
     QVector<HAnimationClip> m_dirtyAnimationClips;
     QVector<HChannelMapper> m_dirtyChannelMappers;
+    QVector<HClipAnimator> m_dirtyClipAnimatorMaps;
     QVector<HClipAnimator> m_dirtyClipAnimators;
     QVector<HBlendedClipAnimator> m_dirtyBlendedAnimators;
 
@@ -146,6 +153,7 @@ private:
 
     QSharedPointer<LoadAnimationClipJob> m_loadAnimationClipJob;
     QSharedPointer<FindRunningClipAnimatorsJob> m_findRunningClipAnimatorsJob;
+    QSharedPointer<UpdatePropertyMapJob> m_updatePropertyMapJob;
     QVector<QSharedPointer<EvaluateClipAnimatorJob>> m_evaluateClipAnimatorJobs;
     QVector<EvaluateBlendClipAnimatorJobPtr> m_evaluateBlendClipAnimatorJobs;
     BuildBlendTreesJobPtr m_buildBlendTreesJob;
@@ -154,6 +162,7 @@ private:
 
 #if defined(QT_BUILD_INTERNAL)
     friend class QT_PREPEND_NAMESPACE(tst_Handler);
+    friend class ::tst_ClipAnimator;
 #endif
 };
 
diff --git a/src/animation/backend/updatepropertymapjob.cpp b/src/animation/backend/updatepropertymapjob.cpp
new file mode 100644
index 000000000..6184ea235
--- /dev/null
+++ b/src/animation/backend/updatepropertymapjob.cpp
@@ -0,0 +1,116 @@
+/****************************************************************************
+**
+** Copyright (C) 2019 Klaralvdalens Datakonsult AB (KDAB).
+** Contact: http://www.qt-project.org/legal
+**
+** This file is part of the Qt3D module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL3$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see http://www.qt.io/terms-conditions. For further
+** information use the contact form at http://www.qt.io/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 3 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPLv3 included in the
+** packaging of this file. Please review the following information to
+** ensure the GNU Lesser General Public License version 3 requirements
+** will be met: https://www.gnu.org/licenses/lgpl.html.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 2.0 or later as published by the Free
+** Software Foundation and appearing in the file LICENSE.GPL included in
+** the packaging of this file. Please review the following information to
+** ensure the GNU General Public License version 2.0 requirements will be
+** met: http://www.gnu.org/licenses/gpl-2.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#include "updatepropertymapjob_p.h"
+
+#include "findrunningclipanimatorsjob_p.h"
+#include <Qt3DAnimation/private/handler_p.h>
+#include <Qt3DAnimation/private/managers_p.h>
+#include <Qt3DAnimation/private/animationlogging_p.h>
+#include <Qt3DAnimation/private/animationutils_p.h>
+#include <Qt3DAnimation/private/job_common_p.h>
+
+QT_BEGIN_NAMESPACE
+
+namespace Qt3DAnimation {
+namespace Animation {
+
+UpdatePropertyMapJob::UpdatePropertyMapJob()
+    : Qt3DCore::QAspectJob()
+{
+    SET_JOB_RUN_STAT_TYPE(this, JobTypes::UpdatePropertyMap, 0)
+}
+
+void UpdatePropertyMapJob::setHandler(Handler *handler)
+{
+    m_handler = handler;
+}
+
+void UpdatePropertyMapJob::setDirtyClipAnimators(const QVector<HClipAnimator> &clipAnimators)
+{
+    m_clipAnimatorHandles = clipAnimators;
+}
+
+void UpdatePropertyMapJob::run()
+{
+    Q_ASSERT(m_handler);
+
+    const auto &runningClipAnimators = m_handler->runningClipAnimators();
+    auto *clipAnimatorManager = m_handler->clipAnimatorManager();
+    for (const auto &clipAnimatorHandle : qAsConst(m_clipAnimatorHandles)) {
+
+        ClipAnimator *clipAnimator = clipAnimatorManager->data(clipAnimatorHandle);
+        Q_ASSERT(clipAnimator);
+        if (!clipAnimator->isEnabled())
+            continue;
+
+        // Check the clipAnimator is running
+        // If its not running, no sense in updating the mapping
+        if (std::find(std::begin(runningClipAnimators), std::end(runningClipAnimators), clipAnimatorHandle) == std::end(runningClipAnimators))
+            continue;
+
+        // The clip animator needs to know how to map fcurve values through to properties on QNodes.
+        // Now we know this animator can run, build the mapping table. Even though this could be
+        // done a little simpler in the non-blended case, we follow the same code path as the
+        // blended clip animator for consistency and ease of maintenance.
+        const ChannelMapper *mapper = m_handler->channelMapperManager()->lookupResource(clipAnimator->mapperId());
+        Q_ASSERT(mapper);
+        const QVector<ChannelMapping *> channelMappings = mapper->mappings();
+
+        const QVector<ChannelNameAndType> channelNamesAndTypes
+                = buildRequiredChannelsAndTypes(m_handler, mapper);
+        const QVector<ComponentIndices> channelComponentIndices
+                = assignChannelComponentIndices(channelNamesAndTypes);
+
+        const AnimationClip *clip = m_handler->animationClipLoaderManager()->lookupResource(clipAnimator->clipId());
+        Q_ASSERT(clip);
+        const ClipFormat format = generateClipFormatIndices(channelNamesAndTypes,
+                                                            channelComponentIndices,
+                                                            clip);
+        clipAnimator->setClipFormat(format);
+
+        const QVector<MappingData> mappingData = buildPropertyMappings(channelMappings,
+                                                                       channelNamesAndTypes,
+                                                                       format.formattedComponentIndices,
+                                                                       format.sourceClipMask);
+        clipAnimator->setMappingData(mappingData);
+    }
+}
+
+} //  Animation
+} //  Qt3DAnimation
+
+QT_END_NAMESPACE
diff --git a/src/animation/backend/updatepropertymapjob_p.h b/src/animation/backend/updatepropertymapjob_p.h
new file mode 100644
index 000000000..fc29282f9
--- /dev/null
+++ b/src/animation/backend/updatepropertymapjob_p.h
@@ -0,0 +1,82 @@
+/****************************************************************************
+**
+** Copyright (C) 2019 Klaralvdalens Datakonsult AB (KDAB).
+** Contact: http://www.qt-project.org/legal
+**
+** This file is part of the Qt3D module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL3$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see http://www.qt.io/terms-conditions. For further
+** information use the contact form at http://www.qt.io/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 3 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPLv3 included in the
+** packaging of this file. Please review the following information to
+** ensure the GNU Lesser General Public License version 3 requirements
+** will be met: https://www.gnu.org/licenses/lgpl.html.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 2.0 or later as published by the Free
+** Software Foundation and appearing in the file LICENSE.GPL included in
+** the packaging of this file. Please review the following information to
+** ensure the GNU General Public License version 2.0 requirements will be
+** met: http://www.gnu.org/licenses/gpl-2.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#ifndef QT3DANIMATION_ANIMATION_UPDATEPROPERTYMAPJOB_P_H
+#define QT3DANIMATION_ANIMATION_UPDATEPROPERTYMAPJOB_P_H
+
+//
+//  W A R N I N G
+//  -------------
+//
+// This file is not part of the Qt API.  It exists for the convenience
+// of other Qt classes.  This header file may change from version to
+// version without notice, or even be removed.
+//
+// We mean it.
+//
+
+#include <Qt3DCore/qaspectjob.h>
+#include <Qt3DAnimation/private/handle_types_p.h>
+#include <QtCore/qvector.h>
+
+QT_BEGIN_NAMESPACE
+
+namespace Qt3DAnimation {
+namespace Animation {
+
+class Handler;
+
+class Q_AUTOTEST_EXPORT UpdatePropertyMapJob : public Qt3DCore::QAspectJob
+{
+public:
+    UpdatePropertyMapJob();
+
+    void setHandler(Handler *handler);
+    void setDirtyClipAnimators(const QVector<HClipAnimator> &clipAnimators);
+
+    void run() override;
+
+private:
+    QVector<HClipAnimator> m_clipAnimatorHandles;
+    Handler *m_handler;
+};
+
+} // Animation
+} // Qt3DAnimation
+
+QT_END_NAMESPACE
+
+#endif // QT3DANIMATION_ANIMATION_UPDATEPROPERTYMAPJOB_P_H
diff --git a/src/animation/job_common_p.h b/src/animation/job_common_p.h
index 882246a42..2473b61f8 100644
--- a/src/animation/job_common_p.h
+++ b/src/animation/job_common_p.h
@@ -65,7 +65,8 @@ enum JobType {
     EvaluateBlendClipAnimator,
     EvaluateClipAnimator,
     LoadAnimationClip,
-    FindRunningClipAnimator
+    FindRunningClipAnimator,
+    UpdatePropertyMap
 };
 
 } // JobTypes