fix: improve thread safety (#102)

package/cpp/core/EngineWrapper.cpp

@@ -122,7 +122,7 @@ void EngineWrapper::loadHybridMethods() {
   registerHybridMethod("setEntityScale", &EngineWrapper::setEntityScale, this);
   registerHybridMethod("setIsPaused", &EngineWrapper::setIsPaused, this);
   registerHybridMethod("getTransformManager", &EngineWrapper::getTransformManager, this);
-  registerHybridMethod("getRenderableManager", &EngineWrapper::getRendererableManager, this);
+  registerHybridMethod("getRenderableManager", &EngineWrapper::createRenderableManager, this);
   registerHybridMethod("createMaterial", &EngineWrapper::createMaterial, this);
 
   // Combined Physics API:
@@ -348,6 +348,7 @@ std::shared_ptr<CameraWrapper> EngineWrapper::createCamera() {
 }
 
 std::shared_ptr<FilamentAssetWrapper> EngineWrapper::loadAsset(const std::shared_ptr<FilamentBuffer>& modelBuffer) {
+  std::unique_lock lock(_mutex);
   const std::shared_ptr<ManagedBuffer>& buffer = modelBuffer->getBuffer();
   gltfio::FilamentAsset* assetPtr = _assetLoader->createAsset(buffer->getData(), buffer->getSize());
 
@@ -356,6 +357,7 @@ std::shared_ptr<FilamentAssetWrapper> EngineWrapper::loadAsset(const std::shared
 
 std::shared_ptr<FilamentAssetWrapper> EngineWrapper::loadInstancedAsset(const std::shared_ptr<FilamentBuffer>& modelBuffer,
                                                                         int instanceCount) {
+  std::unique_lock lock(_mutex);
   const std::shared_ptr<ManagedBuffer>& buffer = modelBuffer->getBuffer();
   FilamentInstance* instances[instanceCount]; // Memory managed by the FilamentAsset
   gltfio::FilamentAsset* assetPtr = _assetLoader->createInstancedAsset(buffer->getData(), buffer->getSize(), instances, instanceCount);
@@ -391,6 +393,7 @@ std::shared_ptr<FilamentAssetWrapper> EngineWrapper::makeAssetWrapper(FilamentAs
 
 // Default light is a directional light for shadows + a default IBL
 void EngineWrapper::setIndirectLight(const std::shared_ptr<FilamentBuffer>& iblBuffer) {
+  std::unique_lock lock(_mutex);
   if (!_scene) {
     throw std::runtime_error("Scene not initialized");
   }
@@ -422,6 +425,7 @@ void EngineWrapper::setIndirectLight(const std::shared_ptr<FilamentBuffer>& iblB
 
 std::shared_ptr<EntityWrapper> EngineWrapper::createLightEntity(const std::string& lightTypeStr, double colorFahrenheit, double intensity,
                                                                 double directionX, double directionY, double directionZ, bool castShadows) {
+  std::unique_lock lock(_mutex);
   auto lightEntity = _engine->getEntityManager().create();
 
   // TODO(Marc): Fix enum converter
@@ -479,6 +483,7 @@ void EngineWrapper::synchronizePendingFrames() {
  * @param multiplyCurrent If true, the current transform will be multiplied with the new transform, otherwise it will be replaced
  */
 void EngineWrapper::updateTransform(math::mat4 transform, const std::shared_ptr<EntityWrapper>& entity, bool multiplyCurrent) {
+  std::unique_lock lock(_mutex);
   if (!entity) {
     throw std::invalid_argument("Entity is null");
   }
@@ -516,6 +521,7 @@ void EngineWrapper::setEntityScale(const std::shared_ptr<EntityWrapper>& entity,
 
 void EngineWrapper::updateTransformByRigidBody(const std::shared_ptr<EntityWrapper>& entityWrapper,
                                                const std::shared_ptr<RigidBodyWrapper>& rigidBody) {
+  std::unique_lock lock(_mutex);
   if (!rigidBody) {
     throw std::invalid_argument("RigidBody is null");
   }
@@ -563,7 +569,8 @@ void EngineWrapper::updateTransformByRigidBody(const std::shared_ptr<EntityWrapp
   tm.setTransform(entityInstance, newTransform);
 }
 
-std::shared_ptr<RenderableManagerWrapper> EngineWrapper::getRendererableManager() {
+std::shared_ptr<RenderableManagerWrapper> EngineWrapper::createRenderableManager() {
+  std::unique_lock lock(_mutex);
   RenderableManager& rm = _engine->getRenderableManager();
 
   // Create a new texture provider
@@ -573,6 +580,7 @@ std::shared_ptr<RenderableManagerWrapper> EngineWrapper::getRendererableManager(
 }
 
 std::shared_ptr<MaterialWrapper> EngineWrapper::createMaterial(const std::shared_ptr<FilamentBuffer>& materialBuffer) {
+  std::unique_lock lock(_mutex);
   auto buffer = materialBuffer->getBuffer();
   if (buffer->getSize() == 0) {
     throw std::runtime_error("Material buffer is empty");

package/cpp/core/EngineWrapper.h

@@ -87,14 +87,15 @@ class EngineWrapper : public HybridObject {
   void setEntityScale(const std::shared_ptr<EntityWrapper>& entity, std::vector<double> scaleVec, bool multiplyCurrent);
   void updateTransformByRigidBody(const std::shared_ptr<EntityWrapper>& entityWrapper, const std::shared_ptr<RigidBodyWrapper>& rigidBody);
 
-  std::shared_ptr<RenderableManagerWrapper> getRendererableManager();
+  std::shared_ptr<RenderableManagerWrapper> createRenderableManager();
 
   std::shared_ptr<MaterialWrapper> createMaterial(const std::shared_ptr<FilamentBuffer>& materialBuffer);
 
   // Internal helper method to turn an FilamentAsset ptr into a FilamentAssetWrapper
   std::shared_ptr<FilamentAssetWrapper> makeAssetWrapper(FilamentAsset* assetPtr);
 
 private:
+  std::mutex _mutex;
   std::shared_ptr<Dispatcher> _dispatcher;
   std::shared_ptr<Engine> _engine;
   std::shared_ptr<SurfaceProvider> _surfaceProvider;

package/cpp/core/FilamentAssetWrapper.cpp

@@ -28,6 +28,7 @@ void FilamentAssetWrapper::loadHybridMethods() {
  * Sets up a root transform on the current model to make it fit into a unit cube.
  */
 void FilamentAssetWrapper::transformToUnitCube(TransformManager& transformManager) {
+  std::unique_lock lock(_mutex);
   Aabb aabb = _asset->getBoundingBox();
   math::details::TVec3<float> center = aabb.center();
   math::details::TVec3<float> halfExtent = aabb.extent();
@@ -44,6 +45,7 @@ std::shared_ptr<EntityWrapper> FilamentAssetWrapper::getRoot() {
 }
 
 void FilamentAssetWrapper::releaseSourceData() {
+  std::unique_lock lock(_mutex);
   _asset->releaseSourceData();
 }
 

package/cpp/core/FilamentAssetWrapper.h

@@ -52,6 +52,7 @@ class FilamentAssetWrapper : public HybridObject {
   std::vector<std::shared_ptr<FilamentInstanceWrapper>> getAssetInstances();
 
 private: // Internal state:
+  std::mutex _mutex;
   std::shared_ptr<gltfio::FilamentAsset> _asset;
   std::shared_ptr<SceneWrapper> _scene; // The scene the asset is currently attached to
 };

package/cpp/core/MaterialInstanceWrapper.cpp

@@ -16,12 +16,16 @@ void MaterialInstanceWrapper::loadHybridMethods() {
 }
 
 void MaterialInstanceWrapper::setCullingMode(std::string mode) {
+  std::unique_lock lock(_mutex);
+
   backend::CullingMode cullingMode;
   EnumMapper::convertJSUnionToEnum(mode, &cullingMode);
   _materialInstance->setCullingMode(cullingMode);
 }
 
 void MaterialInstanceWrapper::setTransparencyMode(std::string mode) {
+  std::unique_lock lock(_mutex);
+
   TransparencyMode transparencyMode;
   EnumMapper::convertJSUnionToEnum(mode, &transparencyMode);
   _materialInstance->setTransparencyMode(transparencyMode);
@@ -39,10 +43,14 @@ void MaterialInstanceWrapper::changeAlpha(MaterialInstance* materialInstance, do
 }
 
 void MaterialInstanceWrapper::changeAlpha(double alpha) {
+  std::unique_lock lock(_mutex);
+
   changeAlpha(_materialInstance, alpha);
 }
 
 void MaterialInstanceWrapper::setParameter(std::string name, double value) {
+  std::unique_lock lock(_mutex);
+
   const Material* material = _materialInstance->getMaterial();
 
   if (!material->hasParameter(name.c_str())) {

package/cpp/core/MaterialInstanceWrapper.h

@@ -36,6 +36,7 @@ class MaterialInstanceWrapper : public HybridObject {
   }
 
 private:
+  std::mutex _mutex;
   MaterialInstance* _materialInstance;
 };
 

package/cpp/core/MaterialWrapper.cpp

@@ -9,6 +9,8 @@ void MaterialWrapper::loadHybridMethods() {
 }
 
 std::shared_ptr<MaterialInstanceWrapper> MaterialWrapper::createInstance() {
+  std::unique_lock lock(_mutex);
+
   MaterialInstance* materialInstance = _material->createInstance();
   // TODO: Who managed the memory of the material instance and cleans it up?
   auto instance = std::make_shared<MaterialInstanceWrapper>(materialInstance);
@@ -17,13 +19,17 @@ std::shared_ptr<MaterialInstanceWrapper> MaterialWrapper::createInstance() {
 }
 
 std::shared_ptr<MaterialInstanceWrapper> MaterialWrapper::getDefaultInstance() {
+  std::unique_lock lock(_mutex);
+
   MaterialInstance* materialInstance = _material->getDefaultInstance();
   auto instance = std::make_shared<MaterialInstanceWrapper>(materialInstance);
   _instances.push_back(instance);
   return instance;
 }
 
 void MaterialWrapper::setDefaultParameter(std::string name, double value) {
+  std::unique_lock lock(_mutex);
+
   if (!_material->hasParameter(name.c_str())) {
     throw std::runtime_error("MaterialWrapper::setDefaultParameter: Material does not have parameter \"" + name + "\"!");
   }

package/cpp/core/MaterialWrapper.h

@@ -28,6 +28,7 @@ class MaterialWrapper : public HybridObject {
   }
 
 private:
+  std::mutex _mutex;
   std::shared_ptr<MaterialInstanceWrapper> createInstance();
   std::shared_ptr<MaterialInstanceWrapper> getDefaultInstance();
   void setDefaultParameter(std::string name, double value);

package/cpp/core/SceneWrapper.cpp

@@ -13,6 +13,8 @@ void margelo::SceneWrapper::loadHybridMethods() {
 }
 
 void margelo::SceneWrapper::addEntity(const std::shared_ptr<EntityWrapper>& entity) {
+  std::unique_lock lock(_mutex);
+
   if (!entity) {
     throw std::invalid_argument("Entity is null");
   }
@@ -21,6 +23,8 @@ void margelo::SceneWrapper::addEntity(const std::shared_ptr<EntityWrapper>& enti
 }
 
 void SceneWrapper::removeEntity(const std::shared_ptr<EntityWrapper>& entity) {
+  std::unique_lock lock(_mutex);
+
   if (!entity) {
     throw std::invalid_argument("Entity is null");
   }
@@ -29,6 +33,8 @@ void SceneWrapper::removeEntity(const std::shared_ptr<EntityWrapper>& entity) {
 }
 
 void SceneWrapper::addAsset(const std::shared_ptr<gltfio::FilamentAsset>& asset) {
+  std::unique_lock lock(_mutex);
+
   if (asset == nullptr) {
     Logger::log("SceneWrapper", "Can't add asset an asset from scene as it was null.");
     return;
@@ -38,6 +44,8 @@ void SceneWrapper::addAsset(const std::shared_ptr<gltfio::FilamentAsset>& asset)
 }
 
 void SceneWrapper::removeAsset(const std::shared_ptr<gltfio::FilamentAsset>& asset) {
+  std::unique_lock lock(_mutex);
+
   if (asset == nullptr) {
     Logger::log("SceneWrapper", "Can't remove asset an asset from scene as it was null.");
     return;

package/cpp/core/SceneWrapper.h

@@ -27,6 +27,7 @@ class SceneWrapper : public HybridObject {
   void removeAsset(const std::shared_ptr<gltfio::FilamentAsset>& asset);
 
 private:
+  std::mutex _mutex;
   std::shared_ptr<Scene> _scene;
   std::shared_ptr<gltfio::AssetLoader> _assetLoader;
 

package/cpp/core/ViewWrapper.cpp

@@ -40,6 +40,7 @@ const std::shared_ptr<CameraWrapper>& ViewWrapper::getCamera() {
 }
 
 void ViewWrapper::setViewport(int x, int y, int width, int height) {
+  std::unique_lock lock(_mutex);
   if (width < 0 || height < 0) {
     throw std::invalid_argument("Invalid viewport size");
   }

package/cpp/core/ViewWrapper.h

@@ -30,6 +30,7 @@ class ViewWrapper : public HybridObject {
   double getAspectRatio();
 
 private:
+  std::mutex _mutex;
   std::shared_ptr<View> _view;
   std::shared_ptr<SceneWrapper> _scene;
   std::shared_ptr<CameraWrapper> _camera;

package/example/src/WorkletExample.tsx

@@ -5,6 +5,8 @@ import {
   Engine,
   Filament,
   Float3,
+  getAssetFromModel,
+  useAssetAnimator,
   useCamera,
   useEngine,
   useModel,
@@ -55,7 +57,7 @@ function Renderer({ engine }: { engine: Engine }) {
   const camera = useCamera(engine)
 
   const prevAspectRatio = useSharedValue(0)
-  const assetAnimator = asset.state === 'loaded' ? asset.animator : undefined
+  const assetAnimator = useAssetAnimator(getAssetFromModel(asset))
   useRenderCallback(
     engine,
     useWorkletCallback(

package/example/src/useCoin.tsx

@@ -3,6 +3,7 @@ import {
   DiscreteDynamicWorld,
   Engine,
   Float3,
+  getAssetFromModel,
   useCylinderShape,
   useModel,
   useRigidBody,
@@ -24,8 +25,8 @@ export function useCoin(engine: Engine, world: DiscreteDynamicWorld, origin: Flo
   const originY = origin[1]
   const originZ = origin[2]
 
-  const coinAsset = coin.state === 'loaded' ? coin.asset : undefined
-  const renderableEntities = coin.state === 'loaded' ? coin.renderableEntities : undefined
+  const coinAsset = getAssetFromModel(coin)
+  const renderableEntities = useMemo(() => coinAsset?.getRenderableEntities(), [coinAsset])
 
   // Takes the first entity which is the mesh and applies a random rotation, scale and position.
   // It returns the entity and its transform. The transform is needed to create the rigid body.

package/src/hooks/useRenderableManager.ts

@@ -2,5 +2,5 @@ import { useMemo } from 'react'
 import type { Engine } from '../types'
 
 export function useRenderableManager(engine: Engine) {
-  return useMemo(() => engine.getRenderableManager(), [engine])
+  return useMemo(() => engine.createRenderableManager(), [engine])
 }

package/src/index.tsx

@@ -15,10 +15,12 @@ export * from './hooks/useScene'
 export * from './hooks/useView'
 export * from './hooks/useTransformManager'
 export * from './hooks/useRenderableManager'
+export * from './hooks/useAssetAnimator'
 
 // utilities
 export * from './utilities/runOnJS'
 export * from './utilities/runOnWorklet'
+export * from './utilities/getAssetFromModel'
 
 // Bullet 3
 export * from './bullet'

package/src/types/Engine.ts

@@ -94,7 +94,7 @@ export interface Engine {
    * Note: the reference returned isn't stable, and each call will return a new reference.
    * Prefer using the hook `useRenderableManager` to get a stable reference.
    */
-  getRenderableManager(): RenderableManager
+  createRenderableManager(): RenderableManager
 
   /**
    * Creates a new material from the given FilamentBuffer.

package/src/utilities/getAssetFromModel.ts

@@ -0,0 +1,9 @@
+import { FilamentModel } from '../hooks/useModel'
+import { FilamentAsset } from '../types'
+
+/**
+ * Helper function to unwrap model.asset from a filament model.
+ */
+export function getAssetFromModel(model: FilamentModel): FilamentAsset | undefined {
+  return model.state === 'loaded' ? model.asset : undefined
+}