From c54ca8bf402c1b021958eb8989f5fb90d4389de4 Mon Sep 17 00:00:00 2001
From: Sam Hocevar <sam@hocevar.net>
Date: Wed, 6 Mar 2013 15:20:00 +0000
Subject: [PATCH] image: try to reuse existing images when loading them, and
don't unload tilesets yet, but make sure the architecture will allow it.
---
src/image/image-private.h | 34 +++++++++-------
src/image/image.cpp | 81 +++++++++++++++++++++++++++++++--------
src/lol/image/image.h | 2 +
3 files changed, 88 insertions(+), 29 deletions(-)
diff --git a/src/image/image-private.h b/src/image/image-private.h
index 77a7b60d..439649fb 100644
--- a/src/image/image-private.h
+++ b/src/image/image-private.h
@@ -19,17 +19,17 @@
namespace lol
{
-class ImageLoader
+class ImageCodec
{
- friend class Image;
+ friend class ImageLoader;
friend class ImageData;
public:
ImageData *(*fun)(char const *path);
- ImageLoader *next;
+ ImageCodec *next;
int priority;
- static void RegisterLoader(ImageLoader *loader)
+ static void RegisterLoader(ImageCodec *loader)
{
Helper(loader);
}
@@ -37,7 +37,7 @@ public:
private:
static ImageData *Load(char const *path)
{
- ImageLoader *parser = Helper(nullptr);
+ ImageCodec *parser = Helper(nullptr);
ImageData *ret = nullptr;
while (parser && !ret)
@@ -49,14 +49,14 @@ private:
return ret;
}
- static ImageLoader *Helper(ImageLoader *set)
+ static ImageCodec *Helper(ImageCodec *set)
{
- static ImageLoader *loaders = nullptr;
+ static ImageCodec *loaders = nullptr;
if (!set)
return loaders;
- ImageLoader **parser = &loaders;
+ ImageCodec **parser = &loaders;
while (*parser && (*parser)->priority > set->priority)
parser = &(*parser)->next;
set->next = *parser;
@@ -69,8 +69,15 @@ private:
class ImageData
{
friend class Image;
+ friend class ImageLoader;
public:
+ inline ImageData()
+ : m_size(0, 0),
+ m_format(PixelFormat::Unknown),
+ m_refcount(0)
+ { }
+
virtual ~ImageData() {}
virtual bool Open(char const *) = 0;
@@ -81,6 +88,7 @@ public:
protected:
ivec2 m_size;
PixelFormat m_format;
+ int m_refcount;
};
#define REGISTER_IMAGE_LOADER(name) \
@@ -88,12 +96,12 @@ protected:
Register##name();
#define DECLARE_IMAGE_LOADER(name, prio) \
- template<typename T> class name##ImageLoader : public ImageLoader \
+ template<typename T> class name##ImageCodec : public ImageCodec \
{ \
public: \
- name##ImageLoader() \
+ name##ImageCodec() \
{ \
- static ImageLoader loader; \
+ static ImageCodec loader; \
loader.fun = Load; \
loader.priority = prio; \
RegisterLoader(&loader); \
@@ -110,10 +118,10 @@ protected:
} \
}; \
class name; \
- name##ImageLoader<name> name##ImageLoaderInstance; \
+ name##ImageCodec<name> name##ImageCodecInstance; \
void Register##name() \
{ \
- (void)&name##ImageLoaderInstance; \
+ (void)&name##ImageCodecInstance; \
} \
class name : public ImageData
diff --git a/src/image/image.cpp b/src/image/image.cpp
index 2d2828ce..9d3c655c 100644
--- a/src/image/image.cpp
+++ b/src/image/image.cpp
@@ -20,18 +20,18 @@ using namespace std;
namespace lol
{
+/* HACK: We cannot make this an ImageCodec member function, because the
+ * REGISTER_IMAGE_LOADER macro forward-declares free functions from
+ * the "lol" namespace. An apparent bug in Visual Studio's compiler
+ * makes it think these functions are actually in the top-level
+ * namespace when the forward declaration is in a class member function.
+ * To avoid the problem, we make the forward declaration in a free
+ * function.
+ * The bug was reported to Microsoft and fixed by them, but the fix
+ * is not yet available.
+ * https://connect.microsoft.com/VisualStudio/feedback/details/730878/ */
static bool RegisterAllLoaders()
{
- /* HACK: We cannot make this an ImageLoader member function, because the
- * REGISTER_IMAGE_LOADER macro forward-declares free functions from
- * the "lol" namespace. An apparent bug in Visual Studio's compiler
- * makes it think these functions are actually in the top-level
- * namespace when the forward declaration is in a class member function.
- * To avoid the problem, we make the forward declaration in a free
- * function.
- * The bug was reported to Microsoft and fixed by them, but the fix
- * is not yet available.
- * https://connect.microsoft.com/VisualStudio/feedback/details/730878/ */
#if defined __ANDROID__
REGISTER_IMAGE_LOADER(AndroidImageData)
#endif
@@ -53,23 +53,72 @@ static bool RegisterAllLoaders()
}
/*
- * Static image loader
+ * Our static image loader
*/
-Image *Image::Create(char const *path)
+static class ImageLoader
+{
+public:
+ Image *Create(char const *path);
+ void Destroy(Image *img);
+
+private:
+ Map<String, Image *> m_images;
+}
+image_loader;
+
+Image *ImageLoader::Create(char const *path)
{
/* Initialise loaders (see above) */
static bool init = RegisterAllLoaders();
UNUSED(init);
- Image *ret = new Image();
- ret->m_data = ImageLoader::Load(path);
- return ret;
+ /* Is our image already in the bank? If so, no need to create it. */
+ Image *img;
+
+ if (m_images.HasKey(path))
+ {
+ img = m_images[path];
+ }
+ else
+ {
+ img = new Image();
+ img->m_data = ImageCodec::Load(path);
+ m_images[path] = img;
+ }
+
+ ++img->m_data->m_refcount;
+ return img;
+}
+
+void ImageLoader::Destroy(Image *img)
+{
+ ASSERT(img->m_data->m_refcount > 0);
+ if (--img->m_data->m_refcount == 0)
+ {
+ /* FIXME: unload images etc. here */
+ /* XXX: we’re gonna hit a problem here because we didn’t keep
+ * the image path within the object, so unless we store it
+ * ourselves we’re good for a O(n) lookup… which we can’t do
+ * on Map objects anyway. */
+ /* TODO: 1. remove image from Map
+ 2. delete img; */
+ }
+}
+
+
+/*
+ * Static image methods
+ */
+
+Image *Image::Create(char const *path)
+{
+ return image_loader.Create(path);
}
void Image::Destroy(Image *img)
{
- delete img;
+ return image_loader.Destroy(img);
}
/*
diff --git a/src/lol/image/image.h b/src/lol/image/image.h
index ea6499b3..cc2491ea 100644
--- a/src/lol/image/image.h
+++ b/src/lol/image/image.h
@@ -23,6 +23,8 @@ namespace lol
class Image
{
+ friend class ImageLoader;
+
public:
static Image *Create(char const *path);
static void Destroy(Image *img);