Collect deleted scene objects pending deletion into a trashbin

This commit aims at avoiding dangling references in rendering threads
when scene is updated while rendering engine is running

Warning: this commit is not complete as, the trashbin is never emptied (TODO).

Author: howetuft

include/luxrays/core/namedobjectvector.h

@@ -94,6 +94,23 @@ class NamedObjectVector {
 		return std::make_tuple(std::ref(newDerivedRef), std::move(oldDerivedPtr));
 	}
 
+	/// Delete a named object from the container.
+	///
+	/// Please not the name object is just removed, not
+	/// destroyed. The object holder (unique_ptr) is returned
+	/// to caller for further finalization
+	template<typename T>
+	std::unique_ptr<T> DeleteObj(const std::string &name) {
+
+		auto oldObjPtr = DeleteObj<NamedObject>(name);
+		auto oldDerivedPtr = oldObjPtr ?
+			dynamic_uptr_cast<T>(std::move(oldObjPtr)) :
+			std::unique_ptr<T>();
+		return std::move(oldDerivedPtr);
+
+	}
+
+	void DeleteObjs(const std::vector<std::string> &names);
 
 	bool IsObjDefined(const std::string &name) const;
 
@@ -131,8 +148,6 @@ class NamedObjectVector {
 		return std::vector<std::reference_wrapper<NamedObject>>(view.begin(), view.end());
 	}
 
-	void DeleteObj(const std::string &name);
-	void DeleteObjs(const std::vector<std::string> &names);
 
 	std::string ToString() const;
 
@@ -164,7 +179,11 @@ template<>
 std::tuple< NamedObject&, std::unique_ptr<NamedObject> >
 NamedObjectVector::DefineObj(std::unique_ptr<NamedObject>&& obj);
 
-}
+
+template<>
+NamedObjectUPtr NamedObjectVector::DeleteObj(const std::string &name);
+
+}  // Namespace luxrays
 
 #endif	/* _LUXRAYS_NAMEDOBJECTVECTOR_H */
 // vim: autoindent noexpandtab tabstop=4 shiftwidth=4

include/slg/lights/lightsourcedefs.h

@@ -26,6 +26,7 @@
 #include "slg/lights/light.h"
 #include "slg/lights/trianglelight.h"
 #include "slg/lights/strategies/lightstrategy.h"
+#include "slg/usings.h"
 
 namespace slg {
 
@@ -45,7 +46,7 @@ class LightSourceDefinitions {
 
 	void UpdateVisibilityMaps(SceneConstRef scene, const bool useRTMode);
 
-	void DefineLightSource(LightSourceUPtr&& l);
+	LightSourceUPtr DefineLightSource(LightSourceUPtr&& l);
 	bool IsLightSourceDefined(const std::string &name) const;
 
 	LightSourceRef GetLightSource(const std::string &name);
@@ -56,9 +57,9 @@ class LightSourceDefinitions {
 	u_int GetSize() const { return static_cast<u_int>(lightsByName.size()); }
 	std::vector<std::string> GetLightSourceNames() const;
 
-	void DeleteLightSource(const std::string &name);
-	void DeleteLightSourceStartWith(const std::string &namePrefix);
-	void DeleteLightSourceByMaterial(MaterialConstRef mat);
+	LightSourceUPtr DeleteLightSource(const std::string &name);
+	std::vector<LightSourceUPtr> DeleteLightSourceStartWith(const std::string &namePrefix);
+	std::vector<LightSourceUPtr> DeleteLightSourceByMaterial(MaterialConstRef mat);
 
 	void UpdateVolumeReferences(VolumeConstRef oldVol, VolumeRef newVol);
 

include/slg/materials/materialdefs.h

@@ -75,8 +75,8 @@ class MaterialDefinitions {
 		return mats.GetNames();
 	}
 
-	void DeleteMaterial(const std::string &name) {
-		mats.DeleteObj(name);
+	MaterialUPtr DeleteMaterial(const std::string &name) {
+		return mats.DeleteObj<Material>(name);
 	}
 
 	void UpdateTextureReferences(TextureConstRef oldTex, TextureRef newTex);

include/slg/scene/extmeshcache.h

@@ -26,6 +26,7 @@
 #include "luxrays/core/context.h"
 #include "luxrays/core/exttrianglemesh.h"
 #include "luxrays/core/namedobjectvector.h"
+#include "luxrays/usings.h"
 #include "luxrays/utils/serializationutils.h"
 #include "slg/core/sdl.h"
 
@@ -52,7 +53,7 @@ class ExtMeshCache {
 
 	// Note: before calls to DeleteExtMesh, be sure to not have any instance referencing
 	// the geometry
-	void DeleteExtMesh(const std::string &meshName);
+	ExtMeshUPtr DeleteExtMesh(const std::string &meshName);
 
 	u_int GetSize() const;
 	auto GetExtMeshNames() const {

include/slg/scene/scene.h

@@ -25,6 +25,7 @@
 
 #include "luxrays/core/exttrianglemesh.h"
 #include "luxrays/core/geometry/bsphere.h"
+#include "luxrays/core/namedobjectvector.h"
 #include "luxrays/usings.h"
 #include "slg/imagemap/imagemap.h"
 #include "slg/usings.h"
@@ -183,7 +184,8 @@ class Scene {
 		const unsigned int index, float *data);
 
 	// Strands shape
-	void DefineStrands(const std::string &shapeName, const luxrays::cyHairFile &strandsFile,
+	Scene::ReturnType<ExtTriangleMesh> DefineStrands(
+		const std::string &shapeName, const luxrays::cyHairFile &strandsFile,
 		const StrendsShape::TessellationType tesselType,
 		const u_int adaptiveMaxDepth, const float adaptiveError,
 		const u_int solidSideCount, const bool solidCapBottom, const bool solidCapTop,
@@ -259,12 +261,17 @@ class Scene {
 
 	void SetEnableParsePrint(bool status) { enableParsePrint = status; }
 
+	void moveToTrash(luxrays::NamedObjectUPtr&&);
+	void emptyTrash();
+
 	// Serialization
 	static SceneUPtr LoadSerialized(const std::string &fileName);
 	static void SaveSerialized(const std::string &fileName, SceneUPtr&& scene);
 
 	static std::string EncodeTriangleLightNamePrefix(const std::string &objectName);
 
+	// 
+
 protected:
 	//--------------------------------------------------------------------------
 
@@ -280,9 +287,17 @@ class Scene {
 	SceneObjectDefinitions objDefs; // SceneObject definitions
 	LightSourceDefinitions lightDefs; // LightSource definitions
 
-	// DataSet ownership is not very clear, we keep a shared at
-	// the moment
+	// The trash bin container collects items that are pending deletion, in
+	// order to avoid dangling references in real time rendering when the scene
+	// is updated while the render engine is running. Trash bin allows to keep
+	// the deleted objects alive while they are still in use in rendering. It
+	// is up to the rendering engine to empty trash bin.
+	std::vector<luxrays::NamedObjectUPtr> trashBin;
+	mutable std::mutex trashMtx;
+
+	// DataSet ownership is not very clear, we keep it shared
 	luxrays::DataSetSPtr dataSet;
+
 	// The bounding sphere of the scene (including the camera)
 	luxrays::BSphere sceneBSphere;
 

include/slg/scene/sceneobjectdefs.h

@@ -80,8 +80,8 @@ class SceneObjectDefinitions {
 		std::unordered_set<const SceneObject *> &modifiedObjsList
 	);
 
-	void DeleteSceneObject(const std::string &name) {
-		objs.DeleteObj(name);
+	SceneObjectUPtr DeleteSceneObject(const std::string &name) {
+		return objs.DeleteObj<SceneObject>(name);
 	}
 
 	void DeleteSceneObjects(const std::vector<std::string> &names) {

include/slg/slg.h

@@ -23,6 +23,7 @@
 
 #include "luxrays/luxrays.h"
 #include "slg/usings.h"
+#include "slg/slg.h"
 
 /*!
  * \namespace slg

include/slg/textures/texturedefs.h

@@ -71,8 +71,8 @@ class TextureDefinitions {
 		return texs.GetNames();
 	}
 
-	void DeleteTexture(const std::string &name) {
-		texs.DeleteObj(name);
+	TextureUPtr DeleteTexture(const std::string &name) {
+		return std::move(texs.DeleteObj<Texture>(name));
 	}
 
 	const std::vector<std::string> GetTextureSortedNames() const;

src/luxrays/core/namedobjectvector.cpp

@@ -159,59 +159,56 @@ u_int NamedObjectVector::GetSize()const {
 		//names[i] = GetName(i);
 //}
 
-void NamedObjectVector::DeleteObj(const string &name) {
+template<>
+NamedObjectUPtr NamedObjectVector::DeleteObj(const string &name) {
 	// We swap remove target and last object, and pop back
+	if (objs.empty()) throw("Trying to delete an object in an empty container");
+
+	// Record index
+	auto removeObjIndex = GetIndex(name);
+	auto lastObjIndex = objs.size() - 1;
+
+	// Record names
+	const std::string removeObjName{objs[removeObjIndex]->GetName()};
+	const std::string lastObjName{objs[lastObjIndex]->GetName()};
+
+	if (lastObjIndex != removeObjIndex)
+	{
+
+		std::swap(objs[removeObjIndex], objs[lastObjIndex]);
 
-	if (objs.size() > 0)
+		// redo links
+		name2index.left.erase(removeObjName);
+		name2index.left.erase(lastObjName);
+		name2index.insert(Name2IndexType::value_type(lastObjName, removeObjIndex));
+
+		index2obj.left.erase(lastObjIndex);
+		index2obj.left.erase(removeObjIndex);
+		index2obj.insert(
+			Index2ObjType::value_type(
+				removeObjIndex, std::ref(*objs[removeObjIndex])
+			)
+		);
+
+	}
+	else
 	{
-		// Record index
-		auto removeObjIndex = GetIndex(name);
-		auto lastObjIndex = objs.size() - 1;
-
-		// Record names
-		const string removeObjName{objs[removeObjIndex]->GetName()};
-		const string lastObjName{objs[lastObjIndex]->GetName()};
-
-		if (lastObjIndex != removeObjIndex)
-		{
-
-			std::swap(objs[removeObjIndex], objs[lastObjIndex]);
-
-			//TODO
-			//NamedObject* lastObj = objs[lastIndex];
-			////int newIndex = index;
-			//objs[index] = lastObj;
-
-			// redo links
-			name2index.left.erase(removeObjName);
-			name2index.left.erase(lastObjName);
-			name2index.insert(Name2IndexType::value_type(lastObjName, removeObjIndex));
-
-			index2obj.left.erase(lastObjIndex);
-			index2obj.left.erase(removeObjIndex);
-			index2obj.insert(
-				Index2ObjType::value_type(
-					removeObjIndex, std::ref(*objs[removeObjIndex])
-				)
-			);
-
-		}
-		else
-		{
-			// last
-			name2index.left.erase(removeObjName);
-			index2obj.left.erase(removeObjIndex);
-		}
+		// last
+		name2index.left.erase(removeObjName);
+		index2obj.left.erase(removeObjIndex);
 	}
 
 	// remove last
+	NamedObjectUPtr ret = std::move(objs.back());
 	objs.pop_back();
+	return std::move(ret);
 }
 
+
 void NamedObjectVector::DeleteObjs(const std::vector<string> &names) {
 
 	for (const string& name : names) {
-		DeleteObj(name);
+		DeleteObj<NamedObject>(name);
 	}
 }
 

src/slg/lights/lightsourcedefs.cpp

@@ -24,6 +24,7 @@
 #include "slg/scene/scene.h"
 #include "slg/lights/trianglelight.h"
 #include "slg/lights/strategies/logpower.h"
+#include "slg/usings.h"
 
 using namespace std;
 using namespace luxrays;
@@ -45,18 +46,27 @@ LightSourceDefinitions::LightSourceDefinitions() :
 LightSourceDefinitions::~LightSourceDefinitions() {
 }
 
-void LightSourceDefinitions::DefineLightSource(LightSourceUPtr&& newLight) {
+/// Insert a new light source in the container
+///
+/// If a light source with the same name already exists in the container,
+/// it is extracted and returned.
+LightSourceUPtr LightSourceDefinitions::DefineLightSource(LightSourceUPtr&& newLight) {
 	const string &name = newLight->GetName();
 
-	if (IsLightSourceDefined(name)) {
-		auto& oldLight = GetLightSource(name);
+	auto e = lightsByName.find(name);
+
+	if (e != lightsByName.end()) {
+		// Light source already exists
+		auto oldLight = std::move(e->second);
 
 		// Update name/LightSource definition
-		lightsByName.erase(name);
+		lightsByName.erase(e);
 		lightsByName[name] = std::move(newLight);
+		return oldLight;
 	} else {
 		// Add the new LightSource
 		lightsByName[name] = std::move(newLight);
+		return LightSourceUPtr();
 	}
 }
 
@@ -120,31 +130,47 @@ vector<string> LightSourceDefinitions::GetLightSourceNames() const {
 	return names;
 }
 
-void LightSourceDefinitions::DeleteLightSource(const string &name) {
+LightSourceUPtr LightSourceDefinitions::DeleteLightSource(const string &name) {
 	auto e = lightsByName.find(name);
 
 	if (e == lightsByName.end())
 		throw runtime_error("Reference to an undefined LightSource in LightSourceDefinitions::DeleteLightSource(): " + name);
-	else {
-		lightsByName.erase(name);
-	}
+
+
+	auto oldLight = std::move(e->second);
+	lightsByName.erase(e);
+	return oldLight;
 }
 
-void LightSourceDefinitions::DeleteLightSourceStartWith(const string &namePrefix) {
+
+std::vector<LightSourceUPtr> LightSourceDefinitions::DeleteLightSourceStartWith(
+	const string &namePrefix
+) {
+	std::vector<LightSourceUPtr> oldLights;
+
 	// Build the list of lights to delete
-	vector<string> nameList;
+	std::vector<string> nameList;
 	for (auto const &e : lightsByName) {
 		const string &name = e.first;
 
 		if (name.starts_with(namePrefix))
 			nameList.push_back(name);
 	}
 
-	for (auto const &name : nameList)
-		DeleteLightSource(name);
+	// Remove the lights from the container and return the removed lights
+	for (auto const &name : nameList) {
+		auto oldLight = DeleteLightSource(name);
+		oldLights.push_back(std::move(oldLight));
+	}
+	return oldLights;
 }
 
-void LightSourceDefinitions::DeleteLightSourceByMaterial(MaterialConstRef mat) {
+
+std::vector<LightSourceUPtr> LightSourceDefinitions::DeleteLightSourceByMaterial(
+	MaterialConstRef mat
+) {
+	std::vector<LightSourceUPtr> oldLights;
+
 	// Build the list of lights to delete
 	std::vector<string> nameList;
 	for (auto const &e : lightsByName) {
@@ -159,8 +185,12 @@ void LightSourceDefinitions::DeleteLightSourceByMaterial(MaterialConstRef mat) {
 		}
 	}
 
-	for (auto const &name : nameList)
-		DeleteLightSource(name);
+	// Remove the lights from the container and return the removed lights
+	for (auto const &name : nameList) {
+		auto oldLight = DeleteLightSource(name);
+		oldLights.push_back(std::move(oldLight));
+	}
+	return oldLights;
 }
 
 void LightSourceDefinitions::SetLightStrategy(const luxrays::Properties &props) {