Further cleanups to texture caching, from bug 523:

- Texture::filename was used for only one purpose, so we might as well
  make that clear.

- Set the filename in just one place: before a texture is added to the
  cache.  This is intended to be obviously correct.

- Other minor adjustments.

Thanks to Matt McCutchen for this patch.

SVN-Revision: 6294

diff --git a/src/video/texture.hpp b/src/video/texture.hpp
index a4beb86..e814547 100644 (file)
--- a/src/video/texture.hpp
+++ b/src/video/texture.hpp
@@ -43,15 +43,20 @@ enum DrawingEffect {
  */
 class Texture
 {
-protected:
-  std::string filename;
+private:
+  friend class TextureManager;
+  /* The name under which this texture is cached by the texture manager,
+   * or the empty string if not. */
+  std::string cache_filename;
 
 public:
-  Texture() : filename() {}
+  Texture() : cache_filename() {}
   virtual ~Texture() 
   {
-    if (texture_manager)
-      texture_manager->release(this);
+    if (texture_manager && cache_filename != "")
+      /* The cache entry is now useless: its weak pointer to us has been
+       * cleared.  Remove the entry altogether to save memory. */
+      texture_manager->reap_cache_entry(cache_filename);
   }
 
   virtual unsigned int get_texture_width() const = 0;
@@ -59,16 +64,6 @@ public:
   virtual unsigned int get_image_width() const = 0;
   virtual unsigned int get_image_height() const = 0;
 
-  std::string get_filename() const
-  {
-    return filename;
-  }
-
-  void set_filename(std::string filename)
-  {
-    this->filename = filename;
-  }
-
 private:
   Texture(const Texture&);
   Texture& operator=(const Texture&);

diff --git a/src/video/texture_manager.cpp b/src/video/texture_manager.cpp
index 83313f3..7343a45 100644 (file)
--- a/src/video/texture_manager.cpp
+++ b/src/video/texture_manager.cpp
@@ -47,7 +47,7 @@ TextureManager::~TextureManager()
 {
   for(ImageTextures::iterator i = image_textures.begin(); i != image_textures.end(); ++i)
   {
-    if(i->second.lock())
+    if(!i->second.expired())
     {
       log_warning << "Texture '" << i->first << "' not freed" << std::endl;
     }
@@ -67,7 +67,8 @@ TextureManager::get(const std::string& _filename)
 
   if(!texture) {
     texture = create_image_texture(filename);
-    image_textures[texture->get_filename()] = texture;
+    texture->cache_filename = filename;
+    image_textures[filename] = texture;
   }
 
   return texture;
@@ -81,9 +82,12 @@ TextureManager::get(const std::string& filename, const Rect& rect)
 }
 
 void
-TextureManager::release(Texture* texture)
+TextureManager::reap_cache_entry(const std::string& filename)
 {
-  image_textures.erase(texture->get_filename());
+  ImageTextures::iterator i = image_textures.find(filename);
+  assert(i != image_textures.end());
+  assert(i->second.expired());
+  image_textures.erase(i);
 }
 
 #ifdef HAVE_OPENGL
@@ -110,9 +114,7 @@ TextureManager::create_image_texture(const std::string& filename, const Rect& re
   catch(const std::exception& err)
   {
     log_warning << "Couldn't load texture '" << filename << "' (now using dummy texture): " << err.what() << std::endl;
-    TexturePtr texture = create_dummy_texture();
-    texture->set_filename(filename);
-    return texture;
+    return create_dummy_texture();
   }
 }
 
@@ -150,9 +152,7 @@ TextureManager::create_image_texture_raw(const std::string& filename, const Rect
         SDL_SetColors(subimage.get(), image->format->palette->colors, 0, image->format->palette->ncolors);
       }
 
-      TexturePtr result = VideoSystem::new_texture(subimage.get());
-      result->set_filename(filename);
-      return result;
+      return VideoSystem::new_texture(subimage.get());
     }
   }
 }
@@ -167,9 +167,7 @@ TextureManager::create_image_texture(const std::string& filename)
   catch (const std::exception& err)
   {
     log_warning << "Couldn't load texture '" << filename << "' (now using dummy texture): " << err.what() << std::endl;
-    TexturePtr texture = create_dummy_texture();
-    texture->set_filename(filename);
-    return texture;
+    return create_dummy_texture();
   }
 }
 
@@ -185,9 +183,7 @@ TextureManager::create_image_texture_raw(const std::string& filename)
   }
   else
   {
-    TexturePtr result = VideoSystem::new_texture(image.get());
-    result->set_filename(filename);
-    return result;
+    return VideoSystem::new_texture(image.get());
   }
 }
 
@@ -213,10 +209,8 @@ TextureManager::create_dummy_texture()
     }
     else
     {
-      TexturePtr result = VideoSystem::new_texture(image.get());
-      result->set_filename("-dummy-texture-.png");
       log_warning << "Couldn't load texture '" << dummy_texture_fname << "' (now using empty one): " << err.what() << std::endl;
-      return result;
+      return VideoSystem::new_texture(image.get());
     }
   }
 }

diff --git a/src/video/texture_manager.hpp b/src/video/texture_manager.hpp
index e88223d..61350fb 100644 (file)
--- a/src/video/texture_manager.hpp
+++ b/src/video/texture_manager.hpp
@@ -51,7 +51,7 @@ public:
 
 private:
   friend class Texture;
-  void release(Texture* texture);
+  void reap_cache_entry(const std::string& filename);
 
   typedef std::map<std::string, boost::weak_ptr<Texture> > ImageTextures;
   ImageTextures image_textures;