Blender: iterative loading code for the linked list of scene objects, fixes #28 but is preliminary as it touches code that is normally machine-generated.

code/BlenderDNA.h

@@ -278,19 +278,23 @@ public:
 	// --------------------------------------------------------
 	// field parsing for pointer or dynamic array types 
 	// (boost::shared_ptr or boost::shared_array)
+	// The return value indicates whether the data was already cached.
 	template <int error_policy, template <typename> class TOUT, typename T>
-	void ReadFieldPtr(TOUT<T>& out, const char* name, 
+	bool ReadFieldPtr(TOUT<T>& out, const char* name, 
-		const FileDatabase& db) const;
+		const FileDatabase& db,
+		bool non_recursive = false) const;
 
 	// --------------------------------------------------------
 	// field parsing for static arrays of pointer or dynamic
 	// array types (boost::shared_ptr[] or boost::shared_array[])
+	// The return value indicates whether the data was already cached.
 	template <int error_policy, template <typename> class TOUT, typename T, size_t N>
-	void ReadFieldPtr(TOUT<T> (&out)[N], const char* name, 
+	bool ReadFieldPtr(TOUT<T> (&out)[N], const char* name, 
 		const FileDatabase& db) const;
 
 	// --------------------------------------------------------
 	// field parsing for `normal` values
+	// The return value indicates whether the data was already cached.
 	template <int error_policy, typename T>
 	void ReadField(T& out, const char* name, 
 		const FileDatabase& db) const;
@@ -299,17 +303,18 @@ private:
 
 	// --------------------------------------------------------
 	template <template <typename> class TOUT, typename T>
-	void ResolvePointer(TOUT<T>& out, const Pointer & ptrval, 
+	bool ResolvePointer(TOUT<T>& out, const Pointer & ptrval, 
-		const FileDatabase& db, const Field& f) const;
+		const FileDatabase& db, const Field& f,
+		bool non_recursive = false) const;
 
 	// --------------------------------------------------------
 	template <template <typename> class TOUT, typename T>
-	void ResolvePointer(vector< TOUT<T> >& out, const Pointer & ptrval, 
+	bool ResolvePointer(vector< TOUT<T> >& out, const Pointer & ptrval, 
-		const FileDatabase& db, const Field& f) const;
+		const FileDatabase& db, const Field& f, bool) const;
 
 	// --------------------------------------------------------
-	void ResolvePointer( boost::shared_ptr< FileOffset >& out, const Pointer & ptrval, 
+	bool ResolvePointer( boost::shared_ptr< FileOffset >& out, const Pointer & ptrval, 
-		const FileDatabase& db, const Field& f) const;
+		const FileDatabase& db, const Field& f, bool) const;
 
 	// --------------------------------------------------------
 	inline const FileBlockHead* LocateFileBlockForAddress(
@@ -384,10 +389,11 @@ template <> struct Structure :: _defaultInitializer<ErrorPolicy_Fail> {
 };
 
 // -------------------------------------------------------------------------------------------------------
-template <> inline void Structure :: ResolvePointer<boost::shared_ptr,ElemBase>(boost::shared_ptr<ElemBase>& out, 
+template <> inline bool Structure :: ResolvePointer<boost::shared_ptr,ElemBase>(boost::shared_ptr<ElemBase>& out, 
 	const Pointer & ptrval, 
 	const FileDatabase& db, 
-	const Field& f
+	const Field& f,
+	bool
 	) const;
 
 

code/BlenderDNA.inl

@@ -180,7 +180,8 @@ void Structure :: ReadFieldArray2(T (& out)[M][N], const char* name, const FileD
 
 //--------------------------------------------------------------------------------
 template <int error_policy, template <typename> class TOUT, typename T>
-void Structure :: ReadFieldPtr(TOUT<T>& out, const char* name, const FileDatabase& db) const
+bool Structure :: ReadFieldPtr(TOUT<T>& out, const char* name, const FileDatabase& db,
+	bool non_recursive /*= false*/) const
 {
 	const StreamReaderAny::pos old = db.reader->GetCurrentPos();
 	Pointer ptrval;
@@ -203,23 +204,27 @@ void Structure :: ReadFieldPtr(TOUT<T>& out, const char* name, const FileDatabas
 		_defaultInitializer<error_policy>()(out,e.what());
 
 		out.reset();
-		return;
+		return false;
 	}
 
 	// resolve the pointer and load the corresponding structure
-	ResolvePointer(out,ptrval,db,*f);
+	const bool res = ResolvePointer(out,ptrval,db,*f, non_recursive);
 
-	// and recover the previous stream position
+	if(!non_recursive) {
-	db.reader->SetCurrentPos(old);
+		// and recover the previous stream position
+		db.reader->SetCurrentPos(old);
+	}
 
 #ifndef ASSIMP_BUILD_BLENDER_NO_STATS
 	++db.stats().fields_read;
 #endif
+
+	return res;
 }
 
 //--------------------------------------------------------------------------------
 template <int error_policy, template <typename> class TOUT, typename T, size_t N>
-void Structure :: ReadFieldPtr(TOUT<T> (&out)[N], const char* name, 
+bool Structure :: ReadFieldPtr(TOUT<T> (&out)[N], const char* name, 
 	const FileDatabase& db) const
 {
 	// XXX see if we can reduce this to call to the 'normal' ReadFieldPtr
@@ -253,11 +258,13 @@ void Structure :: ReadFieldPtr(TOUT<T> (&out)[N], const char* name,
 		for(size_t i = 0; i < N; ++i) {
 			out[i].reset();
 		}
-		return;
+		return false;
 	}
+
+	bool res = true;
 	for(size_t i = 0; i < N; ++i) {
 		// resolve the pointer and load the corresponding structure
-		ResolvePointer(out[i],ptrval[i],db,*f);
+		res = ResolvePointer(out[i],ptrval[i],db,*f) && res;
 	}
 
 	// and recover the previous stream position
@@ -266,6 +273,7 @@ void Structure :: ReadFieldPtr(TOUT<T> (&out)[N], const char* name,
 #ifndef ASSIMP_BUILD_BLENDER_NO_STATS
 	++db.stats().fields_read;
 #endif
+	return res;
 }
 
 //--------------------------------------------------------------------------------
@@ -296,11 +304,13 @@ void Structure :: ReadField(T& out, const char* name, const FileDatabase& db) co
 
 //--------------------------------------------------------------------------------
 template <template <typename> class TOUT, typename T>
-void Structure :: ResolvePointer(TOUT<T>& out, const Pointer & ptrval, const FileDatabase& db, const Field& f) const 
+bool Structure :: ResolvePointer(TOUT<T>& out, const Pointer & ptrval, const FileDatabase& db, 
+	const Field& f, 
+	bool non_recursive /*= false*/) const 
 {
-	out.reset();
+	out.reset(); // ensure null pointers work
 	if (!ptrval.val) { 
-		return;
+		return false;
 	}
 	const Structure& s = db.dna[f.type];
 	// find the file block the pointer is pointing to
@@ -318,7 +328,7 @@ void Structure :: ResolvePointer(TOUT<T>& out, const Pointer & ptrval, const Fil
 	// try to retrieve the object from the cache
 	db.cache(out).get(s,out,ptrval); 
 	if (out) {
-		return;
+		return true;
 	}
 
 	// seek to this location, but save the previous stream pointer.
@@ -334,27 +344,36 @@ void Structure :: ResolvePointer(TOUT<T>& out, const Pointer & ptrval, const Fil
 	// cache the object before we convert it to avoid cyclic recursion.
 	db.cache(out).set(s,out,ptrval); 
 
-	for (size_t i = 0; i < num; ++i,++o) {
+	// if the non_recursive flag is set, we don't do anything but leave
-		s.Convert(*o,db);
+	// the cursor at the correct position to resolve the object.
-	}
+	if (!non_recursive) {
+		for (size_t i = 0; i < num; ++i,++o) {
+			s.Convert(*o,db);
+		}
 
-	db.reader->SetCurrentPos(pold);
+		db.reader->SetCurrentPos(pold);
+	}
 
 #ifndef ASSIMP_BUILD_BLENDER_NO_STATS
 	if(out) {
 		++db.stats().pointers_resolved;
 	}
 #endif
+	return false;
 }
 
+
 //--------------------------------------------------------------------------------
-inline void Structure :: ResolvePointer( boost::shared_ptr< FileOffset >& out, const Pointer & ptrval, const FileDatabase& db, const Field& /*f*/) const
+inline bool Structure :: ResolvePointer( boost::shared_ptr< FileOffset >& out, const Pointer & ptrval, 
+	const FileDatabase& db, 
+	const Field&,
+	bool) const
 {
 	// Currently used exclusively by PackedFile::data to represent
 	// a simple offset into the mapped BLEND file. 
 	out.reset();
 	if (!ptrval.val) { 
-		return;
+		return false;
 	}
 
 	// find the file block the pointer is pointing to
@@ -362,11 +381,15 @@ inline void Structure :: ResolvePointer( boost::shared_ptr< FileOffset >& out, c
 
 	out =  boost::shared_ptr< FileOffset > (new FileOffset());
 	out->val = block->start+ static_cast<size_t>((ptrval.val - block->address.val) );
+	return false;
 }
 
 //--------------------------------------------------------------------------------
 template <template <typename> class TOUT, typename T>
-void Structure :: ResolvePointer(vector< TOUT<T> >& out, const Pointer & ptrval, const FileDatabase& db, const Field& f) const 
+bool Structure :: ResolvePointer(vector< TOUT<T> >& out, const Pointer & ptrval, 
+	const FileDatabase& db, 
+	const Field& f,
+	bool) const 
 {
 	// This is a function overload, not a template specialization. According to
 	// the partial ordering rules, it should be selected by the compiler
@@ -374,7 +397,7 @@ void Structure :: ResolvePointer(vector< TOUT<T> >& out, const Pointer & ptrval,
 
 	out.reset();
 	if (!ptrval.val) { 
-		return;
+		return false;
 	}
 
 	// find the file block the pointer is pointing to
@@ -385,6 +408,7 @@ void Structure :: ResolvePointer(vector< TOUT<T> >& out, const Pointer & ptrval,
 	const StreamReaderAny::pos pold = db.reader->GetCurrentPos();
 	db.reader->SetCurrentPos(block->start+ static_cast<size_t>((ptrval.val - block->address.val) ));
 
+	bool res = false;
 	// allocate raw storage for the array
 	out.resize(num);
 	for (size_t i = 0; i< num; ++i) {
@@ -392,17 +416,19 @@ void Structure :: ResolvePointer(vector< TOUT<T> >& out, const Pointer & ptrval,
 		Convert(val,db);
 
 		// and resolve the pointees
-		ResolvePointer(out[i],val,db,f); 
+		res = ResolvePointer(out[i],val,db,f) && res; 
 	}
 
 	db.reader->SetCurrentPos(pold);
+	return res;
 }
 
 //--------------------------------------------------------------------------------
-template <> void Structure :: ResolvePointer<boost::shared_ptr,ElemBase>(boost::shared_ptr<ElemBase>& out, 
+template <> bool Structure :: ResolvePointer<boost::shared_ptr,ElemBase>(boost::shared_ptr<ElemBase>& out, 
 	const Pointer & ptrval, 
 	const FileDatabase& db, 
-	const Field& /*f*/
+	const Field&,
+	bool
 ) const 
 {
 	// Special case when the data type needs to be determined at runtime.
@@ -410,7 +436,7 @@ template <> void Structure :: ResolvePointer<boost::shared_ptr,ElemBase>(boost::
 
 	out.reset();
 	if (!ptrval.val) { 
-		return;
+		return false;
 	}
 
 	// find the file block the pointer is pointing to
@@ -422,7 +448,7 @@ template <> void Structure :: ResolvePointer<boost::shared_ptr,ElemBase>(boost::
 	// try to retrieve the object from the cache
 	db.cache(out).get(s,out,ptrval); 
 	if (out) {
-		return;
+		return true;
 	}
 
 	// seek to this location, but save the previous stream pointer.
@@ -440,7 +466,7 @@ template <> void Structure :: ResolvePointer<boost::shared_ptr,ElemBase>(boost::
 		DefaultLogger::get()->warn((Formatter::format(),
 			"Failed to find a converter for the `",s.name,"` structure"
 			));
-		return;
+		return false;
 	}
 
 	// allocate the object hull
@@ -459,11 +485,11 @@ template <> void Structure :: ResolvePointer<boost::shared_ptr,ElemBase>(boost::
 	// to perform additional type checking.
 	out->dna_type = s.name.c_str();
 
-	
 
 #ifndef ASSIMP_BUILD_BLENDER_NO_STATS
 	++db.stats().pointers_resolved;
 #endif
+	return false;
 }
 
 //--------------------------------------------------------------------------------

code/BlenderScene.cpp

@@ -241,16 +241,43 @@ template <> void Structure :: Convert<Base> (
     const FileDatabase& db
     ) const
 { 
+	// note: as per https://github.com/assimp/assimp/issues/128,
+	// reading the Object linked list recursively is prone to stack overflow.
+	// This structure converter is therefore an hand-written exception that
+	// does it iteratively.
 
-    {
+	const int initial_pos = db.reader->GetCurrentPos();
-        boost::shared_ptr<Base> prev;
-        ReadFieldPtr<ErrorPolicy_Warn>(prev,"*prev",db);
-        dest.prev = prev.get();
-    }
-    ReadFieldPtr<ErrorPolicy_Warn>(dest.next,"*next",db);
-    ReadFieldPtr<ErrorPolicy_Warn>(dest.object,"*object",db);
 
-	db.reader->IncPtr(size);
+	std::pair<Base*, int> todo = std::make_pair(&dest, initial_pos);
+
+	Base* saved_prev = NULL;
+
+	while(true) {
+	
+		Base& cur_dest = *todo.first;
+		db.reader->SetCurrentPos(todo.second);
+
+		// we know that this is a double-linked, circular list which we never
+		// traverse backwards, so don't bother resolving the back links.
+		cur_dest.prev = NULL;
+
+		ReadFieldPtr<ErrorPolicy_Warn>(cur_dest.object,"*object",db);
+
+		// just record the offset of the blob data and allocate storage.
+		// Does _not_ invoke Convert() recursively.
+		const int old = db.reader->GetCurrentPos();
+
+		// the return value of ReadFieldPtr indicates whether the object 
+		// was already cached. In this case, we don't need to resolve
+		// it again.
+		if(!ReadFieldPtr<ErrorPolicy_Warn>(cur_dest.next,"*next",db, true) && cur_dest.next) {
+			todo = std::make_pair(&*cur_dest.next, db.reader->GetCurrentPos());
+			continue;
+		}
+		break;
+	}
+	
+	db.reader->SetCurrentPos(initial_pos + size);
 }
 
 //--------------------------------------------------------------------------------