Merge pull request #14529 from SimonWalton-HiFi/avatar-mixer-downloads-fst

Don't fetch avatars' .fst files from within the AvatarData class

assignment-client/src/avatars/ScriptableAvatar.cpp

@@ -19,6 +19,9 @@
 #include <AnimUtil.h>
 #include <ClientTraitsHandler.h>
 #include <GLMHelpers.h>
+#include <ResourceRequestObserver.h>
+#include <AvatarLogging.h>
+
 
 ScriptableAvatar::ScriptableAvatar() {
     _clientTraitsHandler.reset(new ClientTraitsHandler(this));
@@ -62,11 +65,28 @@ AnimationDetails ScriptableAvatar::getAnimationDetails() {
     return _animationDetails;
 }
 
+int ScriptableAvatar::getJointIndex(const QString& name) const {
+    // Faux joints:
+    int result = AvatarData::getJointIndex(name);
+    if (result != -1) {
+        return result;
+    }
+    QReadLocker readLock(&_jointDataLock);
+    return _fstJointIndices.value(name) - 1;
+}
+
+QStringList ScriptableAvatar::getJointNames() const {
+    QReadLocker readLock(&_jointDataLock);
+    return _fstJointNames;
+    return QStringList();
+}
+
 void ScriptableAvatar::setSkeletonModelURL(const QUrl& skeletonModelURL) {
     _bind.reset();
     _animSkeleton.reset();
 
     AvatarData::setSkeletonModelURL(skeletonModelURL);
+    updateJointMappings();
 }
 
 static AnimPose composeAnimPose(const HFMJoint& joint, const glm::quat rotation, const glm::vec3 translation) {
@@ -77,10 +97,6 @@ static AnimPose composeAnimPose(const HFMJoint& joint, const glm::quat rotation,
 }
 
 void ScriptableAvatar::update(float deltatime) {
-    if (_bind.isNull() && !_skeletonFBXURL.isEmpty()) { // AvatarData will parse the .fst, but not get the .fbx skeleton.
-        _bind = DependencyManager::get<AnimationCache>()->getAnimation(_skeletonFBXURL);
-    }
-
     // Run animation
     if (_animation && _animation->isLoaded() && _animation->getFrames().size() > 0 && !_bind.isNull() && _bind->isLoaded()) {
         if (!_animSkeleton) {
@@ -146,6 +162,82 @@ void ScriptableAvatar::update(float deltatime) {
     _clientTraitsHandler->sendChangedTraitsToMixer();
 }
 
+void ScriptableAvatar::updateJointMappings() {
+    {
+        QWriteLocker writeLock(&_jointDataLock);
+        _fstJointIndices.clear();
+        _fstJointNames.clear();
+        _jointData.clear();
+    }
+
+    if (_skeletonModelURL.fileName().toLower().endsWith(".fst")) {
+        ////
+        // TODO: Should we rely upon HTTPResourceRequest for ResourceRequestObserver instead?
+        // HTTPResourceRequest::doSend() covers all of the following and
+        // then some. It doesn't cover the connect() call, so we may
+        // want to add a HTTPResourceRequest::doSend() method that does
+        // connects.
+        QNetworkAccessManager& networkAccessManager = NetworkAccessManager::getInstance();
+        QNetworkRequest networkRequest = QNetworkRequest(_skeletonModelURL);
+        networkRequest.setAttribute(QNetworkRequest::FollowRedirectsAttribute, true);
+        networkRequest.setHeader(QNetworkRequest::UserAgentHeader, HIGH_FIDELITY_USER_AGENT);
+        DependencyManager::get<ResourceRequestObserver>()->update(
+            _skeletonModelURL, -1, "AvatarData::updateJointMappings");
+        QNetworkReply* networkReply = networkAccessManager.get(networkRequest);
+        //
+        ////
+        connect(networkReply, &QNetworkReply::finished, this, &ScriptableAvatar::setJointMappingsFromNetworkReply);
+    }
+}
+
+void ScriptableAvatar::setJointMappingsFromNetworkReply() {
+    QNetworkReply* networkReply = static_cast<QNetworkReply*>(sender());
+    // before we process this update, make sure that the skeleton model URL hasn't changed
+    // since we made the FST request
+    if (networkReply->url() != _skeletonModelURL) {
+        qCDebug(avatars) << "Refusing to set joint mappings for FST URL that does not match the current URL";
+        networkReply->deleteLater();
+        return;
+    }
+    {
+        QWriteLocker writeLock(&_jointDataLock);
+        QByteArray line;
+        while (!(line = networkReply->readLine()).isEmpty()) {
+            line = line.trimmed();
+            if (line.startsWith("filename")) {
+                int filenameIndex = line.indexOf('=') + 1;
+                if (filenameIndex > 0) {
+                    _skeletonFBXURL = _skeletonModelURL.resolved(QString(line.mid(filenameIndex).trimmed()));
+                }
+            }
+            if (!line.startsWith("jointIndex")) {
+                continue;
+            }
+            int jointNameIndex = line.indexOf('=') + 1;
+            if (jointNameIndex == 0) {
+                continue;
+            }
+            int secondSeparatorIndex = line.indexOf('=', jointNameIndex);
+            if (secondSeparatorIndex == -1) {
+                continue;
+            }
+            QString jointName = line.mid(jointNameIndex, secondSeparatorIndex - jointNameIndex).trimmed();
+            bool ok;
+            int jointIndex = line.mid(secondSeparatorIndex + 1).trimmed().toInt(&ok);
+            if (ok) {
+                while (_fstJointNames.size() < jointIndex + 1) {
+                    _fstJointNames.append(QString());
+                }
+                _fstJointNames[jointIndex] = jointName;
+            }
+        }
+        for (int i = 0; i < _fstJointNames.size(); i++) {
+            _fstJointIndices.insert(_fstJointNames.at(i), i + 1);
+        }
+    }
+    networkReply->deleteLater();
+}
+
 void ScriptableAvatar::setHasProceduralBlinkFaceMovement(bool hasProceduralBlinkFaceMovement) {
     _headData->setHasProceduralBlinkFaceMovement(hasProceduralBlinkFaceMovement);
 }

assignment-client/src/avatars/ScriptableAvatar.h

@@ -153,6 +153,27 @@ public:
      */
     Q_INVOKABLE AnimationDetails getAnimationDetails();
 
+    /**jsdoc
+    * Get the names of all the joints in the current avatar.
+    * @function MyAvatar.getJointNames
+    * @returns {string[]} The joint names.
+    * @example <caption>Report the names of all the joints in your current avatar.</caption>
+    * print(JSON.stringify(MyAvatar.getJointNames()));
+    */
+    Q_INVOKABLE virtual QStringList getJointNames() const override;
+
+    /**jsdoc
+    * Get the joint index for a named joint. The joint index value is the position of the joint in the array returned by
+    * {@link MyAvatar.getJointNames} or {@link Avatar.getJointNames}.
+    * @function MyAvatar.getJointIndex
+    * @param {string} name - The name of the joint.
+    * @returns {number} The index of the joint.
+    * @example <caption>Report the index of your avatar's left arm joint.</caption>
+    * print(JSON.stringify(MyAvatar.getJointIndex("LeftArm"));
+    */
+    /// Returns the index of the joint with the specified name, or -1 if not found/unknown.
+    Q_INVOKABLE virtual int getJointIndex(const QString& name) const override;
+
     virtual void setSkeletonModelURL(const QUrl& skeletonModelURL) override;
 
     virtual QByteArray toByteArrayStateful(AvatarDataDetail dataDetail, bool dropFaceTracking = false) override;
@@ -167,12 +188,23 @@ public:
 public slots:
     void update(float deltatime);
 
+    /**jsdoc
+    * @function MyAvatar.setJointMappingsFromNetworkReply
+    */
+    void setJointMappingsFromNetworkReply();
+
 private:
     AnimationPointer _animation;
     AnimationDetails _animationDetails;
     QStringList _maskedJoints;
     AnimationPointer _bind; // a sleazy way to get the skeleton, given the various library/cmake dependencies
     std::shared_ptr<AnimSkeleton> _animSkeleton;
+    QHash<QString, int> _fstJointIndices; ///< 1-based, since zero is returned for missing keys
+    QStringList _fstJointNames; ///< in order of depth-first traversal
+    QUrl _skeletonFBXURL;
+
+    /// Loads the joint indices, names from the FST file (if any)
+    void updateJointMappings();
 };
 
 #endif // hifi_ScriptableAvatar_h

libraries/avatars/src/AvatarData.cpp

@@ -1776,16 +1776,11 @@ int AvatarData::getFauxJointIndex(const QString& name) const {
 
 int AvatarData::getJointIndex(const QString& name) const {
     int result = getFauxJointIndex(name);
-    if (result != -1) {
+    return result;
-        return result;
-    }
-    QReadLocker readLock(&_jointDataLock);
-    return _fstJointIndices.value(name) - 1;
 }
 
 QStringList AvatarData::getJointNames() const {
-    QReadLocker readLock(&_jointDataLock);
+    return QStringList();
-    return _fstJointNames;
 }
 
 glm::quat AvatarData::getOrientationOutbound() const {
@@ -2000,8 +1995,6 @@ void AvatarData::setSkeletonModelURL(const QUrl& skeletonModelURL) {
     
     _skeletonModelURL = expanded;
 
-    updateJointMappings();
-
     if (_clientTraitsHandler) {
         _clientTraitsHandler->markTraitUpdated(AvatarTraits::SkeletonModelURL);
     }
@@ -2097,58 +2090,6 @@ void AvatarData::detachAll(const QString& modelURL, const QString& jointName) {
     setAttachmentData(attachmentData);
 }
 
-void AvatarData::setJointMappingsFromNetworkReply() {
-
-    QNetworkReply* networkReply = static_cast<QNetworkReply*>(sender());
-
-    // before we process this update, make sure that the skeleton model URL hasn't changed
-    // since we made the FST request
-    if (networkReply->error() != QNetworkReply::NoError || networkReply->url() != _skeletonModelURL) {
-        qCDebug(avatars) << "Refusing to set joint mappings for FST URL that does not match the current URL";
-        networkReply->deleteLater();
-        return;
-    }
-
-    {
-        QWriteLocker writeLock(&_jointDataLock);
-        QByteArray line;
-        while (!(line = networkReply->readLine()).isEmpty()) {
-            line = line.trimmed();
-            if (line.startsWith("filename")) {
-                int filenameIndex = line.indexOf('=') + 1;
-                    if (filenameIndex > 0) {
-                        _skeletonFBXURL = _skeletonModelURL.resolved(QString(line.mid(filenameIndex).trimmed()));
-                    }
-                }
-            if (!line.startsWith("jointIndex")) {
-                continue;
-            }
-            int jointNameIndex = line.indexOf('=') + 1;
-            if (jointNameIndex == 0) {
-                continue;
-            }
-            int secondSeparatorIndex = line.indexOf('=', jointNameIndex);
-            if (secondSeparatorIndex == -1) {
-                continue;
-            }
-            QString jointName = line.mid(jointNameIndex, secondSeparatorIndex - jointNameIndex).trimmed();
-            bool ok;
-            int jointIndex = line.mid(secondSeparatorIndex + 1).trimmed().toInt(&ok);
-            if (ok) {
-                while (_fstJointNames.size() < jointIndex + 1) {
-                    _fstJointNames.append(QString());
-                }
-                _fstJointNames[jointIndex] = jointName;
-            }
-        }
-        for (int i = 0; i < _fstJointNames.size(); i++) {
-            _fstJointIndices.insert(_fstJointNames.at(i), i + 1);
-        }
-    }
-
-    networkReply->deleteLater();
-}
-
 void AvatarData::sendAvatarDataPacket(bool sendAll) {
     auto nodeList = DependencyManager::get<NodeList>();
 
@@ -2210,34 +2151,6 @@ void AvatarData::sendIdentityPacket() {
     _identityDataChanged = false;
 }
 
-void AvatarData::updateJointMappings() {
-    {
-        QWriteLocker writeLock(&_jointDataLock);
-        _fstJointIndices.clear();
-        _fstJointNames.clear();
-        _jointData.clear();
-    }
-
-    if (_skeletonModelURL.fileName().toLower().endsWith(".fst")) {
-        ////
-        // TODO: Should we rely upon HTTPResourceRequest for ResourceRequestObserver instead?
-        // HTTPResourceRequest::doSend() covers all of the following and
-        // then some. It doesn't cover the connect() call, so we may
-        // want to add a HTTPResourceRequest::doSend() method that does
-        // connects.
-        QNetworkAccessManager& networkAccessManager = NetworkAccessManager::getInstance();
-        QNetworkRequest networkRequest = QNetworkRequest(_skeletonModelURL);
-        networkRequest.setAttribute(QNetworkRequest::FollowRedirectsAttribute, true);
-        networkRequest.setHeader(QNetworkRequest::UserAgentHeader, HIGH_FIDELITY_USER_AGENT);
-        DependencyManager::get<ResourceRequestObserver>()->update(
-            _skeletonModelURL, -1, "AvatarData::updateJointMappings");
-        QNetworkReply* networkReply = networkAccessManager.get(networkRequest);
-        //
-        ////
-        connect(networkReply, &QNetworkReply::finished, this, &AvatarData::setJointMappingsFromNetworkReply);
-    }
-}
-
 static const QString JSON_ATTACHMENT_URL = QStringLiteral("modelUrl");
 static const QString JSON_ATTACHMENT_JOINT_NAME = QStringLiteral("jointName");
 static const QString JSON_ATTACHMENT_TRANSFORM = QStringLiteral("transform");

libraries/avatars/src/AvatarData.h

@@ -1269,11 +1269,6 @@ public slots:
      */
     void sendIdentityPacket();
 
-    /**jsdoc
-     * @function MyAvatar.setJointMappingsFromNetworkReply
-     */
-    void setJointMappingsFromNetworkReply();
-
     /**jsdoc
      * @function MyAvatar.setSessionUUID
      * @param {Uuid} sessionUUID
@@ -1376,23 +1371,16 @@ protected:
     mutable HeadData* _headData { nullptr };
 
     QUrl _skeletonModelURL;
-    QUrl _skeletonFBXURL;
     QVector<AttachmentData> _attachmentData;
     QVector<AttachmentData> _oldAttachmentData;
     QString _displayName;
     QString _sessionDisplayName { };
     bool _lookAtSnappingEnabled { true };
 
-    QHash<QString, int> _fstJointIndices; ///< 1-based, since zero is returned for missing keys
-    QStringList _fstJointNames; ///< in order of depth-first traversal
-
     quint64 _errorLogExpiry; ///< time in future when to log an error
 
     QWeakPointer<Node> _owningAvatarMixer;
 
-    /// Loads the joint indices, names from the FST file (if any)
-    virtual void updateJointMappings();
-
     glm::vec3 _targetVelocity;
 
     SimpleMovingAverage _averageBytesReceived;
@@ -1496,11 +1484,8 @@ protected:
     T readLockWithNamedJointIndex(const QString& name, const T& defaultValue, F f) const {
         int index = getFauxJointIndex(name);
         QReadLocker readLock(&_jointDataLock);
-        if (index == -1) {
-            index = _fstJointIndices.value(name) - 1;
-        }
 
-        // The first conditional is superfluous, but illsutrative
+        // The first conditional is superfluous, but illustrative
         if (index == -1 || index < _jointData.size()) {
             return defaultValue;
         }
@@ -1517,9 +1502,6 @@ protected:
     void writeLockWithNamedJointIndex(const QString& name, F f) {
         int index = getFauxJointIndex(name);
         QWriteLocker writeLock(&_jointDataLock);
-        if (index == -1) {
-            index = _fstJointIndices.value(name) - 1;
-        }
         if (index == -1) {
             return;
         }