help with SDL Surfaces

Started by
7 comments, last by Storyyeller 15 years, 1 month ago
I am trying to figure out how to create a system to manage all of the different images used in my program and load and unload them when appropriate, but I am very confused. Currently, I have a system that sort of works, but I am at a loss as to how to make it support colorkeys, alpha, etc. It sounds like I'd need to have multiple copies of each image, just for colorkeys. Also, is the refs variable in my imgrecord struct actually necessary? Based on http://sdl.beuc.net/sdl.wiki/SDL_Surface, it looks like SDL Surfaces already come with a variable intended for that purpose. Can I use it instead, and is this advisable? Do I have to worry about memory management? Do I have memory leaks? I guess I am basically asking, what is the best way to do what I am trying to do? Here is what I have so far imagemanager.h

#ifndef IMAGEMANAGER_H
#define IMAGEMANAGER_H

#include "SDL/SDL.h"
#include <string>
#include <vector>

struct imgrecord
{
    std::string name;
    //int r,g,b;
    SDL_Surface* ptr;
    int refs;
};

//////////////////////////////////////////////////////////////

class imageManager
{
    static std::vector<imgrecord> images;

    static SDL_Surface* loadfrombmp( std::string );

    public:
    static SDL_Surface* demand( std::string);
    static void release( std:: string);
};

#endif


imagemanager.cpp

#include "imagemanager.h"

std::vector<imgrecord> imageManager::images; //declaring static data members

SDL_Surface *imageManager::loadfrombmp(std::string imgname)
{
    SDL_Surface * loadedimage;
    SDL_Surface * displayimage;

    loadedimage = SDL_LoadBMP( imgname.c_str() );

    if (loadedimage == NULL)
    {
        return NULL;
    }

    displayimage = SDL_DisplayFormat( loadedimage ); //converts the surface to a form
                                                     //that is 'suitable for fast blitting'
    SDL_FreeSurface (loadedimage);

    return displayimage;
}

SDL_Surface *imageManager::demand(std::string imgname)
{
    imgrecord newrecord;

    for (int i=0;i< images.size();i++)
    {
        if (imgname == images.name)
        {
            images.at(i).refs++;
            return images.at(i).ptr;
        }
    }

    newrecord.ptr = loadfrombmp( imgname);
    if (newrecord.ptr != NULL)
    {
        newrecord.refs=1;
        newrecord.name= imgname;
        images.push_back(newrecord);
        return newrecord.ptr;
    }

    return NULL;
}

void imageManager::release(std::string imgname)
{
    imgrecord newrecord;
    int i;

    for (i=0;i<images.size();i++)
    {
        if (imgname == images.at(i).name)
        {
            images.at(i).refs--;
            break;
        }
    }
    if (images.at(i).refs<=0)
    {
        SDL_FreeSurface( images.at(i).ptr );
    }
    images.erase(images.begin() + i);
}


I trust exceptions about as far as I can throw them.
Advertisement
You might find some ideas towards the end of this recent thread.
I've been trying to use rip-off's immutable image class, but I'm still confused

For example, what the heck does this mean? How can you reference a prototype? What is going on here?
    Image(const std::string &file, const SDL_Color &colourkey);

I trust exceptions about as far as I can throw them.
It is a constructor declaration (one of many overloads IIRC). The constructor takes two parameters - a std::string instance by const reference and a SDL_Color instance by const reference.

(This next part sounds nasty, but I mean it nicely) Which part aren't you familiar with?

In any case, I think I prefer the final iteration best. Its feature set is (IIRC):
  • Implemented as a shared_ptr to a SDL_Surface "under the hood", reducing memory usage. (Shallow) copying of images is cheap.

  • A separate "alphakey" type (maybe not the best name, something like "Transparency Options/Settings" might be clearer), again reducing the number of surfaces in memory. Also allows modulating the alpha value of a sprite over time, which is a nice feature.

  • Surface caching transparently handled by the class.

This implementation has pros and cons, but it is simple and works, which is a pretty big pro.
I'm confused by the const references. Are they pointers or what? How would you call it?
I trust exceptions about as far as I can throw them.
Quote:Original post by Storyyeller
I'm confused by the const references. Are they pointers or what? How would you call it?
Suggested reading. Also this.
OK, I think I have the class working, but the next issue I ran up against is how to get Boost shared_ptrs working. I tried downloading it, but the whole thing was over 200MBs, which is a bit big to fit on a flash drive.
I thought about only including the shared_ptr.hpp file, but it includes lots of other files, and I wasn't sure what to do about that.

I'm beginning to think that it would just be easier to do the reference counting myself!
I trust exceptions about as far as I can throw them.
I've done it before. Its pretty simple, just #include <boost/shared_ptr.hpp> and keep following the errors and adding the headers it is looking for until it compiles cleanly. It seems like a lot of files at first, but it will take less time and be less buggy than rolling your own. Plus it is more flexible (are you going to implement weak pointers, custom deletion function support etc?).
Thanks for the help. I've got everything working now, although I haven't tried to implement colorkeys or alpha yet.

One concern I had is that the surfaces are never freed because my imagemanager class holds on to a copy of the pointer in its map. How could I fix this? Do I need to use weak pointers or something? Is this even something I need to worry about?

Here is what my code looks like now

imagemanager.h
#ifndef IMAGEMANAGER_H#define IMAGEMANAGER_H#include "SDL/SDL.h"#include "shared_ptr.hpp"#include <string>#include <map>class Image;typedef boost::shared_ptr<Image> img_sptr;class Image{    SDL_Surface *mysurface;    // Only implement these if you really need to.    // Most of the time you don't want to be copying SDL_Surfaces,    // it is expensive    // Better to simple use boost::shared_ptr<Image>    //    // Declare them here, private and don't implement them    // This means that the compiler won't generate its defaults    Image(const Image &other);    Image &operator=(const Image &other);    // You should only need to store the surface pointer here    // Other state information like the per-surface alpha and    // colour key can be obtained from surface->format IIRC.    public:    // you could have another constructor that takes a SDL_RWops    // if you want to make Image instances from memory data.    //    //    Image(const std::string &file);    //not implemented yet        //Image(const std::string &file, const SDL_Color &colorkey);        //Image(const std::string &file, Uint8 alpha);    // This class is not intended to be subclassed.    // This means you can avoid having a virtual destructor    ~Image();    // You can have as many overloads as you like    // For example, to perform clipped blits you could have an overload    // that takes a SDL_Rect parameter.    // These should be marked as const, but I'm not sure if    // SDL is const-correct with surface blitting due to internal    // issues, but try to do so if you can.    void draw(int x, int y, SDL_Surface *screen, SDL_Rect *clip = NULL) const;    // Don't bother providing set/get SDL_Surface, I don't think you need it    // Best not to provide functions like setColorKey or setAlpha at all    // if you can avoid it. See below for details.};//////////////////////////////////////////////////////////////class imageManager{    static std::map<std::string, img_sptr > mymap;    public:    static img_sptr demand( std::string);};#endif


imagemanager.cpp
#include "imagemanager.h"#include <iostream>Image::Image(const std::string &file){    SDL_Surface * loadedimage;    SDL_Surface * displayimage;    loadedimage = SDL_LoadBMP( file.c_str() );    if (loadedimage != NULL)    {        mysurface = SDL_DisplayFormat( loadedimage ); //converts the surface to a form                                                      //that is "suitable for fast blitting"        std::cout << file << " loaded sucessfully\n";    }    else    {        mysurface = NULL;    }}Image::~Image(){    std::cout << "An image was deconstructed\n";    SDL_FreeSurface (mysurface);}void Image::draw(int x, int y, SDL_Surface *screen, SDL_Rect *clip) const{    SDL_Rect temp;    temp.x=x;    temp.y=y;    if (screen == NULL)    {        std::cout << "Screen is NULL\n";    }    if (mysurface == NULL)    {        std::cout << "MySurface is NULL\n";    }    std::cout << "Draw begun ...\n";    std::cout << "Mysurface is " << mysurface << "\n";    std::cout << SDL_BlitSurface(mysurface, clip, screen, &temp) << "\n";    std::cout << "Draw finished\n";}std::map<std::string, img_sptr > imageManager::mymap;img_sptr imageManager::demand(std::string imgname){    std::map<std::string, img_sptr >::iterator iter;    img_sptr newpointer;    std::cout << imgname << " demanded.\n";    iter = mymap.find(imgname);    if (iter == mymap.end())    {        std::cout << "Demand not in map\n";        newpointer.reset( new Image( imgname ));        mymap.insert( std::pair<std::string, img_sptr >( imgname, newpointer ) );        return newpointer;    }    else    {        std::cout << "Demand in map\n";        return iter->second;    }}
I trust exceptions about as far as I can throw them.

This topic is closed to new replies.

Advertisement