diff --git a/libraries/animation/src/AnimVariant.h b/libraries/animation/src/AnimVariant.h index 7a80321f7b..bd96dda77d 100644 --- a/libraries/animation/src/AnimVariant.h +++ b/libraries/animation/src/AnimVariant.h @@ -163,7 +163,7 @@ public: bool hasKey(const QString& key) const { return _map.find(key) != _map.end(); } // Answer a Plain Old Javascript Object (for the given engine) all of our values set as properties. - QScriptValue animVariantMapToScriptValue(QScriptEngine* engine) const; + QScriptValue animVariantMapToScriptValue(QScriptEngine* engine, const QStringList& names, bool useNames) const; // Side-effect us with the value of object's own properties. (No inherited properties.) void animVariantMapFromScriptValue(const QScriptValue& object); void copyVariantsFrom(const AnimVariantMap& other); @@ -206,7 +206,7 @@ protected: std::set _triggers; }; -typedef std::function AnimVariantResultHandler; +typedef std::function AnimVariantResultHandler; Q_DECLARE_METATYPE(AnimVariantResultHandler); Q_DECLARE_METATYPE(AnimVariantMap) diff --git a/libraries/animation/src/AnimVariantMap.cpp b/libraries/animation/src/AnimVariantMap.cpp index 59f10a96e0..0c808bd404 100644 --- a/libraries/animation/src/AnimVariantMap.cpp +++ b/libraries/animation/src/AnimVariantMap.cpp @@ -15,36 +15,49 @@ #include #include "AnimVariant.h" -QScriptValue AnimVariantMap::animVariantMapToScriptValue(QScriptEngine* engine) const { +QScriptValue AnimVariantMap::animVariantMapToScriptValue(QScriptEngine* engine, const QStringList& names, bool useNames) const { if (QThread::currentThread() != engine->thread()) { qCWarning(animation) << "Cannot create Javacript object from non-script thread" << QThread::currentThread(); return QScriptValue(); } QScriptValue target = engine->newObject(); - for (auto& pair : _map) { - switch (pair.second.getType()) { + auto setOne = [&] (QString name, AnimVariant value) { + switch (value.getType()) { case AnimVariant::Type::Bool: - target.setProperty(pair.first, pair.second.getBool()); + target.setProperty(name, value.getBool()); break; case AnimVariant::Type::Int: - target.setProperty(pair.first, pair.second.getInt()); + target.setProperty(name, value.getInt()); break; case AnimVariant::Type::Float: - target.setProperty(pair.first, pair.second.getFloat()); + target.setProperty(name, value.getFloat()); break; case AnimVariant::Type::String: - target.setProperty(pair.first, pair.second.getString()); + target.setProperty(name, value.getString()); break; case AnimVariant::Type::Vec3: - target.setProperty(pair.first, vec3toScriptValue(engine, pair.second.getVec3())); + target.setProperty(name, vec3toScriptValue(engine, value.getVec3())); break; case AnimVariant::Type::Quat: - target.setProperty(pair.first, quatToScriptValue(engine, pair.second.getQuat())); + target.setProperty(name, quatToScriptValue(engine, value.getQuat())); break; default: // Note that we don't do mat4 in Javascript currently, and there's not yet a reason to start now. assert("AnimVariant::Type" == "valid"); } + }; + if (useNames) { // copy only the requested names + for (const QString& name : names) { + auto search = _map.find(name); + if (search != _map.end()) { // scripts are allowed to request names that do not exist + setOne(name, search->second); + } + } + + } else { // copy all of them + for (auto& pair : _map) { + setOne(pair.first, pair.second); + } } return target; } diff --git a/libraries/animation/src/Rig.cpp b/libraries/animation/src/Rig.cpp index 0576a8e420..7810bb8693 100644 --- a/libraries/animation/src/Rig.cpp +++ b/libraries/animation/src/Rig.cpp @@ -606,36 +606,51 @@ void Rig::computeMotionAnimationState(float deltaTime, const glm::vec3& worldPos } // Allow script to add/remove handlers and report results, from within their thread. -// TODO: iterate multiple handlers, but with one shared arg. -// TODO: fill the properties based on the union of requested properties. (Keep all properties objs and compute new union when add/remove handler.) QScriptValue Rig::addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList) { // called in script thread - _stateHandlers = handler; - return handler; // suitable for giving to removeAnimationStateHandler -} -void Rig::removeAnimationStateHandler(QScriptValue handler) { // called in script thread - _stateHandlers = QScriptValue(); - QMutexLocker locker(&_stateMutex); // guarding access to results - _stateHandlersResults.clearMap(); // TODO: When we have multiple handlers, we'll need to clear only his handler's results. -} -void Rig::animationStateHandlerResult(QScriptValue handler, QScriptValue result) { // called synchronously from script - // handler is currently ignored but might be used in storing individual results QMutexLocker locker(&_stateMutex); - if (!_stateHandlers.isValid()) { + int identifier = ++_nextStateHandlerId; // 0 is unused + StateHandler& data = _stateHandlers[identifier]; + data.function = handler; + data.useNames = propertiesList.isArray(); + if (data.useNames) { + data.propertyNames = propertiesList.toVariant().toStringList(); + } + return QScriptValue(identifier); // suitable for giving to removeAnimationStateHandler +} +void Rig::removeAnimationStateHandler(QScriptValue identifier) { // called in script thread + QMutexLocker locker(&_stateMutex); + _stateHandlers.remove(identifier.isNumber() ? identifier.toInt32() : 0); // silently continues if handler not present +} +void Rig::animationStateHandlerResult(int identifier, QScriptValue result) { // called synchronously from script + QMutexLocker locker(&_stateMutex); + auto found = _stateHandlers.find(identifier); + if (found == _stateHandlers.end()) { return; // Don't use late-breaking results that got reported after the handler was removed. } - _stateHandlersResults.animVariantMapFromScriptValue(result); // Into our own copy. + found.value().results.animVariantMapFromScriptValue(result); // Into our own copy. } void Rig::updateAnimationStateHandlers() { // called on avatar update thread (which may be main thread) - if (_stateHandlers.isValid()) { - auto handleResult = [this](QScriptValue handler, QScriptValue result) { - animationStateHandlerResult(handler, result); + QMutexLocker locker(&_stateMutex); + // It might pay to produce just one AnimVariantMap copy here, with a union of all the requested propertyNames, + // rather than having each callAnimationStateHandler invocation make its own copy. + // However, that copying is done on the script's own time rather than ours, so even if it's less cpu, it would be more + // work on the avatar update thread (which is possibly the main thread). + for (auto data = _stateHandlers.begin(); data != _stateHandlers.end(); data++) { + // call out: + int identifier = data.key(); + StateHandler& value = data.value(); + QScriptValue& function = value.function; + auto handleResult = [this, identifier](QScriptValue result) { + animationStateHandlerResult(identifier, result); }; // invokeMethod makes a copy of the args, and copies of AnimVariantMap do copy the underlying map, so this will correctly capture // the state of _animVars and allow continued changes to _animVars in this thread without conflict. - QMetaObject::invokeMethod(_stateHandlers.engine(), "callAnimationStateHandler", Qt::QueuedConnection, - Q_ARG(QScriptValue, _stateHandlers), + QMetaObject::invokeMethod(function.engine(), "callAnimationStateHandler", Qt::QueuedConnection, + Q_ARG(QScriptValue, function), Q_ARG(AnimVariantMap, _animVars), + Q_ARG(QStringList, value.propertyNames), + Q_ARG(bool, value.useNames), Q_ARG(AnimVariantResultHandler, handleResult)); // It turns out that, for thread-safety reasons, ScriptEngine::callAnimationStateHandler will invoke itself if called from other // than the script thread. Thus the above _could_ be replaced with an ordinary call, which will then trigger the same @@ -643,12 +658,13 @@ void Rig::updateAnimationStateHandlers() { // called on avatar update thread (wh // We could create an AnimVariantCallingMixin class in shared, with an abstract virtual slot // AnimVariantCallingMixin::callAnimationStateHandler (and move AnimVariantMap/AnimVaraintResultHandler to shared), but the // call site here would look like this instead of the above: - // dynamic_cast(_stateHandlers.engine())->callAnimationStateHandler(_stateHandlers, _animVars, handleResult); + // dynamic_cast(function.engine())->callAnimationStateHandler(function, ..., handleResult); // This works (I tried it), but the result would be that we would still have same runtime type checks as the invokeMethod above // (occuring within the ScriptEngine::callAnimationStateHandler invokeMethod trampoline), _plus_ another runtime check for the dynamic_cast. + + // gather results in (likely from an earlier update): + _animVars.copyVariantsFrom(value.results); // If multiple handlers write the same anim var, the last registgered wins. (_map preserves order). } - QMutexLocker locker(&_stateMutex); // as we examine/copy most recently computed state, if any. (Typically an earlier invocation.) - _animVars.copyVariantsFrom(_stateHandlersResults); } void Rig::updateAnimations(float deltaTime, glm::mat4 rootTransform) { diff --git a/libraries/animation/src/Rig.h b/libraries/animation/src/Rig.h index 83f1a02b8a..5551b21e40 100644 --- a/libraries/animation/src/Rig.h +++ b/libraries/animation/src/Rig.h @@ -53,6 +53,12 @@ typedef std::shared_ptr RigPointer; class Rig : public QObject, public std::enable_shared_from_this { public: + struct StateHandler { + AnimVariantMap results; + QStringList propertyNames; + QScriptValue function; + bool useNames; + }; struct HeadParameters { float leanSideways = 0.0f; // degrees @@ -203,7 +209,7 @@ public: bool disableHands {false}; // should go away with rig animation (and Rig::inverseKinematics) QScriptValue addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList); void removeAnimationStateHandler(QScriptValue handler); - void animationStateHandlerResult(QScriptValue handler, QScriptValue result); + void animationStateHandlerResult(int identifier, QScriptValue result); bool getModelOffset(glm::vec3& modelOffsetOut) const; @@ -249,8 +255,8 @@ public: float _rightHandOverlayAlpha = 0.0f; private: - QScriptValue _stateHandlers; - AnimVariantMap _stateHandlersResults; + QMap _stateHandlers; + int _nextStateHandlerId {0}; QMutex _stateMutex; }; diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 86eb9570d8..91df9fa793 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -262,7 +262,8 @@ void ScriptEngine::errorInLoadingScript(const QUrl& url) { // callAnimationStateHandler requires that the type be registered. // These two are meaningful, if we ever do want to use them... static QScriptValue animVarMapToScriptValue(QScriptEngine* engine, const AnimVariantMap& parameters) { - return parameters.animVariantMapToScriptValue(engine); + QStringList unused; + return parameters.animVariantMapToScriptValue(engine, unused, false); } static void animVarMapFromScriptValue(const QScriptValue& value, AnimVariantMap& parameters) { parameters.animVariantMapFromScriptValue(value); @@ -741,7 +742,7 @@ void ScriptEngine::stop() { } // Other threads can invoke this through invokeMethod, which causes the callback to be asynchronously executed in this script's thread. -void ScriptEngine::callAnimationStateHandler(QScriptValue callback, AnimVariantMap parameters, AnimVariantResultHandler resultHandler) { +void ScriptEngine::callAnimationStateHandler(QScriptValue callback, AnimVariantMap parameters, QStringList names, bool useNames, AnimVariantResultHandler resultHandler) { if (QThread::currentThread() != thread()) { #ifdef THREAD_DEBUGGING qDebug() << "*** WARNING *** ScriptEngine::callAnimationStateHandler() called on wrong thread [" << QThread::currentThread() << "], invoking on correct thread [" << thread() << "] name:" << name; @@ -749,14 +750,16 @@ void ScriptEngine::callAnimationStateHandler(QScriptValue callback, AnimVariantM QMetaObject::invokeMethod(this, "callAnimationStateHandler", Q_ARG(QScriptValue, callback), Q_ARG(AnimVariantMap, parameters), + Q_ARG(QStringList, names), + Q_ARG(bool, useNames), Q_ARG(AnimVariantResultHandler, resultHandler)); return; } - QScriptValue javascriptParametgers = parameters.animVariantMapToScriptValue(this); + QScriptValue javascriptParametgers = parameters.animVariantMapToScriptValue(this, names, useNames); QScriptValueList callingArguments; callingArguments << javascriptParametgers; QScriptValue result = callback.call(QScriptValue(), callingArguments); - resultHandler(callback, result); + resultHandler(result); } void ScriptEngine::timerFired() { diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index b6f736c846..30468880a2 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -144,7 +144,7 @@ public: ArrayBufferClass* getArrayBufferClass() { return _arrayBufferClass; } public slots: - void callAnimationStateHandler(QScriptValue callback, AnimVariantMap parameters, AnimVariantResultHandler resultHandler); + void callAnimationStateHandler(QScriptValue callback, AnimVariantMap parameters, QStringList names, bool useNames, AnimVariantResultHandler resultHandler); signals: void scriptLoaded(const QString& scriptFilename);