Loading images

Started by
9 comments, last by Mybowlcut 15 years, 9 months ago
Hey. I have a several classes that all derive from XBase_Image. They all load images from a .png file. My issue is that the way that these objects deal with the surface of the image that was loaded when they are assigned to another object... for example. XImage x1 (a class that derives from XBase_Image) might load an_image.png and then XImage x2 is created from x1 (copy constructed). x2 will take everything of x1's except the surface.. which it re-loads from the file name stored in x1. That's my problem.. I need to find a way to stop re-loading the image if it has already been loaded elsewhere. I was thinking of using maybe a shared pointer or some Surface_Manager class or something? Anyone have any ideas of how to solve this problem? Cheers!

Advertisement
The way i handle this in my engine is that i have an build in database. And each ImageHandle contains an id instead of a handle to the image. And whenever i need to know the image that is currently being reference i can just use the image index on the database and that returns an handle to the image. That way all your images are loaded once and stored in one location.
Quote:Original post by BornToCode
The way i handle this in my engine is that i have an build in database. And each ImageHandle contains an id instead of a handle to the image. And whenever i need to know the image that is currently being reference i can just use the image index on the database and that returns an handle to the image. That way all your images are loaded once and stored in one location.
Sounds pretty good. What database do you use? How do you store the handle in a database? Is it just a string that contains a memory address? Could you elaborate on it a bit as it's a bit fuzzy to understand atm.

Cheers.

this is an quick examples.

class Database
{
public:
vector<Image*> images;
void LoadImage(const char* fname);
int GetImageIndex(const char* fname);
};

struct Actor
{
int imageid;
void LoadImage(const char* fname)
{
database.LoadImage(fname);
imageid = database.GetImageIndex(fname);
}
};

You should make the database class an singleton.
Also before loading an image into the database you can always scan and see
if the image is already loaded and you do not have to load it again.

So in reality your database is just an image manager.
That looks interesting! Thanks for that. I'm gonna go mess around with it now.

Cheers. :)

I'm using a map instead of a vector at the moment and I'm just wondering what would be faster... because each time I retrieve the surface from the map, a search has to be done to ensure it exists.. if it doesn't then an exception is thrown.

I'm thinking it might be quicker to use a vector and just return the element at the passed in index(even if it doesn't exist), but then I'd have to use an int for the key (currently using a string)...

I guess I *could* add a surface_id or something to Base_XImage...

You can combine the caching and the shared pointers:
class Image : boost::noncopyable{public:    Image( const std::string &filename );    // ...};class ImageCache{public:    typedef boost::shared_ptr<Image> ImagePtr;    typedef std::map<std::string,ImagePtr> Cache;    ImagePtr load( const std::string & filename )    {        Cache::iterator it = cached.find(filename);        if( it == cached.end() )        {            // note: the Image constructor could throw an exception            // to handle "couldn't load image" error.            ImagePtr image( new Image(filename) );            cache.insert(std::make_pair(filename,image);            return image;        }        return it->second;    }private:    Cache cached;};


It helps if the Image class is immutable.

Don't bother with Singletons for such a class. If you want you could make a global instance of the ImageCache. Just be careful because using a global will mean the map with the shared_ptrs will be destructed after main(), so be careful what you decide to do in the Image destructor. An alternative is to keep weak pointers in the map, but that could lead to an image being loaded from file more than once in the lifetime of the program.
Quote:Original post by rip-off
You can combine the caching and the shared pointers:
*** Source Snippet Removed ***

It helps if the Image class is immutable.

Don't bother with Singletons for such a class. If you want you could make a global instance of the ImageCache. Just be careful because using a global will mean the map with the shared_ptrs will be destructed after main(), so be careful what you decide to do in the Image destructor. An alternative is to keep weak pointers in the map, but that could lead to an image being loaded from file more than once in the lifetime of the program.
I'm not sure if this is frowned upon... but I'm using the Surface_Manager (or ImageCache in your terms) in a static class called Draw_Engine:
/*	Universal class. Edits affect multiple projects.*/#ifndef DRAW_ENGINE#define DRAW_ENGINE//test#include <algorithm>#include <map>#include <string>#include <iostream>#include <exception>#include "SDL/SDL.h"#include "SDL/SDL_image.h"#include "SDL/SDL_ttf.h"#include "Point.h"#include "SDL_Tools.h"#include "Uncopyable.h"const Uint8 DEFAULT_FC_R = 0;const Uint8 DEFAULT_FC_G = 0;const Uint8 DEFAULT_FC_B = 0;// When autofit is specific in an Enhanced_Write_Options object,// Enhanced_Write sets the text size to the height of the bounds.// This can result in text exceeding the bounds, so we use this to trim the size.const double AUTOFIT_ADJ = 0.75;enum JUSTIFICATION {JUSTIFY_NONE, JUSTIFY_CENTRED, JUSTIFY_LEFT, JUSTIFY_RIGHT};struct Enhanced_Write_Options{	Enhanced_Write_Options();	Enhanced_Write_Options(		JUSTIFICATION justification,		int left_pad,		int right_pad,		const SDL_Rect& bounds,		const std::string& text,		int font_size,		const std::string& font_name,		SDL_Color colour = SDL_Tools::Colours::BLACK);	friend std::istream& operator>>(std::istream& stream, Enhanced_Write_Options& ewo);	JUSTIFICATION justification;	int left_pad, right_pad, font_size;	SDL_Rect bounds;	std::string text, font_name;	SDL_Color colour;};class Draw_Engine : public Uncopyable{public:	typedef unsigned int uint;	typedef unsigned short ushort;		static void Initialise(		ushort SCREEN_WIDTH,		ushort SCREEN_HEIGHT,		ushort SCREEN_BPP,		const std::string& SCREEN_CAPTION,		const std::string& FONT,		int FONT_SIZE);	static void Clean_Up();	// Blits a surface onto the screen.	static void Blit_Surface(	SDL_Rect position, const std::string& file_name, SDL_Rect* clip);	// Blits a surface onto the screen (Point variation).	static void Blit_Surface(		Point position, const std::string& file_name, SDL_Rect* clip);		static void New_Font(const std::string& file_name, int font_size);	static void Write(int x, int y, const std::string& text);	static void Write_Enhanced(const Enhanced_Write_Options& ewo);	static void Draw_Rect(SDL_Rect position, SDL_Color colour);		// Draws the screen.	static void Draw_Screen();	// Frees a surface.	static void Free_Surface(SDL_Surface*& surface);	// Loads a surface ready for blitting.	static void Load_Surface(const std::string& file_name, SDL_Colour* colour_key = NULL);protected:	class Surface_Manager	{	public:		Surface_Manager();		~Surface_Manager();		void Add_Surface(const std::string& file_name, SDL_Surface* surface);		bool Surface_Exists(const std::string& file_name) const;		SDL_Surface* Get_Surface(const std::string& file_name);	private:		typedef std::map<const std::string, SDL_Surface*>::iterator surface_it;		std::map<const std::string, SDL_Surface*> loaded_surfaces;	};		Draw_Engine() {}	static void Throw_If_Unitialised(); 	static bool initialised;		static Surface_Manager surface_manager;	static SDL_Surface* screen;	static SDL_Surface* text_sfc;	static TTF_Font* font;	static std::string font_name;	static int font_size;	static SDL_Colour font_colour;	static SDL_Rect blit_position;};#endif

/*	Universal class. Edits affect multiple projects.*/#include "stdafx.h"#include "Draw_Engine.h"#include "Directory.h"Enhanced_Write_Options::Enhanced_Write_Options()	:	justification(JUSTIFY_NONE),	left_pad(0),	right_pad(0),	bounds(SDL_Tools::EMPTY),	font_size(0),	colour(SDL_Tools::Colours::WHITE){}Enhanced_Write_Options::Enhanced_Write_Options(	JUSTIFICATION justification,	int left_pad,	int right_pad,	const SDL_Rect& bounds,	const std::string& text,	int font_size,	const std::string& font_name,	SDL_Color colour):	justification(justification),	left_pad(left_pad),	right_pad(right_pad),	bounds(bounds),	text(text),	font_size(font_size),	font_name(font_name),	colour(colour){}std::istream& operator>>(std::istream& stream, Enhanced_Write_Options& ewo){	std::string buffer;	int int_buf = 0;	stream >> buffer; // <	IO::Check_Opening_Tag(buffer);	stream >> int_buf;	ewo.justification = static_cast<JUSTIFICATION>(int_buf);		stream >> ewo.left_pad >> ewo.right_pad;	SDL_Tools::RW_SDL_Rect rwrect;	stream >> rwrect;	ewo.bounds = rwrect;	stream >> ewo.text;	stream >> ewo.font_size;	if(ewo.font_size > 0) stream >> ewo.font_name;	SDL_Tools::RW_SDL_Color rwcol;	stream >> rwcol;	ewo.colour = rwcol;	stream >> buffer; // >	IO::Check_Closing_Tag(buffer);	return stream;}bool Draw_Engine::initialised = false;Draw_Engine::Surface_Manager Draw_Engine::surface_manager;SDL_Surface* Draw_Engine::screen = NULL;SDL_Surface* Draw_Engine::text_sfc = NULL;TTF_Font* Draw_Engine::font = NULL;std::string Draw_Engine::font_name;int Draw_Engine::font_size = 0;SDL_Colour Draw_Engine::font_colour = SDL_Tools::colour(0,0,0);SDL_Rect Draw_Engine::blit_position = SDL_Tools::EMPTY;void Draw_Engine::Initialise(	ushort SCREEN_WIDTH,	ushort SCREEN_HEIGHT,	ushort SCREEN_BPP,	const std::string& SCREEN_CAPTION,	const std::string& FONT,	int FONT_SIZE){	if(initialised)	{ // Already initialised.		return;	}		font_name = FONT;	font_size = FONT_SIZE;	if(SDL_Init(SDL_INIT_EVERYTHING) == -1)	{		throw std::runtime_error(			std::string("SDL_Init failed: ") + std::string(SDL_GetError()));	}	screen = SDL_SetVideoMode(SCREEN_WIDTH, SCREEN_HEIGHT, SCREEN_BPP, SDL_SWSURFACE);	if(screen == NULL)	{		throw std::runtime_error(std::string("Error: SDL_SetVideoMode failed: " +		std::string(SDL_GetError())));	}	SDL_WM_SetCaption(SCREEN_CAPTION.c_str(), NULL);	//Initialize SDL_ttf    if(TTF_Init() == -1)	{		throw std::runtime_error(std::string("Error: TTF_Init failed: ") +			std::string(TTF_GetError()));	}	//Open the font    font = TTF_OpenFont(font_name.c_str(), font_size);        //If there was an error in loading the font    if(font == NULL)	{		throw std::runtime_error(		std::string("Error: TTF_OpenFont failed: ") +		std::string(TTF_GetError()));	}	//The color of the font	font_colour.r = DEFAULT_FC_R, font_colour.g = DEFAULT_FC_G, font_colour.b = DEFAULT_FC_B;	// Enable key repeating	if(SDL_EnableKeyRepeat(SDL_DEFAULT_REPEAT_DELAY, SDL_DEFAULT_REPEAT_INTERVAL) == -1)	{		throw std::runtime_error(		std::string("Error: SDL_EnableKeyRepeat failed: ") +		std::string(SDL_GetError()));	}	// Initialisation was successful	initialised = true;	}void Draw_Engine::Clean_Up(){	Throw_If_Unitialised();	//Free the text	SDL_FreeSurface(text_sfc);	//Close used font	TTF_CloseFont(font);	//Quit SDL_ttf	TTF_Quit();}void Draw_Engine::Blit_Surface(SDL_Rect position, const std::string& file_name, SDL_Rect* clip){	Throw_If_Unitialised();	bool successful = (SDL_BlitSurface(		surface_manager.Get_Surface(file_name),	clip, screen, &position) == 0);	if(!successful)	{ // Unsuccessful blit.		throw std::runtime_error("Surface failed to blit.");	}}void Draw_Engine::Blit_Surface(Point position, const std::string& file_name, SDL_Rect* clip){	Throw_If_Unitialised();	SDL_Rect pos = {position.x, position.y, 0, 0};	bool successful = (SDL_BlitSurface(		surface_manager.Get_Surface(file_name), clip, screen, &pos) == 0);	if(!successful)	{ // Unsuccessful blit.		throw std::runtime_error("Surface failed to blit.");	}}void Draw_Engine::New_Font(const std::string& file_name, int font_size){	Throw_If_Unitialised();	TTF_CloseFont(font);	font = TTF_OpenFont(file_name.c_str(), font_size);	if(font == NULL)	{		throw std::runtime_error(		std::string("Error: TTF_OpenFont failed: ") +		std::string(TTF_GetError()));	}}void Draw_Engine::Write(int x, int y, const std::string& text){	Throw_If_Unitialised();	text_sfc = TTF_RenderText_Solid(font, text.c_str(), font_colour);	if(text_sfc == NULL)	{		throw std::runtime_error(		std::string("Error: TTF_RenderText_Solid failed: ") +		std::string(TTF_GetError()));	}	else 	{		blit_position.x = x;		blit_position.y = y;		SDL_BlitSurface(text_sfc, NULL, screen, &blit_position);	}	// Always free the text.	Free_Surface(text_sfc);}void Draw_Engine::Write_Enhanced(const Enhanced_Write_Options& ewo){	Throw_If_Unitialised();	bool font_changed = false;	if(ewo.font_size != font_size)	{		font_changed = true;		New_Font(!ewo.font_name.empty() ? ewo.font_name : font_name,			ewo.font_size < 1 ? (int)(ewo.bounds.h * AUTOFIT_ADJ) : ewo.font_size);	}		text_sfc = TTF_RenderText_Solid(font, ewo.text.c_str(), ewo.colour);	if(text_sfc == NULL)	{		throw std::runtime_error(std::string(			"Error: TTF_RenderText_Solid failed: ") + std::string(TTF_GetError()));	}	else	{		int x = 0, y = 0;		switch(ewo.justification)		{		case JUSTIFY_CENTRED:			x = ewo.bounds.x + (ewo.bounds.w/2) - (text_sfc->w/2);			y = ewo.bounds.y + (ewo.bounds.h/2) - (text_sfc->h/2);			break;		case JUSTIFY_LEFT:			x = ewo.bounds.x + ewo.left_pad;			y = ewo.bounds.y + (ewo.bounds.h/2) - (text_sfc->h/2);			break;		case JUSTIFY_RIGHT:			x = (ewo.bounds.x + ewo.bounds.w) - (text_sfc->w + ewo.right_pad);			y = ewo.bounds.y + (ewo.bounds.h/2) - (text_sfc->h/2);			break;		case JUSTIFY_NONE:			x = ewo.bounds.x;			y = ewo.bounds.y;		}		blit_position.x = x;		blit_position.y = y;		SDL_BlitSurface(text_sfc, NULL, screen, &blit_position);	}	Free_Surface(text_sfc);	if(font_changed)	{ // Change font back to previous.		New_Font(font_name, font_size);	}}void Draw_Engine::Draw_Rect(SDL_Rect position, SDL_Color colour){	Throw_If_Unitialised();	if(SDL_FillRect(screen, &position, SDL_MapRGB(	screen->format, colour.r, colour.g, colour.b)) != 0)	{		throw std::runtime_error(			"Error: SDL_FillRect failed: " + std::string(SDL_GetError()));	}}void Draw_Engine::Draw_Screen(){	Throw_If_Unitialised();	if(SDL_Flip(screen) != 0)	{ // Bad draw.		throw std::runtime_error(			"Couldn't draw screen: " + std::string(SDL_GetError()));	}}void Draw_Engine::Free_Surface(SDL_Surface*& surface){	Throw_If_Unitialised();	SDL_FreeSurface(surface);	surface = NULL;}void Draw_Engine::Load_Surface(const std::string& file_name, SDL_Colour* colour_key){	Throw_If_Unitialised();	if(file_name.empty())	{ // Don't issue errors for empty file names.		return;	}		std::string fn = Directory::abs_path(file_name);	SDL_Surface* loaded = NULL;	SDL_Surface* optimized = NULL;	Uint32 map_colour_key = 0;	loaded = IMG_Load(fn.c_str());	if(loaded == NULL && !fn.empty())	{		throw std::runtime_error("Surface at: " + fn + " failed to load."+			SDL_GetError());		std::cout << SDL_GetError() << std::endl;	}	optimized = SDL_DisplayFormat(loaded);	Free_Surface(loaded);	if(optimized == NULL)	{		throw std::runtime_error("Surface at: " + fn + " failed to optimise.");	}	if(colour_key != NULL)	{ // The surface uses a colour key.		// Create the colour key.		map_colour_key = SDL_MapRGB(optimized->format, colour_key->r, colour_key->g, colour_key->b);		// Sets the colour key to the Surface.		SDL_SetColorKey(optimized, SDL_RLEACCEL | SDL_SRCCOLORKEY, map_colour_key);	}	// Surface optimized and loaded successfully; add to surface manager.	surface_manager.Add_Surface(file_name, optimized);}void Draw_Engine::Throw_If_Unitialised(){	if(!initialised)	{		throw std::runtime_error("Draw_Engine not initialised.");	}}Draw_Engine::Surface_Manager::Surface_Manager(){}Draw_Engine::Surface_Manager::~Surface_Manager(){		for(surface_it it = loaded_surfaces.begin();	it != loaded_surfaces.end(); ++it)	{ // Free each surface.		Free_Surface(it->second);	}}void Draw_Engine::Surface_Manager::Add_Surface(const std::string& file_name, SDL_Surface* surface){	if(!Surface_Exists(file_name))	{ // Surface doesn't exist; add.		loaded_surfaces.insert(std::make_pair(file_name, surface));	}}bool Draw_Engine::Surface_Manager::Surface_Exists(const std::string& file_name) const{	return loaded_surfaces.find(file_name) != loaded_surfaces.end();}SDL_Surface* Draw_Engine::Surface_Manager::Get_Surface(const std::string& file_name){	if(!Surface_Exists(file_name))	{ // Surface doesn't exist.		throw std::runtime_error(std::string("Surface_Manager::Get_Surface: ") +			"\"" + file_name + "\" does not exist.");	}	return loaded_surfaces[file_name];}

And then, when an object wants to load a surface:
void XImage::Load(){	Draw_Engine::Load_Surface(		file_name, is_colour_key ? &colour_key : NULL);}void XImage::Draw(){	Draw_Engine::Blit_Surface(position, file_name, NULL);}


What do you guys think? :)

I wouldn't be a fan. First of all, there is no such thing as a "static class" in C++. You either use a class, if you are going to instantiate objects, or a namespace.

Your Draw_Engine class tries to do too much. You should try separate different responsibilities into different classes. Here is a small example (it doesn't expand to cover everything your classes do yet):

class Renderer : boost::noncopyable{public:    Renderer(unsigned width, unsigned height, unsigned bpp, bool fullscreen)    {        SDL_Init(SDL_INIT_VIDEO);        screen = SDL_SetVideoMode(width,height,bpp,fullscreen ? SDL_FULLSCREEN : SDL_SWSURFACE );        if(!screen)        {            throw std::runtime_error(SDL_GetError());        }    }    ~Renderer()    {        SDL_Quit();    }    void draw(SDL_Surface *surface, int x, int y)    {        SDL_Rect dest = {x,y};        SDL_BlitSurface(surface,0,screen,&dest);    }    void drawRect(const SDL_Rect &rect, Uint32 colour)    {        SDL_Rect copy = rect;        SDL_FillRect(screen,©,colour);    }    void render()    {        SDL_Flip(screen);    }    unsigned width() const    {        return screen->w;    }    unsigned height() const    {        return screen->h;    }private:    SDL_Surface *screen;};class Image{public:    Image(const std::string &filename)    {        surface = IMG_Load(filename.c_str());        if(!surface)        {            throw std::runtime_error(SDL_GetError());        }    }    ~Image()    {        SDL_FreeSurface(surface);    }    void draw(Renderer &renderer, int x, int y)    {        renderer.draw(surface,x,y);    }    unsigned width() const    {        return surface->w;    }    unsigned height() const    {        return surface->h;    }private:    SDL_Surface *surface;};class ImageCache{public:    typedef boost::shared_ptr<Image> ImagePtr;    typedef std::map<std::string,ImagePtr> Cache;    ImagePtr load( const std::string & filename )    {        Cache::iterator it = cached.find(filename);        if( it == cached.end() )        {            // note: the Image constructor could throw an exception            // to handle "couldn't load image" error.            ImagePtr image( new Image(filename) );            cached.insert(std::make_pair(filename,image));            return image;        }        return it->second;    }private:    Cache cached;};// sample program, bounces an image aroundint main( int argc, char* argv[]){    try    {        Renderer renderer(1024,768,32,true);        ImageCache cache;        typedef boost::shared_ptr<Image> ImagePtr;        ImagePtr image = cache.load("image.bmp");        int x = 0, y = 0;        int dx = 1, dy = 1;        SDL_Event event;        const SDL_Rect full = {0,0,renderer.width(), renderer.height()};        bool running = true;        while(running)        {            while(SDL_PollEvent(&event))            {                if(event.type == SDL_QUIT)                {                    running = false;                }                else if(event.type == SDL_KEYDOWN && event.key.keysym.sym == SDLK_ESCAPE)                {                    running = false;                }            }            renderer.drawRect(full,0);            image->draw(renderer,x,y);            x += dx;            y += dy;            if( x + image->width() > renderer.width() || x < 0 )            {                dx = -dx;            }            if( y + image->height() > renderer.height() || y < 0 )            {                dy = -dy;            }            renderer.render();        }    }    catch(const std::exception &e)    {        // handle error    }    return 0;}

Notice the lack of global or static variables.
Why not? I've been thinking about this for a while... what's wrong with having a class with all static member functions/data? It allows members to be private (namespace won't) and isn't as messy as a global.

I've taken your advice and made a Surface_Cache class and a Renderer class. XImage for example now has a Draw function that takes a Renderer as a parameter.. the only problem is with the >>operator... I can't pass the Surface_Cache to that... so I have to make the decision of global vs static.... which leads me back to my question above! :)

Cheers for your help!

[Edited by - Mybowlcut on July 22, 2008 6:14:31 AM]

This topic is closed to new replies.

Advertisement