Some refactoring in the threading in GLTF importer

AnKi/Importer/GltfImporter.cpp

@@ -230,43 +230,151 @@ Error GltfImporter::writeAll()
 	ANKI_CHECK(m_sceneFile.writeText("local scene = getSceneGraph()\nlocal events = getEventManager()\n"));
 
 	// Nodes
-	Error err = Error::kNone;
-	for(const cgltf_scene* scene = m_gltf->scenes; scene < m_gltf->scenes + m_gltf->scenes_count && !err; ++scene)
+	for(const cgltf_scene* scene = m_gltf->scenes; scene < m_gltf->scenes + m_gltf->scenes_count; ++scene)
 	{
-		for(cgltf_node* const* node = scene->nodes; node < scene->nodes + scene->nodes_count && !err; ++node)
+		for(cgltf_node* const* node = scene->nodes; node < scene->nodes + scene->nodes_count; ++node)
 		{
-			err = visitNode(*(*node), Transform::getIdentity(), HashMapRaii<CString, StringRaii>(m_pool));
+			ANKI_CHECK(visitNode(*(*node), Transform::getIdentity(), HashMapRaii<CString, StringRaii>(m_pool)));
 		}
 	}
 
-	if(m_hive)
+	// Fire up all requests
+	for(auto& req : m_meshImportRequests)
 	{
-		m_hive->waitAllTasks();
+		if(m_hive)
+		{
+			m_hive->submitTask(
+				[](void* userData, [[maybe_unused]] U32 threadId, [[maybe_unused]] ThreadHive& hive,
+				   [[maybe_unused]] ThreadHiveSemaphore* signalSemaphore) {
+					ImportRequest<const cgltf_mesh*>* req = static_cast<ImportRequest<const cgltf_mesh*>*>(userData);
+					Error err = req->m_importer->m_errorInThread.load();
+
+					if(!err)
+					{
+						err = req->m_importer->writeMesh(*req->m_value);
+					}
+
+					if(err)
+					{
+						req->m_importer->m_errorInThread.store(err._getCode());
+					}
+				},
+				&req);
+		}
+		else
+		{
+			ANKI_CHECK(writeMesh(*req.m_value));
+		}
 	}
 
-	// Check error
-	if(err)
+	for(auto& req : m_materialImportRequests)
 	{
-		ANKI_IMPORTER_LOGE("Error happened in main thread");
-		return err;
+		if(m_hive)
+		{
+			m_hive->submitTask(
+				[](void* userData, [[maybe_unused]] U32 threadId, [[maybe_unused]] ThreadHive& hive,
+				   [[maybe_unused]] ThreadHiveSemaphore* signalSemaphore) {
+					ImportRequest<MaterialImportRequest>* req =
+						static_cast<ImportRequest<MaterialImportRequest>*>(userData);
+					Error err = req->m_importer->m_errorInThread.load();
+
+					if(!err)
+					{
+						err = req->m_importer->writeMaterial(*req->m_value.m_cgltfMaterial, req->m_value.m_writeRt);
+					}
+
+					if(err)
+					{
+						req->m_importer->m_errorInThread.store(err._getCode());
+					}
+				},
+				&req);
+		}
+		else
+		{
+			ANKI_CHECK(writeMaterial(*req.m_value.m_cgltfMaterial, req.m_value.m_writeRt));
+		}
 	}
 
-	const Error threadErr = m_errorInThread.load();
-	if(threadErr)
+	for(auto& req : m_skinImportRequests)
 	{
-		ANKI_IMPORTER_LOGE("Error happened in a thread");
-		return threadErr;
+		if(m_hive)
+		{
+			m_hive->submitTask(
+				[](void* userData, [[maybe_unused]] U32 threadId, [[maybe_unused]] ThreadHive& hive,
+				   [[maybe_unused]] ThreadHiveSemaphore* signalSemaphore) {
+					ImportRequest<const cgltf_skin*>* req = static_cast<ImportRequest<const cgltf_skin*>*>(userData);
+					Error err = req->m_importer->m_errorInThread.load();
+
+					if(!err)
+					{
+						err = req->m_importer->writeSkeleton(*req->m_value);
+					}
+
+					if(err)
+					{
+						req->m_importer->m_errorInThread.store(err._getCode());
+					}
+				},
+				&req);
+		}
+		else
+		{
+			ANKI_CHECK(writeSkeleton(*req.m_value));
+		}
+	}
+
+	for(auto& req : m_modelImportRequests)
+	{
+		if(m_hive)
+		{
+			m_hive->submitTask(
+				[](void* userData, [[maybe_unused]] U32 threadId, [[maybe_unused]] ThreadHive& hive,
+				   [[maybe_unused]] ThreadHiveSemaphore* signalSemaphore) {
+					ImportRequest<const cgltf_mesh*>* req = static_cast<ImportRequest<const cgltf_mesh*>*>(userData);
+					Error err = req->m_importer->m_errorInThread.load();
+
+					if(!err)
+					{
+						err = req->m_importer->writeModel(*req->m_value);
+					}
+
+					if(err)
+					{
+						req->m_importer->m_errorInThread.store(err._getCode());
+					}
+				},
+				&req);
+		}
+		else
+		{
+			ANKI_CHECK(writeModel(*req.m_value));
+		}
 	}
 
+	if(m_hive)
+	{
+		m_hive->waitAllTasks();
+
+		const Error threadErr = m_errorInThread.load();
+		if(threadErr)
+		{
+			ANKI_IMPORTER_LOGE("Error happened in a thread");
+			return threadErr;
+		}
+	}
+
+	// Animations
 	for(const cgltf_animation* anim = m_gltf->animations; anim < m_gltf->animations + m_gltf->animations_count; ++anim)
 	{
 		ANKI_CHECK(writeAnimation(*anim));
 	}
 
-	return err;
+	ANKI_IMPORTER_LOGV("Importing GLTF has completed");
+	return Error::kNone;
 }
 
-Error GltfImporter::getExtras(const cgltf_extras& extras, HashMapRaii<CString, StringRaii>& out)
+Error GltfImporter::getExtras(const cgltf_extras& extras, HashMapRaii<CString, StringRaii>& out) const
 {
 	cgltf_size extrasSize;
 	cgltf_copy_extras_json(m_gltf, &extras, nullptr, &extrasSize);
@@ -360,7 +468,7 @@ void GltfImporter::populateNodePtrToIdx()
 	}
 }
 
-StringRaii GltfImporter::getNodeName(const cgltf_node& node)
+StringRaii GltfImporter::getNodeName(const cgltf_node& node) const
 {
 	StringRaii out(m_pool);
 
@@ -448,8 +556,6 @@ Error GltfImporter::visitNode(const cgltf_node& node, const Transform& parentTrf
 
 		HashMapRaii<CString, StringRaii>::Iterator it;
 
-		const Bool skipRt = (it = extras.find("no_rt")) != extras.getEnd() && (*it == "true" || *it == "1");
-
 		if((it = extras.find("particles")) != extras.getEnd())
 		{
 			const StringRaii& fname = *it;
@@ -680,73 +786,23 @@ Error GltfImporter::visitNode(const cgltf_node& node, const Transform& parentTrf
 		{
 			// Model node
 
-			// Async because it's slow
-			struct Ctx
-			{
-				GltfImporter* m_importer;
-				const cgltf_mesh* m_mesh;
-				Array<const cgltf_material*, 128> m_materials;
-				U32 m_materialCount = 0;
-				const cgltf_skin* m_skin;
-				Bool m_rayTracing;
-			};
-			Ctx* ctx = newInstance<Ctx>(*m_pool);
-			ctx->m_importer = this;
-			ctx->m_mesh = node.mesh;
+			const Bool skipRt = (it = extras.find("no_rt")) != extras.getEnd() && (*it == "true" || *it == "1");
+
+			addRequest<const cgltf_mesh*>(node.mesh, m_meshImportRequests);
 			for(U32 i = 0; i < node.mesh->primitives_count; ++i)
 			{
-				ctx->m_materials[ctx->m_materialCount++] = node.mesh->primitives[i].material;
+				const MaterialImportRequest req = {node.mesh->primitives[i].material, !skipRt};
+				addRequest<MaterialImportRequest>(req, m_materialImportRequests);
 			}
-			ctx->m_skin = node.skin;
-			ctx->m_rayTracing = !skipRt;
 
-			HashMapRaii<CString, StringRaii>::Iterator it2;
-			const Bool selfCollision = (it2 = extras.find("collision_mesh")) != extras.getEnd() && *it2 == "self";
-
-			// Thread task
-			auto callback = [](void* userData, [[maybe_unused]] U32 threadId, [[maybe_unused]] ThreadHive& hive,
-							   [[maybe_unused]] ThreadHiveSemaphore* signalSemaphore) {
-				Ctx& self = *static_cast<Ctx*>(userData);
-
-				Error err = self.m_importer->m_errorInThread.load();
-
-				if(!err)
-				{
-					err = self.m_importer->writeMesh(*self.m_mesh);
-				}
-
-				for(U32 i = 0; i < self.m_materialCount && !err; ++i)
-				{
-					err = self.m_importer->writeMaterial(*self.m_materials[i], self.m_rayTracing);
-				}
-
-				if(!err)
-				{
-					err = self.m_importer->writeModel(*self.m_mesh);
-				}
-
-				if(!err && self.m_skin)
-				{
-					err = self.m_importer->writeSkeleton(*self.m_skin);
-				}
-
-				if(err)
-				{
-					self.m_importer->m_errorInThread.store(err._getCode());
-				}
-
-				deleteInstance(*self.m_importer->m_pool, &self);
-			};
-
-			if(m_hive != nullptr)
-			{
-				m_hive->submitTask(callback, ctx);
-			}
-			else
+			if(node.skin)
 			{
-				callback(ctx, 0, *m_hive, nullptr);
+				addRequest<const cgltf_skin*>(node.skin, m_skinImportRequests);
 			}
 
+			HashMapRaii<CString, StringRaii>::Iterator it2;
+			const Bool selfCollision = (it2 = extras.find("collision_mesh")) != extras.getEnd() && *it2 == "self";
+
 			ANKI_CHECK(writeModelNode(node, parentExtras));
 
 			Transform localTrf;
@@ -812,7 +868,7 @@ Error GltfImporter::writeTransform(const Transform& trf)
 	return Error::kNone;
 }
 
-Error GltfImporter::writeModel(const cgltf_mesh& mesh)
+Error GltfImporter::writeModel(const cgltf_mesh& mesh) const
 {
 	const StringRaii modelFname = computeModelResourceFilename(mesh);
 	ANKI_IMPORTER_LOGV("Importing model %s", modelFname.cstr());
@@ -866,7 +922,7 @@ Error GltfImporter::writeModel(const cgltf_mesh& mesh)
 	return Error::kNone;
 }
 
-Error GltfImporter::writeSkeleton(const cgltf_skin& skin)
+Error GltfImporter::writeSkeleton(const cgltf_skin& skin) const
 {
 	StringRaii fname(m_pool);
 	fname.sprintf("%s%s", m_outDir.cstr(), computeSkeletonResourceFilename(skin).cstr());

AnKi/Importer/GltfImporter.h

@@ -60,7 +60,7 @@ private:
 	};
 
 	// Data
-	BaseMemoryPool* m_pool = nullptr;
+	mutable BaseMemoryPool* m_pool = nullptr;
 
 	StringRaii m_inputFname = {m_pool};
 	StringRaii m_outDir = {m_pool};
@@ -75,7 +75,7 @@ private:
 
 	File m_sceneFile;
 
-	Atomic<I32> m_errorInThread{0};
+	mutable Atomic<I32> m_errorInThread = {0};
 
 	HashMapRaii<const void*, U32, PtrHasher> m_nodePtrToIdx = {m_pool}; ///< Need an index for the unnamed nodes.
 
@@ -91,12 +91,58 @@ private:
 
 	Bool m_importTextures = false;
 
+	template<typename T>
+	class ImportRequest
+	{
+	public:
+		const GltfImporter* m_importer = nullptr;
+		T m_value = {};
+	};
+
+	class MaterialImportRequest
+	{
+	public:
+		const cgltf_material* m_cgltfMaterial = nullptr;
+		Bool m_writeRt = true;
+
+		Bool operator==(const MaterialImportRequest& b) const
+		{
+			return m_cgltfMaterial == b.m_cgltfMaterial && m_writeRt == b.m_writeRt;
+		}
+	};
+
+	DynamicArrayRaii<ImportRequest<const cgltf_mesh*>> m_meshImportRequests = {m_pool};
+	DynamicArrayRaii<ImportRequest<MaterialImportRequest>> m_materialImportRequests = {m_pool};
+	DynamicArrayRaii<ImportRequest<const cgltf_skin*>> m_skinImportRequests = {m_pool};
+	DynamicArrayRaii<ImportRequest<const cgltf_mesh*>> m_modelImportRequests = {m_pool};
+
 	// Misc
-	Error getExtras(const cgltf_extras& extras, HashMapRaii<CString, StringRaii>& out);
+	template<typename T>
+	void addRequest(const T& value, DynamicArrayRaii<ImportRequest<T>>& array) const
+	{
+		Bool found = false;
+		for(const auto& req : array)
+		{
+			ANKI_ASSERT(req.m_importer == this);
+			if(req.m_value == value)
+			{
+				found = true;
+				break;
+			}
+		}
+
+		if(!found)
+		{
+			const ImportRequest<T> req = {this, value};
+			array.emplaceBack(req);
+		}
+	}
+
+	Error getExtras(const cgltf_extras& extras, HashMapRaii<CString, StringRaii>& out) const;
 	Error parseArrayOfNumbers(CString str, DynamicArrayRaii<F64>& out, const U32* expectedArraySize = nullptr);
 	void populateNodePtrToIdx();
 	void populateNodePtrToIdxInternal(const cgltf_node& node, U32& idx);
-	StringRaii getNodeName(const cgltf_node& node);
+	StringRaii getNodeName(const cgltf_node& node) const;
 
 	template<typename T, typename TFunc>
 	static void visitAccessor(const cgltf_accessor& accessor, TFunc func);
@@ -138,11 +184,11 @@ private:
 	StringRaii computeSkeletonResourceFilename(const cgltf_skin& skin) const;
 
 	// Resources
-	Error writeMesh(const cgltf_mesh& mesh);
-	Error writeMaterial(const cgltf_material& mtl, Bool writeRayTracing);
-	Error writeModel(const cgltf_mesh& mesh);
+	Error writeMesh(const cgltf_mesh& mesh) const;
+	Error writeMaterial(const cgltf_material& mtl, Bool writeRayTracing) const;
+	Error writeModel(const cgltf_mesh& mesh) const;
 	Error writeAnimation(const cgltf_animation& anim);
-	Error writeSkeleton(const cgltf_skin& skin);
+	Error writeSkeleton(const cgltf_skin& skin) const;
 
 	// Scene
 	Error writeTransform(const Transform& trf);

AnKi/Importer/GltfImporterAnimation.cpp

@@ -364,18 +364,14 @@ Error GltfImporter::writeAnimation(const cgltf_animation& anim)
 
 		// Only animate cameras for now
 		const cgltf_node& node = *channel.m_targetNode;
-		if((node.camera == nullptr) || node.name == nullptr)
+		if(node.camera == nullptr || node.name == nullptr)
 		{
 			continue;
 		}
 
-		// ANKI_CHECK(m_sceneFile.writeText("--[[\n"));
-
 		ANKI_CHECK(m_sceneFile.writeTextf("\nnode = scene:tryFindSceneNode(\"%s\")\n", node.name));
 		ANKI_CHECK(m_sceneFile.writeTextf("getEventManager():newAnimationEvent(\"%s%s\", \"%s\", node)\n",
 										  m_rpath.cstr(), animFname.cstr(), node.name));
-
-		// ANKI_CHECK(m_sceneFile.writeText("--]]\n"));
 	}
 
 	return Error::kNone;

AnKi/Importer/GltfImporterMaterial.cpp

@@ -149,7 +149,7 @@ static void fixImageUri(StringRaii& uri)
 	uri.replaceAll(".jpeg", ".ankitex");
 }
 
-Error GltfImporter::writeMaterial(const cgltf_material& mtl, Bool writeRayTracing)
+Error GltfImporter::writeMaterial(const cgltf_material& mtl, Bool writeRayTracing) const
 {
 	StringRaii fname(m_pool);
 	fname.sprintf("%s%s", m_outDir.cstr(), computeMaterialResourceFilename(mtl).cstr());

AnKi/Importer/GltfImporterMesh.cpp

@@ -440,7 +440,7 @@ U32 GltfImporter::getMeshTotalVertexCount(const cgltf_mesh& mesh)
 	return totalVertexCount;
 }
 
-Error GltfImporter::writeMesh(const cgltf_mesh& mesh)
+Error GltfImporter::writeMesh(const cgltf_mesh& mesh) const
 {
 	StringRaii meshName = computeMeshResourceFilename(mesh);
 	StringRaii fname(m_pool);

Samples/Sponza/Assets/Scene.lua

@@ -1,4 +1,4 @@
--- Generated by: C:\Users\godli\src\anki\out\build\x64-Release\Bin\GltfImporter.exe sponza_crytek_7_pbr_3.0.gltf C:/Users/godli/src/anki/Samples/Sponza/Assets/ -rpath Assets -texrpath Assets -lod-count 2 -light-scale 0.01
+-- Generated by: C:\Users\godli\src\anki\out\build\x64-Debug\Bin\GltfImporter.exe sponza_crytek_7_pbr_3.0.gltf C:/Users/godli/src/anki/Samples/Sponza/Assets/ -rpath Assets -texrpath Assets -lod-count 2 -light-scale 0.01 -v
 local scene = getSceneGraph()
 local events = getEventManager()