Fix reformatting of clip evaluation results

This now correctly handles both the old ChannelMapping and the
new SkeletonMapping use cases. There was some ambiguity about
which way around the indices were being generated. It worked in
previous cases for the limited data available to test with. This
has been tested to work with both the JSON animation format and
skeletal animations loaded from JSON and glTF 2 files.

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

1 files changed, 48 insertions, 12 deletions

diff --git a/src/animation/backend/animationutils.cpp b/src/animation/backend/animationutils.cpp
index 7051fd4d4..8308fab34 100644
--- a/src/animation/backend/animationutils.cpp
+++ b/src/animation/backend/animationutils.cpp
@@ -147,6 +147,17 @@ double phaseFromGlobalTime(double t_global, double t_start_global,
     return t_local / duration;
 }
 
+/*!
+    \internal
+
+    Calculates the indices required to map from the component ordering within the
+    provided \a channel, into the standard channel orderings expected by Qt types.
+
+    For example, given a channel representing a rotation with the components ordered
+    as X, Y, Z, Y, this function will return the indices [3, 0, 1, 2] which can then
+    later be used as part of the format vector in the formatClipResults() function to
+    remap the channels into the standard W, X, Y, Z order required by QQuaternion.
+*/
 ComponentIndices channelComponentsToIndices(const Channel &channel, int dataType, int offset)
 {
 #if defined Q_COMPILER_UNIFORM_INIT
@@ -170,9 +181,9 @@ ComponentIndices channelComponentsToIndices(const Channel &channel, int dataType
 }
 
 ComponentIndices channelComponentsToIndicesHelper(const Channel &channel,
-                                              int dataType,
-                                              int offset,
-                                              const QVector<char> &suffixes)
+                                                  int dataType,
+                                                  int offset,
+                                                  const QVector<char> &suffixes)
 {
     const int expectedComponentCount = componentsForType(dataType);
     const int actualComponentCount = channel.channelComponents.size();
@@ -182,15 +193,37 @@ ComponentIndices channelComponentsToIndicesHelper(const Channel &channel,
     }
 
     ComponentIndices indices(expectedComponentCount);
+
+    // Generate the set of channel suffixes
+    QVector<char> channelSuffixes;
+    channelSuffixes.reserve(expectedComponentCount);
     for (int i = 0; i < expectedComponentCount; ++i) {
         const QString &componentName = channel.channelComponents[i].name;
-        char suffix = componentName.at(componentName.length() - 1).toLatin1();
-        int index = suffixes.indexOf(suffix);
+
+        // An unset component name indicates that the no mapping is necessary
+        // and the index can be used as-is.
+        if (componentName.isEmpty()) {
+            indices[i] = i + offset;
+            continue;
+        }
+
+        char channelSuffix = componentName.at(componentName.length() - 1).toLatin1();
+        channelSuffixes.push_back(channelSuffix);
+    }
+
+    // We can short-circuit if the channels were all unnamed (in order)
+    if (channelSuffixes.isEmpty())
+        return indices;
+
+    // Find index of standard index in channel indexes
+    for (int i = 0; i < expectedComponentCount; ++i) {
+        int index = channelSuffixes.indexOf(suffixes[i]);
         if (index != -1)
             indices[i] = index + offset;
         else
             indices[i] = -1;
     }
+
     return indices;
 }
 
@@ -456,16 +489,14 @@ ComponentIndices generateClipFormatIndices(const QVector<ChannelNameAndType> &ta
         const int componentCount = targetIndices[i].size();
 
         if (clipChannelIndex != -1) {
-            // Found a matching channel in the clip. Get the base channel
-            // component index and populate the format indices for this channel.
+            // Found a matching channel in the clip. Populate the corresponding
+            // entries in the format vector with the *source indices*
+            // needed to build the formatted results.
             const int baseIndex = clip->channelComponentBaseIndex(clipChannelIndex);
-
-            // Within this group, match channel names with index ordering
             const auto channelIndices = channelComponentsToIndices(clip->channels()[clipChannelIndex],
                                                                    targetChannel.type,
                                                                    baseIndex);
             std::copy(channelIndices.begin(), channelIndices.end(), formatIt);
-
         } else {
             // No such channel in this clip. We'll use default values when
             // mapping from the clip to the formatted clip results.
@@ -486,11 +517,16 @@ ClipResults formatClipResults(const ClipResults &rawClipResults,
     ClipResults formattedClipResults(elementCount);
 
     // Perform a gather operation to format the data
+
     // TODO: For large numbers of components do this in parallel with
     // for e.g. a parallel_for() like construct
+    // TODO: We could potentially avoid having holes in these intermediate
+    // vectors by adjusting the component indices stored in the MappingData
+    // and format vectors. Needs careful investigation!
     for (int i = 0; i < elementCount; ++i) {
-        const float value = format[i] != -1 ? rawClipResults[format[i]] : 0.0f;
-        formattedClipResults[i] = value;
+        if (format[i] == -1)
+            continue;
+        formattedClipResults[i] = rawClipResults[format[i]];
     }
 
     return formattedClipResults;