diff --git a/assignment-client/src/avatars/ScriptableAvatar.cpp b/assignment-client/src/avatars/ScriptableAvatar.cpp
index 1d0114d703..07a9ab1dbc 100644
--- a/assignment-client/src/avatars/ScriptableAvatar.cpp
+++ b/assignment-client/src/avatars/ScriptableAvatar.cpp
@@ -32,24 +32,6 @@
ScriptableAvatar::ScriptableAvatar() {
_clientTraitsHandler.reset(new ClientTraitsHandler(this));
- std::lock_guard<std::mutex> lock(_scriptEngineLock);
- _scriptEngine = newScriptEngine();
- if (!_scriptEngineThread) {
- _scriptEngineThread.reset(new QThread());
- _scriptEngine->setThread(_scriptEngineThread.get());
- _scriptEngineThread->start();
- }
-}
-
-ScriptableAvatar::~ScriptableAvatar() {
- std::lock_guard<std::mutex> lock(_scriptEngineLock);
- if (_scriptEngine) {
- if (_scriptEngineThread) {
- _scriptEngineThread->quit();
- _scriptEngineThread->wait();
- }
- _scriptEngine.reset();
- }
}
QByteArray ScriptableAvatar::toByteArrayStateful(AvatarDataDetail dataDetail, bool dropFaceTracking) {
@@ -333,10 +315,9 @@ AvatarEntityMap ScriptableAvatar::getAvatarEntityDataInternal(bool allProperties
EntityItemProperties properties = entity->getProperties(desiredProperties);
QByteArray blob;
- {
- std::lock_guard<std::mutex> lock(_scriptEngineLock);
- EntityItemProperties::propertiesToBlob(*_scriptEngine, sessionID, properties, blob, allProperties);
- }
+ _helperScriptEngine.run( [&] {
+ EntityItemProperties::propertiesToBlob(*_helperScriptEngine.get(), sessionID, properties, blob, allProperties);
+ });
data[id] = blob;
}
});
@@ -361,10 +342,11 @@ void ScriptableAvatar::setAvatarEntityData(const AvatarEntityMap& avatarEntityDa
EntityItemProperties properties;
const QByteArray& blob = dataItr.value();
{
- std::lock_guard<std::mutex> lock(_scriptEngineLock);
- if (!blob.isNull() && EntityItemProperties::blobToProperties(*_scriptEngine, blob, properties)) {
- newProperties[dataItr.key()] = properties;
- }
+ _helperScriptEngine.run( [&] {
+ if (!blob.isNull() && EntityItemProperties::blobToProperties(*_helperScriptEngine.get(), blob, properties)) {
+ newProperties[dataItr.key()] = properties;
+ }
+ });
}
++dataItr;
}
@@ -444,8 +426,12 @@ void ScriptableAvatar::updateAvatarEntity(const QUuid& entityID, const QByteArra
EntityItemPointer entity;
EntityItemProperties properties;
{
- std::lock_guard<std::mutex> lock(_scriptEngineLock);
- if (!EntityItemProperties::blobToProperties(*_scriptEngine, entityData, properties)) {
+ // TODO: checking how often this happens and what is the performance impact of having the script engine on separate thread
+ // If it's happening often, a method to move HelperScriptEngine into the current thread would be a good idea
+ bool result = _helperScriptEngine.runWithResult<bool> ( [&]() {
+ return EntityItemProperties::blobToProperties(*_helperScriptEngine.get(), entityData, properties);
+ });
+ if (!result) {
// entityData is corrupt
return;
}
diff --git a/assignment-client/src/avatars/ScriptableAvatar.h b/assignment-client/src/avatars/ScriptableAvatar.h
index 56f9d2169d..ec4f15d3dc 100644
--- a/assignment-client/src/avatars/ScriptableAvatar.h
+++ b/assignment-client/src/avatars/ScriptableAvatar.h
@@ -19,6 +19,7 @@
#include <AvatarData.h>
#include <ScriptEngine.h>
#include <EntityItem.h>
+#include <HelperScriptEngine.h>
/*@jsdoc
* The <code>Avatar</code> API is used to manipulate scriptable avatars on the domain. This API is a subset of the
@@ -111,7 +112,6 @@ class ScriptableAvatar : public AvatarData, public Dependency {
public:
ScriptableAvatar();
- ~ScriptableAvatar();
/*@jsdoc
* Starts playing an animation on the avatar.
@@ -224,9 +224,7 @@ private:
QStringList _fstJointNames; ///< in order of depth-first traversal
QUrl _skeletonFBXURL;
- mutable std::mutex _scriptEngineLock;
- mutable ScriptEnginePointer _scriptEngine { nullptr };
- std::shared_ptr<QThread> _scriptEngineThread { nullptr };
+ mutable HelperScriptEngine _helperScriptEngine;
std::map<QUuid, EntityItemPointer> _entities;
diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp
index de2ba27a77..bb6afb9280 100644
--- a/interface/src/avatar/MyAvatar.cpp
+++ b/interface/src/avatar/MyAvatar.cpp
@@ -406,14 +406,6 @@ MyAvatar::~MyAvatar() {
if (_addAvatarEntitiesToTreeTimer.isActive()) {
_addAvatarEntitiesToTreeTimer.stop();
}
- std::lock_guard<std::mutex> lock(_scriptEngineLock);
- if (_scriptEngine) {
- if (_scriptEngineThread) {
- _scriptEngineThread->quit();
- _scriptEngineThread->wait();
- }
- _scriptEngine.reset();
- }
}
QString MyAvatar::getDominantHand() const {
@@ -1747,10 +1739,11 @@ void MyAvatar::handleChangedAvatarEntityData() {
blobFailed = true; // blob doesn't exist
return;
}
- std::lock_guard<std::mutex> guard(_scriptEngineLock);
- if (!EntityItemProperties::blobToProperties(*_scriptEngine, itr.value(), properties)) {
- blobFailed = true; // blob is corrupt
- }
+ _helperScriptEngine.run( [&] {
+ if (!EntityItemProperties::blobToProperties(*_helperScriptEngine.get(), itr.value(), properties)) {
+ blobFailed = true; // blob is corrupt
+ }
+ });
});
if (blobFailed) {
// remove from _cachedAvatarEntityBlobUpdatesToSkip just in case:
@@ -1783,10 +1776,11 @@ void MyAvatar::handleChangedAvatarEntityData() {
skip = true;
return;
}
- std::lock_guard<std::mutex> guard(_scriptEngineLock);
- if (!EntityItemProperties::blobToProperties(*_scriptEngine, itr.value(), properties)) {
- skip = true;
- }
+ _helperScriptEngine.run( [&] {
+ if (!EntityItemProperties::blobToProperties(*_helperScriptEngine.get(), itr.value(), properties)) {
+ skip = true;
+ }
+ });
});
if (!skip && canRezAvatarEntites) {
sanitizeAvatarEntityProperties(properties);
@@ -1891,10 +1885,9 @@ bool MyAvatar::updateStaleAvatarEntityBlobs() const {
if (found) {
++numFound;
QByteArray blob;
- {
- std::lock_guard<std::mutex> guard(_scriptEngineLock);
- EntityItemProperties::propertiesToBlob(*_scriptEngine, getID(), properties, blob);
- }
+ _helperScriptEngine.run( [&] {
+ EntityItemProperties::propertiesToBlob(*_helperScriptEngine.get(), getID(), properties, blob);
+ });
_avatarEntitiesLock.withWriteLock([&] {
_cachedAvatarEntityBlobs[id] = blob;
});
@@ -1955,10 +1948,9 @@ AvatarEntityMap MyAvatar::getAvatarEntityData() const {
EntityItemProperties properties = entity->getProperties(desiredProperties);
QByteArray blob;
- {
- std::lock_guard<std::mutex> guard(_scriptEngineLock);
- EntityItemProperties::propertiesToBlob(*_scriptEngine, getID(), properties, blob, true);
- }
+ _helperScriptEngine.run( [&] {
+ EntityItemProperties::propertiesToBlob(*_helperScriptEngine.get(), getID(), properties, blob, true);
+ });
data[entityID] = blob;
}
@@ -2100,17 +2092,6 @@ void MyAvatar::avatarEntityDataToJson(QJsonObject& root) const {
}
void MyAvatar::loadData() {
- {
- std::lock_guard<std::mutex> lock(_scriptEngineLock);
- if (!_scriptEngine) {
- _scriptEngine = newScriptEngine();
- if (!_scriptEngineThread) {
- _scriptEngineThread.reset(new QThread());
- _scriptEngine->setThread(_scriptEngineThread.get());
- _scriptEngineThread->start();
- }
- }
- }
getHead()->setBasePitch(_headPitchSetting.get());
_yawSpeed = _yawSpeedSetting.get(_yawSpeed);
@@ -2716,11 +2697,10 @@ QVariantList MyAvatar::getAvatarEntitiesVariant() {
QVariantMap avatarEntityData;
avatarEntityData["id"] = entityID;
EntityItemProperties entityProperties = entity->getProperties(desiredProperties);
- {
- std::lock_guard<std::mutex> guard(_scriptEngineLock);
- ScriptValue scriptProperties = EntityItemPropertiesToScriptValue(_scriptEngine.get(), entityProperties);
+ _helperScriptEngine.run( [&] {
+ ScriptValue scriptProperties = EntityItemPropertiesToScriptValue(_helperScriptEngine.get(), entityProperties);
avatarEntityData["properties"] = scriptProperties.toVariant();
- }
+ });
avatarEntitiesData.append(QVariant(avatarEntityData));
}
}
diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h
index f3a04284c3..30ef8784b3 100644
--- a/interface/src/avatar/MyAvatar.h
+++ b/interface/src/avatar/MyAvatar.h
@@ -28,6 +28,7 @@
#include <controllers/Pose.h>
#include <controllers/Actions.h>
#include <EntityItem.h>
+#include <HelperScriptEngine.h>
#include <ThreadSafeValueCache.h>
#include <Rig.h>
#include <SettingHandle.h>
@@ -3103,9 +3104,7 @@ private:
// keep a ScriptEngine around so we don't have to instantiate on the fly (these are very slow to create/delete)
// TODO: profile if it performs better when script engine is on avatar thread or on its own thread
// Own thread is safer from deadlocks
- mutable std::mutex _scriptEngineLock;
- ScriptEnginePointer _scriptEngine { nullptr };
- std::shared_ptr<QThread> _scriptEngineThread { nullptr };
+ mutable HelperScriptEngine _helperScriptEngine;
bool _needToSaveAvatarEntitySettings { false };
diff --git a/libraries/baking/src/MaterialBaker.cpp b/libraries/baking/src/MaterialBaker.cpp
index d28fc9aacf..2a3f2b2021 100644
--- a/libraries/baking/src/MaterialBaker.cpp
+++ b/libraries/baking/src/MaterialBaker.cpp
@@ -38,22 +38,6 @@ MaterialBaker::MaterialBaker(const QString& materialData, bool isURL, const QStr
_bakedOutputDir(bakedOutputDir),
_textureOutputDir(bakedOutputDir + "/materialTextures/" + QString::number(materialNum++))
{
- std::lock_guard<std::mutex> lock(_scriptEngineLock);
- _scriptEngine = newScriptEngine();
- _scriptEngineThread.reset(new QThread());
- _scriptEngine->setThread(_scriptEngineThread.get());
- _scriptEngineThread->start();
-}
-
-MaterialBaker::~MaterialBaker() {
- std::lock_guard<std::mutex> lock(_scriptEngineLock);
- if (_scriptEngine) {
- if (_scriptEngineThread) {
- _scriptEngineThread->quit();
- _scriptEngineThread->wait();
- }
- _scriptEngine.reset();
- }
}
void MaterialBaker::bake() {
@@ -229,18 +213,20 @@ void MaterialBaker::outputMaterial() {
if (_materialResource->parsedMaterials.networkMaterials.size() == 1) {
auto networkMaterial = _materialResource->parsedMaterials.networkMaterials.begin();
auto scriptableMaterial = scriptable::ScriptableMaterial(networkMaterial->second);
- std::lock_guard<std::mutex> lock(_scriptEngineLock);
- QVariant materialVariant =
- scriptable::scriptableMaterialToScriptValue(_scriptEngine.get(), scriptableMaterial).toVariant();
- json.insert("materials", QJsonDocument::fromVariant(materialVariant).object());
+ _helperScriptEngine.run( [&] {
+ QVariant materialVariant =
+ scriptable::scriptableMaterialToScriptValue(_helperScriptEngine.get(), scriptableMaterial).toVariant();
+ json.insert("materials", QJsonDocument::fromVariant(materialVariant).object());
+ });
} else {
QJsonArray materialArray;
for (auto networkMaterial : _materialResource->parsedMaterials.networkMaterials) {
auto scriptableMaterial = scriptable::ScriptableMaterial(networkMaterial.second);
- std::lock_guard<std::mutex> lock(_scriptEngineLock);
- QVariant materialVariant =
- scriptable::scriptableMaterialToScriptValue(_scriptEngine.get(), scriptableMaterial).toVariant();
- materialArray.append(QJsonDocument::fromVariant(materialVariant).object());
+ _helperScriptEngine.run( [&] {
+ QVariant materialVariant =
+ scriptable::scriptableMaterialToScriptValue(_helperScriptEngine.get(), scriptableMaterial).toVariant();
+ materialArray.append(QJsonDocument::fromVariant(materialVariant).object());
+ });
}
json.insert("materials", materialArray);
}
diff --git a/libraries/baking/src/MaterialBaker.h b/libraries/baking/src/MaterialBaker.h
index 986dead3ed..4fd8948b03 100644
--- a/libraries/baking/src/MaterialBaker.h
+++ b/libraries/baking/src/MaterialBaker.h
@@ -21,6 +21,7 @@
#include "TextureBaker.h"
#include "baking/TextureFileNamer.h"
+#include <HelperScriptEngine.h>
#include <procedural/ProceduralMaterialCache.h>
#include <ScriptEngine.h>
@@ -32,7 +33,6 @@ class MaterialBaker : public Baker {
Q_OBJECT
public:
MaterialBaker(const QString& materialData, bool isURL, const QString& bakedOutputDir, QUrl destinationPath = QUrl());
- virtual ~MaterialBaker();
QString getMaterialData() const { return _materialData; }
bool isURL() const { return _isURL; }
@@ -73,9 +73,7 @@ private:
QString _textureOutputDir;
QString _bakedMaterialData;
- mutable std::mutex _scriptEngineLock;
- ScriptEnginePointer _scriptEngine { nullptr };
- std::shared_ptr<QThread> _scriptEngineThread { nullptr };
+ HelperScriptEngine _helperScriptEngine;
static std::function<QThread*()> _getNextOvenWorkerThreadOperator;
TextureFileNamer _textureFileNamer;
diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp
index 61535b913e..a28cf0b790 100644
--- a/libraries/entities/src/EntityTree.cpp
+++ b/libraries/entities/src/EntityTree.cpp
@@ -50,12 +50,6 @@ static const QString DOMAIN_UNLIMITED = "domainUnlimited";
EntityTree::EntityTree(bool shouldReaverage) :
Octree(shouldReaverage)
{
- std::lock_guard<std::mutex> lock(_scriptEngineLock);
- _scriptEngine = newScriptEngine();
- _scriptEngineThread.reset(new QThread());
- _scriptEngine->setThread(_scriptEngineThread.get());
- _scriptEngineThread->start();
-
resetClientEditStats();
}
@@ -69,14 +63,6 @@ EntityTree::~EntityTree() {
// TODO: EntityTreeElement::_tree should be raw back pointer.
// AND: EntityItem::_element should be a raw back pointer.
//eraseAllOctreeElements(false); // KEEP THIS
- std::lock_guard<std::mutex> lock(_scriptEngineLock);
- if (_scriptEngine) {
- if (_scriptEngineThread) {
- _scriptEngineThread->quit();
- _scriptEngineThread->wait();
- }
- _scriptEngine.reset();
- }
}
void EntityTree::setEntityScriptSourceWhitelist(const QString& entityScriptSourceWhitelist) {
@@ -2567,11 +2553,10 @@ bool EntityTree::writeToMap(QVariantMap& entityDescription, OctreeElementPointer
}
entityDescription["DataVersion"] = _persistDataVersion;
entityDescription["Id"] = _persistID;
- const std::lock_guard<std::mutex> scriptLock(_scriptEngineLock);
- RecurseOctreeToMapOperator theOperator(entityDescription, element, _scriptEngine.get(), skipDefaultValues,
- skipThoseWithBadParents, _myAvatar);
- withReadLock([&] {
- recurseTreeWithOperator(&theOperator);
+ _helperScriptEngine.run( [&] {
+ RecurseOctreeToMapOperator theOperator(entityDescription, element, _helperScriptEngine.get(), skipDefaultValues,
+ skipThoseWithBadParents, _myAvatar);
+ withReadLock([&] { recurseTreeWithOperator(&theOperator); });
});
return true;
}
@@ -2742,11 +2727,10 @@ bool EntityTree::readFromMap(QVariantMap& map, const bool isImport) {
}
EntityItemProperties properties;
- {
- const std::lock_guard<std::mutex> scriptLock(_scriptEngineLock);
- ScriptValue entityScriptValue = variantMapToScriptValue(entityMap, *_scriptEngine);
+ _helperScriptEngine.run( [&] {
+ ScriptValue entityScriptValue = variantMapToScriptValue(entityMap, *_helperScriptEngine.get());
EntityItemPropertiesFromScriptValueIgnoreReadOnly(entityScriptValue, properties);
- }
+ });
EntityItemID entityItemID;
if (entityMap.contains("id")) {
@@ -2895,13 +2879,12 @@ bool EntityTree::readFromMap(QVariantMap& map, const bool isImport) {
}
bool EntityTree::writeToJSON(QString& jsonString, const OctreeElementPointer& element) {
- const std::lock_guard<std::mutex> scriptLock(_scriptEngineLock);
- RecurseOctreeToJSONOperator theOperator(element, _scriptEngine.get(), jsonString);
- withReadLock([&] {
- recurseTreeWithOperator(&theOperator);
- });
+ _helperScriptEngine.run( [&] {
+ RecurseOctreeToJSONOperator theOperator(element, _helperScriptEngine.get(), jsonString);
+ withReadLock([&] { recurseTreeWithOperator(&theOperator); });
- jsonString = theOperator.getJson();
+ jsonString = theOperator.getJson();
+ });
return true;
}
diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h
index 9d348e87a7..c93a3b1527 100644
--- a/libraries/entities/src/EntityTree.h
+++ b/libraries/entities/src/EntityTree.h
@@ -15,6 +15,7 @@
#include <QSet>
#include <QVector>
+#include <HelperScriptEngine.h>
#include <Octree.h>
#include <SpatialParentFinder.h>
@@ -387,9 +388,7 @@ private:
MovingEntitiesOperator& moveOperator, bool force, bool tellServer);
// Script engine for writing entity tree data to and from JSON
- std::mutex _scriptEngineLock;
- ScriptEnginePointer _scriptEngine{ nullptr };
- std::shared_ptr<QThread> _scriptEngineThread { nullptr };
+ HelperScriptEngine _helperScriptEngine;
};
void convertGrabUserDataToProperties(EntityItemProperties& properties);
diff --git a/libraries/script-engine/src/HelperScriptEngine.cpp b/libraries/script-engine/src/HelperScriptEngine.cpp
new file mode 100644
index 0000000000..313fac74da
--- /dev/null
+++ b/libraries/script-engine/src/HelperScriptEngine.cpp
@@ -0,0 +1,31 @@
+//
+// HelperScriptEngine.h
+// libraries/script-engine/src/HelperScriptEngine.h
+//
+// Created by dr Karol Suprynowicz on 2024/04/28.
+// Copyright 2024 Overte e.V.
+//
+// Distributed under the Apache License, Version 2.0.
+// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+#include "HelperScriptEngine.h"
+
+HelperScriptEngine::HelperScriptEngine() {
+ std::lock_guard<std::mutex> lock(_scriptEngineLock);
+ _scriptEngine = newScriptEngine();
+ _scriptEngineThread.reset(new QThread());
+ _scriptEngine->setThread(_scriptEngineThread.get());
+ _scriptEngineThread->start();
+}
+
+HelperScriptEngine::~HelperScriptEngine() {
+ std::lock_guard<std::mutex> lock(_scriptEngineLock);
+ if (_scriptEngine) {
+ if (_scriptEngineThread) {
+ _scriptEngineThread->quit();
+ _scriptEngineThread->wait();
+ }
+ _scriptEngine.reset();
+ }
+}
diff --git a/libraries/script-engine/src/HelperScriptEngine.h b/libraries/script-engine/src/HelperScriptEngine.h
new file mode 100644
index 0000000000..53d2e89306
--- /dev/null
+++ b/libraries/script-engine/src/HelperScriptEngine.h
@@ -0,0 +1,65 @@
+//
+// HelperScriptEngine.h
+// libraries/script-engine/src/HelperScriptEngine.h
+//
+// Created by dr Karol Suprynowicz on 2024/04/28.
+// Copyright 2024 Overte e.V.
+//
+// Distributed under the Apache License, Version 2.0.
+// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+#ifndef overte_HelperScriptEngine_h
+#define overte_HelperScriptEngine_h
+
+#include <mutex>
+#include "QThread"
+
+#include "ScriptEngine.h"
+
+/**
+ * @brief Provides a wrapper around script engine that does not have ScriptManager
+ *
+ * HelperScriptEngine is used for performing smaller tasks, like for example conversions between entity
+ * properties and JSON data.
+ * For thread safety all accesses to helper script engine need to be done either through HelperScriptEngine::run()
+ * or HelperScriptEngine::runWithResult().
+ *
+ */
+
+
+class HelperScriptEngine {
+public:
+ HelperScriptEngine();
+ ~HelperScriptEngine();
+
+ template <typename F>
+ inline void run(F&& f) {
+ std::lock_guard<std::mutex> guard(_scriptEngineLock);
+ f();
+ }
+
+ template <typename T, typename F>
+ inline T runWithResult(F&& f) {
+ T result;
+ {
+ std::lock_guard<std::mutex> guard(_scriptEngineLock);
+ result = f();
+ }
+ return result;
+ }
+
+ /**
+ * @brief Returns pointer to the script engine
+ *
+ * This function should be used only inside HelperScriptEngine::run() or HelperScriptEngine::runWithResult()
+ */
+ ScriptEngine* get() { return _scriptEngine.get(); };
+ ScriptEnginePointer getShared() { return _scriptEngine; };
+private:
+ std::mutex _scriptEngineLock;
+ ScriptEnginePointer _scriptEngine { nullptr };
+ std::shared_ptr<QThread> _scriptEngineThread { nullptr };
+};
+
+#endif //overte_HelperScriptEngine_h