summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorThorbjørn Lindeijer <thorbjorn@lindeijer.nl>2012-02-08 22:09:07 +0100
committerThorbjørn Lindeijer <thorbjorn@lindeijer.nl>2012-02-09 23:47:51 +0100
commit32996cee607c52ecef9be4638df554dd89b39c24 (patch)
treeb3c9987ea39b52e46e5969f938c3aa9002a4bd4c
parent8aeb42b16430c85e4bc4052d881b8335d4a2ff36 (diff)
downloadmana-32996cee607c52ecef9be4638df554dd89b39c24.tar.gz
mana-32996cee607c52ecef9be4638df554dd89b39c24.tar.bz2
mana-32996cee607c52ecef9be4638df554dd89b39c24.tar.xz
mana-32996cee607c52ecef9be4638df554dd89b39c24.zip
Fixed wallpaper prescaling issues
Image::SDLgetScaledImage was changed so that it tries to find an existing scaled version of the image first, and generates it when none exists. When it needs to generate one, this resource is added to the resource manager, partly to avoid duplicating the work later but mainly to keep memory management straightforward. This function also used to leak the scaled SDL_Surface since it wrongly assumed that Image::load would free it. To avoid filling up the memory with scaled wallpapers that are waiting 30 seconds until they will be deleted, the Resource::decRef function was extended with a parameter that allows telling it what to do with orphans. Calling decRef with Resource::DeleteImmediately will delete the resource immediately in case the resource is orphaned. Reviewed-by: Yohann Ferreira
-rw-r--r--src/graphics.cpp50
-rw-r--r--src/gui/widgets/desktop.cpp43
-rw-r--r--src/resources/ambientlayer.cpp10
-rw-r--r--src/resources/image.cpp49
-rw-r--r--src/resources/image.h9
-rw-r--r--src/resources/resource.cpp25
-rw-r--r--src/resources/resource.h23
-rw-r--r--src/resources/resourcemanager.cpp28
-rw-r--r--src/resources/resourcemanager.h27
9 files changed, 174 insertions, 90 deletions
diff --git a/src/graphics.cpp b/src/graphics.cpp
index dfdef349..7b56d974 100644
--- a/src/graphics.cpp
+++ b/src/graphics.cpp
@@ -161,14 +161,16 @@ bool Graphics::drawRescaledImage(Image *image, int srcX, int srcY,
bool useColor)
{
// Check that preconditions for blitting are met.
- if (!mTarget || !image) return false;
- if (!image->mSDLSurface) return false;
+ if (!mTarget || !image)
+ return false;
+ if (!image->mSDLSurface)
+ return false;
Image *tmpImage = image->SDLgetScaledImage(desiredWidth, desiredHeight);
bool returnValue = false;
- if (!tmpImage) return false;
- if (!tmpImage->mSDLSurface) return false;
+ if (!tmpImage)
+ return false;
dstX += mClipStack.top().xOffset;
dstY += mClipStack.top().yOffset;
@@ -183,9 +185,10 @@ bool Graphics::drawRescaledImage(Image *image, int srcX, int srcY,
srcRect.w = width;
srcRect.h = height;
- returnValue = !(SDL_BlitSurface(tmpImage->mSDLSurface, &srcRect, mTarget, &dstRect) < 0);
+ returnValue = !(SDL_BlitSurface(tmpImage->mSDLSurface, &srcRect,
+ mTarget, &dstRect) < 0);
- delete tmpImage;
+ tmpImage->decRef(Resource::DeleteImmediately);
return returnValue;
}
@@ -228,13 +231,16 @@ void Graphics::drawImage(gcn::Image const *image, int srcX, int srcY,
void Graphics::drawImagePattern(Image *image, int x, int y, int w, int h)
{
// Check that preconditions for blitting are met.
- if (!mTarget || !image) return;
- if (!image->mSDLSurface) return;
+ if (!mTarget || !image)
+ return;
+ if (!image->mSDLSurface)
+ return;
const int iw = image->getWidth();
const int ih = image->getHeight();
- if (iw == 0 || ih == 0) return;
+ if (iw == 0 || ih == 0)
+ return;
for (int py = 0; py < h; py += ih) // Y position on pattern plane
{
@@ -259,22 +265,32 @@ void Graphics::drawImagePattern(Image *image, int x, int y, int w, int h)
}
}
-void Graphics::drawRescaledImagePattern(Image *image, int x, int y,
- int w, int h, int scaledWidth, int scaledHeight)
+void Graphics::drawRescaledImagePattern(Image *image,
+ int x, int y,
+ int w, int h,
+ int scaledWidth, int scaledHeight)
{
// Check that preconditions for blitting are met.
- if (!mTarget || !image) return;
- if (!image->mSDLSurface) return;
+ if (!mTarget || !image)
+ return;
+ if (!image->mSDLSurface)
+ return;
- if (scaledHeight == 0 || scaledWidth == 0) return;
+ if (scaledHeight == 0 || scaledWidth == 0)
+ return;
Image *tmpImage = image->SDLgetScaledImage(scaledWidth, scaledHeight);
- if (!tmpImage) return;
+ if (!tmpImage)
+ return;
const int iw = tmpImage->getWidth();
const int ih = tmpImage->getHeight();
- if (iw == 0 || ih == 0) return;
+ if (iw == 0 || ih == 0)
+ {
+ tmpImage->decRef(Resource::DeleteImmediately);
+ return;
+ }
for (int py = 0; py < h; py += ih) // Y position on pattern plane
{
@@ -298,7 +314,7 @@ void Graphics::drawRescaledImagePattern(Image *image, int x, int y,
}
}
- delete tmpImage;
+ tmpImage->decRef(Resource::DeleteImmediately);
}
void Graphics::drawImageRect(int x, int y, int w, int h,
diff --git a/src/gui/widgets/desktop.cpp b/src/gui/widgets/desktop.cpp
index bf833898..baf3e951 100644
--- a/src/gui/widgets/desktop.cpp
+++ b/src/gui/widgets/desktop.cpp
@@ -105,30 +105,39 @@ void Desktop::draw(gcn::Graphics *graphics)
void Desktop::setBestFittingWallpaper()
{
- const std::string wallpaperName =
- Wallpaper::getWallpaper(getWidth(), getHeight());
+ const int width = getWidth();
+ const int height = getHeight();
- Image *nWallPaper = ResourceManager::getInstance()->getImage(wallpaperName);
+ if (width == 0 || height == 0)
+ return;
- if (nWallPaper)
+ const std::string wallpaperName = Wallpaper::getWallpaper(width, height);
+
+ if (wallpaperName.empty())
+ return;
+
+ ResourceManager *resman = ResourceManager::getInstance();
+ Image *wallpaper = resman->getImage(wallpaperName);
+
+ if (wallpaper)
{
if (mWallpaper)
- mWallpaper->decRef();
+ mWallpaper->decRef(Resource::DeleteImmediately);
+
+ mWallpaper = wallpaper;
- if (!nWallPaper->useOpenGL() && (nWallPaper->getWidth() != getWidth()
- || nWallPaper->getHeight() != getHeight()))
+ // In software mode we try to prescale the image for performance
+ if (!wallpaper->useOpenGL() &&
+ (wallpaper->getWidth() != width ||
+ wallpaper->getHeight() != height))
{
- // We rescale to obtain a fullscreen wallpaper...
- Image *newRsclWlPpr = nWallPaper->SDLgetScaledImage(getWidth(), getHeight());
- std::string idPath = nWallPaper->getIdPath();
-
- // We replace the resource in the resource manager
- nWallPaper->decRef();
- ResourceManager::getInstance()->addResource(idPath, newRsclWlPpr);
- mWallpaper = newRsclWlPpr;
+ if (Image *prescaled = wallpaper->SDLgetScaledImage(width, height))
+ {
+ // Make sure the original can be freed
+ wallpaper->decRef();
+ mWallpaper = prescaled;
+ }
}
- else
- mWallpaper = nWallPaper;
}
else
{
diff --git a/src/resources/ambientlayer.cpp b/src/resources/ambientlayer.cpp
index d1fc93d3..c31afbac 100644
--- a/src/resources/ambientlayer.cpp
+++ b/src/resources/ambientlayer.cpp
@@ -32,6 +32,7 @@ AmbientLayer::AmbientLayer(Image *img, float parallax,
mSpeedX(speedX), mSpeedY(speedY),
mKeepRatio(keepRatio)
{
+ mImage->incRef();
if (keepRatio && !mImage->useOpenGL()
&& defaultScreenWidth != 0
@@ -47,17 +48,10 @@ AmbientLayer::AmbientLayer(Image *img, float parallax,
if (rescaledOverlay)
{
- // Replace the resource with the new one...
- std::string idPath = mImage->getIdPath() + "_rescaled";
- ResourceManager::getInstance()->addResource(idPath, rescaledOverlay);
+ mImage->decRef();
mImage = rescaledOverlay;
- rescaledOverlay->incRef();
}
- else
- mImage->incRef();
}
- else
- mImage->incRef();
}
AmbientLayer::~AmbientLayer()
diff --git a/src/resources/image.cpp b/src/resources/image.cpp
index 3a6f7236..975bd647 100644
--- a/src/resources/image.cpp
+++ b/src/resources/image.cpp
@@ -326,31 +326,52 @@ void Image::setAlpha(float alpha)
}
}
-Image* Image::SDLgetScaledImage(int width, int height)
+Image *Image::SDLgetScaledImage(int width, int height)
{
- // No scaling on incorrect new values.
if (width == 0 || height == 0)
- return NULL;
+ return 0;
- // No scaling when there is ... no different given size ...
+ // Increase our reference count and return ourselves in case of same size
if (width == getWidth() && height == getHeight())
- return NULL;
+ {
+ incRef();
+ return this;
+ }
- Image* scaledImage = NULL;
- SDL_Surface* scaledSurface = NULL;
+ if (!mSDLSurface)
+ return 0;
- if (mSDLSurface)
+ ResourceManager *resman = ResourceManager::getInstance();
+
+ // Generate a unique ID path for storing the scaled version in the
+ // resource manager.
+ std::string idPath = getIdPath();
+ idPath += ":scaled:";
+ idPath += toString(width);
+ idPath += "x";
+ idPath += toString(height);
+
+ // Try whether a scaled version is already available
+ Image *scaledImage = static_cast<Image*>(resman->get(idPath));
+
+ if (!scaledImage)
{
- scaledSurface = zoomSurface(mSDLSurface,
- (double) width / getWidth(),
- (double) height / getHeight(),
- 1);
+ // No scaled version with this size exists already, so create one
+ SDL_Surface *scaledSurface = zoomSurface(mSDLSurface,
+ (double) width / getWidth(),
+ (double) height / getHeight(),
+ 1);
- // The load function takes care of the SDL<->OpenGL implementation
- // and about freeing the given SDL_surface*.
if (scaledSurface)
+ {
scaledImage = load(scaledSurface);
+ SDL_FreeSurface(scaledSurface);
+
+ // Place the scaled image in the resource manager
+ resman->addResource(idPath, scaledImage);
+ }
}
+
return scaledImage;
}
diff --git a/src/resources/image.h b/src/resources/image.h
index b826112d..7df74c3f 100644
--- a/src/resources/image.h
+++ b/src/resources/image.h
@@ -147,14 +147,15 @@ class Image : public Resource
{ return mDisableTransparency; }
/**
- * Gets an scaled instance of an image.
+ * Gets an scaled instance of an image. The returned image is managed
+ * by the ResourceManager.
*
- * @param width The desired width of the scaled image.
+ * @param width The desired width of the scaled image.
* @param height The desired height of the scaled image.
*
- * @return A new Image* object.
+ * @return An Image resource, or 0 on failure.
*/
- Image* SDLgetScaledImage(int width, int height);
+ Image *SDLgetScaledImage(int width, int height);
/**
* Get the alpha Channel of a SDL surface.
diff --git a/src/resources/resource.cpp b/src/resources/resource.cpp
index 4f6a2519..cdff8060 100644
--- a/src/resources/resource.cpp
+++ b/src/resources/resource.cpp
@@ -27,16 +27,7 @@
#include <cassert>
-Resource::~Resource()
-{
-}
-
-void Resource::incRef()
-{
- ++mRefCount;
-}
-
-void Resource::decRef()
+void Resource::decRef(OrphanPolicy orphanPolicy)
{
// Reference may not already have reached zero
if (mRefCount == 0) {
@@ -48,8 +39,18 @@ void Resource::decRef()
if (mRefCount == 0)
{
- // Warn the manager that this resource is no longer used.
ResourceManager *resman = ResourceManager::getInstance();
- resman->release(this);
+
+ switch (orphanPolicy)
+ {
+ case DeleteLater:
+ default:
+ resman->release(this);
+ break;
+ case DeleteImmediately:
+ resman->remove(this);
+ delete this;
+ break;
+ }
}
}
diff --git a/src/resources/resource.h b/src/resources/resource.h
index 53b05ee8..af688eb0 100644
--- a/src/resources/resource.h
+++ b/src/resources/resource.h
@@ -33,21 +33,26 @@ class Resource
friend class ResourceManager;
public:
- Resource(): mRefCount(0) {}
+ enum OrphanPolicy {
+ DeleteLater,
+ DeleteImmediately
+ };
+
+ Resource():
+ mRefCount(0)
+ {}
/**
* Increments the internal reference count.
*/
- void incRef();
+ void incRef() { ++mRefCount; }
/**
- * Decrements the reference count and deletes the object
- * if no references are left.
- *
- * @return <code>true</code> if the object was deleted
- * <code>false</code> otherwise.
+ * Decrements the reference count. When no references are left, either
+ * schedules the object for deletion or deletes it immediately,
+ * depending on the \a orphanPolicy.
*/
- void decRef();
+ void decRef(OrphanPolicy orphanPolicy = DeleteLater);
/**
* Return the path identifying this resource.
@@ -56,7 +61,7 @@ class Resource
{ return mIdPath; }
protected:
- virtual ~Resource();
+ virtual ~Resource() {}
private:
std::string mIdPath; /**< Path identifying this resource. */
diff --git a/src/resources/resourcemanager.cpp b/src/resources/resourcemanager.cpp
index 15d7c8eb..a9e7e565 100644
--- a/src/resources/resourcemanager.cpp
+++ b/src/resources/resourcemanager.cpp
@@ -117,9 +117,11 @@ void ResourceManager::cleanOrphans()
timeval tv;
gettimeofday(&tv, NULL);
// Delete orphaned resources after 30 seconds.
- time_t oldest = tv.tv_sec, threshold = oldest - 30;
+ time_t oldest = tv.tv_sec;
+ time_t threshold = oldest - 30;
- if (mOrphanedResources.empty() || mOldestOrphan >= threshold) return;
+ if (mOrphanedResources.empty() || mOldestOrphan >= threshold)
+ return;
ResourceIterator iter = mOrphanedResources.begin();
while (iter != mOrphanedResources.end())
@@ -128,7 +130,8 @@ void ResourceManager::cleanOrphans()
time_t t = res->mTimeStamp;
if (t >= threshold)
{
- if (t < oldest) oldest = t;
+ if (t < oldest)
+ oldest = t;
++iter;
}
else
@@ -234,6 +237,17 @@ bool ResourceManager::addResource(const std::string &idPath,
return false;
}
+Resource *ResourceManager::get(const std::string &idPath)
+{
+ ResourceIterator resIter = mResources.find(idPath);
+ if (resIter != mResources.end())
+ {
+ resIter->second->incRef();
+ return resIter->second;
+ }
+ return 0;
+}
+
Resource *ResourceManager::get(const std::string &idPath, generator fun,
void *data)
{
@@ -392,12 +406,18 @@ void ResourceManager::release(Resource *res)
time_t timestamp = tv.tv_sec;
res->mTimeStamp = timestamp;
- if (mOrphanedResources.empty()) mOldestOrphan = timestamp;
+ if (mOrphanedResources.empty())
+ mOldestOrphan = timestamp;
mOrphanedResources.insert(*resIter);
mResources.erase(resIter);
}
+void ResourceManager::remove(Resource *res)
+{
+ mResources.erase(res->mIdPath);
+}
+
ResourceManager *ResourceManager::getInstance()
{
// Create a new instance if necessary.
diff --git a/src/resources/resourcemanager.h b/src/resources/resourcemanager.h
index 5295be5c..5abc81e2 100644
--- a/src/resources/resourcemanager.h
+++ b/src/resources/resourcemanager.h
@@ -107,6 +107,16 @@ class ResourceManager
std::string getPath(const std::string &file);
/**
+ * Returns the resource with the specified idPath, or 0 when no such
+ * resource is available. Increments the reference count.
+ *
+ * @param idPath The resource identifier path.
+ * @return A valid resource or <code>NULL</code> if the resource could
+ * not be found.
+ */
+ Resource *get(const std::string &idPath);
+
+ /**
* Creates a resource and adds it to the resource map.
*
* @param idPath The resource identifier path.
@@ -178,11 +188,6 @@ class ResourceManager
SpriteDef *getSprite(const std::string &path, int variant = 0);
/**
- * Releases a resource, placing it in the set of orphaned resources.
- */
- void release(Resource *);
-
- /**
* Allocates data into a buffer pointer for raw data loading. The
* returned data is expected to be freed using <code>free()</code>.
*
@@ -225,6 +230,18 @@ class ResourceManager
private:
/**
+ * Releases a resource, placing it in the set of orphaned resources.
+ * Only called from Resource::decRef,
+ */
+ void release(Resource *);
+
+ /**
+ * Removes a resource from the list of resources managed by the
+ * resource manager. Only called from Resource::decRef,
+ */
+ void remove(Resource *);
+
+ /**
* Deletes the resource after logging a cleanup message.
*/
static void cleanUp(Resource *resource);