changeset 146:54b3984e1afc

The getIndex function was a hack that shadowed another bug that caused the pools to misbehave. AddResourceFromFile works now exactly as getIndex, it's just faster. Fixed GUIImage to not hold a reference, it uses the index directly anyway. Plus heaps of minor adjustments for more informative debug output and statistics.
author phoku@33b003aa-7bff-0310-803a-e67f0ece8222
date Thu, 09 Oct 2008 12:36:21 +0000
parents e7a431577c95
children fb6ccb367dd1
files engine/core/audio/soundclippool.h engine/core/gui/base/gui_image.cpp engine/core/util/resource/pool.cpp engine/core/util/resource/pool.h engine/core/util/resource/resource_location.h engine/core/video/animationpool.h engine/core/video/image_location.cpp engine/core/video/imagepool.h engine/extensions/loaders.py
diffstat 9 files changed, 48 insertions(+), 38 deletions(-) [+]
line wrap: on
line diff
--- a/engine/core/audio/soundclippool.h	Thu Oct 09 08:23:13 2008 +0000
+++ b/engine/core/audio/soundclippool.h	Thu Oct 09 12:36:21 2008 +0000
@@ -44,12 +44,12 @@
 	public:
 		/** Default constructor.
 		 */
-		SoundClipPool(): Pool() {
+		SoundClipPool(const std::string& name = "SoundPool"): Pool(name) {
 		}
 	
 		/** Destructor.
 		 */
-	   virtual ~SoundClipPool() {}
+		virtual ~SoundClipPool() {}
 	
 		SoundClip& getSoundClip(unsigned int index)  {
 			return dynamic_cast<SoundClip&>(get(index));
--- a/engine/core/gui/base/gui_image.cpp	Thu Oct 09 08:23:13 2008 +0000
+++ b/engine/core/gui/base/gui_image.cpp	Thu Oct 09 12:36:21 2008 +0000
@@ -39,11 +39,11 @@
 	}
 
 	GuiImage::GuiImage(int id, ImagePool& pool): gcn::Image(), m_poolid(id), m_pool(&pool) {
-		 m_pool->getImage(m_poolid).addRef();
+		 m_pool->getImage(m_poolid);
 	}
 
 	GuiImage::~GuiImage() {
-		 m_pool->getImage(m_poolid).decRef();
+// 		 m_pool->release(m_poolid,true);
 	}
 
 	void GuiImage::free() {
--- a/engine/core/util/resource/pool.cpp	Thu Oct 09 08:23:13 2008 +0000
+++ b/engine/core/util/resource/pool.cpp	Thu Oct 09 12:36:21 2008 +0000
@@ -35,17 +35,18 @@
 namespace FIFE {
 	static Logger _log(LM_POOL);
 	
-	Pool::Pool(): 
+	Pool::Pool(const std::string& name): 
 		m_entries(),
 		m_location_to_entry(),
 		m_listeners(),
 		m_loaders(),
-		m_curind(0)
+		m_curind(0),
+		m_name(name)
 	{
 	}
 
 	Pool::~Pool() {
-		FL_LOG(_log, LMsg("Pool destroyed "));
+		FL_LOG(_log, LMsg("Pool destroyed: ") << m_name);
 		printStatistics();
 		clear();
 		std::vector<ResourceLoader*>::iterator loader;
@@ -61,6 +62,12 @@
 		}
 		std::vector<PoolEntry*>::iterator entry;
 		for (entry = m_entries.begin(); entry != m_entries.end(); entry++) {
+			// Warn about leaks, but at least display ALL of them
+			// Instead of bailing out with an exception in the FifeClass destructor.
+			if( (*entry)->resource && (*entry)->resource->getRefCount() > 0 ) {
+				FL_WARN(_log, LMsg(m_name + " leak: ") << (*entry)->location->getFilename());
+				(*entry)->resource = 0;
+			}
 			delete (*entry);
 		}
 		m_entries.clear();
@@ -80,9 +87,9 @@
 		PoolEntry* entry = new PoolEntry();
 		entry->location = loc.clone();
 		m_entries.push_back(entry);
-		size_t index = m_entries.size();
+		size_t index = m_entries.size() - 1;
 		m_location_to_entry[loc] = index;
-		return index - 1;
+		return index;
 	}
 
 	int Pool::addResourceFromFile(const std::string& filename) {
@@ -91,7 +98,7 @@
 
 	IResource& Pool::get(unsigned int index, bool inc) {
 		if (index >= m_entries.size()) {
-			FL_ERR(_log, LMsg("Tried to get with index ") << index << ", only " << m_entries.size() << " items in pool");
+			FL_ERR(_log, LMsg("Tried to get with index ") << index << ", only " << m_entries.size() << " items in pool " + m_name);
 			throw IndexOverflow( __FUNCTION__ );
 		}
 		IResource* res = NULL;
@@ -104,15 +111,20 @@
 			} else {
 				entry->resource = entry->loader->loadResource(*entry->location);
 			}
+
 			if (!entry->loader) {
 				LMsg msg("No suitable loader was found for resource ");
-				msg << entry->location->getFilename();
+				msg << "#" << index << "<" << entry->location->getFilename()
+				    << "> in pool " << m_name;
 				FL_ERR(_log, msg);
+	      
 				throw NotFound(msg.str);
 			}
+
 			if (!entry->resource) {
 				LMsg msg("No loader was able to load the requested resource ");
-				msg << entry->location->getFilename();
+				msg << "#" << index << "<" << entry->location->getFilename()
+				    << "> in pool " << m_name;
 				FL_ERR(_log, msg);
 				throw NotFound(msg.str);
 			}
@@ -125,19 +137,7 @@
 		return *res;
 	}
 	
-	unsigned int Pool::getIndex(const std::string& filename) {
-		std::vector<PoolEntry*>::iterator it = m_entries.begin();
-		int index = 0;
-		
-		// look for the appropriate entry
-		for (; it != m_entries.end(); it++) {
-			
-				if ((*it)->location->getFilename() == filename) {
-					return index;
-				}
-			index++;
-		}
-		
+	int Pool::getIndex(const std::string& filename) {
 		// create resource
 		return addResourceFromFile(filename);
 	}
@@ -213,6 +213,16 @@
 	void Pool::printStatistics() {
 		FL_LOG(_log, LMsg("Pool not loaded =") << getResourceCount(RES_NON_LOADED));
 		FL_LOG(_log, LMsg("Pool loaded     =") << getResourceCount(RES_LOADED));
+		int amount = 0;
+		std::vector<PoolEntry*>::iterator entry;
+		for (entry = m_entries.begin(); entry != m_entries.end(); entry++) {
+			if ((*entry)->resource) {
+				if ((*entry)->resource->getRefCount() > 0) {
+					amount++;
+				}
+			}
+		}
+		FL_LOG(_log, LMsg("Pool locked     =") << amount);
 		FL_LOG(_log, LMsg("Pool total size =") << m_entries.size());
 	}
 }
--- a/engine/core/util/resource/pool.h	Thu Oct 09 08:23:13 2008 +0000
+++ b/engine/core/util/resource/pool.h	Thu Oct 09 12:36:21 2008 +0000
@@ -67,7 +67,7 @@
 
 		/** Default constructor.
 		 */
-		Pool();
+		Pool(const std::string& name);
 
 		/** Destructor.
 		 */
@@ -100,7 +100,7 @@
 		/** Gets resource index from pool with given filename
 		 * The resource will be created if it is not in the pool
 		 */
-		virtual unsigned int getIndex(const std::string& filename);
+		virtual int getIndex(const std::string& filename);
 
 		/** Removes the resource from pool if reference counter is null
 		 * 
@@ -150,12 +150,14 @@
 		};
 
 		void findAndSetProvider(PoolEntry& entry);
+
 		std::vector<PoolEntry*> m_entries;
 		typedef std::map<ResourceLocation, int> ResourceLocationToEntry;
 		ResourceLocationToEntry m_location_to_entry;
 		std::vector<IPoolListener*> m_listeners;
 		std::vector<ResourceLoader*> m_loaders;
 		int m_curind;
+		std::string m_name;
 	};
 
 } // FIFE
--- a/engine/core/util/resource/resource_location.h	Thu Oct 09 08:23:13 2008 +0000
+++ b/engine/core/util/resource/resource_location.h	Thu Oct 09 12:36:21 2008 +0000
@@ -72,13 +72,7 @@
 		 *  This is needed as the locations should be stored in a \c std::map
 		 */
 		virtual bool operator <(const ResourceLocation& loc) const {
-			if (m_filename < loc.m_filename) {
-				return true;
-			}
-			if (m_filename != loc.m_filename) {
-				return false;
-			}
-			return true;
+			return m_filename < loc.m_filename;
 		}
 
 		/** Creates copy of this location
--- a/engine/core/video/animationpool.h	Thu Oct 09 08:23:13 2008 +0000
+++ b/engine/core/video/animationpool.h	Thu Oct 09 12:36:21 2008 +0000
@@ -43,7 +43,7 @@
 	public:
 		/** Default constructor.
 		 */
-		AnimationPool(): Pool() {
+		AnimationPool(const std::string& name = "AnimationPool"): Pool(name) {
 		}
 
 		/** Destructor.
--- a/engine/core/video/image_location.cpp	Thu Oct 09 08:23:13 2008 +0000
+++ b/engine/core/video/image_location.cpp	Thu Oct 09 12:36:21 2008 +0000
@@ -45,7 +45,7 @@
 		}
 		const ImageLocation* r = dynamic_cast<const ImageLocation*>(&loc);
 		if (!r) {
-			return true;
+			return false;
 		}
 		
 		if (m_xshift != r->m_xshift) {
@@ -72,6 +72,10 @@
 		}
 
 		const ImageLocation* r = dynamic_cast<const ImageLocation*>(&loc);
+		if (!r) {
+			return true;
+		}
+
 		if (m_xshift < r->m_xshift) {
 			return false;
 		}
--- a/engine/core/video/imagepool.h	Thu Oct 09 08:23:13 2008 +0000
+++ b/engine/core/video/imagepool.h	Thu Oct 09 12:36:21 2008 +0000
@@ -43,7 +43,7 @@
 	public:
 		/** Default constructor.
 		 */
-		ImagePool(): Pool() {
+		ImagePool(const std::string& name = "ImagePool"): Pool(name) {
 		}
 
 		/** Destructor.
--- a/engine/extensions/loaders.py	Thu Oct 09 08:23:13 2008 +0000
+++ b/engine/extensions/loaders.py	Thu Oct 09 12:36:21 2008 +0000
@@ -23,7 +23,7 @@
 	"""
 	map_loader = XMLMapLoader(engine, callback)
 	map = map_loader.loadResource(fife.ResourceLocation(path))
-	#print "--- Loading map took: ", map_loader.time_to_load, " seconds."
+	print "--- Loading map took: ", map_loader.time_to_load, " seconds."
 	return map